From 56c45d1330cc39944d85499a9119f2d525e5d78f Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 9 Mar 2016 11:31:08 -0800 Subject: [PATCH] Bypass per domain rate limit if FQDN set was previously issued In ra.checkCertificatesPerName allow a bypass of the rate limit if the exact name set has previously been issued for. This should make a few current scenarios people have been running into slightly less painful. --- core/interfaces.go | 1 + mocks/mocks.go | 5 ++++ ra/registration-authority.go | 11 +++++++ ra/registration-authority_test.go | 4 +++ rpc/rpc-wrappers.go | 49 +++++++++++++++++++++++++++++++ sa/storage-authority.go | 18 ++++++++++-- sa/storage-authority_test.go | 22 ++++++++++++++ 7 files changed, 108 insertions(+), 2 deletions(-) diff --git a/core/interfaces.go b/core/interfaces.go index 991ef8ff7..b3e26241d 100644 --- a/core/interfaces.go +++ b/core/interfaces.go @@ -106,6 +106,7 @@ type StorageGetter interface { CountPendingAuthorizations(regID int64) (int, error) GetSCTReceipt(string, string) (SignedCertificateTimestamp, error) CountFQDNSets(time.Duration, []string) (int64, error) + FQDNSetExists([]string) (bool, error) } // StorageAdder are the Boulder SA's write/update methods diff --git a/mocks/mocks.go b/mocks/mocks.go index afe0f947f..16b89052e 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -252,6 +252,11 @@ func (sa *StorageAuthority) CountFQDNSets(since time.Duration, names []string) ( return 0, nil } +// FQDNSetExists is a mock +func (sa *StorageAuthority) FQDNSetExists(names []string) (bool, error) { + return false, nil +} + // GetLatestValidAuthorization is a mock func (sa *StorageAuthority) GetLatestValidAuthorization(registrationID int64, identifier core.AcmeIdentifier) (authz core.Authorization, err error) { if registrationID == 1 && identifier.Type == "dns" { diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 7e27b4520..79eb38513 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -635,6 +635,17 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(names []strin } } if len(badNames) > 0 { + // check if there is already a existing certificate for + // the exact name set we are issuing for. If so bypass the + // the certificatesPerName limit. + exists, err := ra.SA.FQDNSetExists(names) + if err != nil { + return err + } + if exists { + ra.certsForDomainStats.Inc("FQDNSetBypass", 1) + return nil + } domains := strings.Join(badNames, ", ") ra.certsForDomainStats.Inc("Exceeded", 1) ra.log.Info(fmt.Sprintf("Rate limit exceeded, CertificatesForDomain, regID: %d, domains: %s", regID, domains)) diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index 6b71bac22..32d56eea6 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -814,6 +814,10 @@ func (m mockSAWithNameCounts) CountCertificatesByNames(names []string, earliest, return m.nameCounts, nil } +func (m mockSAWithNameCounts) FQDNSetExists(names []string) (bool, error) { + return false, nil +} + func TestCheckCertificatesPerNameLimit(t *testing.T) { _, _, ra, fc, cleanUp := initAuthorities(t) defer cleanUp() diff --git a/rpc/rpc-wrappers.go b/rpc/rpc-wrappers.go index 5ab5f17e9..048ec3a0b 100644 --- a/rpc/rpc-wrappers.go +++ b/rpc/rpc-wrappers.go @@ -71,6 +71,7 @@ const ( MethodSubmitToCT = "SubmitToCT" // Pub MethodRevokeAuthorizationsByDomain = "RevokeAuthorizationsByDomain" // SA MethodCountFQDNSets = "CountFQDNSets" // SA + MethodFQDNSetExists = "FQDNSetExists" // SA ) // Request structs @@ -175,6 +176,10 @@ type countFQDNsRequest struct { Names []string } +type fqdnSetExistsRequest struct { + Names []string +} + // Response structs type caaResponse struct { Present bool @@ -191,6 +196,10 @@ type countFQDNSetsResponse struct { Count int64 } +type fqdnSetExistsResponse struct { + Exists bool +} + func improperMessage(method string, err error, obj interface{}) { log := blog.GetAuditLogger() log.AuditErr(fmt.Errorf("Improper message. method: %s err: %s data: %+v", method, err, obj)) @@ -1142,6 +1151,30 @@ func NewStorageAuthorityServer(rpc Server, impl core.StorageAuthority) error { return }) + rpc.Handle(MethodFQDNSetExists, func(req []byte) (response []byte, err error) { + var r fqdnSetExistsRequest + err = json.Unmarshal(req, &r) + if err != nil { + // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 + errorCondition(MethodFQDNSetExists, err, req) + return + } + exists, err := impl.FQDNSetExists(r.Names) + if err != nil { + // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 + errorCondition(MethodFQDNSetExists, err, req) + return + } + response, err = json.Marshal(fqdnSetExistsResponse{exists}) + if err != nil { + // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 + errorCondition(MethodFQDNSetExists, err, req) + return + } + + return + }) + return nil } @@ -1530,3 +1563,19 @@ func (cac StorageAuthorityClient) CountFQDNSets(window time.Duration, names []st err = json.Unmarshal(response, &count) return count.Count, err } + +// FQDNSetExists returns a bool indicating whether the FQDN set |name| +// exists in the database +func (cac StorageAuthorityClient) FQDNSetExists(names []string) (bool, error) { + data, err := json.Marshal(fqdnSetExistsRequest{names}) + if err != nil { + return false, err + } + response, err := cac.rpc.DispatchSync(MethodFQDNSetExists, data) + if err != nil { + return false, err + } + var exists fqdnSetExistsResponse + err = json.Unmarshal(response, exists) + return exists.Exists, err +} diff --git a/sa/storage-authority.go b/sa/storage-authority.go index 391d12d36..289d14d45 100644 --- a/sa/storage-authority.go +++ b/sa/storage-authority.go @@ -913,10 +913,24 @@ func (ssa *SQLStorageAuthority) CountFQDNSets(window time.Duration, names []stri err := ssa.dbMap.SelectOne( &count, `SELECT COUNT(1) FROM fqdnSets - WHERE setHash = ? - AND issued > ?`, + WHERE setHash = ? + AND issued > ? + LIMIT 1`, hashNames(names), ssa.clk.Now().Add(-window), ) return count, err } + +// FQDNSetExists returns a bool indicating if one or more FQDN sets |names| +// exists in the database +func (ssa *SQLStorageAuthority) FQDNSetExists(names []string) (bool, error) { + var count int64 + err := ssa.dbMap.SelectOne( + &count, + `SELECT COUNT(1) FROM fqdnSets + WHERE setHash = ?`, + hashNames(names), + ) + return count > 0, err +} diff --git a/sa/storage-authority_test.go b/sa/storage-authority_test.go index 05d0f948c..6edc28f2d 100644 --- a/sa/storage-authority_test.go +++ b/sa/storage-authority_test.go @@ -725,6 +725,28 @@ func TestFQDNSets(t *testing.T) { test.AssertEquals(t, count, int64(2)) } +func TestFQDNSetsExists(t *testing.T) { + sa, fc, cleanUp := initSA(t) + defer cleanUp() + + names := []string{"a.example.com", "B.example.com"} + exists, err := sa.FQDNSetExists(names) + test.AssertNotError(t, err, "Failed to check FQDN set existence") + test.Assert(t, !exists, "FQDN set shouldn't exist") + + tx, err := sa.dbMap.Begin() + test.AssertNotError(t, err, "Failed to open transaction") + expires := fc.Now().Add(time.Hour * 2).UTC() + issued := fc.Now() + err = addFQDNSet(tx, names, "serial", issued, expires) + test.AssertNotError(t, err, "Failed to add name set") + test.AssertNotError(t, tx.Commit(), "Failed to commit transaction") + + exists, err = sa.FQDNSetExists(names) + test.AssertNotError(t, err, "Failed to check FQDN set existence") + test.Assert(t, exists, "FQDN set does exist") +} + type execRecorder struct { query string args []interface{}