From 59d57ea9b3fee74d4c868b5c00c894faea6a362f Mon Sep 17 00:00:00 2001
From: dlorenc <dlorenc@google.com>
Date: Wed, 22 Dec 2021 08:07:59 -0600
Subject: [PATCH] More refactoring to remove intermediate objects. (#558)

A lot of these only support one key type, so we don't need to go through
the map.

Signed-off-by: Dan Lorenc <lorenc.d@gmail.com>
---
 pkg/types/alpine/v0.0.1/entry.go  |  7 +---
 pkg/types/helm/provenance_test.go |  8 +----
 pkg/types/helm/v0.0.1/entry.go    |  9 ++---
 pkg/types/intoto/v0.0.1/entry.go  |  8 +----
 pkg/types/jar/v0.0.1/entry.go     |  9 ++---
 pkg/types/rekord/v0.0.1/entry.go  | 56 +++++++++++++++++++------------
 pkg/types/rpm/v0.0.1/entry.go     |  6 +---
 pkg/types/tuf/v0.0.1/entry.go     | 10 ++----
 8 files changed, 47 insertions(+), 66 deletions(-)

diff --git a/pkg/types/alpine/v0.0.1/entry.go b/pkg/types/alpine/v0.0.1/entry.go
index 4d18773..a148f63 100644
--- a/pkg/types/alpine/v0.0.1/entry.go
+++ b/pkg/types/alpine/v0.0.1/entry.go
@@ -118,11 +118,6 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		return types.ValidationError(err)
 	}
 
-	artifactFactory, err := pki.NewArtifactFactory(pki.X509)
-	if err != nil {
-		return err
-	}
-
 	g, ctx := errgroup.WithContext(ctx)
 
 	hashR, hashW := io.Pipe()
@@ -188,7 +183,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 		defer keyReadCloser.Close()
 
-		v.keyObj, err = artifactFactory.NewPublicKey(keyReadCloser)
+		v.keyObj, err = x509.NewPublicKey(keyReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
diff --git a/pkg/types/helm/provenance_test.go b/pkg/types/helm/provenance_test.go
index 29a623c..77af5a8 100644
--- a/pkg/types/helm/provenance_test.go
+++ b/pkg/types/helm/provenance_test.go
@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	"github.com/sigstore/rekor/pkg/generated/models"
-	"github.com/sigstore/rekor/pkg/pki"
 	"github.com/sigstore/rekor/pkg/pki/pgp"
 )
 
@@ -62,12 +61,7 @@ func TestProvenance(t *testing.T) {
 		t.Fatalf("failed to parse public key: %v", err)
 	}
 
-	artifactFactory, err := pki.NewArtifactFactory(pki.PGP)
-	if err != nil {
-		t.Fatalf("Failed to create PGP pki factory %v", err)
-	}
-
-	sig, err := artifactFactory.NewSignature(provenance.Block.ArmoredSignature.Body)
+	sig, err := pgp.NewSignature(provenance.Block.ArmoredSignature.Body)
 	if err != nil {
 		t.Fatalf("Failed to create signature %v", err)
 	}
diff --git a/pkg/types/helm/v0.0.1/entry.go b/pkg/types/helm/v0.0.1/entry.go
index 7f94a14..2665f81 100644
--- a/pkg/types/helm/v0.0.1/entry.go
+++ b/pkg/types/helm/v0.0.1/entry.go
@@ -126,11 +126,6 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		return types.ValidationError(err)
 	}
 
-	artifactFactory, err := pki.NewArtifactFactory(pki.PGP)
-	if err != nil {
-		return err
-	}
-
 	g, ctx := errgroup.WithContext(ctx)
 
 	provenanceR, provenanceW := io.Pipe()
@@ -165,7 +160,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 		defer keyReadCloser.Close()
 
-		v.keyObj, err = artifactFactory.NewPublicKey(keyReadCloser)
+		v.keyObj, err = pgp.NewPublicKey(keyReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
@@ -191,7 +186,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 
 		// Set signature
-		sig, err := artifactFactory.NewSignature(provenance.Block.ArmoredSignature.Body)
+		sig, err := pgp.NewSignature(provenance.Block.ArmoredSignature.Body)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
diff --git a/pkg/types/intoto/v0.0.1/entry.go b/pkg/types/intoto/v0.0.1/entry.go
index 262e8b5..1c2fc28 100644
--- a/pkg/types/intoto/v0.0.1/entry.go
+++ b/pkg/types/intoto/v0.0.1/entry.go
@@ -121,13 +121,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 		return err
 	}
 
-	// Only support x509 signatures for intoto attestations
-	af, err := pki.NewArtifactFactory(pki.X509)
-	if err != nil {
-		return err
-	}
-
-	v.keyObj, err = af.NewPublicKey(bytes.NewReader(*v.IntotoObj.PublicKey))
+	v.keyObj, err = x509.NewPublicKey(bytes.NewReader(*v.IntotoObj.PublicKey))
 	if err != nil {
 		return err
 	}
diff --git a/pkg/types/jar/v0.0.1/entry.go b/pkg/types/jar/v0.0.1/entry.go
index 8e395eb..8a75749 100644
--- a/pkg/types/jar/v0.0.1/entry.go
+++ b/pkg/types/jar/v0.0.1/entry.go
@@ -32,6 +32,7 @@ import (
 
 	"github.com/sigstore/rekor/pkg/log"
 	"github.com/sigstore/rekor/pkg/pki"
+	"github.com/sigstore/rekor/pkg/pki/pkcs7"
 	"github.com/sigstore/rekor/pkg/types"
 	"github.com/sigstore/rekor/pkg/types/jar"
 	"github.com/sigstore/rekor/pkg/util"
@@ -162,22 +163,18 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	}
 	v.jarObj = jarObj[0]
 
-	af, err := pki.NewArtifactFactory(pki.PKCS7)
-	if err != nil {
-		return err
-	}
 	// we need to find and extract the PKCS7 bundle from the JAR file manually
 	sigPKCS7, err := extractPKCS7SignatureFromJAR(zipReader)
 	if err != nil {
 		return types.ValidationError(err)
 	}
 
-	v.keyObj, err = af.NewPublicKey(bytes.NewReader(sigPKCS7))
+	v.keyObj, err = pkcs7.NewPublicKey(bytes.NewReader(sigPKCS7))
 	if err != nil {
 		return types.ValidationError(err)
 	}
 
-	v.sigObj, err = af.NewSignature(bytes.NewReader(sigPKCS7))
+	v.sigObj, err = pkcs7.NewSignature(bytes.NewReader(sigPKCS7))
 	if err != nil {
 		return types.ValidationError(err)
 	}
diff --git a/pkg/types/rekord/v0.0.1/entry.go b/pkg/types/rekord/v0.0.1/entry.go
index e3f634e..c7b4b8b 100644
--- a/pkg/types/rekord/v0.0.1/entry.go
+++ b/pkg/types/rekord/v0.0.1/entry.go
@@ -16,6 +16,7 @@
 package rekord
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -52,8 +53,6 @@ func init() {
 
 type V001Entry struct {
 	RekordObj models.RekordV001Schema
-	keyObj    pki.PublicKey
-	sigObj    pki.Signature
 }
 
 func (v V001Entry) APIVersion() string {
@@ -67,7 +66,16 @@ func NewEntry() types.EntryImpl {
 func (v V001Entry) IndexKeys() ([]string, error) {
 	var result []string
 
-	key, err := v.keyObj.CanonicalValue()
+	af, err := pki.NewArtifactFactory(pki.Format(v.RekordObj.Signature.Format))
+	if err != nil {
+		return nil, err
+	}
+	keyObj, err := af.NewPublicKey(bytes.NewReader(v.RekordObj.Signature.PublicKey.Content))
+	if err != nil {
+		return nil, err
+	}
+
+	key, err := keyObj.CanonicalValue()
 	if err != nil {
 		log.Logger.Error(err)
 	} else {
@@ -75,7 +83,7 @@ func (v V001Entry) IndexKeys() ([]string, error) {
 		result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:])))
 	}
 
-	result = append(result, v.keyObj.EmailAddresses()...)
+	result = append(result, keyObj.EmailAddresses()...)
 
 	if v.RekordObj.Data.Hash != nil {
 		hashKey := strings.ToLower(fmt.Sprintf("%s:%s", *v.RekordObj.Data.Hash.Algorithm, *v.RekordObj.Data.Hash.Value))
@@ -99,6 +107,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	if err := v.RekordObj.Validate(strfmt.Default); err != nil {
 		return err
 	}
+
 	// cross field validation
 	return v.validate()
 
@@ -117,9 +126,14 @@ func (v *V001Entry) hasExternalEntities() bool {
 	return false
 }
 
-func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
+func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, pki.Signature, error) {
 	g, ctx := errgroup.WithContext(ctx)
 
+	af, err := pki.NewArtifactFactory(pki.Format(v.RekordObj.Signature.Format))
+	if err != nil {
+		return nil, nil, err
+	}
+
 	hashR, hashW := io.Pipe()
 	sigR, sigW := io.Pipe()
 	defer hashR.Close()
@@ -131,10 +145,6 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	if v.RekordObj.Data.Hash != nil && v.RekordObj.Data.Hash.Value != nil {
 		oldSHA = swag.StringValue(v.RekordObj.Data.Hash.Value)
 	}
-	artifactFactory, err := pki.NewArtifactFactory(pki.Format(v.RekordObj.Signature.Format))
-	if err != nil {
-		return types.ValidationError(err)
-	}
 
 	g.Go(func() error {
 		defer hashW.Close()
@@ -188,7 +198,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 		defer sigReadCloser.Close()
 
-		signature, err := artifactFactory.NewSignature(sigReadCloser)
+		signature, err := af.NewSignature(sigReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
@@ -213,7 +223,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 		defer keyReadCloser.Close()
 
-		key, err := artifactFactory.NewPublicKey(keyReadCloser)
+		key, err := af.NewPublicKey(keyReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
@@ -226,15 +236,19 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 	})
 
+	var (
+		keyObj pki.PublicKey
+		sigObj pki.Signature
+	)
 	g.Go(func() error {
-		v.keyObj, v.sigObj = <-keyResult, <-sigResult
+		keyObj, sigObj = <-keyResult, <-sigResult
 
-		if v.keyObj == nil || v.sigObj == nil {
+		if keyObj == nil || sigObj == nil {
 			return closePipesOnError(errors.New("failed to read signature or public key"))
 		}
 
 		var err error
-		if err = v.sigObj.Verify(sigR, v.keyObj); err != nil {
+		if err = sigObj.Verify(sigR, keyObj); err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
 
@@ -249,7 +263,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	computedSHA := <-hashResult
 
 	if err := g.Wait(); err != nil {
-		return err
+		return nil, nil, err
 	}
 
 	// if we get here, all goroutines succeeded without error
@@ -259,11 +273,12 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		v.RekordObj.Data.Hash.Value = swag.String(computedSHA)
 	}
 
-	return nil
+	return keyObj, sigObj, nil
 }
 
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
-	if err := v.fetchExternalEntities(ctx); err != nil {
+	keyObj, sigObj, err := v.fetchExternalEntities(ctx)
+	if err != nil {
 		return nil, err
 	}
 
@@ -274,15 +289,14 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	// signature URL (if known) is not set deliberately
 	canonicalEntry.Signature.Format = v.RekordObj.Signature.Format
 
-	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.RekordV001SchemaSignaturePublicKey{}
-	canonicalEntry.Signature.PublicKey.Content, err = v.keyObj.CanonicalValue()
+	canonicalEntry.Signature.PublicKey.Content, err = keyObj.CanonicalValue()
 	if err != nil {
 		return nil, err
 	}
@@ -428,7 +442,7 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 	}
 
 	if re.hasExternalEntities() {
-		if err := re.fetchExternalEntities(ctx); err != nil {
+		if _, _, err := re.fetchExternalEntities(ctx); err != nil {
 			return nil, fmt.Errorf("error retrieving external entities: %v", err)
 		}
 	}
diff --git a/pkg/types/rpm/v0.0.1/entry.go b/pkg/types/rpm/v0.0.1/entry.go
index c9d71f0..219b4a3 100644
--- a/pkg/types/rpm/v0.0.1/entry.go
+++ b/pkg/types/rpm/v0.0.1/entry.go
@@ -137,10 +137,6 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 	if v.RPMModel.Package.Hash != nil && v.RPMModel.Package.Hash.Value != nil {
 		oldSHA = swag.StringValue(v.RPMModel.Package.Hash.Value)
 	}
-	artifactFactory, err := pki.NewArtifactFactory(pki.PGP)
-	if err != nil {
-		return err
-	}
 
 	g.Go(func() error {
 		defer hashW.Close()
@@ -191,7 +187,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		}
 		defer keyReadCloser.Close()
 
-		v.keyObj, err = artifactFactory.NewPublicKey(keyReadCloser)
+		v.keyObj, err = pgp.NewPublicKey(keyReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
diff --git a/pkg/types/tuf/v0.0.1/entry.go b/pkg/types/tuf/v0.0.1/entry.go
index afe78fe..4020493 100644
--- a/pkg/types/tuf/v0.0.1/entry.go
+++ b/pkg/types/tuf/v0.0.1/entry.go
@@ -40,6 +40,7 @@ import (
 	"github.com/go-openapi/strfmt"
 
 	"github.com/sigstore/rekor/pkg/pki"
+
 	ptuf "github.com/sigstore/rekor/pkg/pki/tuf"
 
 	"github.com/go-openapi/swag"
@@ -156,11 +157,6 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	closePipesOnError := types.PipeCloser(metaR, metaW, rootR, rootW)
 
 	// verify artifact signature
-	artifactFactory, err := pki.NewArtifactFactory(pki.Tuf)
-	if err != nil {
-		return err
-	}
-
 	sigResult := make(chan pki.Signature)
 
 	g.Go(func() error {
@@ -182,7 +178,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 		defer sigReadCloser.Close()
 
-		signature, err := artifactFactory.NewSignature(sigReadCloser)
+		signature, err := ptuf.NewSignature(sigReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
@@ -216,7 +212,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 		}
 		defer keyReadCloser.Close()
 
-		key, err := artifactFactory.NewPublicKey(keyReadCloser)
+		key, err := ptuf.NewPublicKey(keyReadCloser)
 		if err != nil {
 			return closePipesOnError(types.ValidationError(err))
 		}
-- 
GitLab