From 12d1a47c7ac986932a2734cb855c642ac01ffde4 Mon Sep 17 00:00:00 2001 From: dlorenc <dlorenc@google.com> Date: Mon, 27 Dec 2021 07:20:32 -0600 Subject: [PATCH] Remove the attestation media type field. (#565) This was never actually correct - these are technically "payloadTypes", which are not actually mediaTypes. Some implementations mistakenly sent incorrect media types, so it appeared to work. The GCS storage layer rejected correct implementations that sent the payloadType, because these are not valid mediaTypes. We never used this field anyway, so let's drop it. I verified that the API correctly ignores unknown fields, so removing this will not break clients that send it. Signed-off-by: Dan Lorenc <lorenc.d@gmail.com> --- cmd/rekor-cli/app/get.go | 1 - openapi.yaml | 4 +--- pkg/api/entries.go | 11 +++++------ pkg/api/index.go | 4 ++-- pkg/generated/models/log_entry.go | 3 --- pkg/generated/restapi/embedded_spec.go | 9 --------- pkg/storage/storage.go | 26 ++++++++++---------------- pkg/types/alpine/v0.0.1/entry.go | 4 ++-- pkg/types/entries.go | 2 +- pkg/types/hashedrekord/v0.0.1/entry.go | 4 ++-- pkg/types/helm/v0.0.1/entry.go | 4 ++-- pkg/types/intoto/v0.0.1/entry.go | 6 +++--- pkg/types/jar/v0.0.1/entry.go | 4 ++-- pkg/types/rekord/v0.0.1/entry.go | 4 ++-- pkg/types/rfc3161/v0.0.1/entry.go | 4 ++-- pkg/types/rpm/v0.0.1/entry.go | 4 ++-- pkg/types/test_util.go | 4 ++-- pkg/types/tuf/v0.0.1/entry.go | 4 ++-- 18 files changed, 40 insertions(+), 62 deletions(-) diff --git a/cmd/rekor-cli/app/get.go b/cmd/rekor-cli/app/get.go index 6c85222..fe66a27 100644 --- a/cmd/rekor-cli/app/get.go +++ b/cmd/rekor-cli/app/get.go @@ -159,7 +159,6 @@ func parseEntry(uuid string, e models.LogEntryAnon) (interface{}, error) { if e.Attestation != nil { obj.Attestation = string(e.Attestation.Data) - obj.AttestationType = e.Attestation.MediaType } return &obj, nil diff --git a/openapi.yaml b/openapi.yaml index 63c64b2..2c0987c 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -468,9 +468,7 @@ definitions: type: object properties: data: - format: byte - mediaType: - format: string + format: byte format: byte verification: diff --git a/pkg/api/entries.go b/pkg/api/entries.go index fd6d2bb..58701e2 100644 --- a/pkg/api/entries.go +++ b/pkg/api/entries.go @@ -94,13 +94,12 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC uuid := hex.EncodeToString(leaf.MerkleLeafHash) if viper.GetBool("enable_attestation_storage") { - att, typ, err := storageClient.FetchAttestation(ctx, uuid) + att, err := storageClient.FetchAttestation(ctx, uuid) if err != nil { log.Logger.Errorf("error fetching attestation: %s %s", uuid, err) } else { logEntryAnon.Attestation = &models.LogEntryAnonAttestation{ - Data: att, - MediaType: typ, + Data: att, } } } @@ -210,12 +209,12 @@ func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middl if viper.GetBool("enable_attestation_storage") { go func() { - typ, attestation := entry.Attestation() - if typ == "" { + attestation := entry.Attestation() + if attestation == nil { log.RequestIDLogger(params.HTTPRequest).Infof("no attestation for %s", uuid) return } - if err := storeAttestation(context.Background(), uuid, typ, attestation); err != nil { + if err := storeAttestation(context.Background(), uuid, attestation); err != nil { log.RequestIDLogger(params.HTTPRequest).Errorf("error storing attestation: %s", err) } }() diff --git a/pkg/api/index.go b/pkg/api/index.go index 8aa088a..27a9f5c 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -96,6 +96,6 @@ func addToIndex(ctx context.Context, key, value string) error { return redisClient.Do(ctx, radix.Cmd(nil, "LPUSH", key, value)) } -func storeAttestation(ctx context.Context, uuid, attestationType string, attestation []byte) error { - return storageClient.StoreAttestation(ctx, uuid, attestationType, attestation) +func storeAttestation(ctx context.Context, uuid string, attestation []byte) error { + return storageClient.StoreAttestation(ctx, uuid, attestation) } diff --git a/pkg/generated/models/log_entry.go b/pkg/generated/models/log_entry.go index c1ab077..6aa7f4e 100644 --- a/pkg/generated/models/log_entry.go +++ b/pkg/generated/models/log_entry.go @@ -300,9 +300,6 @@ type LogEntryAnonAttestation struct { // data // Format: byte Data strfmt.Base64 `json:"data,omitempty"` - - // media type - MediaType string `json:"mediaType,omitempty"` } // Validate validates this log entry anon attestation diff --git a/pkg/generated/restapi/embedded_spec.go b/pkg/generated/restapi/embedded_spec.go index 7441c79..0dde72c 100644 --- a/pkg/generated/restapi/embedded_spec.go +++ b/pkg/generated/restapi/embedded_spec.go @@ -498,9 +498,6 @@ func init() { "properties": { "data": { "format": "byte" - }, - "mediaType": { - "format": "string" } } }, @@ -1982,9 +1979,6 @@ func init() { "properties": { "data": { "format": "byte" - }, - "mediaType": { - "format": "string" } } }, @@ -2025,9 +2019,6 @@ func init() { "properties": { "data": { "format": "byte" - }, - "mediaType": { - "format": "string" } } }, diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index bf2c76d..2e182e4 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -31,8 +31,8 @@ import ( ) type AttestationStorage interface { - StoreAttestation(ctx context.Context, key string, attestationType string, attestation []byte) error - FetchAttestation(ctx context.Context, key string) ([]byte, string, error) + StoreAttestation(ctx context.Context, key string, attestation []byte) error + FetchAttestation(ctx context.Context, key string) ([]byte, error) } func NewAttestationStorage() (AttestationStorage, error) { @@ -53,11 +53,9 @@ type Blob struct { bucket *blob.Bucket } -func (b *Blob) StoreAttestation(ctx context.Context, key, attestationType string, attestation []byte) error { - log.Logger.Infof("storing attestation of type %s at %s", attestationType, key) - w, err := b.bucket.NewWriter(ctx, key, &blob.WriterOptions{ - ContentType: attestationType, - }) +func (b *Blob) StoreAttestation(ctx context.Context, key string, attestation []byte) error { + log.Logger.Infof("storing attestation at %s", key) + w, err := b.bucket.NewWriter(ctx, key, nil) if err != nil { return err } @@ -67,23 +65,19 @@ func (b *Blob) StoreAttestation(ctx context.Context, key, attestationType string return w.Close() } -func (b *Blob) FetchAttestation(ctx context.Context, key string) ([]byte, string, error) { +func (b *Blob) FetchAttestation(ctx context.Context, key string) ([]byte, error) { log.Logger.Infof("fetching attestation %s", key) exists, err := b.bucket.Exists(ctx, key) if err != nil { - return nil, "", err + return nil, err } if !exists { - return nil, "", nil - } - att, err := b.bucket.Attributes(ctx, key) - if err != nil { - return nil, "", err + return nil, nil } data, err := b.bucket.ReadAll(ctx, key) if err != nil { - return nil, "", err + return nil, err } - return data, att.ContentType, nil + return data, nil } diff --git a/pkg/types/alpine/v0.0.1/entry.go b/pkg/types/alpine/v0.0.1/entry.go index a148f63..3116852 100644 --- a/pkg/types/alpine/v0.0.1/entry.go +++ b/pkg/types/alpine/v0.0.1/entry.go @@ -300,8 +300,8 @@ func (v V001Entry) validate() error { return nil } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/entries.go b/pkg/types/entries.go index 0d5c3eb..dda638b 100644 --- a/pkg/types/entries.go +++ b/pkg/types/entries.go @@ -35,7 +35,7 @@ type EntryImpl interface { IndexKeys() ([]string, error) // the keys that should be added to the external index for this 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 - Attestation() (string, []byte) + Attestation() []byte CreateFromArtifactProperties(context.Context, ArtifactProperties) (models.ProposedEntry, error) } diff --git a/pkg/types/hashedrekord/v0.0.1/entry.go b/pkg/types/hashedrekord/v0.0.1/entry.go index 9bb3702..1cb646c 100644 --- a/pkg/types/hashedrekord/v0.0.1/entry.go +++ b/pkg/types/hashedrekord/v0.0.1/entry.go @@ -182,8 +182,8 @@ func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) { return sigObj, keyObj, nil } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.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 2665f81..545d3cc 100644 --- a/pkg/types/helm/v0.0.1/entry.go +++ b/pkg/types/helm/v0.0.1/entry.go @@ -299,8 +299,8 @@ func (v V001Entry) validate() error { return nil } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/intoto/v0.0.1/entry.go b/pkg/types/intoto/v0.0.1/entry.go index 1c2fc28..66cc065 100644 --- a/pkg/types/intoto/v0.0.1/entry.go +++ b/pkg/types/intoto/v0.0.1/entry.go @@ -193,12 +193,12 @@ func (v *V001Entry) validate() error { return nil } -func (v *V001Entry) Attestation() (string, []byte) { +func (v *V001Entry) Attestation() []byte { if len(v.env.Payload) > viper.GetInt("max_attestation_size") { log.Logger.Infof("Skipping attestation storage, size %d is greater than max %d", len(v.env.Payload), viper.GetInt("max_attestation_size")) - return "", nil + return nil } - return v.env.PayloadType, []byte(v.env.Payload) + return []byte(v.env.Payload) } type verifier struct { diff --git a/pkg/types/jar/v0.0.1/entry.go b/pkg/types/jar/v0.0.1/entry.go index 8a75749..6bbbe00 100644 --- a/pkg/types/jar/v0.0.1/entry.go +++ b/pkg/types/jar/v0.0.1/entry.go @@ -290,8 +290,8 @@ func extractPKCS7SignatureFromJAR(inz *zip.Reader) ([]byte, error) { return nil, errors.New("unable to locate signature in JAR file") } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/rekord/v0.0.1/entry.go b/pkg/types/rekord/v0.0.1/entry.go index c7b4b8b..d0f6d4c 100644 --- a/pkg/types/rekord/v0.0.1/entry.go +++ b/pkg/types/rekord/v0.0.1/entry.go @@ -353,8 +353,8 @@ func (v V001Entry) validate() error { return nil } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/rfc3161/v0.0.1/entry.go b/pkg/types/rfc3161/v0.0.1/entry.go index f67e1e5..67450a3 100644 --- a/pkg/types/rfc3161/v0.0.1/entry.go +++ b/pkg/types/rfc3161/v0.0.1/entry.go @@ -173,8 +173,8 @@ func (v V001Entry) validate() error { return nil } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(_ context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/rpm/v0.0.1/entry.go b/pkg/types/rpm/v0.0.1/entry.go index 219b4a3..72e6695 100644 --- a/pkg/types/rpm/v0.0.1/entry.go +++ b/pkg/types/rpm/v0.0.1/entry.go @@ -321,8 +321,8 @@ func (v V001Entry) validate() error { return nil } -func (v V001Entry) Attestation() (string, []byte) { - return "", nil +func (v V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/test_util.go b/pkg/types/test_util.go index bd663e0..5b91104 100644 --- a/pkg/types/test_util.go +++ b/pkg/types/test_util.go @@ -48,8 +48,8 @@ func (u BaseUnmarshalTester) Validate() error { return nil } -func (u BaseUnmarshalTester) Attestation() (string, []byte) { - return "", nil +func (u BaseUnmarshalTester) Attestation() []byte { + return nil } func (u BaseUnmarshalTester) CreateFromArtifactProperties(_ context.Context, _ ArtifactProperties) (models.ProposedEntry, error) { diff --git a/pkg/types/tuf/v0.0.1/entry.go b/pkg/types/tuf/v0.0.1/entry.go index 4020493..9820463 100644 --- a/pkg/types/tuf/v0.0.1/entry.go +++ b/pkg/types/tuf/v0.0.1/entry.go @@ -313,8 +313,8 @@ func (v V001Entry) Validate() error { return nil } -func (v *V001Entry) Attestation() (string, []byte) { - return "", nil +func (v *V001Entry) Attestation() []byte { + return nil } func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types.ArtifactProperties) (models.ProposedEntry, error) { -- GitLab