Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
james/secure enclave cmd signer #1514
james/secure enclave cmd signer #1514
Changes from 32 commits
2ee7698
308ab38
8239d5d
97622d9
212176c
864d02f
ced7149
7fe2cf3
b59d2ab
0ce5224
29ae0d6
96073c4
99cb683
e4fa69f
611c7ec
9886c45
d4ebf25
15655b5
ce0b00c
58f77fe
1282f15
92b37d4
e41a4f9
ee114e0
d1ad0a1
d82d81a
9ae8c3c
cb3753a
66981b8
41847ee
7a4650b
bd2cb6f
7986154
c18e994
6644944
e66b948
6d2f4b4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about deletion/uninstall.
As it stands today, the uninstall message is not signed. So how do we uninstall these keys? Do we need to?
If we need to delete them, we probably need a signed deletion message.
If we don't need to, we could do somewhere where launcher records a nonce in the launcherdb, and then we pass the nonce along to the sign command. (so that we can find the correct key). (And by nonce, I probably mean the public key, we could stash it in the launcher db. Though that will be weird cross users. Probably still correct)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's a usability win, but I wonder if we can discern what kind of request it is from the signed blob.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this should never trigger in real use, I kinda want a giant log warning for this condition. (But I see slogger isn't here yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I know enough to know, but do we need to do any kind of tracking around which k2 key requested this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how or where we could track this? We could put it in logs and the way we do with desktop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talking aloud... If we create a key for a developer environment, and then get a signing request for the prod environment, what do we do? (or vice versa)
I'm not sure there's a security issue here, but I'm not sure how launcher picks the correct key to sign the response with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This ends up tying up to the deletion comments above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what we want to sign? Or do we want to pull some specific field out of the Msg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the existing code paths?