Handle expired OCSP responses from server (#24193)

* Handle expired OCSP responses from server

 - If a server replied with what we considered an expired OCSP response (nextUpdate is now or in the past), and it was our only response we would panic due to missing error handling logic.

* Add cl

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,14 +6,20 @@ 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"
@@ -21,6 +27,7 @@ import (
 	"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)