Check both current and old fqdnSets tables (#5668)

In `sa.checkFQDNSetExists`, query both the normal `fqdnSets` and the
`fqdnSets_old` tables. The `fqdnSets` table was recently truncated to
only have 7 days worth of data, but this helper function is used to
bypass other rate limits if there exists a prior certificate for the
exact same set of names, and that functionality cares about at least
90 days worth of data. Therefore we need to query both tables, at least
until `fqdnSets` contains 90 days worth of data again.

Also make a variety of other changes to support this change: creating
the `fqdnSets_old` table in our test environment, documenting various
places where it needs to be cleaned up, and removing some unused code.

Fixes #5671
This commit is contained in:
Aaron Gable 2021-09-24 12:34:25 -07:00 committed by GitHub
parent e0c3e2c1df
commit f21ba0d8a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 47 additions and 237 deletions

View File

@ -0,0 +1 @@
../../_db/migrations/20210924100000_OldFQDNSets.sql

View File

@ -0,0 +1,24 @@
-- TODO(#5670): Remove this file and the _db-next pointer to it.
-- +goose Up
-- SQL in section 'Up' is executed when this migration is applied
CREATE TABLE `fqdnSets_old` (
`id` bigint(20) NOT NULL AUTO_INCREMENT,
`setHash` binary(32) NOT NULL,
`serial` varchar(255) NOT NULL,
`issued` datetime NOT NULL,
`expires` datetime NOT NULL,
PRIMARY KEY (`id`),
UNIQUE KEY `serial` (`serial`),
KEY `setHash_issued_idx` (`setHash`,`issued`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
ALTER TABLE fqdnSets DROP INDEX IF EXISTS serial, ADD INDEX serial (serial);
ALTER TABLE fqdnSets PARTITION BY RANGE(id) (
PARTITION p_start VALUES LESS THAN MAXVALUE);
-- +goose Down
-- SQL section 'Down' is executed when this migration is rolled back
DROP TABLE `fqdnSets_old`

136
sa/sa.go
View File

@ -656,6 +656,10 @@ func (ssa *SQLStorageAuthority) CountFQDNSets(ctx context.Context, req *sapb.Cou
var count int64
err := ssa.dbReadOnlyMap.WithContext(ctx).SelectOne(
&count,
// We don't do a select across both fqdnSets and fqdnSets_old here because
// this method is only used for rate-limiting and we don't care to spend the
// extra CPU cycles checking the old table.
// TODO(#5670): Remove this comment when the partitioning is fixed.
`SELECT COUNT(1) FROM fqdnSets
WHERE setHash = ?
AND issued > ?`,
@ -668,119 +672,6 @@ func (ssa *SQLStorageAuthority) CountFQDNSets(ctx context.Context, req *sapb.Cou
// setHash is a []byte representing the hash of an FQDN Set
type setHash []byte
// getFQDNSetsBySerials finds the setHashes corresponding to a set of
// certificate serials. These serials can be used to check whether any
// certificates have been issued for the same set of names previously.
func (ssa *SQLStorageAuthority) getFQDNSetsBySerials(
dbMap db.Selector,
serials []string,
) ([]setHash, error) {
var fqdnSets []setHash
// It is unexpected that this function would be called with no serials
if len(serials) == 0 {
err := fmt.Errorf("getFQDNSetsBySerials called with no serials")
ssa.log.AuditErr(err.Error())
return nil, err
}
qmarks := make([]string, len(serials))
params := make([]interface{}, len(serials))
for i, serial := range serials {
params[i] = serial
qmarks[i] = "?"
}
query := "SELECT setHash FROM fqdnSets " +
"WHERE serial IN (" + strings.Join(qmarks, ",") + ")"
_, err := dbMap.Select(
&fqdnSets,
query,
params...)
if err != nil {
return nil, err
}
// The serials existed when we found them in issuedNames, they should continue
// to exist here. Otherwise an internal consistency violation occurred and
// needs to be audit logged
if db.IsNoRows(err) {
err := fmt.Errorf("getFQDNSetsBySerials returned no rows - internal consistency violation")
ssa.log.AuditErr(err.Error())
return nil, err
}
return fqdnSets, nil
}
// getNewIssuancesByFQDNSet returns a count of new issuances (renewals are not
// included) for a given slice of fqdnSets that occurred after the earliest
// parameter.
func (ssa *SQLStorageAuthority) getNewIssuancesByFQDNSet(
dbMap db.Selector,
fqdnSets []setHash,
earliest time.Time,
) (int, error) {
var results []struct {
Serial string
SetHash setHash
Issued time.Time
}
qmarks := make([]string, len(fqdnSets))
params := make([]interface{}, len(fqdnSets))
for i, setHash := range fqdnSets {
// We have to cast the setHash back to []byte here since the sql package
// isn't able to convert `sa.setHash` for the parameter value itself
params[i] = []byte(setHash)
qmarks[i] = "?"
}
query := "SELECT serial, setHash, issued FROM fqdnSets " +
"WHERE setHash IN (" + strings.Join(qmarks, ",") + ") " +
"ORDER BY setHash, issued"
// First, find the serial, sethash and issued date from the fqdnSets table for
// the given fqdn set hashes
_, err := dbMap.Select(
&results,
query,
params...)
if err != nil {
// If there are no results we have encountered a major error and
// should loudly complain
if db.IsNoRows(err) {
ssa.log.AuditErrf("Found no results from fqdnSets for setHashes known to exist: %#v", fqdnSets)
return 0, err
}
return -1, err
}
processedSetHashes := make(map[string]bool)
issuanceCount := 0
// Loop through each set hash result, counting issuances per unique set hash
// that are within the window specified by the earliest parameter
for _, result := range results {
key := string(result.SetHash)
// Skip set hashes that we have already processed - we only care about the
// first issuance
if processedSetHashes[key] {
continue
}
// If the issued date is before our earliest cutoff then skip it
if result.Issued.Before(earliest) {
continue
}
// Otherwise note the issuance and mark the set hash as processed
issuanceCount++
processedSetHashes[key] = true
}
// Return the count of how many non-renewal issuances there were
return issuanceCount, nil
}
// FQDNSetExists returns a bool indicating if one or more FQDN sets |names|
// exists in the database
func (ssa *SQLStorageAuthority) FQDNSetExists(ctx context.Context, req *sapb.FQDNSetExistsRequest) (*sapb.Exists, error) {
@ -801,15 +692,20 @@ type oneSelectorFunc func(holder interface{}, query string, args ...interface{})
// checkFQDNSetExists uses the given oneSelectorFunc to check whether an fqdnSet
// for the given names exists.
func (ssa *SQLStorageAuthority) checkFQDNSetExists(selector oneSelectorFunc, names []string) (bool, error) {
var count int64
namehash := hashNames(names)
var exists bool
err := selector(
&count,
`SELECT COUNT(1) FROM fqdnSets
WHERE setHash = ?
LIMIT 1`,
hashNames(names),
&exists,
// We select on both tables here because this function is used to determine
// if a given issuance is a renewal, and for that we care about 90 days
// worth of data, not just 7 days like the current fqdnSets table holds.
// TODO(#5670): Remove this OR when the partitioning is fixed.
`SELECT EXISTS (SELECT id FROM fqdnSets WHERE setHash = ? LIMIT 1)
OR EXISTS (SELECT id FROM fqdnSets_old WHERE setHash = ? LIMIT 1)`,
namehash,
namehash,
)
return count > 0, err
return exists, err
}
// PreviousCertificateExists returns true iff there was at least one certificate

View File

@ -885,123 +885,6 @@ func setupFQDNSets(t *testing.T, db *db.WrappedMap, fc clock.FakeClock) map[stri
return testcases
}
func TestGetFQDNSetsBySerials(t *testing.T) {
sa, fc, cleanUp := initSA(t)
defer cleanUp()
// Add the test fqdn sets
testcases := setupFQDNSets(t, sa.dbMap, fc)
// Asking for the fqdnSets for no serials should produce an error since this
// is not expected in normal conditions
fqdnSets, err := sa.getFQDNSetsBySerials(sa.dbMap, []string{})
test.AssertError(t, err, "No error calling getFQDNSetsBySerials for empty serials")
test.AssertEquals(t, len(fqdnSets), 0)
// Asking for the fqdnSets for serials that don't exist should return nothing
fqdnSets, err = sa.getFQDNSetsBySerials(sa.dbMap, []string{"this", "doesn't", "exist"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for non-existent serials")
test.AssertEquals(t, len(fqdnSets), 0)
// Asking for the fqdnSets for serial "a" should return the expectedHashA hash
fqdnSets, err = sa.getFQDNSetsBySerials(sa.dbMap, []string{"a"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"a\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["a"].ExpectedHash))
// Asking for the fqdnSets for serial "b" should return the expectedHashA hash
// because cert "b" has namesA subjects
fqdnSets, err = sa.getFQDNSetsBySerials(sa.dbMap, []string{"b"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"b\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["b"].ExpectedHash))
// Asking for the fqdnSets for serial "d" should return the expectedHashC hash
// because cert "d" has namesC subjects
fqdnSets, err = sa.getFQDNSetsBySerials(sa.dbMap, []string{"d"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"d\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["d"].ExpectedHash))
// Asking for the fqdnSets for serial "c" should return the expectedHashB hash
// because cert "c" has namesB subjects
fqdnSets, err = sa.getFQDNSetsBySerials(sa.dbMap, []string{"c"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"c\"")
test.AssertEquals(t, len(fqdnSets), 1)
test.AssertEquals(t, string(fqdnSets[0]), string(testcases["c"].ExpectedHash))
// Asking for the fqdnSets for serial "a", "b", "c" and "made up" should return
// the three expected hashes - two expectedHashA (for "a" and "b"), one
// expectedHashB (for "c")
expectedHashes := map[string]int{
string(testcases["a"].ExpectedHash): 2,
string(testcases["c"].ExpectedHash): 1,
}
fqdnSets, err = sa.getFQDNSetsBySerials(sa.dbMap, []string{"a", "b", "c", "made up"})
test.AssertNotError(t, err, "Error calling getFQDNSetsBySerials for serial \"a\", \"b\", \"c\", \"made up\"")
for _, setHash := range fqdnSets {
setHashKey := string(setHash)
if _, present := expectedHashes[setHashKey]; !present {
t.Errorf("Unexpected setHash in results: %#v", setHash)
}
expectedHashes[setHashKey]--
if expectedHashes[setHashKey] <= 0 {
delete(expectedHashes, setHashKey)
}
}
if len(expectedHashes) != 0 {
t.Errorf("Some expected setHashes were not observed: %#v", expectedHashes)
}
}
func TestGetNewIssuancesByFQDNSet(t *testing.T) {
sa, fc, cleanUp := initSA(t)
defer cleanUp()
// Add the test fqdn sets
testcases := setupFQDNSets(t, sa.dbMap, fc)
// Use one hour ago as the earliest cut off
earliest := fc.Now().Add(-time.Hour)
// Calling getNewIssuancesByFQDNSet with an empty FQDNSet should error
count, err := sa.getNewIssuancesByFQDNSet(sa.dbMap, nil, earliest)
test.AssertError(t, err, "No error calling getNewIssuancesByFQDNSet for empty fqdn set")
test.AssertEquals(t, count, -1)
// Calling getNewIssuancesByFQDNSet with FQDNSet hashes that don't exist
// should return 0
count, err = sa.getNewIssuancesByFQDNSet(sa.dbMap, []setHash{{0xC0, 0xFF, 0xEE}, {0x13, 0x37}}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for non-existent set hashes")
test.AssertEquals(t, count, 0)
// Calling getNewIssuancesByFQDNSet with the "a" expected hash should return
// 1, since both testcase "b" was a renewal of testcase "a"
count, err = sa.getNewIssuancesByFQDNSet(sa.dbMap, []setHash{testcases["a"].ExpectedHash}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase a")
test.AssertEquals(t, count, 1)
// Calling getNewIssuancesByFQDNSet with the "c" expected hash should return
// 1, since there is only one issuance for this sethash
count, err = sa.getNewIssuancesByFQDNSet(sa.dbMap, []setHash{testcases["c"].ExpectedHash}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase c")
test.AssertEquals(t, count, 1)
// Calling getNewIssuancesByFQDNSet with the "c" and "d" expected hashes should return
// only 1, since there is only one issuance for the provided set hashes that
// is within the earliest window. The issuance for "d" was too far in the past
// to be counted
count, err = sa.getNewIssuancesByFQDNSet(sa.dbMap, []setHash{testcases["c"].ExpectedHash, testcases["d"].ExpectedHash}, earliest)
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase c and d")
test.AssertEquals(t, count, 1)
// But by moving the earliest point behind the "d" issuance, we should now get a count of 2
count, err = sa.getNewIssuancesByFQDNSet(sa.dbMap, []setHash{testcases["c"].ExpectedHash, testcases["d"].ExpectedHash}, earliest.Add(-6*time.Hour))
test.AssertNotError(t, err, "Error calling getNewIssuancesByFQDNSet for testcase c and d with adjusted earliest")
test.AssertEquals(t, count, 2)
}
func TestNewOrder(t *testing.T) {
sa, _, cleanup := initSA(t)
defer cleanup()

View File

@ -83,3 +83,9 @@ GRANT SELECT ON registrations TO 'badkeyrevoker'@'localhost';
-- Test setup and teardown
GRANT ALL PRIVILEGES ON * to 'test_setup'@'localhost';
-- Temporary fqdnSets_old permissions
-- TODO(#5670): Remove these when partitioning is fixed.
GRANT SELECT,INSERT on fqdnSets_old TO 'sa'@'localhost';
GRANT SELECT on fqdnSets_old TO 'sa_ro'@'localhost';
GRANT SELECT ON fqdnSets_old TO 'mailer'@'localhost';