-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Support for filesystem migration after SGX SVN update #855
Comments
For context, the gramine/pal/src/host/linux-sgx/enclave_framework.c Lines 241 to 270 in 90ea5a5
Note the I currently have no good ideas what to do about it. |
As I see it,
|
Yeah, exactly. Changing |
+1. This seems pretty hard to get right and easy to accidentally worsen security of the whole Gramine. We currently have a lot of pending design discussion and this one is only a quality-of-life improvement (not a security fix, and not really a new functionality), so IMO it should have low priority for now. Also, this feature isn't as straightforward as it seems and is IMO a bit controversial. If a specific version of an enclave was vulnerable, then using its state (i.e. its filesystem, in our case) in the patched version propagates the potentially attacker-controller state to the new one. What users should do is to redeploy the system from a clean state or from a trusted-server-verified state. |
How do you expect migration to work? If I am not mistaken, sealing keys depend on platform-specific secrets, so copying the protected filesystem data won't help you so much. Do you mean to let the developer define some logic that will be invoked in case a file with older SVN was opened? I think this is too complicated, because there may be too many possible cases and it might become too complicated very fast.
This is a problem, but I would expect the application to validate all inputs from filesystem (regardless of SGX). I don't think protected files are an exception to this rule.
Starting from scratch with the updated enclave sounds like the most secure option, but I think it's a very bad idea to tell developers they lose access to all their data when an SVN is incremented: it may lead to developers not incrementing their ISVSVNs even if vulnerabilities were fixed, which puts data created by the fixed enclave at risk. And to make things worse, note that CPU SVNs also affect the derived keys: the same enclave may be launched on the same platform, but it might lose access to its protected filesystem because of a platform update. In conclusion, it may be a controversial feature, but I think it might be just as controversial not to support loading keys with older SVNs, because access to protected file may be lost also in case of platform updates (which may be out of the enclave's control). I don't know if allowing explicit support for migration is the right way, because it may significantly complicate the developer's life. Therefore, I believe the most pragmatic approach is to support reading files with older SVNs, but to write data using the current enclave's SVNs (this prevents platform/enclave with older SVNs from reading the new data). |
I think we should, by default, support opening existing files, when writing we should always use the latest SVN. Especially for CPU SVN, as there is probably no way to go back. If there is no such mechanism, the user data stored in encrypted files may be lost forever after CPU SVN changes. Also in SGX SDK, the protected file format does record the cpu_svn, isv_svn and key_id in the plain part, and use them upon opening. So unless we explicitly document the opposite, users familiar with SGX may be surprised when they can't open existing files. |
@mkow - I think we have mixed two topics here:
I think it will be better to create a separate issue for key derivation APIs |
This is new to me. @lejunzhu Was it added rather recently (less than 2 years ago)? Because I don't see anything like this in the current Gramine format, and it was taken directly from SGX SDK: gramine/common/src/protected_files/protected_files_format.h Lines 31 to 37 in f7995b6
|
I created a separate issue for possible key derivation API changes: #857 |
I only read the code today and am not sure about the history. It looks like this now: And the usage of stored SVN: |
Yes, and new key generation should look something like this: Note that:
In addition to that, I think CONFIGSVN field should be with the same semantics as other SVNs (I wonder why I don't see it in SGX SDK). It will ease implementing KSS support in the future (perhaps the details should be discussed in #800 ). |
Reviving this 6-months-old thread. We hit this problem again, and need to solve it sooner than later. Let me summarize the problem:
The last two bullet items constitute the problem: there are only two "derive-from" sealing keys available in Gramine for any Encrypted File, and each of these sealing keys is tied to the current CPU, ISV and CONFIG numbers (as reported by This problem manifests in the following scenario:
SGX SDK solves this problem in a sub-optimal way, trading usability for security (see @mkow and @DL8 discussion on loading data from a previously-vulnerable enclave version):
This allows SGX SDK to solve the problematic scenario above: each Encrypted File simple saves all "version metadata" in its header, and SGX SDK reads this metadata first, generates the sealing key based on this metadata, and now the key can decrypt the file. Gramine can't do this easily, because Gramine doesn't support such generation of sealing keys on a per-file basis. I currently don't have a good proposal to allow old-version Encrypted Files in a new-version SGX enclave. It seems that the following properties are desired at a minimum:
|
But if the application is not specifically modified for Gramine, it may refuse to run if the file is r/o. Then the data will be lost if it is encrypted with MRENCLAVE, as the application cannot be modified when the user notice the upgrade. |
That is true. The alternative will be to allow writes as well, and print a warning (or have a manifest option like |
But changing the manifest will change MRENCLAVE as well... |
Warnings here are pointless IMO, if they appear then it's already too late to react to them, because in this scenario they appear only after running on the updated host and the only thing they'll do is hinting the untrusted host owner that the enclave is using old, potentially compromised files. |
MRENCLAVE is subject to change per build regardless of this issue. In general, if you need data to persist across different versions, you should use MRSIGNER-based keys
The easiest way I can see to support read-write files is to use the SVNs from the file to protect its data, but then new data has no rollback protection, which is insecure. Therefore, I think there is a missing requirement that writes always use the current SVNs. If I understand correctly, the motivation behind this requirement is to prevent a scenario where chunks with different SVNs need to written into the same file. If that's indeed the case:
Not allowing old user-controlled SVNs means users lose access to old files when ISV SVN is updated. This may lead developers not to increment ISV SVN when needed, which practically disables SGX's anti-rollback mechanism
Note that CPU SVN is a vector with several SVNs, so it may be quite cumbersome to handle (even without enclave SVNs). Unfortunately, even if SVNs are added to file metadata, such a guessing mechanism may be necessary to maintain compatibility with current protected filesystem format |
I'm thinking of the situation that the application user doesn't intend to change the software, but the underlying hardware (BIOS) upgrade makes the encrypted file unaccessible. Therefore it is important for the user to get the same access to the file, so that the availability of the software is not affected. That gives user time to evaluate the hardware upgrade and decide what to do next. Of course, sometimes the file integrity is so important that the user would rather lose the data than open a possibly compromised file, therefore we probably need an option in the manifest for the user to choose between these behaviors. |
Looking at this thread again, my current idea is this:
With this construction, Gramine will start the life of the application with some meaningful CPUSVN and ISV SVN numbers. After reboots, Gramine will pick up this metadata file and continue using these CPUSVN and ISV SVN numbers, thus solving our problem of migration/upgrade. Why don't we hard-code e.g. CPUSVN in the manifest file? This is because CPUSVN is an opaque 128-bit byte sequence; there is no spec on CPUSVN format. So we need to bootstrap CPUSVN from the processor itself, e.g. on the first graminized-app run. Why don't we introduce CPUSVN per-file, as Intel SGX SDK does? Because I believe this flexibility comes at a huge price of code complexity and worse usability. E.g., we currently have a single Why do we need HMAC? Because we want integrity protection of the metadata file, otherwise the attacker could simply replace CPUSVN with all-zeros, and then the file could be read by very old platform version. Why don't we encrypt the file? Because this is not necessary -- none of the fields need to be secret. We only need a guarantee that the read file was indeed produced by the application enclave. What is this specially-for-this-purpose MRENCLAVE key? It's a sealing key generated similarly to how we currently do it in Gramine, but with CPUSVN, ISVSVN, CONFIGID set to all-zeros, so that this file can be always read-and-verified on any version of the platform and software. This MRENCLAVE key will also have We probably need to add a This whole new logic should be hidden behind a new manifest option, for backwards compatibility. Maybe With this scheme, the attacker can (1) delete the metadata file at will, and (2) replace the newer version of the metadata file with the older version. The delete action will lead to Gramine not being able to decrypt the old encrypted files, because e.g. the CPUSVN version will be newer than that of the files. The replace action may lead to Gramine decrypting some really old files, but at least it will be limited by the very-first metadata file contents. 1 Maybe I should use the MRSIGNER key instead? |
@DL8 @lejunzhu @kailun-qin Does the above implementation make sense to you? |
A single, oldest CPUSVN is probably not desirable. When there is a security update, the old keys are potentially compromised. If we keep using the old keys to encrypt new data after upgrade, it will allow this kind of attack: 1. upgrade the platform, 2. run the application and get some new data with remote attestation, the verification result will be up-to-date. 3. make the application write the data to encrypted file, using the old key. 4. rollback the upgrade somehow and use the known exploit to retrieve the new data from the same app running on the CPU with old microcode.
|
I think @lejunzhu raised a good point (that we need the latest CPUSVN and some kind of migration process for the potential downgrade attacks) but it seems that the per-file record has to be introduced then (but as argued by @dimakuv, it comes at a huge price of code complexity and worse usability). Otherwise, as some random thoughts, will an offline migration tool (or a migration mode of Gramine, that only conduct the file migration for all encrypted files from a certain older CPUSVN to a newer one) help? |
Ok, looks like there is strong opposition to allowing older CPUSVNs. Then I will not pursue that approach. I think there is nothing else we can do but go with @lejunzhu's proposal:
There are two differences from what @lejunzhu proposed:
As @lejunzhu mentioned, this new migration logic can be gated by a new manifest sub-option: This has the benefit that Gramine still maintains in itself only a single sealing key (based on the latest CPUSVN), and generates "old" sealing keys only on demand when migration is required. I also believe that Gramine codebase has all the building blocks to implement such a feature. Should be doable with not-too-drastic modifications to Gramine. |
Re-encrypting the whole file is slower if the file is big. I think we only need to re-encrypt the encrypted metadata, which contains the mht_key. That is much faster. If the old subkeys are (potentially) compromised, an attacker wanting to snoop or modify data can always do so using the old file. So there is no advantage(?) to re-encrypt the same file with a set of new keys. Any new data written to the file after CPUSVN upgrade will re-create the chain of subkeys until the mht_key, and the attacker can't touch the new data because at this point we will use the new CPUSVN. |
I like this idea, as it doesn't induce any complexity into Gramine until there's a need for migration. One potential footgun here is that people may just fire this tool on an untrusted machine passing the encryption key into it. So, we need to think carefully how we name and describe everything.
Same from my side, the whole idea here is to migrate out from insecure TCB and thus we need to prevent downgrade attacks.
Looks good from my side, although I only did a quick read. Or actually... Isn't this completely insecure? Two cases:
But
What do you mean by subkeys here? I don't remember the PF format exactly now, but I think there was no support for multiple keys for different parts of the file. |
For files encrypted using mrenclave, there's probably nothing we can do. If the files are encrypted by mrsigner, the user can either deploy a migration tool as suggested by @kailun-qin or do the following: 1. temporarily add allow_migration to the manifest, which changes the value of mrenclave and invalidates remote attestation 2. run the app once and lazy migrate the files by accessing them, 3. delete the temporary binary and continue use the old manifest (without allow_migration), and its mrenclave is the valid remote attestation value.
This is a problem only if it's a zero-day attack. Once the new TCB appears in the remote attestation service, the attacker can't retrieve the key from remote server, unless the server allows out-of-date proof. This is different from CPU generated keys, which can always be generated.
Yes. Keep using this key is equivalent to the "sealed files" type of risk you mentioned above. This is inevitable and why the application developer will have to decide whether to allow migration for a certain file.
I mean the key used to encrypt data blocks, and the key used to encrypt the mht block which contains the data block key, and so on, until the mht_key. When we write even one byte to the file, existing code in Gramine will re-generate this chain of keys (including mht_key). This should prevent the attacker from accessing the new data block, even if he knows everything of the old file. |
Wait, I think you're forgetting some properties of SGX sealing keys:
So, as you can see, MRSIGNER-based keys are malleable to this "offline migration tool" approach, though with some quirks (needs to run inside the SGX enclave, needs to be signed with the same signing key). And MRENCLAVE-based keys simply too restrictive to use the "offline migration tool" approach. To support both MRSIGNER- and MRENCLAVE-based keys, the only common denominator is to introduce the "migration" code path in Gramine. I'm strongly against other ideas which would either break MRENCLAVE-based keys migration completely, or would result in two different ways of handling MRSIGNER- and MRENCLAVE-based keys.
First of all, that's why we'll introduce the new manifest sub-option Second of all, if the old sealing key is leaked, then can we do any better than the proposed scheme? To me, the only way to protect from this attack is to disallow completely the migration... By the way, what do you mean by "leaking the old sealing key"? You mean the derived (final) sealing key, not the secret in the SGX CPU fuses? Because in the latter case, the whole CPU is totally vulnerable iiuc. But if you mean the final sealing key, then it looks like the app developer will need to assess the risk and re-sign and re-deploy the application (in this case, a definitely different sealing key will be derived, even if the CPUSVN is specified as an old one).
See @lejunzhu reply. If the remotely-deployed key was leaked, then the key-releasing remote party must mark this key as unusable, and never release this key again. Also, this seems to be a completely different problem from what we're discussing in this issue. |
But the problem here is that only files accessed in that particular run will be upgraded, potentially missing some.
The attacker might have backed up the files. It's an untrusted host, after all.
There's always a window between the attack being public and everyone applying patches. Also, with the current super-long embargoes for side-channels, bugs can easily leak, like it was with Spectre and Meltdown.
That's what we IMO should do, a tool like Kailun proposed which changes encryption key + recommendation for users how to use it. But see below, I missed that it has to be a part of Gramine itself.
@dimakuv: I meant this particular point only for RA-deployed keys, sorry, I wasn't clear here, there are actually 3 different cases we need to consider (mrsigned-sealed, mrenclave-sealed, ra-deployed).
Yes, the enclave author can migrate all the files eagerly and then ship an enclave which refuses to use the old-SVN-based keys.
Yes.
But I think it requires the same solution - a tool which allows to reencrypt all the files with another key. Or maybe actually not, because this has to happen on the developer's trusted machine, and sealed keys has to be reencrypted on the untrusted machine... This will be very confusing for the users... |
This scenario sounds like an app issue, not a Gramine issue. In general data from files is untrusted and must be properly validated. I believe the same applies for protected files and perhaps it should be clarified in the documentation1. It may be useful to add an indication that a file is protected by keys with older SVNs, but I'm not sure what's the right way to do it.
While I am not against configurable migration support, I don't think it's very useful to disable it: since microcode updates are not necessarily under the user's control (e.g. in the cloud),
Why not add ISV SVN to the per-file metadata and apply the same logic to it? Footnotes
|
Hard disagree, it's just not true in real world for any serious app, they trust project files and configs. The whole point of protected and trusted files in Gramine is to provide a way to deploy filesystem which is trusted and can't be modified by the untrusted host.
So, we can't just say that the app should not trust files on disk. |
That's right. Maybe Gramine can walk through the encrypted mount points and try to migrate every file when in migration mode?
This is especially hard if the file is encrypted using MRENCLAVE based keys. I don't know if it's even possible.
For ra-deployed keys, the re-encryption (or simply changing the key) can also happen on the untrusted machine, if the tool can run inside an enclave, and can temporarily retrieve both the old and new key from remote key server(s). |
What do you mean by "after migration you upgrade the enclave to a version ..."? In which sense you upgrade the enclave, how exactly? Also, if we upgrade the enclave, then the MRENCLAVE measurement is changed, and the new enclave will refuse to read MRENCLAVE-encrypted files (no matter which CPU SVN).
Same question -- what do you mean by "ship an enclave ..."? Any new enclave makes old MRENCLAVE-encrypted files broken (even if they were migrated to a newer CPU SVN). In general, I'm struggling to see how MRENCLAVE-encrypted files can be safely migrated to a new CPU SVN, and still protect against the outlined attack of "the attacker managed to leak some old sealing key." |
Hmm, right, MRENCLAVE-sealed files are tough. Seems like the users just need to delete everything and redeploy the updated enclave in that case... |
Oh-oh, are we back to square one? Any other ideas how to fix the MRENCLAVE problem? (Reminder: MRENCLAVE-sealed CPUSVN-out-of-date files may be triggered for update by an attacker who previously stole the old-CPUSVN sealing key, and thus the attacker can inject random data using this to-be-updated file.) |
Bumping this thread again, as there is another customer who hit this problem. |
Bumping for myself, since I also faced it... |
@szymek156 Very quick answer: we discussed this problem several times, and each time we came to a conclusion that there is no simple and secure way of performing migration of files in this case. If you can contribute to our discussions, please propose your solution. |
@dimakuv (assuming all SVNs information is in the metadata of a file, as it is in SGX SDK)
Where Add to manifest: Add to documentation: Alternatively: do this, but lazy, when enclave wants to open a file |
Description of the problem
SGX key derivation gets SVN fields in the request (ISVSVN, CPUSVN and CONFIGSVN). This mechanism provides backwards compatibility and anti-rollback protection as follows:
Current implementation of sgx_get_seal_key() uses the current enclave's SVNs (ISV, CPU and CONFIG) in a hardcoded manner. Consequently, Gramine does not provide backwards compatibility mechanism: if the enclave or the platform were updated, previously sealed blob will no longer be accessible.
This is problematic not only for user code, but could also break backwards compatibility of protected filesystem: if a file was written with one combinations of SVNs, it will no longer be accessible after they were changed (scenario and expected vs. actual results are below).
I think the following changes will be required:
Steps to reproduce
Setup:
ISVSVN=1
and enclave B withISVSVN=2
, such that both enclave access the same protected files and both are signed with the same keyTest flow:
Expected results
Actual results
The text was updated successfully, but these errors were encountered: