Skip to content

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

Merged
AlCutter merged 3 commits intogoogle:masterfrom
robstradling:check_public_and_private_key_consistency
Mar 24, 2023
Merged

If a public key has been configured for a log, check that it is consistent with the private key#1044
AlCutter merged 3 commits intogoogle:masterfrom
robstradling:check_public_and_private_key_consistency

Conversation

@robstradling
Copy link
Copy Markdown
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 thread trillian/ctfe/instance.go
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

3 participants