Repository: sigstore/rekor
Pull request #370 (2021-07-13 21:06:01)

Clean up EntryImpl interface
@loosebazooka this originated from your comment: https://github.com/sigstore/rekor/pull/324#pullrequestreview-684149150 where you rightly pointed out that the responsibility of the `EntryImpl` interface was not clear. This removes several of the methods from the interface definition shifting responsibility for basic verification to the `Unmarshal` method and fetching external resources & signature verification to the `Canonicalize` method. 

<!--
Thanks for opening a pull request!

Please remember to:
- mention any issue(s) that this PR closes using a closing keyword as well as the issue number, such as "Closes #XYZ" or "Resolves sigstore/repo-name#XYZ", cf.
  [documentation](https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
- ensure your commits are signed-off, as sigstore uses the [DCO](https://github.com/enarx/enarx/wiki/How-to-contribute-code#developer-certificate-of-origin) using `git commit -s`, or `git commit -s --amend` if you want to amend already existing commits
- lastly, ensure there are no merge commits!
Thank you :)
-->



Patch:
diff --git a/pkg/api/entries.go b/pkg/api/entries.go
index 05f0697..265bc72 100644
--- a/pkg/api/entries.go
+++ b/pkg/api/entries.go
@@ -311,15 +311,6 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
 				if err != nil {
 					return err
 				}
-				if err := entry.Validate(); err != nil {
-					return err
-				}
-
-				if entry.HasExternalEntities() {
-					if err := entry.FetchExternalEntities(httpReqCtx); err != nil {
-						return err
-					}
-				}
 
 				leaf, err := entry.Canonicalize(httpReqCtx)
 				if err != nil {
diff --git a/pkg/api/error.go b/pkg/api/error.go
index bff642c..06cd862 100644
--- a/pkg/api/error.go
+++ b/pkg/api/error.go
@@ -111,6 +111,7 @@ func handleRekorAPIError(params interface{}, code int, err error, message string
 			}
 			return resp
 		default:
+			logMsg(params.HTTPRequest)
 			return entries.NewCreateLogEntryDefault(code).WithPayload(errorMsg(message, code))
 		}
 	case entries.SearchLogQueryParams:
diff --git a/pkg/types/README.md b/pkg/types/README.md
index c7fead0..3a9db59 100644
--- a/pkg/types/README.md
+++ b/pkg/types/README.md
@@ -6,7 +6,19 @@ Rekor supports pluggable types (aka different schemas) for entries stored in the
 
 ### Currently supported types
 
-- Rekord (default type) [schema](rekord/rekord_schema.json)
+- Alpine Packages [schema](alpine/alpine_schema.json)
+  - Versions: 0.0.1
+- Helm Provenance Files [schema](helm/helm_schema.json)
+  - Versions: 0.0.1
+- In-Toto Attestations [schema](intoto/intoto_schema.json)
+  - Versions: 0.0.1
+- Java Archives (JAR Files) [schema](jar/jar_schema.json)
+  - Versions: 0.0.1
+- Rekord *(default type)* [schema](rekord/rekord_schema.json)
+  - Versions: 0.0.1
+- RFC3161 Timestamps [schema](rfc3161/rfc3161_schema.json)
+  - Versions: 0.0.1
+- RPM Packages [schema](rpm/rpm_schema.json)
   - Versions: 0.0.1
 
 
@@ -61,14 +73,20 @@ To add a new type (called `newType` in this example):
 3. In this new Go package, define a struct that:
 ```go
 type TypeImpl interface {
+	CreateProposedEntry(context.Context, string, ArtifactProperties) (models.ProposedEntry, error)
+	DefaultVersion() string
+	SupportedVersions() []string
 	UnmarshalEntry(pe models.ProposedEntry) (EntryImpl, error)
 }
 ```
   - implements the `TypeImpl` interface as defined in `types.go`:
+    - `CreateProposedEntry` will be called with string specifying the requested version and an ArtifactProperties struct that calls the version's `CreateFromArtifactProperties` method
+    - `DefaultVersion` specifies the default version for the type to be used if one is not explicitly requested
+    - `SupportedVersions` specifies a list of all version strings that are supported by this Rekor instance
     - `UnmarshalEntry` will be called with a pointer to a struct that was automatically generated for the type defined in `openapi.yaml` by the [go-swagger](http://github.com/go-swagger/go-swagger) tool used by Rekor
       - This struct will be defined in the generated file at `pkg/generated/models/newType.go` (where `newType` is replaced with the name of the type you are adding)
       - This method should return a pointer to an instance of a struct that implements the `EntryImpl` interface as defined in `types.go`, or a `nil` pointer with an error specified
-  - embedds the `RekorType` type into the struct definition 
+  - embeds the `RekorType` type into the struct definition 
     - The purpose of this is to set the Kind variable to match the type name
     - `RekorType` also includes a `VersionMap` field, which provides the lookup for a version string from a proposed entry to find the correct implmentation code
 
@@ -78,21 +96,20 @@ type EntryImpl interface {
 	APIVersion() string
 	IndexKeys() []string
 	Canonicalize(ctx context.Context) ([]byte, error)
-	FetchExternalEntities(ctx context.Context) error
-	HasExternalEntities() bool
 	Unmarshal(pe models.ProposedEntry) error
-	Validate() error
+	Attestation() (string, []byte)
+	CreateFromArtifactProperties(context.Context, ArtifactProperties) (models.ProposedEntry, error)
 }
 ```
 
   - `APIVersion` should return a version string that identifies the version of the type supported by the Rekor server
   - `IndexKeys` should return a `[]string` that extracts the keys from an entry to be stored in the search index
   - `Canonicalize` should return a `[]byte` containing the canonicalized contents representing the entry. The canonicalization of contents is important as we should have one record per unique signed object in the transparency log.
-  - `FetchExternalEntities` should retrieve any entities that make up the entry which were not included in the object provided in the HTTP request to the Rekor server
-  - `HasExternalEntities` indicates whether the instance of the struct has any external entities it has yet to fetch and resolve
   - `Unmarshal` will be called with a pointer to a struct that was automatically generated for the type defined in `openapi.yaml` by the [go-swagger](http://github.com/go-swagger/go-swagger) tool used by Rekor
     - This method should validate the contents of the struct to ensure any string or cross-field dependencies are met to successfully insert an entry of this type into the transparency log
-  - `Validate` performs cross-field validation for fields in object that can not be expressed easily in the OpenAPI definition
+    - This method should also succesfully unmarshal entries that have been canonicalized and inserted into the log; however, it is worth noting that you may not be able to re-canonicalize an entry (since the original content required to verify the signature may not be present in the persisted log entry).
+  - `Attestation` provides types to return an in-toto attestation that should be persisted.
+  - `CreateFromArtifactProperties` creates a new entry given the specified artifact properties (typically passed in by `rekor-cli`).
 
 5. In the Go package you have created for the new type, be sure to add an entry in the `TypeMap` in `github.com/sigstore/rekor/pkg/types` for your new type in the `init` method for your package. The key for the map is the unique string used to define your type in `openapi.yaml` (e.g. `newType`), and the value for the map is the name of a factory function for an instance of `TypeImpl`.
 
@@ -104,7 +121,9 @@ func init() {
 
 6. Add an entry to `pluggableTypeMap` in `cmd/rekor-server/app/serve.go` that provides a reference to your package. This ensures that the `init` function of your type (and optionally, your version implementation) will be called before the server starts to process incoming requests and therefore will be added to the map that is used to route request processing for different types.
 
-7. After adding sufficient unit & integration tests, submit a pull request to `github.com/sigstore/rekor` for review and addition to the codebase.
+7. Add an import statement to `cmd/rekor-cli/app/root.go` that provides a reference to your package/new version. This ensures that the `init` function of your type (and optionally, your version implementation) will be called before the CLI runs; this populates the required type maps and allows the CLI to interact with the type implementations in a loosely coupled manner.
+
+8. After adding sufficient unit & integration tests, submit a pull request to `github.com/sigstore/rekor` for review and addition to the codebase.
 
 ## Adding a New Version of the `Rekord` type
 
@@ -120,4 +139,6 @@ To add new version of the default `Rekord` type:
 
 5. Add an entry to `pluggableTypeMap` in `cmd/rekor-server/app/serve.go` that provides a reference to the Go package implementing the new version. This ensures that the `init` function will be called before the server starts to process incoming requests and therefore will be added to the map that is used to route request processing for different types.
 
-6. After adding sufficient unit & integration tests, submit a pull request to `github.com/sigstore/rekor` for review and addition to the codebase.
+6. Add an import statement to `cmd/rekor-cli/app/root.go` that provides a reference to your package/new version. This ensures that the `init` function of your type (and optionally, your version implementation) will be called before the CLI runs; this populates the required type maps and allows the CLI to interact with the type implementations in a loosely coupled manner.
+
+7. After adding sufficient unit & integration tests, submit a pull request to `github.com/sigstore/rekor` for review and addition to the codebase.
diff --git a/pkg/types/alpine/v0.0.1/entry.go b/pkg/types/alpine/v0.0.1/entry.go
index eaaf581..08dfd98 100644
--- a/pkg/types/alpine/v0.0.1/entry.go
+++ b/pkg/types/alpine/v0.0.1/entry.go
@@ -69,8 +69,8 @@ func NewEntry() types.EntryImpl {
 func (v V001Entry) IndexKeys() []string {
 	var result []string
 
-	if v.HasExternalEntities() {
-		if err := v.FetchExternalEntities(context.Background()); err != nil {
+	if v.hasExternalEntities() {
+		if err := v.fetchExternalEntities(context.Background()); err != nil {
 			log.Logger.Error(err)
 			return result
 		}
@@ -108,11 +108,11 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	if err := v.AlpineModel.Validate(strfmt.Default); err != nil {
 		return err
 	}
-	return nil
 
+	return v.validate()
 }
 
-func (v V001Entry) HasExternalEntities() bool {
+func (v V001Entry) hasExternalEntities() bool {
 	if v.fetchedExternalEntities {
 		return false
 	}
@@ -126,12 +126,12 @@ func (v V001Entry) HasExternalEntities() bool {
 	return false
 }
 
-func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
+func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	if v.fetchedExternalEntities {
 		return nil
 	}
 
-	if err := v.Validate(); err != nil {
+	if err := v.validate(); err != nil {
 		return err
 	}
 
@@ -269,7 +269,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 }
 
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
-	if err := v.FetchExternalEntities(ctx); err != nil {
+	if err := v.fetchExternalEntities(ctx); err != nil {
 		return nil, err
 	}
 	if v.keyObj == nil {
@@ -308,8 +308,8 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	return json.Marshal(&apk)
 }
 
-// Validate performs cross-field validation for fields in object
-func (v V001Entry) Validate() error {
+// validate performs cross-field validation for fields in object
+func (v V001Entry) validate() error {
 	key := v.AlpineModel.PublicKey
 	if key == nil {
 		return errors.New("missing public key")
@@ -323,15 +323,13 @@ func (v V001Entry) Validate() error {
 		return errors.New("missing package")
 	}
 
-	if len(pkg.Content) == 0 && pkg.URL.String() == "" {
-		return errors.New("one of 'content' or 'url' must be specified for package")
-	}
-
 	hash := pkg.Hash
 	if hash != nil {
 		if !govalidator.IsHash(swag.StringValue(hash.Value), swag.StringValue(hash.Algorithm)) {
 			return errors.New("invalid value for hash")
 		}
+	} else if len(pkg.Content) == 0 && pkg.URL.String() == "" {
+		return errors.New("one of 'content' or 'url' must be specified for package")
 	}
 
 	return nil
@@ -387,12 +385,12 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 		re.AlpineModel.PublicKey.Content = strfmt.Base64(publicKeyBytes)
 	}
 
-	if err := re.Validate(); err != nil {
+	if err := re.validate(); err != nil {
 		return nil, err
 	}
 
-	if re.HasExternalEntities() {
-		if err := re.FetchExternalEntities(ctx); err != nil {
+	if re.hasExternalEntities() {
+		if err := re.fetchExternalEntities(ctx); err != nil {
 			return nil, fmt.Errorf("error retrieving external entities: %v", err)
 		}
 	}
diff --git a/pkg/types/entries.go b/pkg/types/entries.go
index 7eafef9..dd87973 100644
--- a/pkg/types/entries.go
+++ b/pkg/types/entries.go
@@ -31,11 +31,8 @@ import (
 type EntryImpl interface {
 	APIVersion() string                               // the supported versions for this implementation
 	IndexKeys() []string                              // the keys that should be added to the external index for this entry
-	Canonicalize(ctx context.Context) ([]byte, error) // generate the canonical entry to be put into the tlog
-	FetchExternalEntities(ctx context.Context) error  // gather all external content required to process the entry
-	HasExternalEntities() bool                        // indicates whether there is a need fetch any additional external content required to process the entry
+	Canonicalize(ctx context.Context) ([]byte, error) // marshal the canonical entry to be put into the tlog
 	Unmarshal(e models.ProposedEntry) error           // unmarshal the abstract entry into the specific struct for this versioned type
-	Validate() error                                  // performs any cross-field validation that is not expressed in the OpenAPI spec
 	Attestation() (string, []byte)
 	CreateFromArtifactProperties(context.Context, ArtifactProperties) (models.ProposedEntry, error)
 }
diff --git a/pkg/types/helm/v0.0.1/entry.go b/pkg/types/helm/v0.0.1/entry.go
index fe630ba..30bc7f2 100644
--- a/pkg/types/helm/v0.0.1/entry.go
+++ b/pkg/types/helm/v0.0.1/entry.go
@@ -70,8 +70,8 @@ func NewEntry() types.EntryImpl {
 func (v V001Entry) IndexKeys() []string {
 	var result []string
 
-	if v.HasExternalEntities() {
-		if err := v.FetchExternalEntities(context.Background()); err != nil {
+	if v.hasExternalEntities() {
+		if err := v.fetchExternalEntities(context.Background()); err != nil {
 			log.Logger.Error(err)
 			return result
 		}
@@ -115,13 +115,12 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	if err := v.HelmObj.Validate(strfmt.Default); err != nil {
 		return err
 	}
-	// cross field validation
-	return nil
 
+	// cross field validation
+	return v.validate()
 }
 
-func (v V001Entry) HasExternalEntities() bool {
-
+func (v V001Entry) hasExternalEntities() bool {
 	if v.fetchedExternalEntities {
 		return false
 	}
@@ -136,13 +135,12 @@ func (v V001Entry) HasExternalEntities() bool {
 	return false
 }
 
-func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
-
+func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	if v.fetchedExternalEntities {
 		return nil
 	}
 
-	if err := v.Validate(); err != nil {
+	if err := v.validate(); err != nil {
 		return err
 	}
 
@@ -257,8 +255,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 }
 
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
-
-	if err := v.FetchExternalEntities(ctx); err != nil {
+	if err := v.fetchExternalEntities(ctx); err != nil {
 		return nil, err
 	}
 
@@ -317,8 +314,8 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 
 }
 
-// Validate performs cross-field validation for fields in object
-func (v V001Entry) Validate() error {
+// validate performs cross-field validation for fields in object
+func (v V001Entry) validate() error {
 
 	key := v.HelmObj.PublicKey
 
@@ -342,8 +339,10 @@ func (v V001Entry) Validate() error {
 		return errors.New("missing provenance")
 	}
 
-	if len(provenance.Content) == 0 && provenance.URL.String() == "" {
-		return errors.New("one of 'content' or 'url' must be specified for provenance")
+	if provenance.Signature == nil || provenance.Signature.Content == nil {
+		if len(provenance.Content) == 0 && provenance.URL.String() == "" {
+			return errors.New("one of 'content' or 'url' must be specified for provenance")
+		}
 	}
 
 	return nil
@@ -395,12 +394,12 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 		re.HelmObj.PublicKey.Content = strfmt.Base64(publicKeyBytes)
 	}
 
-	if err := re.Validate(); err != nil {
+	if err := re.validate(); err != nil {
 		return nil, err
 	}
 
-	if re.HasExternalEntities() {
-		if err := re.FetchExternalEntities(ctx); err != nil {
+	if re.hasExternalEntities() {
+		if err := re.fetchExternalEntities(ctx); err != nil {
 			return nil, fmt.Errorf("error retrieving external entities: %v", err)
 		}
 	}
diff --git a/pkg/types/intoto/v0.0.1/entry.go b/pkg/types/intoto/v0.0.1/entry.go
index b1926ef..5fdf052 100644
--- a/pkg/types/intoto/v0.0.1/entry.go
+++ b/pkg/types/intoto/v0.0.1/entry.go
@@ -134,15 +134,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 		return err
 	}
 
-	return v.Validate()
-}
-
-func (v V001Entry) HasExternalEntities() bool {
-	return false
-}
-
-func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
-	return nil
+	return v.validate()
 }
 
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
@@ -179,8 +171,8 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	return bytes, nil
 }
 
-// Validate performs cross-field validation for fields in object
-func (v *V001Entry) Validate() error {
+// validate performs cross-field validation for fields in object
+func (v *V001Entry) validate() error {
 	// TODO handle multiple
 	pk := v.keyObj.(*x509.PublicKey)
 
diff --git a/pkg/types/jar/v0.0.1/entry.go b/pkg/types/jar/v0.0.1/entry.go
index 560ce73..ee0ac69 100644
--- a/pkg/types/jar/v0.0.1/entry.go
+++ b/pkg/types/jar/v0.0.1/entry.go
@@ -74,8 +74,8 @@ func NewEntry() types.EntryImpl {
 func (v V001Entry) IndexKeys() []string {
 	var result []string
 
-	if v.HasExternalEntities() {
-		if err := v.FetchExternalEntities(context.Background()); err != nil {
+	if v.hasExternalEntities() {
+		if err := v.fetchExternalEntities(context.Background()); err != nil {
 			log.Logger.Error(err)
 			return result
 		}
@@ -111,11 +111,11 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	if err := v.JARModel.Validate(strfmt.Default); err != nil {
 		return err
 	}
-	return nil
 
+	return v.validate()
 }
 
-func (v V001Entry) HasExternalEntities() bool {
+func (v V001Entry) hasExternalEntities() bool {
 	if v.fetchedExternalEntities {
 		return false
 	}
@@ -126,12 +126,12 @@ func (v V001Entry) HasExternalEntities() bool {
 	return false
 }
 
-func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
+func (v *V001Entry) fetchExternalEntities(ctx context.Context) error {
 	if v.fetchedExternalEntities {
 		return nil
 	}
 
-	if err := v.Validate(); err != nil {
+	if err := v.validate(); err != nil {
 		return err
 	}
 
@@ -211,7 +211,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 }
 
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
-	if err := v.FetchExternalEntities(ctx); err != nil {
+	if err := v.fetchExternalEntities(ctx); err != nil {
 		return nil, err
 	}
 	if v.jarObj == nil {
@@ -264,15 +264,18 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	return bytes, nil
 }
 
-// Validate performs cross-field validation for fields in object
-func (v V001Entry) Validate() error {
+// validate performs cross-field validation for fields in object
+func (v V001Entry) validate() error {
 	archive := v.JARModel.Archive
 	if archive == nil {
 		return errors.New("missing package")
 	}
 
-	if len(archive.Content) == 0 && archive.URL.String() == "" {
-		return errors.New("one of 'content' or 'url' must be specified for package")
+	// if the signature isn't present, then we need content to extract
+	if v.JARModel.Signature == nil || v.JARModel.Signature.Content == nil {
+		if len(archive.Content) == 0 && archive.URL.String() == "" {
+			return errors.New("one of 'content' or 'url' must be specified for package")
+		}
 	}
 
 	hash := archive.Hash
@@ -355,12 +358,12 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 		re.JARModel.Archive.Content = strfmt.Base64(artifactBytes)
 	}
 
-	if err := re.Validate(); err != nil {
+	if err := re.validate(); err != nil {
 		return nil, err
 	}
 
-	if re.HasExternalEntities() {
-		if err := re.FetchExternalEntities(ctx); err != nil {
+	if re.hasExternalEntities() {
+		if err := re.fetchExternalEntities(ctx); err != nil {
 			return nil, fmt.Errorf("error retrieving external entities: %v", err)
 		}
 	}
diff --git a/pkg/types/rekord/v0.0.1/entry.go b/pkg/types/rekord/v0.0.1/entry.go
index 6635971..de5a241 100644
--- a/pkg/types/rekord/v0.0.1/entry.go
+++ b/pkg/types/rekord/v0.0.1/entry.go
@@ -108,7 +108,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 		return err
 	}
 	// cross field validation
-	return nil
+	return v.validate()
 
 }
 
@@ -134,7 +134,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		return nil
 	}
 
-	if err := v.Validate(); err != nil {
+	if err := v.validate(); err != nil {
 		return err
 	}
 
@@ -347,9 +347,8 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	return bytes, nil
 }
 
-// Validate performs cross-field validation for fields in object
-func (v V001Entry) Validate() error {
-
+// validate performs cross-field validation for fields in object
+func (v V001Entry) validate() error {
 	sig := v.RekordObj.Signature
 	if sig == nil {
 		return errors.New("missing signature")
@@ -371,15 +370,13 @@ func (v V001Entry) Validate() error {
 		return errors.New("missing data")
 	}
 
-	if len(data.Content) == 0 && data.URL.String() == "" {
-		return errors.New("one of 'content' or 'url' must be specified for data")
-	}
-
 	hash := data.Hash
 	if hash != nil {
 		if !govalidator.IsHash(swag.StringValue(hash.Value), swag.StringValue(hash.Algorithm)) {
 			return errors.New("invalid value for hash")
 		}
+	} else if len(data.Content) == 0 && data.URL.String() == "" {
+		return errors.New("one of 'content' or 'url' must be specified for data")
 	}
 
 	return nil
@@ -469,7 +466,7 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 		re.RekordObj.Signature.PublicKey.Content = strfmt.Base64(publicKeyBytes)
 	}
 
-	if err := re.Validate(); err != nil {
+	if err := re.validate(); err != nil {
 		return nil, err
 	}
 
diff --git a/pkg/types/rfc3161/v0.0.1/entry.go b/pkg/types/rfc3161/v0.0.1/entry.go
index b22c9d1..8d9af94 100644
--- a/pkg/types/rfc3161/v0.0.1/entry.go
+++ b/pkg/types/rfc3161/v0.0.1/entry.go
@@ -79,13 +79,6 @@ func NewEntryFromBytes(timestamp []byte) models.ProposedEntry {
 func (v V001Entry) IndexKeys() []string {
 	var result []string
 
-	if v.HasExternalEntities() {
-		if err := v.FetchExternalEntities(context.Background()); err != nil {
-			log.Logger.Error(err)
-			return result
-		}
-	}
-
 	str := v.Rfc3161Obj.Tsr.Content.String()
 	tb, err := base64.StdEncoding.DecodeString(str)
 	if err != nil {
@@ -113,11 +106,10 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 
 	// field validation
 	if err := v.Rfc3161Obj.Validate(strfmt.Default); err != nil {
-
 		return err
 	}
 
-	if err := v.Validate(); err != nil {
+	if err := v.validate(); err != nil {
 		return err
 	}
 
@@ -126,15 +118,10 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	return nil
 }
 
-func (v V001Entry) HasExternalEntities() bool {
-	return false
-}
-
-func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
-	return nil
-}
-
 func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
+	if v.tsrContent == nil {
+		return nil, errors.New("tsr content must be set before canonicalizing")
+	}
 	canonicalEntry := models.Rfc3161V001Schema{
 		Tsr: &models.Rfc3161V001SchemaTsr{
 			Content: v.tsrContent,
@@ -152,8 +139,8 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	return json.Marshal(&ref3161Obj)
 }
 
-// Validate performs cross-field validation for fields in object
-func (v V001Entry) Validate() error {
+// validate performs cross-field validation for fields in object
+func (v V001Entry) validate() error {
 	data := v.Rfc3161Obj.Tsr
 	if data == nil {
 		return errors.New("missing tsr data")
diff --git a/pkg/types/rpm/v0.0.1/entry.go b/pkg/types/rpm/v0.0.1/entry.go
index ac6d4bd..8677fe5 100644
--- a/pkg/types/rpm/v0.0.1/entry.go
+++ b/pkg/types/rpm/v0.0.1/entry.go
@@ -110,8 +110,8 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 	if err := v.RPMModel.Validate(strfmt.Default); err != nil {
 		return err
 	}
-	return nil
 
+	return v.validate()
 }
 
 func (v V001Entry) HasExternalEntities() bool {
@@ -133,7 +133,7 @@ func (v *V001Entry) FetchExternalEntities(ctx context.Context) error {
 		return nil
 	}
 
-	if err := v.Validate(); err != nil {
+	if err := v.validate(); err != nil {
 		return err
 	}
 
@@ -330,8 +330,8 @@ func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
 	return json.Marshal(&rpm)
 }
 
-// Validate performs cross-field validation for fields in object
-func (v V001Entry) Validate() error {
+// validate performs cross-field validation for fields in object
+func (v V001Entry) validate() error {
 	key := v.RPMModel.PublicKey
 	if key == nil {
 		return errors.New("missing public key")
@@ -345,15 +345,13 @@ func (v V001Entry) Validate() error {
 		return errors.New("missing package")
 	}
 
-	if len(pkg.Content) == 0 && pkg.URL.String() == "" {
-		return errors.New("one of 'content' or 'url' must be specified for package")
-	}
-
 	hash := pkg.Hash
 	if hash != nil {
 		if !govalidator.IsHash(swag.StringValue(hash.Value), swag.StringValue(hash.Algorithm)) {
 			return errors.New("invalid value for hash")
 		}
+	} else if len(pkg.Content) == 0 && pkg.URL.String() == "" {
+		return errors.New("one of 'content' or 'url' must be specified for package")
 	}
 
 	return nil
@@ -409,7 +407,7 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 		re.RPMModel.PublicKey.Content = strfmt.Base64(publicKeyBytes)
 	}
 
-	if err := re.Validate(); err != nil {
+	if err := re.validate(); err != nil {
 		return nil, err
 	}
 


Test patch:
diff --git a/pkg/types/alpine/v0.0.1/entry_test.go b/pkg/types/alpine/v0.0.1/entry_test.go
index d7998a1..ebb4f76 100644
--- a/pkg/types/alpine/v0.0.1/entry_test.go
+++ b/pkg/types/alpine/v0.0.1/entry_test.go
@@ -16,6 +16,7 @@
 package alpine
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -26,11 +27,13 @@ import (
 	"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"
+	"github.com/sigstore/rekor/pkg/types"
 )
 
 func TestMain(m *testing.M) {
@@ -363,7 +366,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)
 		}
 
@@ -377,18 +380,28 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err := v.Unmarshal(&r); err != nil {
 				return err
 			}
-			return v.Validate()
+			return v.validate()
 		}
 		if err := unmarshalAndValidate(); (err == nil) != tc.expectUnmarshalSuccess {
 			t.Errorf("unexpected result in '%v': %v", tc.caseDesc, err)
 		}
 
-		if tc.entry.HasExternalEntities() != tc.hasExtEntities {
+		if tc.entry.hasExternalEntities() != tc.hasExtEntities {
 			t.Errorf("unexpected result from HasExternalEntities for '%v'", tc.caseDesc)
 		}
 
-		if _, err := tc.entry.Canonicalize(context.TODO()); (err == nil) != tc.expectCanonicalizeSuccess {
+		b, err := v.Canonicalize(context.TODO())
+		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		}
+		if b != nil {
+			pe, err := models.UnmarshalProposedEntry(bytes.NewReader(b), runtime.JSONConsumer())
+			if err != nil {
+				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
+			}
+			if _, err := types.NewEntry(pe); err != nil {
+				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
+			}
+		}
 	}
 }
diff --git a/pkg/types/helm/v0.0.1/entry_test.go b/pkg/types/helm/v0.0.1/entry_test.go
index 6fce3c2..02eebe3 100644
--- a/pkg/types/helm/v0.0.1/entry_test.go
+++ b/pkg/types/helm/v0.0.1/entry_test.go
@@ -16,6 +16,7 @@
 package helm
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"io/ioutil"
@@ -24,11 +25,13 @@ import (
 	"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"
+	"github.com/sigstore/rekor/pkg/types"
 )
 
 func TestMain(m *testing.M) {
@@ -215,7 +218,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)
 		}
 
@@ -229,7 +232,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
@@ -239,12 +242,22 @@ func TestCrossFieldValidation(t *testing.T) {
 			t.Errorf("unexpected result in '%v': %v", tc.caseDesc, err)
 		}
 
-		if tc.entry.HasExternalEntities() != tc.hasExtEntities {
+		if tc.entry.hasExternalEntities() != tc.hasExtEntities {
 			t.Errorf("unexpected result from HasExternalEntities for '%v'", tc.caseDesc)
 		}
 
-		if _, err := tc.entry.Canonicalize(context.TODO()); (err == nil) != tc.expectCanonicalizeSuccess {
+		b, err := v.Canonicalize(context.TODO())
+		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		}
+		if b != nil {
+			pe, err := models.UnmarshalProposedEntry(bytes.NewReader(b), runtime.JSONConsumer())
+			if err != nil {
+				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
+			}
+			if _, err := types.NewEntry(pe); err != nil {
+				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
+			}
+		}
 	}
 }
diff --git a/pkg/types/intoto/v0.0.1/entry_test.go b/pkg/types/intoto/v0.0.1/entry_test.go
index 6bb2931..1744d9e 100644
--- a/pkg/types/intoto/v0.0.1/entry_test.go
+++ b/pkg/types/intoto/v0.0.1/entry_test.go
@@ -170,7 +170,7 @@ func TestV001Entry_Unmarshal(t *testing.T) {
 				if err := v.Unmarshal(it); err != nil {
 					return err
 				}
-				if err := v.Validate(); err != nil {
+				if err := v.validate(); err != nil {
 					return err
 				}
 				keys := v.IndexKeys()
diff --git a/pkg/types/jar/v0.0.1/entry_test.go b/pkg/types/jar/v0.0.1/entry_test.go
index ca9dac7..44240b5 100644
--- a/pkg/types/jar/v0.0.1/entry_test.go
+++ b/pkg/types/jar/v0.0.1/entry_test.go
@@ -16,6 +16,7 @@
 package jar
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -26,9 +27,11 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/swag"
 	"github.com/sigstore/rekor/pkg/generated/models"
+	"github.com/sigstore/rekor/pkg/types"
 	"go.uber.org/goleak"
 )
 
@@ -196,10 +199,6 @@ func TestCrossFieldValidation(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		if err := tc.entry.Validate(); (err == nil) != tc.expectUnmarshalSuccess {
-			t.Errorf("unexpected result in '%v': %v", tc.caseDesc, err)
-		}
-
 		v := &V001Entry{}
 		r := models.Jar{
 			APIVersion: swag.String(tc.entry.APIVersion()),
@@ -210,18 +209,28 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err := v.Unmarshal(&r); err != nil {
 				return err
 			}
-			return v.Validate()
+			return v.validate()
 		}
 		if err := unmarshalAndValidate(); (err == nil) != tc.expectUnmarshalSuccess {
 			t.Errorf("unexpected result in '%v': %v", tc.caseDesc, err)
 		}
 
-		if tc.entry.HasExternalEntities() != tc.hasExtEntities {
+		if tc.entry.hasExternalEntities() != tc.hasExtEntities {
 			t.Errorf("unexpected result from HasExternalEntities for '%v'", tc.caseDesc)
 		}
 
-		if _, err := tc.entry.Canonicalize(context.TODO()); (err == nil) != tc.expectCanonicalizeSuccess {
+		b, err := v.Canonicalize(context.TODO())
+		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		}
+		if b != nil {
+			pe, err := models.UnmarshalProposedEntry(bytes.NewReader(b), runtime.JSONConsumer())
+			if err != nil {
+				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
+			}
+			if _, err := types.NewEntry(pe); err != nil {
+				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
+			}
+		}
 	}
 }
diff --git a/pkg/types/rekord/v0.0.1/entry_test.go b/pkg/types/rekord/v0.0.1/entry_test.go
index 782a11d..2dc847a 100644
--- a/pkg/types/rekord/v0.0.1/entry_test.go
+++ b/pkg/types/rekord/v0.0.1/entry_test.go
@@ -16,6 +16,7 @@
 package rekord
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -26,11 +27,13 @@ import (
 	"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"
+	"github.com/sigstore/rekor/pkg/types"
 )
 
 func TestMain(m *testing.M) {
@@ -519,7 +522,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)
 		}
 
@@ -533,7 +536,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
@@ -547,8 +550,18 @@ func TestCrossFieldValidation(t *testing.T) {
 			t.Errorf("unexpected result from HasExternalEntities for '%v'", tc.caseDesc)
 		}
 
-		if _, err := tc.entry.Canonicalize(context.TODO()); (err == nil) != tc.expectCanonicalizeSuccess {
+		b, err := v.Canonicalize(context.TODO())
+		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		}
+		if b != nil {
+			pe, err := models.UnmarshalProposedEntry(bytes.NewReader(b), runtime.JSONConsumer())
+			if err != nil {
+				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
+			}
+			if _, err := types.NewEntry(pe); err != nil {
+				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
+			}
+		}
 	}
 }
diff --git a/pkg/types/rfc3161/v0.0.1/entry_test.go b/pkg/types/rfc3161/v0.0.1/entry_test.go
index e2d2711..972cbcb 100644
--- a/pkg/types/rfc3161/v0.0.1/entry_test.go
+++ b/pkg/types/rfc3161/v0.0.1/entry_test.go
@@ -16,6 +16,7 @@
 package rfc3161
 
 import (
+	"bytes"
 	"context"
 	"encoding/asn1"
 	"errors"
@@ -28,11 +29,13 @@ import (
 
 	"github.com/sassoftware/relic/lib/pkcs9"
 
+	"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"
+	"github.com/sigstore/rekor/pkg/types"
 )
 
 func TestMain(m *testing.M) {
@@ -50,7 +53,6 @@ func TestCrossFieldValidation(t *testing.T) {
 	type TestCase struct {
 		caseDesc                     string
 		entry                        V001Entry
-		hasExtEntities               bool
 		expectUnmarshalSuccess       bool
 		expectCanonicalizeSuccess    bool
 		expectValidationErrorMessage string
@@ -82,9 +84,8 @@ func TestCrossFieldValidation(t *testing.T) {
 		{
 			caseDesc:                     "empty obj",
 			entry:                        V001Entry{},
-			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
-			expectCanonicalizeSuccess:    true,
+			expectCanonicalizeSuccess:    false,
 			expectValidationErrorMessage: "validation failure",
 		},
 		{
@@ -96,7 +97,6 @@ func TestCrossFieldValidation(t *testing.T) {
 					},
 				},
 			},
-			hasExtEntities:            false,
 			expectUnmarshalSuccess:    true,
 			expectCanonicalizeSuccess: true,
 		},
@@ -109,9 +109,8 @@ func TestCrossFieldValidation(t *testing.T) {
 					},
 				},
 			},
-			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
-			expectCanonicalizeSuccess:    true,
+			expectCanonicalizeSuccess:    false,
 			expectValidationErrorMessage: "tsr exceeds maximum allowed size (10kB)",
 		},
 		{
@@ -123,9 +122,8 @@ func TestCrossFieldValidation(t *testing.T) {
 					},
 				},
 			},
-			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
-			expectCanonicalizeSuccess:    true,
+			expectCanonicalizeSuccess:    false,
 			expectValidationErrorMessage: "tsr status not granted: 2",
 		},
 		{
@@ -137,9 +135,8 @@ func TestCrossFieldValidation(t *testing.T) {
 					},
 				},
 			},
-			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
-			expectCanonicalizeSuccess:    true,
+			expectCanonicalizeSuccess:    false,
 			expectValidationErrorMessage: "tsr wrong content type: 0.0.0.0.42",
 		},
 		{
@@ -151,9 +148,8 @@ func TestCrossFieldValidation(t *testing.T) {
 					},
 				},
 			},
-			hasExtEntities:               false,
 			expectUnmarshalSuccess:       false,
-			expectCanonicalizeSuccess:    true,
+			expectCanonicalizeSuccess:    false,
 			expectValidationErrorMessage: "tsr verification error",
 		},
 		{
@@ -166,7 +162,6 @@ func TestCrossFieldValidation(t *testing.T) {
 					ExtraData: []byte("{\"something\": \"here\""),
 				},
 			},
-			hasExtEntities:            false,
 			expectUnmarshalSuccess:    true,
 			expectCanonicalizeSuccess: true,
 		},
@@ -187,13 +182,19 @@ func TestCrossFieldValidation(t *testing.T) {
 			}
 		}
 
-		if tc.entry.HasExternalEntities() != tc.hasExtEntities {
-			t.Errorf("unexpected result from HasExternalEntities for '%v'", tc.caseDesc)
-		}
-
-		if _, err := tc.entry.Canonicalize(context.TODO()); (err == nil) != tc.expectCanonicalizeSuccess {
+		b, err := v.Canonicalize(context.TODO())
+		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		}
+		if b != nil {
+			pe, err := models.UnmarshalProposedEntry(bytes.NewReader(b), runtime.JSONConsumer())
+			if err != nil {
+				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
+			}
+			if _, err := types.NewEntry(pe); err != nil {
+				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
+			}
+		}
 	}
 }
 
diff --git a/pkg/types/rpm/v0.0.1/entry_test.go b/pkg/types/rpm/v0.0.1/entry_test.go
index 6b0cfdf..6df90a0 100644
--- a/pkg/types/rpm/v0.0.1/entry_test.go
+++ b/pkg/types/rpm/v0.0.1/entry_test.go
@@ -16,6 +16,7 @@
 package rpm
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -26,11 +27,13 @@ import (
 	"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"
+	"github.com/sigstore/rekor/pkg/types"
 )
 
 func TestMain(m *testing.M) {
@@ -363,7 +366,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)
 		}
 
@@ -377,7 +380,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err := v.Unmarshal(&r); err != nil {
 				return err
 			}
-			return v.Validate()
+			return v.validate()
 		}
 		if err := unmarshalAndValidate(); (err == nil) != tc.expectUnmarshalSuccess {
 			t.Errorf("unexpected result in '%v': %v", tc.caseDesc, err)
@@ -387,8 +390,18 @@ func TestCrossFieldValidation(t *testing.T) {
 			t.Errorf("unexpected result from HasExternalEntities for '%v'", tc.caseDesc)
 		}
 
-		if _, err := tc.entry.Canonicalize(context.TODO()); (err == nil) != tc.expectCanonicalizeSuccess {
+		b, err := v.Canonicalize(context.TODO())
+		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		}
+		if b != nil {
+			pe, err := models.UnmarshalProposedEntry(bytes.NewReader(b), runtime.JSONConsumer())
+			if err != nil {
+				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
+			}
+			if _, err := types.NewEntry(pe); err != nil {
+				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
+			}
+		}
 	}
 }