updating to reflect new changes in notation-core-go PR #134

Signed-off-by: Kody Kimberl <[email protected]>
notaryproject · Mar 30, 2023 · e6275da · e6275da
1 parent 5eb8697
commit e6275da
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 37 deletions.
diff --git a/verifier/verifier.go b/verifier/verifier.go
@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"github.com/notaryproject/notation-core-go/revocation"
-	"github.com/notaryproject/notation-core-go/revocation/ocsp"
 	"github.com/notaryproject/notation-core-go/signature"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/dir"
@@ -550,40 +549,32 @@ func verifyRevocation(outcome *notation.VerificationOutcome, client *http.Client
 	authenticSigningTime := outcome.EnvelopeContent.SignerInfo.SignedAttributes.SigningTime
 	// TODO use authenticSigningTime from signerInfo
 	// https://github.com/notaryproject/notation-core-go/issues/38
-	revocationErr := r.Validate(outcome.EnvelopeContent.SignerInfo.CertificateChain, authenticSigningTime)
+	revocationErrs := r.Validate(outcome.EnvelopeContent.SignerInfo.CertificateChain, authenticSigningTime)
 
 	result := &notation.ValidationResult{
 		Type:   trustpolicy.TypeRevocation,
 		Action: outcome.VerificationLevel.Enforcement[trustpolicy.TypeRevocation],
 	}
 
-	if revocationErr != nil {
-		// If there is an error and it is designated as unimportant, log but pass the validation
-		switch t := revocationErr.(type) {
-		case ocsp.RevokedError:
-			result.Error = t
-		case ocsp.UnknownStatusError:
-			result.Error = t
-		case ocsp.TimeoutError:
-			logger.Debug("Revocation unavailable. OCSP requests timed out")
-			result.Error = t
-		case ocsp.PKIXNoCheckError:
-			// Since CRL is not yet implemented, not considering this an error
-			// Logging and passing validation
-			// TODO: add CRL support
-			// https://github.com/notaryproject/notation-core-go/issues/125
-			logger.Debugf("%v", t)
-		case ocsp.NoOCSPServerError:
-			// Since OCSP cannot ever be checked for this cert, not considering this an error
-			// Logging and passing validation
-			logger.Debugf("%v", t)
-			logger.Debug("1 or more certificates in the chain did not have OCSP configured. Ignoring these certificates")
-		default:
-			// For any ocsp.CheckOCSPErrors or other errors
-			result.Error = revocationErr
+	// Log all non-nil errors as debug
+	for i, serverErrs := range revocationErrs {
+		for _, err := range serverErrs {
+			if err != nil {
+				logger.Debugf("error for certificate #%d in chain: %v", (i + 1), err)
+			}
 		}
 	}
 
+	switch revocation.GetResultFromErrors(revocationErrs) {
+	case revocation.OK:
+		logger.Debug("no important errors encountered while checking revocation, status is OK")
+	case revocation.Revoked:
+		result.Error = errors.New("one or more certificates in the chain were determined to be revoked, status is Revoked")
+	default:
+		// revocation.Unknown
+		result.Error = errors.New("important error(s) encountered during revocation check, status is Unknown")
+	}
+
 	return result
 }
 

diff --git a/verifier/verifier_test.go b/verifier/verifier_test.go
@@ -344,6 +344,9 @@ func TestVerifyRevocation(t *testing.T) {
 	pkixNoCheckClient := testhelper.MockClient(revokableTuples, []ocsp.ResponseStatus{ocsp.Unknown}, nil, false)
 	timeoutClient := &http.Client{Timeout: 1 * time.Nanosecond}
 
+	unknownMsg := "important error(s) encountered during revocation check, status is Unknown"
+	revokedMsg := "one or more certificates in the chain were determined to be revoked, status is Revoked"
+
 	t.Run("verifyRevocation non-revoked", func(t *testing.T) {
 		result := verifyRevocation(createMockOutcome(revokableChain), goodClient, logger)
 		if result.Error != nil {
@@ -352,29 +355,26 @@ func TestVerifyRevocation(t *testing.T) {
 	})
 	t.Run("verifyRevocation OCSP revoked", func(t *testing.T) {
 		result := verifyRevocation(createMockOutcome(revokableChain), revokedClient, logger)
-		expectedMsg := "certificate is revoked via OCSP"
-		if result.Error == nil || result.Error.Error() != expectedMsg {
-			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", expectedMsg, result.Error)
+		if result.Error == nil || result.Error.Error() != revokedMsg {
+			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", revokedMsg, result.Error)
 		}
 	})
 	t.Run("verifyRevocation OCSP unknown", func(t *testing.T) {
 		result := verifyRevocation(createMockOutcome(revokableChain), unknownClient, logger)
-		expectedMsg := "certificate has unknown status via OCSP"
-		if result.Error == nil || result.Error.Error() != expectedMsg {
-			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", expectedMsg, result.Error)
+		if result.Error == nil || result.Error.Error() != unknownMsg {
+			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", unknownMsg, result.Error)
 		}
 	})
 	t.Run("verifyRevocation missing id-pkix-ocsp-nocheck", func(t *testing.T) {
 		result := verifyRevocation(createMockOutcome(revokableChain), pkixNoCheckClient, logger)
-		if result.Error != nil {
-			t.Fatalf("expected verifyRevocation to succeed, but got %v", result.Error)
+		if result.Error == nil || result.Error.Error() != unknownMsg {
+			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", unknownMsg, result.Error)
 		}
 	})
 	t.Run("verifyRevocation timeout", func(t *testing.T) {
 		result := verifyRevocation(createMockOutcome(revokableChain), timeoutClient, logger)
-		expectedMsg := fmt.Sprintf("exceeded timeout threshold of %.2f seconds for OCSP check", timeoutClient.Timeout.Seconds())
-		if result.Error == nil || result.Error.Error() != expectedMsg {
-			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", expectedMsg, result.Error)
+		if result.Error == nil || result.Error.Error() != unknownMsg {
+			t.Fatalf("expected verifyRevocation to fail with %s, but got %v", unknownMsg, result.Error)
 		}
 	})
 }