diff --git a/cmd/launcher/secure_enclave_darwin.go b/cmd/launcher/secure_enclave_darwin.go index 5e819b321..46810a1bc 100644 --- a/cmd/launcher/secure_enclave_darwin.go +++ b/cmd/launcher/secure_enclave_darwin.go @@ -4,12 +4,10 @@ package main import ( - "bytes" "crypto" "crypto/ecdsa" "crypto/rand" "encoding/base64" - "encoding/json" "errors" "fmt" "os" @@ -19,9 +17,7 @@ import ( "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/krypto/pkg/secureenclave" "github.com/kolide/launcher/ee/agent/certs" - "github.com/kolide/launcher/ee/localserver" "github.com/kolide/launcher/ee/secureenclavesigner" - "github.com/osquery/osquery-go/plugin/distributed" "github.com/vmihailenco/msgpack/v5" ) @@ -100,7 +96,7 @@ func createSecureEnclaveKey(requestB64 string) error { return fmt.Errorf("unmarshaling msgpack request: %w", err) } - if err := verifySecureEnclaveChallenge(createKeyRequest.SecureEnclaveRequest); err != nil { + if _, err := extractVerifiedSecureEnclaveChallenge(createKeyRequest.SecureEnclaveRequest); err != nil { return fmt.Errorf("verifying challenge: %w", err) } @@ -129,14 +125,11 @@ func signWithSecureEnclave(signRequestB64 string) error { return fmt.Errorf("unmarshaling msgpack sign request: %w", err) } - if err := verifySecureEnclaveChallenge(signRequest.SecureEnclaveRequest); err != nil { + challenge, err := extractVerifiedSecureEnclaveChallenge(signRequest.SecureEnclaveRequest) + if err != nil { return fmt.Errorf("verifying challenge: %w", err) } - if err := validateSecureEnclaveData(signRequest.Data); err != nil { - return fmt.Errorf("validating data: %w", err) - } - secureEnclavePubKey, err := echelper.PublicB64DerToEcdsaKey(signRequest.SecureEnclavePubKey) if err != nil { return fmt.Errorf("marshalling b64 der to public key: %w", err) @@ -147,11 +140,7 @@ func signWithSecureEnclave(signRequestB64 string) error { return fmt.Errorf("creating secure enclave signer: %w", err) } - if err := validateSecureEnclaveData(signRequest.Data); err != nil { - return fmt.Errorf("validating data: %w", err) - } - - digest, err := echelper.HashForSignature(signRequest.Data) + digest, err := echelper.HashForSignature(challenge.Msg) if err != nil { return fmt.Errorf("hashing data for signature: %w", err) } @@ -165,47 +154,27 @@ func signWithSecureEnclave(signRequestB64 string) error { return nil } -func verifySecureEnclaveChallenge(request secureenclavesigner.SecureEnclaveRequest) error { - c, err := challenge.UnmarshalChallenge(request.Challenge) +func extractVerifiedSecureEnclaveChallenge(request secureenclavesigner.SecureEnclaveRequest) (*challenge.OuterChallenge, error) { + challengeUnmarshalled, err := challenge.UnmarshalChallenge(request.Challenge) if err != nil { - return fmt.Errorf("unmarshaling challenge: %w", err) + return nil, fmt.Errorf("unmarshaling challenge: %w", err) } serverPubKey, ok := serverPubKeys[string(request.ServerPubKey)] if !ok { - return errors.New("server public key not found") + return nil, errors.New("server public key not found") } - if err := c.Verify(*serverPubKey); err != nil { - return fmt.Errorf("verifying challenge: %w", err) + if err := challengeUnmarshalled.Verify(*serverPubKey); err != nil { + return nil, fmt.Errorf("verifying challenge: %w", err) } // Check the timestamp, this prevents people from saving a challenge and then // reusing it a bunch. However, it will fail if the clocks are too far out of sync. - timestampDelta := time.Now().Unix() - c.Timestamp() + timestampDelta := time.Now().Unix() - challengeUnmarshalled.Timestamp() if timestampDelta > secureEnclaveTimestampValiditySeconds || timestampDelta < -secureEnclaveTimestampValiditySeconds { - return fmt.Errorf("timestamp delta %d is outside of validity range %d", timestampDelta, secureEnclaveTimestampValiditySeconds) + return nil, fmt.Errorf("timestamp delta %d is outside of validity range %d", timestampDelta, secureEnclaveTimestampValiditySeconds) } - return nil -} - -// validateSecureEnclaveData validates that the data is in a format that we expect to be signed, -// currently these are the responses returned by local server. -func validateSecureEnclaveData(data []byte) error { - if err := jsonStrictDecode(data, &localserver.RequestIdsResponse{}); err == nil { - return nil - } - - if err := jsonStrictDecode(data, &[]distributed.Result{}); err == nil { - return nil - } - - return errors.New("unrecognized data format") -} - -func jsonStrictDecode(data []byte, v interface{}) error { - decoder := json.NewDecoder(bytes.NewReader(data)) - decoder.DisallowUnknownFields() - return decoder.Decode(v) + return challengeUnmarshalled, nil } diff --git a/cmd/launcher/secure_enclave_test.go b/cmd/launcher/secure_enclave_test.go index 87ec74742..74f0e4f00 100644 --- a/cmd/launcher/secure_enclave_test.go +++ b/cmd/launcher/secure_enclave_test.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "encoding/base64" - "encoding/json" "fmt" "os" "os/exec" @@ -18,9 +17,7 @@ import ( "github.com/kolide/kit/ulid" "github.com/kolide/krypto/pkg/challenge" "github.com/kolide/krypto/pkg/echelper" - "github.com/kolide/launcher/ee/localserver" "github.com/kolide/launcher/ee/secureenclavesigner" - "github.com/osquery/osquery-go/plugin/distributed" "github.com/stretchr/testify/require" "github.com/vmihailenco/msgpack/v5" ) @@ -115,12 +112,12 @@ func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest serverPubKeys[string(testServerPubKeyB64Der)] = &testServerPrivKey.PublicKey someData := []byte(ulid.New()) - challenge, _, err := challenge.Generate(testServerPrivKey, someData, someData, someData) + challengeBytes, _, err := challenge.Generate(testServerPrivKey, someData, someData, someData) require.NoError(t, err) requestBytes, err := msgpack.Marshal(secureenclavesigner.CreateKeyRequest{ SecureEnclaveRequest: secureenclavesigner.SecureEnclaveRequest{ - Challenge: challenge, + Challenge: challengeBytes, ServerPubKey: testServerPubKeyB64Der, }, }) @@ -145,18 +142,11 @@ func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, secureEnclavePubKey, "should be able to get public key") - // try to sign unrecognized data format - dataToSignBadFormat, err := json.Marshal(map[string]string{ - "foo": "bar", - }) - require.NoError(t, err) - signRequest := secureenclavesigner.SignRequest{ SecureEnclaveRequest: secureenclavesigner.SecureEnclaveRequest{ - Challenge: challenge, + Challenge: challengeBytes, ServerPubKey: testServerPubKeyB64Der, }, - Data: dataToSignBadFormat, SecureEnclavePubKey: createKeyResponse, } @@ -165,25 +155,6 @@ func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest pipeReader, pipeWriter, err = os.Pipe() require.NoError(t, err) - os.Stdout = pipeWriter - require.Error(t, runSecureEnclave([]string{secureenclavesigner.SignCmd, base64.StdEncoding.EncodeToString(signRequestBytes)}), - "should get error for unrecognized data format") - - require.NoError(t, pipeWriter.Close()) - - // try to sign with good format - dataToSignGoodFormat, err := json.Marshal(localserver.RequestIdsResponse{ - RequestId: ulid.New(), - }) - require.NoError(t, err) - - signRequest.Data = dataToSignGoodFormat - - signRequestBytes = msgpackMustMarshall(t, signRequest) - - pipeReader, pipeWriter, err = os.Pipe() - require.NoError(t, err) - os.Stdout = pipeWriter require.NoError(t, runSecureEnclave([]string{secureenclavesigner.SignCmd, base64.StdEncoding.EncodeToString(signRequestBytes)})) require.NoError(t, pipeWriter.Close()) @@ -195,7 +166,10 @@ func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest sig, err := base64.StdEncoding.DecodeString(string(buf.Bytes())) require.NoError(t, err) - require.NoError(t, echelper.VerifySignature(secureEnclavePubKey, dataToSignGoodFormat, sig)) + c, err := challenge.UnmarshalChallenge(challengeBytes) + require.NoError(t, err) + + require.NoError(t, echelper.VerifySignature(secureEnclavePubKey, c.Msg, sig)) } func TestSecureEnclaveCmdValidation(t *testing.T) { //nolint:paralleltest @@ -297,65 +271,3 @@ func msgpackMustMarshall(t *testing.T, v interface{}) []byte { require.NoError(t, err) return b } - -func Test_validateSecureEnclaveData(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - data func() []byte - errFunc require.ErrorAssertionFunc - }{ - { - name: "RequestIdsResponse", - data: func() []byte { - data, err := json.Marshal(localserver.RequestIdsResponse{ - RequestId: ulid.New(), - }) - require.NoError(t, err) - return data - }, - errFunc: require.NoError, - }, - { - name: "QueryResults", - data: func() []byte { - data, err := json.Marshal([]distributed.Result{ - { - QueryName: "foo", - }, - }) - require.NoError(t, err) - return data - }, - errFunc: require.NoError, - }, - { - name: "unrecognized format", - data: func() []byte { - data, err := json.Marshal(map[string]string{ - "RequestId": ulid.New(), - "who_is_sneaky": "me", - }) - require.NoError(t, err) - return data - }, - errFunc: require.Error, - }, - { - name: "not json", - data: func() []byte { - return []byte("blah_blah_blah") - }, - errFunc: require.Error, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - tt.errFunc(t, validateSecureEnclaveData(tt.data())) - }) - } -} diff --git a/ee/localserver/request-id.go b/ee/localserver/request-id.go index e190d6d2a..1772c4c50 100644 --- a/ee/localserver/request-id.go +++ b/ee/localserver/request-id.go @@ -12,7 +12,7 @@ import ( "github.com/kolide/launcher/pkg/traces" ) -type Identifiers struct { +type identifiers struct { UUID string InstanceId string HardwareSerial string @@ -70,7 +70,7 @@ func (ls *localServer) requestIdHandlerFunc(w http.ResponseWriter, r *http.Reque Nonce: ulid.New(), Timestamp: time.Now(), } - response.Identifiers = ls.identifiers + response.identifiers = ls.identifiers jsonBytes, err := json.Marshal(response) if err != nil { diff --git a/ee/localserver/server.go b/ee/localserver/server.go index 355ef5b7b..4142791fb 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -44,7 +44,7 @@ type localServer struct { slogger *slog.Logger knapsack types.Knapsack srv *http.Server - identifiers Identifiers + identifiers identifiers limiter *rate.Limiter tlsCerts []tls.Certificate querier Querier diff --git a/ee/secureenclavesigner/secureenclavesigner.go b/ee/secureenclavesigner/secureenclavesigner.go index 8480127ce..049847aee 100644 --- a/ee/secureenclavesigner/secureenclavesigner.go +++ b/ee/secureenclavesigner/secureenclavesigner.go @@ -27,8 +27,6 @@ const ( type SignRequest struct { SecureEnclaveRequest - // Data is the data to be signed - Data []byte `msgpack:"data"` // SecureEnclavePubKey is the B64 encoded DER of the public key to be used to verify the signature SecureEnclavePubKey []byte `msgpack:"secure_enclave_pub_key"` } @@ -138,7 +136,6 @@ func (ses *secureEnclaveSigner) Sign(rand io.Reader, data []byte, opts crypto.Si Challenge: ses.challenge, ServerPubKey: ses.serverPubKeyB64Der, }, - Data: data, SecureEnclavePubKey: pubKeyBytes, } diff --git a/ee/secureenclavesigner/test_app_resources/embedded.provisionprofile b/ee/secureenclavesigner/test_app_resources/embedded.provisionprofile index bcc7bad11..148e28081 100644 Binary files a/ee/secureenclavesigner/test_app_resources/embedded.provisionprofile and b/ee/secureenclavesigner/test_app_resources/embedded.provisionprofile differ