Add MTCLog structures and serialization by phbnf · Pull Request #1079 · transparency-dev/tessera

phbnf · 2026-07-28T08:23:44Z

Towards #945.

I'll add MTCProof when we get to it.

AlCutter · 2026-07-28T10:40:32Z

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package types


types is a bit generic, like util, and frowned upon. I'd be tempted to call this internal/entry with what's there currently - then you'd have: entry.New(), entry.ErrTooLarge, entry.Extension, etc. which I think are quite nicely readable from the point of use.

Done. Although I've removed error types. I don't thin there's anything meaningful to be done with them for now.

AlCutter · 2026-07-28T10:43:16Z

+	// "An MTCLogEntry's size SHOULD NOT exceed 65535 (2^16-1) bytes.
+	// Doing so may exceed size limits in common log-serving protocols,
+	// such as [TLOG-TILES]."
+	MaxMTCLogEntrySize = 65535 // 2^16 - 1


Just write it as the comment does - 1<<16 - 1, then the "comment" can't ever be out of date :)

AlCutter · 2026-07-28T10:45:18Z

+// MTCLogEntryExtension represents a single key-value metadata extension
+// appended to an MTCLogEntry.
+type MTCLogEntryExtension struct {
+	Type uint16 // 2-byte extension identifier


Might be worth defining type ExtensionType uint16 and type EntryType uint16 for self-doc and type-checking purposes.

AlCutter · 2026-07-28T10:48:44Z

+	b.AddUint16LengthPrefixed(func(child *cryptobyte.Builder) {
+		for i, ext := range exts {
+			if i > 0 && ext.Type == exts[i-1].Type {
+				if !bytes.Equal(ext.Data, exts[i-1].Data) {


The comment above suggests that this is an outright error, regardless of whether the the data bytes match.

I think this is working as intended since this loop also deduplicate extensions, but I might be missing something? I've added clarifying comments.

I was thinking more that it means the CA is ~broken with respect to the MTC spec (they're sending us invalid inputs), so better to return it an error so they fix it than us silently fixing it up.

There is not MTCLogEntry extension that has been defined yet, and I'm not sure where they could come from, so it's really hard to tell for now. I'm not sure these would come the CA, I was thinking they would be set by the MTC server. My goal was to write an implementation that would prevent adding a bad entry if/when someone uses extensions. I've modified the code such that it simply rejects an entry if extensions are not in order, or if there are duplicates.

Spoiler alert: in a followup PR, I'll make the MTCLogEntry fields private, thus making it impossible with the current API to set an extension. We'll add an API for this when/if we need that.I

AlCutter · 2026-07-28T10:49:51Z

+	// "An MTCLogEntry's size SHOULD NOT exceed 65535 (2^16-1) bytes.
+	// Doing so may exceed size limits in common log-serving protocols,
+	// such as [TLOG-TILES]."
+	if len(res) > MaxMTCLogEntrySize {


if l := len(res); ... then you can use l in the error too.

AlCutter · 2026-07-28T10:52:57Z

+	// SPEC: https://c2sp.org/mtc-log
+	// "MTC CAs following this profile MUST use SHA-256 as the hash algorithm".
+	// Hence, the hash size MUST be 32 bytes.
+	if len(e.SubjectPublicKeyInfoHash) != 32 {


Can use sha256.Size here rather than 32 to document.

AlCutter · 2026-07-28T11:00:33Z

-type TBSCertificateLogEntry struct{}
+// TBSCertificateLogEntry represents a log entry as per
+// draft-ietf-plants-merkle-tree-certs section 7.2.
+type TBSCertificateLogEntry struct {


Could just be TBSCertificate (it's in log so its purpose is clear)?

I can yeah, but I though it would be nice to use the exact same name that is used in the RFC. TBSCertificate is also already a bit overloaded because it's used in x509 with RawTBSCertificate. These are different packages, but I thought it would be nice to err on clarity. With that in mind do you still think I should rename this to TBSCertificate? I don't have strong preferences, I'm just explaining where I'm coming from.

Yeah, if that's the name in the RFC then I take it back - it should be the same :)

AlCutter · 2026-07-28T11:05:03Z

+
+	var b cryptobyte.Builder
+
+	if e.Version != 0 {


This is marked as EXPLICIT so presumably should always be present even if it has the value 0?

So did I think.... This was due a comment, and I forgot to put it.
Here's the matching encoding/asn1 code: https://cs.opensource.google/go/go/+/master:src/encoding/asn1/marshal.go;l=592-599

I don't know for sure what should happen here, but wrt to the code you linked Version is not optional so not clear if that branch should apply, but the spec does say it's explicit which I guess would match here instead(?): https://cs.opensource.google/go/go/+/master:src/encoding/asn1/marshal.go;l=697-712

Have you tried parsing someone else's MTC leaf and see what they do? :D

I didn't check on a live log, but I had a look at Cactus when writing this, which does the same: https://github.com/mcpherrinm/cactus/blob/5aff278c051fe89a229f4780d3d26a148c8938b7/cert/entry.go#L204-L208.

I don't know much about ASN.1, so I kept digging: optional and explicit are not mutually exclusive (?!). Actually, in crypto/x509 the Version field of a tbsCertificate is both optional and explicit.

I got my new ~~best~~ friend to a write a small code snippet that shows how crypto/asn1 deals with this: https://go.dev/play/p/uP9v7tXS1Hp.

AlCutter · 2026-07-28T11:11:09Z

+	s := cryptobyte.String(data)
+	var elem cryptobyte.String
+	if !s.ReadASN1(&elem, expectedTag) || !s.Empty() {
+		return fmt.Errorf("%s: malformed ASN.1 or incorrect tag (expected %v): %w", fieldName, expectedTag, errInvalidSequence)


%w is unusual since the error type isn't exported so can't be part of the package's public API.

I agree with you, but I never really know what to do in these situations:

I can just return an error, but then we lose test granularity since string matching in tests is also an anti-pattern

I can export errors, but I don't think anyone will / should use them

I've gone with option 1. since I think this is the most common thing to do and it reduces the API surface.
I've done the same for types (now renamed to entry) inside internal (even if it's less of a problem since they it is internal)

Add MTCLog serialization structures and validation

93725d4

phbnf requested a review from AlCutter July 28, 2026 08:23

phbnf assigned AlCutter Jul 28, 2026

phbnf marked this pull request as ready for review July 28, 2026 08:23

phbnf requested a review from a team as a code owner July 28, 2026 08:23

AlCutter reviewed Jul 28, 2026

View reviewed changes

comments

671bf29

AlCutter approved these changes Jul 28, 2026

View reviewed changes

[MTC] Require extensions to be strictly sorted and unique in MTCLogEntry

d39e24a

phbnf merged commit bcf07ff into transparency-dev:main Jul 29, 2026
20 checks passed

Uh oh!

Conversation

phbnf commented Jul 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants