Adopt TrustAnchorIdentifiers from v03 of MTC draft
#5
Conversation
As introduced in davidben/merkle-tree-certs#87 Solves: bwesterb#3
mtc.go
Outdated
| type TrustAnchorIdentifier []byte | ||
|
|
||
| func (tai *TrustAnchorIdentifier) ProofType() ProofType { | ||
| // TODO make this dependent on the OID |
There was a problem hiding this comment.
We can't see it from the OID itself: we need some lookup. A caller will have a list of known CAs that include OID, public key, and proof type. The question is how to do this elegantly in the API. Perhaps we have a MTCContext struct, which when created can be passed with a CAStore interface that has a OID -> CAParams function.
There was a problem hiding this comment.
I've built a simple construction, such that the CA params essentially are the only entry in the 'root store'. Let me know if you would like something more elaborate, or feel free to change it yourself.
|
Thanks a lot for this meaningful review! I'll add more commits and let you know when I think it might be in a mergeable state. |
| }) | ||
|
|
||
| buf, err = c.Proof.TrustAnchorIdentifier().MarshalBinary() | ||
| b.AddBytes(buf) |
There was a problem hiding this comment.
The MarshalBinary() method of the tai does encode the OID with the TLS style length prefix. From how I understand the I-D, there is no other, outer length encoding needed. See Section 5.5.3 and mct.go:1603 for reference.
There was a problem hiding this comment.
The definition is
opaque TrustAnchorIdentifier<1..2^8-1>
As you can read from section 4.3 from RFC 5246, the angular brackets indicate a length prefixed value.
There was a problem hiding this comment.
Agreed. Maybe I'm just confusing something here, but in my mind, this is taken care of in mct.go:1603. Maybe it becomes more clear if I move this length prefix out of the TAI MarshalBinary() method?
There was a problem hiding this comment.
Oh, you're right. Now my complain becomes the opposite: there are a few places where a second length prefix is included.
Good progress. There are still a couple of issues — see my comments, but they shouldn't be too hard to fix. |
This solves #3