Skip to content

Commit

Permalink
backport of commit bcbd45b (#24198)
Browse files Browse the repository at this point in the history
Co-authored-by: Steven Clark <[email protected]>
  • Loading branch information
1 parent 0930024 commit d1a1dd4
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 1 deletion.
3 changes: 3 additions & 0 deletions changelog/24193.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
auth/cert: Handle errors related to expired OCSP server responses
```
11 changes: 10 additions & 1 deletion sdk/helper/ocsp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
89 changes: 89 additions & 0 deletions sdk/helper/ocsp/ocsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit d1a1dd4

Please sign in to comment.