Add new MTCB service to build MTCs from db and tree - #8986
Conversation
| // GetStandalone requests that the MTCB return the standalone version of the | ||
| // certificate with the given serial number issued by the given MTCA. |
There was a problem hiding this comment.
| // GetStandalone requests that the MTCB return the standalone version of the | |
| // certificate with the given serial number issued by the given MTCA. | |
| // GetStandalone returns a standalone certificate. |
"requests that the MTCB return" is redundant when defining an RPC service. "Returns" is simpler.
"given serial number issued by the given MTCA" restates the parameters; better to leave those defined in the StandaloneRequest so they don't drift.
|
|
||
| message StandaloneRequest { | ||
| // Next unused field number: 3 | ||
| string mtcaID = 1; |
There was a problem hiding this comment.
In many places we have a choice of what to store and pass:
- mtcaID, serial
- mtcLogID, entryIndex
- mtcLogID, serial
FWIW in #8912 I proposed (mtcLogID, entryIndex). This PR currently uses (mtcaID, serial) for this request.
Rethinking #8912 I maybe want to settle on (mtcLogID, serial). It's slightly redundant (log number is carried in two places), but:
- Redundancy can be good, it gives us a cross-check.
- CRLs care about serials (not entryIndexes).
- Tlogs care about MTC log IDs (not CA IDs).
WDYT?
| Bucket() string | ||
| } | ||
|
|
||
| func getCAID(issuerCert *x509.Certificate) (string, error) { |
There was a problem hiding this comment.
We should factor this out from mtca, probably into the issuance package as an accessor on Issuer.
| // The serial is always a 64-bit int. However, there might be many leading | ||
| // zeroes which aren't represented in the hex string, so it needs to be | ||
| // left-padded with zeroes out to 64 bits. | ||
| var paddedSerialBytes [8]byte | ||
| copy(paddedSerialBytes[8-len(serialBytes):8], serialBytes) |
There was a problem hiding this comment.
We could probably express this a little more simply with strconv.ParseUint with a base of 16 and then some bit math based on the resulting uint64.
| b.AddASN1(asn1.TagSequence, func(b *cryptobyte.Builder) { | ||
| b.AddBytes(proof.SigAlgEncoded()) | ||
| }) |
There was a problem hiding this comment.
Since SigAlgEncoded() includes the wrapping SEQUENCE, this should just be:
| b.AddASN1(asn1.TagSequence, func(b *cryptobyte.Builder) { | |
| b.AddBytes(proof.SigAlgEncoded()) | |
| }) | |
| b.AddBytes(proof.SigAlgEncoded()) |
Also, minor refactoring: I expect that GetLandmarkRelative() will also need this code (serialize an MTCProof and put it into an RFC 5280 Certificate), so we'll want to factor it out either now or later. One argument in favor of "now": this is a nice-sized chunk to test on its own, for instance checking that the result parses.
2 participants
Note
This is very much a draft. It is based on top of #8958, should incorporate #8971 and #8968 before merging, and would benefit greatly from the work that @jsha is doing to factor out the database interface for tests and readability. Also, it doesn't have tests yet.
That said, the main-line logic is reviewable, so take a look at that if you're curious!
Fixes #8914
Fixes #8918