Improve separation between type implementations and CLI code (#339)

* Refactor PKI factory and add type checking

This allows for more DRY addition of new PKI types, and stricter
type checking. This also allows for simpler enumeration of
supported PKI formats which will be used in further updates to
simplify the CLI codebase.

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* revamp CLI flags; support different versions for upload

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* Add Alpine Package type

This adds support for the alpine package format used by Alpine Linux,
which is the concatenation of three tgz files (signature, control data,
and then the actual package files).

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* use shaFlag for --artifact-hash

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* change arg type to PKIFormat

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* defer type-specific validation logic to type code (instead of in CLI); also use CliLogger throughout CLI

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* refactor factory code

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

* review comments

Signed-off-by: Bob Callaway <bob.callaway@gmail.com>

pkg/types/rekord/rekord_test.go

@@ -66,6 +66,10 @@ func (u UnmarshalTester) Attestation() (string, []byte) {
 	return "", nil
 }
 
+func (u UnmarshalTester) CreateFromArtifactProperties(_ context.Context, _ types.ArtifactProperties) (models.ProposedEntry, error) {
+	return nil, nil
+}
+
 type UnmarshalFailsTester struct {
 	UnmarshalTester
 }

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

@@ -23,6 +23,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
+	"path/filepath"
 	"strings"
 
 	"github.com/asaskevich/govalidator"
@@ -161,7 +163,10 @@ 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 := pki.NewArtifactFactory(v.RekordObj.Signature.Format)
+	artifactFactory, err := pki.NewArtifactFactory(pki.Format(v.RekordObj.Signature.Format))
+	if err != nil {
+		return err
+	}
 
 	g.Go(func() error {
 		defer hashW.Close()
@@ -383,3 +388,99 @@ func (v V001Entry) Validate() error {
 func (v V001Entry) Attestation() (string, []byte) {
 	return "", nil
 }
+
+func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) {
+	returnVal := models.Rekord{}
+	re := V001Entry{}
+
+	// we will need artifact, public-key, signature
+	re.RekordObj.Data = &models.RekordV001SchemaData{}
+
+	var err error
+	artifactBytes := props.ArtifactBytes
+	if artifactBytes == nil {
+		if props.ArtifactPath == nil {
+			return nil, errors.New("path to artifact (file or URL) must be specified")
+		}
+		if props.ArtifactPath.IsAbs() {
+			re.RekordObj.Data.URL = strfmt.URI(props.ArtifactPath.String())
+			if props.ArtifactHash != "" {
+				re.RekordObj.Data.Hash = &models.RekordV001SchemaDataHash{
+					Algorithm: swag.String(models.RekordV001SchemaDataHashAlgorithmSha256),
+					Value:     swag.String(props.ArtifactHash),
+				}
+			}
+		} else {
+			artifactBytes, err := ioutil.ReadFile(filepath.Clean(props.ArtifactPath.Path))
+			if err != nil {
+				return nil, fmt.Errorf("error reading artifact file: %w", err)
+			}
+			re.RekordObj.Data.Content = strfmt.Base64(artifactBytes)
+		}
+	} else {
+		re.RekordObj.Data.Content = strfmt.Base64(artifactBytes)
+	}
+
+	re.RekordObj.Signature = &models.RekordV001SchemaSignature{}
+	switch props.PKIFormat {
+	case "pgp":
+		re.RekordObj.Signature.Format = models.RekordV001SchemaSignatureFormatPgp
+	case "minisign":
+		re.RekordObj.Signature.Format = models.RekordV001SchemaSignatureFormatMinisign
+	case "x509":
+		re.RekordObj.Signature.Format = models.RekordV001SchemaSignatureFormatX509
+	case "ssh":
+		re.RekordObj.Signature.Format = models.RekordV001SchemaSignatureFormatSSH
+	}
+	sigBytes := props.SignatureBytes
+	if sigBytes == nil {
+		if props.SignaturePath == nil {
+			return nil, errors.New("a detached signature must be provided")
+		}
+		if props.SignaturePath.IsAbs() {
+			re.RekordObj.Signature.URL = strfmt.URI(props.SignaturePath.String())
+		} else {
+			sigBytes, err = ioutil.ReadFile(filepath.Clean(props.SignaturePath.Path))
+			if err != nil {
+				return nil, fmt.Errorf("error reading signature file: %w", err)
+			}
+			re.RekordObj.Signature.Content = strfmt.Base64(sigBytes)
+		}
+	} else {
+		re.RekordObj.Signature.Content = strfmt.Base64(sigBytes)
+	}
+
+	re.RekordObj.Signature.PublicKey = &models.RekordV001SchemaSignaturePublicKey{}
+	publicKeyBytes := props.PublicKeyBytes
+	if publicKeyBytes == nil {
+		if props.PublicKeyPath == nil {
+			return nil, errors.New("public key must be provided to verify detached signature")
+		}
+		if props.PublicKeyPath.IsAbs() {
+			re.RekordObj.Signature.PublicKey.URL = strfmt.URI(props.PublicKeyPath.String())
+		} else {
+			publicKeyBytes, err = ioutil.ReadFile(filepath.Clean(props.PublicKeyPath.Path))
+			if err != nil {
+				return nil, fmt.Errorf("error reading public key file: %w", err)
+			}
+			re.RekordObj.Signature.PublicKey.Content = strfmt.Base64(publicKeyBytes)
+		}
+	} else {
+		re.RekordObj.Signature.PublicKey.Content = strfmt.Base64(publicKeyBytes)
+	}
+
+	if err := re.Validate(); err != nil {
+		return nil, err
+	}
+
+	if re.HasExternalEntities() {
+		if err := re.FetchExternalEntities(ctx); err != nil {
+			return nil, fmt.Errorf("error retrieving external entities: %v", err)
+		}
+	}
+
+	returnVal.APIVersion = swag.String(re.APIVersion())
+	returnVal.Spec = re.RekordObj
+
+	return &returnVal, nil
+}

pkg/types/rfc3161/rfc3161.go

@@ -16,8 +16,9 @@
 package rfc3161
 
 import (
-	"errors"
+	"context"
 
+	"github.com/pkg/errors"
 	"github.com/sigstore/rekor/pkg/generated/models"
 	"github.com/sigstore/rekor/pkg/types"
 )
@@ -35,15 +36,15 @@ func init() {
 }
 
 func New() types.TypeImpl {
-	brt := BaseTimestampType{}
-	brt.Kind = KIND
-	brt.VersionMap = VersionMap
-	return &brt
+	btt := BaseTimestampType{}
+	btt.Kind = KIND
+	btt.VersionMap = VersionMap
+	return &btt
 }
 
 var VersionMap = types.NewSemVerEntryFactoryMap()
 
-func (rt BaseTimestampType) UnmarshalEntry(pe models.ProposedEntry) (types.EntryImpl, error) {
+func (btt BaseTimestampType) UnmarshalEntry(pe models.ProposedEntry) (types.EntryImpl, error) {
 	if pe == nil {
 		return nil, errors.New("proposed entry cannot be nil")
 	}
@@ -53,5 +54,20 @@ func (rt BaseTimestampType) UnmarshalEntry(pe models.ProposedEntry) (types.Entry
 		return nil, errors.New("cannot unmarshal non-Timestamp types")
 	}
 
-	return rt.VersionedUnmarshal(rfc3161, *rfc3161.APIVersion)
+	return btt.VersionedUnmarshal(rfc3161, *rfc3161.APIVersion)
+}
+
+func (btt *BaseTimestampType) CreateProposedEntry(ctx context.Context, version string, props types.ArtifactProperties) (models.ProposedEntry, error) {
+	if version == "" {
+		version = btt.DefaultVersion()
+	}
+	ei, err := btt.VersionedUnmarshal(nil, version)
+	if err != nil {
+		return nil, errors.Wrap(err, "fetching RFC3161 version implementation")
+	}
+	return ei.CreateFromArtifactProperties(ctx, props)
+}
+
+func (btt BaseTimestampType) DefaultVersion() string {
+	return "0.0.1"
 }

pkg/types/rfc3161/rfc3161_test.go

@@ -69,6 +69,9 @@ func (u UnmarshalTester) Unmarshal(pe models.ProposedEntry) error {
 func (u UnmarshalFailsTester) Attestation() (string, []byte) {
 	return "", nil
 }
+func (u UnmarshalTester) CreateFromArtifactProperties(_ context.Context, _ types.ArtifactProperties) (models.ProposedEntry, error) {
+	return nil, nil
+}
 
 type UnmarshalFailsTester struct {
 	UnmarshalTester

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

@@ -24,6 +24,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/ioutil"
+	"path/filepath"
 
 	"github.com/sigstore/rekor/pkg/types/rfc3161"
 
@@ -175,14 +177,14 @@ func (v V001Entry) Validate() error {
 		return err
 	}
 	if tsr.Status.Status != pkcs9.StatusGranted && tsr.Status.Status != pkcs9.StatusGrantedWithMods {
-		return fmt.Errorf("Tsr status not granted: %v", tsr.Status.Status)
+		return fmt.Errorf("tsr status not granted: %v", tsr.Status.Status)
 	}
 	if !tsr.TimeStampToken.ContentType.Equal(asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 7, 2}) {
-		return fmt.Errorf("Tsr wrong content type: %v", tsr.TimeStampToken.ContentType)
+		return fmt.Errorf("tsr wrong content type: %v", tsr.TimeStampToken.ContentType)
 	}
 	_, err = tsr.TimeStampToken.Content.Verify(nil, false)
 	if err != nil {
-		return fmt.Errorf("Tsr verification error: %v", err)
+		return fmt.Errorf("tsr verification error: %v", err)
 	}
 
 	return nil
@@ -191,3 +193,36 @@ func (v V001Entry) Validate() error {
 func (v V001Entry) Attestation() (string, []byte) {
 	return "", nil
 }
+
+func (v V001Entry) CreateFromArtifactProperties(_ context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) {
+	returnVal := models.Rfc3161{}
+
+	var err error
+	artifactBytes := props.ArtifactBytes
+	if artifactBytes == nil {
+		if props.ArtifactPath == nil {
+			return nil, errors.New("path to artifact file must be specified")
+		}
+		if props.ArtifactPath.IsAbs() {
+			return nil, errors.New("RFC3161 timestamps cannot be fetched over HTTP(S)")
+		}
+		artifactBytes, err = ioutil.ReadFile(filepath.Clean(props.ArtifactPath.Path))
+		if err != nil {
+			return nil, fmt.Errorf("error reading artifact file: %w", err)
+		}
+	}
+
+	b64 := strfmt.Base64(artifactBytes)
+	re := V001Entry{
+		Rfc3161Obj: models.Rfc3161V001Schema{
+			Tsr: &models.Rfc3161V001SchemaTsr{
+				Content: &b64,
+			},
+		},
+	}
+
+	returnVal.Spec = re.Rfc3161Obj
+	returnVal.APIVersion = swag.String(re.APIVersion())
+
+	return &returnVal, nil
+}

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

@@ -126,7 +126,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
 			expectCanonicalizeSuccess:    true,
-			expectValidationErrorMessage: "Tsr status not granted: 2",
+			expectValidationErrorMessage: "tsr status not granted: 2",
 		},
 		{
 			caseDesc: "invalid obj - bad content type",
@@ -140,7 +140,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
 			expectCanonicalizeSuccess:    true,
-			expectValidationErrorMessage: "Tsr wrong content type: 0.0.0.0.42",
+			expectValidationErrorMessage: "tsr wrong content type: 0.0.0.0.42",
 		},
 		{
 			caseDesc: "invalid obj - bad content",
@@ -154,7 +154,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
 			expectCanonicalizeSuccess:    true,
-			expectValidationErrorMessage: "Tsr verification error",
+			expectValidationErrorMessage: "tsr verification error",
 		},
 		{
 			caseDesc: "valid obj with extra data",

pkg/types/rpm/rpm.go

@@ -16,8 +16,9 @@
 package rpm
 
 import (
-	"errors"
+	"context"
 
+	"github.com/pkg/errors"
 	"github.com/sigstore/rekor/pkg/generated/models"
 	"github.com/sigstore/rekor/pkg/types"
 )
@@ -55,3 +56,18 @@ func (brt *BaseRPMType) UnmarshalEntry(pe models.ProposedEntry) (types.EntryImpl
 
 	return brt.VersionedUnmarshal(rpm, *rpm.APIVersion)
 }
+
+func (brt *BaseRPMType) CreateProposedEntry(ctx context.Context, version string, props types.ArtifactProperties) (models.ProposedEntry, error) {
+	if version == "" {
+		version = brt.DefaultVersion()
+	}
+	ei, err := brt.VersionedUnmarshal(nil, version)
+	if err != nil {
+		return nil, errors.Wrap(err, "fetching RPM version implementation")
+	}
+	return ei.CreateFromArtifactProperties(ctx, props)
+}
+
+func (brt BaseRPMType) DefaultVersion() string {
+	return "0.0.1"
+}

pkg/types/rpm/rpm_test.go

@@ -66,6 +66,10 @@ func (u UnmarshalTester) Attestation() (string, []byte) {
 	return "", nil
 }
 
+func (u UnmarshalTester) CreateFromArtifactProperties(_ context.Context, _ types.ArtifactProperties) (models.ProposedEntry, error) {
+	return nil, nil
+}
+
 type UnmarshalFailsTester struct {
 	UnmarshalTester
 }

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

@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -163,7 +164,10 @@ 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 := pki.NewArtifactFactory("pgp")
+	artifactFactory, err := pki.NewArtifactFactory(pki.PGP)
+	if err != nil {
+		return err
+	}
 
 	g.Go(func() error {
 		defer hashW.Close()
@@ -358,3 +362,65 @@ func (v V001Entry) Validate() error {
 func (v V001Entry) Attestation() (string, []byte) {
 	return "", nil
 }
+
+func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) {
+	returnVal := models.Rpm{}
+	re := V001Entry{}
+
+	// we will need artifact, public-key, signature
+	re.RPMModel = models.RpmV001Schema{}
+	re.RPMModel.Package = &models.RpmV001SchemaPackage{}
+
+	var err error
+	artifactBytes := props.ArtifactBytes
+	if artifactBytes == nil {
+		if props.ArtifactPath == nil {
+			return nil, errors.New("path to RPM file (file or URL) must be specified")
+		}
+		if props.ArtifactPath.IsAbs() {
+			re.RPMModel.Package.URL = strfmt.URI(props.ArtifactPath.String())
+		} else {
+			artifactBytes, err = ioutil.ReadFile(filepath.Clean(props.ArtifactPath.Path))
+			if err != nil {
+				return nil, fmt.Errorf("error reading RPM file: %w", err)
+			}
+			re.RPMModel.Package.Content = strfmt.Base64(artifactBytes)
+		}
+	} else {
+		re.RPMModel.Package.Content = strfmt.Base64(artifactBytes)
+	}
+
+	re.RPMModel.PublicKey = &models.RpmV001SchemaPublicKey{}
+	publicKeyBytes := props.PublicKeyBytes
+	if publicKeyBytes == nil {
+		if props.PublicKeyPath == nil {
+			return nil, errors.New("public key must be provided to verify RPM signature")
+		}
+		if props.PublicKeyPath.IsAbs() {
+			re.RPMModel.PublicKey.URL = strfmt.URI(props.PublicKeyPath.String())
+		} else {
+			publicKeyBytes, err = ioutil.ReadFile(filepath.Clean(props.PublicKeyPath.Path))
+			if err != nil {
+				return nil, fmt.Errorf("error reading public key file: %w", err)
+			}
+			re.RPMModel.PublicKey.Content = strfmt.Base64(publicKeyBytes)
+		}
+	} else {
+		re.RPMModel.PublicKey.Content = strfmt.Base64(publicKeyBytes)
+	}
+
+	if err := re.Validate(); err != nil {
+		return nil, err
+	}
+
+	if re.HasExternalEntities() {
+		if err := re.FetchExternalEntities(context.Background()); err != nil {
+			return nil, fmt.Errorf("error retrieving external entities: %v", err)
+		}
+	}
+
+	returnVal.APIVersion = swag.String(re.APIVersion())
+	returnVal.Spec = re.RPMModel
+
+	return &returnVal, nil
+}

pkg/types/types.go

@@ -16,6 +16,7 @@
 package types
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"sync"
@@ -37,26 +38,43 @@ type RekorType struct {
 // TypeImpl is implemented by all types to support the polymorphic conversion of abstract
 // proposed entry to a working implementation for the versioned type requested, if supported
 type TypeImpl interface {
+	CreateProposedEntry(context.Context, string, ArtifactProperties) (models.ProposedEntry, error)
+	DefaultVersion() string
+	SupportedVersions() []string
 	UnmarshalEntry(pe models.ProposedEntry) (EntryImpl, error)
 }
 
 // VersionedUnmarshal extracts the correct implementing factory function from the type's version map,
 // creates an entry of that versioned type and then calls that versioned type's unmarshal method
 func (rt *RekorType) VersionedUnmarshal(pe models.ProposedEntry, version string) (EntryImpl, error) {
-	if pe == nil {
-		return nil, errors.New("proposed entry cannot be nil")
-	}
-
 	ef, err := rt.VersionMap.GetEntryFactory(version)
 	if err != nil {
-		return nil, fmt.Errorf("%s implementation for version '%v' not found: %w", pe.Kind(), version, err)
+		return nil, fmt.Errorf("%s implementation for version '%v' not found: %w", rt.Kind, version, err)
 	}
 	entry := ef()
 	if entry == nil {
 		return nil, errors.New("failure generating object")
 	}
-	if err := entry.Unmarshal(pe); err != nil {
-		return nil, err
+	if pe == nil {
+		return entry, nil
 	}
-	return entry, nil
+	return entry, entry.Unmarshal(pe)
+}
+
+func (rt *RekorType) SupportedVersions() []string {
+	return rt.VersionMap.SupportedVersions()
+}
+
+// ListImplementedTypes returns a list of all type strings currently known to
+// be implemented
+func ListImplementedTypes() []string {
+	retVal := []string{}
+	TypeMap.Range(func(k interface{}, v interface{}) bool {
+		tf := v.(func() TypeImpl)
+		for _, verStr := range tf().SupportedVersions() {
+			retVal = append(retVal, fmt.Sprintf("%v:%v", k.(string), verStr))
+		}
+		return true
+	})
+	return retVal
 }

pkg/types/versionmap.go

@@ -29,6 +29,7 @@ type VersionEntryFactoryMap interface {
 	GetEntryFactory(string) (EntryFactory, error) // return the entry factory for the specified version
 	SetEntryFactory(string, EntryFactory) error   // set the entry factory for the specified version
 	Count() int                                   // return the count of entry factories currently in the map
+	SupportedVersions() []string                  // return a list of versions currently stored in the map
 }
 
 // SemVerEntryFactoryMap implements a map that allows implementations to specify their supported versions using
@@ -86,3 +87,11 @@ func (s *SemVerEntryFactoryMap) SetEntryFactory(constraint string, ef EntryFacto
 	s.factoryMap[constraint] = ef
 	return nil
 }
+
+func (s *SemVerEntryFactoryMap) SupportedVersions() []string {
+	var versions []string
+	for k := range s.factoryMap {
+		versions = append(versions, k)
+	}
+	return versions
+}