Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update error message from notation-go #345

Merged
merged 23 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
Two-Hearts marked this conversation as resolved.
Show resolved Hide resolved

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 {
Two-Hearts marked this conversation as resolved.
Show resolved Hide resolved
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