just sign challenge msg after verifying

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
 }

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()))
-		})
-	}
-}

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 {

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

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,
 	}