diff --git a/.github/.codecov.yml b/.github/.codecov.yml index cb44908a..56e810e1 100644 --- a/.github/.codecov.yml +++ b/.github/.codecov.yml @@ -14,5 +14,8 @@ coverage: status: project: + default: + target: 80% + patch: default: target: 80% \ No newline at end of file diff --git a/example_signWithTimestmap_test.go b/example_signWithTimestmap_test.go index 8e0ebe5c..ef4eeb47 100644 --- a/example_signWithTimestmap_test.go +++ b/example_signWithTimestmap_test.go @@ -21,6 +21,8 @@ import ( "oras.land/oras-go/v2/registry/remote" + "github.com/notaryproject/notation-core-go/revocation" + "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/testhelper" "github.com/notaryproject/notation-go" "github.com/notaryproject/notation-go/registry" @@ -77,12 +79,21 @@ func Example_signWithTimestamp() { tsaRootCAs := x509.NewCertPool() tsaRootCAs.AddCert(tsaRootCert) + // enable timestamping certificate chain revocation check + tsaRevocationValidator, err := revocation.NewWithOptions(revocation.Options{ + CertChainPurpose: purpose.Timestamping, + }) + if err != nil { + panic(err) // Handle error + } + // exampleSignOptions is an example of notation.SignOptions. exampleSignOptions := notation.SignOptions{ SignerSignOptions: notation.SignerSignOptions{ - SignatureMediaType: exampleSignatureMediaType, - Timestamper: httpTimestamper, - TSARootCAs: tsaRootCAs, + SignatureMediaType: exampleSignatureMediaType, + Timestamper: httpTimestamper, + TSARootCAs: tsaRootCAs, + TSARevocationValidator: tsaRevocationValidator, }, ArtifactReference: exampleArtifactReference, } diff --git a/go.mod b/go.mod index a3c819e6..500bf084 100644 --- a/go.mod +++ b/go.mod @@ -4,14 +4,14 @@ go 1.22.0 require ( github.com/go-ldap/ldap/v3 v3.4.8 - github.com/notaryproject/notation-core-go v1.2.0-rc.1 + github.com/notaryproject/notation-core-go v1.2.0-rc.1.0.20241129024749-95d89543c9f9 github.com/notaryproject/notation-plugin-framework-go v1.0.0 - github.com/notaryproject/tspclient-go v0.2.0 + github.com/notaryproject/tspclient-go v0.2.1-0.20241030015323-90a141e7525c github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0 - github.com/veraison/go-cose v1.1.0 - golang.org/x/crypto v0.27.0 - golang.org/x/mod v0.21.0 + github.com/veraison/go-cose v1.3.0 + golang.org/x/crypto v0.29.0 + golang.org/x/mod v0.22.0 oras.land/oras-go/v2 v2.5.0 ) @@ -19,7 +19,7 @@ require ( github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect - github.com/golang-jwt/jwt/v4 v4.5.0 // indirect + github.com/golang-jwt/jwt/v4 v4.5.1 // indirect github.com/google/uuid v1.6.0 // indirect github.com/x448/float16 v0.8.4 // indirect golang.org/x/sync v0.6.0 // indirect diff --git a/go.sum b/go.sum index 87b2a338..887c93a4 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ github.com/go-asn1-ber/asn1-ber v1.5.5 h1:MNHlNMBDgEKD4TcKr36vQN68BA00aDfjIt3/bD github.com/go-asn1-ber/asn1-ber v1.5.5/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0= github.com/go-ldap/ldap/v3 v3.4.8 h1:loKJyspcRezt2Q3ZRMq2p/0v8iOurlmeXDPw6fikSvQ= github.com/go-ldap/ldap/v3 v3.4.8/go.mod h1:qS3Sjlu76eHfHGpUdWkAXQTw4beih+cHsco2jXlIXrk= -github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= -github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo= +github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= @@ -32,12 +32,12 @@ github.com/jcmturner/gokrb5/v8 v8.4.4 h1:x1Sv4HaTpepFkXbt2IkL29DXRf8sOfZXo8eRKh6 github.com/jcmturner/gokrb5/v8 v8.4.4/go.mod h1:1btQEpgT6k+unzCwX1KdWMEwPPkkgBtP+F6aCACiMrs= github.com/jcmturner/rpc/v2 v2.0.3 h1:7FXXj8Ti1IaVFpSAziCZWNzbNuZmnvw/i6CqLNdWfZY= github.com/jcmturner/rpc/v2 v2.0.3/go.mod h1:VUJYCIDm3PVOEHw8sgt091/20OJjskO/YJki3ELg/Hc= -github.com/notaryproject/notation-core-go v1.2.0-rc.1 h1:VMFlG+9a1JoNAQ3M96g8iqCq0cDRtE7XBaiTD8Ouvqw= -github.com/notaryproject/notation-core-go v1.2.0-rc.1/go.mod h1:b/70rA4OgOHlg0A7pb8zTWKJadFO6781zS3a37KHEJQ= +github.com/notaryproject/notation-core-go v1.2.0-rc.1.0.20241129024749-95d89543c9f9 h1:FURo9xpGLKmghWCcWypCPQTlcOGKxzayeXacGfb8WUU= +github.com/notaryproject/notation-core-go v1.2.0-rc.1.0.20241129024749-95d89543c9f9/go.mod h1:Umjn4NKGmuHpVffMgKVcUnArNG3Qtd3duKYpPILUBg4= github.com/notaryproject/notation-plugin-framework-go v1.0.0 h1:6Qzr7DGXoCgXEQN+1gTZWuJAZvxh3p8Lryjn5FaLzi4= github.com/notaryproject/notation-plugin-framework-go v1.0.0/go.mod h1:RqWSrTOtEASCrGOEffq0n8pSg2KOgKYiWqFWczRSics= -github.com/notaryproject/tspclient-go v0.2.0 h1:g/KpQGmyk/h7j60irIRG1mfWnibNOzJ8WhLqAzuiQAQ= -github.com/notaryproject/tspclient-go v0.2.0/go.mod h1:LGyA/6Kwd2FlM0uk8Vc5il3j0CddbWSHBj/4kxQDbjs= +github.com/notaryproject/tspclient-go v0.2.1-0.20241030015323-90a141e7525c h1:bX6gGxFw9+DShmYTgbD+vr6neF1SoXIMUU2fDgdLsfA= +github.com/notaryproject/tspclient-go v0.2.1-0.20241030015323-90a141e7525c/go.mod h1:LGyA/6Kwd2FlM0uk8Vc5il3j0CddbWSHBj/4kxQDbjs= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug= @@ -52,8 +52,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/veraison/go-cose v1.1.0 h1:AalPS4VGiKavpAzIlBjrn7bhqXiXi4jbMYY/2+UC+4o= -github.com/veraison/go-cose v1.1.0/go.mod h1:7ziE85vSq4ScFTg6wyoMXjucIGOf4JkFEZi/an96Ct4= +github.com/veraison/go-cose v1.3.0 h1:2/H5w8kdSpQJyVtIhx8gmwPJ2uSz1PkyWFx0idbd7rk= +github.com/veraison/go-cose v1.3.0/go.mod h1:df09OV91aHoQWLmy1KsDdYiagtXgyAwAl8vFeFn1gMc= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= @@ -62,12 +62,12 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= -golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A= -golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70= +golang.org/x/crypto v0.29.0 h1:L5SG1JTTXupVV3n6sUqMTeWbjAyfPwoda2DLX8J8FrQ= +golang.org/x/crypto v0.29.0/go.mod h1:+F4F4N5hv6v38hfeYwTdx20oUvLLc+QfrE9Ax9HtgRg= 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.21.0 h1:vvrHzRwRfVKSiLrG+d4FMl/Qi4ukBCE6kZlTUkDYRT0= -golang.org/x/mod v0.21.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= +golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= diff --git a/internal/file/file.go b/internal/file/file.go index c6e95849..464db1cc 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -119,8 +119,15 @@ func TrimFileExtension(fileName string) string { // WriteFile writes content to a temporary file and moves it to path. // If path already exists and is a file, WriteFile overwrites it. -func WriteFile(path string, content []byte) (writeErr error) { - tempFile, err := os.CreateTemp("", tempFileNamePrefix) +// +// Parameters: +// - tempDir is the directory to create the temporary file. It should be +// in the same mount point as path. If tempDir is empty, the default +// directory for temporary files is used. +// - path is the destination file path. +// - content is the content to write. +func WriteFile(tempDir, path string, content []byte) (writeErr error) { + tempFile, err := os.CreateTemp(tempDir, tempFileNamePrefix) if err != nil { return fmt.Errorf("failed to create temp file: %w", err) } diff --git a/internal/file/file_test.go b/internal/file/file_test.go index 306a0a5b..aeeea399 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -30,7 +30,7 @@ func TestCopyToDir(t *testing.T) { if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { t.Fatal(err) } - if err := WriteFile(filename, data); err != nil { + if err := WriteFile(tempDir, filename, data); err != nil { t.Fatal(err) } @@ -52,7 +52,7 @@ func TestCopyToDir(t *testing.T) { if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { t.Fatal(err) } - if err := WriteFile(filename, data); err != nil { + if err := WriteFile(tempDir, filename, data); err != nil { t.Fatal(err) } @@ -87,7 +87,7 @@ func TestCopyToDir(t *testing.T) { if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { t.Fatal(err) } - if err := WriteFile(filename, data); err != nil { + if err := WriteFile(tempDir, filename, data); err != nil { t.Fatal(err) } // forbid reading @@ -113,7 +113,7 @@ func TestCopyToDir(t *testing.T) { if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { t.Fatal(err) } - if err := WriteFile(filename, data); err != nil { + if err := WriteFile(tempDir, filename, data); err != nil { t.Fatal(err) } // forbid dest directory operation @@ -139,7 +139,7 @@ func TestCopyToDir(t *testing.T) { if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { t.Fatal(err) } - if err := WriteFile(filename, data); err != nil { + if err := WriteFile(tempDir, filename, data); err != nil { t.Fatal(err) } // forbid writing to destTempDir @@ -159,7 +159,7 @@ func TestCopyToDir(t *testing.T) { if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { t.Fatal(err) } - if err := WriteFile(filename, data); err != nil { + if err := WriteFile(tempDir, filename, data); err != nil { t.Fatal(err) } @@ -192,7 +192,7 @@ func TestWriteFile(t *testing.T) { if err != nil { t.Fatal(err) } - err = WriteFile(filepath.Join(tempDir, "testFile"), content) + err = WriteFile(tempDir, filepath.Join(tempDir, "testFile"), content) if err == nil || !strings.Contains(err.Error(), "permission denied") { t.Fatalf("expected permission denied error, but got %s", err) } diff --git a/internal/pkix/fuzz_test.go b/internal/pkix/fuzz_test.go new file mode 100644 index 00000000..2f6a6cc3 --- /dev/null +++ b/internal/pkix/fuzz_test.go @@ -0,0 +1,24 @@ +// 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 pkix + +import ( + "testing" +) + +func FuzzParseDistinguishedName(f *testing.F) { + f.Fuzz(func(t *testing.T, name string) { + _, _ = ParseDistinguishedName(name) + }) +} diff --git a/notation.go b/notation.go index d64fd2cf..294664b0 100644 --- a/notation.go +++ b/notation.go @@ -29,6 +29,7 @@ import ( orasRegistry "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote" + "github.com/notaryproject/notation-core-go/revocation" "github.com/notaryproject/notation-core-go/signature" "github.com/notaryproject/notation-core-go/signature/cose" "github.com/notaryproject/notation-core-go/signature/jws" @@ -67,6 +68,11 @@ type SignerSignOptions struct { // TSARootCAs is the cert pool holding caller's TSA trust anchor TSARootCAs *x509.CertPool + + // TSARevocationValidator is used for validating revocation status of + // timestamping certificate chain with context during signing. + // When present, only used when timestamping is performed. + TSARevocationValidator revocation.Validator } // Signer is a generic interface for signing an OCI artifact. @@ -128,7 +134,7 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts } // artifactRef is a tag logger.Warnf("Always sign the artifact using digest(`@sha256:...`) rather than a tag(`:%s`) because tags are mutable and a tag reference can point to a different artifact than the one signed", artifactRef) - logger.Infof("Resolved artifact tag `%s` to digest `%s` before signing", artifactRef, targetDesc.Digest.String()) + logger.Infof("Resolved artifact tag `%s` to digest `%v` before signing", artifactRef, targetDesc.Digest) } descToSign, err := addUserMetadataToDescriptor(ctx, targetDesc, signOpts.UserMetadata) if err != nil { @@ -372,7 +378,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve } if ref.ValidateReferenceAsDigest() != nil { // artifactRef is not a digest reference - logger.Infof("Resolved artifact tag `%s` to digest `%s` before verification", ref.Reference, artifactDescriptor.Digest.String()) + logger.Infof("Resolved artifact tag `%s` to digest `%v` before verification", ref.Reference, artifactDescriptor.Digest) logger.Warn("The resolved digest may not point to the same signed artifact, since tags are mutable") } else if ref.Reference != artifactDescriptor.Digest.String() { return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("user input digest %s does not match the resolved digest %s", ref.Reference, artifactDescriptor.Digest.String())} diff --git a/plugin/plugin.go b/plugin/plugin.go index e4579d85..71a8c895 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -226,6 +226,9 @@ func (c execCommander) Output(ctx context.Context, name string, command plugin.C cmd.Stdout = &stdout err := cmd.Run() if err != nil { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return nil, stderr.Bytes(), fmt.Errorf("'%s %s' command execution timeout: %w", name, string(command), err); + } return nil, stderr.Bytes(), err } return stdout.Bytes(), nil, nil diff --git a/plugin/plugin_test.go b/plugin/plugin_test.go index 171767b8..fc77d13a 100644 --- a/plugin/plugin_test.go +++ b/plugin/plugin_test.go @@ -19,9 +19,11 @@ import ( "errors" "os" "reflect" + "runtime" "strconv" "strings" "testing" + "time" "github.com/notaryproject/notation-go/plugin/proto" ) @@ -181,7 +183,7 @@ func TestValidateMetadata(t *testing.T) { } } -func TestNewCLIPlugin_PathError(t *testing.T) { +func TestNewCLIPlugin_Error(t *testing.T) { ctx := context.Background() t.Run("plugin directory exists without executable.", func(t *testing.T) { p, err := NewCLIPlugin(ctx, "emptyplugin", "./testdata/plugins/emptyplugin/notation-emptyplugin") @@ -203,6 +205,25 @@ func TestNewCLIPlugin_PathError(t *testing.T) { t.Errorf("NewCLIPlugin() plugin = %v, want nil", p) } }) + + t.Run("plugin timeout error", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + expectedErrMsg := "'sleep 2' command execution timeout: signal: killed" + ctxWithTimout, cancel := context.WithTimeout(ctx, 10 * time.Millisecond) + defer cancel() + + var twoSeconds proto.Command + twoSeconds = "2" + _, _, err := execCommander{}.Output(ctxWithTimout, "sleep", twoSeconds, nil); + if err == nil { + t.Errorf("execCommander{}.Output() expected error = %v, got nil", expectedErrMsg) + } + if err.Error() != expectedErrMsg { + t.Errorf("execCommander{}.Output() error = %v, want %v", err, expectedErrMsg) + } + }) } func TestNewCLIPlugin_ValidError(t *testing.T) { diff --git a/signer/signer.go b/signer/signer.go index 242c862e..1f7fe567 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -107,7 +107,6 @@ func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts if err != nil { return nil, nil, fmt.Errorf("envelope payload can't be marshalled: %w", err) } - var signingAgentId string if opts.SigningAgent != "" { signingAgentId = opts.SigningAgent @@ -125,12 +124,13 @@ func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts ContentType: envelope.MediaTypePayloadV1, Content: payloadBytes, }, - Signer: s.signer, - SigningTime: time.Now(), - SigningScheme: signature.SigningSchemeX509, - SigningAgent: signingAgentId, - Timestamper: opts.Timestamper, - TSARootCAs: opts.TSARootCAs, + Signer: s.signer, + SigningTime: time.Now(), + SigningScheme: signature.SigningSchemeX509, + SigningAgent: signingAgentId, + Timestamper: opts.Timestamper, + TSARootCAs: opts.TSARootCAs, + TSARevocationValidator: opts.TSARevocationValidator, } // Add expiry only if ExpiryDuration is not zero @@ -144,6 +144,12 @@ func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts logger.Debugf(" Expiry: %v", signReq.Expiry) logger.Debugf(" SigningScheme: %v", signReq.SigningScheme) logger.Debugf(" SigningAgent: %v", signReq.SigningAgent) + if signReq.Timestamper != nil { + logger.Debug("Enabled timestamping") + if signReq.TSARevocationValidator != nil { + logger.Debug("Enabled timestamping certificate chain revocation check") + } + } // Add ctx to the SignRequest signReq = signReq.WithContext(ctx) @@ -153,12 +159,10 @@ func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts if err != nil { return nil, nil, err } - sig, err := sigEnv.Sign(signReq) if err != nil { return nil, nil, err } - envContent, err := sigEnv.Verify() if err != nil { return nil, nil, fmt.Errorf("generated signature failed verification: %v", err) diff --git a/signer/signer_test.go b/signer/signer_test.go index 1dac5ebe..a2f8848f 100644 --- a/signer/signer_test.go +++ b/signer/signer_test.go @@ -30,6 +30,8 @@ import ( "testing" "time" + "github.com/notaryproject/notation-core-go/revocation" + "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/signature" _ "github.com/notaryproject/notation-core-go/signature/cose" _ "github.com/notaryproject/notation-core-go/signature/jws" @@ -257,6 +259,27 @@ func TestSignWithTimestamping(t *testing.T) { if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } + + // timestamping with unknown authority + desc, sOpts = generateSigningContent() + sOpts.SignatureMediaType = envelopeType + sOpts.Timestamper, err = tspclient.NewHTTPTimestamper(nil, rfc3161URL) + if err != nil { + t.Fatal(err) + } + sOpts.TSARootCAs = x509.NewCertPool() + tsaRevocationValidator, err := revocation.NewWithOptions(revocation.Options{ + CertChainPurpose: purpose.Timestamping, + }) + if err != nil { + t.Fatal(err) + } + sOpts.TSARevocationValidator = tsaRevocationValidator + _, _, err = s.Sign(ctx, desc, sOpts) + expectedErrMsg = "timestamp: failed to verify signed token: cms verification failure: x509: certificate signed by unknown authority" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err) + } } func TestSignWithoutExpiry(t *testing.T) { diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 9ebd16db..d7fdc27f 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -110,11 +110,11 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error // check expiry if err := checkExpiry(ctx, bundle.BaseCRL.NextUpdate); err != nil { - return nil, err + return nil, fmt.Errorf("check BaseCRL expiry failed: %w", err) } if bundle.DeltaCRL != nil { if err := checkExpiry(ctx, bundle.DeltaCRL.NextUpdate); err != nil { - return nil, err + return nil, fmt.Errorf("check DeltaCRL expiry failed: %w", err) } } @@ -144,7 +144,7 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) if err != nil { return fmt.Errorf("failed to store crl bundle in file cache: %w", err) } - if err := file.WriteFile(filepath.Join(c.root, c.fileName(url)), contentBytes); err != nil { + if err := file.WriteFile(c.root, filepath.Join(c.root, c.fileName(url)), contentBytes); err != nil { return fmt.Errorf("failed to store crl bundle in file cache: %w", err) } return nil diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index 6eafca3c..1d1a9dce 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -270,7 +270,7 @@ func TestGetFailed(t *testing.T) { t.Fatal(err) } _, err = cache.Get(ctx, "expiredKey") - expectedErrMsg := "crl bundle retrieved from file cache does not contain valid NextUpdate" + expectedErrMsg := "check BaseCRL expiry failed: crl bundle retrieved from file cache does not contain valid NextUpdate" if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %v", expectedErrMsg, err) } diff --git a/verifier/testdata/truststore/x509/tsa/test-mismatch/DigiCertTSARootSHA384.cer b/verifier/testdata/truststore/x509/tsa/test-mismatch/DigiCertTSARootSHA384.cer new file mode 100644 index 00000000..99bcc84b Binary files /dev/null and b/verifier/testdata/truststore/x509/tsa/test-mismatch/DigiCertTSARootSHA384.cer differ diff --git a/verifier/testdata/truststore/x509/tsa/test-mismatch/wabbit-networks.io.crt b/verifier/testdata/truststore/x509/tsa/test-nonCA/wabbit-networks.io.crt similarity index 100% rename from verifier/testdata/truststore/x509/tsa/test-mismatch/wabbit-networks.io.crt rename to verifier/testdata/truststore/x509/tsa/test-nonCA/wabbit-networks.io.crt diff --git a/verifier/testdata/truststore/x509/tsa/test-nonSelfIssued/nonSelfIssued.crt b/verifier/testdata/truststore/x509/tsa/test-nonSelfIssued/nonSelfIssued.crt new file mode 100644 index 00000000..6ec50052 Binary files /dev/null and b/verifier/testdata/truststore/x509/tsa/test-nonSelfIssued/nonSelfIssued.crt differ diff --git a/verifier/timestamp_test.go b/verifier/timestamp_test.go index f7eb90ec..22c75660 100644 --- a/verifier/timestamp_test.go +++ b/verifier/timestamp_test.go @@ -216,7 +216,7 @@ func TestAuthenticTimestamp(t *testing.T) { VerificationLevel: trustpolicy.LevelStrict, } authenticTimestampResult := verifyAuthenticTimestamp(context.Background(), dummyTrustPolicy.Name, dummyTrustPolicy.TrustStores, dummyTrustPolicy.SignatureVerification, trustStore, revocationTimestampingValidator, outcome) - expectedErrMsg := "failed to parse timestamp countersignature with error: unexpected content type: 1.2.840.113549.1.7.1" + expectedErrMsg := "failed to parse timestamp countersignature with error: unexpected content type: 1.2.840.113549.1.7.1. Expected to be id-ct-TSTInfo (1.2.840.113549.1.9.16.1.4)" if err := authenticTimestampResult.Error; err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } @@ -287,6 +287,29 @@ func TestAuthenticTimestamp(t *testing.T) { } }) + t.Run("verify Authentic Timestamp failed due to signing time after timestamp value", func(t *testing.T) { + signedToken, err := os.ReadFile("testdata/timestamp/countersignature/TimeStampToken.p7s") + if err != nil { + t.Fatalf("failed to get signedToken: %v", err) + } + envContent, err := parseEnvContent("testdata/timestamp/sigEnv/withoutTimestamp.sig", jws.MediaTypeEnvelope) + if err != nil { + t.Fatalf("failed to get signature envelope content: %v", err) + } + envContent.SignerInfo.UnsignedAttributes.TimestampSignature = signedToken + envContent.SignerInfo.Signature = []byte("notation") + envContent.SignerInfo.SignedAttributes.SigningTime = time.Date(3000, time.November, 10, 23, 0, 0, 0, time.UTC) + outcome := ¬ation.VerificationOutcome{ + EnvelopeContent: envContent, + VerificationLevel: trustpolicy.LevelStrict, + } + authenticTimestampResult := verifyAuthenticTimestamp(context.Background(), dummyTrustPolicy.Name, dummyTrustPolicy.TrustStores, dummyTrustPolicy.SignatureVerification, trustStore, revocationTimestampingValidator, outcome) + expectedErrMsg := "timestamp [2021-09-17T14:09:09Z, 2021-09-17T14:09:11Z] is not bounded after the signing time \"3000-11-10 23:00:00 +0000 UTC\"" + if err := authenticTimestampResult.Error; err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %s", expectedErrMsg, err) + } + }) + t.Run("verify Authentic Timestamp failed due to trust store does not exist", func(t *testing.T) { dummyTrustPolicy := &trustpolicy.TrustPolicy{ Name: "test-timestamp", diff --git a/verifier/truststore/truststore.go b/verifier/truststore/truststore.go index 067f5e63..067372c1 100644 --- a/verifier/truststore/truststore.go +++ b/verifier/truststore/truststore.go @@ -15,6 +15,7 @@ package truststore import ( + "bytes" "context" "crypto/x509" "errors" @@ -106,6 +107,14 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType if err := ValidateCertificates(certs); err != nil { 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)} } + // we require TSA certificates in trust store to be root CA certificates + if storeType == TypeTSA { + for _, cert := range certs { + if err := isRootCACertificate(cert); err != nil { + return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("trusted certificate %s in trust store %s of type %s is invalid: %v", certFileName, namedStore, storeType, err.Error())} + } + } + } certificates = append(certificates, certs...) } if len(certificates) < 1 { @@ -137,3 +146,14 @@ func ValidateCertificates(certs []*x509.Certificate) error { func isValidStoreType(storeType Type) bool { return slices.Contains(Types, storeType) } + +// isRootCACertificate returns nil if cert is a root CA certificate +func isRootCACertificate(cert *x509.Certificate) error { + if err := cert.CheckSignatureFrom(cert); err != nil { + return fmt.Errorf("certificate with subject %q is not a root CA certificate: %w", cert.Subject, err) + } + if !bytes.Equal(cert.RawSubject, cert.RawIssuer) { + return fmt.Errorf("certificate with subject %q is not a root CA certificate: issuer (%s) and subject (%s) are not the same", cert.Subject, cert.Issuer, cert.Subject) + } + return nil +} diff --git a/verifier/truststore/truststore_test.go b/verifier/truststore/truststore_test.go index 762553e5..3ee3c740 100644 --- a/verifier/truststore/truststore_test.go +++ b/verifier/truststore/truststore_test.go @@ -98,3 +98,31 @@ func TestValidateCertsWithLeafCert(t *testing.T) { t.Fatalf("leaf cert in a trust store should return error %q, got: %v", expectedErr, err) } } + +func TestGetCertFromValidTsaTrustStore(t *testing.T) { + // testing ../testdata/truststore/x509/tsa/test-nonCA/globalsignRoot.cer + _, err := trustStore.GetCertificates(context.Background(), "tsa", "test-timestamp") + if err != nil { + t.Fatalf("expected nil error, but got %s", err) + } +} + +func TestGetCertFromInvalidTsaTrustStore(t *testing.T) { + t.Run("non CA certificate", func(t *testing.T) { + // testing ../testdata/truststore/x509/tsa/test-nonCA/wabbit-networks.io + expectedErrMsg := `trusted certificate wabbit-networks.io.crt in trust store test-nonCA of type tsa is invalid: certificate with subject "CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US" is not a root CA certificate: x509: invalid signature: parent certificate cannot sign this kind of certificate` + _, err := trustStore.GetCertificates(context.Background(), "tsa", "test-nonCA") + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected error: %s, but got %s", expectedErrMsg, err) + } + }) + + t.Run("not self-issued", func(t *testing.T) { + //testing ../testdata/truststore/x509/tsa/test-nonSelfIssued/nonSelfIssued.crt + expectedErrMsg := `trusted certificate nonSelfIssued.crt in trust store test-nonSelfIssued of type tsa is invalid: certificate with subject "CN=Notation Test Revokable RSA Chain Cert 2,O=Notary,L=Seattle,ST=WA,C=US" is not a root CA certificate: issuer (CN=Notation Test Revokable RSA Chain Cert Root,O=Notary,L=Seattle,ST=WA,C=US) and subject (CN=Notation Test Revokable RSA Chain Cert 2,O=Notary,L=Seattle,ST=WA,C=US) are not the same` + _, err := trustStore.GetCertificates(context.Background(), "tsa", "test-nonSelfIssued") + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected error: %s, but got %s", expectedErrMsg, err) + } + }) +} diff --git a/verifier/verifier.go b/verifier/verifier.go index e852a96a..b50869ba 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -678,7 +678,7 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult, certResult := certResults[i] if certResult.RevocationMethod == revocationresult.RevocationMethodOCSPFallbackCRL { // log the fallback warning - logger.Warnf("OCSP check failed with unknown error and fallback to CRL check for certificate #%d in chain with subject %v", (i + 1), cert.Subject.String()) + logger.Warnf("OCSP check failed with unknown error and fallback to CRL check for certificate #%d in chain with subject %v", (i + 1), cert.Subject) } for _, serverResult := range certResult.ServerResults { if serverResult.Error != nil { @@ -687,10 +687,10 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult, // when the final revocation method is OCSPFallbackCRL, // the OCSP server results should not be logged as an error // since the CRL revocation check can succeed. - logger.Debugf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject.String(), revocationresult.RevocationMethodOCSP, serverResult.Server, serverResult.Error) + logger.Debugf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject, revocationresult.RevocationMethodOCSP, serverResult.Server, serverResult.Error) continue } - logger.Errorf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject.String(), serverResult.RevocationMethod, serverResult.Server, serverResult.Error) + logger.Errorf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject, serverResult.RevocationMethod, serverResult.Server, serverResult.Error) } } @@ -704,6 +704,10 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult, revokedCertSubject = problematicCertSubject } } + + if i < len(certResults)-1 && certResult.Result == revocationresult.ResultNonRevokable { + logger.Warnf("Certificate #%d in the chain with subject %v neither has an OCSP nor a CRL revocation method.", (i + 1), cert.Subject) + } } if revokedFound { problematicCertSubject = revokedCertSubject @@ -924,6 +928,9 @@ func verifyTimestamp(ctx context.Context, policyName string, trustStores []strin if err != nil { return fmt.Errorf("failed to verify the timestamp countersignature with error: %w", err) } + if !timestamp.BoundedAfter(signerInfo.SignedAttributes.SigningTime) { + return fmt.Errorf("timestamp %s is not bounded after the signing time %q", timestamp.Format(time.RFC3339), signerInfo.SignedAttributes.SigningTime) + } // 3. Validate timestamping certificate chain logger.Debug("Validating timestamping certificate chain...")