From 73c295816799bc3ed03cfb6ded7a0d8efd4a5781 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Thu, 19 Sep 2024 09:53:46 +0000 Subject: [PATCH] fix: update Signed-off-by: Junjie Gao --- revocation/revocation.go | 27 ++++++------- revocation/revocation_test.go | 73 +++++++++++++++++++++++------------ 2 files changed, 60 insertions(+), 40 deletions(-) diff --git a/revocation/revocation.go b/revocation/revocation.go index dd260768..f249d915 100644 --- a/revocation/revocation.go +++ b/revocation/revocation.go @@ -101,14 +101,10 @@ type Options struct { // OPTIONAL. OCSPHTTPClient *http.Client - // CRLHTTPClient is the HTTP client for CRL request. If not provided, - // a default *http.Client with timeout of 5 seconds will be used. - // OPTIONAL. - CRLHTTPClient *http.Client - - // CRLCache is the cache client used to store the CRL. if not provided, - // no cache will be used. - CRLCache crlutil.Cache + // CRLFetcher is a fetcher for CRL with cache. If not provided, a default + // fetcher with an HTTP client and a timeout of 5 seconds will be used + // without cache. + CRLFetcher crlutil.Fetcher // CertChainPurpose is the purpose of the certificate chain. Supported // values are CodeSigning and Timestamping. Default value is CodeSigning. @@ -122,8 +118,13 @@ func NewWithOptions(opts Options) (Validator, error) { opts.OCSPHTTPClient = &http.Client{Timeout: 2 * time.Second} } - if opts.CRLHTTPClient == nil { - opts.CRLHTTPClient = &http.Client{Timeout: 5 * time.Second} + fetcher := opts.CRLFetcher + if fetcher == nil { + newFetcher, err := crlutil.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) + if err != nil { + return nil, err + } + fetcher = newFetcher } switch opts.CertChainPurpose { @@ -132,12 +133,6 @@ func NewWithOptions(opts Options) (Validator, error) { return nil, fmt.Errorf("unsupported certificate chain purpose %v", opts.CertChainPurpose) } - fetcher, err := crlutil.NewHTTPFetcher(opts.CRLHTTPClient) - if err != nil { - return nil, err - } - fetcher.Cache = opts.CRLCache - return &revocation{ ocspHTTPClient: opts.OCSPHTTPClient, crlFetcher: fetcher, diff --git a/revocation/revocation_test.go b/revocation/revocation_test.go index 2ac8b4c9..00e9597d 100644 --- a/revocation/revocation_test.go +++ b/revocation/revocation_test.go @@ -28,6 +28,7 @@ import ( "testing" "time" + "github.com/notaryproject/notation-core-go/revocation/crl" revocationocsp "github.com/notaryproject/notation-core-go/revocation/internal/ocsp" "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-core-go/revocation/result" @@ -1035,15 +1036,20 @@ func TestCRL(t *testing.T) { t.Run("CRL check valid", func(t *testing.T) { chain := testhelper.GetRevokableRSAChainWithRevocations(3, false, true) - revocationClient, err := NewWithOptions(Options{ - CRLHTTPClient: &http.Client{ - Timeout: 5 * time.Second, - Transport: &crlRoundTripper{ - CertChain: chain, - Revoked: false, - }, + fetcher, err := crl.NewHTTPFetcher(&http.Client{ + Timeout: 5 * time.Second, + Transport: &crlRoundTripper{ + CertChain: chain, + Revoked: false, }, + }) + if err != nil { + t.Errorf("Expected successful creation of fetcher, but received error: %v", err) + } + + revocationClient, err := NewWithOptions(Options{ OCSPHTTPClient: &http.Client{}, + CRLFetcher: fetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil { @@ -1084,15 +1090,20 @@ func TestCRL(t *testing.T) { t.Run("CRL check with revoked status", func(t *testing.T) { chain := testhelper.GetRevokableRSAChainWithRevocations(3, false, true) - revocationClient, err := NewWithOptions(Options{ - CRLHTTPClient: &http.Client{ - Timeout: 5 * time.Second, - Transport: &crlRoundTripper{ - CertChain: chain, - Revoked: true, - }, + fetcher, err := crl.NewHTTPFetcher(&http.Client{ + Timeout: 5 * time.Second, + Transport: &crlRoundTripper{ + CertChain: chain, + Revoked: true, }, + }) + if err != nil { + t.Errorf("Expected successful creation of fetcher, but received error: %v", err) + } + + revocationClient, err := NewWithOptions(Options{ OCSPHTTPClient: &http.Client{}, + CRLFetcher: fetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil { @@ -1140,17 +1151,21 @@ func TestCRL(t *testing.T) { t.Run("OCSP fallback to CRL", func(t *testing.T) { chain := testhelper.GetRevokableRSAChainWithRevocations(3, true, true) + fetcher, err := crl.NewHTTPFetcher(&http.Client{ + Timeout: 5 * time.Second, + Transport: &crlRoundTripper{ + CertChain: chain, + Revoked: true, + FailOCSP: true, + }, + }) + if err != nil { + t.Errorf("Expected successful creation of fetcher, but received error: %v", err) + } revocationClient, err := NewWithOptions(Options{ - CRLHTTPClient: &http.Client{ - Timeout: 5 * time.Second, - Transport: &crlRoundTripper{ - CertChain: chain, - Revoked: true, - FailOCSP: true, - }, - }, OCSPHTTPClient: &http.Client{}, + CRLFetcher: fetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil { @@ -1218,9 +1233,14 @@ func TestPanicHandling(t *testing.T) { Transport: panicTransport{}, } + fetcher, err := crl.NewHTTPFetcher(client) + if err != nil { + t.Errorf("Expected successful creation of fetcher, but received error: %v", err) + } + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CRLHTTPClient: client, + CRLFetcher: fetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil { @@ -1245,9 +1265,14 @@ func TestPanicHandling(t *testing.T) { Transport: panicTransport{}, } + fetcher, err := crl.NewHTTPFetcher(client) + if err != nil { + t.Errorf("Expected successful creation of fetcher, but received error: %v", err) + } + r, err := NewWithOptions(Options{ OCSPHTTPClient: client, - CRLHTTPClient: client, + CRLFetcher: fetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil {