graphene icon indicating copy to clipboard operation
graphene copied to clipboard

Redesign manifest

Open woju opened this issue 4 years ago • 12 comments

This is split from a discussion in #2050. We should rework manifest specification, to have it more clear, and possibly take advantage of TOML structures (e.g. list), which were unavailable in previous manifest syntax.

Ideas:

  • decide on entrypoints ([libos.]entrypoint, pal.entrypoint/loader.preload)
  • sgx.trusted_files = []
  • [[fs.mount]]
  • move all insecure to top-level insecure.* ? (IDK if this is good idea, but that would aid auditing: all insecure things would have to be gathered together)
  • ?
entrypoint = '...' # ?

[libos]
entrypoint = '...' # ?

[pal]
entrypoint = '...'

[loader.env] # [pal.env]?
ENVVAR = 'value'

[[fs.mount]] # currently this would be [fs.mount.whatever1], [fs.mount.whatever2], ...
type = 'type1'
path = 'path1'
uri = 'uri1'

[[fs.mount]]
type = 'type2'
path = 'path2'
uri = 'uri2'

[sgx]
enclave_size = "..."
trusted_files = [
    'file:file1',
    'file:file2',
]

woju avatar Jan 12 '21 13:01 woju

sgx.trusted_files is already a TOML table. You want it to be a TOML array, iiuc. But I actually prefer the current syntax.

In other words, you advocate smth like this:

sgx.trusted_files = ["filepath1", "filepath2"]

Whereas the current syntax also allows to have keys:

sgx.trusted_files.uniquefileid1 = "filepath1"
sgx.trusted_files.uniquefileid2 = "filepath2"

# or the same in another form

[sgx.trusted_files]
uniquefileid1 = "filepath1"
uniquefileid2 = "filepath2"

I find the current TOML table syntax quite nice. UPDATE: I take it back, I have no good technical rationale to prefer the old style.

dimakuv avatar Jan 12 '21 13:01 dimakuv

Some ideas/comments:

  • I definitely don't like all those keyed arrays in our current syntax, they can be just plain arrays (current syntax forces users to invent key names for each file). Also, they are verbose without (IMO) any advantages of this verbosity.
  • What do you think about using top-level tables? Something like:
    [pal]
    entrypoint = "asdfg"
    
    [pal.sgx]
    something_sgx_specific = "12312313"
    
    [libos]
    entrypoint = "qwerty"
    
    (taking aside the entrypoint discussion, this is just an example to show the idea)
  • We should use bools instead of 0/1 (AFAIR this was left temporarily to minimize the breaking changes done at once, but we want to change it).
  • Executable envs and args could very nicely fit into TOML syntax, instead of the keyed "arrays" we use right now.

mkow avatar Jan 12 '21 14:01 mkow

What do you think about using top-level tables?

All the above examples are already possible. We're just using a different format of presenting TOML tables (a "chained keys" format) in our example manifests.

dimakuv avatar Jan 12 '21 14:01 dimakuv

What for do you need the unique file id in the manifest?

woju avatar Jan 12 '21 16:01 woju

I guess I'm just used to it and use them as comments. E.g. I sometimes do

sgx.trusted_files.lib_needed_for_python_when_it_fails = "file:/usr/lib/python/pythonapt"

Well, yes, this doesn't really make sense :) I'm retracting my comment.

dimakuv avatar Jan 12 '21 16:01 dimakuv

Not strictly related to this issue, but another thing I wanted to do: change the file naming convention we use for manifests and actually include the correct extension (.toml). This will make all the tools recognize the syntax used inside, plus inform users that we use TOML (if they skipped reading docs).

mkow avatar Jan 13 '21 00:01 mkow

Not strictly related to this issue, but another thing I wanted to do: change the file naming convention we use for manifests and actually include the correct extension (.toml). This will make all the tools recognize the syntax used inside, plus inform users that we use TOML (if they skipped reading docs).

Agreed with Michal. We in general should fix this build process appname.manifest.template -> appname.manifest -> appname.manifest.sgx. The first stage is just replacement with sed... We must do something simpler and more understandable.

dimakuv avatar Jan 13 '21 08:01 dimakuv

Maybe we need some real templating solution in place of that sed hack, which is duplicated everywhere in Examples/. In theory we shouldn't really need this in production, because signed manifest will not be altered, but it could greatly help in preparing the manifest before signing (*cough* Examples/ *cough*). I think either m4 or jinja would be good. m4 is "standard", doesn't need python at all (in fact it's part of POSIX, so it's everywhere), but I believe jinja is more familiar to many people.

I'm slightly inclined to suggest m4, because this will make Examples/ "more example" than using jinja:

# m4
sgx.trusted_files.whateverlib = "PREFIX/lib/whatever.so.0"
# jinja2
sgx.trusted_files.whateverlib = "{{ prefix }}/lib/whatever.so.0"

though if token is to be joined to alphanumeric character, this might not be as good:

# m4: note `'
sgx.trusted_files.python = "/usr/bin/python`'PYTHON_VERSION"
# not m4, does not work:
sgx.trusted_files.python = "/usr/bin/pythonPYTHON_VERSION"
# jinja2:
sgx.trusted_files.python = "/usr/bin/python{{ python_version }}"

woju avatar Jan 13 '21 11:01 woju

I definitely prefer Jinja, nowadays it's much more popular and most people know it (and if not, the syntax is obvious and intuitive).

Anyways, the first step here would be not to change the syntax, but to change how we add trusted files. If we implemented an option to add a whole directory recursively, then a lot of places requiring templating would disappear.

mkow avatar Jan 13 '21 13:01 mkow

@mkow @woju Do you think we should still keep this issue open? We added "specifying whole directories in trusted files", and now our manifest templates look much cleaner (though still not TOML arrays but TOML tables for sgx files). We also changed from the sed hack to graphene-manifest.

dimakuv avatar Jul 22 '21 09:07 dimakuv

I'd keep it until we add the TOML arrays and basically use more TOML-ish constructions for everything.

mkow avatar Jul 22 '21 21:07 mkow

After #2593/#2644 fs.mount also should be an array and that would pretty much be it I think.

woju avatar Aug 11 '21 17:08 woju