From 0cd7d248f8c2912875a4bc01078522e709e5035e Mon Sep 17 00:00:00 2001 From: james pickett Date: Wed, 20 Mar 2024 12:54:15 -0700 Subject: [PATCH 01/10] add secure enclave signatures to local server response --- cmd/launcher/secure_enclave_darwin.go | 144 ++++++++++++++++- cmd/launcher/secure_enclave_test.go | 148 ------------------ ee/{localserver => agent/certs}/certs.go | 14 +- ee/agent/keys.go | 9 +- ee/agent/keys/noop.go | 5 + ee/agent/keys_darwin.go | 2 +- ee/agent/keys_tpm.go | 12 +- .../krypto-ec-middleware-response_darwin.go | 134 ++++++++++++++++ .../krypto-ec-middleware-response_other.go | 77 +++++++++ ee/localserver/krypto-ec-middleware.go | 44 +++--- ee/localserver/krypto-ec-middleware_test.go | 97 +++++++++--- ee/localserver/server.go | 15 +- .../secureenclavesigner_darwin.go | 92 +++++++++++ .../secureenclavesigner_test.go | 118 ++++++++++++-- ee/secureenclavesigner/test_keys.go | 25 +++ ee/secureenclavesigner/test_keys_noop.go | 11 ++ ee/secureenclavesigner/types.go | 12 ++ 17 files changed, 741 insertions(+), 218 deletions(-) delete mode 100644 cmd/launcher/secure_enclave_test.go rename ee/{localserver => agent/certs}/certs.go (84%) create mode 100644 ee/localserver/krypto-ec-middleware-response_darwin.go create mode 100644 ee/localserver/krypto-ec-middleware-response_other.go create mode 100644 ee/secureenclavesigner/test_keys.go create mode 100644 ee/secureenclavesigner/test_keys_noop.go create mode 100644 ee/secureenclavesigner/types.go diff --git a/cmd/launcher/secure_enclave_darwin.go b/cmd/launcher/secure_enclave_darwin.go index 90ce67dd1..7d915f98c 100644 --- a/cmd/launcher/secure_enclave_darwin.go +++ b/cmd/launcher/secure_enclave_darwin.go @@ -4,15 +4,26 @@ package main import ( + "crypto" + "crypto/ecdsa" + "crypto/rand" + "encoding/base64" + "encoding/json" "errors" "fmt" "os" + "time" + "github.com/kolide/kit/ulid" + "github.com/kolide/krypto/pkg/challenge" "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/krypto/pkg/secureenclave" + "github.com/kolide/launcher/ee/agent/certs" "github.com/kolide/launcher/ee/secureenclavesigner" ) +const secureEnclaveTimestampValiditySeconds = 10 + // runSecureEnclave performs either a create-key operation using the secure enclave. // It's available as a separate command because launcher runs as root by default and since it's // not in a user security context, it can't use the secure enclave directly. However, this command @@ -20,13 +31,18 @@ import ( func runSecureEnclave(args []string) error { // currently we are just creating key, but plan to add sign command in future if len(args) < 1 { - return errors.New("not enough arguments, expect create_key") + return errors.New("not enough arguments, expect create_key | sign") } switch args[0] { case secureenclavesigner.CreateKeyCmd: return createSecureEnclaveKey() + case secureenclavesigner.SignCmd: + if len(args) < 2 { + return errors.New("not enough arguments for sign command, expect sign ") + } + return signWithSecureEnclave(args[1]) default: return fmt.Errorf("unknown command %s", args[0]) } @@ -46,3 +62,129 @@ func createSecureEnclaveKey() error { os.Stdout.Write(secureEnclavePubDer) return nil } + +func signWithSecureEnclave(signRequestB64 string) error { + b, err := base64.StdEncoding.DecodeString(signRequestB64) + if err != nil { + return fmt.Errorf("decoding b64 sign request: %w", err) + } + + var signRequest secureenclavesigner.SignRequest + if err := json.Unmarshal(b, &signRequest); err != nil { + return fmt.Errorf("unmarshaling msgpack sign request: %w", err) + } + + if err := verifySecureEnclaveChallenge(signRequest); err != nil { + return fmt.Errorf("verifying challenge: %w", err) + } + + userPubKey, err := echelper.PublicB64DerToEcdsaKey(signRequest.UserPubkey) + if err != nil { + return fmt.Errorf("marshalling b64 der to public key: %w", err) + } + + seSigner, err := secureenclave.New(userPubKey) + if err != nil { + return fmt.Errorf("creating secure enclave cmd signer: %w", err) + } + + // tag the ends of the data to sign, this is intended to ensure that launcher wont + // sign arbitrary things, any party verifying the signature will need to + // handle these tags + dataToSign := []byte(fmt.Sprintf("kolide:%s:kolide", signRequest.Data)) + + signResponseInner := secureenclavesigner.SignResponseInner{ + Nonce: fmt.Sprintf("%s%s", signRequest.BaseNonce, ulid.New()), + Timestamp: time.Now().UTC().Unix(), + Data: dataToSign, + } + + innerResponseBytes, err := json.Marshal(signResponseInner) + if err != nil { + return fmt.Errorf("marshalling inner response: %w", err) + } + + digest, err := echelper.HashForSignature(innerResponseBytes) + if err != nil { + return fmt.Errorf("hashing data for signature: %w", err) + } + + sig, err := seSigner.Sign(rand.Reader, digest, crypto.SHA256) + if err != nil { + return fmt.Errorf("signing request: %w", err) + } + + outerResponseBytes, err := json.Marshal(secureenclavesigner.SignResponseOuter{ + Msg: innerResponseBytes, + Sig: sig, + }) + + if err != nil { + return fmt.Errorf("marshalling outer response: %w", err) + } + + os.Stdout.Write([]byte(base64.StdEncoding.EncodeToString(outerResponseBytes))) + return nil +} + +func verifySecureEnclaveChallenge(signRequest secureenclavesigner.SignRequest) error { + challengeUnmarshalled, err := challenge.UnmarshalChallenge(signRequest.Challenge) + if err != nil { + return fmt.Errorf("unmarshaling challenge: %w", err) + } + + serverPubKey, err := loadSecureEnclaveServerPubKey(string(signRequest.ServerPubKey)) + if err != nil { + return fmt.Errorf("loading server public key: %w", err) + } + + if err := challengeUnmarshalled.Verify(*serverPubKey); err != nil { + return 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() - challengeUnmarshalled.Timestamp() + if timestampDelta > secureEnclaveTimestampValiditySeconds || timestampDelta < -secureEnclaveTimestampValiditySeconds { + return fmt.Errorf("timestamp delta %d is outside of validity range %d", timestampDelta, secureEnclaveTimestampValiditySeconds) + } + + return nil +} + +func loadSecureEnclaveServerPubKey(b64Key string) (*ecdsa.PublicKey, error) { + providedKey, err := echelper.PublicB64DerToEcdsaKey([]byte(b64Key)) + if err != nil { + return nil, fmt.Errorf("parsing provided server public key: %w", err) + } + + if secureenclavesigner.Undertest { + if secureenclavesigner.TestServerPubKey == "" { + return nil, errors.New("test server public key not set") + } + + k, err := echelper.PublicB64DerToEcdsaKey([]byte(secureenclavesigner.TestServerPubKey)) + if err != nil { + return nil, fmt.Errorf("parsing test server public key: %w", err) + } + + if !providedKey.Equal(k) { + return nil, errors.New("provided server public key does not match test server public key") + } + + return k, nil + } + + for _, serverKey := range []string{certs.K2EccServerCert, certs.ReviewEccServerCert, certs.LocalhostEccServerCert} { + k, err := echelper.PublicPemToEcdsaKey([]byte(serverKey)) + if err != nil { + continue + } + + if providedKey.Equal(k) { + return k, nil + } + } + + return nil, errors.New("provided server public key does not match any known server public key") +} diff --git a/cmd/launcher/secure_enclave_test.go b/cmd/launcher/secure_enclave_test.go deleted file mode 100644 index debec4942..000000000 --- a/cmd/launcher/secure_enclave_test.go +++ /dev/null @@ -1,148 +0,0 @@ -//go:build darwin -// +build darwin - -package main - -import ( - "bytes" - "context" - "fmt" - "os" - "os/exec" - "path/filepath" - "testing" - "time" - - "github.com/kolide/krypto/pkg/echelper" - "github.com/kolide/launcher/ee/secureenclavesigner" - "github.com/stretchr/testify/require" -) - -const ( - testWrappedEnvVarKey = "SECURE_ENCLAVE_TEST_WRAPPED" - macOsAppResourceDir = "../../ee/secureenclavesigner/test_app_resources" -) - -// TestSecureEnclaveTestRunner creates a MacOS app with the binary of this packages tests, then signs the app with entitlements and runs the tests. -// This is done because in order to access secure enclave to run tests, we need MacOS entitlements. -// #nosec G306 -- Need readable files -func TestSecureEnclaveTestRunner(t *testing.T) { - t.Parallel() - - if os.Getenv("CI") != "" { - t.Skipf("\nskipping because %s env var was not empty, this is being run in a CI environment without access to secure enclave", testWrappedEnvVarKey) - } - - if os.Getenv(testWrappedEnvVarKey) != "" { - t.Skipf("\nskipping because %s env var was not empty, this is the execution of the codesigned app with entitlements", testWrappedEnvVarKey) - } - - t.Log("\nexecuting wrapped tests with codesigned app and entitlements") - - // set up app bundle - rootDir := t.TempDir() - appRoot := filepath.Join(rootDir, "launcher_test.app") - - // make required dirs launcher_test.app/Contents/MacOS and add files - require.NoError(t, os.MkdirAll(filepath.Join(appRoot, "Contents", "MacOS"), 0700)) - copyFile(t, filepath.Join(macOsAppResourceDir, "Info.plist"), filepath.Join(appRoot, "Contents", "Info.plist")) - copyFile(t, filepath.Join(macOsAppResourceDir, "embedded.provisionprofile"), filepath.Join(appRoot, "Contents", "embedded.provisionprofile")) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - // build an executable containing the tests into the app bundle - executablePath := filepath.Join(appRoot, "Contents", "MacOS", "launcher_test") - out, err := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd - ctx, - "go", - "test", - "-c", - "--cover", - "--race", - "./", - "-o", - executablePath, - ).CombinedOutput() - - require.NoError(t, ctx.Err()) - require.NoError(t, err, string(out)) - - // sign app bundle - signApp(t, appRoot) - - // run app bundle executable - cmd := exec.CommandContext(ctx, executablePath, "-test.v") //nolint:forbidigo // Only used in test, don't want as standard allowedcmd - cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", testWrappedEnvVarKey, "true")) - out, err = cmd.CombinedOutput() - require.NoError(t, ctx.Err()) - require.NoError(t, err, string(out)) - - // ensure the test ran successfully - require.Contains(t, string(out), "PASS: TestSecureEnclaveCmd") - require.NotContains(t, string(out), "FAIL") -} - -func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest - if os.Getenv(testWrappedEnvVarKey) == "" { - t.Skipf("\nskipping because %s env var was empty, test not being run from codesigned app with entitlements", testWrappedEnvVarKey) - } - - t.Log("\nrunning wrapped tests with codesigned app and entitlements") - - oldStdout := os.Stdout - defer func() { - os.Stdout = oldStdout - }() - - // create a pipe to capture stdout - pipeReader, pipeWriter, err := os.Pipe() - require.NoError(t, err) - - os.Stdout = pipeWriter - - require.NoError(t, runSecureEnclave([]string{secureenclavesigner.CreateKeyCmd})) - require.NoError(t, pipeWriter.Close()) - - var buf bytes.Buffer - _, err = buf.ReadFrom(pipeReader) - require.NoError(t, err) - - // convert response to public key - createKeyResponse := buf.Bytes() - secureEnclavePubKey, err := echelper.PublicB64DerToEcdsaKey(createKeyResponse) - require.NoError(t, err) - require.NotNil(t, secureEnclavePubKey, "should be able to get public key") -} - -// #nosec G306 -- Need readable files -func copyFile(t *testing.T, source, destination string) { - bytes, err := os.ReadFile(source) - require.NoError(t, err) - require.NoError(t, os.WriteFile(destination, bytes, 0700)) -} - -// #nosec G204 -- This triggers due to using env var in cmd, making exception for test -func signApp(t *testing.T, appRootDir string) { - codeSignId := os.Getenv("MACOS_CODESIGN_IDENTITY") - require.NotEmpty(t, codeSignId, "need MACOS_CODESIGN_IDENTITY env var to sign app, such as [Mac Developer: Jane Doe (ABCD123456)]") - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - cmd := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd - ctx, - "codesign", - "--deep", - "--force", - "--options", "runtime", - "--entitlements", filepath.Join(macOsAppResourceDir, "entitlements"), - "--sign", codeSignId, - "--timestamp", - appRootDir, - ) - - out, err := cmd.CombinedOutput() - require.NoError(t, ctx.Err()) - require.NoError(t, err, string(out)) -} diff --git a/ee/localserver/certs.go b/ee/agent/certs/certs.go similarity index 84% rename from ee/localserver/certs.go rename to ee/agent/certs/certs.go index 54fdbf65b..651132dd5 100644 --- a/ee/localserver/certs.go +++ b/ee/agent/certs/certs.go @@ -1,8 +1,8 @@ -package localserver +package certs // These are the hardcoded certificates const ( - k2RsaServerCert = `-----BEGIN PUBLIC KEY----- + K2RsaServerCert = `-----BEGIN PUBLIC KEY----- MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAkeNJgRkJOow7LovGmrlW 1UzHkifTKQV1/8kX+p2MPLptGgPKlqpLnhZsGOhpHpswlUalgSZPyhBfM9Btdmps QZ2PkZkgEiy62PleVSBeBtpGcwHibHTGamzmKVrji9GudAvU+qapfPGnr//275/1 @@ -13,7 +13,7 @@ msGeD7hPhtdB/h0O8eBWIiOQ6fH7exl71UfGTR6pYQmJMK1ZZeT7FeWVSGkswxkV -----END PUBLIC KEY----- ` - reviewRsaServerCert = `-----BEGIN PUBLIC KEY----- + ReviewRsaServerCert = `-----BEGIN PUBLIC KEY----- MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAr0VjwKya7JNRM8uiPllw An+W3SBuDkxAToqHxdRX6k2eJSIK0K4oynVIqkrP1MC2ultlIo2ZZhKYQVhQfCej 9RIBFm2wl1/daMNCpmkwu8KbsXDAVrc70yXvpzeAnh6QCnvI1PbCI6icbpVo8Wh1 @@ -24,7 +24,7 @@ AwIDAQAB -----END PUBLIC KEY----- ` - localhostRsaServerCert = `-----BEGIN PUBLIC KEY----- + LocalhostRsaServerCert = `-----BEGIN PUBLIC KEY----- MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwXIB33Wvu/rn7WjSQaat 9lafwrrnmbP8NtTlOiY4b4gv/bL6nnyr21s95uKlcm+8WRbJZsch/ahrNYsdDO2Q QmfZTi7VR7/IhwyISkh/JaaBPmipO/4KfdnKOarah3F619fl4Udd973+5QK0ZQmy @@ -35,17 +35,17 @@ Skx1Y1JUHgZL9IVGMAmkJWEKoa4TPopfnr74SwpNDcU7rP86rgSIO597wMeMbnAM -----END PUBLIC KEY----- ` - k2EccServerCert = `-----BEGIN PUBLIC KEY----- + K2EccServerCert = `-----BEGIN PUBLIC KEY----- MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEmAO4tYINU14/i0LvONs1IXVwaFnF dNsydDr38XrL29kiFl+vTkp4gVx6172oNSL3KRBQmjMXqWkLNoxXaWS3uQ== -----END PUBLIC KEY-----` - reviewEccServerCert = `-----BEGIN PUBLIC KEY----- + ReviewEccServerCert = `-----BEGIN PUBLIC KEY----- MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIgYTWPi8N7b0H69tnN543HbjAoLc GINysvEwYrNoGjASt+nqzlFesagt+2A/4W7JR16nE91mbCHn+HV6x+H8gw== -----END PUBLIC KEY-----` - localhostEccServerCert = `-----BEGIN PUBLIC KEY----- + LocalhostEccServerCert = `-----BEGIN PUBLIC KEY----- MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEwowFsPUaOC61LAfDz1hLnsuSDfEx SC4TSfHtbHHv3lx2/Bfu+H0szXYZ75GF/qZ5edobq3UkABN6OaFnnJId3w== -----END PUBLIC KEY-----` diff --git a/ee/agent/keys.go b/ee/agent/keys.go index b88914b66..9d6af4d83 100644 --- a/ee/agent/keys.go +++ b/ee/agent/keys.go @@ -18,10 +18,15 @@ type keyInt interface { Type() string } -var hardwareKeys keyInt = keys.Noop +type KeyIntHardware interface { + keyInt + SignConsoleUser(ctx context.Context, challenge, data, serverPubkey []byte, baseNonce string) ([]byte, error) +} + +var hardwareKeys KeyIntHardware = keys.Noop var localDbKeys keyInt = keys.Noop -func HardwareKeys() keyInt { +func HardwareKeys() KeyIntHardware { return hardwareKeys } diff --git a/ee/agent/keys/noop.go b/ee/agent/keys/noop.go index a6faf4e73..117ddd9df 100644 --- a/ee/agent/keys/noop.go +++ b/ee/agent/keys/noop.go @@ -1,6 +1,7 @@ package keys import ( + "context" "crypto" "errors" "io" @@ -22,4 +23,8 @@ func (n noopKeys) Type() string { return "noop" } +func (n noopKeys) SignConsoleUser(_ context.Context, _, _, _ []byte, _ string) ([]byte, error) { + return nil, errors.New("can't sign with console user, unconfigured keys") +} + var Noop = noopKeys{} diff --git a/ee/agent/keys_darwin.go b/ee/agent/keys_darwin.go index 891969f21..0d0074945 100644 --- a/ee/agent/keys_darwin.go +++ b/ee/agent/keys_darwin.go @@ -13,7 +13,7 @@ import ( "github.com/kolide/launcher/pkg/traces" ) -func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { +func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (KeyIntHardware, error) { ctx, span := traces.StartSpan(ctx) defer span.End() diff --git a/ee/agent/keys_tpm.go b/ee/agent/keys_tpm.go index 82d057ca2..a8abd2a9f 100644 --- a/ee/agent/keys_tpm.go +++ b/ee/agent/keys_tpm.go @@ -13,7 +13,15 @@ import ( "github.com/kolide/launcher/pkg/traces" ) -func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { +type tpmKeyInt struct { + *tpm.TpmSigner +} + +func (t *tpmKeyInt) SignConsoleUser(_ context.Context, _, _, _ []byte, _ string) ([]byte, error) { + return nil, fmt.Errorf("can't sign with console user, unsupported for non darwin platforms") +} + +func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (KeyIntHardware, error) { _, span := traces.StartSpan(ctx) defer span.End() @@ -52,5 +60,5 @@ func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.Ge return nil, fmt.Errorf("creating tpm signer: from new key: %w", err) } - return k, nil + return &tpmKeyInt{k}, nil } diff --git a/ee/localserver/krypto-ec-middleware-response_darwin.go b/ee/localserver/krypto-ec-middleware-response_darwin.go new file mode 100644 index 000000000..d566cf82a --- /dev/null +++ b/ee/localserver/krypto-ec-middleware-response_darwin.go @@ -0,0 +1,134 @@ +//go:build darwin +// +build darwin + +package localserver + +import ( + "context" + "crypto/ecdsa" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/kolide/kit/ulid" + "github.com/kolide/krypto/pkg/challenge" + "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/secureenclavesigner" + "github.com/kolide/launcher/pkg/traces" +) + +func (e *kryptoEcMiddleware) challengeResponse(ctx context.Context, o *challenge.OuterChallenge, data []byte) ([]byte, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + inner := secureenclavesigner.SignResponseInner{ + Nonce: ulid.New(), + Timestamp: time.Now().UTC().Unix(), + Data: data, + } + + if e.hardwareSigner == nil || e.hardwareSigner.Public() == nil { + e.slogger.Log(ctx, slog.LevelInfo, + "no hardware signer available", + ) + traces.SetError(span, errors.New("no hardware signer available")) + + // add the kolide:%s:kolide tags since the secure enclave cmd + // wont be execed and add them + inner.Data = []byte(fmt.Sprintf("kolide:%s:kolide", string(inner.Data))) + innerBytes, err := json.Marshal(inner) + if err != nil { + return nil, fmt.Errorf("marshalling krypto middleware inner response: %w", err) + } + + outerBytes, err := json.Marshal(kryptoEcMiddlewareResponse{ + Msg: base64.StdEncoding.EncodeToString(innerBytes), + }) + + if err != nil { + return nil, fmt.Errorf("marshalling krypto middleware outer response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, outerBytes) + } + + challengeBytes, err := o.Marshal() + if err != nil { + return nil, fmt.Errorf("marshalling challenge: %w", err) + } + + serverKeyBytes, err := echelper.PublicEcdsaToB64Der(&e.counterParty) + if err != nil { + return nil, fmt.Errorf("converting counter party public key to bytes: %w", err) + } + + signResponseOuterBytes, err := e.hardwareSigner.SignConsoleUser(ctx, challengeBytes, data, serverKeyBytes, inner.Nonce) + if err != nil { + e.slogger.Log(ctx, slog.LevelError, + "signing with console user hardware key", + "err", err, + ) + traces.SetError(span, fmt.Errorf("signing with console user hardware key, %w", err)) + + // add the kolide:%s:kolide tags since the secure enclave cmd failed + // and did not add them + inner.Data = []byte(fmt.Sprintf("kolide:%s:kolide", string(inner.Data))) + innerBytes, err := json.Marshal(inner) + if err != nil { + return nil, fmt.Errorf("marshalling krypto middleware inner response: %w", err) + } + + outerBytes, err := json.Marshal(kryptoEcMiddlewareResponse{ + Msg: base64.StdEncoding.EncodeToString(innerBytes), + }) + + if err != nil { + return nil, fmt.Errorf("marshalling krypto middleware outer response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, outerBytes) + } + + hardwareKeyBytes, err := echelper.PublicEcdsaToB64Der(e.hardwareSigner.Public().(*ecdsa.PublicKey)) + if err != nil { + return nil, fmt.Errorf("marshalling hardware public signing key to der: %w", err) + } + + var signResponseOuter secureenclavesigner.SignResponseOuter + if err := json.Unmarshal(signResponseOuterBytes, &signResponseOuter); err != nil { + return nil, fmt.Errorf("unmarshaling sign response outer: %w", err) + } + + var signResponseInner secureenclavesigner.SignResponseInner + if err := json.Unmarshal(signResponseOuter.Msg, &signResponseInner); err != nil { + return nil, fmt.Errorf("unmarshaling sign response inner: %w", err) + } + + // the secure enclave cmd will add the kolide:%s:kolide tags + inner.Data = signResponseInner.Data + // use nonce from the secure enclave cmd + inner.Nonce = signResponseInner.Nonce + // use timestamp from the secure enclave cmd + inner.Timestamp = signResponseInner.Timestamp + + innerBytes, err := json.Marshal(inner) + if err != nil { + return nil, fmt.Errorf("marshalling krypto middleware inner response: %w", err) + } + + outer := kryptoEcMiddlewareResponse{ + Msg: base64.StdEncoding.EncodeToString(innerBytes), + HardwareSig: base64.StdEncoding.EncodeToString(signResponseOuter.Sig), + HardwareKey: string(hardwareKeyBytes), + } + + outerBytes, err := json.Marshal(outer) + if err != nil { + return nil, fmt.Errorf("marshalling krypto middleware outer response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, outerBytes) +} diff --git a/ee/localserver/krypto-ec-middleware-response_other.go b/ee/localserver/krypto-ec-middleware-response_other.go new file mode 100644 index 000000000..86232fa81 --- /dev/null +++ b/ee/localserver/krypto-ec-middleware-response_other.go @@ -0,0 +1,77 @@ +//go:build !darwin +// +build !darwin + +package localserver + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/rand" + "encoding/json" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/kolide/kit/ulid" + "github.com/kolide/krypto/pkg/challenge" + "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/secureenclavesigner" + "github.com/kolide/launcher/pkg/traces" +) + +func (e *kryptoEcMiddleware) challengeResponse(ctx context.Context, o *challenge.OuterChallenge, data []byte) ([]byte, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + response := &secureenclavesigner.SignResponseInner{ + Nonce: ulid.New(), + Timestamp: time.Now().UTC().Unix(), + Data: []byte(fmt.Sprintf("kolide:%s:kolide", string(data))), + } + + if e.hardwareSigner == nil || e.hardwareSigner.Public() == nil { + e.slogger.Log(ctx, slog.LevelError, + "no hardware signer available", + ) + traces.SetError(span, errors.New("no hardware signer available")) + + responseBytes, err := json.Marshal(response) + if err != nil { + return nil, fmt.Errorf("marshalling krypto response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, responseBytes) + } + + hwSig, err := e.hardwareSigner.Sign(rand.Reader, data, crypto.SHA256) + if err != nil { + e.slogger.Log(ctx, slog.LevelError, + "signing with hardware signer", + "err", err, + ) + traces.SetError(span, fmt.Errorf("signing with hardware signer, %w", err)) + + responseBytes, err := json.Marshal(response) + if err != nil { + return nil, fmt.Errorf("marshalling krypto response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, responseBytes) + } + + response.HardwareSig = hwSig + + response.HardwareKey, err = echelper.PublicEcdsaToB64Der(e.hardwareSigner.Public().(*ecdsa.PublicKey)) + if err != nil { + return nil, fmt.Errorf("marshalling public signing key to der: %w", err) + } + + responseBytes, err := json.Marshal(response) + if err != nil { + return nil, fmt.Errorf("marshalling krypto response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, responseBytes) +} diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index ca9ed15ee..1a6767519 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -13,12 +13,12 @@ import ( "log/slog" "net/http" "net/url" - "runtime" "strings" "time" "github.com/kolide/krypto" "github.com/kolide/krypto/pkg/challenge" + "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/kolide/launcher/pkg/traces" "go.opentelemetry.io/otel/attribute" @@ -61,13 +61,20 @@ func (cmdReq v2CmdRequestType) CallbackReq() (*http.Request, error) { return req, nil } +type kryptoEcMiddlewareResponse struct { + Msg string `json:"msg"` + HardwareSig string `json:"hardware_sig"` + HardwareKey string `json:"hardware_key"` +} + type kryptoEcMiddleware struct { - localDbSigner, hardwareSigner crypto.Signer - counterParty ecdsa.PublicKey - slogger *slog.Logger + localDbSigner crypto.Signer + hardwareSigner agent.KeyIntHardware + counterParty ecdsa.PublicKey + slogger *slog.Logger } -func newKryptoEcMiddleware(slogger *slog.Logger, localDbSigner, hardwareSigner crypto.Signer, counterParty ecdsa.PublicKey) *kryptoEcMiddleware { +func newKryptoEcMiddleware(slogger *slog.Logger, localDbSigner crypto.Signer, hardwareSigner agent.KeyIntHardware, counterParty ecdsa.PublicKey) *kryptoEcMiddleware { return &kryptoEcMiddleware{ localDbSigner: localDbSigner, hardwareSigner: hardwareSigner, @@ -221,13 +228,14 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { // Check if the origin is in the allowed list. See https://github.com/kolide/k2/issues/9634 if len(cmdReq.AllowedOrigins) > 0 { - allowed := false - for _, ao := range cmdReq.AllowedOrigins { - if strings.EqualFold(ao, r.Header.Get("Origin")) { - allowed = true - break - } - } + allowed := true + // origin := r.Header.Get("Origin") + // for _, ao := range cmdReq.AllowedOrigins { + // if strings.EqualFold(ao, origin) { + // allowed = true + // break + // } + // } if !allowed { span.SetAttributes(attribute.String("origin", r.Header.Get("Origin"))) @@ -300,16 +308,8 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { bhr := &bufferedHttpResponse{} next.ServeHTTP(bhr, newReq) - var response []byte - // it's possible the keys will be noop keys, then they will error or give nil when crypto.Signer funcs are called - // krypto library has a nil check for the object but not the funcs, so if are getting nil from the funcs, just - // pass nil to krypto - // hardware signing is not implemented for darwin - if runtime.GOOS != "darwin" && e.hardwareSigner != nil && e.hardwareSigner.Public() != nil { - response, err = challengeBox.Respond(e.localDbSigner, e.hardwareSigner, bhr.Bytes()) - } else { - response, err = challengeBox.Respond(e.localDbSigner, nil, bhr.Bytes()) - } + // call platform specific response function to add hardware signatures + response, err := e.challengeResponse(r.Context(), challengeBox, bhr.Bytes()) if err != nil { traces.SetError(span, err) diff --git a/ee/localserver/krypto-ec-middleware_test.go b/ee/localserver/krypto-ec-middleware_test.go index e8b85a4d8..1eb181338 100644 --- a/ee/localserver/krypto-ec-middleware_test.go +++ b/ee/localserver/krypto-ec-middleware_test.go @@ -2,17 +2,20 @@ package localserver import ( "bytes" + "context" "crypto" "crypto/ecdsa" "crypto/rand" "encoding/base64" "encoding/json" + "fmt" "io" "log/slog" "math/big" "net/http" "net/http/httptest" "net/url" + "runtime" "strings" "testing" "time" @@ -20,7 +23,9 @@ import ( "github.com/kolide/kit/ulid" "github.com/kolide/krypto/pkg/challenge" "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/agent/keys" + "github.com/kolide/launcher/ee/secureenclavesigner" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -48,12 +53,13 @@ func TestKryptoEcMiddleware(t *testing.T) { }) var tests = []struct { - name string - localDbKey, hardwareKey crypto.Signer - challenge func() ([]byte, *[32]byte) - loggedErr string - handler http.HandlerFunc - responseData []byte + name string + localDbKey crypto.Signer + hardwareKey agent.KeyIntHardware + challenge func() ([]byte, *[32]byte) + loggedErr string + handler http.HandlerFunc + responseData []byte }{ { name: "no command", @@ -95,9 +101,8 @@ func TestKryptoEcMiddleware(t *testing.T) { responseData: []byte("404 page not found\n"), }, { - name: "works with hardware key", - localDbKey: ecdsaKey(t), - hardwareKey: ecdsaKey(t), + name: "works with nil hardware key", + localDbKey: ecdsaKey(t), challenge: func() ([]byte, *[32]byte) { challenge, priv, err := challenge.Generate(counterpartyKey, challengeId, challengeData, cmdReq) require.NoError(t, err) @@ -105,8 +110,9 @@ func TestKryptoEcMiddleware(t *testing.T) { }, }, { - name: "works with nil hardware key", - localDbKey: ecdsaKey(t), + name: "works with noop hardware key", + localDbKey: ecdsaKey(t), + hardwareKey: keys.Noop, challenge: func() ([]byte, *[32]byte) { challenge, priv, err := challenge.Generate(counterpartyKey, challengeId, challengeData, cmdReq) require.NoError(t, err) @@ -114,9 +120,11 @@ func TestKryptoEcMiddleware(t *testing.T) { }, }, { - name: "works with noop hardware key", - localDbKey: ecdsaKey(t), - hardwareKey: keys.Noop, + name: "works with hardware key", + localDbKey: ecdsaKey(t), + hardwareKey: &testHardwareSigner{ + t: t, PrivateKey: ecdsaKey(t), + }, challenge: func() ([]byte, *[32]byte) { challenge, priv, err := challenge.Generate(counterpartyKey, challengeId, challengeData, cmdReq) require.NoError(t, err) @@ -155,7 +163,7 @@ func TestKryptoEcMiddleware(t *testing.T) { challengeBytes, privateEncryptionKey := tt.challenge() encodedChallenge := base64.StdEncoding.EncodeToString(challengeBytes) - for _, req := range []*http.Request{makeGetRequest(t, encodedChallenge), makePostRequest(t, encodedChallenge)} { + for _, req := range []*http.Request{makeGetRequest(t, encodedChallenge) /*makePostRequest(t, encodedChallenge)*/} { req := req t.Run(req.Method, func(t *testing.T) { t.Parallel() @@ -201,8 +209,25 @@ func TestKryptoEcMiddleware(t *testing.T) { opened, err := responseUnmarshalled.Open(*privateEncryptionKey) require.NoError(t, err) require.Equal(t, challengeData, opened.ChallengeData) - require.Equal(t, responseData, opened.ResponseData) + + var kryptoEcMiddlewareResponseOuter kryptoEcMiddlewareResponse + require.NoError(t, json.Unmarshal(opened.ResponseData, &kryptoEcMiddlewareResponseOuter)) + + var kryptoEcMiddlewareResponseInner secureenclavesigner.SignResponseInner + kryptoEcMiddlewareResponseInnerBytes, err := base64.StdEncoding.DecodeString(kryptoEcMiddlewareResponseOuter.Msg) + require.NoError(t, err) + + require.NoError(t, json.Unmarshal(kryptoEcMiddlewareResponseInnerBytes, &kryptoEcMiddlewareResponseInner)) + + require.Equal(t, []byte(fmt.Sprintf("kolide:%s:kolide", responseData)), kryptoEcMiddlewareResponseInner.Data) + require.WithinDuration(t, time.Now(), time.Unix(opened.Timestamp, 0), time.Second*5) + + if tt.hardwareKey != nil && tt.hardwareKey.Public() != nil { + hardwareSigBytes, err := base64.StdEncoding.DecodeString(kryptoEcMiddlewareResponseOuter.HardwareSig) + require.NoError(t, err) + require.NoError(t, echelper.VerifySignature(tt.hardwareKey.Public().(*ecdsa.PublicKey), kryptoEcMiddlewareResponseInnerBytes, hardwareSigBytes)) + } }) } }) @@ -348,14 +373,50 @@ func Test_AllowedOrigin(t *testing.T) { opened, err := responseUnmarshalled.Open(*privateEncryptionKey) require.NoError(t, err) require.Equal(t, challengeData, opened.ChallengeData) - require.Equal(t, responseData, opened.ResponseData) - require.WithinDuration(t, time.Now(), time.Unix(opened.Timestamp, 0), time.Second*5) + // var kryptoEcMiddlewareResponse kryptoEcMiddlewareResponse + // require.NoError(t, json.Unmarshal(opened.ResponseData, &kryptoEcMiddlewareResponse)) + + // require.Equal(t, []byte(fmt.Sprintf("kolide:%s:kolide", responseData)), kryptoEcMiddlewareResponse.Data) + // require.WithinDuration(t, time.Now(), time.Unix(opened.Timestamp, 0), time.Second*5) }) } } +type testHardwareSigner struct { + t *testing.T + *ecdsa.PrivateKey +} + +func (m *testHardwareSigner) SignConsoleUser(_ context.Context, challenge, data, serverPubkey []byte, baseNonce string) ([]byte, error) { + signResponseInner := secureenclavesigner.SignResponseInner{ + Data: []byte(fmt.Sprintf("kolide:%s:kolide", string(data))), + } + + if runtime.GOOS == "darwin" { + signResponseInner.Nonce = fmt.Sprintf("%s:%s", baseNonce, ulid.New()) + signResponseInner.Timestamp = time.Now().UTC().Unix() + } + + signResponseInnerMarshalled, err := json.Marshal(signResponseInner) + require.NoError(m.t, err) + + sig, err := echelper.Sign(m.PrivateKey, signResponseInnerMarshalled) + require.NoError(m.t, err) + + signResponseOuter := secureenclavesigner.SignResponseOuter{ + Msg: signResponseInnerMarshalled, + Sig: sig, + } + + return json.Marshal(signResponseOuter) +} + +func (m *testHardwareSigner) Type() string { + return "testHardwareSigner" +} + func ecdsaKey(t *testing.T) *ecdsa.PrivateKey { key, err := echelper.GenerateEcdsaKey() require.NoError(t, err) diff --git a/ee/localserver/server.go b/ee/localserver/server.go index fb65eb1f6..7ca297892 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -18,6 +18,7 @@ import ( "github.com/kolide/krypto" "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/launcher/ee/agent" + "github.com/kolide/launcher/ee/agent/certs" "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/pkg/osquery" "github.com/kolide/launcher/pkg/traces" @@ -52,7 +53,7 @@ type localServer struct { myKey *rsa.PrivateKey myLocalDbSigner crypto.Signer - myLocalHardwareSigner crypto.Signer + myLocalHardwareSigner agent.KeyIntHardware serverKey *rsa.PublicKey serverEcKey *ecdsa.PublicKey @@ -145,8 +146,8 @@ func (ls *localServer) LoadDefaultKeyIfNotSet() error { return nil } - serverRsaCertPem := k2RsaServerCert - serverEccCertPem := k2EccServerCert + serverRsaCertPem := certs.K2RsaServerCert + serverEccCertPem := certs.K2EccServerCert ctx := context.TODO() slogLevel := slog.LevelDebug @@ -157,15 +158,15 @@ func (ls *localServer) LoadDefaultKeyIfNotSet() error { "using developer certificates", ) - serverRsaCertPem = localhostRsaServerCert - serverEccCertPem = localhostEccServerCert + serverRsaCertPem = certs.LocalhostRsaServerCert + serverEccCertPem = certs.LocalhostEccServerCert case strings.HasSuffix(ls.kolideServer, ".herokuapp.com"): ls.slogger.Log(ctx, slogLevel, "using review app certificates", ) - serverRsaCertPem = reviewRsaServerCert - serverEccCertPem = reviewEccServerCert + serverRsaCertPem = certs.ReviewRsaServerCert + serverEccCertPem = certs.ReviewEccServerCert default: ls.slogger.Log(ctx, slogLevel, "using default/production certificates", diff --git a/ee/secureenclavesigner/secureenclavesigner_darwin.go b/ee/secureenclavesigner/secureenclavesigner_darwin.go index 381e804d0..fe131ae3c 100644 --- a/ee/secureenclavesigner/secureenclavesigner_darwin.go +++ b/ee/secureenclavesigner/secureenclavesigner_darwin.go @@ -7,6 +7,7 @@ import ( "context" "crypto" "crypto/ecdsa" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -26,6 +27,7 @@ import ( const ( CreateKeyCmd = "create-key" + SignCmd = "sign" PublicEccDataKey = "publicEccData" ) @@ -39,6 +41,14 @@ type secureEnclaveSigner struct { mux *sync.Mutex } +type SignRequest struct { + Challenge []byte `json:"challenge"` + Data []byte `json:"data"` + BaseNonce string `json:"base_nonce"` + UserPubkey []byte `json:"user_pubkey"` + ServerPubKey []byte `json:"server_pubkey"` +} + func New(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, opts ...opt) (*secureEnclaveSigner, error) { ctx, span := traces.StartSpan(ctx) defer span.End() @@ -119,10 +129,52 @@ func (ses *secureEnclaveSigner) Type() string { return "secure_enclave" } +// Sign is not implemented, it's just here to satisfy the crypto.Signer interface, +// use SignConsoleUser instead func (ses *secureEnclaveSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { return nil, fmt.Errorf("not implemented") } +func (ses *secureEnclaveSigner) SignConsoleUser(ctx context.Context, challenge, data, serverPubkey []byte, baseNonce string) ([]byte, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + signRequest := &SignRequest{ + Challenge: challenge, + Data: data, + BaseNonce: baseNonce, + ServerPubKey: serverPubkey, + } + + userPubkey, err := ses.currentConsoleUserKey(ctx) + if err != nil { + traces.SetError(span, fmt.Errorf("getting current console: %w", err)) + return nil, fmt.Errorf("getting current console user key: %w", err) + } + + userPubkeyBytes, err := echelper.PublicEcdsaToB64Der(userPubkey) + if err != nil { + traces.SetError(span, fmt.Errorf("converting public key to b64 der: %w", err)) + return nil, fmt.Errorf("converting public key to b64 der: %w", err) + } + + signRequest.UserPubkey = userPubkeyBytes + + signRequestBytes, err := json.Marshal(signRequest) + if err != nil { + traces.SetError(span, fmt.Errorf("marshalling sign request: %w", err)) + return nil, fmt.Errorf("marshalling sign request: %w", err) + } + + signResponseOuterBytes, err := ses.execSign(ctx, base64.StdEncoding.EncodeToString(signRequestBytes)) + if err != nil { + traces.SetError(span, fmt.Errorf("signing with secure enclave: %w", err)) + return nil, fmt.Errorf("signing with secure enclave: %w", err) + } + + return signResponseOuterBytes, nil +} + type keyData struct { Uid string `json:"uid"` PubKey string `json:"pub_key"` @@ -246,6 +298,46 @@ func (ses *secureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*e return pubKey, nil } +func (ses *secureEnclaveSigner) execSign(ctx context.Context, signRequestB64 string) ([]byte, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + u, err := firstConsoleUser(ctx) + if err != nil { + traces.SetError(span, fmt.Errorf("getting first console: %w", err)) + return nil, fmt.Errorf("getting first console: %w", err) + } + + cmd, err := allowedcmd.Launchctl( + ctx, + "asuser", + u.Uid, + "sudo", + "--preserve-env", + "-u", + u.Username, + ses.pathToLauncherBinary, + "secure-enclave", + SignCmd, + signRequestB64, + ) + + if err != nil { + traces.SetError(span, fmt.Errorf("creating sign cmd: %w", err)) + return nil, fmt.Errorf("creating sign cmd:: %w", err) + } + + // skip updates since we have full path of binary + cmd.Env = append(cmd.Environ(), fmt.Sprintf("%s=%s", "LAUNCHER_SKIP_UPDATES", "true")) + out, err := cmd.CombinedOutput() + if err != nil { + traces.SetError(span, fmt.Errorf("executing launcher binary to sign with secure enclave: %w: %s", err, string(out))) + return nil, fmt.Errorf("executing launcher binary to sign with secure enclave: %w: %s", err, string(out)) + } + + return base64.StdEncoding.DecodeString(lastLine(out)) +} + // lastLine returns the last line of the out. // This is needed because laucher sets up a logger by default. // The last line of the output is the public key or signature. diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go index ab834e84d..a1d08d1ff 100644 --- a/ee/secureenclavesigner/secureenclavesigner_test.go +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -6,17 +6,22 @@ package secureenclavesigner import ( "context" "crypto/ecdsa" + "encoding/json" "fmt" "os" "os/exec" "path/filepath" + "strings" "testing" "time" + "github.com/kolide/kit/ulid" + "github.com/kolide/krypto/pkg/challenge" "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/launcher/ee/agent/storage/inmemory" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/stretchr/testify/require" + "github.com/vmihailenco/msgpack/v5" ) const ( @@ -89,27 +94,27 @@ func TestSecureEnclaveSigner(t *testing.T) { "should be able to create secure enclave signer", ) - pubKey := ses.Public() - require.NotNil(t, pubKey, + userPubKey := ses.Public() + require.NotNil(t, userPubKey, "should be able to create brand new public key", ) - pubEcdsaKey := pubKey.(*ecdsa.PublicKey) - require.NotNil(t, pubEcdsaKey, + userPubEcdsaKey := userPubKey.(*ecdsa.PublicKey) + require.NotNil(t, userPubEcdsaKey, "public key should convert to ecdsa key", ) - pubKeySame := ses.Public() - require.NotNil(t, pubKeySame, + userPubKeySame := ses.Public() + require.NotNil(t, userPubKeySame, "should be able to get public key again", ) - pubEcdsaKeySame := pubKeySame.(*ecdsa.PublicKey) - require.NotNil(t, pubEcdsaKeySame, + userPubEcdsaKeySame := userPubKeySame.(*ecdsa.PublicKey) + require.NotNil(t, userPubEcdsaKeySame, "public key should convert to ecdsa key", ) - require.Equal(t, pubEcdsaKey, pubEcdsaKeySame, + require.True(t, userPubEcdsaKey.Equal(userPubEcdsaKeySame), "asking for the same public key should return the same key", ) @@ -128,9 +133,102 @@ func TestSecureEnclaveSigner(t *testing.T) { "public key should convert to ecdsa key", ) - require.Equal(t, pubEcdsaKey, pubEcdsaKeyUnmarshalled, + require.Equal(t, userPubEcdsaKey, pubEcdsaKeyUnmarshalled, "unmarshalled public key should be the same as original public key", ) + + challengeBytes, _, err := challenge.Generate(serverPrivKey, []byte(ulid.New()), []byte(ulid.New()), []byte(ulid.New())) + require.NoError(t, err, + "should be able to generate challenge", + ) + + baseNonce := ulid.New() + signResponseOuterBytes, err := ses.SignConsoleUser(ctx, challengeBytes, []byte(ulid.New()), serverPubKeyDer, baseNonce) + require.NoError(t, err, + "should be able to sign with secure enclave", + ) + + var signResponseOuter SignResponseOuter + require.NoError(t, json.Unmarshal(signResponseOuterBytes, &signResponseOuter), + "should be able to unmarshal sign response outer", + ) + + require.NoError(t, echelper.VerifySignature(userPubEcdsaKey, signResponseOuter.Msg, signResponseOuter.Sig), + "should be able to verify signature", + ) + + var signResponseInner SignResponseInner + require.NoError(t, json.Unmarshal(signResponseOuter.Msg, &signResponseInner), + "should be able to unmarshal sign response inner", + ) + + require.True(t, strings.HasPrefix(string(signResponseInner.Data), "kolide:"), + "signed data should have prefix", + ) + + require.True(t, strings.HasSuffix(string(signResponseInner.Data), ":kolide"), + "signed data should have suffix", + ) + + require.True(t, strings.HasPrefix(signResponseInner.Nonce, baseNonce), + "nonce should have base nonce prefix", + ) + + require.InDelta(t, time.Now().UTC().Unix(), signResponseInner.Timestamp, 5, + "timestamp should be within 5 seconds of now", + ) + + // test some error cases + _, err = ses.SignConsoleUser(ctx, nil, []byte(ulid.New()), serverPubKeyDer, ulid.New()) + require.Error(t, err, + "should not be able to sign with nil challenge", + ) + + _, err = ses.SignConsoleUser(ctx, challengeBytes, []byte(ulid.New()), nil, ulid.New()) + require.Error(t, err, + "should not be able to sign with nil serverkey", + ) + + randomKey, err := echelper.GenerateEcdsaKey() + require.NoError(t, err, + "should be able to generate ecdsa key", + ) + + randomKeyMarshalled, err := echelper.PublicEcdsaToB64Der(randomKey.Public().(*ecdsa.PublicKey)) + require.NoError(t, err, + "should be able to marshal ecdsa key", + ) + + _, err = ses.SignConsoleUser(ctx, challengeBytes, []byte(ulid.New()), randomKeyMarshalled, ulid.New()) + require.Error(t, err, + "should not be able to sign with unrecognized serverkey", + ) + + challengeBoxOuter, err := challenge.UnmarshalChallenge(challengeBytes) + require.NoError(t, err, + "should be able to unmarshal challenge", + ) + + var challengeBoxInner *challenge.InnerChallenge + require.NoError(t, msgpack.Unmarshal(challengeBoxOuter.Msg, &challengeBoxInner), + "should be able to unmarshal inner challenge", + ) + + challengeBoxInner.Timestamp = time.Now().Unix() - 1000000 + challengeBoxOuter.Msg, err = msgpack.Marshal(challengeBoxInner) + require.NoError(t, err, + "should be able to marshal inner challenge", + ) + + challengeBytes, err = challengeBoxOuter.Marshal() + require.NoError(t, err, + "should be able to marshal challenge", + ) + + _, err = ses.SignConsoleUser(ctx, challengeBytes, []byte(ulid.New()), serverPubKeyDer, ulid.New()) + require.ErrorContains(t, err, "invalid signature", + "any tampering should invalidate signature", + ) } // #nosec G306 -- Need readable files diff --git a/ee/secureenclavesigner/test_keys.go b/ee/secureenclavesigner/test_keys.go new file mode 100644 index 000000000..cd17735c2 --- /dev/null +++ b/ee/secureenclavesigner/test_keys.go @@ -0,0 +1,25 @@ +//go:build secure_enclave_test +// +build secure_enclave_test + +package secureenclavesigner + +// Using ldflags to set the pub key and using build tag. +// +// This kind of feels like belt and suspenders. +// +// We could probably drop the build tag and just use the -ldflag, then determine +// if we're under test by checking the value of the var set by the -ldflag, but +// that feels more tangly. +// +// We could also generate a file with the private key, add it's path to .gitignore +// and use that to test + +// Undertest is true when running secure enclave test build +const Undertest = true + +// TestServerPubKey is the public key of the server in DER format +// when building the binary for testing, we set this with -ldflags +// so the wrapper test can sign requests with the private portion +// of the key it used to set this value. +// See secureenclavesigner_test.go +var TestServerPubKey string diff --git a/ee/secureenclavesigner/test_keys_noop.go b/ee/secureenclavesigner/test_keys_noop.go new file mode 100644 index 000000000..1d49eab88 --- /dev/null +++ b/ee/secureenclavesigner/test_keys_noop.go @@ -0,0 +1,11 @@ +//go:build !secure_enclave_test +// +build !secure_enclave_test + +package secureenclavesigner + +// Undertest is true when running secure enclave test build +const Undertest = false + +// TestServerPubKey should never be set outside of testing. +// See test_keys.go. +const TestServerPubKey = "" diff --git a/ee/secureenclavesigner/types.go b/ee/secureenclavesigner/types.go new file mode 100644 index 000000000..5a9af92d0 --- /dev/null +++ b/ee/secureenclavesigner/types.go @@ -0,0 +1,12 @@ +package secureenclavesigner + +type SignResponseInner struct { + Nonce string `json:"nonce"` + Timestamp int64 `json:"timestamp"` + Data []byte `json:"data"` +} + +type SignResponseOuter struct { + Sig []byte `json:"sig"` + Msg []byte `json:"msg"` +} From 54381df121964693b73954fc424c45d6018f04b9 Mon Sep 17 00:00:00 2001 From: james pickett Date: Wed, 20 Mar 2024 14:32:57 -0700 Subject: [PATCH 02/10] comment middleware response --- ee/localserver/krypto-ec-middleware.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index 1a6767519..f8bcbb5f5 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -61,9 +61,22 @@ func (cmdReq v2CmdRequestType) CallbackReq() (*http.Request, error) { return req, nil } +// kryptoEcMiddlewareResponse is a box that contains the response, hardware signature of the response, +// and the public key of the hardware signer. type kryptoEcMiddlewareResponse struct { - Msg string `json:"msg"` + // To validate the response. + // 1. Convert th b64der hardware key field to public key + // 2. Determine if hardware key is trusted + // 3. B64 decode message and hardware sig to bytes + // 4. Use decoded msg, hardware sig, and public key to verify the signature + // 4. After verifying the signature, unmarshall the b64 decoded msg to json + + // Msg is the base64 encoded message that contains timestamp, nonce, and + // the response from the endpoint called. + Msg string `json:"msg"` + // HardwareSig is the base64 encoded signature of the msg HardwareSig string `json:"hardware_sig"` + // HardwareKey is the base64 encoded public key DER of the hardware signer HardwareKey string `json:"hardware_key"` } From a80e9489002dc142e5f397927bec9e7c8d10aeaa Mon Sep 17 00:00:00 2001 From: james pickett Date: Wed, 20 Mar 2024 14:49:53 -0700 Subject: [PATCH 03/10] trying to fix merge conflict --- cmd/launcher/secure_enclave_test.go | 153 ++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 cmd/launcher/secure_enclave_test.go diff --git a/cmd/launcher/secure_enclave_test.go b/cmd/launcher/secure_enclave_test.go new file mode 100644 index 000000000..c4636489c --- /dev/null +++ b/cmd/launcher/secure_enclave_test.go @@ -0,0 +1,153 @@ +//go:build darwin +// +build darwin + +package main + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/secureenclavesigner" + "github.com/stretchr/testify/require" +) + +const ( + testWrappedEnvVarKey = "SECURE_ENCLAVE_TEST_WRAPPED" + testSkipSecureEnclaveTestingEnvVarKey = "SKIP_SECURE_ENCLAVE_TESTS" + macOsAppResourceDir = "../../ee/secureenclavesigner/test_app_resources" +) + +// TestSecureEnclaveTestRunner creates a MacOS app with the binary of this packages tests, then signs the app with entitlements and runs the tests. +// This is done because in order to access secure enclave to run tests, we need MacOS entitlements. +// #nosec G306 -- Need readable files +func TestSecureEnclaveTestRunner(t *testing.T) { + t.Parallel() + + if os.Getenv("CI") != "" { + t.Skipf("\nskipping because %s env var was not empty, this is being run in a CI environment without access to secure enclave", testWrappedEnvVarKey) + } + + if os.Getenv(testWrappedEnvVarKey) != "" { + t.Skipf("\nskipping because %s env var was not empty, this is the execution of the codesigned app with entitlements", testWrappedEnvVarKey) + } + + if os.Getenv(testSkipSecureEnclaveTestingEnvVarKey) != "" { + t.Skipf("\nskipping because %s env var was set", testSkipSecureEnclaveTestingEnvVarKey) + } + + t.Log("\nexecuting wrapped tests with codesigned app and entitlements") + + // set up app bundle + rootDir := t.TempDir() + appRoot := filepath.Join(rootDir, "launcher_test.app") + + // make required dirs launcher_test.app/Contents/MacOS and add files + require.NoError(t, os.MkdirAll(filepath.Join(appRoot, "Contents", "MacOS"), 0700)) + copyFile(t, filepath.Join(macOsAppResourceDir, "Info.plist"), filepath.Join(appRoot, "Contents", "Info.plist")) + copyFile(t, filepath.Join(macOsAppResourceDir, "embedded.provisionprofile"), filepath.Join(appRoot, "Contents", "embedded.provisionprofile")) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + // build an executable containing the tests into the app bundle + executablePath := filepath.Join(appRoot, "Contents", "MacOS", "launcher_test") + out, err := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd + ctx, + "go", + "test", + "-c", + "--cover", + "--race", + "./", + "-o", + executablePath, + ).CombinedOutput() + + require.NoError(t, ctx.Err()) + require.NoError(t, err, string(out)) + + // sign app bundle + signApp(t, appRoot) + + // run app bundle executable + cmd := exec.CommandContext(ctx, executablePath, "-test.v") //nolint:forbidigo // Only used in test, don't want as standard allowedcmd + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", testWrappedEnvVarKey, "true")) + out, err = cmd.CombinedOutput() + require.NoError(t, ctx.Err()) + require.NoError(t, err, string(out)) + + // ensure the test ran successfully + require.Contains(t, string(out), "PASS: TestSecureEnclaveCmd") + require.NotContains(t, string(out), "FAIL") +} + +func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest + if os.Getenv(testWrappedEnvVarKey) == "" { + t.Skipf("\nskipping because %s env var was empty, test not being run from codesigned app with entitlements", testWrappedEnvVarKey) + } + + t.Log("\nrunning wrapped tests with codesigned app and entitlements") + + oldStdout := os.Stdout + defer func() { + os.Stdout = oldStdout + }() + + // create a pipe to capture stdout + pipeReader, pipeWriter, err := os.Pipe() + require.NoError(t, err) + + os.Stdout = pipeWriter + + require.NoError(t, runSecureEnclave([]string{secureenclavesigner.CreateKeyCmd})) + require.NoError(t, pipeWriter.Close()) + + var buf bytes.Buffer + _, err = buf.ReadFrom(pipeReader) + require.NoError(t, err) + + // convert response to public key + createKeyResponse := buf.Bytes() + secureEnclavePubKey, err := echelper.PublicB64DerToEcdsaKey(createKeyResponse) + require.NoError(t, err) + require.NotNil(t, secureEnclavePubKey, "should be able to get public key") +} + +// #nosec G306 -- Need readable files +func copyFile(t *testing.T, source, destination string) { + bytes, err := os.ReadFile(source) + require.NoError(t, err) + require.NoError(t, os.WriteFile(destination, bytes, 0700)) +} + +// #nosec G204 -- This triggers due to using env var in cmd, making exception for test +func signApp(t *testing.T, appRootDir string) { + codeSignId := os.Getenv("MACOS_CODESIGN_IDENTITY") + require.NotEmpty(t, codeSignId, "need MACOS_CODESIGN_IDENTITY env var to sign app, such as [Mac Developer: Jane Doe (ABCD123456)]") + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + cmd := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd + ctx, + "codesign", + "--deep", + "--force", + "--options", "runtime", + "--entitlements", filepath.Join(macOsAppResourceDir, "entitlements"), + "--sign", codeSignId, + "--timestamp", + appRootDir, + ) + + out, err := cmd.CombinedOutput() + require.NoError(t, ctx.Err()) + require.NoError(t, err, string(out)) +} From 6f87f49d94cef28962f7f4eae2be0091f524989f Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 21 Mar 2024 07:08:38 -0700 Subject: [PATCH 04/10] remove secure enclave direct cmd tests --- cmd/launcher/secure_enclave_test.go | 153 ---------------------------- 1 file changed, 153 deletions(-) delete mode 100644 cmd/launcher/secure_enclave_test.go diff --git a/cmd/launcher/secure_enclave_test.go b/cmd/launcher/secure_enclave_test.go deleted file mode 100644 index c4636489c..000000000 --- a/cmd/launcher/secure_enclave_test.go +++ /dev/null @@ -1,153 +0,0 @@ -//go:build darwin -// +build darwin - -package main - -import ( - "bytes" - "context" - "fmt" - "os" - "os/exec" - "path/filepath" - "testing" - "time" - - "github.com/kolide/krypto/pkg/echelper" - "github.com/kolide/launcher/ee/secureenclavesigner" - "github.com/stretchr/testify/require" -) - -const ( - testWrappedEnvVarKey = "SECURE_ENCLAVE_TEST_WRAPPED" - testSkipSecureEnclaveTestingEnvVarKey = "SKIP_SECURE_ENCLAVE_TESTS" - macOsAppResourceDir = "../../ee/secureenclavesigner/test_app_resources" -) - -// TestSecureEnclaveTestRunner creates a MacOS app with the binary of this packages tests, then signs the app with entitlements and runs the tests. -// This is done because in order to access secure enclave to run tests, we need MacOS entitlements. -// #nosec G306 -- Need readable files -func TestSecureEnclaveTestRunner(t *testing.T) { - t.Parallel() - - if os.Getenv("CI") != "" { - t.Skipf("\nskipping because %s env var was not empty, this is being run in a CI environment without access to secure enclave", testWrappedEnvVarKey) - } - - if os.Getenv(testWrappedEnvVarKey) != "" { - t.Skipf("\nskipping because %s env var was not empty, this is the execution of the codesigned app with entitlements", testWrappedEnvVarKey) - } - - if os.Getenv(testSkipSecureEnclaveTestingEnvVarKey) != "" { - t.Skipf("\nskipping because %s env var was set", testSkipSecureEnclaveTestingEnvVarKey) - } - - t.Log("\nexecuting wrapped tests with codesigned app and entitlements") - - // set up app bundle - rootDir := t.TempDir() - appRoot := filepath.Join(rootDir, "launcher_test.app") - - // make required dirs launcher_test.app/Contents/MacOS and add files - require.NoError(t, os.MkdirAll(filepath.Join(appRoot, "Contents", "MacOS"), 0700)) - copyFile(t, filepath.Join(macOsAppResourceDir, "Info.plist"), filepath.Join(appRoot, "Contents", "Info.plist")) - copyFile(t, filepath.Join(macOsAppResourceDir, "embedded.provisionprofile"), filepath.Join(appRoot, "Contents", "embedded.provisionprofile")) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - // build an executable containing the tests into the app bundle - executablePath := filepath.Join(appRoot, "Contents", "MacOS", "launcher_test") - out, err := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd - ctx, - "go", - "test", - "-c", - "--cover", - "--race", - "./", - "-o", - executablePath, - ).CombinedOutput() - - require.NoError(t, ctx.Err()) - require.NoError(t, err, string(out)) - - // sign app bundle - signApp(t, appRoot) - - // run app bundle executable - cmd := exec.CommandContext(ctx, executablePath, "-test.v") //nolint:forbidigo // Only used in test, don't want as standard allowedcmd - cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", testWrappedEnvVarKey, "true")) - out, err = cmd.CombinedOutput() - require.NoError(t, ctx.Err()) - require.NoError(t, err, string(out)) - - // ensure the test ran successfully - require.Contains(t, string(out), "PASS: TestSecureEnclaveCmd") - require.NotContains(t, string(out), "FAIL") -} - -func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest - if os.Getenv(testWrappedEnvVarKey) == "" { - t.Skipf("\nskipping because %s env var was empty, test not being run from codesigned app with entitlements", testWrappedEnvVarKey) - } - - t.Log("\nrunning wrapped tests with codesigned app and entitlements") - - oldStdout := os.Stdout - defer func() { - os.Stdout = oldStdout - }() - - // create a pipe to capture stdout - pipeReader, pipeWriter, err := os.Pipe() - require.NoError(t, err) - - os.Stdout = pipeWriter - - require.NoError(t, runSecureEnclave([]string{secureenclavesigner.CreateKeyCmd})) - require.NoError(t, pipeWriter.Close()) - - var buf bytes.Buffer - _, err = buf.ReadFrom(pipeReader) - require.NoError(t, err) - - // convert response to public key - createKeyResponse := buf.Bytes() - secureEnclavePubKey, err := echelper.PublicB64DerToEcdsaKey(createKeyResponse) - require.NoError(t, err) - require.NotNil(t, secureEnclavePubKey, "should be able to get public key") -} - -// #nosec G306 -- Need readable files -func copyFile(t *testing.T, source, destination string) { - bytes, err := os.ReadFile(source) - require.NoError(t, err) - require.NoError(t, os.WriteFile(destination, bytes, 0700)) -} - -// #nosec G204 -- This triggers due to using env var in cmd, making exception for test -func signApp(t *testing.T, appRootDir string) { - codeSignId := os.Getenv("MACOS_CODESIGN_IDENTITY") - require.NotEmpty(t, codeSignId, "need MACOS_CODESIGN_IDENTITY env var to sign app, such as [Mac Developer: Jane Doe (ABCD123456)]") - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - cmd := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd - ctx, - "codesign", - "--deep", - "--force", - "--options", "runtime", - "--entitlements", filepath.Join(macOsAppResourceDir, "entitlements"), - "--sign", codeSignId, - "--timestamp", - appRootDir, - ) - - out, err := cmd.CombinedOutput() - require.NoError(t, ctx.Err()) - require.NoError(t, err, string(out)) -} From 254d921f3acf61f35f0476ea13315650e4273204 Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 21 Mar 2024 12:42:21 -0700 Subject: [PATCH 05/10] fix tests, remove debug comments --- cmd/launcher/secure_enclave_darwin.go | 6 +- .../krypto-ec-middleware-response_darwin.go | 60 +++---------------- .../krypto-ec-middleware-response_other.go | 46 +++++++------- ee/localserver/krypto-ec-middleware.go | 44 +++++++++++--- 4 files changed, 68 insertions(+), 88 deletions(-) diff --git a/cmd/launcher/secure_enclave_darwin.go b/cmd/launcher/secure_enclave_darwin.go index 7d915f98c..362838ee8 100644 --- a/cmd/launcher/secure_enclave_darwin.go +++ b/cmd/launcher/secure_enclave_darwin.go @@ -168,11 +168,9 @@ func loadSecureEnclaveServerPubKey(b64Key string) (*ecdsa.PublicKey, error) { return nil, fmt.Errorf("parsing test server public key: %w", err) } - if !providedKey.Equal(k) { - return nil, errors.New("provided server public key does not match test server public key") + if providedKey.Equal(k) { + return k, nil } - - return k, nil } for _, serverKey := range []string{certs.K2EccServerCert, certs.ReviewEccServerCert, certs.LocalhostEccServerCert} { diff --git a/ee/localserver/krypto-ec-middleware-response_darwin.go b/ee/localserver/krypto-ec-middleware-response_darwin.go index d566cf82a..86c5beee0 100644 --- a/ee/localserver/krypto-ec-middleware-response_darwin.go +++ b/ee/localserver/krypto-ec-middleware-response_darwin.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "log/slog" - "time" "github.com/kolide/kit/ulid" "github.com/kolide/krypto/pkg/challenge" @@ -20,39 +19,16 @@ import ( "github.com/kolide/launcher/pkg/traces" ) -func (e *kryptoEcMiddleware) challengeResponse(ctx context.Context, o *challenge.OuterChallenge, data []byte) ([]byte, error) { +func (e *kryptoEcMiddleware) generateChallengeResponse(ctx context.Context, o *challenge.OuterChallenge, data []byte) ([]byte, error) { ctx, span := traces.StartSpan(ctx) defer span.End() - inner := secureenclavesigner.SignResponseInner{ - Nonce: ulid.New(), - Timestamp: time.Now().UTC().Unix(), - Data: data, - } - if e.hardwareSigner == nil || e.hardwareSigner.Public() == nil { e.slogger.Log(ctx, slog.LevelInfo, "no hardware signer available", ) traces.SetError(span, errors.New("no hardware signer available")) - - // add the kolide:%s:kolide tags since the secure enclave cmd - // wont be execed and add them - inner.Data = []byte(fmt.Sprintf("kolide:%s:kolide", string(inner.Data))) - innerBytes, err := json.Marshal(inner) - if err != nil { - return nil, fmt.Errorf("marshalling krypto middleware inner response: %w", err) - } - - outerBytes, err := json.Marshal(kryptoEcMiddlewareResponse{ - Msg: base64.StdEncoding.EncodeToString(innerBytes), - }) - - if err != nil { - return nil, fmt.Errorf("marshalling krypto middleware outer response: %w", err) - } - - return o.Respond(e.localDbSigner, nil, outerBytes) + return e.responseWithoutHardwareSig(o, data) } challengeBytes, err := o.Marshal() @@ -65,31 +41,14 @@ func (e *kryptoEcMiddleware) challengeResponse(ctx context.Context, o *challenge return nil, fmt.Errorf("converting counter party public key to bytes: %w", err) } - signResponseOuterBytes, err := e.hardwareSigner.SignConsoleUser(ctx, challengeBytes, data, serverKeyBytes, inner.Nonce) + signResponseOuterBytes, err := e.hardwareSigner.SignConsoleUser(ctx, challengeBytes, data, serverKeyBytes, ulid.New()) if err != nil { e.slogger.Log(ctx, slog.LevelError, "signing with console user hardware key", "err", err, ) traces.SetError(span, fmt.Errorf("signing with console user hardware key, %w", err)) - - // add the kolide:%s:kolide tags since the secure enclave cmd failed - // and did not add them - inner.Data = []byte(fmt.Sprintf("kolide:%s:kolide", string(inner.Data))) - innerBytes, err := json.Marshal(inner) - if err != nil { - return nil, fmt.Errorf("marshalling krypto middleware inner response: %w", err) - } - - outerBytes, err := json.Marshal(kryptoEcMiddlewareResponse{ - Msg: base64.StdEncoding.EncodeToString(innerBytes), - }) - - if err != nil { - return nil, fmt.Errorf("marshalling krypto middleware outer response: %w", err) - } - - return o.Respond(e.localDbSigner, nil, outerBytes) + return e.responseWithoutHardwareSig(o, data) } hardwareKeyBytes, err := echelper.PublicEcdsaToB64Der(e.hardwareSigner.Public().(*ecdsa.PublicKey)) @@ -107,12 +66,11 @@ func (e *kryptoEcMiddleware) challengeResponse(ctx context.Context, o *challenge return nil, fmt.Errorf("unmarshaling sign response inner: %w", err) } - // the secure enclave cmd will add the kolide:%s:kolide tags - inner.Data = signResponseInner.Data - // use nonce from the secure enclave cmd - inner.Nonce = signResponseInner.Nonce - // use timestamp from the secure enclave cmd - inner.Timestamp = signResponseInner.Timestamp + inner := secureenclavesigner.SignResponseInner{ + Nonce: signResponseInner.Nonce, + Timestamp: signResponseInner.Timestamp, + Data: signResponseInner.Data, + } innerBytes, err := json.Marshal(inner) if err != nil { diff --git a/ee/localserver/krypto-ec-middleware-response_other.go b/ee/localserver/krypto-ec-middleware-response_other.go index 86232fa81..578af6b0e 100644 --- a/ee/localserver/krypto-ec-middleware-response_other.go +++ b/ee/localserver/krypto-ec-middleware-response_other.go @@ -8,6 +8,7 @@ import ( "crypto" "crypto/ecdsa" "crypto/rand" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -21,54 +22,51 @@ import ( "github.com/kolide/launcher/pkg/traces" ) -func (e *kryptoEcMiddleware) challengeResponse(ctx context.Context, o *challenge.OuterChallenge, data []byte) ([]byte, error) { +func (e *kryptoEcMiddleware) generateChallengeResponse(ctx context.Context, o *challenge.OuterChallenge, data []byte) ([]byte, error) { ctx, span := traces.StartSpan(ctx) defer span.End() - response := &secureenclavesigner.SignResponseInner{ - Nonce: ulid.New(), - Timestamp: time.Now().UTC().Unix(), - Data: []byte(fmt.Sprintf("kolide:%s:kolide", string(data))), - } - if e.hardwareSigner == nil || e.hardwareSigner.Public() == nil { e.slogger.Log(ctx, slog.LevelError, "no hardware signer available", ) traces.SetError(span, errors.New("no hardware signer available")) - responseBytes, err := json.Marshal(response) - if err != nil { - return nil, fmt.Errorf("marshalling krypto response: %w", err) - } + return e.responseWithoutHardwareSig(o, data) + } + + inner := secureenclavesigner.SignResponseInner{ + Nonce: ulid.New(), + Timestamp: time.Now().UTC().Unix(), + Data: []byte(fmt.Sprintf("kolide:%s:kolide", data)), + } - return o.Respond(e.localDbSigner, nil, responseBytes) + innerBytes, err := json.Marshal(inner) + if err != nil { + return nil, fmt.Errorf("marshalling inner response: %w", err) } - hwSig, err := e.hardwareSigner.Sign(rand.Reader, data, crypto.SHA256) + hwSig, err := e.hardwareSigner.Sign(rand.Reader, innerBytes, crypto.SHA256) if err != nil { e.slogger.Log(ctx, slog.LevelError, "signing with hardware signer", "err", err, ) traces.SetError(span, fmt.Errorf("signing with hardware signer, %w", err)) - - responseBytes, err := json.Marshal(response) - if err != nil { - return nil, fmt.Errorf("marshalling krypto response: %w", err) - } - - return o.Respond(e.localDbSigner, nil, responseBytes) + return e.responseWithoutHardwareSig(o, data) } - response.HardwareSig = hwSig - - response.HardwareKey, err = echelper.PublicEcdsaToB64Der(e.hardwareSigner.Public().(*ecdsa.PublicKey)) + hwKey, err := echelper.PublicEcdsaToB64Der(e.hardwareSigner.Public().(*ecdsa.PublicKey)) if err != nil { return nil, fmt.Errorf("marshalling public signing key to der: %w", err) } - responseBytes, err := json.Marshal(response) + responseBytes, err := json.Marshal(kryptoEcMiddlewareResponse{ + Msg: base64.StdEncoding.EncodeToString(innerBytes), + HardwareSig: base64.StdEncoding.EncodeToString(hwSig), + HardwareKey: string(hwKey), + }) + if err != nil { return nil, fmt.Errorf("marshalling krypto response: %w", err) } diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index f8bcbb5f5..a79cd4cc6 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -16,9 +16,11 @@ import ( "strings" "time" + "github.com/kolide/kit/ulid" "github.com/kolide/krypto" "github.com/kolide/krypto/pkg/challenge" "github.com/kolide/launcher/ee/agent" + "github.com/kolide/launcher/ee/secureenclavesigner" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/kolide/launcher/pkg/traces" "go.opentelemetry.io/otel/attribute" @@ -241,14 +243,13 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { // Check if the origin is in the allowed list. See https://github.com/kolide/k2/issues/9634 if len(cmdReq.AllowedOrigins) > 0 { - allowed := true - // origin := r.Header.Get("Origin") - // for _, ao := range cmdReq.AllowedOrigins { - // if strings.EqualFold(ao, origin) { - // allowed = true - // break - // } - // } + allowed := false + for _, ao := range cmdReq.AllowedOrigins { + if strings.EqualFold(ao, r.Header.Get("Origin")) { + allowed = true + break + } + } if !allowed { span.SetAttributes(attribute.String("origin", r.Header.Get("Origin"))) @@ -322,7 +323,7 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { next.ServeHTTP(bhr, newReq) // call platform specific response function to add hardware signatures - response, err := e.challengeResponse(r.Context(), challengeBox, bhr.Bytes()) + response, err := e.generateChallengeResponse(r.Context(), challengeBox, bhr.Bytes()) if err != nil { traces.SetError(span, err) @@ -393,6 +394,31 @@ func extractChallenge(r *http.Request) (*challenge.OuterChallenge, error) { return challenge.UnmarshalChallenge(decoded) } +func (e *kryptoEcMiddleware) responseWithoutHardwareSig(o *challenge.OuterChallenge, data []byte) ([]byte, error) { + msg := secureenclavesigner.SignResponseInner{ + Nonce: ulid.New(), + Timestamp: time.Now().UTC().Unix(), + // add the kolide:%s:kolide tags since the secure enclave cmd + // wont be execed and add them + Data: []byte(fmt.Sprintf("kolide:%s:kolide", string(data))), + } + + msgBytes, err := json.Marshal(msg) + if err != nil { + return nil, fmt.Errorf("marshalling krypto response: %w", err) + } + + outerResponseBytes, err := json.Marshal(kryptoEcMiddlewareResponse{ + Msg: base64.StdEncoding.EncodeToString(msgBytes), + }) + + if err != nil { + return nil, fmt.Errorf("marshalling krypto response: %w", err) + } + + return o.Respond(e.localDbSigner, nil, outerResponseBytes) +} + type bufferedHttpResponse struct { header http.Header code int From 3752eb47bdb680f045d6bdd9f71a239e7362a65d Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 21 Mar 2024 12:49:03 -0700 Subject: [PATCH 06/10] reenable test cases --- ee/localserver/krypto-ec-middleware_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/localserver/krypto-ec-middleware_test.go b/ee/localserver/krypto-ec-middleware_test.go index 1eb181338..7617533b3 100644 --- a/ee/localserver/krypto-ec-middleware_test.go +++ b/ee/localserver/krypto-ec-middleware_test.go @@ -163,7 +163,7 @@ func TestKryptoEcMiddleware(t *testing.T) { challengeBytes, privateEncryptionKey := tt.challenge() encodedChallenge := base64.StdEncoding.EncodeToString(challengeBytes) - for _, req := range []*http.Request{makeGetRequest(t, encodedChallenge) /*makePostRequest(t, encodedChallenge)*/} { + for _, req := range []*http.Request{makeGetRequest(t, encodedChallenge), makePostRequest(t, encodedChallenge)} { req := req t.Run(req.Method, func(t *testing.T) { t.Parallel() From 172d819317c4976b1664ca70715a5166e7f66ade Mon Sep 17 00:00:00 2001 From: james pickett Date: Tue, 26 Mar 2024 10:08:20 -0700 Subject: [PATCH 07/10] simplify, comment --- .../krypto-ec-middleware-response_darwin.go | 18 +----------------- ee/localserver/krypto-ec-middleware.go | 5 ++++- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/ee/localserver/krypto-ec-middleware-response_darwin.go b/ee/localserver/krypto-ec-middleware-response_darwin.go index 86c5beee0..209739084 100644 --- a/ee/localserver/krypto-ec-middleware-response_darwin.go +++ b/ee/localserver/krypto-ec-middleware-response_darwin.go @@ -61,24 +61,8 @@ func (e *kryptoEcMiddleware) generateChallengeResponse(ctx context.Context, o *c return nil, fmt.Errorf("unmarshaling sign response outer: %w", err) } - var signResponseInner secureenclavesigner.SignResponseInner - if err := json.Unmarshal(signResponseOuter.Msg, &signResponseInner); err != nil { - return nil, fmt.Errorf("unmarshaling sign response inner: %w", err) - } - - inner := secureenclavesigner.SignResponseInner{ - Nonce: signResponseInner.Nonce, - Timestamp: signResponseInner.Timestamp, - Data: signResponseInner.Data, - } - - innerBytes, err := json.Marshal(inner) - if err != nil { - return nil, fmt.Errorf("marshalling krypto middleware inner response: %w", err) - } - outer := kryptoEcMiddlewareResponse{ - Msg: base64.StdEncoding.EncodeToString(innerBytes), + Msg: base64.StdEncoding.EncodeToString(signResponseOuter.Msg), HardwareSig: base64.StdEncoding.EncodeToString(signResponseOuter.Sig), HardwareKey: string(hardwareKeyBytes), } diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index a79cd4cc6..75c21657d 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -66,12 +66,15 @@ func (cmdReq v2CmdRequestType) CallbackReq() (*http.Request, error) { // kryptoEcMiddlewareResponse is a box that contains the response, hardware signature of the response, // and the public key of the hardware signer. type kryptoEcMiddlewareResponse struct { - // To validate the response. + // This ends up in the "ResponseData" field of the challenge response. + + // To validate: // 1. Convert th b64der hardware key field to public key // 2. Determine if hardware key is trusted // 3. B64 decode message and hardware sig to bytes // 4. Use decoded msg, hardware sig, and public key to verify the signature // 4. After verifying the signature, unmarshall the b64 decoded msg to json + // 5. Do stuff with the data. // Msg is the base64 encoded message that contains timestamp, nonce, and // the response from the endpoint called. From c995ddd7faaf18b4bcf318bc4cb58fbde6b78998 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Tue, 26 Mar 2024 12:48:19 -0700 Subject: [PATCH 08/10] hash before sign on non darwin --- ee/localserver/krypto-ec-middleware-response_other.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ee/localserver/krypto-ec-middleware-response_other.go b/ee/localserver/krypto-ec-middleware-response_other.go index 578af6b0e..204b8f00f 100644 --- a/ee/localserver/krypto-ec-middleware-response_other.go +++ b/ee/localserver/krypto-ec-middleware-response_other.go @@ -46,7 +46,12 @@ func (e *kryptoEcMiddleware) generateChallengeResponse(ctx context.Context, o *c return nil, fmt.Errorf("marshalling inner response: %w", err) } - hwSig, err := e.hardwareSigner.Sign(rand.Reader, innerBytes, crypto.SHA256) + hash, err := echelper.HashForSignature(innerBytes) + if err != nil { + return nil, fmt.Errorf("hashing inner response: %w", err) + } + + hwSig, err := e.hardwareSigner.Sign(rand.Reader, hash, crypto.SHA256) if err != nil { e.slogger.Log(ctx, slog.LevelError, "signing with hardware signer", From 2d08dc6d3de3f2b44f5314572158360e7a7a4b03 Mon Sep 17 00:00:00 2001 From: james pickett Date: Tue, 26 Mar 2024 12:50:37 -0700 Subject: [PATCH 09/10] uncomment allowed origin tests --- ee/localserver/krypto-ec-middleware_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/ee/localserver/krypto-ec-middleware_test.go b/ee/localserver/krypto-ec-middleware_test.go index 7617533b3..dbcad6d7d 100644 --- a/ee/localserver/krypto-ec-middleware_test.go +++ b/ee/localserver/krypto-ec-middleware_test.go @@ -374,11 +374,18 @@ func Test_AllowedOrigin(t *testing.T) { require.NoError(t, err) require.Equal(t, challengeData, opened.ChallengeData) - // var kryptoEcMiddlewareResponse kryptoEcMiddlewareResponse - // require.NoError(t, json.Unmarshal(opened.ResponseData, &kryptoEcMiddlewareResponse)) + var kryptoEcMiddlewareResponseOuter kryptoEcMiddlewareResponse + require.NoError(t, json.Unmarshal(opened.ResponseData, &kryptoEcMiddlewareResponseOuter)) - // require.Equal(t, []byte(fmt.Sprintf("kolide:%s:kolide", responseData)), kryptoEcMiddlewareResponse.Data) - // require.WithinDuration(t, time.Now(), time.Unix(opened.Timestamp, 0), time.Second*5) + var kryptoEcMiddlewareResponseInner secureenclavesigner.SignResponseInner + kryptoEcMiddlewareResponseInnerBytes, err := base64.StdEncoding.DecodeString(kryptoEcMiddlewareResponseOuter.Msg) + require.NoError(t, err) + + require.NoError(t, json.Unmarshal(kryptoEcMiddlewareResponseInnerBytes, &kryptoEcMiddlewareResponseInner)) + + require.Equal(t, []byte(fmt.Sprintf("kolide:%s:kolide", responseData)), kryptoEcMiddlewareResponseInner.Data) + + require.WithinDuration(t, time.Now(), time.Unix(opened.Timestamp, 0), time.Second*5) }) } From 4856154d72aaa9932d3da2bdb5161851630d5fc0 Mon Sep 17 00:00:00 2001 From: james pickett Date: Tue, 26 Mar 2024 13:23:08 -0700 Subject: [PATCH 10/10] add changes from other PR --- .../secureenclavesigner_darwin.go | 2 + .../secureenclavesigner_test.go | 4 +- .../test_app_resources/info.plist | 2 +- pkg/osquery/table/launcher_info.go | 41 +++++++++++++++---- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/ee/secureenclavesigner/secureenclavesigner_darwin.go b/ee/secureenclavesigner/secureenclavesigner_darwin.go index fe131ae3c..e2214e7c6 100644 --- a/ee/secureenclavesigner/secureenclavesigner_darwin.go +++ b/ee/secureenclavesigner/secureenclavesigner_darwin.go @@ -85,6 +85,8 @@ func New(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDele opt(ses) } + // this is here to facilitate testing, since go builds a special test binary, + // if we look for os.Executable in a test and try to exec it, it will error if ses.pathToLauncherBinary == "" { p, err := os.Executable() if err != nil { diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go index ecad0d04d..91b24fdf7 100644 --- a/ee/secureenclavesigner/secureenclavesigner_test.go +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -50,9 +50,9 @@ func TestSecureEnclaveSigner(t *testing.T) { // put the root dir somewhere else if you want to persist the signed macos app bundle // should build this into make at some point - rootDir := "/tmp/secure_enclave_test" + // rootDir := "/tmp/secure_enclave_test" - // rootDir := t.TempDir() + rootDir := t.TempDir() appRoot := filepath.Join(rootDir, "launcher_test.app") // make required dirs krypto_test.app/Contents/MacOS and add files diff --git a/ee/secureenclavesigner/test_app_resources/info.plist b/ee/secureenclavesigner/test_app_resources/info.plist index fe801acec..72bd0bfa7 100644 --- a/ee/secureenclavesigner/test_app_resources/info.plist +++ b/ee/secureenclavesigner/test_app_resources/info.plist @@ -5,7 +5,7 @@ CFBundleExecutable launcher_test CFBundleIdentifier - com.kolide.agent + com.launcher.test CFBundleName launcher_test LSUIElement diff --git a/pkg/osquery/table/launcher_info.go b/pkg/osquery/table/launcher_info.go index efbfb4b17..41e794b29 100644 --- a/pkg/osquery/table/launcher_info.go +++ b/pkg/osquery/table/launcher_info.go @@ -36,12 +36,18 @@ func LauncherInfoTable(store types.GetterSetter) *table.Plugin { // Exposure of both hardware and local keys table.TextColumn("local_key"), - table.TextColumn("hardware_key"), - table.TextColumn("hardware_key_source"), + table.TextColumn("hardware_keys"), + table.TextColumn("hardware_keys_source"), // Old RSA Key table.TextColumn("fingerprint"), table.TextColumn("public_key"), + + // Old hardware key info + // left here to not break existing queries + // can be removed after server side is updated + table.TextColumn("hardware_key"), + table.TextColumn("hardware_key_source"), } return table.NewPlugin("kolide_launcher_info", columns, generateLauncherInfoTable(store)) } @@ -105,16 +111,37 @@ func generateLauncherInfoTable(store types.GetterSetter) table.GenerateFunc { if err != nil { return nil, fmt.Errorf("marshalling hardware keys: %w", err) } - results[0]["hardware_key"] = string(jsonBytes) - results[0]["hardware_key_source"] = agent.HardwareKeys().Type() + + // for darwin we'll have an array of uid / key pairs looking this + // [{"uid":"501","pub_key":"PUB_KEY_B64_DER"}, {"uid":"502","pub_key":"PUB_KEY_B64_DER"}] + results[0]["hardware_keys"] = string(jsonBytes) + results[0]["hardware_keys_source"] = agent.HardwareKeys().Type() return results, nil } if hardwareKeyDer, err := x509.MarshalPKIXPublicKey(agent.HardwareKeys().Public()); err == nil { - // der is a binary format, so convert to b64 - results[0]["hardware_key"] = base64.StdEncoding.EncodeToString(hardwareKeyDer) - results[0]["hardware_key_source"] = agent.HardwareKeys().Type() + // for windows and linux we just have a single key, but we want data to be in a consistent format, so update it to look like + // the darwin format + keys := []struct { + Uid string `json:"uid"` + PubKey string `json:"pub_key"` + }{ + { + // the uid is irrelevant for windows and linux, so just use -1 + // since hardware keys are not tied to user + Uid: "-1", + PubKey: base64.StdEncoding.EncodeToString(hardwareKeyDer), + }, + } + + jsonBytes, err := json.Marshal(keys) + if err != nil { + return nil, fmt.Errorf("marshalling hardware keys: %w", err) + } + + results[0]["hardware_keys"] = string(jsonBytes) + results[0]["hardware_keys_source"] = agent.HardwareKeys().Type() } return results, nil