Skip to content

Commit a1cf84f

Browse files
authored
Clean up dangling OCSP configs (#8461)
Fixes #8345 Part of #8177
1 parent abafa0a commit a1cf84f

File tree

13 files changed

+38
-148
lines changed

13 files changed

+38
-148
lines changed

ca/ca_test.go

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -134,21 +134,19 @@ func newCAArgs(t *testing.T) *caArgs {
134134
test.AssertNotError(t, err, "Couldn't set identifier policy")
135135

136136
legacy, err := issuance.NewProfile(issuance.ProfileConfig{
137-
IncludeCRLDistributionPoints: true,
138-
MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90},
139-
MaxValidityBackdate: config.Duration{Duration: time.Hour},
140-
IgnoredLints: []string{"w_subject_common_name_included"},
137+
MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90},
138+
MaxValidityBackdate: config.Duration{Duration: time.Hour},
139+
IgnoredLints: []string{"w_subject_common_name_included"},
141140
})
142141
test.AssertNotError(t, err, "Loading test profile")
143142
modern, err := issuance.NewProfile(issuance.ProfileConfig{
144-
OmitCommonName: true,
145-
OmitKeyEncipherment: true,
146-
OmitClientAuth: true,
147-
OmitSKID: true,
148-
IncludeCRLDistributionPoints: true,
149-
MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 6},
150-
MaxValidityBackdate: config.Duration{Duration: time.Hour},
151-
IgnoredLints: []string{"w_ext_subject_key_identifier_missing_sub_cert"},
143+
OmitCommonName: true,
144+
OmitKeyEncipherment: true,
145+
OmitClientAuth: true,
146+
OmitSKID: true,
147+
MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 6},
148+
MaxValidityBackdate: config.Duration{Duration: time.Hour},
149+
IgnoredLints: []string{"w_ext_subject_key_identifier_missing_sub_cert"},
152150
})
153151
test.AssertNotError(t, err, "Loading test profile")
154152
profiles := map[string]*issuance.Profile{
@@ -161,7 +159,6 @@ func newCAArgs(t *testing.T) *caArgs {
161159
issuers[i], err = issuance.LoadIssuer(issuance.IssuerConfig{
162160
Active: true,
163161
IssuerURL: fmt.Sprintf("http://not-example.com/i/%s", name),
164-
OCSPURL: "http://not-example.com/o",
165162
CRLURLBase: fmt.Sprintf("http://not-example.com/c/%s/", name),
166163
CRLShards: 10,
167164
Location: issuance.IssuerLoc{
@@ -886,7 +883,6 @@ func TestPickIssuer_Inactive(t *testing.T) {
886883
issuer, err := issuance.LoadIssuer(issuance.IssuerConfig{
887884
Active: i%2 == 0,
888885
IssuerURL: fmt.Sprintf("http://not-example.com/i/%s", name),
889-
OCSPURL: "http://not-example.com/o",
890886
CRLURLBase: fmt.Sprintf("http://not-example.com/c/%s/", name),
891887
CRLShards: 10,
892888
Location: issuance.IssuerLoc{

cmd/boulder-ca/main.go

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/letsencrypt/boulder/ca"
1313
capb "github.com/letsencrypt/boulder/ca/proto"
1414
"github.com/letsencrypt/boulder/cmd"
15-
"github.com/letsencrypt/boulder/config"
1615
"github.com/letsencrypt/boulder/ctpolicy/loglist"
1716
"github.com/letsencrypt/boulder/features"
1817
"github.com/letsencrypt/boulder/goodkey"
@@ -75,12 +74,6 @@ type Config struct {
7574
// configurations.
7675
MaxNames int `validate:"required,min=1,max=100"`
7776

78-
// LifespanOCSP is how long OCSP responses are valid for. Per the BRs,
79-
// Section 4.9.10, it MUST NOT be more than 10 days. Default 96h.
80-
//
81-
// Deprecated: TODO(#8345): Remove this.
82-
LifespanOCSP config.Duration
83-
8477
// GoodKey is an embedded config stanza for the goodkey library.
8578
GoodKey goodkey.Config
8679

@@ -89,17 +82,6 @@ type Config struct {
8982
// OCSP and CRL audit log emission. Recommended to be around 4000.
9083
OCSPLogMaxLength int
9184

92-
// Maximum period (in Go duration format) to wait to accumulate a max-length
93-
// OCSP audit log line. We will emit a log line at least once per period,
94-
// if there is anything to be logged. Keeping this low minimizes the risk
95-
// of losing logs during a catastrophic failure. Making it too high
96-
// means logging more often than necessary, which is inefficient in terms
97-
// of bytes and log system resources.
98-
// Recommended to be around 500ms.
99-
//
100-
// Deprecated: TODO(#8345): Remove this.
101-
OCSPLogPeriod config.Duration
102-
10385
// CTLogListFile is the path to a JSON file on disk containing the set of
10486
// all logs trusted by Chrome. The file must match the v3 log list schema:
10587
// https://www.gstatic.com/ct/log_list/v3/log_list_schema.json
@@ -108,11 +90,7 @@ type Config struct {
10890
// DisableCertService causes the CertificateAuthority gRPC service to not
10991
// start, preventing any certificates or precertificates from being issued.
11092
DisableCertService bool
111-
// DisableOCSPService causes the OCSPGenerator gRPC service to not start,
112-
// preventing any OCSP responses from being issued.
113-
//
114-
// Deprecated: TODO(#8345): Remove this.
115-
DisableOCSPService bool
93+
11694
// DisableCRLService causes the CRLGenerator gRPC service to not start,
11795
// preventing any CRLs from being issued.
11896
DisableCRLService bool

cmd/boulder-ra/main.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@ type Config struct {
4545
CAService *cmd.GRPCClientConfig
4646
PublisherService *cmd.GRPCClientConfig
4747

48-
// Deprecated: TODO(#8345): Remove this.
49-
AkamaiPurgerService *cmd.GRPCClientConfig
50-
5148
// Deprecated: TODO(#8349): Remove this when removing the corresponding
5249
// service from the CA.
5350
OCSPService *cmd.GRPCClientConfig
@@ -105,15 +102,6 @@ type Config struct {
105102
// default.
106103
DefaultProfileName string `validate:"required"`
107104

108-
// MustStapleAllowList specified the path to a YAML file containing a
109-
// list of account IDs permitted to request certificates with the OCSP
110-
// Must-Staple extension.
111-
//
112-
// Deprecated: This field no longer has any effect, all Must-Staple requests
113-
// are rejected.
114-
// TODO(#8345): Remove this field.
115-
MustStapleAllowList string `validate:"omitempty"`
116-
117105
// GoodKey is an embedded config stanza for the goodkey library.
118106
GoodKey goodkey.Config
119107

cmd/crl-updater/main.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,6 @@ type Config struct {
7272
// of magnitude greater than our p99 update latency.
7373
UpdateTimeout config.Duration `validate:"-"`
7474

75-
// TemporallyShardedSerialPrefixes is a list of prefixes that were used to
76-
// issue certificates with no CRLDistributionPoints extension, and which are
77-
// therefore temporally sharded. If it's non-empty, the CRL Updater will
78-
// require matching serials when querying by temporal shard. When querying
79-
// by explicit shard, any prefix is allowed.
80-
//
81-
// This should be set to the current set of serial prefixes in production.
82-
// When deploying explicit sharding (i.e. the CRLDistributionPoints extension),
83-
// the CAs should be configured with a new set of serial prefixes that haven't
84-
// been used before.
85-
//
86-
// Deprecated: The updater no longer supports temporal sharding.
87-
// TODO(#8345): Remove this.
88-
TemporallyShardedSerialPrefixes []string
89-
9075
// MaxParallelism controls how many workers may be running in parallel.
9176
// A higher value reduces the total time necessary to update all CRL shards
9277
// that this updater is responsible for, but also increases the memory used

issuance/cert.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,6 @@ import (
3131

3232
// ProfileConfig describes the certificate issuance constraints for all issuers.
3333
type ProfileConfig struct {
34-
// AllowMustStaple, when false, causes all IssuanceRequests which specify the
35-
// OCSP Must Staple extension to be rejected.
36-
//
37-
// Deprecated: This has no effect, Must Staple is always omitted.
38-
// TODO(#8177): Remove this.
39-
AllowMustStaple bool
40-
4134
// OmitCommonName causes the CN field to be excluded from the resulting
4235
// certificate, regardless of its inclusion in the IssuanceRequest.
4336
OmitCommonName bool
@@ -49,20 +42,6 @@ type ProfileConfig struct {
4942
OmitClientAuth bool
5043
// OmitSKID causes the Subject Key Identifier extension to be omitted.
5144
OmitSKID bool
52-
// OmitOCSP causes the OCSP URI field to be omitted from the Authority
53-
// Information Access extension. This cannot be true unless
54-
// IncludeCRLDistributionPoints is also true, to ensure that every
55-
// certificate has at least one revocation mechanism included.
56-
//
57-
// Deprecated: This has no effect; OCSP is always omitted.
58-
// TODO(#8177): Remove this.
59-
OmitOCSP bool
60-
// IncludeCRLDistributionPoints causes the CRLDistributionPoints extension to
61-
// be added to all certificates issued by this profile.
62-
//
63-
// Deprecated: This has no effect; CRLDP is always included.
64-
// TODO(#8177): Remove this.
65-
IncludeCRLDistributionPoints bool
6645

6746
MaxValidityPeriod config.Duration
6847
MaxValidityBackdate config.Duration

issuance/cert_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -920,9 +920,8 @@ func TestNewProfile(t *testing.T) {
920920
{
921921
name: "happy path",
922922
config: ProfileConfig{
923-
MaxValidityBackdate: config.Duration{Duration: 1 * time.Hour},
924-
MaxValidityPeriod: config.Duration{Duration: 90 * 24 * time.Hour},
925-
IncludeCRLDistributionPoints: true,
923+
MaxValidityBackdate: config.Duration{Duration: 1 * time.Hour},
924+
MaxValidityPeriod: config.Duration{Duration: 90 * 24 * time.Hour},
926925
},
927926
},
928927
{

issuance/issuer.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,6 @@ type IssuerConfig struct {
158158
IssuerURL string `validate:"required,url"`
159159
CRLURLBase string `validate:"required,url,startswith=http://,endswith=/"`
160160

161-
// TODO(#8177): Remove this.
162-
OCSPURL string `validate:"omitempty,url"`
163-
164161
// Number of CRL shards. Must be positive, but can be 1 for no sharding.
165162
CRLShards int `validate:"required,min=1"`
166163

issuance/issuer_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@ import (
2424

2525
func defaultProfileConfig() ProfileConfig {
2626
return ProfileConfig{
27-
AllowMustStaple: true,
28-
IncludeCRLDistributionPoints: true,
29-
MaxValidityPeriod: config.Duration{Duration: time.Hour},
30-
MaxValidityBackdate: config.Duration{Duration: time.Hour},
27+
MaxValidityPeriod: config.Duration{Duration: time.Hour},
28+
MaxValidityBackdate: config.Duration{Duration: time.Hour},
3129
IgnoredLints: []string{
3230
// Ignore the two SCT lints because these tests don't get SCTs.
3331
"w_ct_sct_policy_count_unsatisfied",

ra/ra.go

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,6 @@ type RegistrationAuthorityImpl struct {
104104
inflightFinalizes prometheus.Gauge
105105
certCSRMismatch prometheus.Counter
106106
pauseCounter *prometheus.CounterVec
107-
// TODO(#8177): Remove once the rate of requests failing to finalize due to
108-
// requesting Must-Staple has diminished.
109-
mustStapleRequestsCounter *prometheus.CounterVec
110107
}
111108

112109
var _ rapb.RegistrationAuthorityServer = (*RegistrationAuthorityImpl)(nil)
@@ -225,42 +222,35 @@ func NewRegistrationAuthorityImpl(
225222
}, []string{"paused", "repaused", "grace"})
226223
stats.MustRegister(pauseCounter)
227224

228-
mustStapleRequestsCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
229-
Name: "must_staple_requests",
230-
Help: "Number of times a must-staple request is made, labeled by allowlist=[allowed|denied]",
231-
}, []string{"allowlist"})
232-
stats.MustRegister(mustStapleRequestsCounter)
233-
234225
issuersByNameID := make(map[issuance.NameID]*issuance.Certificate)
235226
for _, issuer := range issuers {
236227
issuersByNameID[issuer.NameID()] = issuer
237228
}
238229

239230
ra := &RegistrationAuthorityImpl{
240-
clk: clk,
241-
log: logger,
242-
profiles: profiles,
243-
maxContactsPerReg: maxContactsPerReg,
244-
keyPolicy: keyPolicy,
245-
limiter: limiter,
246-
txnBuilder: txnBuilder,
247-
started: clk.Now(),
248-
publisher: pubc,
249-
finalizeTimeout: finalizeTimeout,
250-
ctpolicy: ctp,
251-
ctpolicyResults: ctpolicyResults,
252-
issuersByNameID: issuersByNameID,
253-
namesPerCert: namesPerCert,
254-
newRegCounter: newRegCounter,
255-
recheckCAACounter: recheckCAACounter,
256-
newCertCounter: newCertCounter,
257-
revocationReasonCounter: revocationReasonCounter,
258-
authzAges: authzAges,
259-
orderAges: orderAges,
260-
inflightFinalizes: inflightFinalizes,
261-
certCSRMismatch: certCSRMismatch,
262-
pauseCounter: pauseCounter,
263-
mustStapleRequestsCounter: mustStapleRequestsCounter,
231+
clk: clk,
232+
log: logger,
233+
profiles: profiles,
234+
maxContactsPerReg: maxContactsPerReg,
235+
keyPolicy: keyPolicy,
236+
limiter: limiter,
237+
txnBuilder: txnBuilder,
238+
started: clk.Now(),
239+
publisher: pubc,
240+
finalizeTimeout: finalizeTimeout,
241+
ctpolicy: ctp,
242+
ctpolicyResults: ctpolicyResults,
243+
issuersByNameID: issuersByNameID,
244+
namesPerCert: namesPerCert,
245+
newRegCounter: newRegCounter,
246+
recheckCAACounter: recheckCAACounter,
247+
newCertCounter: newCertCounter,
248+
revocationReasonCounter: revocationReasonCounter,
249+
authzAges: authzAges,
250+
orderAges: orderAges,
251+
inflightFinalizes: inflightFinalizes,
252+
certCSRMismatch: certCSRMismatch,
253+
pauseCounter: pauseCounter,
264254
}
265255
return ra
266256
}
@@ -1091,7 +1081,6 @@ func (ra *RegistrationAuthorityImpl) validateFinalizeRequest(
10911081
}
10921082

10931083
if containsMustStaple(csr.Extensions) {
1094-
ra.mustStapleRequestsCounter.WithLabelValues("denied").Inc()
10951084
return nil, berrors.UnauthorizedError(
10961085
"OCSP must-staple extension is no longer available: see https://letsencrypt.org/2024/12/05/ending-ocsp",
10971086
)
@@ -1504,7 +1493,6 @@ func (ra *RegistrationAuthorityImpl) checkDCVAndCAA(ctx context.Context, dcvReq
15041493
func (ra *RegistrationAuthorityImpl) PerformValidation(
15051494
ctx context.Context,
15061495
req *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
1507-
15081496
// Clock for start of PerformValidation.
15091497
vStart := ra.clk.Now()
15101498

ra/ra_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2940,7 +2940,6 @@ func TestFinalizeWithMustStaple(t *testing.T) {
29402940
})
29412941
test.AssertError(t, err, "finalization should fail")
29422942
test.AssertContains(t, err.Error(), "no longer available")
2943-
test.AssertMetricWithLabelsEquals(t, ra.mustStapleRequestsCounter, prometheus.Labels{"allowlist": "denied"}, 1)
29442943
}
29452944

29462945
func TestIssueCertificateAuditLog(t *testing.T) {

0 commit comments

Comments
 (0)