backport: timestamping cert chain revocation check during signing from main to release-1.3 branch (#1121)

Signed-off-by: Patrick Zheng <[email protected]>

Author: Two-Hearts

cmd/notation/sign.go

@@ -23,13 +23,15 @@ import (
 	"strings"
 	"time"
 
+	"github.com/notaryproject/notation-core-go/revocation/purpose"
 	corex509 "github.com/notaryproject/notation-core-go/x509"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/log"
 	"github.com/notaryproject/notation/cmd/notation/internal/experimental"
 	"github.com/notaryproject/notation/internal/cmd"
 	"github.com/notaryproject/notation/internal/envelope"
 	"github.com/notaryproject/notation/internal/httputil"
+	clirev "github.com/notaryproject/notation/internal/revocation"
 	nx509 "github.com/notaryproject/notation/internal/x509"
 	"github.com/notaryproject/tspclient-go"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -251,6 +253,11 @@ func prepareSigningOpts(ctx context.Context, opts *signOpts) (notation.SignOptio
 		rootCAs := x509.NewCertPool()
 		rootCAs.AddCert(tsaRootCert)
 		signOpts.TSARootCAs = rootCAs
+		tsaRevocationValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping)
+		if err != nil {
+			return notation.SignOptions{}, fmt.Errorf("failed to create timestamping revocation validator: %w", err)
+		}
+		signOpts.TSARevocationValidator = tsaRevocationValidator
 	}
 	return signOpts, nil
 }

cmd/notation/verify.go

@@ -18,28 +18,22 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
-	"net/http"
 	"os"
 	"reflect"
-	"time"
 
-	"github.com/notaryproject/notation-core-go/revocation"
 	"github.com/notaryproject/notation-core-go/revocation/purpose"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/dir"
 	"github.com/notaryproject/notation-go/plugin"
 	"github.com/notaryproject/notation-go/verifier"
-	"github.com/notaryproject/notation-go/verifier/crl"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
 	"github.com/notaryproject/notation-go/verifier/truststore"
 	"github.com/notaryproject/notation/cmd/notation/internal/experimental"
 	"github.com/notaryproject/notation/internal/cmd"
-	"github.com/notaryproject/notation/internal/httputil"
 	"github.com/notaryproject/notation/internal/ioutil"
 	"github.com/spf13/cobra"
 
-	corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
-	clicrl "github.com/notaryproject/notation/internal/crl"
+	clirev "github.com/notaryproject/notation/internal/revocation"
 )
 
 type verifyOpts struct {
@@ -234,39 +228,11 @@ func printMetadataIfPresent(outcome *notation.VerificationOutcome) {
 
 func getVerifier(ctx context.Context) (notation.Verifier, error) {
 	// 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}))
+	revocationCodeSigningValidator, err := clirev.NewRevocationValidator(ctx, purpose.CodeSigning)
 	if err != nil {
 		return nil, err
 	}
-	crlFetcher.DiscardCacheError = true // discard crl cache error
-	cacheRoot, err := dir.CacheFS().SysPath(dir.PathCRLCache)
-	if err != nil {
-		return nil, err
-	}
-	fileCache, err := crl.NewFileCache(cacheRoot)
-	if err != nil {
-		// discard NewFileCache error as cache errors are not critical
-		fmt.Fprintf(os.Stderr, "Warning: %v\n", err)
-	} else {
-		crlFetcher.Cache = &clicrl.CacheWithLog{
-			Cache:             fileCache,
-			DiscardCacheError: crlFetcher.DiscardCacheError,
-		}
-	}
-	revocationCodeSigningValidator, err := revocation.NewWithOptions(revocation.Options{
-		OCSPHTTPClient:   ocspHttpClient,
-		CRLFetcher:       crlFetcher,
-		CertChainPurpose: purpose.CodeSigning,
-	})
-	if err != nil {
-		return nil, err
-	}
-	revocationTimestampingValidator, err := revocation.NewWithOptions(revocation.Options{
-		OCSPHTTPClient:   ocspHttpClient,
-		CRLFetcher:       crlFetcher,
-		CertChainPurpose: purpose.Timestamping,
-	})
+	revocationTimestampingValidator, err := clirev.NewRevocationValidator(ctx, purpose.Timestamping)
 	if err != nil {
 		return nil, err
 	}

internal/crl/crl.go renamed to internal/revocation/crl/crl.go

@@ -0,0 +1,54 @@
+// 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 revocation
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"os"
+	"time"
+
+	"github.com/notaryproject/notation-core-go/revocation"
+	corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
+	"github.com/notaryproject/notation-core-go/revocation/purpose"
+	"github.com/notaryproject/notation-go/dir"
+	"github.com/notaryproject/notation-go/verifier/crl"
+	"github.com/notaryproject/notation/internal/httputil"
+	clicrl "github.com/notaryproject/notation/internal/revocation/crl"
+)
+
+// NewRevocationValidator returns a revocation.Validator given the certificate
+// purpose
+func NewRevocationValidator(ctx context.Context, purpose purpose.Purpose) (revocation.Validator, error) {
+	// err is always nil
+	crlFetcher, _ := corecrl.NewHTTPFetcher(httputil.NewClient(ctx, &http.Client{Timeout: 5 * time.Second}))
+	crlFetcher.DiscardCacheError = true                     // discard crl cache error
+	cacheRoot, _ := dir.CacheFS().SysPath(dir.PathCRLCache) // err is always nil
+	fileCache, err := crl.NewFileCache(cacheRoot)
+	if err != nil {
+		// discard NewFileCache error as cache errors are not critical
+		fmt.Fprintf(os.Stderr, "Warning: %v\n", err)
+	} else {
+		crlFetcher.Cache = &clicrl.CacheWithLog{
+			Cache:             fileCache,
+			DiscardCacheError: crlFetcher.DiscardCacheError,
+		}
+	}
+	return revocation.NewWithOptions(revocation.Options{
+		OCSPHTTPClient:   httputil.NewClient(ctx, &http.Client{Timeout: 2 * time.Second}),
+		CRLFetcher:       crlFetcher,
+		CertChainPurpose: purpose,
+	})
+}

internal/crl/crl_test.go renamed to internal/revocation/crl/crl_test.go

@@ -0,0 +1,75 @@
+// 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 revocation
+
+import (
+	"context"
+	"net/http"
+	"os"
+	"runtime"
+	"testing"
+	"time"
+
+	corecrl "github.com/notaryproject/notation-core-go/revocation/crl"
+	"github.com/notaryproject/notation-core-go/revocation/purpose"
+	"github.com/notaryproject/notation-go/dir"
+	"github.com/notaryproject/notation/internal/httputil"
+)
+
+func TestNewRevocationValidator(t *testing.T) {
+	defer func(oldCacheDir string) {
+		dir.UserCacheDir = oldCacheDir
+	}(dir.UserCacheDir)
+
+	t.Run("Success", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("skipping test on Windows")
+		}
+		if _, err := NewRevocationValidator(context.Background(), purpose.Timestamping); err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	tempRoot := t.TempDir()
+	t.Run("Success but without permission to create cache directory", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("skipping test on Windows")
+		}
+		dir.UserCacheDir = tempRoot
+		if err := os.Chmod(tempRoot, 0); err != nil {
+			t.Fatal(err)
+		}
+		defer func() {
+			// restore permission
+			if err := os.Chmod(tempRoot, 0755); err != nil {
+				t.Fatalf("failed to change permission: %v", err)
+			}
+		}()
+		if _, err := NewRevocationValidator(context.Background(), purpose.Timestamping); err != nil {
+			t.Fatal(err)
+		}
+	})
+}
+
+func TestNilError(t *testing.T) {
+	_, err := corecrl.NewHTTPFetcher(httputil.NewClient(context.Background(), &http.Client{Timeout: 5 * time.Second}))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = dir.CacheFS().SysPath(dir.PathCRLCache)
+	if err != nil {
+		t.Fatal(err)
+	}
+}

internal/revocation/revocation.go

@@ -229,7 +229,7 @@ var _ = Describe("notation verify", func() {
 			notation.Exec("sign", "--timestamp-url", "http://timestamp.digicert.com", "--timestamp-root-cert", filepath.Join(NotationE2EConfigPath, "timestamp", "DigiCertTSARootSHA384.cer"), artifact.ReferenceWithDigest()).
 				MatchKeyWords(SignSuccessfully)
 
-			notation.Exec("verify", artifact.ReferenceWithDigest(), "-v").
+			notation.Exec("verify", artifact.ReferenceWithDigest(), "-d").
 				MatchKeyWords(VerifySuccessfully).
 				MatchErrKeyWords("Timestamp verification disabled")
 		})
@@ -251,7 +251,7 @@ var _ = Describe("notation verify", func() {
 			notation.Exec("sign", "--timestamp-url", "http://timestamp.digicert.com", "--timestamp-root-cert", filepath.Join(NotationE2EConfigPath, "timestamp", "DigiCertTSARootSHA384.cer"), artifact.ReferenceWithDigest()).
 				MatchKeyWords(SignSuccessfully)
 
-			notation.Exec("verify", artifact.ReferenceWithDigest(), "-v").
+			notation.Exec("verify", artifact.ReferenceWithDigest(), "-d").
 				MatchKeyWords(VerifySuccessfully).
 				MatchErrKeyWords("Timestamp verification disabled: verifyTimestamp is set to \"afterCertExpiry\" and signing cert chain unexpired")
 		})