Skip to content

Commit

Permalink
Add an OCSP responder to Vault's PKI plugin (#16723)
Browse files Browse the repository at this point in the history
* Refactor existing CRL function to storage getRevocationConfig

* Introduce ocsp_disable config option in config/crl

* Introduce OCSPSigning usage flag on issuer

* Add ocsp-request passthrough within lower layers of Vault

* Add OCSP responder to Vault PKI

* Add API documentation for OCSP

* Add cl

* Revert PKI storage migration modifications for OCSP

* Smaller PR feedback items

 - pki.mdx doc update
 - parens around logical.go comment to indicate DER encoded request is
   related to OCSP and not the snapshots
 - Use AllIssuers instead of writing them all out
 - Drop zero initialization of crl config's Disable flag if not present
 - Upgrade issuer on the fly instead of an initial migration

* Additional clean up backing out the writeRevocationConfig refactoring

* Remove Dirty issuer flag and update comment about not writing upgrade to
storage

* Address PR feedback and return Unknown response when mismatching issuer

* make fmt

* PR Feedback.

* More PR feedback

 - Leverage ocsp response constant
 - Remove duplicate errors regarding unknown issuers
  • Loading branch information
stevendpclark committed Aug 22, 2022
1 parent 867c3bc commit c8fb36a
Show file tree
Hide file tree
Showing 15 changed files with 1,240 additions and 52 deletions.
26 changes: 26 additions & 0 deletions builtin/logical/pki/backend.go
Expand Up @@ -8,6 +8,8 @@ import (
"sync/atomic"
"time"

atomic2 "go.uber.org/atomic"

"github.com/hashicorp/vault/sdk/helper/consts"

"github.com/armon/go-metrics"
Expand Down Expand Up @@ -84,6 +86,8 @@ func Backend(conf *logical.BackendConfig) *backend {
"issuer/+/der",
"issuer/+/json",
"issuers",
"ocsp", // OCSP POST
"ocsp/*", // OCSP GET
},

LocalStorage: []string{
Expand Down Expand Up @@ -158,6 +162,10 @@ func Backend(conf *logical.BackendConfig) *backend {
pathFetchValidRaw(&b),
pathFetchValid(&b),
pathFetchListCerts(&b),

// OCSP APIs
buildPathOcspGet(&b),
buildPathOcspPost(&b),
},

Secrets: []*framework.Secret{
Expand All @@ -179,6 +187,7 @@ func Backend(conf *logical.BackendConfig) *backend {
b.pkiStorageVersion.Store(0)

b.crlBuilder = &crlBuilder{}
b.isOcspDisabled = atomic2.NewBool(false)
return &b
}

Expand All @@ -199,6 +208,9 @@ type backend struct {

// Write lock around issuers and keys.
issuersLock sync.RWMutex

// Optimization to not read the CRL config on every OCSP request.
isOcspDisabled *atomic2.Bool
}

type (
Expand Down Expand Up @@ -295,6 +307,9 @@ func (b *backend) metricsWrap(callType string, roleMode int, ofunc roleOperation

// initialize is used to perform a possible PKI storage migration if needed
func (b *backend) initialize(ctx context.Context, _ *logical.InitializationRequest) error {
// load ocsp enabled status
setOcspStatus(b, ctx)

// Grab the lock prior to the updating of the storage lock preventing us flipping
// the storage flag midway through the request stream of other requests.
b.issuersLock.Lock()
Expand All @@ -320,6 +335,14 @@ func (b *backend) initialize(ctx context.Context, _ *logical.InitializationReque
return nil
}

func setOcspStatus(b *backend, ctx context.Context) {
sc := b.makeStorageContext(ctx, b.storage)
config, err := sc.getRevocationConfig()
if config != nil && err == nil {
b.isOcspDisabled.Store(config.OcspDisable)
}
}

func (b *backend) useLegacyBundleCaStorage() bool {
// This helper function is here to choose whether or not we use the newer
// issuer/key storage format or the older legacy ca bundle format.
Expand Down Expand Up @@ -373,6 +396,9 @@ func (b *backend) invalidate(ctx context.Context, key string) {
} else {
b.Logger().Debug("Ignoring invalidation updates for issuer as the PKI migration has yet to complete.")
}
case key == "config/crl":
// We may need to reload our OCSP status flag
setOcspStatus(b, ctx)
}
}

Expand Down
12 changes: 9 additions & 3 deletions builtin/logical/pki/cert_util.go
Expand Up @@ -15,6 +15,7 @@ import (
"errors"
"fmt"
"io"
"math/big"
"net"
"net/url"
"regexp"
Expand Down Expand Up @@ -150,6 +151,10 @@ func (sc *storageContext) fetchCAInfoByIssuerId(issuerId issuerID, usage issuerU
return caInfo, nil
}

func fetchCertBySerialBigInt(ctx context.Context, b *backend, req *logical.Request, prefix string, serial *big.Int) (*logical.StorageEntry, error) {
return fetchCertBySerial(ctx, b, req, prefix, serialFromBigInt(serial))
}

// Allows fetching certificates from the backend; it handles the slightly
// separate pathing for CRL, and revoked certificates.
//
Expand Down Expand Up @@ -915,9 +920,10 @@ func signCert(b *backend,

// otherNameRaw describes a name related to a certificate which is not in one
// of the standard name formats. RFC 5280, 4.2.1.6:
// OtherName ::= SEQUENCE {
// type-id OBJECT IDENTIFIER,
// value [0] EXPLICIT ANY DEFINED BY type-id }
//
// OtherName ::= SEQUENCE {
// type-id OBJECT IDENTIFIER,
// value [0] EXPLICIT ANY DEFINED BY type-id }
type otherNameRaw struct {
TypeID asn1.ObjectIdentifier
Value asn1.RawValue
Expand Down
59 changes: 59 additions & 0 deletions builtin/logical/pki/crl_test.go
Expand Up @@ -3,6 +3,7 @@ package pki
import (
"context"
"encoding/asn1"
"fmt"
"strings"
"testing"
"time"
Expand All @@ -26,6 +27,64 @@ func TestBackend_CRL_EnableDisableRoot(t *testing.T) {
crlEnableDisableTestForBackend(t, b, s, []string{caSerial})
}

func TestBackend_CRLConfig(t *testing.T) {
t.Parallel()

tests := []struct {
expiry string
disable bool
ocspDisable bool
}{
{expiry: "24h", disable: true, ocspDisable: true},
{expiry: "16h", disable: false, ocspDisable: true},
{expiry: "8h", disable: true, ocspDisable: false},
}
for _, tc := range tests {
name := fmt.Sprintf("%s-%t-%t", tc.expiry, tc.disable, tc.ocspDisable)
t.Run(name, func(t *testing.T) {
b, s := createBackendWithStorage(t)

resp, err := CBWrite(b, s, "config/crl", map[string]interface{}{
"expiry": tc.expiry,
"disable": tc.disable,
"ocsp_disable": tc.ocspDisable,
})
requireSuccessNilResponse(t, resp, err)

resp, err = CBRead(b, s, "config/crl")
requireSuccessNonNilResponse(t, resp, err)
requireFieldsSetInResp(t, resp, "disable", "expiry", "ocsp_disable")

require.Equal(t, tc.expiry, resp.Data["expiry"])
require.Equal(t, tc.disable, resp.Data["disable"])
require.Equal(t, tc.ocspDisable, resp.Data["ocsp_disable"])
})
}

badValueTests := []struct {
expiry string
disable string
ocspDisable string
}{
{expiry: "not a duration", disable: "true", ocspDisable: "true"},
{expiry: "16h", disable: "not a boolean", ocspDisable: "true"},
{expiry: "8h", disable: "true", ocspDisable: "not a boolean"},
}
for _, tc := range badValueTests {
name := fmt.Sprintf("bad-%s-%s-%s", tc.expiry, tc.disable, tc.ocspDisable)
t.Run(name, func(t *testing.T) {
b, s := createBackendWithStorage(t)

_, err := CBWrite(b, s, "config/crl", map[string]interface{}{
"expiry": tc.expiry,
"disable": tc.disable,
"ocsp_disable": tc.ocspDisable,
})
require.Error(t, err)
})
}
}

func TestBackend_CRL_AllKeyTypeSigAlgos(t *testing.T) {
type testCase struct {
KeyType string
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/pki/crl_util.go
Expand Up @@ -611,7 +611,7 @@ func augmentWithRevokedIssuers(issuerIDEntryMap map[issuerID]*issuerEntry, issue
// Builds a CRL by going through the list of revoked certificates and building
// a new CRL with the stored revocation times and serial numbers.
func buildCRL(sc *storageContext, forceNew bool, thisIssuerId issuerID, revoked []pkix.RevokedCertificate, identifier crlID, crlNumber int64) error {
crlInfo, err := sc.Backend.CRL(sc.Context, sc.Storage)
crlInfo, err := sc.getRevocationConfig()
if err != nil {
return errutil.InternalError{Err: fmt.Sprintf("error fetching CRL config information: %s", err)}
}
Expand Down

0 comments on commit c8fb36a

Please sign in to comment.