Deprecate renewal rate limiting feature flags (#4161)
This commit is contained in:
Jacob Hoffman-Andrews
Roland Bracewell Shoemaker
parent
7ff30cf857
commit
4e20c83d96
|
|
@ -14,22 +14,22 @@ func _() {
|
|||
_ = x[SimplifiedVAHTTP-3]
|
||||
_ = x[TLSSNIRevalidation-4]
|
||||
_ = x[AllowRenewalFirstRL-5]
|
||||
_ = x[CAAValidationMethods-6]
|
||||
_ = x[CAAAccountURI-7]
|
||||
_ = x[ProbeCTLogs-8]
|
||||
_ = x[HeadNonceStatusOK-9]
|
||||
_ = x[NewAuthorizationSchema-10]
|
||||
_ = x[RevokeAtRA-11]
|
||||
_ = x[SetIssuedNamesRenewalBit-12]
|
||||
_ = x[SetIssuedNamesRenewalBit-6]
|
||||
_ = x[CAAValidationMethods-7]
|
||||
_ = x[CAAAccountURI-8]
|
||||
_ = x[ProbeCTLogs-9]
|
||||
_ = x[HeadNonceStatusOK-10]
|
||||
_ = x[NewAuthorizationSchema-11]
|
||||
_ = x[RevokeAtRA-12]
|
||||
_ = x[EarlyOrderRateLimit-13]
|
||||
_ = x[EnforceMultiVA-14]
|
||||
_ = x[MultiVAFullResults-15]
|
||||
_ = x[RemoveWFE2AccountID-16]
|
||||
}
|
||||
|
||||
const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLCAAValidationMethodsCAAAccountURIProbeCTLogsHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRASetIssuedNamesRenewalBitEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountID"
|
||||
const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitCAAValidationMethodsCAAAccountURIProbeCTLogsHeadNonceStatusOKNewAuthorizationSchemaRevokeAtRAEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountID"
|
||||
|
||||
var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 116, 129, 140, 157, 179, 189, 213, 232, 246, 264, 283}
|
||||
var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 140, 153, 164, 181, 203, 213, 232, 246, 264, 283}
|
||||
|
||||
func (i FeatureFlag) String() string {
|
||||
if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {
|
||||
|
|
|
|||
|
|
@ -16,9 +16,10 @@ const (
|
|||
ACME13KeyRollover
|
||||
SimplifiedVAHTTP
|
||||
TLSSNIRevalidation
|
||||
AllowRenewalFirstRL
|
||||
SetIssuedNamesRenewalBit
|
||||
|
||||
// Currently in-use features
|
||||
AllowRenewalFirstRL
|
||||
// Check CAA and respect validationmethods parameter.
|
||||
CAAValidationMethods
|
||||
// Check CAA and respect accounturi parameter.
|
||||
|
|
@ -32,9 +33,6 @@ const (
|
|||
NewAuthorizationSchema
|
||||
// RevokeAtRA enables revocation in the RA instead of ocsp-updater
|
||||
RevokeAtRA
|
||||
// SetIssuedNamesRenewalBit enables the SA setting the renewal bit for
|
||||
// issuedNames entries during AddCertificate.
|
||||
SetIssuedNamesRenewalBit
|
||||
// EarlyOrderRateLimit enables the RA applying certificate per name/per FQDN
|
||||
// set rate limits in NewOrder in addition to FinalizeOrder.
|
||||
EarlyOrderRateLimit
|
||||
|
|
|
|||
68
sa/sa.go
68
sa/sa.go
|
|
@ -20,7 +20,6 @@ import (
|
|||
"github.com/letsencrypt/boulder/core"
|
||||
corepb "github.com/letsencrypt/boulder/core/proto"
|
||||
berrors "github.com/letsencrypt/boulder/errors"
|
||||
"github.com/letsencrypt/boulder/features"
|
||||
bgrpc "github.com/letsencrypt/boulder/grpc"
|
||||
blog "github.com/letsencrypt/boulder/log"
|
||||
"github.com/letsencrypt/boulder/metrics"
|
||||
|
|
@ -451,72 +450,45 @@ func ReverseName(domain string) string {
|
|||
return strings.Join(labels, ".")
|
||||
}
|
||||
|
||||
// TODO(@cpu): This query can be removed when AllowRenewalFirstRL is the default.
|
||||
const countCertificatesSelect = `
|
||||
SELECT serial from issuedNames
|
||||
WHERE (reversedName = :reversedDomain OR
|
||||
reversedName LIKE CONCAT(:reversedDomain, ".%"))
|
||||
AND notBefore > :earliest AND notBefore <= :latest;`
|
||||
|
||||
const countCertificatesSelectNoRenewals = `
|
||||
SELECT serial from issuedNames
|
||||
WHERE (reversedName = :reversedDomain OR
|
||||
reversedName LIKE CONCAT(:reversedDomain, ".%"))
|
||||
AND NOT renewal AND notBefore > :earliest AND notBefore <= :latest;`
|
||||
|
||||
// TODO(@cpu): This query can be removed when AllowRenewalFirstRL is the default.
|
||||
const countCertificatesExactSelect = `
|
||||
SELECT serial from issuedNames
|
||||
WHERE reversedName = :reversedDomain
|
||||
AND notBefore > :earliest AND notBefore <= :latest;`
|
||||
|
||||
const countCertificatesExactSelectNoRenewals = `
|
||||
SELECT serial from issuedNames
|
||||
WHERE reversedName = :reversedDomain
|
||||
AND NOT renewal AND notBefore > :earliest AND notBefore <= :latest;`
|
||||
|
||||
// countCertificatesByNames returns, for a single domain, the count of
|
||||
// certificates issued in the given time range for that domain and its
|
||||
// subdomains. If the AllowRenewalFirstRL feature flag is enabled then the count
|
||||
// of certificates returned will not include any certificates that were
|
||||
// a renewal of a previous certificate.
|
||||
// subdomains.
|
||||
func (ssa *SQLStorageAuthority) countCertificatesByNameImpl(
|
||||
db dbSelector,
|
||||
domain string,
|
||||
earliest,
|
||||
latest time.Time,
|
||||
) (int, error) {
|
||||
if features.Enabled(features.AllowRenewalFirstRL) {
|
||||
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesSelectNoRenewals)
|
||||
} else {
|
||||
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesSelect)
|
||||
}
|
||||
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesSelect)
|
||||
}
|
||||
|
||||
// countCertificatesByExactNames returns, for a single domain, the count of
|
||||
// certificates issued in the given time range for that domain. In contrast to
|
||||
// countCertificatesByNames subdomains are NOT considered. If the
|
||||
// AllowRenewalFirstRL feature flag is enabled then the count of certificates
|
||||
// returned will not include any certificates that were a renewal of a previous
|
||||
// certificate.
|
||||
// countCertificatesByNames subdomains are NOT considered.
|
||||
func (ssa *SQLStorageAuthority) countCertificatesByExactName(
|
||||
db dbSelector,
|
||||
domain string,
|
||||
earliest,
|
||||
latest time.Time,
|
||||
) (int, error) {
|
||||
if features.Enabled(features.AllowRenewalFirstRL) {
|
||||
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesExactSelectNoRenewals)
|
||||
} else {
|
||||
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesExactSelect)
|
||||
}
|
||||
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesExactSelect)
|
||||
}
|
||||
|
||||
// countCertificates returns, for a single domain, the count of certificate
|
||||
// issuances in the given time range for that domain using the
|
||||
// provided query assumed to be either `countCertificatesExactSelect`,
|
||||
// `countCertificatesExactSelectNoRenewals`, `countCertificatesSelect` or
|
||||
// `countCertificatesSelectNoRenewals`
|
||||
// or `countCertificatesSelect`.
|
||||
func (ssa *SQLStorageAuthority) countCertificates(db dbSelector, domain string, earliest, latest time.Time, query string) (int, error) {
|
||||
var serials []string
|
||||
_, err := db.Select(
|
||||
|
|
@ -932,25 +904,17 @@ func (ssa *SQLStorageAuthority) AddCertificate(
|
|||
return "", Rollback(tx, err)
|
||||
}
|
||||
|
||||
// If the SetIssuedNamesRenewalBit feature flag is enabled then we need to
|
||||
// determine if the certificate being added is a renewal of a previously
|
||||
// issued certificate in order to set the renewal bit of the issued names rows
|
||||
// correctly with `addIssuedNames`.
|
||||
var isRenewal bool
|
||||
if features.Enabled(features.SetIssuedNamesRenewalBit) {
|
||||
// NOTE(@cpu): When we collect up names to check if an FQDN set exists (e.g.
|
||||
// that it is a renewal) we use just the DNSNames from the certificate and
|
||||
// ignore the Subject Common Name (if any). This is a safe assumption because
|
||||
// if a certificate we issued were to have a Subj. CN not present as a SAN it
|
||||
// would be a misissuance and miscalculating whether the cert is a renewal or
|
||||
// not for the purpose of rate limiting is the least of our troubles.
|
||||
prevCertExists, err := ssa.checkFQDNSetExists(
|
||||
txWithCtx.SelectOne,
|
||||
parsedCertificate.DNSNames)
|
||||
if err != nil {
|
||||
return "", Rollback(tx, err)
|
||||
}
|
||||
isRenewal = prevCertExists
|
||||
// NOTE(@cpu): When we collect up names to check if an FQDN set exists (e.g.
|
||||
// that it is a renewal) we use just the DNSNames from the certificate and
|
||||
// ignore the Subject Common Name (if any). This is a safe assumption because
|
||||
// if a certificate we issued were to have a Subj. CN not present as a SAN it
|
||||
// would be a misissuance and miscalculating whether the cert is a renewal or
|
||||
// not for the purpose of rate limiting is the least of our troubles.
|
||||
isRenewal, err := ssa.checkFQDNSetExists(
|
||||
txWithCtx.SelectOne,
|
||||
parsedCertificate.DNSNames)
|
||||
if err != nil {
|
||||
return "", Rollback(tx, err)
|
||||
}
|
||||
|
||||
err = addIssuedNames(txWithCtx, parsedCertificate, isRenewal)
|
||||
|
|
|
|||
|
|
@ -2298,9 +2298,6 @@ func TestAddCertificateRenewalBit(t *testing.T) {
|
|||
sa, fc, cleanUp := initSA(t)
|
||||
defer cleanUp()
|
||||
|
||||
err := features.Set(map[string]bool{"SetIssuedNamesRenewalBit": true})
|
||||
test.AssertNotError(t, err, "Failed to enable SetIssuedNamesRenewalBit feature flag")
|
||||
|
||||
reg := satest.CreateWorkingRegistration(t, sa)
|
||||
|
||||
// An example cert taken from EFF's website
|
||||
|
|
@ -2364,15 +2361,6 @@ func TestCountCertificatesRenewalBit(t *testing.T) {
|
|||
sa, fc, cleanUp := initSA(t)
|
||||
defer cleanUp()
|
||||
|
||||
// Set feature flags required for this test. We need to set both the
|
||||
// renewal bit with the SetIssuedRenewalBit flag and use it with the
|
||||
// AllowRenewalFirstRL flag.
|
||||
err := features.Set(map[string]bool{
|
||||
"SetIssuedNamesRenewalBit": true,
|
||||
"AllowRenewalFirstRL": true,
|
||||
})
|
||||
test.AssertNotError(t, err, "Failed to enable required features flag")
|
||||
|
||||
// Create a test registration
|
||||
reg := satest.CreateWorkingRegistration(t, sa)
|
||||
|
||||
|
|
@ -2467,20 +2455,4 @@ func TestCountCertificatesRenewalBit(t *testing.T) {
|
|||
// be ignored as a renewal and CertC should be ignored because it isn't an
|
||||
// exact match.
|
||||
test.AssertEquals(t, countNameExact(t, "not-example.com"), int64(1))
|
||||
|
||||
// Disable the AllowRenewalFirstRL feature flag and check the counts for the
|
||||
// names in the certificate again.
|
||||
err = features.Set(map[string]bool{
|
||||
"AllowRenewalFirstRL": false,
|
||||
})
|
||||
test.AssertNotError(t, err, "Unexpected err clearing AllowRenewalFirstRL feature flag")
|
||||
|
||||
// The count for the base domain should be 3 now - certA, certB, and certC
|
||||
// should all count. CertB is not ignored as a renewal because the feature
|
||||
// flag is disabled.
|
||||
test.AssertEquals(t, countName(t, "not-example.com"), int64(3))
|
||||
|
||||
// The exact name count for the base domain should be 2 now: certA and certB.
|
||||
// CertB is not ignored as a renewal because the feature flag is disabled.
|
||||
test.AssertEquals(t, countNameExact(t, "not-example.com"), int64(2))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,8 +25,6 @@
|
|||
]
|
||||
},
|
||||
"features": {
|
||||
"AllowRenewalFirstRL": true,
|
||||
"SetIssuedNamesRenewalBit": true
|
||||
}
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -498,12 +498,6 @@ def test_renewal_exemption():
|
|||
and we are testing what we think we are testing. See
|
||||
https://letsencrypt.org/docs/rate-limits/ for more details.
|
||||
"""
|
||||
|
||||
# TODO(@cpu): Once the `AllowRenewalFirstRL` feature flag is enabled by
|
||||
# default, delete this early return.
|
||||
if not CONFIG_NEXT:
|
||||
return
|
||||
|
||||
base_domain = random_domain()
|
||||
# First issuance
|
||||
auth_and_issue(["www." + base_domain])
|
||||
|
|
|
|||
Loading…
Reference in New Issue