From 169aead2647c0909803565308864adfea67c0ef1 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 12 Nov 2024 06:40:55 +0000 Subject: [PATCH] fix: add test cases for expired crl Signed-off-by: Junjie Gao --- cmd/notation/verify.go | 4 +- test/e2e/go.mod | 1 + test/e2e/go.sum | 2 + test/e2e/internal/notation/file.go | 4 +- test/e2e/internal/notation/host.go | 6 +- test/e2e/internal/notation/key.go | 4 +- test/e2e/internal/utils/crl.go | 12 +++ test/e2e/scripts/crl_server.py | 79 ++++++++--------- test/e2e/suite/scenario/crl.go | 82 ++++++++++++++++++ test/e2e/testdata/config/crl/leaf_expired.crl | Bin 0 -> 493 bytes 10 files changed, 141 insertions(+), 53 deletions(-) create mode 100644 test/e2e/testdata/config/crl/leaf_expired.crl diff --git a/cmd/notation/verify.go b/cmd/notation/verify.go index 7b78dfe76..ceb91c533 100644 --- a/cmd/notation/verify.go +++ b/cmd/notation/verify.go @@ -27,7 +27,6 @@ import ( "github.com/notaryproject/notation-core-go/revocation/purpose" "github.com/notaryproject/notation-go" "github.com/notaryproject/notation-go/dir" - "github.com/notaryproject/notation-go/log" "github.com/notaryproject/notation-go/plugin" "github.com/notaryproject/notation-go/verifier" "github.com/notaryproject/notation-go/verifier/crl" @@ -234,7 +233,6 @@ func printMetadataIfPresent(outcome *notation.VerificationOutcome) { } func getVerifier(ctx context.Context) (notation.Verifier, error) { - logger := log.GetLogger(ctx) // revocation check ocspHttpClient := httputil.NewClient(ctx, &http.Client{Timeout: 2 * time.Second}) crlFetcher, err := corecrl.NewHTTPFetcher(httputil.NewClient(ctx, &http.Client{Timeout: 5 * time.Second})) @@ -249,7 +247,7 @@ func getVerifier(ctx context.Context) (notation.Verifier, error) { fileCache, err := crl.NewFileCache(cacheRoot) if err != nil { // discard NewFileCache error directly as cache is not critical - logger.Debugf("failed to create file cache: %v", err) + fmt.Print("Warning:", err) } else { crlFetcher.Cache = &clicrl.CacheWithLog{ Cache: fileCache, diff --git a/test/e2e/go.mod b/test/e2e/go.mod index 05ef045c3..1e0b2fd02 100644 --- a/test/e2e/go.mod +++ b/test/e2e/go.mod @@ -4,6 +4,7 @@ go 1.23 require ( github.com/notaryproject/notation-core-go v1.2.0-rc.1 + github.com/notaryproject/notation-go v1.2.0-beta.1.0.20240926015724-84c2ec076201 github.com/onsi/ginkgo/v2 v2.21.0 github.com/onsi/gomega v1.34.2 github.com/opencontainers/image-spec v1.1.0 diff --git a/test/e2e/go.sum b/test/e2e/go.sum index 57ca73c8a..707618328 100644 --- a/test/e2e/go.sum +++ b/test/e2e/go.sum @@ -12,6 +12,8 @@ github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db h1:097atOisP2aRj7vFgY github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db/go.mod h1:vavhavw2zAxS5dIdcRluK6cSGGPlZynqzFM8NdvU144= 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-go v1.2.0-beta.1.0.20240926015724-84c2ec076201 h1:2QBYa9Df+vMwMiaHaFqPoUiwfx5vcPEgM7KbusivTpw= +github.com/notaryproject/notation-go v1.2.0-beta.1.0.20240926015724-84c2ec076201/go.mod h1:F6zMQl3PhVdCsI1xlIjK66kCorUQhWkoMtlZdvJWxFI= 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/onsi/ginkgo/v2 v2.21.0 h1:7rg/4f3rB88pb5obDgNZrNHrQ4e6WpjonchcpuBRnZM= diff --git a/test/e2e/internal/notation/file.go b/test/e2e/internal/notation/file.go index 049f36a9a..3dc6d7a4d 100644 --- a/test/e2e/internal/notation/file.go +++ b/test/e2e/internal/notation/file.go @@ -19,8 +19,8 @@ import ( "os" ) -// copyFile copies the source file to the destination file -func copyFile(src, dst string) error { +// CopyFile copies the source file to the destination file +func CopyFile(src, dst string) error { in, err := os.Open(src) if err != nil { return err diff --git a/test/e2e/internal/notation/host.go b/test/e2e/internal/notation/host.go index 0ad17854f..8128cc5be 100644 --- a/test/e2e/internal/notation/host.go +++ b/test/e2e/internal/notation/host.go @@ -231,7 +231,7 @@ func AddTimestampTrustStoreOption(namedstore string, srcCertPath string) utils.H // AddTrustPolicyOption adds a valid trust policy for testing. func AddTrustPolicyOption(trustpolicyName string) utils.HostOption { return func(vhost *utils.VirtualHost) error { - return copyFile( + return CopyFile( filepath.Join(NotationE2ETrustPolicyDir, trustpolicyName), vhost.AbsolutePath(NotationDirName, TrustPolicyName), ) @@ -241,7 +241,7 @@ func AddTrustPolicyOption(trustpolicyName string) utils.HostOption { // AddConfigJsonOption adds a valid config.json for testing. func AddConfigJsonOption(configJsonName string) utils.HostOption { return func(vhost *utils.VirtualHost) error { - return copyFile( + return CopyFile( filepath.Join(NotationE2EConfigJsonDir, configJsonName), vhost.AbsolutePath(NotationDirName, ConfigJsonName), ) @@ -262,7 +262,7 @@ func AddPlugin(pluginPath string) utils.HostOption { if err := os.MkdirAll(e2ePluginDir, 0700); err != nil { return err } - return copyFile( + return CopyFile( NotationE2EPluginPath, filepath.Join(e2ePluginDir, "notation-"+PluginName), ) diff --git a/test/e2e/internal/notation/key.go b/test/e2e/internal/notation/key.go index fd11a3852..efa9f5812 100644 --- a/test/e2e/internal/notation/key.go +++ b/test/e2e/internal/notation/key.go @@ -68,10 +68,10 @@ func AddKeyPairs(destNotationConfigDir, srcKeyPath, srcCertPath string) error { os.MkdirAll(localKeysDir, 0700) // copy key and cert files - if err := copyFile(srcKeyPath, filepath.Join(localKeysDir, keyName)); err != nil { + if err := CopyFile(srcKeyPath, filepath.Join(localKeysDir, keyName)); err != nil { return err } - return copyFile(srcCertPath, filepath.Join(localKeysDir, certName)) + return CopyFile(srcCertPath, filepath.Join(localKeysDir, certName)) } // generateSigningKeys generates the signingkeys.json for notation. diff --git a/test/e2e/internal/utils/crl.go b/test/e2e/internal/utils/crl.go index 34a3d29c3..36bc8d0f8 100644 --- a/test/e2e/internal/utils/crl.go +++ b/test/e2e/internal/utils/crl.go @@ -42,6 +42,18 @@ func LeafCRLUnrevoke() error { return nil } +// LeafCRLExpired sends http post request to http://localhost:10086/leaf/expired +func LeafCRLExpired() error { + url := "http://localhost:10086/leaf/expired" + resp, err := http.Post(url, "application/json", nil) + if err != nil { + return err + } + defer resp.Body.Close() + fmt.Printf("CRL of leaf certificate expired with status code: %d\n", resp.StatusCode) + return nil +} + // IntermediateCRLRevoke sends http post request to http://localhost:10086/intermediate/revoke func IntermediateCRLRevoke() error { url := "http://localhost:10086/intermediate/revoke" diff --git a/test/e2e/scripts/crl_server.py b/test/e2e/scripts/crl_server.py index bb2c48fa7..dea91d699 100644 --- a/test/e2e/scripts/crl_server.py +++ b/test/e2e/scripts/crl_server.py @@ -17,65 +17,58 @@ PORT = 10086 DATA_DIR = './testdata/config/crl' -leaf_revoke_flag = False -intermediate_revoke_flag = False +leaf_crl = 'leaf.crl' +intermediate_crl = 'intermediate.crl' class CRLRequestHandler(http.server.SimpleHTTPRequestHandler): def do_GET(self): - global leaf_revoke_flag - global intermediate_revoke_flag + global leaf_crl + global intermediate_crl if self.path == '/leaf.crl': - file_name = 'leaf_revoked.crl' if leaf_revoke_flag else 'leaf.crl' - file_path = os.path.join(DATA_DIR, file_name) - print("crl path", file_path) - if os.path.exists(file_path): - self.send_response(200) - self.send_header('Content-Type', 'application/pkix-crl') - self.end_headers() - with open(file_path, 'rb') as f: - self.wfile.write(f.read()) - else: - self.send_error(404, 'File Not Found') + file_path = os.path.join(DATA_DIR, leaf_crl) + self.crl_response(file_path) elif self.path == '/intermediate.crl': - file_name = 'intermediate_revoked.crl' if intermediate_revoke_flag else 'intermediate.crl' - file_path = os.path.join(DATA_DIR, file_name) - if os.path.exists(file_path): - self.send_response(200) - self.send_header('Content-Type', 'application/pkix-crl') - self.end_headers() - with open(file_path, 'rb') as f: - self.wfile.write(f.read()) - else: - self.send_error(404, 'File Not Found') + file_path = os.path.join(DATA_DIR, intermediate_crl) + self.crl_response(file_path) else: self.send_error(404, 'Not Found') + + def crl_response(self, file_path): + if os.path.exists(file_path): + self.send_response(200) + self.send_header('Content-Type', 'application/pkix-crl') + self.end_headers() + with open(file_path, 'rb') as f: + self.wfile.write(f.read()) + else: + self.send_error(404, 'File Not Found') def do_POST(self): - global leaf_revoke_flag - global intermediate_revoke_flag + global leaf_crl + global intermediate_crl if self.path == '/leaf/revoke': - leaf_revoke_flag = True - self.send_response(201) - self.end_headers() - self.wfile.write(b'ok') + leaf_crl = 'leaf_revoked.crl' + self.post_response() elif self.path == '/leaf/unrevoke': - leaf_revoke_flag = False - self.send_response(201) - self.end_headers() - self.wfile.write(b'ok') + leaf_crl = 'leaf.crl' + self.post_response() + elif self.path == '/leaf/expired': + leaf_crl = 'leaf_expired.crl' + self.post_response() elif self.path == '/intermediate/revoke': - intermediate_revoke_flag = True - self.send_response(201) - self.end_headers() - self.wfile.write(b'ok') + intermediate_crl = 'intermediate_revoked.crl' + self.post_response() elif self.path == '/intermediate/unrevoke': - intermediate_revoke_flag = False - self.send_response(201) - self.end_headers() - self.wfile.write(b'ok') + intermediate_crl = 'intermediate.crl' + self.post_response() else: self.send_error(404, 'Not Found') + + def post_response(self): + self.send_response(201) + self.end_headers() + self.wfile.write(b'ok') class ReusableTCPServer(socketserver.TCPServer): allow_reuse_address = True diff --git a/test/e2e/suite/scenario/crl.go b/test/e2e/suite/scenario/crl.go index 6abadec4b..ce3e60bf2 100644 --- a/test/e2e/suite/scenario/crl.go +++ b/test/e2e/suite/scenario/crl.go @@ -14,8 +14,12 @@ package scenario_test import ( + "context" + "net/http" "os" + crlcore "github.com/notaryproject/notation-core-go/revocation/crl" + "github.com/notaryproject/notation-go/verifier/crl" . "github.com/notaryproject/notation/test/e2e/internal/notation" "github.com/notaryproject/notation/test/e2e/internal/utils" . "github.com/notaryproject/notation/test/e2e/suite/common" @@ -229,4 +233,82 @@ var _ = Describe("notation CRL revocation check", Serial, func() { ) }) }) + + It("succesfully completed with the crl in the cache expired and a roundtrip", func() { + Host(CRLOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) { + ctx := context.Background() + notation.Exec("sign", artifact.ReferenceWithDigest()). + MatchKeyWords(SignSuccessfully) + + utils.LeafCRLExpired() + // write expired CRL cache + fetcher, err := crlcore.NewHTTPFetcher(http.DefaultClient) + if err != nil { + Fail(err.Error()) + } + fetcher.Cache, err = crl.NewFileCache(vhost.AbsolutePath(".cache", "crl")) + if err != nil { + Fail(err.Error()) + } + _, err = fetcher.Fetch(ctx, "http://localhost:10086/leaf.crl") + if err != nil { + Fail(err.Error()) + } + + utils.LeafCRLUnrevoke() + utils.IntermediateCRLUnrevoke() + // verify without cache + notation.Exec("verify", artifact.ReferenceWithDigest(), "-d"). + MatchKeyWords( + VerifySuccessfully, + ). + MatchErrKeyWords( + "CRL bundle retrieved from file cache has expired at 2023-12-25", + `"GET" "http://localhost:10086/leaf.crl"`, + "Retrieving crl bundle from file cache with key", + "Storing crl bundle to file cache with key", + "OCSP check failed with unknown error and fallback to CRL check for certificate #2", + ). + NoMatchErrKeyWords( + "is revoked", + ) + }) + }) + + It("failed with crl in the cache expired and a roundtrip to download a revoked crl", func() { + Host(CRLOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) { + ctx := context.Background() + notation.Exec("sign", artifact.ReferenceWithDigest()). + MatchKeyWords(SignSuccessfully) + + utils.LeafCRLExpired() + // write expired CRL cache + fetcher, err := crlcore.NewHTTPFetcher(http.DefaultClient) + if err != nil { + Fail(err.Error()) + } + fetcher.Cache, err = crl.NewFileCache(vhost.AbsolutePath(".cache", "crl")) + if err != nil { + Fail(err.Error()) + } + _, err = fetcher.Fetch(ctx, "http://localhost:10086/leaf.crl") + if err != nil { + Fail(err.Error()) + } + + utils.LeafCRLRevoke() + utils.IntermediateCRLUnrevoke() + // verify without cache + notation.ExpectFailure().Exec("verify", artifact.ReferenceWithDigest(), "-d"). + MatchErrKeyWords( + VerifyFailed, + "CRL bundle retrieved from file cache has expired at 2023-12-25", + `"GET" "http://localhost:10086/leaf.crl"`, + "Retrieving crl bundle from file cache with key", + "Storing crl bundle to file cache with key", + "OCSP check failed with unknown error and fallback to CRL check for certificate #2", + "is revoked", + ) + }) + }) }) diff --git a/test/e2e/testdata/config/crl/leaf_expired.crl b/test/e2e/testdata/config/crl/leaf_expired.crl new file mode 100644 index 0000000000000000000000000000000000000000..8e526887d7a681696a21a4ad4ffe4af2be009c63 GIT binary patch literal 493 zcmXqLVti@Pc!`OT(SVnYQ>)FR?K>|cBR4C9L7pME0Vf-CC<~h~Q)sXup8*eu!@JUsqI>4|xnRf#2;`FVx{27Dl4ZXR}^aA;m; ziJ`cGC`g2vhtD&wB(*3vH6;^hoU@}iuaU8#k&%gkg`ug5Q52MG3gs>^FwirQ2Wpg6 zW|1%uYY-{abB#0V(c5=>$*MioOilamh&P6TT_h{Q!otKPz<}l*<|al)hVq5wQOjSq z{xaIz@44ZR>C2>f-CFGzKLsaDd*uD4YUKp)itmAbE>Al?zPK86#VVKceO2!y9{b;2 zyX)3BhRD=DyJ#ekS^DSxf1A{$weL=Mi|+YTr)E;{|FtjwhaC6W&!x`s_&1#4Wn299 z|0UgM?P3Alh3_jlS4y_0Sq7!PVz`=hX2Z+#tg5xV+1^Rs0qYKJdA~pP=!}I|-6n<| zF=uJ0RlDc