Skip to content
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

[Docs] Add notes on _sgx_mrsigner and _sgx_mrenclave encryption keys #1725

Merged
merged 1 commit into from
May 7, 2024

Conversation

dimakuv
Copy link
Contributor

@dimakuv dimakuv commented Jan 18, 2024

Description of the changes

This commit is in response to customers' confusion on how to decrypt files encrypted with these keys.

How to test this PR?

N/A.


This change is Reviewable

Copy link

@ynonflumintel ynonflumintel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume my approval doesn't count for anything, but thank you for making this information more accessible

kailun-qin
kailun-qin previously approved these changes Jan 18, 2024
Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


Documentation/manifest-syntax.rst line 1011 at r1 (raw file):

  same platform) to unseal files. This key is not accessible outside of the
  (same) enclave and thus cannot be used to decrypt files encrypted in this
  enclave.

This is weirdly written IMO, not sure if I'd understand this sentence without the context of this PR:

  • It seems to say mostly the same information as the previous sentence. What's the additional information here which is missing?
  • "This key [...] cannot be used to decrypt" - it can, if you extract it somehow. The key can be used, but it's just not accessible outside of the enclave.
  • And it's actually not true, this key is used to decrypt the files, by the enclave itself.

Copy link
Contributor Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @kailun-qin and @mkow)


Documentation/manifest-syntax.rst line 1011 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

This is weirdly written IMO, not sure if I'd understand this sentence without the context of this PR:

  • It seems to say mostly the same information as the previous sentence. What's the additional information here which is missing?
  • "This key [...] cannot be used to decrypt" - it can, if you extract it somehow. The key can be used, but it's just not accessible outside of the enclave.
  • And it's actually not true, this key is used to decrypt the files, by the enclave itself.

Done, I rephrased. The previous text contained only the "recommendation" (...useful to allow...). I wanted to add the text that has the "mandatory limitation" flavor to it.

Well, maybe I should be even more specific and change the verb cannot be used to something like can only be derived by the specific enclave on the specific platform, and thus stays secret to the enclave+platform (unless the enclave software explicitly makes this key public). I don't know, feels like too much text.

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)


Documentation/manifest-syntax.rst line 1009 at r2 (raw file):

* ``"_sgx_mrenclave"`` (SGX only) is the SGX sealing key based on the MRENCLAVE
  identity of the enclave. This is useful to allow only the same enclave (on the
  same platform) to unseal files, i.e., this key cannot be used by any other

Suggestion:

this key is not accessible to any other

Documentation/manifest-syntax.rst line 1014 at r2 (raw file):

* ``"_sgx_mrsigner"`` (SGX only) is the SGX sealing key based on the MRSIGNER
  identity of the enclave. This is useful to allow all enclaves signed with the
  same key (and on the same platform) to unseal files, i.e., this key cannot be

ditto

Copy link
Contributor Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @mkow)


Documentation/manifest-syntax.rst line 1009 at r2 (raw file):

* ``"_sgx_mrenclave"`` (SGX only) is the SGX sealing key based on the MRENCLAVE
  identity of the enclave. This is useful to allow only the same enclave (on the
  same platform) to unseal files, i.e., this key cannot be used by any other

Done.


Documentation/manifest-syntax.rst line 1014 at r2 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

ditto

Done.

This commit is in response to customers' confusion on how to decrypt
files encrypted with these keys.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, "fixup! " found in commit messages' one-liners

@dimakuv dimakuv force-pushed the dimakuv/docs-sgx-keys-add-notes branch from a8298c0 to 64cd864 Compare May 6, 2024 14:54
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mkow mkow merged commit 64cd864 into master May 7, 2024
18 checks passed
@mkow mkow deleted the dimakuv/docs-sgx-keys-add-notes branch May 7, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants