From dec2a8c934cd7e8cfa0419b11c6f568b2aec3b25 Mon Sep 17 00:00:00 2001
From: Junjie Gao <junjiegao@microsoft.com>
Date: Wed, 21 Aug 2024 07:44:42 +0000
Subject: [PATCH] fix: update

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
---
 revocation/internal/crl/crl.go      | 37 +++--------------------------
 revocation/internal/crl/crl_test.go | 30 -----------------------
 revocation/revocation.go            |  3 +--
 3 files changed, 4 insertions(+), 66 deletions(-)

diff --git a/revocation/internal/crl/crl.go b/revocation/internal/crl/crl.go
index 944d6439..134e8117 100644
--- a/revocation/internal/crl/crl.go
+++ b/revocation/internal/crl/crl.go
@@ -64,41 +64,10 @@ type CertCheckStatusOptions struct {
 // If the invalidity date extension is present in the CRL entry and SigningTime
 // is not zero, the certificate is considered revoked if the SigningTime is
 // after the invalidity date. (See RFC 5280, Section 5.3.2)
+//
+// Please ensure that the certificate supports CRL by calling Supported before
+// calling this function.
 func CertCheckStatus(ctx context.Context, cert, issuer *x509.Certificate, opts CertCheckStatusOptions) *result.CertRevocationResult {
-	if cert == nil {
-		return &result.CertRevocationResult{
-			Result: result.ResultUnknown,
-			CRLResults: []*result.CRLResult{{
-				Error: errors.New("certificate should not be nil"),
-			}},
-		}
-	}
-	if issuer == nil {
-		return &result.CertRevocationResult{
-			Result: result.ResultUnknown,
-			CRLResults: []*result.CRLResult{{
-				Error: errors.New("issuer certificate should not be nil"),
-			}},
-		}
-	}
-	if !Supported(cert) {
-		return &result.CertRevocationResult{
-			Result: result.ResultNonRevokable,
-			CRLResults: []*result.CRLResult{{
-				Error: fmt.Errorf("certificate %s does not support CRL", cert.Subject.CommonName),
-			}},
-		}
-	}
-
-	if opts.HTTPClient == nil {
-		return &result.CertRevocationResult{
-			Result: result.ResultUnknown,
-			CRLResults: []*result.CRLResult{{
-				Error: errors.New("HTTP client is nil"),
-			}},
-		}
-	}
-
 	// The CRLDistributionPoints contains the URIs of all the CRL distribution
 	// points. Since it does not distinguish the reason field, it needs to check
 	// all the URIs to avoid missing any partial CRLs.
diff --git a/revocation/internal/crl/crl_test.go b/revocation/internal/crl/crl_test.go
index 540e7d2e..97f45b02 100644
--- a/revocation/internal/crl/crl_test.go
+++ b/revocation/internal/crl/crl_test.go
@@ -33,21 +33,6 @@ import (
 )
 
 func TestCertCheckStatus(t *testing.T) {
-	t.Run("certificate is nil", func(t *testing.T) {
-		r := CertCheckStatus(context.Background(), nil, nil, CertCheckStatusOptions{})
-		if r.CRLResults[0].Error.Error() != "certificate should not be nil" {
-			t.Fatalf("unexpected error, got %v", r.CRLResults[0].Error)
-		}
-	})
-	t.Run("certificate does not support CRL", func(t *testing.T) {
-		r := CertCheckStatus(context.Background(), &x509.Certificate{}, &x509.Certificate{}, CertCheckStatusOptions{
-			HTTPClient: http.DefaultClient,
-		})
-		if r.CRLResults[0].Error == nil {
-			t.Fatal("expected error")
-		}
-	})
-
 	t.Run("download error", func(t *testing.T) {
 		cert := &x509.Certificate{
 			CRLDistributionPoints: []string{"http://example.com"},
@@ -182,21 +167,6 @@ func TestCertCheckStatus(t *testing.T) {
 			t.Fatal("expected ErrDeltaCRLNotChecked")
 		}
 	})
-
-	t.Run("nil issuer", func(t *testing.T) {
-		r := CertCheckStatus(context.Background(), chain[0].Cert, nil, CertCheckStatusOptions{})
-		if r.CRLResults[0].Error.Error() != "issuer certificate should not be nil" {
-			t.Fatalf("unexpected error, got %v", r.CRLResults[0].Error)
-		}
-	})
-
-	t.Run("http client is nil", func(t *testing.T) {
-		// failed to download CRL with a mocked HTTP client
-		r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{})
-		if r.Result != result.ResultUnknown {
-			t.Fatalf("expected Unknown, got %s", r.Result)
-		}
-	})
 }
 
 func TestValidate(t *testing.T) {
diff --git a/revocation/revocation.go b/revocation/revocation.go
index 25e97f41..da52229f 100644
--- a/revocation/revocation.go
+++ b/revocation/revocation.go
@@ -157,11 +157,11 @@ func (r *revocation) Validate(certChain []*x509.Certificate, signingTime time.Ti
 //
 // NOTE: The certificate chain is expected to be in the order of leaf to root.
 func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts ValidateContextOptions) ([]*result.CertRevocationResult, error) {
+	// validate certificate chain
 	if len(validateContextOpts.CertChain) == 0 {
 		return nil, result.InvalidChainError{Err: errors.New("chain does not contain any certificates")}
 	}
 	certChain := validateContextOpts.CertChain
-
 	if err := x509util.ValidateChain(certChain, r.certChainPurpose); err != nil {
 		return nil, err
 	}
@@ -170,7 +170,6 @@ func (r *revocation) ValidateContext(ctx context.Context, validateContextOpts Va
 		HTTPClient:  r.ocspHTTPClient,
 		SigningTime: validateContextOpts.AuthenticSigningTime,
 	}
-
 	crlOpts := crl.CertCheckStatusOptions{
 		HTTPClient:  r.crlHTTPClient,
 		SigningTime: validateContextOpts.AuthenticSigningTime,