Change values of Identity.Raw, add fingerprints (#1628)

* Change values of Identity.Raw, add fingerprints Raw now contains only PKIX public keys or DER encoded certificates. The keys are extracted from minisign, pgp, and TUF verifiers. Also added fingerprints for each verifier. Keys, certificates, and ed25519 keys from minisign are hex-encoded sha-256 digests of the raw key. SSH and PGP use their ecosystem-standard fingerprints. Signed-off-by: Hayden Blauzvern <[email protected]> * Fix lint Signed-off-by: Hayden Blauzvern <[email protected]> --------- Signed-off-by: Hayden Blauzvern <[email protected]>
sigstore · Aug 15, 2023 · 3e1715a · 3e1715a
1 parent c1e6614
commit 3e1715a
Show file tree

Hide file tree

Showing 13 changed files with 221 additions and 73 deletions.
diff --git a/pkg/pki/identity/identity.go b/pkg/pki/identity/identity.go
@@ -15,11 +15,24 @@
 package identity
 
 type Identity struct {
-	// Types include: *rsa.PublicKey, *ecdsa.PublicKey, ed25519.PublicKey,
-	// *x509.Certificate, openpgp.EntityList, *minisign.PublicKey, ssh.PublicKey
+	// Types include:
+	// - *rsa.PublicKey
+	// - *ecdsa.PublicKey
+	// - ed25519.PublicKey
+	// - *x509.Certificate
+	// - openpgp.EntityList (golang.org/x/crypto/openpgp)
+	// - *minisign.PublicKey (github.com/jedisct1/go-minisign)
+	// - ssh.PublicKey (golang.org/x/crypto/ssh)
 	Crypto any
-	// Based on type of Crypto. Possible values include: PEM-encoded public key,
-	// PEM-encoded certificate, canonicalized PGP public key, encoded Minisign
-	// public key, encoded SSH public key
+	// Raw key or certificate extracted from Crypto. Values include:
+	// - PKIX ASN.1 DER-encoded public key
+	// - ASN.1 DER-encoded certificate
 	Raw []byte
+	// For keys, certificates, and minisign, hex-encoded SHA-256 digest
+	// of the public key or certificate
+	// For SSH and PGP, Fingerprint is the standard for each ecosystem
+	// For SSH, unpadded base-64 encoded SHA-256 digest of the key
+	// For PGP, hex-encoded SHA-1 digest of a key, which can be either
+	// a primary key or subkey
+	Fingerprint string
 }
diff --git a/pkg/pki/minisign/minisign.go b/pkg/pki/minisign/minisign.go
@@ -17,13 +17,17 @@ package minisign
 
 import (
 	"bytes"
+	"crypto/ed25519"
+	"crypto/sha256"
 	"encoding/base64"
+	"encoding/hex"
 	"fmt"
 	"io"
 	"strings"
 
 	minisign "github.com/jedisct1/go-minisign"
 	"github.com/sigstore/rekor/pkg/pki/identity"
+	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 	"golang.org/x/crypto/blake2b"
 )
@@ -186,10 +190,15 @@ func (k PublicKey) Subjects() []string {
 
 // Identities implements the pki.PublicKey interface
 func (k PublicKey) Identities() ([]identity.Identity, error) {
-	// returns base64-encoded key (sig alg, key ID, and public key)
-	key, err := k.CanonicalValue()
+	// PKIX encode ed25519 public key
+	pkixKey, err := cryptoutils.MarshalPublicKeyToDER(ed25519.PublicKey(k.key.PublicKey[:]))
 	if err != nil {
 		return nil, err
 	}
-	return []identity.Identity{{Crypto: k.key, Raw: key}}, nil
+	digest := sha256.Sum256(pkixKey)
+	return []identity.Identity{{
+		Crypto:      k.key,
+		Raw:         pkixKey,
+		Fingerprint: hex.EncodeToString(digest[:]),
+	}}, nil
 }
diff --git a/pkg/pki/minisign/minisign_test.go b/pkg/pki/minisign/minisign_test.go
@@ -1,4 +1,3 @@
-//
 // Copyright 2021 The Sigstore Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -17,6 +16,10 @@ package minisign
 
 import (
 	"bytes"
+	"crypto/ed25519"
+	"crypto/sha256"
+	"crypto/x509"
+	"encoding/hex"
 	"errors"
 	"io"
 	"os"
@@ -25,6 +28,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	minisign "github.com/jedisct1/go-minisign"
+	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"go.uber.org/goleak"
 )
 
@@ -72,12 +76,23 @@ func TestReadPublicKey(t *testing.T) {
 			if _, ok := ids[0].Crypto.(*minisign.PublicKey); !ok {
 				t.Fatalf("key is of unexpected type, expected *minisign.PublicKey, got %v", reflect.TypeOf(ids[0].Crypto))
 			}
-			val, err := rawGot.CanonicalValue()
+			expectedDer, err := cryptoutils.MarshalPublicKeyToDER(ed25519.PublicKey(rawBytes))
 			if err != nil {
-				t.Fatalf("error canonicalizing key: %v", err)
+				t.Fatalf("unexpected error generating DER: %v", err)
+			}
+			if !reflect.DeepEqual(expectedDer, ids[0].Raw) {
+				t.Errorf("raw keys are not equal")
+			}
+			edKey, _ := x509.ParsePKIXPublicKey(ids[0].Raw)
+			if err := cryptoutils.EqualKeys(edKey, ed25519.PublicKey(rawBytes)); err != nil {
+				t.Errorf("public keys did not match: %v", err)
+			}
+			if len(ids[0].Fingerprint) != 64 {
+				t.Errorf("%v: fingerprint is not expected length of 64 (hex 32-byte sha256): %d", tc.caseDesc, len(ids[0].Fingerprint))
 			}
-			if !reflect.DeepEqual(val, ids[0].Raw) {
-				t.Errorf("raw key and canonical value are not equal")
+			digest := sha256.Sum256(expectedDer)
+			if hex.EncodeToString(digest[:]) != ids[0].Fingerprint {
+				t.Fatalf("fingerprints don't match")
 			}
 		})
 	}
@@ -313,12 +328,23 @@ func TestCanonicalValuePublicKey(t *testing.T) {
 		if _, ok := ids[0].Crypto.(*minisign.PublicKey); !ok {
 			t.Fatalf("key is of unexpected type, expected *minisign.PublicKey, got %v", reflect.TypeOf(ids[0].Crypto))
 		}
-		val, err := outputKey.CanonicalValue()
+		expectedDer, err := cryptoutils.MarshalPublicKeyToDER(ed25519.PublicKey(rt.key.PublicKey[:]))
 		if err != nil {
-			t.Fatalf("error canonicalizing key: %v", err)
+			t.Fatalf("unexpected error generating DER: %v", err)
+		}
+		if !reflect.DeepEqual(expectedDer, ids[0].Raw) {
+			t.Errorf("raw keys are not equal")
+		}
+		edKey, _ := x509.ParsePKIXPublicKey(ids[0].Raw)
+		if err := cryptoutils.EqualKeys(edKey, ed25519.PublicKey(rt.key.PublicKey[:])); err != nil {
+			t.Errorf("public keys did not match: %v", err)
+		}
+		if len(ids[0].Fingerprint) != 64 {
+			t.Errorf("%v: fingerprint is not expected length of 64 (hex 32-byte sha256): %d", tc.caseDesc, len(ids[0].Fingerprint))
 		}
-		if !reflect.DeepEqual(val, ids[0].Raw) {
-			t.Errorf("raw key and canonical value are not equal")
+		digest := sha256.Sum256(expectedDer)
+		if hex.EncodeToString(digest[:]) != ids[0].Fingerprint {
+			t.Fatalf("fingerprints don't match")
 		}
 	}
 }

diff --git a/pkg/pki/pgp/pgp.go b/pkg/pki/pgp/pgp.go
@@ -19,6 +19,10 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"crypto/ecdsa"
+	"crypto/ed25519"
+	"crypto/rsa"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"io"
@@ -32,6 +36,7 @@ import (
 	"golang.org/x/crypto/openpgp/packet" //nolint:staticcheck
 
 	"github.com/sigstore/rekor/pkg/pki/identity"
+	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 )
 
@@ -307,9 +312,33 @@ func (k PublicKey) Subjects() []string {
 
 // Identities implements the pki.PublicKey interface
 func (k PublicKey) Identities() ([]identity.Identity, error) {
-	key, err := k.CanonicalValue()
-	if err != nil {
-		return nil, err
+	var ids []identity.Identity
+	for _, entity := range k.key {
+		var keys []*packet.PublicKey
+		keys = append(keys, entity.PrimaryKey)
+		for _, subKey := range entity.Subkeys {
+			keys = append(keys, subKey.PublicKey)
+		}
+		for _, pk := range keys {
+			pubKey := pk.PublicKey
+			// Only process supported types. Will ignore DSA
+			// and ElGamal keys.
+			// TODO: For a V2 PGP type, enforce on upload
+			switch pubKey.(type) {
+			case *rsa.PublicKey, *ecdsa.PublicKey, ed25519.PublicKey:
+			default:
+				continue
+			}
+			pkixKey, err := cryptoutils.MarshalPublicKeyToDER(pubKey)
+			if err != nil {
+				return nil, err
+			}
+			ids = append(ids, identity.Identity{
+				Crypto:      pubKey,
+				Raw:         pkixKey,
+				Fingerprint: hex.EncodeToString(pk.Fingerprint[:]),
+			})
+		}
 	}
-	return []identity.Identity{{Crypto: k.key, Raw: key}}, nil
+	return ids, nil
 }
diff --git a/pkg/pki/pgp/pgp_test.go b/pkg/pki/pgp/pgp_test.go
@@ -27,7 +27,6 @@ import (
 	"sort"
 	"testing"
 
-	"github.com/sigstore/rekor/pkg/pki/identity"
 	"go.uber.org/goleak"
 )
 
@@ -348,17 +347,20 @@ func TestEmailAddresses(t *testing.T) {
 		caseDesc  string
 		inputFile string
 		subjects  []string
+		// number of keys in key ring
+		// verified with gpg, ignoring DSA/ElGamal keys
+		keys int
 	}
 
 	var k PublicKey
 	if len(k.Subjects()) != 0 {
 		t.Errorf("Subjects for unitialized key should give empty slice")
 	}
 	tests := []test{
-		{caseDesc: "Valid armored public key", inputFile: "testdata/valid_armored_public.pgp", subjects: []string{}},
-		{caseDesc: "Valid armored public key with multiple subentries", inputFile: "testdata/valid_armored_complex_public.pgp", subjects: []string{"[email protected]", "[email protected]"}},
-		{caseDesc: "Valid binary public key", inputFile: "testdata/valid_binary_public.pgp", subjects: []string{}},
-		{caseDesc: "Valid binary public key with multiple subentries", inputFile: "testdata/valid_binary_complex_public.pgp", subjects: []string{"[email protected]", "[email protected]"}},
+		{caseDesc: "Valid armored public key", inputFile: "testdata/valid_armored_public.pgp", subjects: []string{}, keys: 2},
+		{caseDesc: "Valid armored public key with multiple subentries", inputFile: "testdata/valid_armored_complex_public.pgp", subjects: []string{"[email protected]", "[email protected]"}, keys: 4},
+		{caseDesc: "Valid binary public key", inputFile: "testdata/valid_binary_public.pgp", subjects: []string{}, keys: 2},
+		{caseDesc: "Valid binary public key with multiple subentries", inputFile: "testdata/valid_binary_complex_public.pgp", subjects: []string{"[email protected]", "[email protected]"}, keys: 4},
 	}
 
 	for _, tc := range tests {
@@ -388,14 +390,12 @@ func TestEmailAddresses(t *testing.T) {
 			t.Errorf("%v: Error getting subjects from keys length, got %v, expected %v", tc.caseDesc, len(subjects), len(tc.subjects))
 		}
 
-		keyVal, _ := inputKey.CanonicalValue()
-		expectedIDs := []identity.Identity{{Crypto: inputKey.key, Raw: keyVal}}
 		ids, err := inputKey.Identities()
 		if err != nil {
-			t.Fatalf("unexpected error getting identities: %v", err)
+			t.Fatalf("%v: unexpected error getting identities: %v", tc.caseDesc, err)
 		}
-		if !reflect.DeepEqual(ids, expectedIDs) {
-			t.Errorf("identities are not equal")
+		if len(ids) != tc.keys {
+			t.Fatalf("%v: expected %d keys, got %d", tc.caseDesc, tc.keys, len(ids))
 		}
 	}
 }

diff --git a/pkg/pki/pkcs7/pkcs7.go b/pkg/pki/pkcs7/pkcs7.go
@@ -19,8 +19,10 @@ package pkcs7
 import (
 	"bytes"
 	"crypto"
+	"crypto/sha256"
 	"crypto/x509"
 	"encoding/asn1"
+	"encoding/hex"
 	"encoding/pem"
 	"errors"
 	"fmt"
@@ -227,16 +229,20 @@ func (k PublicKey) Subjects() []string {
 // Identities implements the pki.PublicKey interface
 func (k PublicKey) Identities() ([]identity.Identity, error) {
 	// pkcs7 structure may contain both a key and certificate chain
-	pemCert, err := cryptoutils.MarshalCertificateToPEM(k.certs[0])
+	pkixKey, err := cryptoutils.MarshalPublicKeyToDER(k.key)
 	if err != nil {
 		return nil, err
 	}
-	pemKey, err := cryptoutils.MarshalPublicKeyToPEM(k.key)
-	if err != nil {
-		return nil, err
-	}
-	return []identity.Identity{
-		{Crypto: k.certs[0], Raw: pemCert},
-		{Crypto: k.key, Raw: pemKey},
+	keyDigest := sha256.Sum256(pkixKey)
+	certDigest := sha256.Sum256(k.certs[0].Raw)
+	return []identity.Identity{{
+		Crypto:      k.certs[0],
+		Raw:         k.certs[0].Raw,
+		Fingerprint: hex.EncodeToString(certDigest[:]),
+	}, {
+		Crypto:      k.key,
+		Raw:         pkixKey,
+		Fingerprint: hex.EncodeToString(keyDigest[:]),
+	},
 	}, nil
 }
diff --git a/pkg/pki/pkcs7/pkcs7_test.go b/pkg/pki/pkcs7/pkcs7_test.go
@@ -19,8 +19,10 @@ package pkcs7
 import (
 	"bytes"
 	"crypto"
+	"crypto/sha256"
 	"crypto/x509"
 	"encoding/base64"
+	"encoding/hex"
 	"net/url"
 	"reflect"
 	"sort"
@@ -496,22 +498,31 @@ A6ydFG8HXGWcnVVIVQ==
 
 			// compare certificate
 			cert, _ := cryptoutils.UnmarshalCertificatesFromPEM([]byte(tt.identities[0]))
-			expectedID := identity.Identity{Crypto: cert[0], Raw: []byte(tt.identities[0])}
+			digest := sha256.Sum256(cert[0].Raw)
+			expectedID := identity.Identity{Crypto: cert[0], Raw: cert[0].Raw, Fingerprint: hex.EncodeToString(digest[:])}
 			if !ids[0].Crypto.(*x509.Certificate).Equal(expectedID.Crypto.(*x509.Certificate)) {
 				t.Errorf("certificates did not match")
 			}
 			if !reflect.DeepEqual(ids[0].Raw, expectedID.Raw) {
-				t.Errorf("raw identities did not match, expected %v, got %v", ids[0].Raw, string(expectedID.Raw))
+				t.Errorf("raw identities did not match, expected %v, got %v", string(expectedID.Raw), ids[0].Raw)
+			}
+			if ids[0].Fingerprint != expectedID.Fingerprint {
+				t.Fatalf("fingerprints did not match, expected %v, got %v", expectedID.Fingerprint, ids[0].Fingerprint)
 			}
 
 			// compare public key
 			key, _ := cryptoutils.UnmarshalPEMToPublicKey([]byte(tt.identities[1]))
-			expectedID = identity.Identity{Crypto: key, Raw: []byte(tt.identities[1])}
+			pkixKey, _ := cryptoutils.MarshalPublicKeyToDER(key)
+			digest = sha256.Sum256(pkixKey)
+			expectedID = identity.Identity{Crypto: key, Raw: pkixKey, Fingerprint: hex.EncodeToString(digest[:])}
 			if err := cryptoutils.EqualKeys(expectedID.Crypto, ids[1].Crypto); err != nil {
 				t.Errorf("%v: public keys did not match: %v", tt.name, err)
 			}
 			if !reflect.DeepEqual(ids[1].Raw, expectedID.Raw) {
-				t.Errorf("%v: raw identities did not match", tt.name)
+				t.Errorf("%v: raw key identities did not match", tt.name)
+			}
+			if ids[1].Fingerprint != expectedID.Fingerprint {
+				t.Fatalf("key fingerprints did not match, expected %v, got %v", expectedID.Fingerprint, ids[1].Fingerprint)
 			}
 		})
 	}

diff --git a/pkg/pki/ssh/ssh.go b/pkg/pki/ssh/ssh.go
@@ -16,13 +16,13 @@
 package ssh
 
 import (
-	"bytes"
 	"fmt"
 	"io"
 	"net/http"
 
 	"github.com/asaskevich/govalidator"
 	"github.com/sigstore/rekor/pkg/pki/identity"
+	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 	"golang.org/x/crypto/ssh"
 )
@@ -122,7 +122,15 @@ func (k PublicKey) Subjects() []string {
 
 // Identities implements the pki.PublicKey interface
 func (k PublicKey) Identities() ([]identity.Identity, error) {
-	// an authorized key format
-	authorizedKey := bytes.TrimSpace(ssh.MarshalAuthorizedKey(k.key))
-	return []identity.Identity{{Crypto: k.key, Raw: authorizedKey}}, nil
+	key := k.key.(ssh.CryptoPublicKey).CryptoPublicKey()
+	pkixKey, err := cryptoutils.MarshalPublicKeyToDER(key)
+	if err != nil {
+		return nil, err
+	}
+	fp := ssh.FingerprintSHA256(k.key)
+	return []identity.Identity{{
+		Crypto:      k.key,
+		Raw:         pkixKey,
+		Fingerprint: fp,
+	}}, nil
 }