Skip to content

Commit

Permalink
fix validation error handling on API (#2217)
Browse files Browse the repository at this point in the history
Signed-off-by: Bob Callaway <[email protected]>
  • Loading branch information
bobcallaway authored Aug 26, 2024
1 parent a676186 commit c16f610
Show file tree
Hide file tree
Showing 18 changed files with 83 additions and 56 deletions.
10 changes: 6 additions & 4 deletions pkg/api/entries.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middl
}
leaf, err := types.CanonicalizeEntry(ctx, entry)
if err != nil {
if _, ok := (err).(types.ValidationError); ok {
var validationErr *types.InputValidationError
if errors.As(err, &validationErr) {
return nil, handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf(validationError, err))
}
return nil, handleRekorAPIError(params, http.StatusInternalServerError, err, failedToGenerateCanonicalEntry)
Expand Down Expand Up @@ -415,8 +416,9 @@ func GetLogEntryByUUIDHandler(params entries.GetLogEntryByUUIDParams) middleware
if errors.Is(err, ErrNotFound) {
return handleRekorAPIError(params, http.StatusNotFound, err, "")
}
if _, ok := (err).(types.ValidationError); ok {
return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("incorrectly formatted uuid %s", params.EntryUUID), params.EntryUUID)
var validationErr *types.InputValidationError
if errors.As(err, &validationErr) {
return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("validation error: %v", err), params.EntryUUID)
}
return handleRekorAPIError(params, http.StatusInternalServerError, err, trillianCommunicationError)
}
Expand Down Expand Up @@ -602,7 +604,7 @@ func retrieveUUIDFromTree(ctx context.Context, uuid string, tid int64) (models.L

hashValue, err := hex.DecodeString(uuid)
if err != nil {
return models.LogEntry{}, types.ValidationError(err)
return models.LogEntry{}, &types.InputValidationError{Err: fmt.Errorf("parsing UUID: %w", err)}
}

tc := trillianclient.NewTrillianClient(ctx, api.logClient, tid)
Expand Down
10 changes: 5 additions & 5 deletions pkg/types/alpine/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {

func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey, *alpine.Package, error) {
if err := v.validate(); err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

g, ctx := errgroup.WithContext(ctx)
Expand Down Expand Up @@ -150,7 +150,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,

computedSHA := hex.EncodeToString(hasher.Sum(nil))
if oldSHA != "" && computedSHA != oldSHA {
return closePipesOnError(types.ValidationError(fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)))
return closePipesOnError(&types.InputValidationError{Err: fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)})
}

select {
Expand All @@ -169,7 +169,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,

keyObj, err := x509.NewPublicKey(keyReadCloser)
if err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

select {
Expand All @@ -186,7 +186,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,
g.Go(func() error {
apk := alpine.Package{}
if err := apk.Unmarshal(apkR); err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

key = <-keyResult
Expand All @@ -195,7 +195,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,
}

if err := apk.VerifySignature(key.CryptoPubKey()); err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

apkObj = &apk
Expand Down
4 changes: 3 additions & 1 deletion pkg/types/alpine/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"crypto/sha256"
"encoding/hex"
"errors"
"os"
"reflect"
"testing"
Expand Down Expand Up @@ -163,7 +164,8 @@ func TestCrossFieldValidation(t *testing.T) {
if (err == nil) != tc.expectCanonicalizeSuccess {
t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
} else if err != nil {
if _, ok := err.(types.ValidationError); !ok {
var validationErr *types.InputValidationError
if !errors.As(err, &validationErr) {
t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
}
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/types/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,15 @@ package types

// ValidationError indicates that there is an issue with the content in the HTTP Request that
// should result in an HTTP 400 Bad Request error being returned to the client
//
// Deprecated: use InputValidationError instead to take advantage of Go's error wrapping
type ValidationError error

// InputValidationError indicates that there is an issue with the content in the HTTP Request that
// should result in an HTTP 400 Bad Request error being returned to the client
type InputValidationError struct {
Err error
}

func (v *InputValidationError) Error() string { return v.Err.Error() }
func (v *InputValidationError) Unwrap() error { return v.Err }
18 changes: 9 additions & 9 deletions pkg/types/hashedrekord/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
func (v *V001Entry) Canonicalize(_ context.Context) ([]byte, error) {
sigObj, keyObj, err := v.validate()
if err != nil {
return nil, types.ValidationError(err)
return nil, &types.InputValidationError{Err: err}
}

canonicalEntry := models.HashedrekordV001Schema{}
Expand Down Expand Up @@ -144,34 +144,34 @@ func (v *V001Entry) Canonicalize(_ context.Context) ([]byte, error) {
func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
sig := v.HashedRekordObj.Signature
if sig == nil {
return nil, nil, types.ValidationError(errors.New("missing signature"))
return nil, nil, &types.InputValidationError{Err: errors.New("missing signature")}
}
// Hashed rekord type only works for x509 signature types
sigObj, err := x509.NewSignatureWithOpts(bytes.NewReader(sig.Content), options.WithED25519ph())
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

key := sig.PublicKey
if key == nil {
return nil, nil, types.ValidationError(errors.New("missing public key"))
return nil, nil, &types.InputValidationError{Err: errors.New("missing public key")}
}
keyObj, err := x509.NewPublicKey(bytes.NewReader(key.Content))
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

data := v.HashedRekordObj.Data
if data == nil {
return nil, nil, types.ValidationError(errors.New("missing data"))
return nil, nil, &types.InputValidationError{Err: errors.New("missing data")}
}

hash := data.Hash
if hash == nil {
return nil, nil, types.ValidationError(errors.New("missing hash"))
return nil, nil, &types.InputValidationError{Err: errors.New("missing hash")}
}
if !govalidator.IsHash(swag.StringValue(hash.Value), swag.StringValue(hash.Algorithm)) {
return nil, nil, types.ValidationError(errors.New("invalid value for hash"))
return nil, nil, &types.InputValidationError{Err: errors.New("invalid value for hash")}
}

var alg crypto.Hash
Expand All @@ -189,7 +189,7 @@ func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
return nil, nil, err
}
if err := sigObj.Verify(nil, keyObj, options.WithDigest(decoded), options.WithCryptoSignerOpts(alg)); err != nil {
return nil, nil, types.ValidationError(fmt.Errorf("verifying signature: %w", err))
return nil, nil, &types.InputValidationError{Err: fmt.Errorf("verifying signature: %w", err)}
}

return sigObj, keyObj, nil
Expand Down
4 changes: 3 additions & 1 deletion pkg/types/hashedrekord/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"crypto/x509/pkix"
"encoding/hex"
"encoding/pem"
"errors"
"math/big"
"reflect"
"testing"
Expand Down Expand Up @@ -451,7 +452,8 @@ func TestCrossFieldValidation(t *testing.T) {
if (err == nil) != tc.expectCanonicalizeSuccess {
t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
} else if err != nil {
if _, ok := err.(types.ValidationError); !ok {
var validationErr *types.InputValidationError
if !errors.As(err, &validationErr) {
t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/types/helm/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {

func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance, *pgp.PublicKey, *pgp.Signature, error) {
if err := v.validate(); err != nil {
return nil, nil, nil, types.ValidationError(err)
return nil, nil, nil, &types.InputValidationError{Err: err}
}

g, ctx := errgroup.WithContext(ctx)
Expand Down Expand Up @@ -148,7 +148,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance

keyObj, err := pgp.NewPublicKey(keyReadCloser)
if err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

select {
Expand All @@ -165,7 +165,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance
g.Go(func() error {

if err := provenance.Unmarshal(provenanceR); err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

key = <-keyResult
Expand All @@ -177,12 +177,12 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance
var err error
sig, err = pgp.NewSignature(provenance.Block.ArmoredSignature.Body)
if err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

// Verify signature
if err := sig.Verify(bytes.NewReader(provenance.Block.Bytes), key); err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

select {
Expand Down
4 changes: 3 additions & 1 deletion pkg/types/helm/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package helm
import (
"bytes"
"context"
"errors"
"os"
"reflect"
"testing"
Expand Down Expand Up @@ -193,7 +194,8 @@ func TestCrossFieldValidation(t *testing.T) {
if (err == nil) != tc.expectCanonicalizeSuccess {
t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
} else if err != nil {
if _, ok := err.(types.ValidationError); !ok {
var validationErr *types.InputValidationError
if !errors.As(err, &validationErr) {
t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
}
}
Expand Down
21 changes: 10 additions & 11 deletions pkg/types/jar/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {

func (v *V001Entry) fetchExternalEntities(_ context.Context) (*pkcs7.PublicKey, *pkcs7.Signature, error) {
if err := v.validate(); err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

oldSHA := ""
Expand All @@ -132,12 +132,12 @@ func (v *V001Entry) fetchExternalEntities(_ context.Context) (*pkcs7.PublicKey,

computedSHA := hex.EncodeToString(hasher.Sum(nil))
if oldSHA != "" && computedSHA != oldSHA {
return nil, nil, types.ValidationError(fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA))
return nil, nil, &types.InputValidationError{Err: fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)}
}

zipReader, err := zip.NewReader(bytes.NewReader(b.Bytes()), n)
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

// Checking that uncompressed metadata files are within acceptable bounds before reading into memory.
Expand All @@ -149,39 +149,38 @@ func (v *V001Entry) fetchExternalEntities(_ context.Context) (*pkcs7.PublicKey,
continue
}
if f.UncompressedSize64 > viper.GetUint64("max_jar_metadata_size") && viper.GetUint64("max_jar_metadata_size") > 0 {
return nil, nil, types.ValidationError(
fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size")))
return nil, nil, &types.InputValidationError{Err: fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size"))}
}
}

// this ensures that the JAR is signed and the signature verifies, as
// well as checks that the hashes in the signed manifest are all valid
jarObjs, err := jarutils.Verify(zipReader, false)
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}
switch len(jarObjs) {
case 0:
return nil, nil, types.ValidationError(errors.New("no signatures detected in JAR archive"))
return nil, nil, &types.InputValidationError{Err: errors.New("no signatures detected in JAR archive")}
case 1:
default:
return nil, nil, types.ValidationError(errors.New("multiple signatures detected in JAR; unable to process"))
return nil, nil, &types.InputValidationError{Err: errors.New("multiple signatures detected in JAR; unable to process")}
}

// we need to find and extract the PKCS7 bundle from the JAR file manually
sigPKCS7, err := extractPKCS7SignatureFromJAR(zipReader)
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

keyObj, err := pkcs7.NewPublicKey(bytes.NewReader(sigPKCS7))
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

sigObj, err := pkcs7.NewSignature(bytes.NewReader(sigPKCS7))
if err != nil {
return nil, nil, types.ValidationError(err)
return nil, nil, &types.InputValidationError{Err: err}
}

// if we get here, all goroutines succeeded without error
Expand Down
4 changes: 3 additions & 1 deletion pkg/types/jar/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package jar
import (
"bytes"
"context"
"errors"
"os"
"reflect"
"strings"
Expand Down Expand Up @@ -124,7 +125,8 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ==
if (err == nil) != tc.expectCanonicalizeSuccess {
t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
} else if err != nil {
if _, ok := err.(types.ValidationError); !ok {
var validationErr *types.InputValidationError
if !errors.As(err, &validationErr) {
t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/types/rekord/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p

computedSHA := hex.EncodeToString(hasher.Sum(nil))
if oldSHA != "" && computedSHA != oldSHA {
return closePipesOnError(types.ValidationError(fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)))
return closePipesOnError(&types.InputValidationError{Err: fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)})
}

select {
Expand All @@ -182,7 +182,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p

signature, err := af.NewSignature(sigReadCloser)
if err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

select {
Expand All @@ -202,7 +202,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p

key, err := af.NewPublicKey(keyReadCloser)
if err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

select {
Expand All @@ -226,7 +226,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p

var err error
if err = sigObj.Verify(sigR, keyObj); err != nil {
return closePipesOnError(types.ValidationError(err))
return closePipesOnError(&types.InputValidationError{Err: err})
}

select {
Expand Down
4 changes: 3 additions & 1 deletion pkg/types/rekord/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"crypto/sha256"
"encoding/hex"
"errors"
"os"
"reflect"
"testing"
Expand Down Expand Up @@ -242,7 +243,8 @@ func TestCrossFieldValidation(t *testing.T) {
if (err == nil) != tc.expectCanonicalizeSuccess {
t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
} else if err != nil {
if _, ok := err.(types.ValidationError); !ok {
var validationErr *types.InputValidationError
if !errors.As(err, &validationErr) {
t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/rfc3161/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {

func (v *V001Entry) Canonicalize(_ context.Context) ([]byte, error) {
if v.tsrContent == nil {
return nil, errors.New("tsr content must be set before canonicalizing")
return nil, &types.InputValidationError{Err: errors.New("tsr content must be set before canonicalizing")}
}
canonicalEntry := models.Rfc3161V001Schema{
Tsr: &models.Rfc3161V001SchemaTsr{
Expand Down
Loading

0 comments on commit c16f610

Please sign in to comment.