From e80c2e9dc9b321253bc300c1238a779c3baa8c59 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Thu, 24 Aug 2023 17:09:48 +0800 Subject: [PATCH 01/17] update Signed-off-by: Patrick Zheng --- dir/path.go | 2 +- notation.go | 14 ++++++++------ verifier/truststore/errors.go | 14 ++++++++++++++ verifier/truststore/truststore.go | 22 +++++++++++----------- verifier/verifier.go | 2 +- 5 files changed, 35 insertions(+), 19 deletions(-) create mode 100644 verifier/truststore/errors.go diff --git a/dir/path.go b/dir/path.go index 5c953eb3..08dbd178 100644 --- a/dir/path.go +++ b/dir/path.go @@ -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...) diff --git a/notation.go b/notation.go index 3e65ade5..51b5e1dd 100644 --- a/notation.go +++ b/notation.go @@ -352,6 +352,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve numOfSignatureProcessed := 0 var verificationFailedErr error = ErrorVerificationFailed{} + var verificationSucceeded bool // get signature manifests logger.Debug("Fetching signature manifests") @@ -380,16 +381,17 @@ 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 } - + verificationOutcomes = append(verificationOutcomes, outcome) 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 @@ -416,7 +418,7 @@ 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 } diff --git a/verifier/truststore/errors.go b/verifier/truststore/errors.go new file mode 100644 index 00000000..e2422cd2 --- /dev/null +++ b/verifier/truststore/errors.go @@ -0,0 +1,14 @@ +package truststore + +// ErrorTrustStoreNonExistence is used when specified trust store or +// certificate path does not exist. +type ErrorTrustStoreNonExistence struct { + Msg string +} + +func (e ErrorTrustStoreNonExistence) Error() string { + if e.Msg != "" { + return e.Msg + } + return "unable to find specified trust store or certificate" +} diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index 0aaed7bd..a0c03c48 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -64,49 +64,49 @@ 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, fmt.Errorf("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, fmt.Errorf("named 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, fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err) } // 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, ErrorTrustStoreNonExistence{Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} } - return nil, err + return nil, fmt.Errorf("failed to access the trust store %q: %w", path, err) } 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, fmt.Errorf("trust store %q is not a regular directory (symlinks are not supported)", path) } files, err := os.ReadDir(path) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to access the trust store %q: %w", path, err) } var certificates []*x509.Certificate for _, file := range files { joinedPath := filepath.Join(path, file.Name()) 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, fmt.Errorf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath) } certs, err := corex509.ReadCertificateFile(joinedPath) if err != nil { - return nil, fmt.Errorf("error while reading certificates from %q: %w", joinedPath, err) + return nil, fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err) } if err := ValidateCertificates(certs); err != nil { - return nil, fmt.Errorf("error while validating certificates from %q: %w", joinedPath, err) + return nil, fmt.Errorf("error while validating trusted certificate %q: %w", joinedPath, err) } certificates = append(certificates, certs...) } if len(certificates) < 1 { - return nil, fmt.Errorf("trust store %q has no x509 certificates", path) + return nil, ErrorTrustStoreNonExistence{Msg: fmt.Sprintf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)} } return certificates, nil } diff --git a/verifier/verifier.go b/verifier/verifier.go index e1ff2308..bf76494e 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -446,7 +446,7 @@ func verifyAuthenticity(ctx context.Context, trustPolicy *trustpolicy.TrustPolic if err != nil { return ¬ation.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], } From 3589832221f0c0c46498159cfc1fac1b7cc359b4 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Aug 2023 11:29:25 +0800 Subject: [PATCH 02/17] initial commit Signed-off-by: Patrick Zheng --- verifier/helpers.go | 4 +-- verifier/truststore/errors.go | 36 +++++++++++++++++++++----- verifier/truststore/truststore.go | 22 ++++++++-------- verifier/truststore/truststore_test.go | 10 +++---- 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/verifier/helpers.go b/verifier/helpers.go index 9ea0c6bf..d96e1989 100644 --- a/verifier/helpers.go +++ b/verifier/helpers.go @@ -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.ErrorTrustStore{WrappedError: fmt.Errorf("error while loading the trust store, unrecognized signing scheme %q", scheme)} } processedStoreSet := set.New[string]() @@ -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 :", policy.Name, trustStore) + return nil, truststore.ErrorTrustStore{WrappedError: fmt.Errorf("error while loading the trust store, trust policy statement %q is missing separator in trust store value %q. The required format is :", policy.Name, trustStore)} } if typeToLoad != truststore.Type(storeType) { continue diff --git a/verifier/truststore/errors.go b/verifier/truststore/errors.go index e2422cd2..852db1a8 100644 --- a/verifier/truststore/errors.go +++ b/verifier/truststore/errors.go @@ -1,14 +1,38 @@ package truststore -// ErrorTrustStoreNonExistence is used when specified trust store or +// ErrorTrustStore is used when accessing specified trust store failed +type ErrorTrustStore struct { + WrappedError error +} + +func (e ErrorTrustStore) Error() string { + if e.WrappedError != nil { + return e.WrappedError.Error() + } + return "unable to access the trust store" +} + +// ErrorCertificate is used when reading a certificate failed +type ErrorCertificate struct { + WrappedError error +} + +func (e ErrorCertificate) Error() string { + if e.WrappedError != nil { + return e.WrappedError.Error() + } + return "unable to read the certificate" +} + +// ErrorNonExistence is used when specified trust store or // certificate path does not exist. -type ErrorTrustStoreNonExistence struct { - Msg string +type ErrorNonExistence struct { + WrappedError error } -func (e ErrorTrustStoreNonExistence) Error() string { - if e.Msg != "" { - return e.Msg +func (e ErrorNonExistence) Error() string { + if e.WrappedError != nil { + return e.WrappedError.Error() } return "unable to find specified trust store or certificate" } diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index a0c03c48..9fd2f4ac 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -64,49 +64,49 @@ 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 trust store type: %s", storeType) + return nil, ErrorTrustStore{WrappedError: fmt.Errorf("unsupported trust store type: %s", storeType)} } if !file.IsValidFileName(namedStore) { - return nil, fmt.Errorf("named store name needs to follow [a-zA-Z0-9_.-]+ format: %s is invalid", namedStore) + return nil, ErrorTrustStore{WrappedError: fmt.Errorf("named 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, fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err) + return nil, ErrorTrustStore{WrappedError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err)} } // 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, ErrorTrustStoreNonExistence{Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} + return nil, ErrorNonExistence{WrappedError: fmt.Errorf("the trust store %q of type %q doesn't exist", namedStore, storeType)} } - return nil, fmt.Errorf("failed to access the trust store %q: %w", path, err) + return nil, ErrorTrustStore{WrappedError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } mode := fileInfo.Mode() if !mode.IsDir() || mode&fs.ModeSymlink != 0 { - return nil, fmt.Errorf("trust store %q is not a regular directory (symlinks are not supported)", path) + return nil, ErrorTrustStore{WrappedError: fmt.Errorf("trust store %q is not a regular directory (symlinks are not supported)", path)} } files, err := os.ReadDir(path) if err != nil { - return nil, fmt.Errorf("failed to access the trust store %q: %w", path, err) + return nil, ErrorTrustStore{WrappedError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } var certificates []*x509.Certificate for _, file := range files { joinedPath := filepath.Join(path, file.Name()) if file.IsDir() || file.Type()&fs.ModeSymlink != 0 { - return nil, fmt.Errorf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath) + return nil, ErrorCertificate{WrappedError: fmt.Errorf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath)} } certs, err := corex509.ReadCertificateFile(joinedPath) if err != nil { - return nil, fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err) + return nil, ErrorCertificate{WrappedError: fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err)} } if err := ValidateCertificates(certs); err != nil { - return nil, fmt.Errorf("error while validating trusted certificate %q: %w", joinedPath, err) + return nil, ErrorCertificate{WrappedError: fmt.Errorf("failed to validate the trusted certificate %q: %w", joinedPath, err)} } certificates = append(certificates, certs...) } if len(certificates) < 1 { - return nil, ErrorTrustStoreNonExistence{Msg: fmt.Sprintf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)} + return nil, ErrorNonExistence{WrappedError: fmt.Errorf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)} } return certificates, nil } diff --git a/verifier/truststore/truststore_test.go b/verifier/truststore/truststore_test.go index b916468b..5268d4cb 100644 --- a/verifier/truststore/truststore_test.go +++ b/verifier/truststore/truststore_test.go @@ -50,7 +50,7 @@ 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) + expectedErr := fmt.Errorf("failed to read the trusted certificate %q: x509: malformed certificate", failurePath) _, 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) @@ -59,7 +59,7 @@ func TestLoadTrustStoreWithInvalidCerts(t *testing.T) { 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) + expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %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) _, 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) @@ -68,7 +68,7 @@ func TestLoadTrustStoreWithLeafCerts(t *testing.T) { 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) + expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %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) _, 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) @@ -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 { @@ -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) From 5200c6a7a367ea17a2dac4b2734e7bcf87206ef6 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Aug 2023 12:43:52 +0800 Subject: [PATCH 03/17] added signature manifest desc to verification outcomes Signed-off-by: Patrick Zheng --- notation.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/notation.go b/notation.go index 51b5e1dd..a9baf77d 100644 --- a/notation.go +++ b/notation.go @@ -193,10 +193,13 @@ type ValidationResult struct { Error error } -// VerificationOutcome encapsulates a signature blob's descriptor, its content, -// the verification level and results for each verification type that was -// performed. +// VerificationOutcome encapsulates a signature manifest's descriptor, +// its envelope blob, its content, the verification level and results for each +// verification type that was performed. type VerificationOutcome struct { + // SignatureManifestDescriptor + SignatureManifestDescriptor *ocispec.Descriptor + // RawSignature is the signature envelope blob RawSignature []byte @@ -347,13 +350,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 verificationFailedErr 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{} - var verificationSucceeded bool - // get signature manifests logger.Debug("Fetching signature manifests") err = repo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error { @@ -384,6 +386,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve if _, ok := outcome.Error.(ErrorUserMetadataVerificationFailed); ok { verificationFailedErr = outcome.Error } + outcome.SignatureManifestDescriptor = &sigManifestDesc verificationOutcomes = append(verificationOutcomes, outcome) continue } @@ -391,6 +394,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve verificationSucceeded = true // on success, verificationOutcomes only contains the // succeeded outcome + outcome.SignatureManifestDescriptor = &sigManifestDesc verificationOutcomes = []*VerificationOutcome{outcome} logger.Debugf("Signature verification succeeded for artifact %v with signature digest %v", artifactDescriptor.Digest, sigManifestDesc.Digest) From 6d192022d0e2ae012c63e706cfbd8b1ef7785928 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Aug 2023 12:58:52 +0800 Subject: [PATCH 04/17] update Signed-off-by: Patrick Zheng --- notation.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/notation.go b/notation.go index a9baf77d..f95efd2b 100644 --- a/notation.go +++ b/notation.go @@ -383,9 +383,6 @@ 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.SignatureManifestDescriptor = &sigManifestDesc verificationOutcomes = append(verificationOutcomes, outcome) continue From c52d266581a20342e9c3e4c49db860422d2d73a5 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Aug 2023 14:32:07 +0800 Subject: [PATCH 05/17] test Signed-off-by: Patrick Zheng --- notation.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/notation.go b/notation.go index f95efd2b..9341900d 100644 --- a/notation.go +++ b/notation.go @@ -384,7 +384,8 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return err } outcome.SignatureManifestDescriptor = &sigManifestDesc - verificationOutcomes = append(verificationOutcomes, outcome) + outcomeCopy := outcome + verificationOutcomes = append(verificationOutcomes, outcomeCopy) continue } // at this point, the signature is verified successfully From a19f69394f006afdf74d8bed8d640fbbc7c28acb Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Aug 2023 14:34:45 +0800 Subject: [PATCH 06/17] test Signed-off-by: Patrick Zheng --- notation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/notation.go b/notation.go index 9341900d..cce31d7a 100644 --- a/notation.go +++ b/notation.go @@ -384,8 +384,8 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return err } outcome.SignatureManifestDescriptor = &sigManifestDesc - outcomeCopy := outcome - verificationOutcomes = append(verificationOutcomes, outcomeCopy) + outcomeCopy := *outcome + verificationOutcomes = append(verificationOutcomes, &outcomeCopy) continue } // at this point, the signature is verified successfully From 48e71bc7361cc84e2c737032db9442d9bae624a0 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Aug 2023 14:53:12 +0800 Subject: [PATCH 07/17] fix Signed-off-by: Patrick Zheng --- notation.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/notation.go b/notation.go index cce31d7a..3efe8872 100644 --- a/notation.go +++ b/notation.go @@ -360,7 +360,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve logger.Debug("Fetching signature manifests") err = repo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error { // process signatures - for _, sigManifestDesc := range signatureManifests { + for ind, sigManifestDesc := range signatureManifests { if numOfSignatureProcessed >= verifyOpts.MaxSignatureAttempts { break } @@ -383,7 +383,7 @@ 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 } - outcome.SignatureManifestDescriptor = &sigManifestDesc + outcome.SignatureManifestDescriptor = &signatureManifests[ind] outcomeCopy := *outcome verificationOutcomes = append(verificationOutcomes, &outcomeCopy) continue @@ -392,7 +392,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve verificationSucceeded = true // on success, verificationOutcomes only contains the // succeeded outcome - outcome.SignatureManifestDescriptor = &sigManifestDesc + outcome.SignatureManifestDescriptor = &signatureManifests[ind] verificationOutcomes = []*VerificationOutcome{outcome} logger.Debugf("Signature verification succeeded for artifact %v with signature digest %v", artifactDescriptor.Digest, sigManifestDesc.Digest) From 54c76462965535f9b442f9244f512cd8fac004e1 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 28 Aug 2023 16:51:20 +0800 Subject: [PATCH 08/17] update Signed-off-by: Patrick Zheng --- notation.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/notation.go b/notation.go index 3efe8872..f3bcd8bb 100644 --- a/notation.go +++ b/notation.go @@ -384,8 +384,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return err } outcome.SignatureManifestDescriptor = &signatureManifests[ind] - outcomeCopy := *outcome - verificationOutcomes = append(verificationOutcomes, &outcomeCopy) + verificationOutcomes = append(verificationOutcomes, outcome) continue } // at this point, the signature is verified successfully From 5914b389da5a906c488b9a533c9b087170bf492d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Aug 2023 06:20:43 +0000 Subject: [PATCH 09/17] build(deps): bump golang.org/x/crypto from 0.11.0 to 0.12.0 (#344) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 8bdc9396..bc209fa6 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0-rc4 github.com/veraison/go-cose v1.1.0 - golang.org/x/crypto v0.11.0 + golang.org/x/crypto v0.12.0 golang.org/x/mod v0.12.0 oras.land/oras-go/v2 v2.2.1 ) diff --git a/go.sum b/go.sum index a4d224a6..9ed5a621 100644 --- a/go.sum +++ b/go.sum @@ -34,8 +34,8 @@ github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5t golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= -golang.org/x/crypto v0.11.0 h1:6Ewdq3tDic1mg5xRO4milcWCfMVQhI4NkqWWvqejpuA= -golang.org/x/crypto v0.11.0/go.mod h1:xgJhtzW8F9jGdVFWZESrid1U1bjeNy4zgy5cRr/CIio= +golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk= +golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= From fcd8ca6c4475d7e2852e6403af192283513613cf Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 29 Aug 2023 09:39:13 +0800 Subject: [PATCH 10/17] fixed license Signed-off-by: Patrick Zheng --- verifier/truststore/errors.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/verifier/truststore/errors.go b/verifier/truststore/errors.go index 852db1a8..cefcaab9 100644 --- a/verifier/truststore/errors.go +++ b/verifier/truststore/errors.go @@ -1,3 +1,16 @@ +// 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 // ErrorTrustStore is used when accessing specified trust store failed From 06b9ea427f0e3e4f6e8a8a20e2a5a470d15323ac Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 30 Aug 2023 12:07:07 +0800 Subject: [PATCH 11/17] updated per code review Signed-off-by: Patrick Zheng --- notation.go | 14 +++++------- verifier/helpers.go | 4 ++-- verifier/truststore/errors.go | 37 +++++++++++++++++-------------- verifier/truststore/truststore.go | 22 +++++++++--------- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/notation.go b/notation.go index f3bcd8bb..8e217f3f 100644 --- a/notation.go +++ b/notation.go @@ -193,13 +193,10 @@ type ValidationResult struct { Error error } -// VerificationOutcome encapsulates a signature manifest's descriptor, -// its envelope blob, its content, the verification level and results for each -// verification type that was performed. +// VerificationOutcome encapsulates a signature envelope blob, its content, +// the verification level and results for each verification type that was +// performed. type VerificationOutcome struct { - // SignatureManifestDescriptor - SignatureManifestDescriptor *ocispec.Descriptor - // RawSignature is the signature envelope blob RawSignature []byte @@ -360,7 +357,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve logger.Debug("Fetching signature manifests") err = repo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error { // process signatures - for ind, sigManifestDesc := range signatureManifests { + for _, sigManifestDesc := range signatureManifests { if numOfSignatureProcessed >= verifyOpts.MaxSignatureAttempts { break } @@ -383,7 +380,7 @@ 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 } - outcome.SignatureManifestDescriptor = &signatureManifests[ind] + outcome.Error = fmt.Errorf("failed to verify signature with digest %v, %w", sigManifestDesc.Digest, outcome.Error) verificationOutcomes = append(verificationOutcomes, outcome) continue } @@ -391,7 +388,6 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve verificationSucceeded = true // on success, verificationOutcomes only contains the // succeeded outcome - outcome.SignatureManifestDescriptor = &signatureManifests[ind] verificationOutcomes = []*VerificationOutcome{outcome} logger.Debugf("Signature verification succeeded for artifact %v with signature digest %v", artifactDescriptor.Digest, sigManifestDesc.Digest) diff --git a/verifier/helpers.go b/verifier/helpers.go index d96e1989..4e46ab17 100644 --- a/verifier/helpers.go +++ b/verifier/helpers.go @@ -58,7 +58,7 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po case signature.SigningSchemeX509SigningAuthority: typeToLoad = truststore.TypeSigningAuthority default: - return nil, truststore.ErrorTrustStore{WrappedError: fmt.Errorf("error while loading the trust store, unrecognized signing scheme %q", scheme)} + return nil, truststore.ErrorTrustStore{Msg: fmt.Sprintf("error while loading the trust store, unrecognized signing scheme %q", scheme)} } processedStoreSet := set.New[string]() @@ -71,7 +71,7 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po storeType, name, found := strings.Cut(trustStore, ":") if !found { - return nil, truststore.ErrorTrustStore{WrappedError: fmt.Errorf("error while loading the trust store, trust policy statement %q is missing separator in trust store value %q. The required format is :", policy.Name, trustStore)} + return nil, truststore.ErrorTrustStore{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 :", policy.Name, trustStore)} } if typeToLoad != truststore.Type(storeType) { continue diff --git a/verifier/truststore/errors.go b/verifier/truststore/errors.go index cefcaab9..908769d2 100644 --- a/verifier/truststore/errors.go +++ b/verifier/truststore/errors.go @@ -15,37 +15,40 @@ package truststore // ErrorTrustStore is used when accessing specified trust store failed type ErrorTrustStore struct { - WrappedError error + Msg string + InnerError error } func (e ErrorTrustStore) Error() string { - if e.WrappedError != nil { - return e.WrappedError.Error() + if e.Msg != "" { + return e.Msg + } + if e.InnerError != nil { + return e.InnerError.Error() } return "unable to access the trust store" } +func (e ErrorTrustStore) Unwrap() error { + return e.InnerError +} + // ErrorCertificate is used when reading a certificate failed type ErrorCertificate struct { - WrappedError error + Msg string + InnerError error } func (e ErrorCertificate) Error() string { - if e.WrappedError != nil { - return e.WrappedError.Error() + if e.Msg != "" { + return e.Msg + } + if e.InnerError != nil { + return e.InnerError.Error() } return "unable to read the certificate" } -// ErrorNonExistence is used when specified trust store or -// certificate path does not exist. -type ErrorNonExistence struct { - WrappedError error -} - -func (e ErrorNonExistence) Error() string { - if e.WrappedError != nil { - return e.WrappedError.Error() - } - return "unable to find specified trust store or certificate" +func (e ErrorCertificate) Unwrap() error { + return e.InnerError } diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index 9fd2f4ac..42128a29 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -64,49 +64,49 @@ 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, ErrorTrustStore{WrappedError: fmt.Errorf("unsupported trust store type: %s", storeType)} + return nil, ErrorTrustStore{Msg: fmt.Sprintf("unsupported trust store type: %s", storeType)} } if !file.IsValidFileName(namedStore) { - return nil, ErrorTrustStore{WrappedError: fmt.Errorf("named store name needs to follow [a-zA-Z0-9_.-]+ format: %s is invalid", namedStore)} + return nil, ErrorTrustStore{Msg: fmt.Sprintf("named 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, ErrorTrustStore{WrappedError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err)} + return nil, ErrorTrustStore{InnerError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err)} } // 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, ErrorNonExistence{WrappedError: fmt.Errorf("the trust store %q of type %q doesn't exist", namedStore, storeType)} + return nil, ErrorTrustStore{InnerError: fs.ErrNotExist, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} } - return nil, ErrorTrustStore{WrappedError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} + return nil, ErrorTrustStore{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } mode := fileInfo.Mode() if !mode.IsDir() || mode&fs.ModeSymlink != 0 { - return nil, ErrorTrustStore{WrappedError: fmt.Errorf("trust store %q is not a regular directory (symlinks are not supported)", path)} + return nil, ErrorTrustStore{Msg: fmt.Sprintf("trust store %q is not a regular directory (symlinks are not supported)", path)} } files, err := os.ReadDir(path) if err != nil { - return nil, ErrorTrustStore{WrappedError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} + return nil, ErrorTrustStore{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } var certificates []*x509.Certificate for _, file := range files { joinedPath := filepath.Join(path, file.Name()) if file.IsDir() || file.Type()&fs.ModeSymlink != 0 { - return nil, ErrorCertificate{WrappedError: fmt.Errorf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath)} + return nil, ErrorCertificate{Msg: fmt.Sprintf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath)} } certs, err := corex509.ReadCertificateFile(joinedPath) if err != nil { - return nil, ErrorCertificate{WrappedError: fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err)} + return nil, ErrorCertificate{InnerError: fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err)} } if err := ValidateCertificates(certs); err != nil { - return nil, ErrorCertificate{WrappedError: fmt.Errorf("failed to validate the trusted certificate %q: %w", joinedPath, err)} + return nil, ErrorCertificate{InnerError: fmt.Errorf("failed to validate the trusted certificate %q: %w", joinedPath, err)} } certificates = append(certificates, certs...) } if len(certificates) < 1 { - return nil, ErrorNonExistence{WrappedError: fmt.Errorf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)} + return nil, ErrorCertificate{InnerError: fs.ErrNotExist, Msg: fmt.Sprintf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)} } return certificates, nil } From f0b5b466f54cd887e8aa8b4bee2229c93a09b03f Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Thu, 31 Aug 2023 15:18:26 +0800 Subject: [PATCH 12/17] updated per code review Signed-off-by: Patrick Zheng --- notation.go | 2 +- verifier/truststore/truststore.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/notation.go b/notation.go index 8e217f3f..6a2cf17d 100644 --- a/notation.go +++ b/notation.go @@ -381,7 +381,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return err } outcome.Error = fmt.Errorf("failed to verify signature with digest %v, %w", sigManifestDesc.Digest, outcome.Error) - verificationOutcomes = append(verificationOutcomes, outcome) + verificationFailedErr = errors.Join(verificationFailedErr, outcome.Error) continue } // at this point, the signature is verified successfully diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index 42128a29..284310c9 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -77,7 +77,7 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType fileInfo, err := os.Lstat(path) if err != nil { if os.IsNotExist(err) { - return nil, ErrorTrustStore{InnerError: fs.ErrNotExist, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} + return nil, ErrorTrustStore{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} } return nil, ErrorTrustStore{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } From 33af9c76fdc9b853b401a85bfe501560082ab1a3 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Thu, 14 Sep 2023 14:49:25 +0800 Subject: [PATCH 13/17] fixed join error Signed-off-by: Patrick Zheng --- notation.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/notation.go b/notation.go index 6a2cf17d..483d9c73 100644 --- a/notation.go +++ b/notation.go @@ -350,6 +350,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve var verificationSucceeded bool var verificationOutcomes []*VerificationOutcome var verificationFailedErr error = ErrorVerificationFailed{} + var verificationFailedErrorArray = []error{verificationFailedErr} errExceededMaxVerificationLimit := ErrorVerificationFailed{Msg: fmt.Sprintf("signature evaluation stopped. The configured limit of %d signatures to verify per artifact exceeded", verifyOpts.MaxSignatureAttempts)} numOfSignatureProcessed := 0 @@ -381,7 +382,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return err } outcome.Error = fmt.Errorf("failed to verify signature with digest %v, %w", sigManifestDesc.Digest, outcome.Error) - verificationFailedErr = errors.Join(verificationFailedErr, outcome.Error) + verificationFailedErrorArray = append(verificationFailedErrorArray, outcome.Error) continue } // at this point, the signature is verified successfully @@ -395,6 +396,9 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return errDoneVerification } + // at this point, the verification process is failed + verificationFailedErr = errors.Join(verificationFailedErrorArray...) + if numOfSignatureProcessed >= verifyOpts.MaxSignatureAttempts { return errExceededMaxVerificationLimit } From 628df5a2b8fc9a7413f346f1c6edef165ac4f81e Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 18 Sep 2023 13:26:18 +0800 Subject: [PATCH 14/17] updated per code review Signed-off-by: Patrick Zheng --- notation.go | 4 +--- verifier/helpers.go | 4 ++-- verifier/truststore/errors.go | 16 ++++++++-------- verifier/truststore/truststore.go | 22 +++++++++++----------- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/notation.go b/notation.go index 483d9c73..86e72b7f 100644 --- a/notation.go +++ b/notation.go @@ -396,9 +396,6 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve return errDoneVerification } - // at this point, the verification process is failed - verificationFailedErr = errors.Join(verificationFailedErrorArray...) - if numOfSignatureProcessed >= verifyOpts.MaxSignatureAttempts { return errExceededMaxVerificationLimit } @@ -421,6 +418,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve // Verification Failed if !verificationSucceeded { logger.Debugf("Signature verification failed for all the signatures associated with artifact %v", artifactDescriptor.Digest) + verificationFailedErr = errors.Join(verificationFailedErrorArray...) return ocispec.Descriptor{}, verificationOutcomes, verificationFailedErr } diff --git a/verifier/helpers.go b/verifier/helpers.go index 4e46ab17..dfabadb0 100644 --- a/verifier/helpers.go +++ b/verifier/helpers.go @@ -58,7 +58,7 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po case signature.SigningSchemeX509SigningAuthority: typeToLoad = truststore.TypeSigningAuthority default: - return nil, truststore.ErrorTrustStore{Msg: fmt.Sprintf("error while loading the trust store, 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]() @@ -71,7 +71,7 @@ func loadX509TrustStores(ctx context.Context, scheme signature.SigningScheme, po storeType, name, found := strings.Cut(trustStore, ":") if !found { - return nil, truststore.ErrorTrustStore{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 :", 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 :", policy.Name, trustStore)} } if typeToLoad != truststore.Type(storeType) { continue diff --git a/verifier/truststore/errors.go b/verifier/truststore/errors.go index 908769d2..dd545dfe 100644 --- a/verifier/truststore/errors.go +++ b/verifier/truststore/errors.go @@ -13,13 +13,13 @@ package truststore -// ErrorTrustStore is used when accessing specified trust store failed -type ErrorTrustStore struct { +// TrustStoreError is used when accessing specified trust store failed +type TrustStoreError struct { Msg string InnerError error } -func (e ErrorTrustStore) Error() string { +func (e TrustStoreError) Error() string { if e.Msg != "" { return e.Msg } @@ -29,17 +29,17 @@ func (e ErrorTrustStore) Error() string { return "unable to access the trust store" } -func (e ErrorTrustStore) Unwrap() error { +func (e TrustStoreError) Unwrap() error { return e.InnerError } -// ErrorCertificate is used when reading a certificate failed -type ErrorCertificate struct { +// CertificateError is used when reading a certificate failed +type CertificateError struct { Msg string InnerError error } -func (e ErrorCertificate) Error() string { +func (e CertificateError) Error() string { if e.Msg != "" { return e.Msg } @@ -49,6 +49,6 @@ func (e ErrorCertificate) Error() string { return "unable to read the certificate" } -func (e ErrorCertificate) Unwrap() error { +func (e CertificateError) Unwrap() error { return e.InnerError } diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index 284310c9..7c6af00b 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -64,49 +64,49 @@ 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, ErrorTrustStore{Msg: fmt.Sprintf("unsupported trust store type: %s", storeType)} + return nil, TrustStoreError{Msg: fmt.Sprintf("unsupported trust store type: %s", storeType)} } if !file.IsValidFileName(namedStore) { - return nil, ErrorTrustStore{Msg: fmt.Sprintf("named store name needs to follow [a-zA-Z0-9_.-]+ format: %s is invalid", namedStore)} + return nil, TrustStoreError{Msg: fmt.Sprintf("named 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, ErrorTrustStore{InnerError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err)} + return nil, TrustStoreError{InnerError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err)} } // 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, ErrorTrustStore{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} + return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} } - return nil, ErrorTrustStore{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} + return nil, TrustStoreError{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } mode := fileInfo.Mode() if !mode.IsDir() || mode&fs.ModeSymlink != 0 { - return nil, ErrorTrustStore{Msg: fmt.Sprintf("trust store %q is not a regular directory (symlinks are not supported)", path)} + return nil, TrustStoreError{Msg: fmt.Sprintf("trust store %q is not a regular directory (symlinks are not supported)", path)} } files, err := os.ReadDir(path) if err != nil { - return nil, ErrorTrustStore{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} + return nil, TrustStoreError{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)} } var certificates []*x509.Certificate for _, file := range files { joinedPath := filepath.Join(path, file.Name()) if file.IsDir() || file.Type()&fs.ModeSymlink != 0 { - return nil, ErrorCertificate{Msg: fmt.Sprintf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath)} + return nil, CertificateError{Msg: fmt.Sprintf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath)} } certs, err := corex509.ReadCertificateFile(joinedPath) if err != nil { - return nil, ErrorCertificate{InnerError: fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err)} + return nil, CertificateError{InnerError: fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err)} } if err := ValidateCertificates(certs); err != nil { - return nil, ErrorCertificate{InnerError: fmt.Errorf("failed to validate the trusted certificate %q: %w", joinedPath, err)} + return nil, CertificateError{InnerError: fmt.Errorf("failed to validate the trusted certificate %q: %w", joinedPath, err)} } certificates = append(certificates, certs...) } if len(certificates) < 1 { - return nil, ErrorCertificate{InnerError: fs.ErrNotExist, Msg: fmt.Sprintf("no x509 certificates were found in trust store %q of type %q", namedStore, storeType)} + 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 } From 8357f92184cd72ccd2c0b05b23034e1d100d1a2d Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Fri, 15 Sep 2023 09:20:01 +0800 Subject: [PATCH 15/17] bump: update oras-go to v2.3.0 (#347) - Update oras-go to v2.3.0. - Replace oras.Pack() with oras.PackManifest() as it is deprecated in v2.3.0. - Generate an empty config blob manually, as oras.PackManifest() does not generate the config blob with the notation artifact type as the media type. Resolves #346 Signed-off-by: Junjie Gao --------- Signed-off-by: Junjie Gao --- go.mod | 2 +- go.sum | 4 +-- registry/repository.go | 49 ++++++++++++++++++++++++++++++++++--- registry/repository_test.go | 6 +++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 4cb6c29e..7fafa14c 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/veraison/go-cose v1.1.0 golang.org/x/crypto v0.13.0 golang.org/x/mod v0.12.0 - oras.land/oras-go/v2 v2.2.1 + oras.land/oras-go/v2 v2.3.0 ) require ( diff --git a/go.sum b/go.sum index d94f50a8..c8db3179 100644 --- a/go.sum +++ b/go.sum @@ -75,5 +75,5 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -oras.land/oras-go/v2 v2.2.1 h1:3VJTYqy5KfelEF9c2jo1MLSpr+TM3mX8K42wzZcd6qE= -oras.land/oras-go/v2 v2.2.1/go.mod h1:GeAwLuC4G/JpNwkd+bSZ6SkDMGaaYglt6YK2WvZP7uQ= +oras.land/oras-go/v2 v2.3.0 h1:lqX1aXdN+DAmDTKjiDyvq85cIaI4RkIKp/PghWlAGIU= +oras.land/oras-go/v2 v2.3.0/go.mod h1:GeAwLuC4G/JpNwkd+bSZ6SkDMGaaYglt6YK2WvZP7uQ= diff --git a/registry/repository.go b/registry/repository.go index cb6e20a3..908040f8 100644 --- a/registry/repository.go +++ b/registry/repository.go @@ -14,8 +14,10 @@ package registry import ( + "bytes" "context" "encoding/json" + "errors" "fmt" "os" @@ -24,6 +26,7 @@ import ( "oras.land/oras-go/v2" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/content/oci" + "oras.land/oras-go/v2/errdef" "oras.land/oras-go/v2/registry" ) @@ -32,6 +35,19 @@ const ( maxManifestSizeLimit = 4 * 1024 * 1024 // 4 MiB ) +var ( + // notationEmptyConfigDesc is the descriptor of an empty notation manifest + // config + // reference: https://github.com/notaryproject/specifications/blob/v1.0.0/specs/signature-specification.md#storage + notationEmptyConfigDesc = ocispec.Descriptor{ + MediaType: ArtifactTypeNotation, + Digest: ocispec.DescriptorEmptyJSON.Digest, + Size: ocispec.DescriptorEmptyJSON.Size, + } + // notationEmptyConfigData is the data of an empty notation manifest config + notationEmptyConfigData = ocispec.DescriptorEmptyJSON.Data +) + // RepositoryOptions provides user options when creating a Repository // it is kept for future extensibility type RepositoryOptions struct{} @@ -187,13 +203,40 @@ func (c *repositoryClient) getSignatureBlobDesc(ctx context.Context, sigManifest // uploadSignatureManifest uploads the signature manifest to the registry func (c *repositoryClient) uploadSignatureManifest(ctx context.Context, subject, blobDesc ocispec.Descriptor, annotations map[string]string) (ocispec.Descriptor, error) { - opts := oras.PackOptions{ + configDesc, err := pushNotationManifestConfig(ctx, c.GraphTarget) + if err != nil { + return ocispec.Descriptor{}, fmt.Errorf("failed to push notation manifest config: %w", err) + } + + opts := oras.PackManifestOptions{ Subject: &subject, ManifestAnnotations: annotations, - PackImageManifest: true, + Layers: []ocispec.Descriptor{blobDesc}, + ConfigDescriptor: &configDesc, + } + + return oras.PackManifest(ctx, c.GraphTarget, oras.PackManifestVersion1_1_RC4, "", opts) +} + +// pushNotationManifestConfig pushes an empty notation manifest config, if it +// doesn't exist. +// +// if the config exists, it returns the descriptor of the config without error. +func pushNotationManifestConfig(ctx context.Context, pusher content.Storage) (ocispec.Descriptor, error) { + // check if the config exists + exists, err := pusher.Exists(ctx, notationEmptyConfigDesc) + if err != nil { + return ocispec.Descriptor{}, fmt.Errorf("unable to verify existence: %s: %s. Details: %w", notationEmptyConfigDesc.Digest.String(), notationEmptyConfigDesc.MediaType, err) + } + if exists { + return notationEmptyConfigDesc, nil } - return oras.Pack(ctx, c.GraphTarget, ArtifactTypeNotation, []ocispec.Descriptor{blobDesc}, opts) + // return nil if the config pushed successfully or it already exists + if err := pusher.Push(ctx, notationEmptyConfigDesc, bytes.NewReader(notationEmptyConfigData)); err != nil && !errors.Is(err, errdef.ErrAlreadyExists) { + return ocispec.Descriptor{}, fmt.Errorf("unable to push: %s: %s. Details: %w", notationEmptyConfigDesc.Digest.String(), notationEmptyConfigDesc.MediaType, err) + } + return notationEmptyConfigDesc, nil } // signatureReferrers returns referrer nodes of desc in target filtered by diff --git a/registry/repository_test.go b/registry/repository_test.go index 38fbcf29..37caab88 100644 --- a/registry/repository_test.go +++ b/registry/repository_test.go @@ -38,6 +38,7 @@ import ( const ( zeroDigest = "sha256:0000000000000000000000000000000000000000000000000000000000000000" + emptyConfigDigest = "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a" validDigest = "6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b" validDigest2 = "1834876dcfb05cb167a5c24953eba58c4ac89b1adf57f28f2f9d09af107ee8f2" invalidDigest = "invaliddigest" @@ -129,6 +130,11 @@ func (c mockRemoteClient) Do(req *http.Request) (*http.Response, error) { "Docker-Content-Digest": {validDigestWithAlgo2}, }, }, nil + case "/v2/test/blobs/" + emptyConfigDigest: + return &http.Response{ + StatusCode: http.StatusNotFound, + Body: io.NopCloser(bytes.NewReader([]byte{})), + }, nil case "/v2/test/manifests/" + invalidDigest: return &http.Response{}, fmt.Errorf(errMsg) case "v2/test/manifest/" + validDigest2: From 2606b29ba9dc065e239f76fdf88df82e8eaaa312 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 18 Sep 2023 15:40:34 +0800 Subject: [PATCH 16/17] update Signed-off-by: Patrick Zheng --- notation.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/notation.go b/notation.go index 86e72b7f..ef8593c7 100644 --- a/notation.go +++ b/notation.go @@ -349,8 +349,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve var verificationSucceeded bool var verificationOutcomes []*VerificationOutcome - var verificationFailedErr error = ErrorVerificationFailed{} - var verificationFailedErrorArray = []error{verificationFailedErr} + 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 @@ -418,8 +417,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve // Verification Failed if !verificationSucceeded { logger.Debugf("Signature verification failed for all the signatures associated with artifact %v", artifactDescriptor.Digest) - verificationFailedErr = errors.Join(verificationFailedErrorArray...) - return ocispec.Descriptor{}, verificationOutcomes, verificationFailedErr + return ocispec.Descriptor{}, verificationOutcomes, errors.Join(verificationFailedErrorArray...) } // Verification Succeeded From 28b581b83f0cedecf7c54fc651231eb1f838870b Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Thu, 26 Oct 2023 14:04:35 +0800 Subject: [PATCH 17/17] updated per code review Signed-off-by: Patrick Zheng --- verifier/truststore/truststore.go | 21 +++++++++++---------- verifier/truststore/truststore_test.go | 12 ++++++------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index 7c6af00b..c98b2c6c 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -67,41 +67,42 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType return nil, TrustStoreError{Msg: fmt.Sprintf("unsupported trust store type: %s", storeType)} } if !file.IsValidFileName(namedStore) { - return nil, TrustStoreError{Msg: fmt.Sprintf("named store name needs to follow [a-zA-Z0-9_.-]+ format: %s is invalid", namedStore)} + 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, TrustStoreError{InnerError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, 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, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)} + return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q does not exist", namedStore, storeType)} } - return nil, TrustStoreError{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, 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, TrustStoreError{Msg: fmt.Sprintf("trust store %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, TrustStoreError{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, 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, CertificateError{Msg: fmt.Sprintf("trusted certificate %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, CertificateError{InnerError: fmt.Errorf("failed to read the trusted certificate %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, CertificateError{InnerError: fmt.Errorf("failed to validate the trusted certificate %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...) } diff --git a/verifier/truststore/truststore_test.go b/verifier/truststore/truststore_test.go index 5268d4cb..2f223a16 100644 --- a/verifier/truststore/truststore_test.go +++ b/verifier/truststore/truststore_test.go @@ -49,8 +49,8 @@ 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("failed to read the trusted certificate %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) @@ -58,8 +58,8 @@ func TestLoadTrustStoreWithInvalidCerts(t *testing.T) { } func TestLoadTrustStoreWithLeafCerts(t *testing.T) { - failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt") - expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %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) @@ -67,8 +67,8 @@ func TestLoadTrustStoreWithLeafCerts(t *testing.T) { } 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("failed to validate the trusted certificate %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)