Refactor Identities API

pkg/pki/identity/identity.go

@@ -0,0 +1,25 @@
+// Copyright 2023 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package identity
+
+type Identity struct {
+	// Types include: *rsa.PublicKey, *ecdsa.PublicKey, ed25519.Publickey,
+	// *x509.Certificate, openpgp.EntityList, *minisign.PublicKey, ssh.PublicKey
+	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 []byte
+}

pkg/pki/minisign/minisign.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	minisign "github.com/jedisct1/go-minisign"
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 	"golang.org/x/crypto/blake2b"
 )
@@ -184,11 +185,11 @@ func (k PublicKey) Subjects() []string {
 }
 
 // Identities implements the pki.PublicKey interface
-func (k PublicKey) Identities() ([]string, error) {
+func (k PublicKey) Identities() ([]identity.Identity, error) {
 	// returns base64-encoded key (sig alg, key ID, and public key)
 	key, err := k.CanonicalValue()
 	if err != nil {
 		return nil, err
 	}
-	return []string{string(key)}, nil
+	return []identity.Identity{{Crypto: k.key, Raw: key}}, nil
 }

pkg/pki/minisign/minisign_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	minisign "github.com/jedisct1/go-minisign"
 	"go.uber.org/goleak"
 )
 
@@ -64,6 +65,20 @@ func TestReadPublicKey(t *testing.T) {
 			if !bytes.Equal(rawGot.key.PublicKey[:], rawBytes) {
 				t.Errorf("expected parsed keys to be equal, %v != %v", rawGot.key.PublicKey, rawBytes)
 			}
+			ids, err := rawGot.Identities()
+			if err != nil {
+				t.Fatalf("unexpected error getting identities: %v", err)
+			}
+			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()
+			if err != nil {
+				t.Fatalf("error canonicalizing key: %v", err)
+			}
+			if !reflect.DeepEqual(val, ids[0].Raw) {
+				t.Errorf("raw key and canonical value are not equal")
+			}
 		})
 	}
 }
@@ -293,10 +308,17 @@ func TestCanonicalValuePublicKey(t *testing.T) {
 		// Identities should be equal to the canonical value for minisign
 		ids, err := outputKey.Identities()
 		if err != nil {
-			t.Errorf("unexpected error getting identities: %v", err)
+			t.Fatalf("unexpected error getting identities: %v", err)
+		}
+		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()
+		if err != nil {
+			t.Fatalf("error canonicalizing key: %v", err)
 		}
-		if !reflect.DeepEqual([]string{string(cvOutput)}, ids) {
-			t.Errorf("identities and canonical value are not equal")
+		if !reflect.DeepEqual(val, ids[0].Raw) {
+			t.Errorf("raw key and canonical value are not equal")
 		}
 	}
 }

pkg/pki/pgp/pgp.go

@@ -31,6 +31,7 @@ import (
 	"golang.org/x/crypto/openpgp/armor"  //nolint:staticcheck
 	"golang.org/x/crypto/openpgp/packet" //nolint:staticcheck
 
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 )
 
@@ -305,14 +306,10 @@ func (k PublicKey) Subjects() []string {
 }
 
 // Identities implements the pki.PublicKey interface
-func (k PublicKey) Identities() ([]string, error) {
-	// returns the email addresses and armored public key
-	var identities []string
-	identities = append(identities, k.Subjects()...)
+func (k PublicKey) Identities() ([]identity.Identity, error) {
 	key, err := k.CanonicalValue()
 	if err != nil {
 		return nil, err
 	}
-	identities = append(identities, string(key))
-	return identities, nil
+	return []identity.Identity{{Crypto: k.key, Raw: key}}, nil
 }

pkg/pki/pgp/pgp_test.go

@@ -27,6 +27,7 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	"go.uber.org/goleak"
 )
 
@@ -387,9 +388,8 @@ 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))
 		}
 
-		expectedIDs := tc.subjects
-		key, _ := inputKey.CanonicalValue()
-		expectedIDs = append(expectedIDs, string(key))
+		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)

pkg/pki/pkcs7/pkcs7.go

@@ -28,6 +28,7 @@ import (
 	"strings"
 
 	"github.com/sassoftware/relic/lib/pkcs7"
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 )
@@ -224,24 +225,18 @@ func (k PublicKey) Subjects() []string {
 }
 
 // Identities implements the pki.PublicKey interface
-func (k PublicKey) Identities() ([]string, error) {
-	var identities []string
-
+func (k PublicKey) Identities() ([]identity.Identity, error) {
 	// pkcs7 structure may contain both a key and certificate chain
 	pemCert, err := cryptoutils.MarshalCertificateToPEM(k.certs[0])
 	if err != nil {
 		return nil, err
 	}
-	identities = append(identities, string(pemCert))
 	pemKey, err := cryptoutils.MarshalPublicKeyToPEM(k.key)
 	if err != nil {
 		return nil, err
 	}
-	identities = append(identities, string(pemKey))
-
-	// Subjects come from the certificate Subject and SANs
-	// SANs could include an email, IP address, DNS name, URI, or any other value in the SAN
-	identities = append(identities, k.Subjects()...)
-
-	return identities, nil
+	return []identity.Identity{
+		{Crypto: k.certs[0], Raw: pemCert},
+		{Crypto: k.key, Raw: pemKey},
+	}, nil
 }

pkg/pki/pkcs7/pkcs7_test.go

@@ -28,6 +28,7 @@ import (
 	"testing"
 
 	"github.com/sassoftware/relic/lib/pkcs7"
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	"github.com/sigstore/rekor/pkg/pki/x509/testutils"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 )
@@ -470,12 +471,12 @@ A6ydFG8HXGWcnVVIVQ==
 		{
 			name:       "email in subject",
 			pkcs7:      pkcsPEMEmail,
-			identities: []string{pkcsEmailCert, pkcsPEMEmailKey, "test@rekor.dev"},
+			identities: []string{pkcsEmailCert, pkcsPEMEmailKey},
 		},
 		{
 			name:       "email and URI in subject alternative name",
 			pkcs7:      string(pkcs7bytes),
-			identities: []string{string(leafCertPEM), string(leafPEM), "subject@example.com", "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.1.1"},
+			identities: []string{string(leafCertPEM), string(leafPEM)},
 		},
 	}
 
@@ -485,23 +486,33 @@ A6ydFG8HXGWcnVVIVQ==
 			if err != nil {
 				t.Fatal(err)
 			}
-			identities, err := pub.Identities()
+			ids, err := pub.Identities()
 			if err != nil {
 				t.Fatalf("unexpected error getting identities: %v", err)
 			}
+			if len(ids) != 2 {
+				t.Fatalf("expected 2 identities, got %d", len(ids))
+			}
 
-			if len(identities) == len(tt.identities) {
-				if len(identities) > 0 {
-					sort.Strings(identities)
-					sort.Strings(tt.identities)
-					if !reflect.DeepEqual(identities, tt.identities) {
-						t.Errorf("%v: Error getting identities from keys, got %v, expected %v", tt.name, identities, tt.identities)
-					}
-				}
-			} else {
-				t.Errorf("%v: Error getting identities from keys, got %v, expected %v", tt.name, identities, tt.identities)
+			// compare certificate
+			cert, _ := cryptoutils.UnmarshalCertificatesFromPEM([]byte(tt.identities[0]))
+			expectedID := identity.Identity{Crypto: cert[0], Raw: []byte(tt.identities[0])}
+			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))
 			}
 
+			// compare public key
+			key, _ := cryptoutils.UnmarshalPEMToPublicKey([]byte(tt.identities[1]))
+			expectedID = identity.Identity{Crypto: key, Raw: []byte(tt.identities[1])}
+			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)
+			}
 		})
 	}
 }

pkg/pki/pki.go

@@ -18,6 +18,7 @@ package pki
 import (
 	"io"
 
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 )
 
@@ -28,8 +29,8 @@ type PublicKey interface {
 	// also return Subject URIs present in public keys.
 	EmailAddresses() []string
 	Subjects() []string
-	// Identities returns PEM-encoded public keys and subjects from either certificate or PGP keys
-	Identities() ([]string, error)
+	// Identities returns a list of typed keys and certificates.
+	Identities() ([]identity.Identity, error)
 }
 
 // Signature Generic object representing a signature (regardless of format & algorithm)

pkg/pki/ssh/ssh.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 
 	"github.com/asaskevich/govalidator"
+	"github.com/sigstore/rekor/pkg/pki/identity"
 	sigsig "github.com/sigstore/sigstore/pkg/signature"
 	"golang.org/x/crypto/ssh"
 )
@@ -108,25 +109,20 @@ func (k PublicKey) CanonicalValue() ([]byte, error) {
 
 // EmailAddresses implements the pki.PublicKey interface
 func (k PublicKey) EmailAddresses() []string {
+	if govalidator.IsEmail(k.comment) {
+		return []string{k.comment}
+	}
 	return nil
 }
 
 // Subjects implements the pki.PublicKey interface
 func (k PublicKey) Subjects() []string {
-	return nil
+	return k.EmailAddresses()
 }
 
 // Identities implements the pki.PublicKey interface
-func (k PublicKey) Identities() ([]string, error) {
-	var identities []string
-
+func (k PublicKey) Identities() ([]identity.Identity, error) {
 	// an authorized key format
-	authorizedKey := string(bytes.TrimSpace(ssh.MarshalAuthorizedKey(k.key)))
-	identities = append(identities, authorizedKey)
-
-	if govalidator.IsEmail(k.comment) {
-		identities = append(identities, k.comment)
-	}
-
-	return identities, nil
+	authorizedKey := bytes.TrimSpace(ssh.MarshalAuthorizedKey(k.key))
+	return []identity.Identity{{Crypto: k.key, Raw: authorizedKey}}, nil
 }

pkg/pki/ssh/ssh_test.go

@@ -17,32 +17,44 @@ package ssh
 import (
 	"math/rand"
 	"reflect"
-	"sort"
 	"strings"
 	"testing"
+
+	"github.com/sigstore/rekor/pkg/pki/identity"
+	"golang.org/x/crypto/ssh"
 )
 
 func TestIdentities(t *testing.T) {
 	// from ssh_e2e_test.go
 	publicKey := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDXofkiahE7uavjWvxnwkUF27qMgz7pdTwzSv0XzVG6EtirOv3PDWct4YKoXE9c0EqbxnIfYEKwEextdvB7zkgwczdJSHxf/18jQumLn/FuoCmugVSk1H5Qli/qzwBpaTnOk3WuakGuoYUl8ZAokKKgOKLA0aZJ1WRQ2ZCZggA3EkwNZiY17y9Q6HqdgQcH6XN8aAMADNVJdMAJb33hSRJjjsAPTmzBTishP8lYDoGRSsSE7/8XRBCEV5E4I8mI9GElcZwV/1KJx98mpH8QvMzXM1idFcwPRtt1NTAOshwgUU0Fu1x8lU5RQIa6ZKW36qNQLvLxy/BscC7B/mdLptoDs/ot9NimUXZcgCR1a2Q3o7Wi6jIgcgJcyV10Nba81ol4RdN4qPHnVZIzuo+dBkqwG3CMtB4Rj84+Qi+7zyU01hIPreoxQDXaayiGPBUUIiAlW9gsiuRWJzNnu3cvuWDLVfQIkjh7Wug58z+v2NOJ7IMdyERillhzDcvVHaq14+U= test@rekor.dev"
+	expectedKey, _, _, _, _ := ssh.ParseAuthorizedKey([]byte(publicKey))
 
 	pub, err := NewPublicKey(strings.NewReader(publicKey))
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	identities, err := pub.Identities()
-	if err != nil {
-		t.Fatalf("unexpected error getting identities: %v", err)
+	if !reflect.DeepEqual(pub.EmailAddresses(), []string{"test@rekor.dev"}) {
+		t.Fatalf("expected email address, got %v", pub.EmailAddresses())
+	}
+	if !reflect.DeepEqual(pub.Subjects(), []string{"test@rekor.dev"}) {
+		t.Fatalf("expected email address as subject, got %v", pub.Subjects())
 	}
 
-	expectedIDs := []string{"test@rekor.dev",
-		"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDXofkiahE7uavjWvxnwkUF27qMgz7pdTwzSv0XzVG6EtirOv3PDWct4YKoXE9c0EqbxnIfYEKwEextdvB7zkgwczdJSHxf/18jQumLn/FuoCmugVSk1H5Qli/qzwBpaTnOk3WuakGuoYUl8ZAokKKgOKLA0aZJ1WRQ2ZCZggA3EkwNZiY17y9Q6HqdgQcH6XN8aAMADNVJdMAJb33hSRJjjsAPTmzBTishP8lYDoGRSsSE7/8XRBCEV5E4I8mI9GElcZwV/1KJx98mpH8QvMzXM1idFcwPRtt1NTAOshwgUU0Fu1x8lU5RQIa6ZKW36qNQLvLxy/BscC7B/mdLptoDs/ot9NimUXZcgCR1a2Q3o7Wi6jIgcgJcyV10Nba81ol4RdN4qPHnVZIzuo+dBkqwG3CMtB4Rj84+Qi+7zyU01hIPreoxQDXaayiGPBUUIiAlW9gsiuRWJzNnu3cvuWDLVfQIkjh7Wug58z+v2NOJ7IMdyERillhzDcvVHaq14+U="}
-
-	sort.Strings(identities)
-	sort.Strings(expectedIDs)
-	if !reflect.DeepEqual(identities, expectedIDs) {
-		t.Errorf("err getting identities from keys, got %v, expected %v", identities, expectedIDs)
+	keyVal := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDXofkiahE7uavjWvxnwkUF27qMgz7pdTwzSv0XzVG6EtirOv3PDWct4YKoXE9c0EqbxnIfYEKwEextdvB7zkgwczdJSHxf/18jQumLn/FuoCmugVSk1H5Qli/qzwBpaTnOk3WuakGuoYUl8ZAokKKgOKLA0aZJ1WRQ2ZCZggA3EkwNZiY17y9Q6HqdgQcH6XN8aAMADNVJdMAJb33hSRJjjsAPTmzBTishP8lYDoGRSsSE7/8XRBCEV5E4I8mI9GElcZwV/1KJx98mpH8QvMzXM1idFcwPRtt1NTAOshwgUU0Fu1x8lU5RQIa6ZKW36qNQLvLxy/BscC7B/mdLptoDs/ot9NimUXZcgCR1a2Q3o7Wi6jIgcgJcyV10Nba81ol4RdN4qPHnVZIzuo+dBkqwG3CMtB4Rj84+Qi+7zyU01hIPreoxQDXaayiGPBUUIiAlW9gsiuRWJzNnu3cvuWDLVfQIkjh7Wug58z+v2NOJ7IMdyERillhzDcvVHaq14+U="
+	expectedID := identity.Identity{Crypto: expectedKey, Raw: []byte(keyVal)}
+	ids, err := pub.Identities()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(ids) != 1 {
+		t.Errorf("too many identities, expected 1, got %v", len(ids))
+	}
+	if !reflect.DeepEqual(ids[0].Crypto.(ssh.PublicKey).Marshal(), expectedID.Crypto.(ssh.PublicKey).Marshal()) {
+		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", string(ids[0].Raw), string(expectedID.Raw))
 	}
 }