RA: Implement leaky bucket for duplicate certificate limit (#6262)

- Modify `ra.checkCertificatesPerFQDNSetLimit()` to use a leaky bucket algorithm
- Return issuance timestamps from `sa.FQDNSetTimestampsForWindow()` in descending order

Resolves #6154
This commit is contained in:
Samantha 2022-07-29 17:39:31 -07:00 committed by GitHub
parent 694d73d67b
commit 1464c34938
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 57 additions and 49 deletions

View File

@ -1376,7 +1376,7 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex
return nil
}
issuanceTimestampsInWindow, err := ra.SA.FQDNSetTimestampsForWindow(ctx, &sapb.CountFQDNSetsRequest{
prevIssuances, err := ra.SA.FQDNSetTimestampsForWindow(ctx, &sapb.CountFQDNSetsRequest{
Domains: names,
Window: limit.Window.Duration.Nanoseconds(),
})
@ -1384,16 +1384,30 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex
return fmt.Errorf("checking duplicate certificate limit for %q: %s", names, err)
}
if int64(len(issuanceTimestampsInWindow.Timestamps)) >= threshold {
// FQDNSetTimestampsForWindow returns the issuance timestamps in
// ascending order so we can assume that the first one is the oldest.
retryAfter := time.Unix(0, issuanceTimestampsInWindow.Timestamps[0]).Add(limit.Window.Duration)
if int64(len(prevIssuances.Timestamps)) < threshold {
// Issuance in window is below the threshold, no need to limit.
return nil
} else {
// Evaluate the rate limit using a leaky bucket algorithm. The bucket
// has a capacity of threshold and is refilled at a rate of 1 token per
// limit.Window/threshold from the time of each issuance timestamp.
now := ra.clk.Now()
nsPerToken := limit.Window.Nanoseconds() / threshold
for i, timestamp := range prevIssuances.Timestamps {
tokensGeneratedSince := now.Add(-time.Duration(int64(i+1) * nsPerToken))
if time.Unix(0, timestamp).Before(tokensGeneratedSince) {
// We know `i+1` tokens were generated since `tokenGeneratedSince`,
// and only `i` certificates were issued, so there's room to allow
// for an additional issuance.
return nil
}
}
retryTime := time.Unix(0, prevIssuances.Timestamps[0]).Add(time.Duration(nsPerToken))
return berrors.DuplicateCertificateError(
"too many certificates (%d) already issued for this exact set of domains in the last %.0f hours: %s, retry after %s",
threshold, limit.Window.Duration.Hours(), strings.Join(names, ","), retryAfter.Format(time.RFC3339),
threshold, limit.Window.Duration.Hours(), strings.Join(names, ","), retryTime.Format(time.RFC3339),
)
}
return nil
}
func (ra *RegistrationAuthorityImpl) checkLimits(ctx context.Context, names []string, regID int64) error {

View File

@ -1223,25 +1223,28 @@ func TestCheckExactCertificateLimit(t *testing.T) {
const dupeCertLimit = 3
rlp := ratelimit.RateLimitPolicy{
Threshold: dupeCertLimit,
Window: cmd.ConfigDuration{Duration: 23 * time.Hour},
Window: cmd.ConfigDuration{Duration: 24 * time.Hour},
}
// Create a mock SA that has a count of already issued certificates for some
// test names
firstIssuanceTimestamp := ra.clk.Now().Add(-23 * time.Hour)
firstIssuanceTimestamp := ra.clk.Now().Add(-rlp.Window.Duration)
issuanceTimestamps := []int64{
firstIssuanceTimestamp.Add(time.Hour * 23).UnixNano(),
firstIssuanceTimestamp.Add(time.Hour * 16).UnixNano(),
firstIssuanceTimestamp.Add(time.Hour * 8).UnixNano(),
firstIssuanceTimestamp.UnixNano(),
firstIssuanceTimestamp.Add(time.Hour).UnixNano(),
firstIssuanceTimestamp.Add(time.Hour * 2).UnixNano(),
firstIssuanceTimestamp.Add(time.Hour * 3).UnixNano(),
}
expectRetryAfter := time.Unix(0, issuanceTimestamps[0]).Add(23 * time.Hour).Format(time.RFC3339)
// Our window is 24 hours and our threshold is 3 issuance. If our most
// recent issuance was 1 hour ago, we expect the next token to be available
// 8 hours from issuance time or 7 hours from now.
expectRetryAfter := time.Unix(0, issuanceTimestamps[0]).Add(time.Hour * 8).Format(time.RFC3339)
ra.SA = &mockSAWithFQDNSet{
issuanceTimestamps: map[string]*sapb.Timestamps{
"none.example.com": {Timestamps: []int64{}},
"under.example.com": {Timestamps: issuanceTimestamps[0 : dupeCertLimit-1]},
"equal.example.com": {Timestamps: issuanceTimestamps[0:dupeCertLimit]},
"over.example.com": {Timestamps: issuanceTimestamps[0 : dupeCertLimit+1]},
"none.example.com": {Timestamps: []int64{}},
"under.example.com": {Timestamps: issuanceTimestamps[3:3]},
"equalbutvalid.example.com": {Timestamps: issuanceTimestamps[1:3]},
"over.example.com": {Timestamps: issuanceTimestamps[0:3]},
},
t: t,
}
@ -1262,18 +1265,15 @@ func TestCheckExactCertificateLimit(t *testing.T) {
ExpectedErr: nil,
},
{
Name: "FQDN set issuances equal to limit",
Domain: "equal.example.com",
ExpectedErr: fmt.Errorf(
"too many certificates (3) already issued for this exact set of domains in the last 23 hours: equal.example.com, retry after %s: see https://letsencrypt.org/docs/duplicate-certificate-limit/",
expectRetryAfter,
),
Name: "FQDN set issuances equal to limit",
Domain: "equalbutvalid.example.com",
ExpectedErr: nil,
},
{
Name: "FQDN set issuances above limit",
Domain: "over.example.com",
ExpectedErr: fmt.Errorf(
"too many certificates (3) already issued for this exact set of domains in the last 23 hours: over.example.com, retry after %s: see https://letsencrypt.org/docs/duplicate-certificate-limit/",
"too many certificates (3) already issued for this exact set of domains in the last 24 hours: over.example.com, retry after %s: see https://letsencrypt.org/docs/duplicate-certificate-limit/",
expectRetryAfter,
),
},

View File

@ -697,8 +697,8 @@ func (ssa *SQLStorageAuthority) CountFQDNSets(ctx context.Context, req *sapb.Cou
}
// FQDNSetTimestampsForWindow returns the issuance timestamps for each
// certificate, issued for a set of domains, during a given window of time, in
// ascending order.
// certificate, issued for a set of domains, during a given window of time,
// starting from the most recent issuance.
func (ssa *SQLStorageAuthority) FQDNSetTimestampsForWindow(ctx context.Context, req *sapb.CountFQDNSetsRequest) (*sapb.Timestamps, error) {
if req.Window == 0 || len(req.Domains) == 0 {
return nil, errIncompleteRequest
@ -712,7 +712,7 @@ func (ssa *SQLStorageAuthority) FQDNSetTimestampsForWindow(ctx context.Context,
`SELECT issued FROM fqdnSets
WHERE setHash = ?
AND issued > ?
ORDER BY issued ASC`,
ORDER BY issued DESC`,
HashNames(req.Domains),
ssa.clk.Now().Add(-time.Duration(req.Window)),
)

View File

@ -637,69 +637,63 @@ func TestFQDNSetTimestampsForWindow(t *testing.T) {
test.AssertNotError(t, err, "Failed to open transaction")
names := []string{"a.example.com", "B.example.com"}
threeHours := time.Hour * 3
window := time.Hour * 3
req := &sapb.CountFQDNSetsRequest{
Domains: names,
Window: threeHours.Nanoseconds(),
Window: window.Nanoseconds(),
}
// zero issuance timestamps for names
// Ensure zero issuance has occurred for names.
resp, err := sa.FQDNSetTimestampsForWindow(ctx, req)
test.AssertNotError(t, err, "Failed to count name sets")
test.AssertEquals(t, len(resp.Timestamps), 0)
// add a valid set inside the window
// Add an issuance for names inside the window.
expires := fc.Now().Add(time.Hour * 2).UTC()
firstIssued := fc.Now()
err = addFQDNSet(tx, names, "serial", firstIssued, expires)
test.AssertNotError(t, err, "Failed to add name set")
test.AssertNotError(t, tx.Commit(), "Failed to commit transaction")
// only one valid
// Ensure there's 1 issuance timestamp for names inside the window.
resp, err = sa.FQDNSetTimestampsForWindow(ctx, req)
test.AssertNotError(t, err, "Failed to count name sets")
test.AssertEquals(t, len(resp.Timestamps), 1)
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[0]).UTC())
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[len(resp.Timestamps)-1]).UTC())
// check hash isn't affected by changing name order/casing
// Ensure that the hash isn't affected by changing name order/casing.
req.Domains = []string{"b.example.com", "A.example.COM"}
resp, err = sa.FQDNSetTimestampsForWindow(ctx, req)
test.AssertNotError(t, err, "Failed to count name sets")
test.AssertEquals(t, len(resp.Timestamps), 1)
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[0]).UTC())
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[len(resp.Timestamps)-1]).UTC())
// add another valid set
// Add another issuance for names inside the window.
tx, err = sa.dbMap.Begin()
test.AssertNotError(t, err, "Failed to open transaction")
err = addFQDNSet(tx, names, "anotherSerial", firstIssued, expires)
test.AssertNotError(t, err, "Failed to add name set")
test.AssertNotError(t, tx.Commit(), "Failed to commit transaction")
// only two valid
// Ensure there are two issuance timestamps for names inside the window.
req.Domains = names
resp, err = sa.FQDNSetTimestampsForWindow(ctx, req)
test.AssertNotError(t, err, "Failed to count name sets")
test.AssertEquals(t, len(resp.Timestamps), 2)
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[0]).UTC())
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[len(resp.Timestamps)-1]).UTC())
// add an expired set
// Add another issuance for names but just outside the window.
tx, err = sa.dbMap.Begin()
test.AssertNotError(t, err, "Failed to open transaction")
err = addFQDNSet(
tx,
names,
"yetAnotherSerial",
firstIssued.Add(-threeHours),
expires.Add(-threeHours),
)
err = addFQDNSet(tx, names, "yetAnotherSerial", firstIssued.Add(-window), expires)
test.AssertNotError(t, err, "Failed to add name set")
test.AssertNotError(t, tx.Commit(), "Failed to commit transaction")
// only two valid
// Ensure there are still only two issuance timestamps in the window.
resp, err = sa.FQDNSetTimestampsForWindow(ctx, req)
test.AssertNotError(t, err, "Failed to count name sets")
test.AssertEquals(t, len(resp.Timestamps), 2)
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[0]).UTC())
test.AssertEquals(t, firstIssued, time.Unix(0, resp.Timestamps[len(resp.Timestamps)-1]).UTC())
}
func TestFQDNSetsExists(t *testing.T) {

View File

@ -50,7 +50,7 @@ certificatesPerFQDNSet:
ecdsa.le.wtf: 10000
must-staple.le.wtf: 10000
certificatesPerFQDNSetFast:
window: 2h
window: 3h
threshold: 2
overrides:
le.wtf: 100