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

If a public key has been configured for a log, check that it is consistent with the private key #1044

Merged

Conversation

robstradling
Copy link
Contributor

@robstradling robstradling commented Mar 21, 2023

Sectigo's Sabre log was recently migrated from SuperDuper to Trillian. Due to a configuration mistake, the CTFE server signed STHs and SCTs using the wrong private key for a period of approximately 20 hours, until the problem was detected and corrected. Before and during this incident, the correct public key was specified in the public_key field in the CTFE configuration, but that field currently is not actually used by the CTFE.

This PR adds a check for the consistency of the public_key and private_key, when both of these fields are specified in the CTFE configuration.

Checklist

Comment on lines +140 to +143
case ed25519.PublicKey:
if !pub.Equal(signer.Public()) {
return nil, errors.New("public key is not consistent with private key")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, I think these should only ever really be RSA or ECDSA P-256 ('cos RFC-6962), but probably no harm in having this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about that. I couldn't find any other restriction on the key algorithm in the codebase though, and since RFC9162 permits Ed25519 I figured that future-proofing was worth these 4 lines of code.

@AlCutter
Copy link
Member

/gcbrun

@robstradling robstradling marked this pull request as ready for review March 21, 2023 21:30
@robstradling robstradling requested a review from a team as a code owner March 21, 2023 21:30
@robstradling robstradling requested review from zkpjedi and removed request for a team March 21, 2023 21:30
@roger2hk
Copy link
Contributor

/gcbrun

@AlCutter AlCutter merged commit 0f11cd0 into google:master Mar 24, 2023
@robstradling robstradling deleted the check_public_and_private_key_consistency branch April 21, 2023 18:17
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 this pull request may close these issues.

None yet

3 participants