Handle expired OCSP responses from server

changelog/24193.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+auth/cert: Handle errors related to expired OCSP server responses
+```

sdk/helper/ocsp/client.go

@@ -517,6 +517,11 @@ func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509.
 			}
 			if isValidOCSPStatus(ret.code) {
 				ocspStatuses[i] = ret
+			} else if ret.err != nil {
+				// This check needs to occur after the isValidOCSPStatus as the unknown
+				// status also sets an err value within ret.
+				errors[i] = ret.err
+				return ret.err
 			}
 			return nil
 		}
@@ -568,8 +573,12 @@ func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509.
 	if (ret == nil || ret.code == ocspStatusUnknown) && firstError != nil {
 		return nil, firstError
 	}
-	// otherwise ret should contain a response for the overall request
+	// An extra safety in case ret and firstError are both nil
+	if ret == nil {
+		return nil, fmt.Errorf("failed to extract a known response code or error from the OCSP server")
+	}
 
+	// otherwise ret should contain a response for the overall request
 	if !isValidOCSPStatus(ret.code) {
 		return ret, nil
 	}

sdk/helper/ocsp/ocsp_test.go

@@ -6,21 +6,28 @@ import (
 	"bytes"
 	"context"
 	"crypto"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
 	"crypto/tls"
 	"crypto/x509"
+	"crypto/x509/pkix"
 	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
+	"math/big"
 	"net"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"testing"
 	"time"
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-retryablehttp"
 	lru "github.com/hashicorp/golang-lru"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ocsp"
 )
 
@@ -201,6 +208,88 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) {
 	}
 }
 
+func TestUnitExpiredOCSPResponse(t *testing.T) {
+	rootCaKey, rootCa, leafCert := createCaLeafCerts(t)
+
+	expiredOcspResponse := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		now := time.Now()
+		ocspRes := ocsp.Response{
+			SerialNumber: big.NewInt(2),
+			ThisUpdate:   now.Add(-1 * time.Hour),
+			NextUpdate:   now.Add(-30 * time.Minute),
+			Status:       ocsp.Good,
+		}
+		response, err := ocsp.CreateResponse(rootCa, rootCa, ocspRes, rootCaKey)
+		if err != nil {
+			_, _ = w.Write(ocsp.InternalErrorErrorResponse)
+			t.Fatalf("failed generating OCSP response: %v", err)
+		}
+		_, _ = w.Write(response)
+	})
+	ts := httptest.NewServer(expiredOcspResponse)
+	defer ts.Close()
+
+	logFactory := func() hclog.Logger {
+		return hclog.NewNullLogger()
+	}
+	client := New(logFactory, 100)
+
+	ctx := context.Background()
+
+	config := &VerifyConfig{
+		OcspEnabled:         true,
+		OcspServersOverride: []string{ts.URL},
+		OcspFailureMode:     FailOpenFalse,
+		QueryAllServers:     false,
+	}
+
+	status, err := client.GetRevocationStatus(ctx, leafCert, rootCa, config)
+	require.ErrorContains(t, err, "invalid validity",
+		"Expected error got response: %v, %v", status, err)
+}
+
+func createCaLeafCerts(t *testing.T) (*ecdsa.PrivateKey, *x509.Certificate, *x509.Certificate) {
+	rootCaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err, "failed generated root key for CA")
+
+	// Validate we reject CSRs that contain CN that aren't in the original order
+	cr := &x509.Certificate{
+		Subject:               pkix.Name{CommonName: "Root Cert"},
+		SerialNumber:          big.NewInt(1),
+		IsCA:                  true,
+		BasicConstraintsValid: true,
+		SignatureAlgorithm:    x509.ECDSAWithSHA256,
+		NotBefore:             time.Now().Add(-1 * time.Second),
+		NotAfter:              time.Now().AddDate(1, 0, 0),
+		KeyUsage:              x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageOCSPSigning},
+	}
+	rootCaBytes, err := x509.CreateCertificate(rand.Reader, cr, cr, &rootCaKey.PublicKey, rootCaKey)
+	require.NoError(t, err, "failed generating root ca")
+
+	rootCa, err := x509.ParseCertificate(rootCaBytes)
+	require.NoError(t, err, "failed parsing root ca")
+
+	leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err, "failed generated leaf key")
+
+	cr = &x509.Certificate{
+		Subject:            pkix.Name{CommonName: "Leaf Cert"},
+		SerialNumber:       big.NewInt(2),
+		SignatureAlgorithm: x509.ECDSAWithSHA256,
+		NotBefore:          time.Now().Add(-1 * time.Second),
+		NotAfter:           time.Now().AddDate(1, 0, 0),
+		KeyUsage:           x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:        []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+	}
+	leafCertBytes, err := x509.CreateCertificate(rand.Reader, cr, rootCa, &leafKey.PublicKey, rootCaKey)
+	require.NoError(t, err, "failed generating root ca")
+
+	leafCert, err := x509.ParseCertificate(leafCertBytes)
+	require.NoError(t, err, "failed parsing root ca")
+	return rootCaKey, rootCa, leafCert
+}
+
 func TestUnitValidateOCSP(t *testing.T) {
 	ocspRes := &ocsp.Response{}
 	ost, err := validateOCSP(ocspRes)