Skip to content

Commit

Permalink
fix: update error message from notation-go (notaryproject#345)
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick Zheng <[email protected]>
  • Loading branch information
Two-Hearts authored Oct 28, 2023
1 parent cb6f009 commit 765d02b
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 37 deletions.
2 changes: 1 addition & 1 deletion dir/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func LocalKeyPath(name string) (keyPath, certPath string) {
//
// items includes named-store and cert-file names.
// the directory follows the pattern of
// {NOTATION_CONFIG}/truststore/x509/{named-store}/{cert-file}
// {NOTATION_CONFIG}/truststore/x509/{store-type}/{named-store}/{cert-file}
func X509TrustStoreDir(items ...string) string {
pathItems := []string{TrustStoreDir, "x509"}
pathItems = append(pathItems, items...)
Expand Down
25 changes: 12 additions & 13 deletions notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ type ValidationResult struct {
Error error
}

// VerificationOutcome encapsulates a signature blob's descriptor, its content,
// VerificationOutcome encapsulates a signature envelope blob, its content,
// the verification level and results for each verification type that was
// performed.
type VerificationOutcome struct {
Expand Down Expand Up @@ -347,12 +347,12 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("user input digest %s does not match the resolved digest %s", ref.Reference, artifactDescriptor.Digest.String())}
}

var verificationSucceeded bool
var verificationOutcomes []*VerificationOutcome
var verificationFailedErrorArray = []error{ErrorVerificationFailed{}}
errExceededMaxVerificationLimit := ErrorVerificationFailed{Msg: fmt.Sprintf("signature evaluation stopped. The configured limit of %d signatures to verify per artifact exceeded", verifyOpts.MaxSignatureAttempts)}
numOfSignatureProcessed := 0

var verificationFailedErr error = ErrorVerificationFailed{}

// get signature manifests
logger.Debug("Fetching signature manifests")
err = repo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error {
Expand Down Expand Up @@ -380,16 +380,15 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
logger.Error("Got nil outcome. Expecting non-nil outcome on verification failure")
return err
}

if _, ok := outcome.Error.(ErrorUserMetadataVerificationFailed); ok {
verificationFailedErr = outcome.Error
}

outcome.Error = fmt.Errorf("failed to verify signature with digest %v, %w", sigManifestDesc.Digest, outcome.Error)
verificationFailedErrorArray = append(verificationFailedErrorArray, outcome.Error)
continue
}
// at this point, the signature is verified successfully. Add
// it to the verificationOutcomes.
verificationOutcomes = append(verificationOutcomes, outcome)
// at this point, the signature is verified successfully
verificationSucceeded = true
// on success, verificationOutcomes only contains the
// succeeded outcome
verificationOutcomes = []*VerificationOutcome{outcome}
logger.Debugf("Signature verification succeeded for artifact %v with signature digest %v", artifactDescriptor.Digest, sigManifestDesc.Digest)

// early break on success
Expand All @@ -416,9 +415,9 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
}

// Verification Failed
if len(verificationOutcomes) == 0 {
if !verificationSucceeded {
logger.Debugf("Signature verification failed for all the signatures associated with artifact %v", artifactDescriptor.Digest)
return ocispec.Descriptor{}, verificationOutcomes, verificationFailedErr
return ocispec.Descriptor{}, verificationOutcomes, errors.Join(verificationFailedErrorArray...)
}

// Verification Succeeded
Expand Down
4 changes: 2 additions & 2 deletions verifier/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po
case signature.SigningSchemeX509SigningAuthority:
typeToLoad = truststore.TypeSigningAuthority
default:
return nil, fmt.Errorf("unrecognized signing scheme %q", scheme)
return nil, truststore.TrustStoreError{Msg: fmt.Sprintf("error while loading the trust store, unrecognized signing scheme %q", scheme)}
}

processedStoreSet := set.New[string]()
Expand All @@ -71,7 +71,7 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po

storeType, name, found := strings.Cut(trustStore, ":")
if !found {
return nil, fmt.Errorf("trust policy statement %q is missing separator in trust store value %q. The required format is <TrustStoreType>:<TrustStoreName>", policy.Name, trustStore)
return nil, truststore.TrustStoreError{Msg: fmt.Sprintf("error while loading the trust store, trust policy statement %q is missing separator in trust store value %q. The required format is <TrustStoreType>:<TrustStoreName>", policy.Name, trustStore)}
}
if typeToLoad != truststore.Type(storeType) {
continue
Expand Down
54 changes: 54 additions & 0 deletions verifier/truststore/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright The Notary Project Authors.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package truststore

// TrustStoreError is used when accessing specified trust store failed
type TrustStoreError struct {
Msg string
InnerError error
}

func (e TrustStoreError) Error() string {
if e.Msg != "" {
return e.Msg
}
if e.InnerError != nil {
return e.InnerError.Error()
}
return "unable to access the trust store"
}

func (e TrustStoreError) Unwrap() error {
return e.InnerError
}

// CertificateError is used when reading a certificate failed
type CertificateError struct {
Msg string
InnerError error
}

func (e CertificateError) Error() string {
if e.Msg != "" {
return e.Msg
}
if e.InnerError != nil {
return e.InnerError.Error()
}
return "unable to read the certificate"
}

func (e CertificateError) Unwrap() error {
return e.InnerError
}
25 changes: 13 additions & 12 deletions verifier/truststore/truststore.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,49 +64,50 @@ type x509TrustStore struct {
// GetCertificates returns certificates under storeType/namedStore
func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType Type, namedStore string) ([]*x509.Certificate, error) {
if !isValidStoreType(storeType) {
return nil, fmt.Errorf("unsupported store type: %s", storeType)
return nil, TrustStoreError{Msg: fmt.Sprintf("unsupported trust store type: %s", storeType)}
}
if !file.IsValidFileName(namedStore) {
return nil, errors.New("named store name needs to follow [a-zA-Z0-9_.-]+ format")
return nil, TrustStoreError{Msg: fmt.Sprintf("trust store name needs to follow [a-zA-Z0-9_.-]+ format, %s is invalid", namedStore)}
}
path, err := trustStore.trustStorefs.SysPath(dir.X509TrustStoreDir(string(storeType), namedStore))
if err != nil {
return nil, err
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("failed to get path of trust store %s of type %s", namedStore, storeType)}
}
// throw error if path is not a directory or is a symlink or does not exist.
fileInfo, err := os.Lstat(path)
if err != nil {
if os.IsNotExist(err) {
return nil, fmt.Errorf("%q does not exist", path)
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q does not exist", namedStore, storeType)}
}
return nil, err
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("failed to access the trust store %q of type %q", namedStore, storeType)}
}
mode := fileInfo.Mode()
if !mode.IsDir() || mode&fs.ModeSymlink != 0 {
return nil, fmt.Errorf("%q is not a regular directory (symlinks are not supported)", path)
return nil, TrustStoreError{Msg: fmt.Sprintf("the trust store %s of type %s with path %s is not a regular directory (symlinks are not supported)", namedStore, storeType, path)}
}
files, err := os.ReadDir(path)
if err != nil {
return nil, err
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("failed to access the trust store %q of type %q", namedStore, storeType)}
}

var certificates []*x509.Certificate
for _, file := range files {
joinedPath := filepath.Join(path, file.Name())
certFileName := file.Name()
joinedPath := filepath.Join(path, certFileName)
if file.IsDir() || file.Type()&fs.ModeSymlink != 0 {
return nil, fmt.Errorf("%q is not a regular file (directories or symlinks are not supported)", joinedPath)
return nil, CertificateError{Msg: fmt.Sprintf("trusted certificate %s in trust store %s of type %s is not a regular file (directories or symlinks are not supported)", certFileName, namedStore, storeType)}
}
certs, err := corex509.ReadCertificateFile(joinedPath)
if err != nil {
return nil, fmt.Errorf("error while reading certificates from %q: %w", joinedPath, err)
return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("failed to read the trusted certificate %s in trust store %s of type %s", certFileName, namedStore, storeType)}
}
if err := ValidateCertificates(certs); err != nil {
return nil, fmt.Errorf("error while validating certificates from %q: %w", joinedPath, err)
return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", certFileName, namedStore, storeType)}
}
certificates = append(certificates, certs...)
}
if len(certificates) < 1 {
return nil, fmt.Errorf("trust store %q has no x509 certificates", path)
return nil, CertificateError{InnerError: fs.ErrNotExist, Msg: fmt.Sprintf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)}
}
return certificates, nil
}
Expand Down
16 changes: 8 additions & 8 deletions verifier/truststore/truststore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,26 @@ func TestLoadValidTrustStoreWithSelfSignedSigningCertificate(t *testing.T) {
}

func TestLoadTrustStoreWithInvalidCerts(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid")
expectedErr := fmt.Errorf("error while reading certificates from %q: x509: malformed certificate", failurePath)
// testing ../testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid
expectedErr := fmt.Errorf("failed to read the trusted certificate %s in trust store %s of type %s", "invalid", "trust-store-with-invalid-certs", "ca")
_, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-invalid-certs")
if err == nil || err.Error() != expectedErr.Error() {
t.Fatalf("invalid certs should return error: %q", expectedErr)
}
}

func TestLoadTrustStoreWithLeafCerts(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt")
expectedErrMsg := fmt.Sprintf("error while validating certificates from %q: certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" is not a CA certificate or self-signed signing certificate", failurePath)
// testing ../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt
expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", "non-ca.crt", "trust-store-with-leaf-certs", "ca")
_, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("leaf cert in a trust store should return error: %s, got: %v", expectedErrMsg, err)
}
}

func TestLoadTrustStoreWithLeafCertsInSingleFile(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt")
expectedErrMsg := fmt.Sprintf("error while validating certificates from %q: certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" is not a CA certificate or self-signed signing certificate", failurePath)
// testing ../testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt
expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", "RootAndLeafCerts.crt", "trust-store-with-leaf-certs-in-single-file", "ca")
_, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs-in-single-file")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("leaf cert in a trust store should return error: %s, got: %v", expectedErrMsg, err)
Expand All @@ -80,7 +80,7 @@ func TestValidateCerts(t *testing.T) {
joinedPath := filepath.FromSlash("../testdata/truststore/x509/ca/valid-trust-store/GlobalSign.der")
certs, err := corex509.ReadCertificateFile(joinedPath)
if err != nil {
t.Fatalf("error while reading certificates from %q: %q", joinedPath, err)
t.Fatalf("failed to read the trusted certificate %q: %q", joinedPath, err)
}
err = ValidateCertificates(certs)
if err != nil {
Expand All @@ -93,7 +93,7 @@ func TestValidateCertsWithLeafCert(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt")
certs, err := corex509.ReadCertificateFile(failurePath)
if err != nil {
t.Fatalf("error while reading certificates from %q: %q", failurePath, err)
t.Fatalf("failed to read the trusted certificate %q: %q", failurePath, err)
}
expectedErr := errors.New("certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" is not a CA certificate or self-signed signing certificate")
err = ValidateCertificates(certs)
Expand Down
2 changes: 1 addition & 1 deletion verifier/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ func verifyAuthenticity(ctx context.Context, trustPolicy *trustpolicy.TrustPolic

if err != nil {
return &notation.ValidationResult{
Error: notation.ErrorVerificationInconclusive{Msg: fmt.Sprintf("error while loading the trust store, %v", err)},
Error: err,
Type: trustpolicy.TypeAuthenticity,
Action: outcome.VerificationLevel.Enforcement[trustpolicy.TypeAuthenticity],
}
Expand Down

0 comments on commit 765d02b

Please sign in to comment.