mtpublisher: ML-DSA signatures using trees/cosignature - #8933
mtpublisher: ML-DSA signatures using trees/cosignature#8933beautifulentropy wants to merge 1 commit into
Conversation
|
@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values. |
ee37590 to
da140c6
Compare
2cc1b2e to
1048f2f
Compare
aarongable
left a comment
aarongable
left a comment
There was a problem hiding this comment.
LGTM modulo some config concerns.
| // MirrorKeyFile holds the PEM-encoded ML-DSA-44 private key used to | ||
| // cosign checkpoints. | ||
| MirrorKeyFile string `validate:"required"` |
There was a problem hiding this comment.
This feels easy to lose track of. This is a test-only stopgap until there are mirror instances out there to do real cosigning. I think this needs to be dripping in red flags -- config key name, doc string, etc -- indicating that it is test only.
Same goes for MirrorPublicKeyFile and MirrorID config keys above.
There was a problem hiding this comment.
This is will be coming out quite quickly. My next task is/was to separate the mirror from the publisher and establish an interface that makes it easy to mock out the Mirror in Publisher unit tests along with the Publisher and Mirror in MTCA unit tests. The part of this is delivered in #8934, the latter half is larger and forthcoming.
|
|
||
| // MirrorPublicKeyFile holds the PEM-encoded ML-DSA-44 public key used | ||
| // to verify cosignatures. | ||
| MirrorPublicKeyFile string `validate:"required"` |
There was a problem hiding this comment.
Is MirrorPublicKeyFile actually necessary? Since we're loading the private key, can't we just use privatekey.Public() to get the bit we need for the Verifier?
This would also allow loadMLDSAPublicKey to be dropped from this PR.
There was a problem hiding this comment.
This is actually quite necessary; as I mentioned above the private key will be coming out but cosignature verification is still essential.
| "SELECT id, mtcLogID, treeSize, mirrorSignature FROM checkpoints WHERE mtcLogID = ? ORDER BY treeSize DESC LIMIT 1", | ||
| `SELECT id, checkpoints.mtcLogID, mtcaSignature, mirrorID, | ||
| mirrorSignature, treeSize, rootHash | ||
| FROM latestCheckpoint JOIN checkpoints |
There was a problem hiding this comment.
JOINs make me nervous from a performance standpoint, but also I'm not good enough at databases to analyze whether this is well-constructed. Can you justify this construction in a comment?
There was a problem hiding this comment.
This and the query below is line for line the same query used in the MTCA today and being removed from it in #8934, a PR which depends on this change.
Given this is a simple relocation, do you feel it still warrants a comment?
3 participants
Generate the mirror key pair in
genmtpki, load the mirror key throughprivatekey.Load(), and load the mirror public key through a newloadMLDSAPublicKey(). Grant the mtpublisher userSELECTonlatestCheckpoint. The mtpublisher's checkpoint query is now identical to the mtca's and will soon replace it.Part of #8738