More validation of delegated OCSP responders (#378)

* More validation of delegated OCSP responders

* Add tests

Author: mholt

ocsp.go

@@ -235,6 +235,10 @@ func getOCSPForCert(ocspConfig OCSPConfig, bundle []byte) ([]byte, *ocsp.Respons
 		return nil, nil, fmt.Errorf("parsing OCSP response: %v", err)
 	}
 
+	if err := validateOCSPResponder(ocspRes, issuerCert); err != nil {
+		return nil, nil, fmt.Errorf("OCSP responder authorization check failed: %v", err)
+	}
+
 	return ocspResBytes, ocspRes, nil
 }
 
@@ -253,3 +257,26 @@ func freshOCSP(resp *ocsp.Response) bool {
 	refreshTime := resp.ThisUpdate.Add(nextUpdate.Sub(resp.ThisUpdate) / 2)
 	return time.Now().Before(refreshTime)
 }
+
+// validateOCSPResponder enforces RFC 6960 §4.2.2.2: "Systems or applications that
+// rely on OCSP responses MUST be capable of detecting and enforcing the use of the
+// id-kp-OCSPSigning value." An issuer-signed response (where the embedded Certificate
+// field is nil, meaning the issuer signed directly) is always acceptable.
+func validateOCSPResponder(ocspResp *ocsp.Response, issuerCert *x509.Certificate) error {
+	respCert := ocspResp.Certificate
+
+	// if response was signed directly by the issuer, or embedded responder cert IS the issuer, accept
+	if respCert == nil || respCert.Equal(issuerCert) {
+		// Response was signed directly by the issuer — always valid.
+		return nil
+	}
+
+	// RFC 6960 §4.2.2.2 requires id-kp-OCSPSigning for delegated responders
+	for _, eku := range respCert.ExtKeyUsage {
+		if eku == x509.ExtKeyUsageOCSPSigning {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("OCSP responder certificate (subject: %s) is not the issuer and does not carry id-kp-OCSPSigning", respCert.Subject)
+}

ocsp_test.go

@@ -4,10 +4,13 @@ import (
 	"bytes"
 	"context"
 	"crypto"
+	"crypto/x509"
+	"crypto/x509/pkix"
 	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"golang.org/x/crypto/ocsp"
@@ -153,6 +156,69 @@ func TestStapleOCSP(t *testing.T) {
 	})
 }
 
+func TestValidateOCSPResponder(t *testing.T) {
+	issuer := mustMakeCertificate(t, caCert, caKey).Leaf
+
+	tests := []struct {
+		name    string
+		resp    *ocsp.Response
+		wantErr string
+	}{
+		{
+			name: "issuer signed response with no embedded cert",
+			resp: &ocsp.Response{Certificate: nil},
+		},
+		{
+			name: "embedded responder cert is issuer cert",
+			resp: &ocsp.Response{Certificate: issuer},
+		},
+		{
+			name: "delegated responder with OCSP signing eku",
+			resp: &ocsp.Response{Certificate: &x509.Certificate{
+				Subject: pkix.Name{CommonName: "Delegated OCSP Responder"},
+				ExtKeyUsage: []x509.ExtKeyUsage{
+					x509.ExtKeyUsageServerAuth,
+					x509.ExtKeyUsageOCSPSigning,
+				},
+			}},
+		},
+		{
+			name: "delegated responder without OCSP signing eku",
+			resp: &ocsp.Response{Certificate: &x509.Certificate{
+				Subject:     pkix.Name{CommonName: "Not Authorized"},
+				ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+			}},
+			wantErr: "does not carry id-kp-OCSPSigning",
+		},
+		{
+			name: "delegated responder with empty eku",
+			resp: &ocsp.Response{Certificate: &x509.Certificate{
+				Subject: pkix.Name{CommonName: "No EKU"},
+			}},
+			wantErr: "does not carry id-kp-OCSPSigning",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateOCSPResponder(tc.resp, issuer)
+			if tc.wantErr == "" {
+				if err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
+				return
+			}
+
+			if err == nil {
+				t.Fatalf("expected error containing %q, got nil", tc.wantErr)
+			}
+			if !strings.Contains(err.Error(), tc.wantErr) {
+				t.Fatalf("expected error containing %q, got %q", tc.wantErr, err.Error())
+			}
+		})
+	}
+}
+
 func mustMakeCertificate(t *testing.T, cert, key string) Certificate {
 	t.Helper()
 	c, err := makeCertificate([]byte(cert), []byte(key))