Skip to content

It's borpin' time! #6982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 17, 2023
Merged

It's borpin' time! #6982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4d76b81
Switch to borp
jsha Jul 6, 2023
e37296c
Add contexts everywhere; remove WithContext
jsha Jul 6, 2023
f73d94b
Unexport borp.DbMap from db.WrappedMap
jsha Jul 7, 2023
131cd62
go mod tidy
jsha Jul 7, 2023
4a5b448
Review feedback part 1
jsha Jul 11, 2023
1f8d9ef
Merge remote-tracking branch 'le/main' into borp
jsha Jul 11, 2023
d0d0280
Bump gorp to 6cc6ce5802433d52a6f8164a3d0e55263bdc00d9
jsha Jul 11, 2023
8584011
s/txWithCtx/tx/g
jsha Jul 11, 2023
a626fe3
Plumb through more context in cert-checker
jsha Jul 11, 2023
2b8c3f8
Use context.TODO in more places
jsha Jul 11, 2023
a39550f
Remove some context assumptions
jsha Jul 12, 2023
657db6d
Fixups
jsha Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions cmd/admin-revoker/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (r *revoker) revokeCertificate(ctx context.Context, certObj core.Certificat
}

func (r *revoker) revokeBySerial(ctx context.Context, serial string, reasonCode revocation.Reason, skipBlockKey bool) error {
certObj, err := sa.SelectPrecertificate(r.dbMap, serial)
certObj, err := sa.SelectPrecertificate(ctx, r.dbMap, serial)
if err != nil {
if db.IsNoRows(err) {
return berrors.NotFoundError("precertificate with serial %q not found", serial)
Expand Down Expand Up @@ -224,7 +224,7 @@ func (r *revoker) revokeSerialBatchFile(ctx context.Context, serialPath string,
// Finding relevant accounts will be very slow because it does not use an index.
func (r *revoker) clearEmailAddress(ctx context.Context, email string) error {
r.log.AuditInfof("Scanning database for accounts with email addresses matching %q in order to clear the email addresses.", email)
regIDs, err := r.getRegIDsMatchingEmail(email)
regIDs, err := r.getRegIDsMatchingEmail(ctx, email)
if err != nil {
return err
}
Expand All @@ -233,7 +233,7 @@ func (r *revoker) clearEmailAddress(ctx context.Context, email string) error {

failures := 0
for _, regID := range regIDs {
err := sa.ClearEmail(r.dbMap, ctx, regID, email)
err := sa.ClearEmail(ctx, r.dbMap, regID, email)
if err != nil {
// Log, but don't fail, because it took a long time to find the relevant registration IDs
// and we don't want to have to redo that work.
Expand Down Expand Up @@ -297,7 +297,7 @@ func (r *revoker) revokeByReg(ctx context.Context, regID int64, reasonCode revoc
return fmt.Errorf("couldn't fetch registration: %w", err)
}

certObjs, err := sa.SelectPrecertificates(r.dbMap, "WHERE registrationID = :regID", map[string]interface{}{"regID": regID})
certObjs, err := sa.SelectPrecertificates(ctx, r.dbMap, "WHERE registrationID = :regID", map[string]interface{}{"regID": regID})
if err != nil {
return err
}
Expand Down Expand Up @@ -372,7 +372,7 @@ func (r *revoker) revokeByPrivateKey(ctx context.Context, privateKey string) err
return err
}

matches, err := r.getCertsMatchingSPKIHash(spkiHash)
matches, err := r.getCertsMatchingSPKIHash(ctx, spkiHash)
if err != nil {
return err
}
Expand Down Expand Up @@ -408,9 +408,9 @@ func (r *revoker) revokeByPrivateKey(ctx context.Context, privateKey string) err
return nil
}

func (r *revoker) spkiHashInBlockedKeys(spkiHash []byte) (bool, error) {
func (r *revoker) spkiHashInBlockedKeys(ctx context.Context, spkiHash []byte) (bool, error) {
var count int
err := r.dbMap.SelectOne(&count, "SELECT COUNT(*) as count FROM blockedKeys WHERE keyHash = ?", spkiHash)
err := r.dbMap.SelectOne(ctx, &count, "SELECT COUNT(*) as count FROM blockedKeys WHERE keyHash = ?", spkiHash)
if err != nil {
return false, err
}
Expand All @@ -421,9 +421,9 @@ func (r *revoker) spkiHashInBlockedKeys(spkiHash []byte) (bool, error) {
return false, nil
}

func (r *revoker) countCertsMatchingSPKIHash(spkiHash []byte) (int, error) {
func (r *revoker) countCertsMatchingSPKIHash(ctx context.Context, spkiHash []byte) (int, error) {
var count int
err := r.dbMap.SelectOne(&count, "SELECT COUNT(*) as count FROM keyHashToSerial WHERE keyHash = ?", spkiHash)
err := r.dbMap.SelectOne(ctx, &count, "SELECT COUNT(*) as count FROM keyHashToSerial WHERE keyHash = ?", spkiHash)
if err != nil {
return 0, err
}
Expand All @@ -434,11 +434,11 @@ func (r *revoker) countCertsMatchingSPKIHash(spkiHash []byte) (int, error) {
// contains the given email address. Since this uses a substring match, it is important
// to subsequently parse the JSON list of addresses and look for exact matches.
// Note: Since this does not use an index, it is very slow.
func (r *revoker) getRegIDsMatchingEmail(email string) ([]int64, error) {
func (r *revoker) getRegIDsMatchingEmail(ctx context.Context, email string) ([]int64, error) {
// We use SQL `CONCAT` rather than interpolating with `+` or `%s` because we want to
// use a `?` placeholder for the email, which prevents SQL injection.
var regIDs []int64
_, err := r.dbMap.Select(&regIDs, "SELECT id FROM registrations WHERE contact LIKE CONCAT('%\"mailto:', ?, '\"%')", email)
_, err := r.dbMap.Select(ctx, &regIDs, "SELECT id FROM registrations WHERE contact LIKE CONCAT('%\"mailto:', ?, '\"%')", email)
if err != nil {
return nil, err
}
Expand All @@ -447,9 +447,9 @@ func (r *revoker) getRegIDsMatchingEmail(email string) ([]int64, error) {

// TODO(#5899) Use an non-wrapped sql.Db client to iterate over results and
// return them on a channel.
func (r *revoker) getCertsMatchingSPKIHash(spkiHash []byte) ([]string, error) {
func (r *revoker) getCertsMatchingSPKIHash(ctx context.Context, spkiHash []byte) ([]string, error) {
var h []string
_, err := r.dbMap.Select(&h, "SELECT certSerial FROM keyHashToSerial WHERE keyHash = ?", spkiHash)
_, err := r.dbMap.Select(ctx, &h, "SELECT certSerial FROM keyHashToSerial WHERE keyHash = ?", spkiHash)
if err != nil {
if db.IsNoRows(err) {
return nil, berrors.NotFoundError("no certificates with a matching SPKI hash were found")
Expand All @@ -466,8 +466,8 @@ func (rc revocationCodes) Len() int { return len(rc) }
func (rc revocationCodes) Less(i, j int) bool { return rc[i] < rc[j] }
func (rc revocationCodes) Swap(i, j int) { rc[i], rc[j] = rc[j], rc[i] }

func privateKeyBlock(r *revoker, dryRun bool, comment string, count int, spkiHash []byte, keyPath string) error {
keyExists, err := r.spkiHashInBlockedKeys(spkiHash)
func privateKeyBlock(ctx context.Context, r *revoker, dryRun bool, comment string, count int, spkiHash []byte, keyPath string) error {
keyExists, err := r.spkiHashInBlockedKeys(ctx, spkiHash)
if err != nil {
return fmt.Errorf("while checking if the provided key already exists in the 'blockedKeys' table: %s", err)
}
Expand Down Expand Up @@ -640,12 +640,12 @@ func main() {
spkiHash, err := getPublicKeySPKIHash(publicKey)
cmd.FailOnError(err, "While obtaining the SPKI hash for the provided key")

count, err := r.countCertsMatchingSPKIHash(spkiHash)
count, err := r.countCertsMatchingSPKIHash(ctx, spkiHash)
cmd.FailOnError(err, "While retrieving a count of certificates matching the provided key")
r.log.AuditInfof("Found %d certificates matching the provided key", count)

if command == "private-key-block" {
err := privateKeyBlock(r, *dryRun, *comment, count, spkiHash, keyPath)
err := privateKeyBlock(ctx, r, *dryRun, *comment, count, spkiHash, keyPath)
cmd.FailOnError(err, "")
}

Expand Down
47 changes: 27 additions & 20 deletions cmd/admin-revoker/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,10 @@ func TestRevokeIncidentTableSerials(t *testing.T) {
test.Assert(t, len(testCtx.log.GetAllMatching("No serials found in incident table")) > 0, "Expected log output not found")
testCtx.log.Clear()

_, err = testIncidentsDbMap.Exec(
ctx := context.Background()

_, err = testIncidentsDbMap.ExecContext(
ctx,
fmt.Sprintf("INSERT INTO incident_foo (%s) VALUES ('%s', %d, %d, '%s')",
"serial, registrationID, orderID, lastNoticeSent",
core.SerialToString(entries[0].serial),
Expand All @@ -104,14 +107,14 @@ func TestRevokeIncidentTableSerials(t *testing.T) {
)
test.AssertNotError(t, err, "while inserting row into incident table")

err = testCtx.revoker.revokeIncidentTableSerials(context.Background(), "incident_foo", 0, 1)
err = testCtx.revoker.revokeIncidentTableSerials(ctx, "incident_foo", 0, 1)
test.AssertNotError(t, err, "revokeIncidentTableSerials failed")

// Ensure that a populated incident table results in the expected log output.
test.AssertNotError(t, err, "revokeIncidentTableSerials failed")
test.Assert(t, len(testCtx.log.GetAllMatching("No serials found in incident table")) <= 0, "Expected log output not found")

status, err := testCtx.ssa.GetCertificateStatus(context.Background(), &sapb.Serial{Serial: core.SerialToString(entries[0].serial)})
status, err := testCtx.ssa.GetCertificateStatus(ctx, &sapb.Serial{Serial: core.SerialToString(entries[0].serial)})
test.AssertNotError(t, err, "failed to retrieve certificate status")
test.AssertEquals(t, core.OCSPStatus(status.Status), core.OCSPStatusRevoked)
}
Expand Down Expand Up @@ -143,9 +146,11 @@ func TestBlockAndRevokeByPrivateKey(t *testing.T) {
spkiHash, err := getPublicKeySPKIHash(&duplicateEntry.testKey.PublicKey)
test.AssertNotError(t, err, "Failed to get SPKI hash for dupe.")

ctx := context.Background()

// Ensure that the SPKI hash hasn't already been added to the blockedKeys
// table.
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(spkiHash)
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(ctx, spkiHash)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.Assert(t, !keyExists, "SPKI hash should not be in blockedKeys")

Expand All @@ -155,19 +160,19 @@ func TestBlockAndRevokeByPrivateKey(t *testing.T) {
switch e.names[0] {
case uniqueEntries[0].names[0]:
// example-1337.com
count, err := testCtx.revoker.countCertsMatchingSPKIHash(e.spkiHash)
count, err := testCtx.revoker.countCertsMatchingSPKIHash(ctx, e.spkiHash)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for entry failed")
test.AssertEquals(t, count, 2)

case uniqueEntries[1].names[0]:
// example-1338.com
count, err := testCtx.revoker.countCertsMatchingSPKIHash(e.spkiHash)
count, err := testCtx.revoker.countCertsMatchingSPKIHash(ctx, e.spkiHash)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for entry failed")
test.AssertEquals(t, count, 1)

case uniqueEntries[2].names[0]:
// example-1339.com
count, err := testCtx.revoker.countCertsMatchingSPKIHash(e.spkiHash)
count, err := testCtx.revoker.countCertsMatchingSPKIHash(ctx, e.spkiHash)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for entry failed")
test.AssertEquals(t, count, 1)
}
Expand All @@ -184,7 +189,7 @@ func TestBlockAndRevokeByPrivateKey(t *testing.T) {
test.AssertNotError(t, err, "While attempting to revoke certificates for the provided key")

// Ensure that the key is not blocked, yet.
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(spkiHash)
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(ctx, spkiHash)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.Assert(t, !keyExists, "SPKI hash should not be in blockedKeys")

Expand All @@ -193,7 +198,7 @@ func TestBlockAndRevokeByPrivateKey(t *testing.T) {
test.AssertNotError(t, err, "While attempting to block issuance for the provided key")

// Ensure that the key is now blocked.
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(spkiHash)
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(ctx, spkiHash)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.Assert(t, keyExists, "SPKI hash should not be in blockedKeys")

Expand All @@ -203,6 +208,7 @@ func TestBlockAndRevokeByPrivateKey(t *testing.T) {
}

func TestPrivateKeyBlock(t *testing.T) {
ctx := context.Background()
testCtx := setup(t)
defer testCtx.cleanUp()

Expand Down Expand Up @@ -231,35 +237,35 @@ func TestPrivateKeyBlock(t *testing.T) {

// Query the 'keyHashToSerial' table for certificates with a matching SPKI
// hash. We expect that since this key was re-used we'll find 2 matches.
count, err := testCtx.revoker.countCertsMatchingSPKIHash(duplicateKeySPKI)
count, err := testCtx.revoker.countCertsMatchingSPKIHash(ctx, duplicateKeySPKI)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.AssertEquals(t, count, 2)

// With dryRun=true this should not block the key.
err = privateKeyBlock(&testCtx.revoker, true, "", count, duplicateKeySPKI, duplicateKeyFile.Name())
err = privateKeyBlock(ctx, &testCtx.revoker, true, "", count, duplicateKeySPKI, duplicateKeyFile.Name())
test.AssertNotError(t, err, "While attempting to block issuance for the provided key")

// Ensure that the key is not blocked, yet.
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(duplicateKeySPKI)
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(ctx, duplicateKeySPKI)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.Assert(t, !keyExists, "SPKI hash should not be in blockedKeys")

// With dryRun=false this should block the key.
comment := "key blocked as part of test"
err = privateKeyBlock(&testCtx.revoker, false, comment, count, duplicateKeySPKI, duplicateKeyFile.Name())
err = privateKeyBlock(ctx, &testCtx.revoker, false, comment, count, duplicateKeySPKI, duplicateKeyFile.Name())
test.AssertNotError(t, err, "While attempting to block issuance for the provided key")

// With dryRun=false this should result in an error as the key is already blocked.
err = privateKeyBlock(&testCtx.revoker, false, "", count, duplicateKeySPKI, duplicateKeyFile.Name())
err = privateKeyBlock(ctx, &testCtx.revoker, false, "", count, duplicateKeySPKI, duplicateKeyFile.Name())
test.AssertError(t, err, "Attempting to block a key which is already blocked should have failed.")

// Ensure that the key is now blocked.
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(duplicateKeySPKI)
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(ctx, duplicateKeySPKI)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.Assert(t, keyExists, "SPKI hash should not be in blockedKeys")

// Ensure that the comment was set as expected
commentFromDB, err := testCtx.dbMap.SelectStr("SELECT comment from blockedKeys WHERE keyHash = ?", duplicateKeySPKI)
commentFromDB, err := testCtx.dbMap.SelectStr(ctx, "SELECT comment from blockedKeys WHERE keyHash = ?", duplicateKeySPKI)
test.AssertNotError(t, err, "Failed to get comment from database")
u, err := user.Current()
test.AssertNotError(t, err, "Failed to get current user")
Expand All @@ -268,6 +274,7 @@ func TestPrivateKeyBlock(t *testing.T) {
}

func TestPrivateKeyRevoke(t *testing.T) {
ctx := context.Background()
testCtx := setup(t)
defer testCtx.cleanUp()

Expand Down Expand Up @@ -296,7 +303,7 @@ func TestPrivateKeyRevoke(t *testing.T) {

// Query the 'keyHashToSerial' table for certificates with a matching SPKI
// hash. We expect that since this key was re-used we'll find 2 matches.
count, err := testCtx.revoker.countCertsMatchingSPKIHash(duplicateKeySPKI)
count, err := testCtx.revoker.countCertsMatchingSPKIHash(ctx, duplicateKeySPKI)
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
test.AssertEquals(t, count, 2)

Expand All @@ -305,7 +312,7 @@ func TestPrivateKeyRevoke(t *testing.T) {
test.AssertNotError(t, err, "While attempting to block issuance for the provided key")

// Ensure that the key is not blocked, yet.
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(duplicateKeySPKI)
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(ctx, duplicateKeySPKI)
test.AssertNotError(t, err, "spkiHashInBlockedKeys failed for key that shouldn't be blocked yet")
test.Assert(t, !keyExists, "SPKI hash should not be in blockedKeys")

Expand All @@ -315,12 +322,12 @@ func TestPrivateKeyRevoke(t *testing.T) {
test.AssertNotError(t, err, "While attempting to block issuance for the provided key")

// Ensure that the key is now blocked.
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(duplicateKeySPKI)
keyExists, err = testCtx.revoker.spkiHashInBlockedKeys(ctx, duplicateKeySPKI)
test.AssertNotError(t, err, "spkiHashInBlockedKeys failed for key that should now be blocked")
test.Assert(t, keyExists, "SPKI hash should not be in blockedKeys")

// Ensure that the comment was set as expected
commentFromDB, err := testCtx.dbMap.SelectStr("SELECT comment from blockedKeys WHERE keyHash = ?", duplicateKeySPKI)
commentFromDB, err := testCtx.dbMap.SelectStr(ctx, "SELECT comment from blockedKeys WHERE keyHash = ?", duplicateKeySPKI)
test.AssertNotError(t, err, "Failed to get comment from database")
u, err := user.Current()
test.AssertNotError(t, err, "Failed to get current user")
Expand Down
Loading