Merge pull request #48 from buildkite/feat_support_crypto_signer

Add support for crypto.Signer when signing and verifying signatures

signature/fixtures/crypto_signer/P256/private.pem

@@ -0,0 +1,5 @@
+-----BEGIN PRIVATE KEY-----
+MHcCAQEEIKG2wc1cI8aGVw4f1GhMfGbhMiDZAE1NeNAyil63wbXPoAoGCCqGSM49
+AwEHoUQDQgAETbursv8d9C1Fa/89/d2FqSo2sw/zeke9F0IWiOKGJuuXENVcp8U5
+FfxStK9sKVH4tPMCpVNPNr1pMa/3NFVlLA==
+-----END PRIVATE KEY-----

signature/fixtures/crypto_signer/P256/public.pem

@@ -0,0 +1,4 @@
+-----BEGIN PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAETbursv8d9C1Fa/89/d2FqSo2sw/z
+eke9F0IWiOKGJuuXENVcp8U5FfxStK9sKVH4tPMCpVNPNr1pMa/3NFVlLA==
+-----END PUBLIC KEY-----

signature/sign.go

@@ -4,14 +4,15 @@ import (
 	"context"
 	"crypto"
 	"crypto/sha256"
-	"encoding/hex"
+	"crypto/x509"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"sort"
 
 	"github.com/buildkite/go-pipeline"
 	"github.com/gowebpki/jcs"
+	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/lestrrat-go/jwx/v2/jws"
 )
@@ -73,9 +74,14 @@ func configureOptions(opts ...Option) options {
 	return options
 }
 
+type Key interface {
+	Algorithm() jwa.KeyAlgorithm
+}
+
 // Sign computes a new signature for an environment (env) combined with an
-// object containing values (sf) using a given key.
-func Sign(_ context.Context, key jwk.Key, sf SignedFielder, opts ...Option) (*pipeline.Signature, error) {
+// object containing values (sf) using a given key. The key can be a jwk.Key
+// or a crypto.Signer. If it is a jwk.Key, the public key thumbprint is logged.
+func Sign(_ context.Context, key Key, sf SignedFielder, opts ...Option) (*pipeline.Signature, error) {
 	options := configureOptions(opts...)
 
 	values, err := sf.SignedFields()
@@ -113,16 +119,28 @@ func Sign(_ context.Context, key jwk.Key, sf SignedFielder, opts ...Option) (*pi
 		return nil, err
 	}
 
-	if options.logger != nil {
+	switch key := key.(type) {
+	case jwk.Key:
 		pk, err := key.PublicKey()
 		if err != nil {
 			return nil, fmt.Errorf("unable to generate public key: %w", err)
 		}
+
 		fingerprint, err := pk.Thumbprint(crypto.SHA256)
 		if err != nil {
 			return nil, fmt.Errorf("calculating key thumbprint: %w", err)
 		}
-		debug(options.logger, "Public Key Thumbprint (sha256): %s", hex.EncodeToString(fingerprint))
+
+		debug(options.logger, "Public Key Thumbprint (sha256): %x", fingerprint)
+	case crypto.Signer:
+		data, err := x509.MarshalPKIXPublicKey(key.Public())
+		if err != nil {
+			return nil, fmt.Errorf("failed to marshal public key: %w", err)
+		}
+
+		debug(options.logger, "Public Key Thumbprint (sha256): %x", sha256.Sum256(data))
+	default:
+		panic(fmt.Sprintf("unsupported key type: %T", key)) // should never happen
 	}
 
 	if options.debugSigning {
@@ -146,8 +164,9 @@ func Sign(_ context.Context, key jwk.Key, sf SignedFielder, opts ...Option) (*pi
 }
 
 // Verify verifies an existing signature against environment (env) combined with
-// an object containing values (sf) using keys from a keySet.
-func Verify(ctx context.Context, s *pipeline.Signature, keySet jwk.Set, sf SignedFielder, opts ...Option) error {
+// the keyset. The keySet can be a jwk.Set or a crypto.Signer. If it is a jwk.Set,
+// the public key thumbprints are logged.
+func Verify(ctx context.Context, s *pipeline.Signature, keySet any, sf SignedFielder, opts ...Option) error {
 	options := configureOptions(opts...)
 
 	if len(s.SignedFields) == 0 {
@@ -187,22 +206,40 @@ func Verify(ctx context.Context, s *pipeline.Signature, keySet jwk.Set, sf Signe
 		return err
 	}
 
-	for it := keySet.Keys(ctx); it.Next(ctx); {
-		pair := it.Pair()
-		publicKey := pair.Value.(jwk.Key)
-		fingerprint, err := publicKey.Thumbprint(crypto.SHA256)
+	if options.debugSigning {
+		debug(options.logger, "Signed Step: %s checksum: %x", payload, sha256.Sum256(payload))
+	}
+
+	var keyOpt jws.VerifyOption
+	switch keySet := keySet.(type) {
+	case jwk.Set:
+		for it := keySet.Keys(ctx); it.Next(ctx); {
+			pair := it.Pair()
+			publicKey := pair.Value.(jwk.Key)
+			fingerprint, err := publicKey.Thumbprint(crypto.SHA256)
+			if err != nil {
+				return fmt.Errorf("calculating key thumbprint: %w", err)
+			}
+
+			debug(options.logger, "Public Key Thumbprint (sha256): %x", fingerprint)
+		}
+
+		keyOpt = jws.WithKeySet(keySet)
+	case crypto.Signer:
+		data, err := x509.MarshalPKIXPublicKey(keySet.Public())
 		if err != nil {
-			return fmt.Errorf("calculating key thumbprint: %w", err)
+			return fmt.Errorf("failed to marshal public key: %w", err)
 		}
-		debug(options.logger, "Public Key Thumbprint (sha256): %s", hex.EncodeToString(fingerprint))
-	}
 
-	if options.debugSigning {
-		debug(options.logger, "Signed Step: %s checksum: %x", payload, sha256.Sum256(payload))
+		debug(options.logger, "Public Key Thumbprint (sha256): %x", sha256.Sum256(data))
+
+		keyOpt = jws.WithKey(jwa.ES256, keySet)
+	default:
+		panic(fmt.Sprintf("unsupported key type: %T", keySet)) // should never happen
 	}
 
 	_, err = jws.Verify([]byte(s.Value),
-		jws.WithKeySet(keySet),
+		keyOpt,
 		jws.WithDetachedPayload(payload),
 	)
 	return err

signature/sign_test.go

@@ -2,8 +2,13 @@ package signature
 
 import (
 	"context"
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/x509"
+	"encoding/pem"
 	"errors"
 	"fmt"
+	"io"
 	"math/rand"
 	"os"
 	"path"
@@ -134,6 +139,133 @@ func TestSignVerify(t *testing.T) {
 	}
 }
 
+var _ crypto.Signer = testECDSASigner{}
+
+type testECDSASigner struct {
+	privateKey crypto.PrivateKey
+	publickKey crypto.PublicKey
+}
+
+func (m testECDSASigner) Public() crypto.PublicKey { return m.publickKey }
+
+func (m testECDSASigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) {
+	return ecdsa.SignASN1(rand, m.privateKey.(*ecdsa.PrivateKey), digest)
+}
+
+func (m testECDSASigner) Algorithm() jwa.KeyAlgorithm {
+	return jwa.ES256
+}
+
+func TestSignVerifyCryptoSigner(t *testing.T) {
+
+	t.Parallel()
+	ctx := context.Background()
+
+	step := &pipeline.CommandStep{
+		Command: "llamas",
+		Plugins: pipeline.Plugins{
+			{
+				Source: "some-plugin#v1.0.0",
+				Config: nil,
+			},
+			{
+				Source: "another-plugin#v3.4.5",
+				Config: map[string]any{"llama": "Kuzco"},
+			},
+		},
+		Env: map[string]string{
+			"CONTEXT": "cats",
+			"DEPLOY":  "0",
+		},
+	}
+	// The pipeline-level env that the agent uploads:
+	signEnv := map[string]string{
+		"DEPLOY": "1",
+	}
+
+	// The backend combines the pipeline and step envs, providing a new env:
+	verifyEnv := map[string]string{
+		"CONTEXT": "cats",
+		"DEPLOY":  "0",
+		"MISC":    "llama drama",
+	}
+
+	stepWithInvariants := &CommandStepWithInvariants{
+		CommandStep:   *step,
+		RepositoryURL: "fake-repo",
+	}
+
+	cases := []struct {
+		name              string
+		alg               jwa.SignatureAlgorithm
+		expectedSignature string
+	}{
+		{
+			name:              "should sign using crypto.Signer",
+			alg:               jwa.ES256,
+			expectedSignature: "eyJhbGciOiJFUzI1NiJ9..Op5KSww95n5s1b9jz0Me5UGqUQPcHzEIFvkWTB_yEv6qEDnnFUO1XsC5592fQoAcB0VnPnHaK31iSiCypREIdA",
+		},
+	}
+
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("os.Getwd() error = %v", err)
+	}
+
+	privateKeyPath := path.Join(wd, "fixtures", "crypto_signer", "P256", "private.pem")
+	pemPrivateKey, err := os.ReadFile(privateKeyPath)
+	if err != nil {
+		t.Fatalf("os.ReadFile(%q) error = %v", privateKeyPath, err)
+	}
+
+	block, _ := pem.Decode([]byte(pemPrivateKey))
+	x509Encoded := block.Bytes
+	privateKey, err := x509.ParseECPrivateKey(x509Encoded)
+	if err != nil {
+		t.Fatalf("x509.ParseECPrivateKey(%v) error = %v", x509Encoded, err)
+	}
+
+	publicKeyPath := path.Join(wd, "fixtures", "crypto_signer", "P256", "public.pem")
+	pemPublicKey, err := os.ReadFile(publicKeyPath)
+	if err != nil {
+		t.Fatalf("os.ReadFile(%q) error = %v", publicKeyPath, err)
+	}
+
+	blockPub, _ := pem.Decode([]byte(pemPublicKey))
+	x509EncodedPub := blockPub.Bytes
+	genericPublicKey, err := x509.ParsePKIXPublicKey(x509EncodedPub)
+	if err != nil {
+		t.Fatalf("x509.ParsePKIXPublicKey(%v) error = %v", x509EncodedPub, err)
+	}
+
+	publicKey := genericPublicKey.(*ecdsa.PublicKey)
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			sKey := testECDSASigner{
+				privateKey: privateKey,
+				publickKey: publicKey,
+			}
+
+			sig, err := Sign(ctx, sKey, stepWithInvariants, WithEnv(signEnv))
+			if err != nil {
+				t.Fatalf("Sign(ctx, sKey, %v, WithEnv(%v)) error = %v", stepWithInvariants, signEnv, err)
+			}
+
+			if sig.Algorithm != tc.alg.String() {
+				t.Errorf("Signature.Algorithm = %v, want %v", sig.Algorithm, tc.alg)
+			}
+
+			if err := Verify(ctx, sig, sKey, stepWithInvariants, WithEnv(verifyEnv)); err != nil {
+				t.Errorf("Verify(ctx, %v, verifier, %v, WithEnv(%v)) = %v", sig, stepWithInvariants, verifyEnv, err)
+			}
+
+		})
+	}
+}
+
 type testFields map[string]any
 
 func (m testFields) SignedFields() (map[string]any, error) { return m, nil }

signature/steps.go

@@ -6,14 +6,13 @@ import (
 	"fmt"
 
 	"github.com/buildkite/go-pipeline"
-	"github.com/lestrrat-go/jwx/v2/jwk"
 )
 
 var errSigningRefusedUnknownStepType = errors.New("refusing to sign pipeline containing a step of unknown type, because the pipeline could be incorrectly parsed - please contact support")
 
 // SignSteps adds signatures to each command step (and recursively to any command steps that are within group steps).
 // The steps are mutated directly, so an error part-way through may leave some steps un-signed.
-func SignSteps(ctx context.Context, s pipeline.Steps, key jwk.Key, repoURL string, opts ...Option) error {
+func SignSteps(ctx context.Context, s pipeline.Steps, key Key, repoURL string, opts ...Option) error {
 	for _, step := range s {
 		switch step := step.(type) {
 		case *pipeline.CommandStep: