james/secure enclave cmd signer #1514
Conversation
James-Pickett
changed the title
| "github.com/vmihailenco/msgpack/v5" | ||
| ) | ||
|
|
||
| type SignRequest struct { |
There was a problem hiding this comment.
kinda weird that some are []bytes of b64 strings ... wonder if they should all be b64 string or all just straight []byte
| pathToLauncherBinary string | ||
| } | ||
|
|
||
| func New(uid string, challenge []byte, opts ...opt) (*secureEnclaveSigner, error) { |
There was a problem hiding this comment.
should we go ahead and create / verify the key exists in secure enclave in the new func? Fail faster at the expense of a possible extra exec?
James-Pickett
changed the title
| } | ||
| } | ||
|
|
||
| func createSecureEnclaveKey(requestB64 string) error { |
There was a problem hiding this comment.
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.
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.
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.
(This ends up tying up to the deletion comments above)
| var serverPubKeys = make(map[string]*ecdsa.PublicKey) | ||
|
|
||
| // runSecureEnclave performs either a create-key or sign operation using the secure enclave. | ||
| // It's available as a separate command because launcher runs a root by default and since it's |
There was a problem hiding this comment.
(nitpick -- typo)
| // It's available as a separate command because launcher runs a root by default and since it's | |
| // It's available as a separate command because launcher runs as root by default and since it's |
There was a problem hiding this comment.
(this now says aa instead of as)
|
(Please don't merge this yet) |
| return fmt.Errorf("creating secure enclave signer: %w", err) | ||
| } | ||
|
|
||
| digest, err := echelper.HashForSignature(challenge.Msg) |
There was a problem hiding this comment.
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.
Can we use the existing code paths?
| } | ||
|
|
||
| func loadServerKeys() error { | ||
| if secureenclavesigner.Undertest { |
There was a problem hiding this comment.
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)
| } | ||
| } | ||
|
|
||
| func createSecureEnclaveKey(requestB64 string) error { |
There was a problem hiding this comment.
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.
| return errors.New("missing request") | ||
| } | ||
|
|
||
| switch args[0] { |
There was a problem hiding this comment.
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)
| } | ||
| } | ||
|
|
||
| func createSecureEnclaveKey(requestB64 string) error { |
There was a problem hiding this comment.
(This ends up tying up to the deletion comments above)
| return fmt.Errorf("creating secure enclave signer: %w", err) | ||
| } | ||
|
|
||
| digest, err := echelper.HashForSignature(challenge.Msg) |
There was a problem hiding this comment.
Can we use the existing code paths?
| // uid is the uid of the user to run the secure enclave commands as | ||
| uid string | ||
| // username is the username of the user to run the secure enclave commands as | ||
| username string |
| return errors.New("missing request") | ||
| } | ||
|
|
||
| switch args[0] { |
There was a problem hiding this comment.
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.
…te krypto middleware with nonce and user sig
| } | ||
|
|
||
| if runtime.GOOS == "darwin" { | ||
| requestDataWithUserSig, err := e.addUserSignature(r.Context(), response, *challengeBox) |
There was a problem hiding this comment.
Should we just go ahead and add a UserSig or MachineSig for all platforms? We could just do the same behavior with TPM.
|
closing in favor of completing #1644 first |
This PR sets up the ground for execing secure enclave in user ctx.
The broad strokes are:
ee/secureenclavesigner is a struct that implements the cyrpto.Signer interface. Its implementation of the the Public() and Sign() functions will exec out to new launcher secure-enclave subcommand when needed. Since the secureEnclaveSigner implements crypto.Signer it can be passed to the krypto/challenge package to sign responses.
follow on PRs:
related krypto PR: kolide/krypto#35