From e7200defb2de478452ff70691257eb81ee8ebee5 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 20 Sep 2024 16:14:19 +0800 Subject: [PATCH 01/29] crl Signed-off-by: Patrick Zheng --- go.mod | 4 +- go.sum | 8 +- verifier/crl/crl.go | 133 ++++++++++++++++++++++ verifier/crl/crl_test.go | 239 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 378 insertions(+), 6 deletions(-) create mode 100644 verifier/crl/crl.go create mode 100644 verifier/crl/crl_test.go diff --git a/go.mod b/go.mod index 0d63313d..144b4ef0 100644 --- a/go.mod +++ b/go.mod @@ -4,13 +4,13 @@ go 1.22.0 require ( github.com/go-ldap/ldap/v3 v3.4.8 - github.com/notaryproject/notation-core-go v1.1.1-0.20240918011623-695ea0c1ad1f + github.com/notaryproject/notation-core-go v1.1.1-0.20240920045731-0786f51de737 github.com/notaryproject/notation-plugin-framework-go v1.0.0 github.com/notaryproject/tspclient-go v0.2.0 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.26.0 + golang.org/x/crypto v0.27.0 golang.org/x/mod v0.21.0 oras.land/oras-go/v2 v2.5.0 ) diff --git a/go.sum b/go.sum index bb03b145..27032854 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,8 @@ 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.1.1-0.20240918011623-695ea0c1ad1f h1:TmwJtM3AZ7iQ1LJEbHRPAMRw4hA52/AbVrllSVjCNP0= -github.com/notaryproject/notation-core-go v1.1.1-0.20240918011623-695ea0c1ad1f/go.mod h1:+6AOh41JPrnVLbW/19SJqdhVHwKgIINBO/np0e7nXJA= +github.com/notaryproject/notation-core-go v1.1.1-0.20240920045731-0786f51de737 h1:Hp93KBCABE29+6zdS0GTg0T1SXj6qGatJyN1JMvTQqk= +github.com/notaryproject/notation-core-go v1.1.1-0.20240920045731-0786f51de737/go.mod h1:b/70rA4OgOHlg0A7pb8zTWKJadFO6781zS3a37KHEJQ= 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= @@ -62,8 +62,8 @@ 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.26.0 h1:RrRspgV4mU+YwB4FYnuBoKsUapNIL5cohGAmSH3azsw= -golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54= +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/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= diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go new file mode 100644 index 00000000..828a6721 --- /dev/null +++ b/verifier/crl/crl.go @@ -0,0 +1,133 @@ +// 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 crl provides functionalities for crl revocation check. +package crl + +import ( + "context" + "crypto/sha256" + "encoding/gob" + "encoding/hex" + "errors" + "fmt" + "os" + "path/filepath" + "time" + + corecrl "github.com/notaryproject/notation-core-go/revocation/crl" + "github.com/notaryproject/notation-go/log" +) + +const ( + // tmpFileName is the prefix of the temporary file + tmpFileName = "notation-*" +) + +// FileCache implements corecrl.Cache. +// +// Key: url of the CRL. +// +// Value: corecrl.Bundle. +// +// This cache builds on top of the UNIX file system to leverage the file system's +// atomic operations. The `rename` and `remove` operations will unlink the old +// file but keep the inode and file descriptor for existing processes to access +// the file. The old inode will be dereferenced when all processes close the old +// file descriptor. Additionally, the operations are proven to be atomic on +// UNIX-like platforms, so there is no need to handle file locking. +// +// NOTE: For Windows, the `open`, `rename` and `remove` operations need file +// locking to ensure atomicity. The current implementation does not handle +// file locking, so the concurrent write from multiple processes may be failed. +// Please do not use this cache in a multi-process environment on Windows. +type FileCache struct { + // root is the root directory of the cache + root string +} + +// NewFileCache creates a FileCache with root as the root directory +func NewFileCache(root string) (*FileCache, error) { + if err := os.MkdirAll(root, 0700); err != nil { + return nil, fmt.Errorf("failed to create crl file cache: %w", err) + } + return &FileCache{ + root: root, + }, nil +} + +// Get retrieves CRL bundle from c given url as key. If the key does not exist +// or the content has expired, corecrl.ErrCacheMiss is returned. +func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error) { + logger := log.GetLogger(ctx) + + // read CRL bundle from file + f, err := os.Open(filepath.Join(c.root, c.fileName(url))) + if err != nil { + if os.IsNotExist(err) { + logger.Infof("CRL file cache miss. Key %q does not exist", url) + return nil, corecrl.ErrCacheMiss + } + return nil, fmt.Errorf("failed to get crl bundle from file cache with key %q: %w", url, err) + } + defer f.Close() + dec := gob.NewDecoder(f) + var bundle corecrl.Bundle + err = dec.Decode(&bundle) + if err != nil { + return nil, fmt.Errorf("failed to decode file retrieved from file cache to CRL Bundle: %w", err) + } + + // check expiry + nextUpdate := bundle.BaseCRL.NextUpdate + if nextUpdate.IsZero() { + return nil, errors.New("crl bundle retrieved from file cache does not contain BaseCRL NextUpdate") + } + if time.Now().After(nextUpdate) { + // content in file cache has expired + logger.Infof("CRL bundle retrieved from file cache with key %q has expired at %s", url, nextUpdate) + return nil, corecrl.ErrCacheMiss + } + + return &bundle, nil +} + +// Set stores the CRL bundle in c with url as key. +func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error { + if bundle == nil { + return errors.New("failed to store crl bundle in file cache: bundle cannot be nil") + } + // save to tmp file + tmpFile, err := os.CreateTemp("", tmpFileName) + if err != nil { + return fmt.Errorf("failed to store crl bundle in file cache: %w", err) + } + enc := gob.NewEncoder(tmpFile) + err = enc.Encode(bundle) + if err != nil { + return fmt.Errorf("failed to store crl bundle in file cache: %w", err) + } + + // rename is atomic on UNIX-like platforms + err = os.Rename(tmpFile.Name(), filepath.Join(c.root, c.fileName(url))) + if err != nil { + return fmt.Errorf("failed to store crl bundle in file cache: %w", err) + } + return nil +} + +// fileName returns the filename of the CRL bundle within c +func (c *FileCache) fileName(url string) string { + hash := sha256.Sum256([]byte(url)) + return hex.EncodeToString(hash[:]) +} diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go new file mode 100644 index 00000000..1a8fa1aa --- /dev/null +++ b/verifier/crl/crl_test.go @@ -0,0 +1,239 @@ +// 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 crl + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/x509" + "errors" + "math/big" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + corecrl "github.com/notaryproject/notation-core-go/revocation/crl" + "github.com/notaryproject/notation-core-go/testhelper" +) + +func TestFileCache(t *testing.T) { + now := time.Now() + certChain := testhelper.GetRevokableRSAChainWithRevocations(2, false, true) + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(1), + NextUpdate: now.Add(time.Hour), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatal(err) + } + baseCRL, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + root := t.TempDir() + cache, err := NewFileCache(root) + if err != nil { + t.Fatal(err) + } + + t.Run("NewFileCache", func(t *testing.T) { + if err != nil { + t.Fatalf("expected no error, but got %v", err) + } + if cache.root != root { + t.Fatalf("expected root %v, but got %v", root, cache.root) + } + }) + + key := "testKey" + bundle := &corecrl.Bundle{BaseCRL: baseCRL} + t.Run("comformance", func(t *testing.T) { + if err := cache.Set(ctx, key, bundle); err != nil { + t.Fatal(err) + } + retrievedBundle, err := cache.Get(ctx, key) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(retrievedBundle.BaseCRL.Raw, bundle.BaseCRL.Raw) { + t.Fatalf("expected bundle %+v, but got %+v", bundle.BaseCRL, retrievedBundle.BaseCRL) + } + }) +} + +func TestNewFileCacheFailed(t *testing.T) { + tempDir := t.TempDir() + t.Run("without permission to create cache directory", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + + if err := os.Chmod(tempDir, 0); err != nil { + t.Fatal(err) + } + root := filepath.Join(tempDir, "test") + _, err := NewFileCache(root) + if !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("expected permission denied error, but got %v", err) + } + // restore permission + if err := os.Chmod(tempDir, 0755); err != nil { + t.Fatalf("failed to change permission: %v", err) + } + }) +} + +func TestGetFailed(t *testing.T) { + tempDir := t.TempDir() + cache, err := NewFileCache(tempDir) + if err != nil { + t.Fatal(err) + } + + t.Run("key does not exist", func(t *testing.T) { + _, err := cache.Get(context.Background(), "nonExistKey") + if !errors.Is(err, corecrl.ErrCacheMiss) { + t.Fatalf("expected ErrCacheMiss, but got %v", err) + } + }) + + invalidFile := filepath.Join(tempDir, cache.fileName("invalid")) + if err := os.WriteFile(invalidFile, []byte("invalid"), 0644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + + t.Run("no permission to read file", func(t *testing.T) { + if err := os.Chmod(invalidFile, 0); err != nil { + t.Fatal(err) + } + _, err := cache.Get(context.Background(), "invalid") + if err == nil || !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("expected permission denied error, but got %v", err) + } + // restore permission + if err := os.Chmod(invalidFile, 0755); err != nil { + t.Fatal(err) + } + }) + + t.Run("invalid bundle", func(t *testing.T) { + _, err := cache.Get(context.Background(), "invalid") + expectedErrMsg := "failed to decode file retrieved from file cache to CRL Bundle: unexpected EOF" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + + now := time.Now() + certChain := testhelper.GetRevokableRSAChainWithRevocations(2, false, true) + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(1), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatalf("failed to create base CRL: %v", err) + } + baseCRL, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatalf("failed to parse base CRL: %v", err) + } + t.Run("bundle with invalid NextUpdate", func(t *testing.T) { + ctx := context.Background() + expiredBundle := &corecrl.Bundle{BaseCRL: baseCRL} + if err := cache.Set(ctx, "expiredKey", expiredBundle); err != nil { + t.Fatal(err) + } + _, err = cache.Get(ctx, "expiredKey") + expectedErrMsg := "crl bundle retrieved from file cache does not contain BaseCRL NextUpdate" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + + crlBytes, err = x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(1), + NextUpdate: now.Add(-time.Hour), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatalf("failed to create base CRL: %v", err) + } + baseCRL, err = x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatalf("failed to parse base CRL: %v", err) + } + t.Run("bundle in cache has expired", func(t *testing.T) { + ctx := context.Background() + expiredBundle := &corecrl.Bundle{BaseCRL: baseCRL} + if err := cache.Set(ctx, "expiredKey", expiredBundle); err != nil { + t.Fatal(err) + } + _, err = cache.Get(ctx, "expiredKey") + if !errors.Is(err, corecrl.ErrCacheMiss) { + t.Fatalf("expected ErrCacheMiss, but got %v", err) + } + }) +} + +func TestSetFailed(t *testing.T) { + tempDir := t.TempDir() + cache, err := NewFileCache(tempDir) + if err != nil { + t.Fatal(err) + } + + now := time.Now() + certChain := testhelper.GetRevokableRSAChainWithRevocations(2, false, true) + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(1), + NextUpdate: now.Add(time.Hour), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatal(err) + } + baseCRL, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + key := "testKey" + + t.Run("nil bundle", func(t *testing.T) { + err := cache.Set(ctx, key, nil) + expectedErrMsg := "failed to store crl bundle in file cache: bundle cannot be nil" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + + t.Run("failed to create tmp file", func(t *testing.T) { + if err := os.Chmod(tempDir, 0); err != nil { + t.Fatal(err) + } + bundle := &corecrl.Bundle{BaseCRL: baseCRL} + err := cache.Set(ctx, key, bundle) + if err == nil || !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("expected permission denied error, but got %v", err) + } + // restore permission + if err := os.Chmod(tempDir, 0755); err != nil { + t.Fatalf("failed to change permission: %v", err) + } + }) +} From 6ebf3ffb66454e6e8e726420c96115ab21894c60 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 20 Sep 2024 17:20:18 +0800 Subject: [PATCH 02/29] crl dir and verifier Signed-off-by: Patrick Zheng --- dir/fs.go | 5 +++++ dir/path.go | 27 ++++++++++++++++++++++++--- verifier/verifier.go | 25 ++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/dir/fs.go b/dir/fs.go index 444179ce..eec834a8 100644 --- a/dir/fs.go +++ b/dir/fs.go @@ -58,3 +58,8 @@ func ConfigFS() SysFS { func PluginFS() SysFS { return NewSysFS(filepath.Join(userLibexecDirPath(), PathPlugins)) } + +// CacheFS is the cache SysFS +func CacheFS() SysFS { + return NewSysFS(userCacheDirPath()) +} diff --git a/dir/path.go b/dir/path.go index a08a9a24..442934e5 100644 --- a/dir/path.go +++ b/dir/path.go @@ -12,7 +12,7 @@ // limitations under the License. // Package dir implements Notation directory structure. -// [directory spec]: https://github.com/notaryproject/notation/blob/main/specs/directory.md +// [directory spec]: https://notaryproject.dev/docs/user-guides/how-to/directory-structure/ // // Example: // @@ -31,7 +31,7 @@ // - Set custom configurations directory: // dir.UserConfigDir = '/path/to/configurations/' // -// Only user level directory is supported for RC.1, and system level directory +// Only user level directory is supported, and system level directory // may be added later. package dir @@ -44,6 +44,7 @@ import ( var ( UserConfigDir string // Absolute path of user level {NOTATION_CONFIG} UserLibexecDir string // Absolute path of user level {NOTATION_LIBEXEC} + UserCacheDir string // Absolute path of user level {NOTATION_CACHE} ) const ( @@ -77,7 +78,12 @@ const ( TrustStoreDir = "truststore" ) -var userConfigDir = os.UserConfigDir // for unit test +// for unit tests +var ( + userConfigDir = os.UserConfigDir + + userCacheDir = os.UserCacheDir +) // userConfigDirPath returns the user level {NOTATION_CONFIG} path. func userConfigDirPath() string { @@ -103,6 +109,21 @@ func userLibexecDirPath() string { return UserLibexecDir } +// userCacheDirPath returns the user level {NOTATION_CACHE} path. +func userCacheDirPath() string { + if UserCacheDir == "" { + userDir, err := userCacheDir() + if err != nil { + // fallback to current directory + UserCacheDir = filepath.Join("."+notation, "cache") + return UserCacheDir + } + // set user config + UserCacheDir = filepath.Join(userDir, notation) + } + return UserCacheDir +} + // LocalKeyPath returns the local key and local cert relative paths. func LocalKeyPath(name string) (keyPath, certPath string) { basePath := path.Join(LocalKeysDir, name) diff --git a/verifier/verifier.go b/verifier/verifier.go index 0b62904e..7c6874a8 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -30,6 +30,7 @@ import ( "oras.land/oras-go/v2/content" "github.com/notaryproject/notation-core-go/revocation" + corecrl "github.com/notaryproject/notation-core-go/revocation/crl" "github.com/notaryproject/notation-core-go/revocation/purpose" revocationresult "github.com/notaryproject/notation-core-go/revocation/result" "github.com/notaryproject/notation-core-go/signature" @@ -43,6 +44,7 @@ import ( trustpolicyInternal "github.com/notaryproject/notation-go/internal/trustpolicy" "github.com/notaryproject/notation-go/log" "github.com/notaryproject/notation-go/plugin" + "github.com/notaryproject/notation-go/verifier/crl" "github.com/notaryproject/notation-go/verifier/trustpolicy" "github.com/notaryproject/notation-go/verifier/truststore" pluginframework "github.com/notaryproject/notation-plugin-framework-go/plugin" @@ -178,12 +180,18 @@ func New(ociTrustPolicy *trustpolicy.OCIDocument, trustStore truststore.X509Trus // setRevocation sets revocation validators of v func (v *verifier) setRevocation(verifierOptions VerifierOptions) error { + // default crl fetcher + crlFetcher, err := crlFetcher() + if err != nil { + return err + } + // timestamping validator revocationTimestampingValidator := verifierOptions.RevocationTimestampingValidator - var err error if revocationTimestampingValidator == nil { revocationTimestampingValidator, err = revocation.NewWithOptions(revocation.Options{ OCSPHTTPClient: &http.Client{Timeout: 2 * time.Second}, + CRLFetcher: crlFetcher, CertChainPurpose: purpose.Timestamping, }) if err != nil { @@ -207,6 +215,7 @@ func (v *verifier) setRevocation(verifierOptions VerifierOptions) error { // both RevocationCodeSigningValidator and RevocationClient are nil revocationCodeSigningValidator, err = revocation.NewWithOptions(revocation.Options{ OCSPHTTPClient: &http.Client{Timeout: 2 * time.Second}, + CRLFetcher: crlFetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil { @@ -1093,3 +1102,17 @@ func verifyTimestamp(ctx context.Context, policyName string, trustStores []strin // success return nil } + +// crlFetcher creates a default crl Fetcher with file cache. +func crlFetcher() (corecrl.Fetcher, error) { + crlFetcher, err := corecrl.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) + if err != nil { + return nil, err + } + cacheRoot, _ := dir.CacheFS().SysPath() // always returns nil err + crlFetcher.Cache, err = crl.NewFileCache(cacheRoot) + if err != nil { + return nil, err + } + return crlFetcher, nil +} From 96165c0c4bef4f420429c4a3a0a6aa61a5e4682d Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 20 Sep 2024 18:03:27 +0800 Subject: [PATCH 03/29] update Signed-off-by: Patrick Zheng --- dir/fs_test.go | 11 +++++++++++ dir/path_test.go | 29 +++++++++++++++++++++++------ verifier/verifier.go | 30 +++++++++--------------------- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/dir/fs_test.go b/dir/fs_test.go index 6488e5de..72b699a1 100644 --- a/dir/fs_test.go +++ b/dir/fs_test.go @@ -71,3 +71,14 @@ func TestPluginFS(t *testing.T) { t.Fatalf(`SysPath() failed. got: %q, want: %q`, path, filepath.Join(userLibexecDirPath(), PathPlugins, "plugin")) } } + +func TestCacheFS(t *testing.T) { + cacheFS := CacheFS() + path, err := cacheFS.SysPath() + if err != nil { + t.Fatalf("SysPath() failed. err = %v", err) + } + if path != filepath.Join(UserCacheDir) { + t.Fatalf(`SysPath() failed. got: %q, want: %q`, path, UserConfigDir) + } +} diff --git a/dir/path_test.go b/dir/path_test.go index ceeafc3f..ee90f2dc 100644 --- a/dir/path_test.go +++ b/dir/path_test.go @@ -15,20 +15,22 @@ package dir import ( "os" + "path/filepath" "testing" ) -func mockGetUserConfig() (string, error) { +func mockUserPath() (string, error) { return "/path/", nil } func setup() { UserConfigDir = "" UserLibexecDir = "" + UserCacheDir = "" } func Test_UserConfigDirPath(t *testing.T) { - userConfigDir = mockGetUserConfig + userConfigDir = mockUserPath setup() got := userConfigDirPath() if got != "/path/notation" { @@ -39,16 +41,22 @@ func Test_UserConfigDirPath(t *testing.T) { func Test_NoHomeVariable(t *testing.T) { t.Setenv("HOME", "") t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("XDG_CACHE_HOME", "") setup() userConfigDir = os.UserConfigDir got := userConfigDirPath() if got != ".notation" { - t.Fatalf(`UserConfigDirPath() = %q, want ".notation"`, UserConfigDir) + t.Fatalf(`userConfigDirPath() = %q, want ".notation"`, got) + } + got = userCacheDirPath() + want := filepath.Join("."+notation, "cache") + if got != want { + t.Fatalf(`userCacheDirPath() = %q, want %q`, got, want) } } func Test_UserLibexecDirPath(t *testing.T) { - userConfigDir = mockGetUserConfig + userConfigDir = mockUserPath setup() got := userLibexecDirPath() if got != "/path/notation" { @@ -56,8 +64,17 @@ func Test_UserLibexecDirPath(t *testing.T) { } } +func Test_UserCacheDirPath(t *testing.T) { + userCacheDir = mockUserPath + setup() + got := userCacheDirPath() + if got != "/path/notation" { + t.Fatalf(`UserCacheDirPath() = %q, want "/path/notation"`, got) + } +} + func TestLocalKeyPath(t *testing.T) { - userConfigDir = mockGetUserConfig + userConfigDir = mockUserPath setup() _ = userConfigDirPath() _ = userLibexecDirPath() @@ -71,7 +88,7 @@ func TestLocalKeyPath(t *testing.T) { } func TestX509TrustStoreDir(t *testing.T) { - userConfigDir = mockGetUserConfig + userConfigDir = mockUserPath setup() _ = userConfigDirPath() _ = userLibexecDirPath() diff --git a/verifier/verifier.go b/verifier/verifier.go index 7c6874a8..11abea15 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -44,7 +44,6 @@ import ( trustpolicyInternal "github.com/notaryproject/notation-go/internal/trustpolicy" "github.com/notaryproject/notation-go/log" "github.com/notaryproject/notation-go/plugin" - "github.com/notaryproject/notation-go/verifier/crl" "github.com/notaryproject/notation-go/verifier/trustpolicy" "github.com/notaryproject/notation-go/verifier/truststore" pluginframework "github.com/notaryproject/notation-plugin-framework-go/plugin" @@ -180,15 +179,14 @@ func New(ociTrustPolicy *trustpolicy.OCIDocument, trustStore truststore.X509Trus // setRevocation sets revocation validators of v func (v *verifier) setRevocation(verifierOptions VerifierOptions) error { - // default crl fetcher - crlFetcher, err := crlFetcher() - if err != nil { - return err - } - // timestamping validator revocationTimestampingValidator := verifierOptions.RevocationTimestampingValidator + var err error if revocationTimestampingValidator == nil { + crlFetcher, err := corecrl.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) + if err != nil { + return err + } revocationTimestampingValidator, err = revocation.NewWithOptions(revocation.Options{ OCSPHTTPClient: &http.Client{Timeout: 2 * time.Second}, CRLFetcher: crlFetcher, @@ -213,6 +211,10 @@ func (v *verifier) setRevocation(verifierOptions VerifierOptions) error { } // both RevocationCodeSigningValidator and RevocationClient are nil + crlFetcher, err := corecrl.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) + if err != nil { + return err + } revocationCodeSigningValidator, err = revocation.NewWithOptions(revocation.Options{ OCSPHTTPClient: &http.Client{Timeout: 2 * time.Second}, CRLFetcher: crlFetcher, @@ -1102,17 +1104,3 @@ func verifyTimestamp(ctx context.Context, policyName string, trustStores []strin // success return nil } - -// crlFetcher creates a default crl Fetcher with file cache. -func crlFetcher() (corecrl.Fetcher, error) { - crlFetcher, err := corecrl.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) - if err != nil { - return nil, err - } - cacheRoot, _ := dir.CacheFS().SysPath() // always returns nil err - crlFetcher.Cache, err = crl.NewFileCache(cacheRoot) - if err != nil { - return nil, err - } - return crlFetcher, nil -} From 0d23fcd745a50226cf54089c65cdec6700564867 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Sat, 21 Sep 2024 19:29:09 +0800 Subject: [PATCH 04/29] crl file cache Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 59 ++++++++++++++++++++++++++++++---------- verifier/crl/crl_test.go | 40 ++++++++++++++++++++++----- verifier/verifier.go | 11 -------- 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 828a6721..2f257e92 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -17,10 +17,12 @@ package crl import ( "context" "crypto/sha256" - "encoding/gob" + "crypto/x509" "encoding/hex" + "encoding/json" "errors" "fmt" + "io/fs" "os" "path/filepath" "time" @@ -56,6 +58,15 @@ type FileCache struct { root string } +// fileCacheContent is the actual content saved in a FileCache +type fileCacheContent struct { + // RawBaseCRL is baseCRL.Raw + RawBaseCRL []byte `json:"rawBaseCRL"` + + // RawDeltaCRL is deltaCRL.Raw + RawDeltaCRL []byte `json:"rawDeltaCRL,omitempty"` +} + // NewFileCache creates a FileCache with root as the root directory func NewFileCache(root string) (*FileCache, error) { if err := os.MkdirAll(root, 0700); err != nil { @@ -71,25 +82,34 @@ func NewFileCache(root string) (*FileCache, error) { func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error) { logger := log.GetLogger(ctx) - // read CRL bundle from file + // get content from file cache f, err := os.Open(filepath.Join(c.root, c.fileName(url))) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { logger.Infof("CRL file cache miss. Key %q does not exist", url) return nil, corecrl.ErrCacheMiss } return nil, fmt.Errorf("failed to get crl bundle from file cache with key %q: %w", url, err) } defer f.Close() - dec := gob.NewDecoder(f) - var bundle corecrl.Bundle - err = dec.Decode(&bundle) + + // decode content to crl Bundle + var content fileCacheContent + err = json.NewDecoder(f).Decode(&content) if err != nil { - return nil, fmt.Errorf("failed to decode file retrieved from file cache to CRL Bundle: %w", err) + return nil, fmt.Errorf("failed to decode file retrieved from file cache: %w", err) + } + // TODO: add DeltaCRL once notation-core-go supports it. Issue: https://github.com/notaryproject/notation-core-go/issues/228 + baseCRL, err := x509.ParseRevocationList(content.RawBaseCRL) + if err != nil { + return nil, fmt.Errorf("failed to parse base CRL of file retrieved from file cache: %w", err) + } + bundle := corecrl.Bundle{ + BaseCRL: baseCRL, } // check expiry - nextUpdate := bundle.BaseCRL.NextUpdate + nextUpdate := baseCRL.NextUpdate if nextUpdate.IsZero() { return nil, errors.New("crl bundle retrieved from file cache does not contain BaseCRL NextUpdate") } @@ -104,18 +124,29 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error // Set stores the CRL bundle in c with url as key. func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error { + // sanity check if bundle == nil { return errors.New("failed to store crl bundle in file cache: bundle cannot be nil") } - // save to tmp file + if bundle.BaseCRL == nil { + return errors.New("failed to store crl bundle in file cache: bundle BaseCRL cannot be nil") + } + + // actual content to be saved in the cache + // + // TODO: add DeltaCRL once notation-core-go supports it. Issue: https://github.com/notaryproject/notation-core-go/issues/228 + content := fileCacheContent{ + RawBaseCRL: bundle.BaseCRL.Raw, + } + + // save content to tmp file tmpFile, err := os.CreateTemp("", tmpFileName) if err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: %w", err) + return fmt.Errorf("failed to store crl bundle in file cache: failed to create temp file: %w", err) } - enc := gob.NewEncoder(tmpFile) - err = enc.Encode(bundle) + err = json.NewEncoder(tmpFile).Encode(content) if err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: %w", err) + return fmt.Errorf("failed to store crl bundle in file cache: failed to encode content: %w", err) } // rename is atomic on UNIX-like platforms @@ -126,7 +157,7 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) return nil } -// fileName returns the filename of the CRL bundle within c +// fileName returns the filename of the content stored in c func (c *FileCache) fileName(url string) string { hash := sha256.Sum256([]byte(url)) return hex.EncodeToString(hash[:]) diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index 1a8fa1aa..967a1af0 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -18,6 +18,7 @@ import ( "context" "crypto/rand" "crypto/x509" + "encoding/json" "errors" "math/big" "os" @@ -49,10 +50,6 @@ func TestFileCache(t *testing.T) { ctx := context.Background() root := t.TempDir() cache, err := NewFileCache(root) - if err != nil { - t.Fatal(err) - } - t.Run("NewFileCache", func(t *testing.T) { if err != nil { t.Fatalf("expected no error, but got %v", err) @@ -134,9 +131,9 @@ func TestGetFailed(t *testing.T) { } }) - t.Run("invalid bundle", func(t *testing.T) { + t.Run("invalid content", func(t *testing.T) { _, err := cache.Get(context.Background(), "invalid") - expectedErrMsg := "failed to decode file retrieved from file cache to CRL Bundle: unexpected EOF" + expectedErrMsg := "failed to decode file retrieved from file cache: invalid character 'i' looking for beginning of value" if err == nil || err.Error() != expectedErrMsg { t.Fatalf("expected %s, but got %v", expectedErrMsg, err) } @@ -154,6 +151,26 @@ func TestGetFailed(t *testing.T) { if err != nil { t.Fatalf("failed to parse base CRL: %v", err) } + + t.Run("invalid RawBaseCRL of content", func(t *testing.T) { + content := fileCacheContent{ + RawBaseCRL: []byte("invalid"), + } + b, err := json.Marshal(content) + if err != nil { + t.Fatal(err) + } + invalidBundleFile := filepath.Join(tempDir, cache.fileName("invalidBundle")) + if err := os.WriteFile(invalidBundleFile, b, 0644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + _, err = cache.Get(context.Background(), "invalidBundle") + expectedErrMsg := "failed to parse base CRL of file retrieved from file cache: x509: malformed crl" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + t.Run("bundle with invalid NextUpdate", func(t *testing.T) { ctx := context.Background() expiredBundle := &corecrl.Bundle{BaseCRL: baseCRL} @@ -222,7 +239,16 @@ func TestSetFailed(t *testing.T) { } }) - t.Run("failed to create tmp file", func(t *testing.T) { + t.Run("nil bundle BaseCRL", func(t *testing.T) { + bundle := &corecrl.Bundle{} + err := cache.Set(ctx, key, bundle) + expectedErrMsg := "failed to store crl bundle in file cache: bundle BaseCRL cannot be nil" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + + t.Run("failed to write into cache due to permission denied", func(t *testing.T) { if err := os.Chmod(tempDir, 0); err != nil { t.Fatal(err) } diff --git a/verifier/verifier.go b/verifier/verifier.go index 11abea15..0b62904e 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -30,7 +30,6 @@ import ( "oras.land/oras-go/v2/content" "github.com/notaryproject/notation-core-go/revocation" - corecrl "github.com/notaryproject/notation-core-go/revocation/crl" "github.com/notaryproject/notation-core-go/revocation/purpose" revocationresult "github.com/notaryproject/notation-core-go/revocation/result" "github.com/notaryproject/notation-core-go/signature" @@ -183,13 +182,8 @@ func (v *verifier) setRevocation(verifierOptions VerifierOptions) error { revocationTimestampingValidator := verifierOptions.RevocationTimestampingValidator var err error if revocationTimestampingValidator == nil { - crlFetcher, err := corecrl.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) - if err != nil { - return err - } revocationTimestampingValidator, err = revocation.NewWithOptions(revocation.Options{ OCSPHTTPClient: &http.Client{Timeout: 2 * time.Second}, - CRLFetcher: crlFetcher, CertChainPurpose: purpose.Timestamping, }) if err != nil { @@ -211,13 +205,8 @@ func (v *verifier) setRevocation(verifierOptions VerifierOptions) error { } // both RevocationCodeSigningValidator and RevocationClient are nil - crlFetcher, err := corecrl.NewHTTPFetcher(&http.Client{Timeout: 5 * time.Second}) - if err != nil { - return err - } revocationCodeSigningValidator, err = revocation.NewWithOptions(revocation.Options{ OCSPHTTPClient: &http.Client{Timeout: 2 * time.Second}, - CRLFetcher: crlFetcher, CertChainPurpose: purpose.CodeSigning, }) if err != nil { From 77e9c9c792d2ee98f924626d3365eb1023d944c1 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 11:00:58 +0800 Subject: [PATCH 05/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 44 ++++++++++----- verifier/crl/crl_test.go | 112 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 135 insertions(+), 21 deletions(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 2f257e92..b2a2a152 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -81,6 +81,7 @@ func NewFileCache(root string) (*FileCache, error) { // or the content has expired, corecrl.ErrCacheMiss is returned. func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error) { logger := log.GetLogger(ctx) + logger.Infof("Retrieving crl bundle from file cache with key %q ...", url) // get content from file cache f, err := os.Open(filepath.Join(c.root, c.fileName(url))) @@ -99,24 +100,26 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error if err != nil { return nil, fmt.Errorf("failed to decode file retrieved from file cache: %w", err) } - // TODO: add DeltaCRL once notation-core-go supports it. Issue: https://github.com/notaryproject/notation-core-go/issues/228 - baseCRL, err := x509.ParseRevocationList(content.RawBaseCRL) + var bundle corecrl.Bundle + bundle.BaseCRL, err = x509.ParseRevocationList(content.RawBaseCRL) if err != nil { return nil, fmt.Errorf("failed to parse base CRL of file retrieved from file cache: %w", err) } - bundle := corecrl.Bundle{ - BaseCRL: baseCRL, + if content.RawDeltaCRL != nil { + bundle.DeltaCRL, err = x509.ParseRevocationList(content.RawDeltaCRL) + if err != nil { + return nil, fmt.Errorf("failed to parse delta CRL of file retrieved from file cache: %w", err) + } } // check expiry - nextUpdate := baseCRL.NextUpdate - if nextUpdate.IsZero() { - return nil, errors.New("crl bundle retrieved from file cache does not contain BaseCRL NextUpdate") + if err := checkExpiry(ctx, bundle.BaseCRL.NextUpdate); err != nil { + return nil, err } - if time.Now().After(nextUpdate) { - // content in file cache has expired - logger.Infof("CRL bundle retrieved from file cache with key %q has expired at %s", url, nextUpdate) - return nil, corecrl.ErrCacheMiss + if content.RawDeltaCRL != nil { + if err := checkExpiry(ctx, bundle.DeltaCRL.NextUpdate); err != nil { + return nil, err + } } return &bundle, nil @@ -133,11 +136,12 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) } // actual content to be saved in the cache - // - // TODO: add DeltaCRL once notation-core-go supports it. Issue: https://github.com/notaryproject/notation-core-go/issues/228 content := fileCacheContent{ RawBaseCRL: bundle.BaseCRL.Raw, } + if bundle.DeltaCRL != nil { + content.RawDeltaCRL = bundle.DeltaCRL.Raw + } // save content to tmp file tmpFile, err := os.CreateTemp("", tmpFileName) @@ -162,3 +166,17 @@ func (c *FileCache) fileName(url string) string { hash := sha256.Sum256([]byte(url)) return hex.EncodeToString(hash[:]) } + +// checkExpiry returns nil when nextUpdate is bounded before current time +func checkExpiry(ctx context.Context, nextUpdate time.Time) error { + logger := log.GetLogger(ctx) + + if nextUpdate.IsZero() { + return errors.New("crl bundle retrieved from file cache does not contain valid NextUpdate") + } + if time.Now().After(nextUpdate) { + logger.Infof("CRL bundle retrieved from file cache has expired at %s", nextUpdate) + return corecrl.ErrCacheMiss + } + return nil +} diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index 967a1af0..e88f272f 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -14,7 +14,6 @@ package crl import ( - "bytes" "context" "crypto/rand" "crypto/x509" @@ -23,6 +22,7 @@ import ( "math/big" "os" "path/filepath" + "reflect" "runtime" "strings" "testing" @@ -60,8 +60,8 @@ func TestFileCache(t *testing.T) { }) key := "testKey" - bundle := &corecrl.Bundle{BaseCRL: baseCRL} t.Run("comformance", func(t *testing.T) { + bundle := &corecrl.Bundle{BaseCRL: baseCRL} if err := cache.Set(ctx, key, bundle); err != nil { t.Fatal(err) } @@ -70,8 +70,31 @@ func TestFileCache(t *testing.T) { t.Fatal(err) } - if !bytes.Equal(retrievedBundle.BaseCRL.Raw, bundle.BaseCRL.Raw) { - t.Fatalf("expected bundle %+v, but got %+v", bundle.BaseCRL, retrievedBundle.BaseCRL) + if !reflect.DeepEqual(retrievedBundle.BaseCRL, bundle.BaseCRL) { + t.Fatalf("expected BaseCRL %+v, but got %+v", bundle.BaseCRL, retrievedBundle.BaseCRL) + } + + if bundle.DeltaCRL != nil { + t.Fatalf("expected DeltaCRL to be nil, but got %+v", retrievedBundle.DeltaCRL) + } + }) + + t.Run("comformance with delta crl", func(t *testing.T) { + bundle := &corecrl.Bundle{BaseCRL: baseCRL, DeltaCRL: baseCRL} + if err := cache.Set(ctx, key, bundle); err != nil { + t.Fatal(err) + } + retrievedBundle, err := cache.Get(ctx, key) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(retrievedBundle.BaseCRL, bundle.BaseCRL) { + t.Fatalf("expected BaseCRL %+v, but got %+v", bundle.BaseCRL, retrievedBundle.BaseCRL) + } + + if !reflect.DeepEqual(retrievedBundle.DeltaCRL, bundle.DeltaCRL) { + t.Fatalf("expected DeltaCRL %+v, but got %+v", bundle.DeltaCRL, retrievedBundle.DeltaCRL) } }) } @@ -152,6 +175,25 @@ func TestGetFailed(t *testing.T) { t.Fatalf("failed to parse base CRL: %v", err) } + t.Run("empty RawBaseCRL of content", func(t *testing.T) { + content := fileCacheContent{ + RawBaseCRL: []byte{}, + } + b, err := json.Marshal(content) + if err != nil { + t.Fatal(err) + } + invalidBundleFile := filepath.Join(tempDir, cache.fileName("invalidBundle")) + if err := os.WriteFile(invalidBundleFile, b, 0644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + _, err = cache.Get(context.Background(), "invalidBundle") + expectedErrMsg := "failed to parse base CRL of file retrieved from file cache: x509: malformed crl" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + t.Run("invalid RawBaseCRL of content", func(t *testing.T) { content := fileCacheContent{ RawBaseCRL: []byte("invalid"), @@ -171,6 +213,26 @@ func TestGetFailed(t *testing.T) { } }) + t.Run("invalid RawDeltaCRL of content", func(t *testing.T) { + content := fileCacheContent{ + RawBaseCRL: baseCRL.Raw, + RawDeltaCRL: []byte("invalid"), + } + b, err := json.Marshal(content) + if err != nil { + t.Fatal(err) + } + invalidBundleFile := filepath.Join(tempDir, cache.fileName("invalidBundle")) + if err := os.WriteFile(invalidBundleFile, b, 0644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + _, err = cache.Get(context.Background(), "invalidBundle") + expectedErrMsg := "failed to parse delta CRL of file retrieved from file cache: x509: malformed crl" + if err == nil || err.Error() != expectedErrMsg { + t.Fatalf("expected %s, but got %v", expectedErrMsg, err) + } + }) + t.Run("bundle with invalid NextUpdate", func(t *testing.T) { ctx := context.Background() expiredBundle := &corecrl.Bundle{BaseCRL: baseCRL} @@ -178,7 +240,7 @@ func TestGetFailed(t *testing.T) { t.Fatal(err) } _, err = cache.Get(ctx, "expiredKey") - expectedErrMsg := "crl bundle retrieved from file cache does not contain BaseCRL NextUpdate" + expectedErrMsg := "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) } @@ -191,13 +253,47 @@ func TestGetFailed(t *testing.T) { if err != nil { t.Fatalf("failed to create base CRL: %v", err) } - baseCRL, err = x509.ParseRevocationList(crlBytes) + expiredBaseCRL, err := x509.ParseRevocationList(crlBytes) if err != nil { t.Fatalf("failed to parse base CRL: %v", err) } - t.Run("bundle in cache has expired", func(t *testing.T) { + t.Run("base crl in cache has expired", func(t *testing.T) { ctx := context.Background() - expiredBundle := &corecrl.Bundle{BaseCRL: baseCRL} + expiredBundle := &corecrl.Bundle{BaseCRL: expiredBaseCRL} + if err := cache.Set(ctx, "expiredKey", expiredBundle); err != nil { + t.Fatal(err) + } + _, err = cache.Get(ctx, "expiredKey") + if !errors.Is(err, corecrl.ErrCacheMiss) { + t.Fatalf("expected ErrCacheMiss, but got %v", err) + } + }) + + t.Run("delta crl in cache has expired", func(t *testing.T) { + ctx := context.Background() + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(1), + NextUpdate: now.Add(time.Hour), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatalf("failed to create base CRL: %v", err) + } + baseCRL, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatalf("failed to parse base CRL: %v", err) + } + crlBytes, err = x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(1), + NextUpdate: now.Add(-time.Hour), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatalf("failed to create base CRL: %v", err) + } + expiredDeltaCRL, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatalf("failed to parse base CRL: %v", err) + } + expiredBundle := &corecrl.Bundle{BaseCRL: baseCRL, DeltaCRL: expiredDeltaCRL} if err := cache.Set(ctx, "expiredKey", expiredBundle); err != nil { t.Fatal(err) } From 18f14aa36574be7c7ef3e8d20190ad37cd395f1d Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 11:04:12 +0800 Subject: [PATCH 06/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 3 +++ verifier/crl/crl_test.go | 13 ++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index b2a2a152..8b043d06 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -127,6 +127,9 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error // Set stores the CRL bundle in c with url as key. func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error { + logger := log.GetLogger(ctx) + logger.Infof("Storing crl bundle to file cache with key %q ...", url) + // sanity check if bundle == nil { return errors.New("failed to store crl bundle in file cache: bundle cannot be nil") diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index e88f272f..1e958ba5 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -80,7 +80,18 @@ func TestFileCache(t *testing.T) { }) t.Run("comformance with delta crl", func(t *testing.T) { - bundle := &corecrl.Bundle{BaseCRL: baseCRL, DeltaCRL: baseCRL} + crlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + Number: big.NewInt(2), + NextUpdate: now.Add(time.Hour), + }, certChain[1].Cert, certChain[1].PrivateKey) + if err != nil { + t.Fatal(err) + } + deltaCRL, err := x509.ParseRevocationList(crlBytes) + if err != nil { + t.Fatal(err) + } + bundle := &corecrl.Bundle{BaseCRL: baseCRL, DeltaCRL: deltaCRL} if err := cache.Set(ctx, key, bundle); err != nil { t.Fatal(err) } From 21709b7a053b2971fe26772cde89da086ced5478 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 11:23:17 +0800 Subject: [PATCH 07/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 8b043d06..d1ba7c47 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -116,7 +116,7 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error if err := checkExpiry(ctx, bundle.BaseCRL.NextUpdate); err != nil { return nil, err } - if content.RawDeltaCRL != nil { + if bundle.DeltaCRL != nil { if err := checkExpiry(ctx, bundle.DeltaCRL.NextUpdate); err != nil { return nil, err } From 48ced53f9a071366e6691ce166c616234281baf9 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 12:40:24 +0800 Subject: [PATCH 08/29] update Signed-off-by: Patrick Zheng --- dir/path.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dir/path.go b/dir/path.go index 442934e5..d9fdc14e 100644 --- a/dir/path.go +++ b/dir/path.go @@ -118,7 +118,7 @@ func userCacheDirPath() string { UserCacheDir = filepath.Join("."+notation, "cache") return UserCacheDir } - // set user config + // set user cache UserCacheDir = filepath.Join(userDir, notation) } return UserCacheDir From 0f2372150022320f9b70419f2bbb7ba56cf3bfe0 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 16:00:51 +0800 Subject: [PATCH 09/29] updated per code review Signed-off-by: Patrick Zheng --- dir/fs.go | 6 +++--- dir/fs_test.go | 2 +- dir/path.go | 6 ++++++ verifier/crl/crl.go | 20 +++++++++++--------- verifier/crl/crl_test.go | 8 ++++---- 5 files changed, 25 insertions(+), 17 deletions(-) diff --git a/dir/fs.go b/dir/fs.go index eec834a8..c0633cd0 100644 --- a/dir/fs.go +++ b/dir/fs.go @@ -59,7 +59,7 @@ func PluginFS() SysFS { return NewSysFS(filepath.Join(userLibexecDirPath(), PathPlugins)) } -// CacheFS is the cache SysFS -func CacheFS() SysFS { - return NewSysFS(userCacheDirPath()) +// CRLFileCacheFS is the crl file cache SysFS +func CRLFileCacheFS() SysFS { + return NewSysFS(filepath.Join(userCacheDirPath(), PathCRLFileCache)) } diff --git a/dir/fs_test.go b/dir/fs_test.go index 72b699a1..68203ad1 100644 --- a/dir/fs_test.go +++ b/dir/fs_test.go @@ -73,7 +73,7 @@ func TestPluginFS(t *testing.T) { } func TestCacheFS(t *testing.T) { - cacheFS := CacheFS() + cacheFS := CRLFileCacheFS() path, err := cacheFS.SysPath() if err != nil { t.Fatalf("SysPath() failed. err = %v", err) diff --git a/dir/path.go b/dir/path.go index d9fdc14e..6c75b28a 100644 --- a/dir/path.go +++ b/dir/path.go @@ -78,6 +78,12 @@ const ( TrustStoreDir = "truststore" ) +// The relative path to {NOTATION_CACHE} +const ( + // PathCRLFileCache is the crl file cache directory relative path. + PathCRLFileCache = "crl" +) + // for unit tests var ( userConfigDir = os.UserConfigDir diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index d1ba7c47..6f5e17d6 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -60,11 +60,11 @@ type FileCache struct { // fileCacheContent is the actual content saved in a FileCache type fileCacheContent struct { - // RawBaseCRL is baseCRL.Raw - RawBaseCRL []byte `json:"rawBaseCRL"` + // BaseCRL is the ASN.1 encoded base CRL + BaseCRL []byte `json:"baseCRL"` - // RawDeltaCRL is deltaCRL.Raw - RawDeltaCRL []byte `json:"rawDeltaCRL,omitempty"` + // DeltaCRL is the ASN.1 encoded delta CRL + DeltaCRL []byte `json:"deltaCRL,omitempty"` } // NewFileCache creates a FileCache with root as the root directory @@ -101,12 +101,12 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error return nil, fmt.Errorf("failed to decode file retrieved from file cache: %w", err) } var bundle corecrl.Bundle - bundle.BaseCRL, err = x509.ParseRevocationList(content.RawBaseCRL) + bundle.BaseCRL, err = x509.ParseRevocationList(content.BaseCRL) if err != nil { return nil, fmt.Errorf("failed to parse base CRL of file retrieved from file cache: %w", err) } - if content.RawDeltaCRL != nil { - bundle.DeltaCRL, err = x509.ParseRevocationList(content.RawDeltaCRL) + if content.DeltaCRL != nil { + bundle.DeltaCRL, err = x509.ParseRevocationList(content.DeltaCRL) if err != nil { return nil, fmt.Errorf("failed to parse delta CRL of file retrieved from file cache: %w", err) } @@ -140,10 +140,10 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) // actual content to be saved in the cache content := fileCacheContent{ - RawBaseCRL: bundle.BaseCRL.Raw, + BaseCRL: bundle.BaseCRL.Raw, } if bundle.DeltaCRL != nil { - content.RawDeltaCRL = bundle.DeltaCRL.Raw + content.DeltaCRL = bundle.DeltaCRL.Raw } // save content to tmp file @@ -151,6 +151,8 @@ 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: failed to create temp file: %w", err) } + defer os.Remove(tmpFile.Name()) + defer tmpFile.Close() err = json.NewEncoder(tmpFile).Encode(content) if err != nil { return fmt.Errorf("failed to store crl bundle in file cache: failed to encode content: %w", err) diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index 1e958ba5..f980156a 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -188,7 +188,7 @@ func TestGetFailed(t *testing.T) { t.Run("empty RawBaseCRL of content", func(t *testing.T) { content := fileCacheContent{ - RawBaseCRL: []byte{}, + BaseCRL: []byte{}, } b, err := json.Marshal(content) if err != nil { @@ -207,7 +207,7 @@ func TestGetFailed(t *testing.T) { t.Run("invalid RawBaseCRL of content", func(t *testing.T) { content := fileCacheContent{ - RawBaseCRL: []byte("invalid"), + BaseCRL: []byte("invalid"), } b, err := json.Marshal(content) if err != nil { @@ -226,8 +226,8 @@ func TestGetFailed(t *testing.T) { t.Run("invalid RawDeltaCRL of content", func(t *testing.T) { content := fileCacheContent{ - RawBaseCRL: baseCRL.Raw, - RawDeltaCRL: []byte("invalid"), + BaseCRL: baseCRL.Raw, + DeltaCRL: []byte("invalid"), } b, err := json.Marshal(content) if err != nil { From c8c74056347b5f692ce15cb2efd04d70cb66f99d Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 16:05:15 +0800 Subject: [PATCH 10/29] fix test Signed-off-by: Patrick Zheng --- dir/fs_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dir/fs_test.go b/dir/fs_test.go index 68203ad1..c985e595 100644 --- a/dir/fs_test.go +++ b/dir/fs_test.go @@ -72,13 +72,13 @@ func TestPluginFS(t *testing.T) { } } -func TestCacheFS(t *testing.T) { - cacheFS := CRLFileCacheFS() - path, err := cacheFS.SysPath() +func TestCRLFileCacheFS(t *testing.T) { + crlFileCacheFS := CRLFileCacheFS() + path, err := crlFileCacheFS.SysPath() if err != nil { t.Fatalf("SysPath() failed. err = %v", err) } - if path != filepath.Join(UserCacheDir) { + if path != filepath.Join(UserCacheDir, PathCRLFileCache) { t.Fatalf(`SysPath() failed. got: %q, want: %q`, path, UserConfigDir) } } From c72826654d463ccfce1b696d505ee21f22eba927 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 16:15:18 +0800 Subject: [PATCH 11/29] update Signed-off-by: Patrick Zheng --- dir/path.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dir/path.go b/dir/path.go index 6c75b28a..c2f8e0fd 100644 --- a/dir/path.go +++ b/dir/path.go @@ -66,8 +66,6 @@ const ( PathOCITrustPolicy = "trustpolicy.oci.json" // PathBlobTrustPolicy is the Blob trust policy file relative path. PathBlobTrustPolicy = "trustpolicy.blob.json" - // PathPlugins is the plugins directory relative path. - PathPlugins = "plugins" // LocalKeysDir is the directory name for local key relative path. LocalKeysDir = "localkeys" // LocalCertificateExtension defines the extension of the certificate files. @@ -78,6 +76,12 @@ const ( TrustStoreDir = "truststore" ) +// The relative path to {NOTATION_LIBEXEC} +const ( + // PathPlugins is the plugins directory relative path. + PathPlugins = "plugins" +) + // The relative path to {NOTATION_CACHE} const ( // PathCRLFileCache is the crl file cache directory relative path. From 6a4876ce20e920a6a550c32e06ded9982bf191f9 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 16:36:27 +0800 Subject: [PATCH 12/29] update Signed-off-by: Patrick Zheng --- dir/fs.go | 6 +++--- dir/fs_test.go | 4 ++-- verifier/crl/crl.go | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dir/fs.go b/dir/fs.go index c0633cd0..eec834a8 100644 --- a/dir/fs.go +++ b/dir/fs.go @@ -59,7 +59,7 @@ func PluginFS() SysFS { return NewSysFS(filepath.Join(userLibexecDirPath(), PathPlugins)) } -// CRLFileCacheFS is the crl file cache SysFS -func CRLFileCacheFS() SysFS { - return NewSysFS(filepath.Join(userCacheDirPath(), PathCRLFileCache)) +// CacheFS is the cache SysFS +func CacheFS() SysFS { + return NewSysFS(userCacheDirPath()) } diff --git a/dir/fs_test.go b/dir/fs_test.go index c985e595..05dffa03 100644 --- a/dir/fs_test.go +++ b/dir/fs_test.go @@ -73,8 +73,8 @@ func TestPluginFS(t *testing.T) { } func TestCRLFileCacheFS(t *testing.T) { - crlFileCacheFS := CRLFileCacheFS() - path, err := crlFileCacheFS.SysPath() + cacheFS := CacheFS() + path, err := cacheFS.SysPath(PathCRLFileCache) if err != nil { t.Fatalf("SysPath() failed. err = %v", err) } diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 6f5e17d6..ef0141b8 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -68,6 +68,8 @@ type fileCacheContent struct { } // NewFileCache creates a FileCache with root as the root directory +// +// An example for root is `dir.CacheFS().SysPath(dir.PathCRLFileCache)` func NewFileCache(root string) (*FileCache, error) { if err := os.MkdirAll(root, 0700); err != nil { return nil, fmt.Errorf("failed to create crl file cache: %w", err) From 661b135dc0364ddc0f9fcf3ad81fea05ab581c0d Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 16:48:27 +0800 Subject: [PATCH 13/29] update Signed-off-by: Patrick Zheng --- dir/fs.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dir/fs.go b/dir/fs.go index eec834a8..8803258a 100644 --- a/dir/fs.go +++ b/dir/fs.go @@ -59,7 +59,10 @@ func PluginFS() SysFS { return NewSysFS(filepath.Join(userLibexecDirPath(), PathPlugins)) } -// CacheFS is the cache SysFS +// CacheFS is the cache SysFS. +// +// To get the directory for crl file cache, one should use +// `CacheFS().SysFS(PathCRLFileCache)`. func CacheFS() SysFS { return NewSysFS(userCacheDirPath()) } From b3878fb53b8f270e0ea803d1d848aae7f7a0b3f0 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 17:05:10 +0800 Subject: [PATCH 14/29] update Signed-off-by: Patrick Zheng --- dir/fs_test.go | 4 ++-- dir/path.go | 4 ++-- verifier/crl/crl.go | 13 ++++++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/dir/fs_test.go b/dir/fs_test.go index 05dffa03..c9129f01 100644 --- a/dir/fs_test.go +++ b/dir/fs_test.go @@ -74,11 +74,11 @@ func TestPluginFS(t *testing.T) { func TestCRLFileCacheFS(t *testing.T) { cacheFS := CacheFS() - path, err := cacheFS.SysPath(PathCRLFileCache) + path, err := cacheFS.SysPath(PathCRLCache) if err != nil { t.Fatalf("SysPath() failed. err = %v", err) } - if path != filepath.Join(UserCacheDir, PathCRLFileCache) { + if path != filepath.Join(UserCacheDir, PathCRLCache) { t.Fatalf(`SysPath() failed. got: %q, want: %q`, path, UserConfigDir) } } diff --git a/dir/path.go b/dir/path.go index c2f8e0fd..7420071e 100644 --- a/dir/path.go +++ b/dir/path.go @@ -84,8 +84,8 @@ const ( // The relative path to {NOTATION_CACHE} const ( - // PathCRLFileCache is the crl file cache directory relative path. - PathCRLFileCache = "crl" + // PathCRLCache is the crl file cache directory relative path. + PathCRLCache = "crl" ) // for unit tests diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index ef0141b8..d19bb525 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -128,7 +128,7 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error } // Set stores the CRL bundle in c with url as key. -func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error { +func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) (setErr error) { logger := log.GetLogger(ctx) logger.Infof("Storing crl bundle to file cache with key %q ...", url) @@ -153,8 +153,15 @@ 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: failed to create temp file: %w", err) } - defer os.Remove(tmpFile.Name()) - defer tmpFile.Close() + defer func() { + if err := tmpFile.Close(); err != nil && setErr == nil { + setErr = fmt.Errorf("failed to close tmpFile while storing crl bundle in file cache: %w", err) + } + // remove the tmp file in case of error. + if setErr != nil { + defer os.Remove(tmpFile.Name()) + } + }() err = json.NewEncoder(tmpFile).Encode(content) if err != nil { return fmt.Errorf("failed to store crl bundle in file cache: failed to encode content: %w", err) From 3b8ecb2523ab731c96b36857f9fd2e77eb95c345 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 17:07:24 +0800 Subject: [PATCH 15/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index d19bb525..4afec662 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -162,14 +162,13 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) defer os.Remove(tmpFile.Name()) } }() - err = json.NewEncoder(tmpFile).Encode(content) - if err != nil { + + if err := json.NewEncoder(tmpFile).Encode(content); err != nil { return fmt.Errorf("failed to store crl bundle in file cache: failed to encode content: %w", err) } // rename is atomic on UNIX-like platforms - err = os.Rename(tmpFile.Name(), filepath.Join(c.root, c.fileName(url))) - if err != nil { + if err := os.Rename(tmpFile.Name(), filepath.Join(c.root, c.fileName(url))); err != nil { return fmt.Errorf("failed to store crl bundle in file cache: %w", err) } return nil From 969ca839633e7f568144f51cd8c96237db9600e6 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 17:09:59 +0800 Subject: [PATCH 16/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 4afec662..c55b8357 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -151,7 +151,7 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) // save content to tmp file tmpFile, err := os.CreateTemp("", tmpFileName) if err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: failed to create temp file: %w", err) + return fmt.Errorf("failed to store crl bundle in file cache: failed to create tmpFile: %w", err) } defer func() { if err := tmpFile.Close(); err != nil && setErr == nil { From 55c361a9d1ab059207c0a0d33a87346c4a11e1b7 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 18:43:31 +0800 Subject: [PATCH 17/29] update Signed-off-by: Patrick Zheng --- dir/fs.go | 3 +-- verifier/crl/crl.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dir/fs.go b/dir/fs.go index 8803258a..93da420e 100644 --- a/dir/fs.go +++ b/dir/fs.go @@ -61,8 +61,7 @@ func PluginFS() SysFS { // CacheFS is the cache SysFS. // -// To get the directory for crl file cache, one should use -// `CacheFS().SysFS(PathCRLFileCache)`. +// To get the root of crl file cache, use `CacheFS().SysFS(PathCRLCache)`. func CacheFS() SysFS { return NewSysFS(userCacheDirPath()) } diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index c55b8357..e736bc28 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -69,7 +69,7 @@ type fileCacheContent struct { // NewFileCache creates a FileCache with root as the root directory // -// An example for root is `dir.CacheFS().SysPath(dir.PathCRLFileCache)` +// An example for root is `dir.CacheFS().SysPath(dir.PathCRLCache)` func NewFileCache(root string) (*FileCache, error) { if err := os.MkdirAll(root, 0700); err != nil { return nil, fmt.Errorf("failed to create crl file cache: %w", err) From 5201cce5bc2402dd3169d6caee27aa81c23e4b37 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Mon, 23 Sep 2024 20:06:17 +0800 Subject: [PATCH 18/29] fix Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index e736bc28..50a3a8e5 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -32,8 +32,8 @@ import ( ) const ( - // tmpFileName is the prefix of the temporary file - tmpFileName = "notation-*" + // tempFileName is the prefix of the temporary file + tempFileName = "notation-*" ) // FileCache implements corecrl.Cache. @@ -148,27 +148,27 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) content.DeltaCRL = bundle.DeltaCRL.Raw } - // save content to tmp file - tmpFile, err := os.CreateTemp("", tmpFileName) + // save content to temp file + tempFile, err := os.CreateTemp("", tempFileName) if err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: failed to create tmpFile: %w", err) + return fmt.Errorf("failed to store crl bundle in file cache: failed to create temp file: %w", err) } defer func() { - if err := tmpFile.Close(); err != nil && setErr == nil { - setErr = fmt.Errorf("failed to close tmpFile while storing crl bundle in file cache: %w", err) - } - // remove the tmp file in case of error. + // remove the temp file in case of error if setErr != nil { - defer os.Remove(tmpFile.Name()) + defer os.Remove(tempFile.Name()) } }() - if err := json.NewEncoder(tmpFile).Encode(content); err != nil { + if err := json.NewEncoder(tempFile).Encode(content); err != nil { return fmt.Errorf("failed to store crl bundle in file cache: failed to encode content: %w", err) } + if err := tempFile.Close(); err != nil { + return fmt.Errorf("failed to store crl bundle in file cache: failed to close temp file: %w", err) + } // rename is atomic on UNIX-like platforms - if err := os.Rename(tmpFile.Name(), filepath.Join(c.root, c.fileName(url))); err != nil { + if err := os.Rename(tempFile.Name(), filepath.Join(c.root, c.fileName(url))); err != nil { return fmt.Errorf("failed to store crl bundle in file cache: %w", err) } return nil From 6bc17bfbffe50e6c14789985f2582b626aad5968 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 08:17:37 +0800 Subject: [PATCH 19/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 50a3a8e5..556787c8 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -83,23 +83,21 @@ func NewFileCache(root string) (*FileCache, error) { // or the content has expired, corecrl.ErrCacheMiss is returned. func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error) { logger := log.GetLogger(ctx) - logger.Infof("Retrieving crl bundle from file cache with key %q ...", url) + logger.Debugf("Retrieving crl bundle from file cache with key %q ...", url) // get content from file cache - f, err := os.Open(filepath.Join(c.root, c.fileName(url))) + contentBytes, err := os.ReadFile(filepath.Join(c.root, c.fileName(url))) if err != nil { if errors.Is(err, fs.ErrNotExist) { - logger.Infof("CRL file cache miss. Key %q does not exist", url) + logger.Debugf("CRL file cache miss. Key %q does not exist", url) return nil, corecrl.ErrCacheMiss } return nil, fmt.Errorf("failed to get crl bundle from file cache with key %q: %w", url, err) } - defer f.Close() // decode content to crl Bundle var content fileCacheContent - err = json.NewDecoder(f).Decode(&content) - if err != nil { + if err := json.Unmarshal(contentBytes, &content); err != nil { return nil, fmt.Errorf("failed to decode file retrieved from file cache: %w", err) } var bundle corecrl.Bundle @@ -130,7 +128,7 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error // Set stores the CRL bundle in c with url as key. func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) (setErr error) { logger := log.GetLogger(ctx) - logger.Infof("Storing crl bundle to file cache with key %q ...", url) + logger.Debugf("Storing crl bundle to file cache with key %q ...", url) // sanity check if bundle == nil { @@ -188,7 +186,7 @@ func checkExpiry(ctx context.Context, nextUpdate time.Time) error { return errors.New("crl bundle retrieved from file cache does not contain valid NextUpdate") } if time.Now().After(nextUpdate) { - logger.Infof("CRL bundle retrieved from file cache has expired at %s", nextUpdate) + logger.Debugf("CRL bundle retrieved from file cache has expired at %s", nextUpdate) return corecrl.ErrCacheMiss } return nil From acc446b803e0ef02faadab63cc0071b86a5e516c Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 09:46:53 +0800 Subject: [PATCH 20/29] log Signed-off-by: Patrick Zheng --- verifier/verifier.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/verifier/verifier.go b/verifier/verifier.go index 0b62904e..17e5cfb2 100644 --- a/verifier/verifier.go +++ b/verifier/verifier.go @@ -816,7 +816,14 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult, } for _, serverResult := range certResult.ServerResults { if serverResult.Error != nil { - // log the revocation error + // log individual server errors + if certResult.RevocationMethod == revocationresult.RevocationMethodOCSPFallbackCRL && serverResult.RevocationMethod == revocationresult.RevocationMethodOCSP { + // 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) + 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) } } From b8fa2c77082445f7a9a2c2f18775b5cba8caaa72 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 14:46:38 +0800 Subject: [PATCH 21/29] update Signed-off-by: Patrick Zheng --- internal/file/file.go | 37 +++++++++++++++++++++++++++++++++++ internal/file/file_test.go | 40 +++++++++++++++++++++++++------------- verifier/crl/crl.go | 31 +++++------------------------ 3 files changed, 69 insertions(+), 39 deletions(-) diff --git a/internal/file/file.go b/internal/file/file.go index 920fa640..823c884b 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -15,6 +15,7 @@ package file import ( "errors" + "fmt" "io" "io/fs" "os" @@ -23,6 +24,11 @@ import ( "strings" ) +const ( + // tempFileName is the prefix of the temporary file + tempFileName = "notation-*" +) + // ErrNotRegularFile is returned when the file is not an regular file. var ErrNotRegularFile = errors.New("not regular file") @@ -110,3 +116,34 @@ func CopyDirToDir(src, dst string) error { func TrimFileExtension(fileName string) string { return strings.TrimSuffix(fileName, filepath.Ext(fileName)) } + +// WriteFile writes content to path with all parent directories created. +// If path already exists and is a file, WriteFile overwrites it. +func WriteFile(path string, content []byte) (writeErr error) { + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + return err + } + tempFile, err := os.CreateTemp("", tempFileName) + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + defer func() { + // remove the temp file in case of error + if writeErr != nil { + tempFile.Close() + os.Remove(tempFile.Name()) + } + }() + + if _, err := tempFile.Write(content); err != nil { + return fmt.Errorf("failed to write content: %w", err) + } + + // close before moving + if err := tempFile.Close(); err != nil { + return fmt.Errorf("failed to close temp file: %w", err) + } + + // rename is atomic on UNIX-like platforms + return os.Rename(tempFile.Name(), path) +} diff --git a/internal/file/file_test.go b/internal/file/file_test.go index a108d0da..67152f49 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" ) @@ -26,7 +27,7 @@ func TestCopyToDir(t *testing.T) { tempDir := t.TempDir() data := []byte("data") filename := filepath.Join(tempDir, "a", "file.txt") - if err := writeFile(filename, data); err != nil { + if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -45,7 +46,7 @@ func TestCopyToDir(t *testing.T) { destDir := t.TempDir() data := []byte("data") filename := filepath.Join(tempDir, "a", "file.txt") - if err := writeFile(filename, data); err != nil { + if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -77,7 +78,7 @@ func TestCopyToDir(t *testing.T) { data := []byte("data") // prepare file filename := filepath.Join(tempDir, "a", "file.txt") - if err := writeFile(filename, data); err != nil { + if err := WriteFile(filename, data); err != nil { t.Fatal(err) } // forbid reading @@ -100,7 +101,7 @@ func TestCopyToDir(t *testing.T) { data := []byte("data") // prepare file filename := filepath.Join(tempDir, "a", "file.txt") - if err := writeFile(filename, data); err != nil { + if err := WriteFile(filename, data); err != nil { t.Fatal(err) } // forbid dest directory operation @@ -123,7 +124,7 @@ func TestCopyToDir(t *testing.T) { data := []byte("data") // prepare file filename := filepath.Join(tempDir, "a", "file.txt") - if err := writeFile(filename, data); err != nil { + if err := WriteFile(filename, data); err != nil { t.Fatal(err) } // forbid writing to destTempDir @@ -140,7 +141,7 @@ func TestCopyToDir(t *testing.T) { tempDir := t.TempDir() data := []byte("data") filename := filepath.Join(tempDir, "a", "file.txt") - if err := writeFile(filename, data); err != nil { + if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -161,6 +162,26 @@ func TestFileNameWithoutExtension(t *testing.T) { } } +func TestWriteFile(t *testing.T) { + tempDir := t.TempDir() + content := []byte("test WriteFile") + + t.Run("permission denied", func(t *testing.T) { + err := os.Chmod(tempDir, 0) + if err != nil { + t.Fatal(err) + } + err = WriteFile(filepath.Join(tempDir, "testFile"), content) + if err == nil || !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("expected permission denied error, but got %s", err) + } + err = os.Chmod(tempDir, 0700) + if err != nil { + t.Fatal(err) + } + }) +} + func validFileContent(t *testing.T, filename string, content []byte) { b, err := os.ReadFile(filename) if err != nil { @@ -170,10 +191,3 @@ func validFileContent(t *testing.T, filename string, content []byte) { t.Fatal("file content is not correct") } } - -func writeFile(path string, data []byte) error { - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { - return err - } - return os.WriteFile(path, data, 0600) -} diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 556787c8..b8c7fd23 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -28,14 +28,10 @@ import ( "time" corecrl "github.com/notaryproject/notation-core-go/revocation/crl" + "github.com/notaryproject/notation-go/internal/file" "github.com/notaryproject/notation-go/log" ) -const ( - // tempFileName is the prefix of the temporary file - tempFileName = "notation-*" -) - // FileCache implements corecrl.Cache. // // Key: url of the CRL. @@ -126,7 +122,7 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error } // Set stores the CRL bundle in c with url as key. -func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) (setErr error) { +func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) error { logger := log.GetLogger(ctx) logger.Debugf("Storing crl bundle to file cache with key %q ...", url) @@ -145,28 +141,11 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) if bundle.DeltaCRL != nil { content.DeltaCRL = bundle.DeltaCRL.Raw } - - // save content to temp file - tempFile, err := os.CreateTemp("", tempFileName) + contentBytes, err := json.Marshal(content) if err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: failed to create temp file: %w", err) - } - defer func() { - // remove the temp file in case of error - if setErr != nil { - defer os.Remove(tempFile.Name()) - } - }() - - if err := json.NewEncoder(tempFile).Encode(content); err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: failed to encode content: %w", err) - } - if err := tempFile.Close(); err != nil { - return fmt.Errorf("failed to store crl bundle in file cache: failed to close temp file: %w", err) + return fmt.Errorf("failed to store crl bundle in file cache: %w", err) } - - // rename is atomic on UNIX-like platforms - if err := os.Rename(tempFile.Name(), filepath.Join(c.root, c.fileName(url))); err != nil { + if err := file.WriteFile(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 From 717e7a1145e8e8639fa252cbe7efc1ec6f7f638e Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 14:48:24 +0800 Subject: [PATCH 22/29] update Signed-off-by: Patrick Zheng --- internal/file/file_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/file/file_test.go b/internal/file/file_test.go index 67152f49..4e007e88 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -167,6 +167,9 @@ func TestWriteFile(t *testing.T) { content := []byte("test WriteFile") t.Run("permission denied", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } err := os.Chmod(tempDir, 0) if err != nil { t.Fatal(err) From 584c76bc76a71639b85a8d9cd76089d29a4d1add Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 15:10:21 +0800 Subject: [PATCH 23/29] update Signed-off-by: Patrick Zheng --- internal/file/file.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/file/file.go b/internal/file/file.go index 823c884b..48124cc5 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -25,8 +25,8 @@ import ( ) const ( - // tempFileName is the prefix of the temporary file - tempFileName = "notation-*" + // tempFileNamePrefix is the prefix of the temporary file + tempFileNamePrefix = "notation-*" ) // ErrNotRegularFile is returned when the file is not an regular file. @@ -123,7 +123,7 @@ func WriteFile(path string, content []byte) (writeErr error) { if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { return err } - tempFile, err := os.CreateTemp("", tempFileName) + tempFile, err := os.CreateTemp("", tempFileNamePrefix) if err != nil { return fmt.Errorf("failed to create temp file: %w", err) } From 44f4c35cf42f008055e41822eca81a0ddf661839 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 15:47:06 +0800 Subject: [PATCH 24/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl.go | 8 ++++++++ verifier/crl/crl_test.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index b8c7fd23..7cc47711 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "io/fs" + nurl "net/url" "os" "path/filepath" "time" @@ -81,6 +82,10 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error logger := log.GetLogger(ctx) logger.Debugf("Retrieving crl bundle from file cache with key %q ...", url) + // sanity check + if _, err := nurl.Parse(url); err != nil { + return nil, fmt.Errorf("invalid url: %w", err) + } // get content from file cache contentBytes, err := os.ReadFile(filepath.Join(c.root, c.fileName(url))) if err != nil { @@ -127,6 +132,9 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) logger.Debugf("Storing crl bundle to file cache with key %q ...", url) // sanity check + if _, err := nurl.Parse(url); err != nil { + return fmt.Errorf("invalid url: %w", err) + } if bundle == nil { return errors.New("failed to store crl bundle in file cache: bundle cannot be nil") } diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index f980156a..e64de716 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -59,7 +59,7 @@ func TestFileCache(t *testing.T) { } }) - key := "testKey" + key := "http://example.com" t.Run("comformance", func(t *testing.T) { bundle := &corecrl.Bundle{BaseCRL: baseCRL} if err := cache.Set(ctx, key, bundle); err != nil { From 997f6d930c7b02c286d700e3d3306bba5633df29 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Tue, 24 Sep 2024 16:27:02 +0800 Subject: [PATCH 25/29] err msg Signed-off-by: Patrick Zheng --- internal/file/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/file/file.go b/internal/file/file.go index 48124cc5..2863e8b0 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -136,7 +136,7 @@ func WriteFile(path string, content []byte) (writeErr error) { }() if _, err := tempFile.Write(content); err != nil { - return fmt.Errorf("failed to write content: %w", err) + return fmt.Errorf("failed to write content to temp file: %w", err) } // close before moving From 69f6b81874ccfab9b81c03463af02cae8565f4db Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 25 Sep 2024 10:38:11 +0800 Subject: [PATCH 26/29] update Signed-off-by: Patrick Zheng --- internal/file/file.go | 3 ++- verifier/crl/crl.go | 9 --------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/internal/file/file.go b/internal/file/file.go index 2863e8b0..97990166 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -117,7 +117,8 @@ func TrimFileExtension(fileName string) string { return strings.TrimSuffix(fileName, filepath.Ext(fileName)) } -// WriteFile writes content to path with all parent directories created. +// WriteFile writes content to a temporary file and move it to path with all +// parent directories created. // If path already exists and is a file, WriteFile overwrites it. func WriteFile(path string, content []byte) (writeErr error) { if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { diff --git a/verifier/crl/crl.go b/verifier/crl/crl.go index 7cc47711..9ebd16db 100644 --- a/verifier/crl/crl.go +++ b/verifier/crl/crl.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "io/fs" - nurl "net/url" "os" "path/filepath" "time" @@ -82,10 +81,6 @@ func (c *FileCache) Get(ctx context.Context, url string) (*corecrl.Bundle, error logger := log.GetLogger(ctx) logger.Debugf("Retrieving crl bundle from file cache with key %q ...", url) - // sanity check - if _, err := nurl.Parse(url); err != nil { - return nil, fmt.Errorf("invalid url: %w", err) - } // get content from file cache contentBytes, err := os.ReadFile(filepath.Join(c.root, c.fileName(url))) if err != nil { @@ -131,10 +126,6 @@ func (c *FileCache) Set(ctx context.Context, url string, bundle *corecrl.Bundle) logger := log.GetLogger(ctx) logger.Debugf("Storing crl bundle to file cache with key %q ...", url) - // sanity check - if _, err := nurl.Parse(url); err != nil { - return fmt.Errorf("invalid url: %w", err) - } if bundle == nil { return errors.New("failed to store crl bundle in file cache: bundle cannot be nil") } From 2a27561e9e97377fcdc5a30c608b55c1837a5a89 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 25 Sep 2024 11:03:29 +0800 Subject: [PATCH 27/29] update Signed-off-by: Patrick Zheng --- verifier/crl/crl_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/verifier/crl/crl_test.go b/verifier/crl/crl_test.go index e64de716..6eafca3c 100644 --- a/verifier/crl/crl_test.go +++ b/verifier/crl/crl_test.go @@ -32,6 +32,21 @@ import ( "github.com/notaryproject/notation-core-go/testhelper" ) +func TestCache(t *testing.T) { + t.Run("file cache implement Cache interface", func(t *testing.T) { + root := t.TempDir() + var coreCache corecrl.Cache + var err error + coreCache, err = NewFileCache(root) + if err != nil { + t.Fatal(err) + } + if _, ok := coreCache.(*FileCache); !ok { + t.Fatal("FileCache does not implement coreCache") + } + }) +} + func TestFileCache(t *testing.T) { now := time.Now() certChain := testhelper.GetRevokableRSAChainWithRevocations(2, false, true) @@ -152,6 +167,10 @@ func TestGetFailed(t *testing.T) { } t.Run("no permission to read file", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + if err := os.Chmod(invalidFile, 0); err != nil { t.Fatal(err) } @@ -356,6 +375,10 @@ func TestSetFailed(t *testing.T) { }) t.Run("failed to write into cache due to permission denied", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on Windows") + } + if err := os.Chmod(tempDir, 0); err != nil { t.Fatal(err) } From c3974c3d754dff40dc206d523ec348ac596dc470 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 25 Sep 2024 11:24:29 +0800 Subject: [PATCH 28/29] update Signed-off-by: Patrick Zheng --- internal/file/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/file/file.go b/internal/file/file.go index 97990166..974dabc2 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -117,7 +117,7 @@ func TrimFileExtension(fileName string) string { return strings.TrimSuffix(fileName, filepath.Ext(fileName)) } -// WriteFile writes content to a temporary file and move it to path with all +// WriteFile writes content to a temporary file and moves it to path with all // parent directories created. // If path already exists and is a file, WriteFile overwrites it. func WriteFile(path string, content []byte) (writeErr error) { From 8df277a74a181a692ebd9e06114b404a96ccdea9 Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Wed, 25 Sep 2024 11:53:56 +0800 Subject: [PATCH 29/29] update Signed-off-by: Patrick Zheng --- internal/file/file.go | 6 +----- internal/file/file_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/internal/file/file.go b/internal/file/file.go index 974dabc2..c6e95849 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -117,13 +117,9 @@ func TrimFileExtension(fileName string) string { return strings.TrimSuffix(fileName, filepath.Ext(fileName)) } -// WriteFile writes content to a temporary file and moves it to path with all -// parent directories created. +// 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) { - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { - return err - } tempFile, err := os.CreateTemp("", 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 4e007e88..306a0a5b 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -27,6 +27,9 @@ func TestCopyToDir(t *testing.T) { tempDir := t.TempDir() data := []byte("data") filename := filepath.Join(tempDir, "a", "file.txt") + if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { + t.Fatal(err) + } if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -46,6 +49,9 @@ func TestCopyToDir(t *testing.T) { destDir := t.TempDir() data := []byte("data") filename := filepath.Join(tempDir, "a", "file.txt") + if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { + t.Fatal(err) + } if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -78,6 +84,9 @@ func TestCopyToDir(t *testing.T) { data := []byte("data") // prepare file filename := filepath.Join(tempDir, "a", "file.txt") + if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { + t.Fatal(err) + } if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -101,6 +110,9 @@ func TestCopyToDir(t *testing.T) { data := []byte("data") // prepare file filename := filepath.Join(tempDir, "a", "file.txt") + if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { + t.Fatal(err) + } if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -124,6 +136,9 @@ func TestCopyToDir(t *testing.T) { data := []byte("data") // prepare file filename := filepath.Join(tempDir, "a", "file.txt") + if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { + t.Fatal(err) + } if err := WriteFile(filename, data); err != nil { t.Fatal(err) } @@ -141,6 +156,9 @@ func TestCopyToDir(t *testing.T) { tempDir := t.TempDir() data := []byte("data") filename := filepath.Join(tempDir, "a", "file.txt") + if err := os.MkdirAll(filepath.Dir(filename), 0700); err != nil { + t.Fatal(err) + } if err := WriteFile(filename, data); err != nil { t.Fatal(err) }