Skip to content

Commit 7d66d67

Browse files
authored
It's borpin' time! (#6982)
This change replaces [gorp] with [borp]. The changes consist of a mass renaming of the import and comments / doc fixups, plus modifications of many call sites to provide a context.Context everywhere, since gorp newly requires this (this was one of the motivating factors for the borp fork). This also refactors `github.com/letsencrypt/boulder/db.WrappedMap` and `github.com/letsencrypt/boulder/db.Transaction` to not embed their underlying gorp/borp objects, but to have them as plain fields. This ensures that we can only call methods on them that are specifically implemented in `github.com/letsencrypt/boulder/db`, so we don't miss wrapping any. This required introducing a `NewWrappedMap` method along with accessors `SQLDb()` and `BorpDB()` to get at the internal fields during metrics and logging setup. Fixes #6944
1 parent 0981768 commit 7d66d67

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+1210
-1840
lines changed

cmd/admin-revoker/main.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (r *revoker) revokeCertificate(ctx context.Context, certObj core.Certificat
167167
}
168168

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

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

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

375-
matches, err := r.getCertsMatchingSPKIHash(spkiHash)
375+
matches, err := r.getCertsMatchingSPKIHash(ctx, spkiHash)
376376
if err != nil {
377377
return err
378378
}
@@ -408,9 +408,9 @@ func (r *revoker) revokeByPrivateKey(ctx context.Context, privateKey string) err
408408
return nil
409409
}
410410

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

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

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

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

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

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

cmd/admin-revoker/main_test.go

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ func TestRevokeIncidentTableSerials(t *testing.T) {
9393
test.Assert(t, len(testCtx.log.GetAllMatching("No serials found in incident table")) > 0, "Expected log output not found")
9494
testCtx.log.Clear()
9595

96-
_, err = testIncidentsDbMap.Exec(
96+
ctx := context.Background()
97+
98+
_, err = testIncidentsDbMap.ExecContext(
99+
ctx,
97100
fmt.Sprintf("INSERT INTO incident_foo (%s) VALUES ('%s', %d, %d, '%s')",
98101
"serial, registrationID, orderID, lastNoticeSent",
99102
core.SerialToString(entries[0].serial),
@@ -104,14 +107,14 @@ func TestRevokeIncidentTableSerials(t *testing.T) {
104107
)
105108
test.AssertNotError(t, err, "while inserting row into incident table")
106109

107-
err = testCtx.revoker.revokeIncidentTableSerials(context.Background(), "incident_foo", 0, 1)
110+
err = testCtx.revoker.revokeIncidentTableSerials(ctx, "incident_foo", 0, 1)
108111
test.AssertNotError(t, err, "revokeIncidentTableSerials failed")
109112

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

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

149+
ctx := context.Background()
150+
146151
// Ensure that the SPKI hash hasn't already been added to the blockedKeys
147152
// table.
148-
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(spkiHash)
153+
keyExists, err := testCtx.revoker.spkiHashInBlockedKeys(ctx, spkiHash)
149154
test.AssertNotError(t, err, "countCertsMatchingSPKIHash for dupe failed")
150155
test.Assert(t, !keyExists, "SPKI hash should not be in blockedKeys")
151156

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

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

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

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

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

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

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

205210
func TestPrivateKeyBlock(t *testing.T) {
211+
ctx := context.Background()
206212
testCtx := setup(t)
207213
defer testCtx.cleanUp()
208214

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

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

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

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

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

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

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

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

270276
func TestPrivateKeyRevoke(t *testing.T) {
277+
ctx := context.Background()
271278
testCtx := setup(t)
272279
defer testCtx.cleanUp()
273280

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

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

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

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

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

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

322329
// Ensure that the comment was set as expected
323-
commentFromDB, err := testCtx.dbMap.SelectStr("SELECT comment from blockedKeys WHERE keyHash = ?", duplicateKeySPKI)
330+
commentFromDB, err := testCtx.dbMap.SelectStr(ctx, "SELECT comment from blockedKeys WHERE keyHash = ?", duplicateKeySPKI)
324331
test.AssertNotError(t, err, "Failed to get comment from database")
325332
u, err := user.Current()
326333
test.AssertNotError(t, err, "Failed to get current user")

0 commit comments

Comments
 (0)