Skip to content

Commit

Permalink
ra: Gate OCSP Must-Staple issuance on account-based allow list (#7976)
Browse files Browse the repository at this point in the history
Add support in the RA for an allow list of accounts permitted to request
certificates containing the OCSP Must-Staple extension. If no allow list
is configured, all accounts are permitted. When a list is provided,
Finalize requests with Must-Staple are rejected unless the account is on
the list, and metrics are updated to track allowed and denied requests.

Fixes #7914
  • Loading branch information
beautifulentropy authored Jan 27, 2025
1 parent 888581b commit 811e607
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 11 deletions.
16 changes: 16 additions & 0 deletions cmd/boulder-ra/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ type Config struct {
AllowList string `validate:"omitempty"`
}

// MustStapleAllowList specifies the path to a YAML file containing a
// list of account IDs permitted to request certificates with the OCSP
// Must-Staple extension. If no path is specified, the extension is
// permitted for all accounts. If the file exists but is empty, the
// extension is disabled for all accounts.
MustStapleAllowList string `validate:"omitempty"`

// GoodKey is an embedded config stanza for the goodkey library.
GoodKey goodkey.Config

Expand Down Expand Up @@ -281,6 +288,14 @@ func main() {
}
}

var mustStapleAllowList *allowlist.List[int64]
if c.RA.MustStapleAllowList != "" {
data, err := os.ReadFile(c.RA.MustStapleAllowList)
cmd.FailOnError(err, "Failed to read allow list for Must-Staple extension")
mustStapleAllowList, err = allowlist.NewFromYAML[int64](data)
cmd.FailOnError(err, "Failed to parse allow list for Must-Staple extension")
}

if features.Get().AsyncFinalize && c.RA.FinalizeTimeout.Duration == 0 {
cmd.Fail("finalizeTimeout must be supplied when AsyncFinalize feature is enabled")
}
Expand Down Expand Up @@ -319,6 +334,7 @@ func main() {
authorizationLifetime,
pendingAuthorizationLifetime,
validationProfiles,
mustStapleAllowList,
pubc,
c.RA.OrderLifetime.Duration,
c.RA.FinalizeTimeout.Duration,
Expand Down
45 changes: 34 additions & 11 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type RegistrationAuthorityImpl struct {
authorizationLifetime time.Duration
pendingAuthorizationLifetime time.Duration
validationProfiles map[string]*ValidationProfile
mustStapleAllowList *allowlist.List[int64]
maxContactsPerReg int
limiter *ratelimits.Limiter
txnBuilder *ratelimits.TransactionBuilder
Expand All @@ -112,17 +113,18 @@ type RegistrationAuthorityImpl struct {

ctpolicy *ctpolicy.CTPolicy

ctpolicyResults *prometheus.HistogramVec
revocationReasonCounter *prometheus.CounterVec
namesPerCert *prometheus.HistogramVec
newRegCounter prometheus.Counter
recheckCAACounter prometheus.Counter
newCertCounter *prometheus.CounterVec
authzAges *prometheus.HistogramVec
orderAges *prometheus.HistogramVec
inflightFinalizes prometheus.Gauge
certCSRMismatch prometheus.Counter
pauseCounter *prometheus.CounterVec
ctpolicyResults *prometheus.HistogramVec
revocationReasonCounter *prometheus.CounterVec
namesPerCert *prometheus.HistogramVec
newRegCounter prometheus.Counter
recheckCAACounter prometheus.Counter
newCertCounter *prometheus.CounterVec
authzAges *prometheus.HistogramVec
orderAges *prometheus.HistogramVec
inflightFinalizes prometheus.Gauge
certCSRMismatch prometheus.Counter
pauseCounter *prometheus.CounterVec
mustStapleRequestsCounter *prometheus.CounterVec
}

var _ rapb.RegistrationAuthorityServer = (*RegistrationAuthorityImpl)(nil)
Expand All @@ -140,6 +142,7 @@ func NewRegistrationAuthorityImpl(
authorizationLifetime time.Duration,
pendingAuthorizationLifetime time.Duration,
validationProfiles map[string]*ValidationProfile,
mustStapleAllowList *allowlist.List[int64],
pubc pubpb.PublisherClient,
orderLifetime time.Duration,
finalizeTimeout time.Duration,
Expand Down Expand Up @@ -236,6 +239,12 @@ func NewRegistrationAuthorityImpl(
}, []string{"paused", "repaused", "grace"})
stats.MustRegister(pauseCounter)

mustStapleRequestsCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
Name: "must_staple_requests",
Help: "Number of times a must-staple request is made, labeled by allowlist=[allowed|denied]",
}, []string{"allowlist"})
stats.MustRegister(mustStapleRequestsCounter)

issuersByNameID := make(map[issuance.NameID]*issuance.Certificate)
for _, issuer := range issuers {
issuersByNameID[issuer.NameID()] = issuer
Expand All @@ -247,6 +256,7 @@ func NewRegistrationAuthorityImpl(
authorizationLifetime: authorizationLifetime,
pendingAuthorizationLifetime: pendingAuthorizationLifetime,
validationProfiles: validationProfiles,
mustStapleAllowList: mustStapleAllowList,
maxContactsPerReg: maxContactsPerReg,
keyPolicy: keyPolicy,
limiter: limiter,
Expand All @@ -269,6 +279,7 @@ func NewRegistrationAuthorityImpl(
inflightFinalizes: inflightFinalizes,
certCSRMismatch: certCSRMismatch,
pauseCounter: pauseCounter,
mustStapleRequestsCounter: mustStapleRequestsCounter,
}
return ra
}
Expand Down Expand Up @@ -945,6 +956,18 @@ func (ra *RegistrationAuthorityImpl) validateFinalizeRequest(
return nil, berrors.BadCSRError("unable to parse CSR: %s", err.Error())
}

if ra.mustStapleAllowList != nil && issuance.ContainsMustStaple(csr.Extensions) {
if !ra.mustStapleAllowList.Contains(req.Order.RegistrationID) {
ra.mustStapleRequestsCounter.WithLabelValues("denied").Inc()
return nil, berrors.UnauthorizedError(
"OCSP must-staple extension is no longer available: see https://letsencrypt.org/2024/12/05/ending-ocsp",
)
} else {
ra.mustStapleRequestsCounter.WithLabelValues("allowed").Inc()
}

}

err = csrlib.VerifyCSR(ctx, csr, ra.maxNames, &ra.keyPolicy, ra.PA)
if err != nil {
// VerifyCSR returns berror instances that can be passed through as-is
Expand Down
120 changes: 120 additions & 0 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import (
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"math"
"math/big"
mrand "math/rand/v2"
"regexp"
Expand Down Expand Up @@ -344,6 +346,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, sapb.StorageAutho
300*24*time.Hour, 7*24*time.Hour,
nil,
nil,
nil,
7*24*time.Hour, 5*time.Minute,
ctp, nil, nil)
ra.SA = sa
Expand Down Expand Up @@ -2607,6 +2610,123 @@ func TestFinalizeOrderDisabledChallenge(t *testing.T) {
test.AssertContains(t, err.Error(), "authorizations for these identifiers not valid")
}

func TestFinalizeWithMustStaple(t *testing.T) {
_, sa, ra, _, fc, cleanUp := initAuthorities(t)
defer cleanUp()

ocspMustStapleExt := pkix.Extension{
// RFC 7633: id-pe-tlsfeature OBJECT IDENTIFIER ::= { id-pe 24 }
Id: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24},
// ASN.1 encoding of:
// SEQUENCE
// INTEGER 5
// where "5" is the status_request feature (RFC 6066)
Value: []byte{0x30, 0x03, 0x02, 0x01, 0x05},
}

testCases := []struct {
name string
mustStapleAllowList *allowlist.List[int64]
expectSuccess bool
expectErrorContains string
expectMetricWithLabel prometheus.Labels
}{
{
name: "Allow only Registration.ID",
mustStapleAllowList: allowlist.NewList([]int64{Registration.Id}),
expectSuccess: true,
expectMetricWithLabel: prometheus.Labels{"allowlist": "allowed"},
},
{
name: "Deny all but account Id 1337",
mustStapleAllowList: allowlist.NewList([]int64{1337}),
expectSuccess: false,
expectErrorContains: "no longer available",
expectMetricWithLabel: prometheus.Labels{"allowlist": "denied"},
},
{
name: "Deny all account Ids",
mustStapleAllowList: allowlist.NewList([]int64{}),
expectSuccess: false,
expectErrorContains: "no longer available",
expectMetricWithLabel: prometheus.Labels{"allowlist": "denied"},
},
{
name: "Allow all account Ids",
mustStapleAllowList: nil,
expectSuccess: true,
// We don't expect this metric to be be emitted if the allowlist is nil.
expectMetricWithLabel: nil,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ra.mustStapleAllowList = tc.mustStapleAllowList

domain := randomDomain()

authzID := createFinalizedAuthorization(
t, sa, domain, fc.Now().Add(24*time.Hour), core.ChallengeTypeHTTP01, fc.Now().Add(-1*time.Hour))

order, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
RegistrationID: Registration.Id,
DnsNames: []string{domain},
})
test.AssertNotError(t, err, "creating test order")
test.AssertEquals(t, order.V2Authorizations[0], authzID)

testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
test.AssertNotError(t, err, "generating test key")

csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{
PublicKey: testKey.Public(),
DNSNames: []string{domain},
ExtraExtensions: []pkix.Extension{ocspMustStapleExt},
}, testKey)
test.AssertNotError(t, err, "creating must-staple CSR")

serial, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64))
test.AssertNotError(t, err, "generating random serial number")
template := &x509.Certificate{
SerialNumber: serial,
Subject: pkix.Name{CommonName: domain},
DNSNames: []string{domain},
NotBefore: fc.Now(),
NotAfter: fc.Now().Add(365 * 24 * time.Hour),
BasicConstraintsValid: true,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
ExtraExtensions: []pkix.Extension{ocspMustStapleExt},
}
cert, err := x509.CreateCertificate(rand.Reader, template, template, testKey.Public(), testKey)
test.AssertNotError(t, err, "creating certificate")
ra.CA = &mocks.MockCA{
PEM: pem.EncodeToMemory(&pem.Block{
Bytes: cert,
Type: "CERTIFICATE",
}),
}

_, err = ra.FinalizeOrder(context.Background(), &rapb.FinalizeOrderRequest{
Order: order,
Csr: csr,
})

if tc.expectSuccess {
test.AssertNotError(t, err, "finalization should succeed")
} else {
test.AssertError(t, err, "finalization should fail")
test.AssertContains(t, err.Error(), tc.expectErrorContains)
}

if tc.expectMetricWithLabel != nil {
test.AssertMetricWithLabelsEquals(t, ra.mustStapleRequestsCounter, tc.expectMetricWithLabel, 1)
}
ra.mustStapleRequestsCounter.Reset()
})
}
}

func TestIssueCertificateAuditLog(t *testing.T) {
_, sa, ra, _, _, cleanUp := initAuthorities(t)
defer cleanUp()
Expand Down

0 comments on commit 811e607

Please sign in to comment.