davidben / merkle-tree-certs

Other
9 stars 4 forks source link

Hash is constrained by TrustAnchor and ProofType #79

Open bwesterb opened 1 year ago

bwesterb commented 1 year ago

Recall

struct {
    opaque issuer_id<1..32>;
    uint32 batch_number;
} MerkleTreeTrustAnchor;

struct {
    TrustAnchor trust_anchor;
    opaque proof_data<0..2^16-1>;
} Proof;

One of the Merkle tree CA parameters is hash, which is implied by the TrustAnchor. But the hash is also implied by the ProofType:

enum { merkle_tree_sha256(0), (2^16-1) } ProofType;

struct {
    ProofType proof_type;
    opaque trust_anchor_data<0..2^8-1>;
} TrustAnchor;

(As an aside: TrustAnchorType would be a reasonable alternate name for ProofType.)

I do not see the advantage of fixing the hash with the ProofType. We do write:

opaque HashValueSHA256[32];

struct {
    uint64 index;
    HashValueSHA256 path<0..2^16-1>;
} MerkleTreeProofSHA256;

But we could have just as well have written

opaque HashValue[hash.len];

struct {
    uint64 index;
    HashValue path<0..2^16-1>;
} MerkleTreeProof;

Note that we do not need to include the hash in the ProofType for negotiation. Indeed: the RP trusts certain anchors whose hashes are fixed — there is no situation where the subscriber would send a proof from a trusted anchor whose hash is not supported.

davidben commented 1 year ago

Arguably we don't need to fix anything in the ProofType. If you know the ID of the trust anchor, you know its properties. If you don't, you don't care. But, if we have a ProofType enum at all, I think capturing the hash is useful for a few reasons:

bwesterb commented 1 year ago
  • If someone writes tooling like Wireshark to lightly parse these, they have enough information to do so.

They already have: we use a length-prefix for the path. For ValidityWindow we do not, but there is no ProofType value anyway.

  • We'll probably eventually need to serialize a CA's information in some way. Then we'll need to capture the hash function. May as well incorporate it into the type enum and avoid inventing yet another enum
  • It's not just the hash function but also if we need to tweak the construction in any way, that can be rolled into it. (E.g. merkle_tree_sha256_v2)
  • Discouraging needless proliferation of variants is a good thing. The last thing we want is for a not-fit-for-purpose hash function like SHA-3 to get in here. I mean, we probably could have just called it merkle_tree and said that codepoint always means SHA-256. If you want a different hash, it's the same as if you need to change any other aspect of the construction: pick a new codepoint.

Except for your characterisation of SHA3, I completely agree. But then why not remove the "sha256" from the struct names, and say it's SHA-256 for "merkle_tree_v1". If we need to switch to anything else (and I doubt that'll happen), we can use "merkle_tree_v2".

davidben commented 1 year ago

No particular opinions on whether there's a "sha256" in the name. It's just naming at that point. I guess it's an implicit answer to people asking "what if we I want to change the hash", while still making it structurally a "codepoint captures all the parameters" kind of deal.

davidben commented 4 months ago

I'm starting to suspect ProofType will be gone after I've finished up #80, but we'll see. We can still have it if we really want, but with TrustAnchors replaced with undecorated OIDs, there won't be a natural excuse to wire it in.