diff --git a/cmd/notation/blob/sign.go b/cmd/notation/blob/sign.go index d814d87c6..4e8e365b6 100644 --- a/cmd/notation/blob/sign.go +++ b/cmd/notation/blob/sign.go @@ -239,7 +239,7 @@ func prepareBlobSigningOpts(ctx context.Context, opts *blobSignOpts) (notation.S if err != nil { return notation.SignBlobOptions{}, err } - tsaRevocationValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping) + tsaRevocationValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping, clirev.DefaultOCSPTimeout) if err != nil { return notation.SignBlobOptions{}, fmt.Errorf("failed to create timestamping revocation validator: %w", err) } diff --git a/cmd/notation/blob/verify.go b/cmd/notation/blob/verify.go index 2b2164815..0c4759c38 100644 --- a/cmd/notation/blob/verify.go +++ b/cmd/notation/blob/verify.go @@ -19,6 +19,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/notaryproject/notation-go" "github.com/notaryproject/notation/v2/cmd/notation/internal/display" @@ -26,6 +27,7 @@ import ( "github.com/notaryproject/notation/v2/cmd/notation/internal/flag" "github.com/notaryproject/notation/v2/cmd/notation/internal/verify" "github.com/notaryproject/notation/v2/internal/envelope" + clirev "github.com/notaryproject/notation/v2/internal/revocation" "github.com/spf13/cobra" ) @@ -38,6 +40,7 @@ type blobVerifyOpts struct { userMetadata []string policyStatementName string blobMediaType string + ocspTimeout time.Duration } func verifyCommand(opts *blobVerifyOpts) *cobra.Command { @@ -90,6 +93,7 @@ Example - Verify the signature on a blob artifact using a policy statement name: command.Flags().StringArrayVar(&opts.pluginConfig, "plugin-config", nil, "{key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values") command.Flags().StringVar(&opts.blobMediaType, "media-type", "", "media type of the blob to verify") command.Flags().StringVar(&opts.policyStatementName, "policy-name", "", "policy name to verify against. If not provided, the global policy is used if exists") + command.Flags().DurationVar(&opts.ocspTimeout, "ocsp-timeout", clirev.DefaultOCSPTimeout, "timeout for OCSP requests during certificate revocation checking") flag.SetPflagUserMetadata(command.Flags(), &opts.userMetadata, flag.PflagUserMetadataVerifyUsage) command.MarkFlagRequired("signature") return command @@ -111,7 +115,7 @@ func runVerify(command *cobra.Command, cmdOpts *blobVerifyOpts) error { if err != nil { return err } - blobVerifier, err := verify.GetBlobVerifier(ctx) + blobVerifier, err := verify.GetBlobVerifier(ctx, cmdOpts.ocspTimeout) if err != nil { return err } diff --git a/cmd/notation/blob/verify_test.go b/cmd/notation/blob/verify_test.go index b3074ccfd..e65e94803 100644 --- a/cmd/notation/blob/verify_test.go +++ b/cmd/notation/blob/verify_test.go @@ -16,6 +16,8 @@ package blob import ( "reflect" "testing" + + clirev "github.com/notaryproject/notation/v2/internal/revocation" ) func TestVerifyCommand_BasicArgs(t *testing.T) { @@ -24,6 +26,7 @@ func TestVerifyCommand_BasicArgs(t *testing.T) { expected := &blobVerifyOpts{ blobPath: "blob_path", signaturePath: "sig_path", + ocspTimeout: clirev.DefaultOCSPTimeout, } if err := command.ParseFlags([]string{ expected.blobPath, @@ -45,6 +48,7 @@ func TestVerifyCommand_MoreArgs(t *testing.T) { blobPath: "blob_path", signaturePath: "sig_path", pluginConfig: []string{"key1=val1", "key2=val2"}, + ocspTimeout: clirev.DefaultOCSPTimeout, } if err := command.ParseFlags([]string{ expected.blobPath, diff --git a/cmd/notation/internal/verify/verify.go b/cmd/notation/internal/verify/verify.go index eac6b4279..221be7137 100644 --- a/cmd/notation/internal/verify/verify.go +++ b/cmd/notation/internal/verify/verify.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "io/fs" + "time" "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-go" @@ -38,8 +39,8 @@ type Verifier interface { } // GetVerifier creates a Verifier. -func GetVerifier(ctx context.Context) (Verifier, error) { - verifierOptions, err := newVerifierOptions(ctx) +func GetVerifier(ctx context.Context, ocspTimeout time.Duration) (Verifier, error) { + verifierOptions, err := newVerifierOptions(ctx, ocspTimeout) if err != nil { return nil, err } @@ -55,8 +56,8 @@ func GetVerifier(ctx context.Context) (Verifier, error) { } // GetBlobVerifier creates a BlobVerifier. -func GetBlobVerifier(ctx context.Context) (Verifier, error) { - verifierOptions, err := newVerifierOptions(ctx) +func GetBlobVerifier(ctx context.Context, ocspTimeout time.Duration) (Verifier, error) { + verifierOptions, err := newVerifierOptions(ctx, ocspTimeout) if err != nil { return nil, err } @@ -72,12 +73,12 @@ func GetBlobVerifier(ctx context.Context) (Verifier, error) { } // newVerifierOptions creates a verifier.VerifierOptions. -func newVerifierOptions(ctx context.Context) (verifier.VerifierOptions, error) { - revocationCodeSigningValidator, err := clirev.NewRevocationValidator(ctx, purpose.CodeSigning) +func newVerifierOptions(ctx context.Context, ocspTimeout time.Duration) (verifier.VerifierOptions, error) { + revocationCodeSigningValidator, err := clirev.NewRevocationValidator(ctx, purpose.CodeSigning, ocspTimeout) if err != nil { return verifier.VerifierOptions{}, err } - revocationTimestampingValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping) + revocationTimestampingValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping, ocspTimeout) if err != nil { return verifier.VerifierOptions{}, err } diff --git a/cmd/notation/internal/verify/verify_test.go b/cmd/notation/internal/verify/verify_test.go index e7c6a866b..adcc92127 100644 --- a/cmd/notation/internal/verify/verify_test.go +++ b/cmd/notation/internal/verify/verify_test.go @@ -23,6 +23,7 @@ import ( "github.com/notaryproject/notation-go" "github.com/notaryproject/notation-go/dir" "github.com/notaryproject/notation-go/verifier/trustpolicy" + clirev "github.com/notaryproject/notation/v2/internal/revocation" ) func TestGetVerifier(t *testing.T) { @@ -41,7 +42,7 @@ func TestGetVerifier(t *testing.T) { } t.Cleanup(func() { os.RemoveAll(tempRoot) }) - _, err := GetVerifier(context.Background()) + _, err := GetVerifier(context.Background(), clirev.DefaultOCSPTimeout) if err != nil { t.Fatal(err) } @@ -50,7 +51,7 @@ func TestGetVerifier(t *testing.T) { t.Run("non-existing oci trust policy", func(t *testing.T) { dir.UserConfigDir = "/" expectedErrMsg := "trust policy is not present. To create a trust policy, see: https://notaryproject.dev/docs/quickstart/#create-a-trust-policy" - _, err := GetVerifier(context.Background()) + _, err := GetVerifier(context.Background(), clirev.DefaultOCSPTimeout) if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } @@ -67,7 +68,7 @@ func TestGetVerifier(t *testing.T) { t.Cleanup(func() { os.RemoveAll(tempRoot) }) expectedErrMsg := "oci trust policy document has empty version, version must be specified" - _, err := GetVerifier(context.Background()) + _, err := GetVerifier(context.Background(), clirev.DefaultOCSPTimeout) if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } @@ -90,7 +91,7 @@ func TestGetBlobVerifier(t *testing.T) { } t.Cleanup(func() { os.RemoveAll(tempRoot) }) - _, err := GetBlobVerifier(context.Background()) + _, err := GetBlobVerifier(context.Background(), clirev.DefaultOCSPTimeout) if err != nil { t.Fatal(err) } @@ -99,7 +100,7 @@ func TestGetBlobVerifier(t *testing.T) { t.Run("non-existing blob trust policy", func(t *testing.T) { dir.UserConfigDir = "/" expectedErrMsg := "trust policy is not present. To create a trust policy, see: https://notaryproject.dev/docs/quickstart/#create-a-trust-policy" - _, err := GetBlobVerifier(context.Background()) + _, err := GetBlobVerifier(context.Background(), clirev.DefaultOCSPTimeout) if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } @@ -116,7 +117,7 @@ func TestGetBlobVerifier(t *testing.T) { t.Cleanup(func() { os.RemoveAll(tempRoot) }) expectedErrMsg := "blob trust policy document has empty version, version must be specified" - _, err := GetBlobVerifier(context.Background()) + _, err := GetBlobVerifier(context.Background(), clirev.DefaultOCSPTimeout) if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %s", expectedErrMsg, err) } diff --git a/cmd/notation/sign.go b/cmd/notation/sign.go index 92ddd566b..1e345f350 100644 --- a/cmd/notation/sign.go +++ b/cmd/notation/sign.go @@ -222,7 +222,7 @@ func prepareSigningOpts(ctx context.Context, opts *signOpts) (notation.SignOptio if err != nil { return notation.SignOptions{}, err } - tsaRevocationValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping) + tsaRevocationValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping, clirev.DefaultOCSPTimeout) if err != nil { return notation.SignOptions{}, fmt.Errorf("failed to create timestamping revocation validator: %w", err) } diff --git a/cmd/notation/verify.go b/cmd/notation/verify.go index a131754a0..b251f4a63 100644 --- a/cmd/notation/verify.go +++ b/cmd/notation/verify.go @@ -16,6 +16,7 @@ package main import ( "errors" "fmt" + "time" "github.com/notaryproject/notation-go" "github.com/notaryproject/notation/v2/cmd/notation/internal/display" @@ -23,6 +24,7 @@ import ( "github.com/notaryproject/notation/v2/cmd/notation/internal/experimental" "github.com/notaryproject/notation/v2/cmd/notation/internal/flag" "github.com/notaryproject/notation/v2/cmd/notation/internal/verify" + clirev "github.com/notaryproject/notation/v2/internal/revocation" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/cobra" ) @@ -38,6 +40,7 @@ type verifyOpts struct { trustPolicyScope string inputType inputType maxSignatureAttempts int + ocspTimeout time.Duration } func verifyCommand(opts *verifyOpts) *cobra.Command { @@ -93,6 +96,7 @@ Example - [Experimental] Verify a signature on an OCI artifact identified by a t command.Flags().StringArrayVar(&opts.pluginConfig, "plugin-config", nil, "{key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values") flag.SetPflagUserMetadata(command.Flags(), &opts.userMetadata, flag.PflagUserMetadataVerifyUsage) command.Flags().IntVar(&opts.maxSignatureAttempts, "max-signatures", 100, "maximum number of signatures to evaluate or examine") + command.Flags().DurationVar(&opts.ocspTimeout, "ocsp-timeout", clirev.DefaultOCSPTimeout, "timeout for OCSP requests during certificate revocation checking") command.Flags().BoolVar(&opts.ociLayout, "oci-layout", false, "[Experimental] verify the artifact stored as OCI image layout") command.Flags().StringVar(&opts.trustPolicyScope, "scope", "", "[Experimental] set trust policy scope for artifact verification, required and can only be used when flag \"--oci-layout\" is set") command.MarkFlagsRequiredTogether("oci-layout", "scope") @@ -106,7 +110,7 @@ func runVerify(command *cobra.Command, opts *verifyOpts) error { // initialize displayHandler := display.NewVerifyHandler(opts.printer) - sigVerifier, err := verify.GetVerifier(ctx) + sigVerifier, err := verify.GetVerifier(ctx, opts.ocspTimeout) if err != nil { return err } diff --git a/cmd/notation/verify_test.go b/cmd/notation/verify_test.go index 6744f03e1..5bdcc7c27 100644 --- a/cmd/notation/verify_test.go +++ b/cmd/notation/verify_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/notaryproject/notation/v2/cmd/notation/internal/flag" + clirev "github.com/notaryproject/notation/v2/internal/revocation" ) func TestVerifyCommand_BasicArgs(t *testing.T) { @@ -31,6 +32,7 @@ func TestVerifyCommand_BasicArgs(t *testing.T) { }, pluginConfig: []string{"key1=val1"}, maxSignatureAttempts: 100, + ocspTimeout: clirev.DefaultOCSPTimeout, } if err := command.ParseFlags([]string{ expected.reference, @@ -57,6 +59,7 @@ func TestVerifyCommand_MoreArgs(t *testing.T) { }, pluginConfig: []string{"key1=val1", "key2=val2"}, maxSignatureAttempts: 100, + ocspTimeout: clirev.DefaultOCSPTimeout, } if err := command.ParseFlags([]string{ expected.reference, diff --git a/internal/revocation/revocation.go b/internal/revocation/revocation.go index 32a8c4b55..c7523fc46 100644 --- a/internal/revocation/revocation.go +++ b/internal/revocation/revocation.go @@ -29,9 +29,13 @@ import ( clicrl "github.com/notaryproject/notation/v2/internal/revocation/crl" ) +// DefaultOCSPTimeout is the default timeout for OCSP requests issued during +// certificate revocation checking. +const DefaultOCSPTimeout = 2 * time.Second + // NewRevocationValidator returns a revocation.Validator given the certificate -// purpose -func NewRevocationValidator(ctx context.Context, purpose purpose.Purpose) (revocation.Validator, error) { +// purpose and the timeout for OCSP requests. +func NewRevocationValidator(ctx context.Context, purpose purpose.Purpose, ocspTimeout time.Duration) (revocation.Validator, error) { // err is always nil crlFetcher, _ := corecrl.NewHTTPFetcher(httputil.NewClient(ctx, &http.Client{Timeout: 5 * time.Second})) crlFetcher.DiscardCacheError = true // discard crl cache error @@ -47,7 +51,7 @@ func NewRevocationValidator(ctx context.Context, purpose purpose.Purpose) (revoc } } return revocation.NewWithOptions(revocation.Options{ - OCSPHTTPClient: httputil.NewClient(ctx, &http.Client{Timeout: 2 * time.Second}), + OCSPHTTPClient: httputil.NewClient(ctx, &http.Client{Timeout: ocspTimeout}), CRLFetcher: crlFetcher, CertChainPurpose: purpose, }) diff --git a/internal/revocation/revocation_test.go b/internal/revocation/revocation_test.go index a1454f3ff..57e7aec17 100644 --- a/internal/revocation/revocation_test.go +++ b/internal/revocation/revocation_test.go @@ -36,7 +36,7 @@ func TestNewRevocationValidator(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("skipping test on Windows") } - if _, err := NewRevocationValidator(context.Background(), purpose.Timestamping); err != nil { + if _, err := NewRevocationValidator(context.Background(), purpose.Timestamping, DefaultOCSPTimeout); err != nil { t.Fatal(err) } }) @@ -56,7 +56,7 @@ func TestNewRevocationValidator(t *testing.T) { t.Fatalf("failed to change permission: %v", err) } }() - if _, err := NewRevocationValidator(context.Background(), purpose.Timestamping); err != nil { + if _, err := NewRevocationValidator(context.Background(), purpose.Timestamping, DefaultOCSPTimeout); err != nil { t.Fatal(err) } }) diff --git a/specs/cmd/blob.md b/specs/cmd/blob.md index 782bcecee..af0905451 100644 --- a/specs/cmd/blob.md +++ b/specs/cmd/blob.md @@ -179,6 +179,7 @@ Flags: -d, --debug debug mode -h, --help help for verify --media-type string media type of the blob to verify + --ocsp-timeout duration timeout for OCSP requests during certificate revocation checking (default 2s) --plugin-config stringArray {key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values --policy-name string policy name to verify against. If not provided, the global policy is used if exists -s --signature string filepath of the signature to be verified diff --git a/specs/cmd/verify.md b/specs/cmd/verify.md index dba173537..7a4296829 100644 --- a/specs/cmd/verify.md +++ b/specs/cmd/verify.md @@ -43,6 +43,7 @@ Flags: --insecure-registry use HTTP protocol while connecting to registries. Should be used only for testing --max-signatures int maximum number of signatures to evaluate or examine (default 100) --oci-layout [Experimental] verify the artifact stored as OCI image layout + --ocsp-timeout duration timeout for OCSP requests during certificate revocation checking (default 2s) -p, --password string password for registry operations (default to $NOTATION_PASSWORD if not specified) --plugin-config stringArray {key}={value} pairs that are passed as it is to a plugin, if the verification is associated with a verification plugin, refer plugin documentation to set appropriate values --scope string [Experimental] set trust policy scope for artifact verification, required and can only be used when flag "--oci-layout" is set