Skip to content

Commit d444864

Browse files
committed
Use a unique index and duplicate detection
1 parent 880055e commit d444864

File tree

3 files changed

+20
-30
lines changed

3 files changed

+20
-30
lines changed

sa/db-next/boulder_sa/20251002000000_AddRevokedSerialsIndex.sql

Lines changed: 0 additions & 9 deletions
This file was deleted.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
-- +migrate Up
2+
-- SQL in section 'Up' is executed when this migration is applied
3+
4+
ALTER TABLE `revokedCertificates` REMOVE PARTITIONING;
5+
ALTER TABLE `revokedCertificates` ADD UNIQUE INDEX `serial` (`serial`);
6+
7+
-- +migrate Down
8+
-- SQL section 'Down' is executed when this migration is rolled back
9+
10+
ALTER TABLE `revokedCertificates` DROP UNIQUE INDEX `serial`;
11+
ALTER TABLE `revokedCertificates` PARTITION BY RANGE(id)
12+
(PARTITION p_start VALUES LESS THAN (MAXVALUE));

sa/sa.go

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -786,34 +786,15 @@ func addRevokedCertificate(ctx context.Context, tx db.Executor, req *sapb.Revoke
786786
return errors.New("cannot add revoked certificate with shard index 0")
787787
}
788788

789-
// If this serial already exists in the revokedCertificates table, then the
790-
// certificate has already been revoked. The RA has special logic for that
791-
// case, so return the specific error that the RA is looking for.
792-
var row struct {
793-
Count int64
794-
}
795-
err := tx.SelectOne(ctx, &row, "SELECT COUNT(*) as count FROM revokedCertificates WHERE serial = ? FOR UPDATE", req.Serial)
796-
if err != nil {
797-
return fmt.Errorf("checking if certificate is already revoked: %w", err)
798-
}
799-
if row.Count > 0 {
800-
return berrors.AlreadyRevokedError("serial %s already in revokedCertificates table", req.Serial)
801-
}
802-
803-
// The revokedCertificates table includes each cert's notAfter to make it easy
804-
// for crl-updater to stop including entries after they've expired, so we need
805-
// to look up that value.
806789
var serial struct {
807790
Expires time.Time
808791
}
809-
err = tx.SelectOne(ctx, &serial, `SELECT expires FROM serials WHERE serial = ?`, req.Serial)
792+
err := tx.SelectOne(
793+
ctx, &serial, `SELECT expires FROM serials WHERE serial = ?`, req.Serial)
810794
if err != nil {
811795
return fmt.Errorf("retrieving revoked certificate expiration: %w", err)
812796
}
813797

814-
// We're guaranteed that this won't be a duplicate insert because we've
815-
// already checked for existence of a row with the same serial above, using
816-
// a FOR UPDATE select in this same transaction.
817798
err = tx.Insert(ctx, &revokedCertModel{
818799
IssuerID: req.IssuerID,
819800
Serial: req.Serial,
@@ -825,6 +806,12 @@ func addRevokedCertificate(ctx context.Context, tx db.Executor, req *sapb.Revoke
825806
NotAfterHour: serial.Expires.Add(time.Hour).Truncate(time.Hour),
826807
})
827808
if err != nil {
809+
if db.IsDuplicate(err) {
810+
// An attempted duplicate insert means that this certificate was already
811+
// revoked. The RA has special logic for that case, so use the specific
812+
// error for it.
813+
return berrors.AlreadyRevokedError("certificate with serial %s already in revokedCertificates table", req.Serial)
814+
}
828815
return fmt.Errorf("inserting revoked certificate row: %w", err)
829816
}
830817

0 commit comments

Comments
 (0)