Skip to content
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.

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

Avoid inserting duplicate certificates or precertificates #5468

Closed
pgporada opened this issue Jun 8, 2021 · 2 comments · Fixed by #5497
Closed

Avoid inserting duplicate certificates or precertificates #5468

pgporada opened this issue Jun 8, 2021 · 2 comments · Fixed by #5497
Assignees

Comments

@pgporada
Copy link
Member

pgporada commented Jun 8, 2021

@jcjones mentioned in #5467

It's always going to be possible now to accidentally have rows in the certificates and precertificates table now referring to the same serial number, so we need to be a bit more robust in handling it.

[boulder]> select registrationID, serial, digest, issued, expires from certificates where serial = 'fac11d1135582055be574949f41cdf603e83';
+----------------+--------------------------------------+---------------------------------------------+---------------------+---------------------+
| registrationID | serial                               | digest                                      | issued              | expires             |
+----------------+--------------------------------------+---------------------------------------------+---------------------+---------------------+
|       xxxxxxxx | fac11d1135582055be574949f41cdf603e83 | C753-HBhX1bU6wCgzbS4IZCQ50DreAdWqjnlQgj7zTU | 2021-03-17 21:48:28 | 2021-06-15 20:48:23 |
|       xxxxxxxx | fac11d1135582055be574949f41cdf603e83 | C753-HBhX1bU6wCgzbS4IZCQ50DreAdWqjnlQgj7zTU | 2021-03-17 21:48:23 | 2021-06-15 20:48:23 |
+----------------+--------------------------------------+---------------------------------------------+---------------------+---------------------+
2 rows in set (0.000 sec)
2021-03-17T21:49:21.054916+00:00 ca02 staging 6 boulder-ca[1138777]: ur73igM [AUDIT] Incorporated orphaned certificate: serial=[fac11d1135582055be574949f41cdf603e83] cert=[308 ...

Coincidently this occurred when we deployed #5311 to staging and restarted the boulder stack. Metrics showed ~1400 orphaned certificates detected as a result of the restart with ~1390 of them being adopted.
orphans

We've scanned the production DB and have not detected any duplicate certs there.

@pgporada pgporada changed the title Dupliate certificate detected in staging after a boulder restart Duplicate certificate detected in staging after a boulder restart Jun 8, 2021
@jcjones
Copy link
Contributor

jcjones commented Jun 8, 2021

Note: This could be as simple as starting a transaction and doing a SelectCertificate(map, serial) and ensuring we get Err.NoRows before continuing with orphan management and then committing the transaction.

While this could be done to all saves to Certificates, it makes sense to me to start with the orphans and, if we end up re-opening, then we could add a robust "insert certificate" procedure for all cases. Given the overhead on a hot path, I'd argue we should consider that as a real stored procedure rather than making the SA do multiple roundtrips on each issuance -- so maybe let's start simple.

@aarongable
Copy link
Contributor

JC says this is high priority. Grabbing for this milestone immediately. Short term: add transaction get-then-insert to Orphan Finder. Mid term: see if there are any other codepaths that might result in duplicate entries. Long term: if yes, consider replacing transaction with stored procedure to cover all possible inserters.

@aarongable aarongable changed the title Duplicate certificate detected in staging after a boulder restart Avoid inserting duplicate certificates or precertificates Jun 15, 2021
andygabby added a commit that referenced this issue Jun 17, 2021
Error at SA if the certificate or precertificate already exist in the
database

Fixes: #5468
andygabby added a commit that referenced this issue Jul 8, 2021
* Check for duplicate certs before adding to db

Error at SA if the certificate or precertificate already exist in the
database

Fixes: #5468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants