RFC: Separate signing

[woju]

This is a proposal for split signing enclaves.

Definitions

• rendering: processing jinja+toml to toml only
• trusted file expansion: recursive walk through directories to pick regular files inside them
• trusted file measurement: calculating sha256 over the regular file
• TBSSIGSTRUCT: SIGSTRUCT to be signed with RSA
• SIGSTRUCT: after signing

Current status

There are two tools: graphene-manifest, which only renders manifest templates, and graphene-sgx-sign, which expands and measures trusted files, then calculates MRENCLAVE (the inputs are: manifest from previous step and libpal.so), TBSSIGSTRUCT, signs it using provided key to SIGSTRUCT and exports .manifest.sgx (manifest after expansion and measurement) and .sig files (SIGSTRUCT). Those two files need to be provided when launching the enclave.

Problems

• production enclave signing key needs to be protected
• some trusted files might not be available and/or too big to be conveniently re-measured every time the manifest is processed
• there are scenarios where people need to recalculate MRENCLAVE themselves: when connecting to an enclave via RA-TLS, client needs to verify MRENCLAVE provided by the other end (enclave)
• last but not least, current manifest schema around trusted files is suboptimal, there is O(n²) processing complexity (SGX startup is slow due to quadratic TOML processing graphene#2593); this is related, because this affects the same manifest attribute

Proposed architecture

App developer would be able to add trusted files hashes to manifest template. Those pre-hashed files will be skipped when doing measurement.

Proposed changes

Manifest changes

New schema:

sgx.trusted_files = [
  {uri = "file:...", sha256 = "..."},
  "file:...",
]

Manifests will be renamed: app.toml.jinja (the template) and app.toml (after rendering and trusted files measurement).

CLI tools

• graphene-manifest will, after manifest rendering, expand and measure trusted files; files with pre-filled hashes will not be measured again
• graphene-sgx-sign will not expand nor measure trusted files, instead will raise an error if there are directories or missing hashes
• graphene-sgx-get-mrenclave new tool, will calculate MRENCLAVE based on manifest and libpal.so
• graphene-sgx-keygen new tool, will generate serviceable RSA-3072 key in proper place

The default key location will be in ~/.config/graphene/enclave-key.pem (really in $XDG_CONFIG_HOME).

Python API

The following functions will be documented:

• graphenelibos.manifest.render(template, ...)
• graphenelibos.manifest.expand_and_measure_trusted_files(manifest)
• graphenelibos.sgx_sign.get_mrenclave(manifest[, libpal])
• graphenelibos.sgx_sign.get_tbssigstruct(manifest[, mrenclave])
• graphenelibos.sgx_sign.sign(tbssigstruct, key)

Python implementation

• use cryptography (https://cryptography.io/) instead of subprocess.run('openssl') and hand-crafted RSA signature
• deprecate argparse, standardise on click (https://click.palletsprojects.com/)

Discussion

Advantages

• simplicity
• no (obvious) footguns

Drawbacks

• graphene-manifest becomes mandatory: it's technically possible to skip this tool currently, but even now all the examples already use it

Considered alternatives

Manifest merging

There is no convincing use case for this, which would not be better served by either protected_files or just copying the hash into manifest template. I might have backup proposal should there be a real need.

Unsolved problems

• manifest signing for submission to signing facility, and audit log
• in the basic setup, libpal.so is needed in signing facility, and parsed using binutils

Future work

• allowed_files, trusted_files and protected_files are poorly named, they should be renamed passthrough_files, measured_files and encrypted_files (not sure about the last one)

[woju]

@mkow @dimakuv @boryspoplawski

[dimakuv]

Manifest schema for trusted files is already changed: gramineproject/graphene#2644

graphene-manifest will, after manifest rendering, expand and measure trusted files; files with pre-filled hashes will not be measured again

I generally agree, but we need to do something about naming. Users could theoretically use Graphene for say both Intel SGX-related sgx.trusted_files and Intel TDX-related tdx.super_trusted_files. In such case, do we put all this functionality in one graphene-manifest tool? Or should we have something like graphene-sgx-manifest, graphene-tdx-manifest, etc?

in the basic setup, libpal.so is needed in signing facility, and parsed using binutils

Why is libpal.so needed at all? I understand that this is artifact of our current implementation, but why can't libpal.so be just like any other file -- declared as sgx.trusted_files (maybe not explicitly but added during graphene-manifest, just like we do with libsysdb.so)? I would vote for adding all binaries as sgx.trusted_files.

For libsysdb.so implicit addition, please check https://github.com/oscarlab/graphene/blob/master/python/graphenelibos/sgx_sign.py#L177-L180 and ask @mkow (he implemented this trick, iirc).

allowed_files, trusted_files and protected_files are poorly named, they should be renamed passthrough_files, measured_files and encrypted_files (not sure about the last one)

I like all these names, but we should allow legacy names too. So we will have aliases to each of these three types. Otherwise users will kill us.

[woju]

In such case, do we put all this functionality in one graphene-manifest tool?

Yes, why not? If there are other measurement schemes/requirements, all that expansion/measurement things could be added. (I'm not familiar with TDX, so I might have missed something important.)

Why is libpal.so needed at all?

Because libpal.so is our memory layout just after enclave loading (plus manifest), right? So it's not sufficient to have just SHA256, we need to know how it loads. So if you really don't like it, we could come up with some equivalent, like parsing it in g-manifest and encoding the layout and contents this in the manifest, but still, this file is unique and doesn't fit trusted_files. It doesn't even need to be available in enclave through any _files.

https://github.com/oscarlab/graphene/blob/35de392aa5c3db52d7c7cc074cb83537c146e86f/python/graphenelibos/sgx_sign.py#L514-L538
https://github.com/oscarlab/graphene/blob/35de392aa5c3db52d7c7cc074cb83537c146e86f/python/graphenelibos/sgx_sign.py#L210-L244

I like all these names, but we should allow legacy names too.

Yeah, that's future work, not now. @mkow thinks (sometimes loudly) about moving this *_files to mounts, so that could be a convenient opportunity to also change the keys, if ever.

[AI-Memory]

It looks good to me, however, there is another PR#2596, that refactors the sign script for adding 2-step signing feature, it is one of major CSP requirements, could you please allow that PR to be accepted ? thanks.

[woju]

Could you please describe here what you understand as "2-step signing feature"? How this should exactly work?

[monavij]

@woju I think Gary is pointing out that PR#2596 touches many of the same files and he has already refactored the code. Since Dmitrii has already reviewed his PR, maybe close that first and then build on top of that? Otherwise all that effort goes wasted.

[woju]

On Thu, Aug 12, 2021 at 01:52:07PM -0700, Mona Vij wrote: @woju I think Gary is pointing out that PR#2596 touches many of the same files and he has already refactored the code. Since Dmitrii has already reveiwed his PR, maybe close that first and then build on top of that? Otherwise all that effort goes wated.

Yes, that's correct, this is overlapping effort, as was the change of trusted_files schema. That's why I put this RFC, because we suddenly had mutliple proposals affecting the same code and manifest data, and without coordination, it's possible more effort will be wasted. Sure I can go review his PR, but the problem is, that PR is refactoring for refactoring, and until the RFC reaches consensus, this is just going in circles.

[monavij]

all right I understand now. Let's not keep going in circles then and waste more effort. Now I remember that you had proposed last week to continue this discussion via this issue. Let's continue reviewing this here as well as in our meeting next week, so we can reach consensus and make sure it works for various different use cases and requirements.

[AI-Memory]

@woju The 2 step signing feature has been presented by @ying2liu in community meeting, her PR is #2528, we got the feedback to refactor the sign code first (PR gramineproject/graphene#2596) and then to apply her PR on it. we already processed it before this RFC presented, the 2-step signing is a feature for production-ready, not nice-to-have one because our customers explicitly asked for it, thanks.

[mkow]

Comments to the RFC

The developer signature

Normally I'd say that this is completely out of scope for Graphene and this should be implemented by the developer's infra (e.g. through some policies/ACLs on the RPC), but I also know that if we won't force people to sign the signing requests then they will just make their signing servers sign any blob they receive...

MRENCLAVE verification

One thing is tricky here - how can the user easily verify if the manifest should be trusted? (suppose that they know the specific claimed Graphene version used + the manifest) The configuration options can be easily checked manually, but for trusted files (which include execs and dynamic libraries!) there's no easy-to-use tool to verify them, the user just sees some hardcoded hashes.
One solution I thought of was to allow the user of this tool to specify some of the trusted files for hash verification. The problem with this solution is that if it would be a part of graphene-sgx-get-mrenclave, then users would skip manually verifying other manifest entries and just check if the tool said that the files looks good, missing all other manifest entries...

btw. I think it should also explicitly print out all insecure options present in the manifest. Probably same for graphene-sgx-sign.

I'd add this to the "Unsolved problems" section.

TBSSIGSTRUCT

I'd prefer more descriptive name (especially that it occurs in the API), now I can guess what this "TB" means (I think), but without this whole context I probably wouldn't. Maybe UNSIGNED_SIGSTRUCT?

+1 to all other proposed changes.

Comments to comments

I generally agree, but we need to do something about naming. Users could theoretically use Graphene for say both Intel SGX-related sgx.trusted_files and Intel TDX-related tdx.super_trusted_files. In such case, do we put all this functionality in one graphene-manifest tool? Or should we have something like graphene-sgx-manifest, graphene-tdx-manifest, etc?

Why would we need different filesystems in different PALs? I actually thought that we should aim to move all of this to the PAL common and unify PALs behavior in this aspect.

@woju The 2 step signing feature has been presented by @ying2liu in community meeting, her PR is #2528, we got the feedback to refactor the sign code first (PR gramineproject/graphene#2596) and then to apply her PR on it.

ad gramineproject/graphene#2528: (two-step signing by @ying2liu)
Yup, it was presented, but at least me and woju weren't impressed by it (we disputed it on the call and woju even said this explicitly under gramineproject/graphene#2528). But the biggest problem which stopped gramineproject/graphene#2528 from going to master is its bad quality, it looks more like a quick hack to make it work than something which should go to master (in terms of both code and PR description/commit message).

ad gramineproject/graphene#2596: (sgx_sign.py refactoring by @bigdata-memory)
If I understand woju correctly, the refactoring you did doesn't help with the design proposed here, we'll need to change everything again soon after.

we already processed it before this RFC presented, [...]

It's not the-first-the-better competition, it doesn't matter who was first to propose something. It matters which design is better and we'll make our decision according to this criterion. We often work on some implementation and then notice that it's not good and we just don't merge it.

the 2-step signing is a feature for production-ready, not nice-to-have one because our customers explicitly asked for it, thanks.

If they need this feature ASAP then you can just point them to your implementation.

[ying2liu]

gramineproject/graphene#2528 two-step signing implementation is based on the current signing script without changing the code structure. I didn't want to trigger debates because of the structure changes, because we have several customers are waiting for this feature for their next releases. If you have any additional comments on code and PR description, please let me know. I will update the patch accordingly.

[dimakuv]

In such case, do we put all this functionality in one graphene-manifest tool?

Yes, why not? If there are other measurement schemes/requirements, all that expansion/measurement things could be added. (I'm not familiar with TDX, so I might have missed something important.)

Ok, fair enough. Sounds like overloading graphene-manifest with several unrelated features, but there won't be many divergences, I guess. And as Michal mentioned, we will probably even make manifest option names more generic, e.g. instead of sgx.enclave_size we will have something like enclave.size (or trusted.size).

[woju]

On Mon, Aug 16, 2021 at 05:19:56AM -0700, Dmitrii Kuvaiskii wrote: Ok, fair enough. Sounds like overloading `graphene-manifest` with several unrelated features, but there won't be many divergences, I guess. And as Michal mentioned, we will probably even make manifest option names more generic, e.g. instead of `sgx.enclave_size` we will have something like `enclave.size` (or `trusted.size`).

Yes, I think everything that counts as transforming manifest would be in-scope in `graphene-manifest`. I could use some good advice where to make a red line, but at least I think this is better than transforming manfest in `graphene-manifest` (jinja render) and then transforming it again in `graphene-sgx-sign` (sgx.trusted_files expansion and measurement). Also this is more aligned to architecture with separate signing, `graphene-manifest` is run by the developer and `graphene-sgx-sign` (or equivalent service) would be run in singing facility.

[AI-Memory]

I don't think the dev. signing should be bound to this process, it adds another layer of complexity while being coupled with dev. private key, the signing tool just need to produce the signing material enclave_sig.dat to be signed in signing facility, without that, the signing part might be similar to @ying2liu's PR. Other features your proposed, e.g. pre-hash, could be separated addons to 2-steps signing PR. thanks.

[woju]

@ying2liu I've read https://01.org/sites/default/files/documentation/intel_sgx_sdk_developer_reference_for_linux_os_pdf.pdf pages 14-20. I understand that we already generate correct structure, and this proposal already states that we should expose a Python function grapenelibos.sgx_sign.get_tbssigstruct to generate that. (TBS SIGSTRUCT, to-be-signed SIGSTRUCT, is a name I invented to distinguish from already signed SIGSTRUCT; it's borrowed from X.509 nomenclature).

I don't think we should create a CLI tool to export this structure, because this would invite insecure architectures to be deployed. In a valid use case, this signing data should be signed with some auxiliary scheme to be submitted to the signing facility and the auxiliary signature should be verified there. The example in SDK documentation omits this critical step, so we will NOT support the exact workflow described there. Instead, we'll ship API to get the SIGSTRUCT from manifest, and to deliver a complete solution the integrator will need to implement verification of material submitted to signing facility, which could be either manifest.sgx or SIGSTRUCT.

In short, because we don't ship this submission signing and verification, as the implementation will inherently be ISV-specific, we also won't ship the finished CLI tool, only API to be used in such tool.

[ying2liu]

Please note, two-step signing script includes: 1. The first-step creates the signing material (the users could decide how to send the material to the signing facility and use whatever secure scheme they want). 2. the second-step does verify the signature file before signing the enclave. This verification is done in the graphene signing script to ensure that the material is not compromised(if any bad thing happens after the first step, it could be caught here). This details are transparent to the user, which is the reason why the verification is not shown in the command line examples. SGX signing tool based on SDK also uses similar approach, which has been widely used in different industries. Thanks Ying From: Wojtek Porczyk ***@***.***> Sent: Tuesday, August 17, 2021 11:58 AM To: oscarlab/graphene ***@***.***> Cc: Liu, Ying2 ***@***.***>; Mention ***@***.***> Subject: Re: [oscarlab/graphene] RFC: Separate signing (#2642) @ying2liu<https://github.com/ying2liu> I've read https://01.org/sites/default/files/documentation/intel_sgx_sdk_developer_reference_for_linux_os_pdf.pdf pages 14-20. I understand that we already generate correct structure, and this proposal already states that we should expose a Python function grapenelibos.sgx_sign.get_tbssigstruct to generate that. (TBS SIGSTRUCT, to-be-signed SIGSTRUCT, is a name I invented to distinguish from already signed SIGSTRUCT; it's borrowed from X.509 nomenclature). I don't think we should create a CLI tool to export this structure, because this would invite insecure architectures to be deployed. In a valid use case, this signing data should be signed with some auxiliary scheme to be submitted to the signing facility and the auxiliary signature should be verified there. The example in SDK documentation omits this critical step, so we will NOT support the exact workflow described there. Instead, we'll ship API to get the SIGSTRUCT from manifest, and to deliver a complete solution the integrator will need to implement verification of material submitted to signing facility, which could be either manifest.sgx or SIGSTRUCT. In short, because we don't ship this submission signing and verification, as the implementation will inherently be ISV-specific, we also won't ship the finished CLI tool, only API to be used in such tool. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://github.com/oscarlab/graphene/issues/2642#issuecomment-900552847>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIUG3QXITVWWL2WAY6DI2BLT5KWKVANCNFSM5B4UA7OQ>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>.

[monavij]

@ying2liu Woju is talking about verifying that the requester is authorized to get something signed by the signing facility. In production signing facilities, we presume that there will be some sort of authorization that happens. If we don't make them stop and think about this step, then they may allow anyone who can access the signing facility get signed. I believe we can build a small wrapper on top of APIs that woju is proposing. Let's discuss this offline and see if it can meet the use case you are looking at.

[woju]

Please note, two-step signing script includes: 1. The first-step creates the signing material (the users could decide how to send the material to the signing facility and use whatever secure scheme they want). 2. the second-step does verify the signature file before signing the enclave. This verification is done in the graphene signing script to ensure that the material is not compromised(if any bad thing happens after the first step, it could be caught here). This details are transparent to the user, which is the reason why the verification is not shown in the command line examples. SGX signing tool based on SDK also uses similar approach, which has been widely used in different industries.

The verification of signature in both the SDK tool and gramineproject/graphene#2528 is only a sanity check against misapplication of the signature from a different enclave build and could be skipped entirely without consequences to the overall security model (if something goes wrong there, CPU will just refuse to execute the enclave). But if somebody wanted this feature, as a test in the build pipeline, I have nothing against shipping another tool (say graphene-sgx-verify-sigstruct) to verify our standalone .sig files.

But @monavij is right, this is not what I meant when talking about auxiliary signatures over the TBSSIGSTRUCT for sending to signing facility.

[AI-Memory]

Based on our discussion and review, this proposal contains some implicit requirements to signing facility and it looks not compatible with the protocol which described in SGX Developer Reference P20, I don't think the GSGX can take control over our customer's signing facility for just serving GSGX only. thanks.

[AI-Memory]

Please refer to the two-steps signing feature request gramineproject/graphene#2617 as well, you can find an attached screenshot over there. thanks.

[ying2liu]

@vij, ***@***.***> Graphene does not have any control over the signing facility. Users could have their own secure scheme, but it is not part of the two-step signing process. “ The ISV takes the enclave signing material file to an external signing platform/facility where the private key is stored, signs the signing material file, and takes the resulting signature file back to the build platform.” The two-step signing process is clearly defined in https://download.01.org/intel-sgx/linux-2.1.3/docs/Intel_SGX_Developer_Reference_Linux_2.1.3_Open_Source.pdf (page 19), which Intel expects the developers to follow. If we find any mistake in Intel SDK, we should raise this issue to Intel instead of changing the process by ourselves. From: Mona Vij ***@***.***> Sent: Tuesday, August 17, 2021 11:08 PM To: oscarlab/graphene ***@***.***> Cc: Liu, Ying2 ***@***.***>; Mention ***@***.***> Subject: Re: [oscarlab/graphene] RFC: Separate signing (#2642) @ying2liu<https://github.com/ying2liu> Woju is talking about verifying that the requester is authorized to get something signed by the signing facility. In production signing facilities, we presume that there will be some sort of authorization that happens. If we don't make them stop and think about this step, then they may allow anyone who can access the signing facility get signed. I believe we can build a small wrapper on top of APIs that woju is proposing. Let's discuss this offline and see if it can meet the use case you are looking at. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://github.com/oscarlab/graphene/issues/2642#issuecomment-900837618>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIUG3QWDMRCCWZXFCADQX4DT5NE2ZANCNFSM5B4UA7OQ>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>.

[boryspoplawski]

After merging #72 TODOs left:

• "Manifests will be renamed: app.toml.jinja (the template) and app.toml (after rendering and trusted files measurement)."
• "graphene-sgx-get-mrenclave new tool, will calculate MRENCLAVE based on manifest and libpal.so" - this needs rethinking, as mrenclave is not enough. Probably we will just add a tool to generate and print some subset of TBSSIGSTRUCT (let's call it EnclaveIdentity or something).
• "graphene-sgx-keygen new tool, will generate serviceable RSA-3072 key in proper place"
• "The default key location will be in ~/.config/graphene/enclave-key.pem (really in $XDG_CONFIG_HOME)." - idk about this one, also my system does not have XDG_CONFIG_HOME set.
• "use cryptography (https://cryptography.io/) instead of subprocess.run('openssl') and hand-crafted RSA signature"
• remove all prints from python library; add some to our script wrappers: gramine-sgx-sign etc (probably behind -v/--verbose flag)
• document new python API (mostly done, but we need to render all classmethods in readthedocs or just document them)

[dimakuv]

@boryspoplawski I think at least "Document new Python API" todo item is done? Please mark which items are done (and which items should not be implemented).