Drop the intermediate keyObj and sigObj variables from the HashedRekord type. (#555)

This is part of a larger series to reduce intermediate state on each rekord type.

Signed-off-by: Dan Lorenc <lorenc.d@gmail.com>

pkg/types/hashedrekord/v0.0.1/entry.go

@@ -34,6 +34,7 @@ import (
 	"github.com/sigstore/rekor/pkg/generated/models"
 	"github.com/sigstore/rekor/pkg/log"
 	"github.com/sigstore/rekor/pkg/pki"
+	"github.com/sigstore/rekor/pkg/pki/x509"
 	"github.com/sigstore/rekor/pkg/types"
 	hashedrekord "github.com/sigstore/rekor/pkg/types/hashedrekord"
 	"github.com/sigstore/sigstore/pkg/signature/options"
@@ -51,8 +52,6 @@ func init() {
 
 type V001Entry struct {
 	HashedRekordObj models.HashedrekordV001Schema
-	keyObj          pki.PublicKey
-	sigObj          pki.Signature
 }
 
 func (v V001Entry) APIVersion() string {
@@ -66,13 +65,15 @@ func NewEntry() types.EntryImpl {
 func (v V001Entry) IndexKeys() ([]string, error) {
 	var result []string
 
-	key, err := v.keyObj.CanonicalValue()
+	key := v.HashedRekordObj.Signature.PublicKey.Content
+	keyHash := sha256.Sum256(key)
+	result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:])))
+
+	pub, err := x509.NewPublicKey(bytes.NewReader(key))
 	if err != nil {
 		return nil, err
 	}
-	keyHash := sha256.Sum256(key)
-	result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:])))
-	result = append(result, v.keyObj.EmailAddresses()...)
+	result = append(result, pub.EmailAddresses()...)
 
 	if v.HashedRekordObj.Data.Hash != nil {
 		hashKey := strings.ToLower(fmt.Sprintf("%s:%s", *v.HashedRekordObj.Data.Hash.Algorithm, *v.HashedRekordObj.Data.Hash.Value))
@@ -98,35 +99,28 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	}
 
 	// cross field validation
-	return v.validate()
+	_, _, err := v.validate()
+	return err
 }
 
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
-	if err := v.validate(); err != nil {
+	sigObj, keyObj, err := v.validate()
+	if err != nil {
 		return nil, types.ValidationError(err)
 	}
 
-	if v.sigObj == nil {
-		return nil, errors.New("signature object not initialized before canonicalization")
-	}
-
-	if v.keyObj == nil {
-		return nil, errors.New("key object not initialized before canonicalization")
-	}
-
 	canonicalEntry := models.HashedrekordV001Schema{}
 
 	// need to canonicalize signature & key content
 	canonicalEntry.Signature = &models.HashedrekordV001SchemaSignature{}
-	var err error
-	canonicalEntry.Signature.Content, err = v.sigObj.CanonicalValue()
+	canonicalEntry.Signature.Content, err = sigObj.CanonicalValue()
 	if err != nil {
 		return nil, err
 	}
 
 	// key URL (if known) is not set deliberately
 	canonicalEntry.Signature.PublicKey = &models.HashedrekordV001SchemaSignaturePublicKey{}
-	canonicalEntry.Signature.PublicKey.Content, err = v.keyObj.CanonicalValue()
+	canonicalEntry.Signature.PublicKey.Content, err = keyObj.CanonicalValue()
 	if err != nil {
 		return nil, err
 	}
@@ -144,52 +138,48 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 }
 
 // validate performs cross-field validation for fields in object
-func (v *V001Entry) validate() error {
+func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
 	sig := v.HashedRekordObj.Signature
 	if sig == nil {
-		return types.ValidationError(errors.New("missing signature"))
+		return nil, nil, types.ValidationError(errors.New("missing signature"))
 	}
 	// Hashed rekord type only works for x509 signature types
-	artifactFactory, err := pki.NewArtifactFactory(pki.X509)
+	sigObj, err := x509.NewSignature(bytes.NewReader(sig.Content))
 	if err != nil {
-		return types.ValidationError(err)
-	}
-	v.sigObj, err = artifactFactory.NewSignature(bytes.NewReader(sig.Content))
-	if err != nil {
-		return types.ValidationError(err)
+		return nil, nil, types.ValidationError(err)
 	}
 
 	key := sig.PublicKey
 	if key == nil {
-		return types.ValidationError(errors.New("missing public key"))
+		return nil, nil, types.ValidationError(errors.New("missing public key"))
 	}
-	v.keyObj, err = artifactFactory.NewPublicKey(bytes.NewReader(key.Content))
+	keyObj, err := x509.NewPublicKey(bytes.NewReader(key.Content))
 	if err != nil {
-		return types.ValidationError(err)
+		return nil, nil, types.ValidationError(err)
 	}
 
 	data := v.HashedRekordObj.Data
 	if data == nil {
-		return types.ValidationError(errors.New("missing data"))
+		return nil, nil, types.ValidationError(errors.New("missing data"))
 	}
 
 	hash := data.Hash
 	if hash == nil {
-		return types.ValidationError(errors.New("missing hash"))
+		return nil, nil, types.ValidationError(errors.New("missing hash"))
 	}
 	if !govalidator.IsHash(swag.StringValue(hash.Value), swag.StringValue(hash.Algorithm)) {
-		return types.ValidationError(errors.New("invalid value for hash"))
+		return nil, nil, types.ValidationError(errors.New("invalid value for hash"))
 	}
 
 	decoded, err := hex.DecodeString(*hash.Value)
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
-	if err = v.sigObj.Verify(nil, v.keyObj, options.WithDigest(decoded)); err != nil {
-		return types.ValidationError(errors.Wrap(err, "verifying signature"))
+	if err := sigObj.Verify(nil, keyObj, options.WithDigest(decoded)); err != nil {
+		return nil, nil, types.ValidationError(errors.Wrap(err, "verifying signature"))
 	}
 
-	return nil
+	return sigObj, keyObj, nil
 }
 
 func (v V001Entry) Attestation() (string, []byte) {
@@ -236,7 +226,7 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 		Value:     swag.String(props.ArtifactHash),
 	}
 
-	if err := re.validate(); err != nil {
+	if _, _, err := re.validate(); err != nil {
 		return nil, err
 	}
 

pkg/types/hashedrekord/v0.0.1/entry_test.go

@@ -24,18 +24,21 @@ import (
 	"crypto/rand"
 	"crypto/sha256"
 	"crypto/x509"
+	"crypto/x509/pkix"
 	"encoding/hex"
 	"encoding/pem"
+	"math/big"
 	"reflect"
 	"testing"
 
 	"github.com/go-openapi/runtime"
+	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/swag"
-	"go.uber.org/goleak"
-
 	"github.com/sigstore/rekor/pkg/generated/models"
+	x509r "github.com/sigstore/rekor/pkg/pki/x509"
 	"github.com/sigstore/rekor/pkg/types"
 	"github.com/sigstore/sigstore/pkg/signature"
+	"go.uber.org/goleak"
 )
 
 func TestMain(m *testing.M) {
@@ -216,7 +219,7 @@ func TestCrossFieldValidation(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		if err := tc.entry.validate(); (err == nil) != tc.expectUnmarshalSuccess {
+		if _, _, err := tc.entry.validate(); (err == nil) != tc.expectUnmarshalSuccess {
 			t.Errorf("unexpected result in '%v': %v", tc.caseDesc, err)
 		}
 
@@ -230,7 +233,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err := v.Unmarshal(&r); err != nil {
 				return err
 			}
-			if err := v.validate(); err != nil {
+			if _, _, err := v.validate(); err != nil {
 				return err
 			}
 			return nil
@@ -259,3 +262,125 @@ func TestCrossFieldValidation(t *testing.T) {
 		}
 	}
 }
+
+func hexHash(b []byte) string {
+	h := sha256.Sum256([]byte(b))
+	return hex.EncodeToString(h[:])
+}
+
+func TestV001Entry_IndexKeys(t *testing.T) {
+	pub, cert, priv := testKeyAndCert(t)
+
+	data := "my random data"
+	h := sha256.Sum256([]byte(data))
+	sig, err := ecdsa.SignASN1(rand.Reader, priv, h[:])
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	hashStr := hex.EncodeToString(h[:])
+	hashIndexKey := "sha256:" + hashStr
+	// Base entry template
+	v := V001Entry{
+		HashedRekordObj: models.HashedrekordV001Schema{
+			Data: &models.HashedrekordV001SchemaData{
+				Hash: &models.HashedrekordV001SchemaDataHash{
+					Algorithm: swag.String("sha256"),
+					Value:     swag.String(hashStr),
+				},
+			},
+			Signature: &models.HashedrekordV001SchemaSignature{
+				Content:   strfmt.Base64(sig),
+				PublicKey: &models.HashedrekordV001SchemaSignaturePublicKey{},
+			},
+		},
+	}
+
+	// Test with a public key and a cert
+
+	// For the public key, we should have the key and the hash.
+	t.Run("public key", func(t *testing.T) {
+		v.HashedRekordObj.Signature.PublicKey.Content = strfmt.Base64(pub)
+
+		k, err := v.IndexKeys()
+		if err != nil {
+			t.Fatal(err)
+		}
+		keys := map[string]struct{}{}
+		for _, key := range k {
+			keys[key] = struct{}{}
+		}
+
+		if _, ok := keys[hashIndexKey]; !ok {
+			t.Errorf("missing hash index entry %s, got %v", hashIndexKey, keys)
+		}
+		want := hexHash(pub)
+		if _, ok := keys[want]; !ok {
+			t.Errorf("missing key index entry %s, got %v", want, keys)
+		}
+	})
+
+	// For the public key, we should have the key and the hash.
+	t.Run("cert", func(t *testing.T) {
+		v.HashedRekordObj.Signature.PublicKey.Content = strfmt.Base64(cert)
+
+		k, err := v.IndexKeys()
+		if err != nil {
+			t.Fatal(err)
+		}
+		keys := map[string]struct{}{}
+		for _, key := range k {
+			keys[key] = struct{}{}
+		}
+
+		if _, ok := keys[hashIndexKey]; !ok {
+			t.Errorf("missing hash index entry for public key test, got %v", keys)
+		}
+		if _, ok := keys[hexHash(cert)]; !ok {
+			t.Errorf("missing key index entry for public key test, got %v", keys)
+		}
+	})
+
+}
+
+func testKeyAndCert(t *testing.T) ([]byte, []byte, *ecdsa.PrivateKey) {
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	der, err := x509.MarshalPKIXPublicKey(&key.PublicKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pub := pem.EncodeToMemory(&pem.Block{
+		Bytes: der,
+		Type:  "PUBLIC KEY",
+	})
+
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ca := &x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject: pkix.Name{
+			Names: []pkix.AttributeTypeAndValue{
+				{
+					Type:  x509r.EmailAddressOID,
+					Value: "foo@bar.com",
+				},
+			},
+		},
+	}
+	cb, err := x509.CreateCertificate(rand.Reader, ca, ca, &priv.PublicKey, priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	certPem := pem.EncodeToMemory(&pem.Block{
+		Type:  "CERTIFICATE",
+		Bytes: cb,
+	})
+
+	return pub, certPem, priv
+}