From 93725d49d4b982403826958b6695a23c226c9bac Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Mon, 27 Jul 2026 09:07:47 +0000 Subject: [PATCH 1/3] Add MTCLog serialization structures and validation --- cmd/mtc/log/internal/types/types.go | 137 +++++++++++ cmd/mtc/log/internal/types/types_test.go | 282 ++++++++++++++++++++++ cmd/mtc/log/mtc.go | 140 ++++++++++- cmd/mtc/log/mtc_test.go | 284 +++++++++++++++++++++++ 4 files changed, 842 insertions(+), 1 deletion(-) create mode 100644 cmd/mtc/log/internal/types/types.go create mode 100644 cmd/mtc/log/internal/types/types_test.go create mode 100644 cmd/mtc/log/mtc_test.go diff --git a/cmd/mtc/log/internal/types/types.go b/cmd/mtc/log/internal/types/types.go new file mode 100644 index 000000000..da53ba997 --- /dev/null +++ b/cmd/mtc/log/internal/types/types.go @@ -0,0 +1,137 @@ +// Copyright 2026 The Tessera authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +import ( + "bytes" + "cmp" + "errors" + "fmt" + "slices" + + "golang.org/x/crypto/cryptobyte" +) + +// Standard log entry type constants from Section 5.2.1. +const ( + MTCLogEntryTypeNull uint16 = 0 + MTCLogEntryTypeTBSCert uint16 = 1 + + // SPEC: draft-ietf-plants-merkle-tree-certs section 5.2.1. + // "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 +) + +var ( + errEntryTooLarge = errors.New("log entry size exceeds tile limit") + errInvalidEntryType = errors.New("unknown or unsupported log entry type") + errMalformedExtensions = errors.New("log entry extensions list is malformed") + errMissingType = errors.New("log entry type field is missing") + errTrailingData = errors.New("unexpected trailing data in log entry") +) + +// MTCLogEntryExtension represents a single key-value metadata extension +// appended to an MTCLogEntry. +type MTCLogEntryExtension struct { + Type uint16 // 2-byte extension identifier + Data []byte // Opaque data payload +} + +// MTCLogEntry represents leaf node as defined in +// draft-ietf-plants-merkle-tree-certs section 5.2.1. +type MTCLogEntry struct { + Extensions []MTCLogEntryExtension + Type uint16 // MTCLogEntryTypeNull, MTCLogEntryTypeTBSCert + EntryData []byte // Raw DER bytes of TBSCertificateLogEntry if Type is TBSCert +} + +// Marshal encodes the MTCLogEntry into TLS Presentation bytes. +// +// Returns an error if the extensions are not specs compliant, or if the +// resulting bytes do not fit in a t-log leaf. +func (e *MTCLogEntry) Marshal() ([]byte, error) { + if e.Type == MTCLogEntryTypeNull && len(e.EntryData) > 0 { + return nil, fmt.Errorf("null entry must have empty EntryData: %w", errTrailingData) + } + // struct {} Empty; + // + // enum { (2^16-1) } MTCLogEntryExtensionType; + // + // struct { + // MTCLogEntryExtensionType extension_type; + // opaque extension_data<0..2^16-1>; + // } MTCLogEntryExtension; + // + // enum { + // null_entry(0), tbs_cert_entry(1), (2^16-1) + // } MTCLogEntryType; + // + // struct { + // MTCLogEntryExtension extensions<0..2^16-1>; + // MTCLogEntryType type; + // select (type) { + // case null_entry: Empty; + // case tbs_cert_entry: opaque tbs_cert_entry_data[N]; + // /* May be extended with future types. */ + // } + // } MTCLogEntry; + var b cryptobyte.Builder + + // SPEC: draft-ietf-plants-merkle-tree-certs section 5.2.1. + // "The extensions list MUST appear in ascending order by extension_type and + // MUST NOT contain two extensions with the same extension_type." + exts := slices.Clone(e.Extensions) + slices.SortStableFunc(exts, func(a, b MTCLogEntryExtension) int { + return cmp.Compare(a.Type, b.Type) + }) + + // Creates a 16-bit (2-byte) length-prefixed block for the list of extensions. + 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) { + child.SetError(fmt.Errorf("conflicting duplicate extension type %d with differing data: %w", ext.Type, errMalformedExtensions)) + return + } + continue + } + child.AddUint16(ext.Type) + // Each extension data block has its own 16-bit length prefix. + child.AddUint16LengthPrefixed(func(dataBlock *cryptobyte.Builder) { + dataBlock.AddBytes(ext.Data) + }) + } + }) + + b.AddUint16(e.Type) + + // EntryData fills up the rest of the structure, no size prefix needed. + b.AddBytes(e.EntryData) + + res, err := b.Bytes() + if err != nil { + return nil, err + } + // SPEC: draft-ietf-plants-merkle-tree-certs section 5.2.1. + // "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 { + return nil, fmt.Errorf("log entry size %d exceeds tile limit %d: %w", len(res), MaxMTCLogEntrySize, errEntryTooLarge) + } + return res, nil +} diff --git a/cmd/mtc/log/internal/types/types_test.go b/cmd/mtc/log/internal/types/types_test.go new file mode 100644 index 000000000..db5d7c263 --- /dev/null +++ b/cmd/mtc/log/internal/types/types_test.go @@ -0,0 +1,282 @@ +// Copyright 2026 The Tessera authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +import ( + "bytes" + "cmp" + "errors" + "fmt" + "slices" + "testing" + + "golang.org/x/crypto/cryptobyte" +) + + +// unmarshal decodes a raw TLS presentation byte stream into an MTCLogEntry structure. +// This is kept in the test suite for verifying round-trip serialization of MTCLogEntry. +func (e *MTCLogEntry) unmarshal(data []byte) error { + s := cryptobyte.String(data) + + var extListStr cryptobyte.String + if !s.ReadUint16LengthPrefixed(&extListStr) { + return errMalformedExtensions + } + + e.Extensions = nil + for !extListStr.Empty() { + var ext MTCLogEntryExtension + if !extListStr.ReadUint16(&ext.Type) { + return fmt.Errorf("failed to read extension type: %w", errMalformedExtensions) + } + var extDataStr cryptobyte.String + if !extListStr.ReadUint16LengthPrefixed(&extDataStr) { + return fmt.Errorf("failed to read extension length: %w", errMalformedExtensions) + } + ext.Data = append([]byte(nil), extDataStr...) + if n := len(e.Extensions); n > 0 { + if ext.Type < e.Extensions[n-1].Type { + return fmt.Errorf("mtc: entry extensions out of order (type %d after %d): %w", ext.Type, e.Extensions[n-1].Type, errMalformedExtensions) + } + if ext.Type == e.Extensions[n-1].Type { + return fmt.Errorf("mtc: duplicate entry extension type %d: %w", ext.Type, errMalformedExtensions) + } + } + e.Extensions = append(e.Extensions, ext) + } + + if !s.ReadUint16(&e.Type) { + return errMissingType + } + + if e.Type != MTCLogEntryTypeNull && e.Type != MTCLogEntryTypeTBSCert { + return fmt.Errorf("%w: type %d", errInvalidEntryType, e.Type) + } + + if e.Type == MTCLogEntryTypeNull && !s.Empty() { + return fmt.Errorf("null entry must have empty data: %w", errTrailingData) + } + + e.EntryData = append([]byte(nil), s...) + return nil +} + +func TestMTCLogEntry_RoundTrip(t *testing.T) { + tests := []struct { + name string + entry MTCLogEntry + }{ + { + name: "null entry no extensions", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeNull, + }, + }, + { + name: "tbs cert entry with sorted extensions", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + EntryData: []byte("fake-der-octets"), + Extensions: []MTCLogEntryExtension{ + {Type: 1, Data: []byte("ext-1-data")}, + {Type: 5, Data: []byte("ext-5-data")}, + {Type: 10, Data: []byte("")}, + }, + }, + }, + { + name: "tbs cert entry with unsorted extensions", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + EntryData: []byte("fake-der-octets"), + Extensions: []MTCLogEntryExtension{ + {Type: 10, Data: []byte("ext-10-data")}, + {Type: 1, Data: []byte("ext-1-data")}, + {Type: 5, Data: []byte("ext-5-data")}, + }, + }, + }, + { + name: "tbs cert entry with identical duplicate extensions", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + EntryData: []byte("fake-der-octets"), + Extensions: []MTCLogEntryExtension{ + {Type: 5, Data: []byte("ext-5-data")}, + {Type: 1, Data: []byte("ext-1-data")}, + {Type: 5, Data: []byte("ext-5-data")}, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + data, err := tc.entry.Marshal() + if err != nil { + t.Fatalf("Marshal() unexpected error: %v", err) + } + + var got MTCLogEntry + if err := got.unmarshal(data); err != nil { + t.Fatalf("unmarshal() unexpected error: %v", err) + } + + if got.Type != tc.entry.Type { + t.Errorf("Type = %d, want %d", got.Type, tc.entry.Type) + } + if !bytes.Equal(got.EntryData, tc.entry.EntryData) { + t.Errorf("EntryData = %x, want %x", got.EntryData, tc.entry.EntryData) + } + + wantExts := slices.Clone(tc.entry.Extensions) + slices.SortStableFunc(wantExts, func(a, b MTCLogEntryExtension) int { + return cmp.Compare(a.Type, b.Type) + }) + wantExts = slices.CompactFunc(wantExts, func(a, b MTCLogEntryExtension) bool { + return a.Type == b.Type && bytes.Equal(a.Data, b.Data) + }) + if len(got.Extensions) != len(wantExts) { + t.Fatalf("len(Extensions) = %d, want %d", len(got.Extensions), len(wantExts)) + } + for i := range got.Extensions { + if got.Extensions[i].Type != wantExts[i].Type || !bytes.Equal(got.Extensions[i].Data, wantExts[i].Data) { + t.Errorf("Extension[%d] = %+v, want %+v", i, got.Extensions[i], wantExts[i]) + } + } + }) + } +} + +func TestMTCLogEntry_MarshalErrors(t *testing.T) { + tests := []struct { + name string + entry MTCLogEntry + wantErr error + }{ + { + name: "trailing data on null entry", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeNull, + EntryData: []byte("unexpected-data"), + }, + wantErr: errTrailingData, + }, + { + name: "conflicting duplicate extensions with different data", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + Extensions: []MTCLogEntryExtension{ + {Type: 2, Data: []byte("data-a")}, + {Type: 2, Data: []byte("data-b")}, + }, + }, + wantErr: errMalformedExtensions, + }, + { + name: "entry size exceeds tile limit", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + EntryData: make([]byte, MaxMTCLogEntrySize), + }, + wantErr: errEntryTooLarge, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := tc.entry.Marshal() + if !errors.Is(err, tc.wantErr) { + t.Errorf("Marshal() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} + +func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { + tests := []struct { + name string + mutate func([]byte) []byte + wantErr error + }{ + { + name: "truncated extension list length prefix", + mutate: func(b []byte) []byte { + return []byte{0x00, 0x05, 0x01} + }, + wantErr: errMalformedExtensions, + }, + { + name: "trailing data on null entry", + mutate: func(b []byte) []byte { + e := MTCLogEntry{Type: MTCLogEntryTypeNull} + data, _ := e.Marshal() + return append(data, 0x00) + }, + wantErr: errTrailingData, + }, + { + name: "unknown entry type", + mutate: func(b []byte) []byte { + var builder cryptobyte.Builder + builder.AddUint16(0) // 0 length extensions + builder.AddUint16(99) // invalid type 99 + return builder.BytesOrPanic() + }, + wantErr: errInvalidEntryType, + }, + { + name: "unsorted extensions over the wire", + mutate: func(b []byte) []byte { + var builder cryptobyte.Builder + var extList cryptobyte.Builder + extList.AddUint16(5) + extList.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte("a")) }) + extList.AddUint16(1) + extList.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte("b")) }) + builder.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes(extList.BytesOrPanic()) }) + builder.AddUint16(MTCLogEntryTypeTBSCert) + return builder.BytesOrPanic() + }, + wantErr: errMalformedExtensions, + }, + { + name: "duplicate extensions over the wire", + mutate: func(b []byte) []byte { + var builder cryptobyte.Builder + var extList cryptobyte.Builder + extList.AddUint16(2) + extList.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte("a")) }) + extList.AddUint16(2) + extList.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte("b")) }) + builder.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes(extList.BytesOrPanic()) }) + builder.AddUint16(MTCLogEntryTypeTBSCert) + return builder.BytesOrPanic() + }, + wantErr: errMalformedExtensions, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var entry MTCLogEntry + err := entry.unmarshal(tc.mutate(nil)) + if !errors.Is(err, tc.wantErr) { + t.Errorf("unmarshal() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} diff --git a/cmd/mtc/log/mtc.go b/cmd/mtc/log/mtc.go index aad56743e..f84c1bfd6 100644 --- a/cmd/mtc/log/mtc.go +++ b/cmd/mtc/log/mtc.go @@ -16,17 +16,155 @@ package log import ( "context" + "errors" + "fmt" "github.com/transparency-dev/tessera" + "golang.org/x/crypto/cryptobyte" + "golang.org/x/crypto/cryptobyte/asn1" ) type MTCLog struct { a *tessera.Appender } +// MTCProof represents an MTC inclusion proof as per +// draft-ietf-plants-merkle-tree-certs section 6.2. type MTCProof struct{} -type TBSCertificateLogEntry struct{} +// TBSCertificateLogEntry represents a log entry as per +// draft-ietf-plants-merkle-tree-certs section 7.2. +type TBSCertificateLogEntry struct { + Version int64 `json:"version"` // 0 = v1 (default), tag 0 + Issuer []byte `json:"issuer"` // Raw DER-encoded SEQUENCE + Validity []byte `json:"validity"` // Raw DER-encoded SEQUENCE + Subject []byte `json:"subject"` // Raw DER-encoded SEQUENCE + SubjectPublicKeyAlgorithm []byte `json:"subjectPublicKeyAlgorithm"` // Raw DER-encoded SEQUENCE + SubjectPublicKeyInfoHash []byte `json:"subjectPublicKeyInfoHash"` // Raw 32-byte public key hash + IssuerUniqueID []byte `json:"issuerUniqueId,omitempty"` // Optional raw IMPLICIT BIT STRING bytes, tag 1 + SubjectUniqueID []byte `json:"subjectUniqueId,omitempty"` // Optional raw IMPLICIT BIT STRING bytes, tag 2 + Extensions []byte `json:"extensions,omitempty"` // Optional raw EXPLICIT SEQUENCE bytes, tag 3 +} + +var ( + errInvalidHashSize = errors.New("subjectPublicKeyInfoHash must be exactly 32 bytes") + errInvalidSequence = errors.New("invalid ASN.1 SEQUENCE structure") + errMalformedVersion = errors.New("version field is malformed") + errMissingField = errors.New("mandatory field is missing") +) + +// Marshal returns the contents octets of the TBSCertificateLogEntry's +// DER encoding, i.e. WITHOUT the outer SEQUENCE tag and length prefix. +// +// SPEC: draft-ietf-plants-merkle-tree-certs section 5.2.1. +// "tbs_cert_entry_data contains the contents octets (i.e. excluding the initial +// identifier and length octets) of the DER" +func (e *TBSCertificateLogEntry) Marshal() ([]byte, error) { + // TBSCertificateLogEntry ::= SEQUENCE { + // version [0] EXPLICIT Version DEFAULT v1, + // issuer Name, + // validity Validity, + // subject Name, + // subjectPublicKeyAlgorithm AlgorithmIdentifier{PUBLIC-KEY, + // {PublicKeyAlgorithms}}, + // subjectPublicKeyInfoHash OCTET STRING, + // issuerUniqueID [1] IMPLICIT UniqueIdentifier OPTIONAL, + // subjectUniqueID [2] IMPLICIT UniqueIdentifier OPTIONAL, + // extensions [3] EXPLICIT Extensions{{CertExtensions}} + // OPTIONAL + // } + if err := e.Validate(); err != nil { + return nil, err + } + + var b cryptobyte.Builder + + if e.Version != 0 { + b.AddASN1(asn1.Tag(0).ContextSpecific().Constructed(), func(b *cryptobyte.Builder) { + b.AddASN1Int64(e.Version) + }) + } + + b.AddBytes(e.Issuer) + b.AddBytes(e.Validity) + b.AddBytes(e.Subject) + b.AddBytes(e.SubjectPublicKeyAlgorithm) + b.AddASN1OctetString(e.SubjectPublicKeyInfoHash) + + if len(e.IssuerUniqueID) > 0 { + b.AddBytes(e.IssuerUniqueID) + } + if len(e.SubjectUniqueID) > 0 { + b.AddBytes(e.SubjectUniqueID) + } + if len(e.Extensions) > 0 { + b.AddBytes(e.Extensions) + } + + return b.Bytes() +} + +func validateASN1Element(data []byte, expectedTag asn1.Tag, fieldName string) error { + if len(data) == 0 { + return nil + } + 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) + } + return nil +} + +// Validate checks that TBSCertificateLogEntry fields are correct. +// It checks that mandatory fields are present, and that all fields are well +// formatted. +func (e *TBSCertificateLogEntry) Validate() error { + if e.Version < 0 || e.Version > 2 { + return fmt.Errorf("invalid version %d: %w", e.Version, errMalformedVersion) + } + + switch { + case len(e.Issuer) == 0: + return fmt.Errorf("issuer: %w", errMissingField) + case len(e.Validity) == 0: + return fmt.Errorf("validity: %w", errMissingField) + case len(e.Subject) == 0: + return fmt.Errorf("subject: %w", errMissingField) + case len(e.SubjectPublicKeyAlgorithm) == 0: + return fmt.Errorf("subjectPublicKeyAlgorithm: %w", errMissingField) + } + + if err := validateASN1Element(e.Issuer, asn1.SEQUENCE, "issuer"); err != nil { + return err + } + if err := validateASN1Element(e.Validity, asn1.SEQUENCE, "validity"); err != nil { + return err + } + if err := validateASN1Element(e.Subject, asn1.SEQUENCE, "subject"); err != nil { + return err + } + if err := validateASN1Element(e.SubjectPublicKeyAlgorithm, asn1.SEQUENCE, "subjectPublicKeyAlgorithm"); err != nil { + return err + } + if err := validateASN1Element(e.IssuerUniqueID, asn1.Tag(1).ContextSpecific(), "issuerUniqueID"); err != nil { + return err + } + if err := validateASN1Element(e.SubjectUniqueID, asn1.Tag(2).ContextSpecific(), "subjectUniqueID"); err != nil { + return err + } + if err := validateASN1Element(e.Extensions, asn1.Tag(3).ContextSpecific().Constructed(), "extensions"); err != nil { + return err + } + + // 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 { + return fmt.Errorf("subjectPublicKeyInfoHash must be 32 bytes, got %d: %w", len(e.SubjectPublicKeyInfoHash), errInvalidHashSize) + } + return nil +} // NewMTCLog creates a new MTCLog compliant with // draft-ietf-plants-merkle-tree-certs and http://c2sp.org/mtc-tlog. diff --git a/cmd/mtc/log/mtc_test.go b/cmd/mtc/log/mtc_test.go new file mode 100644 index 000000000..11de6869d --- /dev/null +++ b/cmd/mtc/log/mtc_test.go @@ -0,0 +1,284 @@ +// Copyright 2026 The Tessera authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import ( + "bytes" + "errors" + "fmt" + "testing" + + "golang.org/x/crypto/cryptobyte" + "golang.org/x/crypto/cryptobyte/asn1" +) + + +// unmarshal decodes the contents octets of a TBSCertificateLogEntry +// (i.e. WITHOUT the outer SEQUENCE tag and length prefix) into the struct. +// This is kept in the test suite for verifying round-trip serialization of TBSCertificateLogEntry. +func (e *TBSCertificateLogEntry) unmarshal(contents []byte) error { + seq := cryptobyte.String(contents) + + e.Version = 0 + versionTag := asn1.Tag(0).ContextSpecific().Constructed() + if seq.PeekASN1Tag(versionTag) { + var verWrapped cryptobyte.String + if !seq.ReadASN1(&verWrapped, versionTag) { + return fmt.Errorf("failed to read version wrapper: %w", errInvalidSequence) + } + if !verWrapped.ReadASN1Integer(&e.Version) || !verWrapped.Empty() { + return errMalformedVersion + } + } + + readElement := func(target *[]byte, tag asn1.Tag, fieldName string) error { + var rawElem cryptobyte.String + if !seq.ReadASN1Element(&rawElem, tag) { + return fmt.Errorf("failed to read %s: %w", fieldName, errInvalidSequence) + } + *target = append([]byte(nil), rawElem...) + return nil + } + + if err := readElement(&e.Issuer, asn1.SEQUENCE, "issuer"); err != nil { + return err + } + if err := readElement(&e.Validity, asn1.SEQUENCE, "validity"); err != nil { + return err + } + if err := readElement(&e.Subject, asn1.SEQUENCE, "subject"); err != nil { + return err + } + if err := readElement(&e.SubjectPublicKeyAlgorithm, asn1.SEQUENCE, "subjectPublicKeyAlgorithm"); err != nil { + return err + } + var spkiHash cryptobyte.String + if !seq.ReadASN1(&spkiHash, asn1.OCTET_STRING) { + return fmt.Errorf("failed to read subjectPublicKeyInfoHash: %w", errInvalidSequence) + } + e.SubjectPublicKeyInfoHash = append([]byte(nil), spkiHash...) + + tag1 := asn1.Tag(1).ContextSpecific() + if seq.PeekASN1Tag(tag1) { + if err := readElement(&e.IssuerUniqueID, tag1, "issuerUniqueID"); err != nil { + return err + } + } + + tag2 := asn1.Tag(2).ContextSpecific() + if seq.PeekASN1Tag(tag2) { + if err := readElement(&e.SubjectUniqueID, tag2, "subjectUniqueID"); err != nil { + return err + } + } + + tag3 := asn1.Tag(3).ContextSpecific().Constructed() + if seq.PeekASN1Tag(tag3) { + if err := readElement(&e.Extensions, tag3, "extensions"); err != nil { + return err + } + } + + if !seq.Empty() { + return fmt.Errorf("sequence has trailing bytes: %w", errInvalidSequence) + } + return nil +} + +func dummySeq(data string) []byte { + var b cryptobyte.Builder + b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) { + b.AddBytes([]byte(data)) + }) + return b.BytesOrPanic() +} + +func dummyTag(tag asn1.Tag, data string) []byte { + var b cryptobyte.Builder + b.AddASN1(tag, func(b *cryptobyte.Builder) { + b.AddBytes([]byte(data)) + }) + return b.BytesOrPanic() +} + +func TestTBSCertificateLogEntry_RoundTrip(t *testing.T) { + tests := []struct { + name string + entry TBSCertificateLogEntry + }{ + { + name: "default version 0 minimal fields", + entry: TBSCertificateLogEntry{ + Version: 0, + Issuer: dummySeq("issuer-der-seq"), + Validity: dummySeq("validity-der-seq"), + Subject: dummySeq("subject-der-seq"), + SubjectPublicKeyAlgorithm: dummySeq("spki-algo-der-seq"), + SubjectPublicKeyInfoHash: make([]byte, 32), + }, + }, + { + name: "version 2 with optional unique IDs and extensions", + entry: TBSCertificateLogEntry{ + Version: 2, + Issuer: dummySeq("issuer-der-seq"), + Validity: dummySeq("validity-der-seq"), + Subject: dummySeq("subject-der-seq"), + SubjectPublicKeyAlgorithm: dummySeq("spki-algo-der-seq"), + SubjectPublicKeyInfoHash: bytes.Repeat([]byte{0x42}, 32), + IssuerUniqueID: dummyTag(asn1.Tag(1).ContextSpecific(), "uid-1"), + SubjectUniqueID: dummyTag(asn1.Tag(2).ContextSpecific(), "uid-2"), + Extensions: dummyTag(asn1.Tag(3).ContextSpecific().Constructed(), "exts"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + data, err := tc.entry.Marshal() + if err != nil { + t.Fatalf("Marshal() unexpected error: %v", err) + } + + var got TBSCertificateLogEntry + if err := got.unmarshal(data); err != nil { + t.Fatalf("Unmarshal() unexpected error: %v", err) + } + + if got.Version != tc.entry.Version { + t.Errorf("Version = %d, want %d", got.Version, tc.entry.Version) + } + if !bytes.Equal(got.Issuer, tc.entry.Issuer) { + t.Errorf("Issuer = %x, want %x", got.Issuer, tc.entry.Issuer) + } + if !bytes.Equal(got.Validity, tc.entry.Validity) { + t.Errorf("Validity = %x, want %x", got.Validity, tc.entry.Validity) + } + if !bytes.Equal(got.Subject, tc.entry.Subject) { + t.Errorf("Subject = %x, want %x", got.Subject, tc.entry.Subject) + } + if !bytes.Equal(got.SubjectPublicKeyAlgorithm, tc.entry.SubjectPublicKeyAlgorithm) { + t.Errorf("SubjectPublicKeyAlgorithm = %x, want %x", got.SubjectPublicKeyAlgorithm, tc.entry.SubjectPublicKeyAlgorithm) + } + if !bytes.Equal(got.SubjectPublicKeyInfoHash, tc.entry.SubjectPublicKeyInfoHash) { + t.Errorf("SubjectPublicKeyInfoHash = %x, want %x", got.SubjectPublicKeyInfoHash, tc.entry.SubjectPublicKeyInfoHash) + } + if !bytes.Equal(got.IssuerUniqueID, tc.entry.IssuerUniqueID) { + t.Errorf("IssuerUniqueID = %x, want %x", got.IssuerUniqueID, tc.entry.IssuerUniqueID) + } + if !bytes.Equal(got.SubjectUniqueID, tc.entry.SubjectUniqueID) { + t.Errorf("SubjectUniqueID = %x, want %x", got.SubjectUniqueID, tc.entry.SubjectUniqueID) + } + if !bytes.Equal(got.Extensions, tc.entry.Extensions) { + t.Errorf("Extensions = %x, want %x", got.Extensions, tc.entry.Extensions) + } + }) + } +} + +func TestTBSCertificateLogEntry_Validate(t *testing.T) { + valid := func() TBSCertificateLogEntry { + return TBSCertificateLogEntry{ + Issuer: dummySeq("issuer"), + Validity: dummySeq("validity"), + Subject: dummySeq("subject"), + SubjectPublicKeyAlgorithm: dummySeq("algo"), + SubjectPublicKeyInfoHash: make([]byte, 32), + } + } + + tests := []struct { + name string + mutate func(*TBSCertificateLogEntry) + wantErr error + }{ + { + name: "valid", + mutate: func(e *TBSCertificateLogEntry) {}, + wantErr: nil, + }, + { + name: "negative version", + mutate: func(e *TBSCertificateLogEntry) { e.Version = -1 }, + wantErr: errMalformedVersion, + }, + { + name: "version out of bounds", + mutate: func(e *TBSCertificateLogEntry) { e.Version = 3 }, + wantErr: errMalformedVersion, + }, + { + name: "malformed issuer asn1", + mutate: func(e *TBSCertificateLogEntry) { e.Issuer = []byte("not-asn1-sequence") }, + wantErr: errInvalidSequence, + }, + { + name: "trailing data on issuer", + mutate: func(e *TBSCertificateLogEntry) { e.Issuer = append(dummySeq("issuer"), 0x00) }, + wantErr: errInvalidSequence, + }, + { + name: "wrong tag on issuerUniqueID", + mutate: func(e *TBSCertificateLogEntry) { e.IssuerUniqueID = dummyTag(asn1.Tag(5).ContextSpecific(), "uid") }, + wantErr: errInvalidSequence, + }, + { + name: "wrong tag on extensions (primitive instead of constructed)", + mutate: func(e *TBSCertificateLogEntry) { e.Extensions = dummyTag(asn1.Tag(3).ContextSpecific(), "exts") }, + wantErr: errInvalidSequence, + }, + { + name: "missing issuer", + mutate: func(e *TBSCertificateLogEntry) { e.Issuer = nil }, + wantErr: errMissingField, + }, + { + name: "missing validity", + mutate: func(e *TBSCertificateLogEntry) { e.Validity = nil }, + wantErr: errMissingField, + }, + { + name: "missing subject", + mutate: func(e *TBSCertificateLogEntry) { e.Subject = nil }, + wantErr: errMissingField, + }, + { + name: "missing spki algo", + mutate: func(e *TBSCertificateLogEntry) { e.SubjectPublicKeyAlgorithm = nil }, + wantErr: errMissingField, + }, + { + name: "invalid hash size (short)", + mutate: func(e *TBSCertificateLogEntry) { e.SubjectPublicKeyInfoHash = make([]byte, 10) }, + wantErr: errInvalidHashSize, + }, + { + name: "invalid hash size (long)", + mutate: func(e *TBSCertificateLogEntry) { e.SubjectPublicKeyInfoHash = make([]byte, 33) }, + wantErr: errInvalidHashSize, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + e := valid() + tc.mutate(&e) + err := e.Validate() + if !errors.Is(err, tc.wantErr) { + t.Errorf("Validate() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} From 671bf29d6188d599dc0546dc28a6a1b9b0799ce9 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Tue, 28 Jul 2026 12:30:05 +0000 Subject: [PATCH 2/3] comments --- .../{types/types.go => entry/entry.go} | 46 ++++++++--------- .../types_test.go => entry/entry_test.go} | 50 +++++++++---------- cmd/mtc/log/mtc.go | 33 ++++++------ cmd/mtc/log/mtc_test.go | 41 ++++++++------- 4 files changed, 86 insertions(+), 84 deletions(-) rename cmd/mtc/log/internal/{types/types.go => entry/entry.go} (75%) rename cmd/mtc/log/internal/{types/types_test.go => entry/entry_test.go} (86%) diff --git a/cmd/mtc/log/internal/types/types.go b/cmd/mtc/log/internal/entry/entry.go similarity index 75% rename from cmd/mtc/log/internal/types/types.go rename to cmd/mtc/log/internal/entry/entry.go index da53ba997..1da98c495 100644 --- a/cmd/mtc/log/internal/types/types.go +++ b/cmd/mtc/log/internal/entry/entry.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package types +package entry import ( "bytes" @@ -26,37 +26,35 @@ import ( // Standard log entry type constants from Section 5.2.1. const ( - MTCLogEntryTypeNull uint16 = 0 - MTCLogEntryTypeTBSCert uint16 = 1 + MTCLogEntryTypeNull EntryType = 0 + MTCLogEntryTypeTBSCert EntryType = 1 // SPEC: draft-ietf-plants-merkle-tree-certs section 5.2.1. // "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 -) - -var ( - errEntryTooLarge = errors.New("log entry size exceeds tile limit") - errInvalidEntryType = errors.New("unknown or unsupported log entry type") - errMalformedExtensions = errors.New("log entry extensions list is malformed") - errMissingType = errors.New("log entry type field is missing") - errTrailingData = errors.New("unexpected trailing data in log entry") + MaxMTCLogEntrySize = 1<<16 - 1 ) // MTCLogEntryExtension represents a single key-value metadata extension // appended to an MTCLogEntry. type MTCLogEntryExtension struct { - Type uint16 // 2-byte extension identifier - Data []byte // Opaque data payload + Type ExtensionType // 2-byte extension identifier + Data []byte // Opaque data payload } +// ExtensionType represents the Type field of an MTCLogentryExtension. +type ExtensionType uint16 + +// EntryType represents the Type field of an MTCLogentry. +type EntryType uint16 + // MTCLogEntry represents leaf node as defined in // draft-ietf-plants-merkle-tree-certs section 5.2.1. type MTCLogEntry struct { Extensions []MTCLogEntryExtension - Type uint16 // MTCLogEntryTypeNull, MTCLogEntryTypeTBSCert - EntryData []byte // Raw DER bytes of TBSCertificateLogEntry if Type is TBSCert + Type EntryType // MTCLogEntryTypeNull, MTCLogEntryTypeTBSCert + EntryData []byte // Raw DER bytes of TBSCertificateLogEntry if Type is TBSCert } // Marshal encodes the MTCLogEntry into TLS Presentation bytes. @@ -65,7 +63,7 @@ type MTCLogEntry struct { // resulting bytes do not fit in a t-log leaf. func (e *MTCLogEntry) Marshal() ([]byte, error) { if e.Type == MTCLogEntryTypeNull && len(e.EntryData) > 0 { - return nil, fmt.Errorf("null entry must have empty EntryData: %w", errTrailingData) + return nil, errors.New("null entry must have empty EntryData") } // struct {} Empty; // @@ -95,21 +93,23 @@ func (e *MTCLogEntry) Marshal() ([]byte, error) { // "The extensions list MUST appear in ascending order by extension_type and // MUST NOT contain two extensions with the same extension_type." exts := slices.Clone(e.Extensions) + // First, sort extensions. slices.SortStableFunc(exts, func(a, b MTCLogEntryExtension) int { return cmp.Compare(a.Type, b.Type) }) - // Creates a 16-bit (2-byte) length-prefixed block for the list of extensions. + // Then, deduplicate extensions, unless duplicate extension + // tags have different data. 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) { - child.SetError(fmt.Errorf("conflicting duplicate extension type %d with differing data: %w", ext.Type, errMalformedExtensions)) + child.SetError(fmt.Errorf("conflicting duplicate extension type %d with differing data", ext.Type)) return } continue } - child.AddUint16(ext.Type) + child.AddUint16(uint16(ext.Type)) // Each extension data block has its own 16-bit length prefix. child.AddUint16LengthPrefixed(func(dataBlock *cryptobyte.Builder) { dataBlock.AddBytes(ext.Data) @@ -117,7 +117,7 @@ func (e *MTCLogEntry) Marshal() ([]byte, error) { } }) - b.AddUint16(e.Type) + b.AddUint16(uint16(e.Type)) // EntryData fills up the rest of the structure, no size prefix needed. b.AddBytes(e.EntryData) @@ -130,8 +130,8 @@ func (e *MTCLogEntry) Marshal() ([]byte, error) { // "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 { - return nil, fmt.Errorf("log entry size %d exceeds tile limit %d: %w", len(res), MaxMTCLogEntrySize, errEntryTooLarge) + if l := len(res); l > MaxMTCLogEntrySize { + return nil, fmt.Errorf("log entry size %d exceeds tile limit %d", l, MaxMTCLogEntrySize) } return res, nil } diff --git a/cmd/mtc/log/internal/types/types_test.go b/cmd/mtc/log/internal/entry/entry_test.go similarity index 86% rename from cmd/mtc/log/internal/types/types_test.go rename to cmd/mtc/log/internal/entry/entry_test.go index db5d7c263..3f6c1ccfb 100644 --- a/cmd/mtc/log/internal/types/types_test.go +++ b/cmd/mtc/log/internal/entry/entry_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package types +package entry import ( "bytes" @@ -33,41 +33,41 @@ func (e *MTCLogEntry) unmarshal(data []byte) error { var extListStr cryptobyte.String if !s.ReadUint16LengthPrefixed(&extListStr) { - return errMalformedExtensions + return errors.New("malformed extension list length") } e.Extensions = nil for !extListStr.Empty() { var ext MTCLogEntryExtension - if !extListStr.ReadUint16(&ext.Type) { - return fmt.Errorf("failed to read extension type: %w", errMalformedExtensions) + if !extListStr.ReadUint16((*uint16)(&ext.Type)) { + return fmt.Errorf("failed to read extension type") } var extDataStr cryptobyte.String if !extListStr.ReadUint16LengthPrefixed(&extDataStr) { - return fmt.Errorf("failed to read extension length: %w", errMalformedExtensions) + return fmt.Errorf("failed to read extension length") } ext.Data = append([]byte(nil), extDataStr...) if n := len(e.Extensions); n > 0 { if ext.Type < e.Extensions[n-1].Type { - return fmt.Errorf("mtc: entry extensions out of order (type %d after %d): %w", ext.Type, e.Extensions[n-1].Type, errMalformedExtensions) + return fmt.Errorf("mtc: entry extensions out of order (type %d after %d)", ext.Type, e.Extensions[n-1].Type) } if ext.Type == e.Extensions[n-1].Type { - return fmt.Errorf("mtc: duplicate entry extension type %d: %w", ext.Type, errMalformedExtensions) + return fmt.Errorf("mtc: duplicate entry extension type %d", ext.Type) } } e.Extensions = append(e.Extensions, ext) } - if !s.ReadUint16(&e.Type) { - return errMissingType + if !s.ReadUint16((*uint16)(&e.Type)) { + return errors.New("missing entry type") } if e.Type != MTCLogEntryTypeNull && e.Type != MTCLogEntryTypeTBSCert { - return fmt.Errorf("%w: type %d", errInvalidEntryType, e.Type) + return fmt.Errorf("unknown or unsupported log entry type %d", e.Type) } if e.Type == MTCLogEntryTypeNull && !s.Empty() { - return fmt.Errorf("null entry must have empty data: %w", errTrailingData) + return fmt.Errorf("null entry must have empty data") } e.EntryData = append([]byte(nil), s...) @@ -165,7 +165,7 @@ func TestMTCLogEntry_MarshalErrors(t *testing.T) { tests := []struct { name string entry MTCLogEntry - wantErr error + wantErr bool }{ { name: "trailing data on null entry", @@ -173,7 +173,7 @@ func TestMTCLogEntry_MarshalErrors(t *testing.T) { Type: MTCLogEntryTypeNull, EntryData: []byte("unexpected-data"), }, - wantErr: errTrailingData, + wantErr: true, }, { name: "conflicting duplicate extensions with different data", @@ -184,7 +184,7 @@ func TestMTCLogEntry_MarshalErrors(t *testing.T) { {Type: 2, Data: []byte("data-b")}, }, }, - wantErr: errMalformedExtensions, + wantErr: true, }, { name: "entry size exceeds tile limit", @@ -192,14 +192,14 @@ func TestMTCLogEntry_MarshalErrors(t *testing.T) { Type: MTCLogEntryTypeTBSCert, EntryData: make([]byte, MaxMTCLogEntrySize), }, - wantErr: errEntryTooLarge, + wantErr: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { _, err := tc.entry.Marshal() - if !errors.Is(err, tc.wantErr) { + if (err != nil) != tc.wantErr { t.Errorf("Marshal() error = %v, wantErr %v", err, tc.wantErr) } }) @@ -210,14 +210,14 @@ func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { tests := []struct { name string mutate func([]byte) []byte - wantErr error + wantErr bool }{ { name: "truncated extension list length prefix", mutate: func(b []byte) []byte { return []byte{0x00, 0x05, 0x01} }, - wantErr: errMalformedExtensions, + wantErr: true, }, { name: "trailing data on null entry", @@ -226,7 +226,7 @@ func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { data, _ := e.Marshal() return append(data, 0x00) }, - wantErr: errTrailingData, + wantErr: true, }, { name: "unknown entry type", @@ -236,7 +236,7 @@ func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { builder.AddUint16(99) // invalid type 99 return builder.BytesOrPanic() }, - wantErr: errInvalidEntryType, + wantErr: true, }, { name: "unsorted extensions over the wire", @@ -248,10 +248,10 @@ func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { extList.AddUint16(1) extList.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte("b")) }) builder.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes(extList.BytesOrPanic()) }) - builder.AddUint16(MTCLogEntryTypeTBSCert) + builder.AddUint16(uint16(MTCLogEntryTypeTBSCert)) return builder.BytesOrPanic() }, - wantErr: errMalformedExtensions, + wantErr: true, }, { name: "duplicate extensions over the wire", @@ -263,10 +263,10 @@ func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { extList.AddUint16(2) extList.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes([]byte("b")) }) builder.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { b.AddBytes(extList.BytesOrPanic()) }) - builder.AddUint16(MTCLogEntryTypeTBSCert) + builder.AddUint16(uint16(MTCLogEntryTypeTBSCert)) return builder.BytesOrPanic() }, - wantErr: errMalformedExtensions, + wantErr: true, }, } @@ -274,7 +274,7 @@ func TestMTCLogEntry_UnmarshalErrors(t *testing.T) { t.Run(tc.name, func(t *testing.T) { var entry MTCLogEntry err := entry.unmarshal(tc.mutate(nil)) - if !errors.Is(err, tc.wantErr) { + if (err != nil) != tc.wantErr { t.Errorf("unmarshal() error = %v, wantErr %v", err, tc.wantErr) } }) diff --git a/cmd/mtc/log/mtc.go b/cmd/mtc/log/mtc.go index f84c1bfd6..1434f2ea9 100644 --- a/cmd/mtc/log/mtc.go +++ b/cmd/mtc/log/mtc.go @@ -16,6 +16,7 @@ package log import ( "context" + "crypto/sha256" "errors" "fmt" @@ -46,13 +47,6 @@ type TBSCertificateLogEntry struct { Extensions []byte `json:"extensions,omitempty"` // Optional raw EXPLICIT SEQUENCE bytes, tag 3 } -var ( - errInvalidHashSize = errors.New("subjectPublicKeyInfoHash must be exactly 32 bytes") - errInvalidSequence = errors.New("invalid ASN.1 SEQUENCE structure") - errMalformedVersion = errors.New("version field is malformed") - errMissingField = errors.New("mandatory field is missing") -) - // Marshal returns the contents octets of the TBSCertificateLogEntry's // DER encoding, i.e. WITHOUT the outer SEQUENCE tag and length prefix. // @@ -79,6 +73,15 @@ func (e *TBSCertificateLogEntry) Marshal() ([]byte, error) { var b cryptobyte.Builder + // SPEC: RFC 5280 section 4.1.2.1. + // "If only basic fields are present, the version SHOULD be 1 (the value is + // omitted from the certificate as the default value)" + // "The encoding of a set value or sequence value shall not include an + // encoding for any component value which is equal to its default value." + // + // SPEC: ITU-T X.690 section 11.5 + // "The encoding of a set value or sequence value shall not include an encoding + // for any component value which is equal to its default value." if e.Version != 0 { b.AddASN1(asn1.Tag(0).ContextSpecific().Constructed(), func(b *cryptobyte.Builder) { b.AddASN1Int64(e.Version) @@ -111,7 +114,7 @@ func validateASN1Element(data []byte, expectedTag asn1.Tag, fieldName string) er 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) + return fmt.Errorf("%s: malformed ASN.1 or incorrect tag (expected %v)", fieldName, expectedTag) } return nil } @@ -121,18 +124,18 @@ func validateASN1Element(data []byte, expectedTag asn1.Tag, fieldName string) er // formatted. func (e *TBSCertificateLogEntry) Validate() error { if e.Version < 0 || e.Version > 2 { - return fmt.Errorf("invalid version %d: %w", e.Version, errMalformedVersion) + return fmt.Errorf("invalid version %d", e.Version) } switch { case len(e.Issuer) == 0: - return fmt.Errorf("issuer: %w", errMissingField) + return errors.New("issuer: mandatory field is missing") case len(e.Validity) == 0: - return fmt.Errorf("validity: %w", errMissingField) + return errors.New("validity: mandatory field is missing") case len(e.Subject) == 0: - return fmt.Errorf("subject: %w", errMissingField) + return errors.New("subject: mandatory field is missing") case len(e.SubjectPublicKeyAlgorithm) == 0: - return fmt.Errorf("subjectPublicKeyAlgorithm: %w", errMissingField) + return errors.New("subjectPublicKeyAlgorithm: mandatory field is missing") } if err := validateASN1Element(e.Issuer, asn1.SEQUENCE, "issuer"); err != nil { @@ -160,8 +163,8 @@ func (e *TBSCertificateLogEntry) Validate() error { // 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 { - return fmt.Errorf("subjectPublicKeyInfoHash must be 32 bytes, got %d: %w", len(e.SubjectPublicKeyInfoHash), errInvalidHashSize) + if len(e.SubjectPublicKeyInfoHash) != sha256.Size { + return fmt.Errorf("subjectPublicKeyInfoHash must be %d bytes, got %d", sha256.Size, len(e.SubjectPublicKeyInfoHash)) } return nil } diff --git a/cmd/mtc/log/mtc_test.go b/cmd/mtc/log/mtc_test.go index 11de6869d..03bb2c29d 100644 --- a/cmd/mtc/log/mtc_test.go +++ b/cmd/mtc/log/mtc_test.go @@ -16,7 +16,6 @@ package log import ( "bytes" - "errors" "fmt" "testing" @@ -36,17 +35,17 @@ func (e *TBSCertificateLogEntry) unmarshal(contents []byte) error { if seq.PeekASN1Tag(versionTag) { var verWrapped cryptobyte.String if !seq.ReadASN1(&verWrapped, versionTag) { - return fmt.Errorf("failed to read version wrapper: %w", errInvalidSequence) + return fmt.Errorf("failed to read version wrapper") } if !verWrapped.ReadASN1Integer(&e.Version) || !verWrapped.Empty() { - return errMalformedVersion + return fmt.Errorf("version field is malformed") } } readElement := func(target *[]byte, tag asn1.Tag, fieldName string) error { var rawElem cryptobyte.String if !seq.ReadASN1Element(&rawElem, tag) { - return fmt.Errorf("failed to read %s: %w", fieldName, errInvalidSequence) + return fmt.Errorf("failed to read %s", fieldName) } *target = append([]byte(nil), rawElem...) return nil @@ -66,7 +65,7 @@ func (e *TBSCertificateLogEntry) unmarshal(contents []byte) error { } var spkiHash cryptobyte.String if !seq.ReadASN1(&spkiHash, asn1.OCTET_STRING) { - return fmt.Errorf("failed to read subjectPublicKeyInfoHash: %w", errInvalidSequence) + return fmt.Errorf("failed to read subjectPublicKeyInfoHash") } e.SubjectPublicKeyInfoHash = append([]byte(nil), spkiHash...) @@ -92,7 +91,7 @@ func (e *TBSCertificateLogEntry) unmarshal(contents []byte) error { } if !seq.Empty() { - return fmt.Errorf("sequence has trailing bytes: %w", errInvalidSequence) + return fmt.Errorf("sequence has trailing bytes") } return nil } @@ -202,72 +201,72 @@ func TestTBSCertificateLogEntry_Validate(t *testing.T) { tests := []struct { name string mutate func(*TBSCertificateLogEntry) - wantErr error + wantErr bool }{ { name: "valid", mutate: func(e *TBSCertificateLogEntry) {}, - wantErr: nil, + wantErr: false, }, { name: "negative version", mutate: func(e *TBSCertificateLogEntry) { e.Version = -1 }, - wantErr: errMalformedVersion, + wantErr: true, }, { name: "version out of bounds", mutate: func(e *TBSCertificateLogEntry) { e.Version = 3 }, - wantErr: errMalformedVersion, + wantErr: true, }, { name: "malformed issuer asn1", mutate: func(e *TBSCertificateLogEntry) { e.Issuer = []byte("not-asn1-sequence") }, - wantErr: errInvalidSequence, + wantErr: true, }, { name: "trailing data on issuer", mutate: func(e *TBSCertificateLogEntry) { e.Issuer = append(dummySeq("issuer"), 0x00) }, - wantErr: errInvalidSequence, + wantErr: true, }, { name: "wrong tag on issuerUniqueID", mutate: func(e *TBSCertificateLogEntry) { e.IssuerUniqueID = dummyTag(asn1.Tag(5).ContextSpecific(), "uid") }, - wantErr: errInvalidSequence, + wantErr: true, }, { name: "wrong tag on extensions (primitive instead of constructed)", mutate: func(e *TBSCertificateLogEntry) { e.Extensions = dummyTag(asn1.Tag(3).ContextSpecific(), "exts") }, - wantErr: errInvalidSequence, + wantErr: true, }, { name: "missing issuer", mutate: func(e *TBSCertificateLogEntry) { e.Issuer = nil }, - wantErr: errMissingField, + wantErr: true, }, { name: "missing validity", mutate: func(e *TBSCertificateLogEntry) { e.Validity = nil }, - wantErr: errMissingField, + wantErr: true, }, { name: "missing subject", mutate: func(e *TBSCertificateLogEntry) { e.Subject = nil }, - wantErr: errMissingField, + wantErr: true, }, { name: "missing spki algo", mutate: func(e *TBSCertificateLogEntry) { e.SubjectPublicKeyAlgorithm = nil }, - wantErr: errMissingField, + wantErr: true, }, { name: "invalid hash size (short)", mutate: func(e *TBSCertificateLogEntry) { e.SubjectPublicKeyInfoHash = make([]byte, 10) }, - wantErr: errInvalidHashSize, + wantErr: true, }, { name: "invalid hash size (long)", mutate: func(e *TBSCertificateLogEntry) { e.SubjectPublicKeyInfoHash = make([]byte, 33) }, - wantErr: errInvalidHashSize, + wantErr: true, }, } @@ -276,7 +275,7 @@ func TestTBSCertificateLogEntry_Validate(t *testing.T) { e := valid() tc.mutate(&e) err := e.Validate() - if !errors.Is(err, tc.wantErr) { + if (err != nil) != tc.wantErr { t.Errorf("Validate() error = %v, wantErr %v", err, tc.wantErr) } }) From d39e24adda6197012d847b99b2a80b8078cc068e Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Tue, 28 Jul 2026 15:15:53 +0000 Subject: [PATCH 3/3] [MTC] Require extensions to be strictly sorted and unique in MTCLogEntry --- cmd/mtc/log/internal/entry/entry.go | 25 ++++----- cmd/mtc/log/internal/entry/entry_test.go | 66 ++++++++++-------------- 2 files changed, 36 insertions(+), 55 deletions(-) diff --git a/cmd/mtc/log/internal/entry/entry.go b/cmd/mtc/log/internal/entry/entry.go index 1da98c495..2b3ad4437 100644 --- a/cmd/mtc/log/internal/entry/entry.go +++ b/cmd/mtc/log/internal/entry/entry.go @@ -15,11 +15,8 @@ package entry import ( - "bytes" - "cmp" "errors" "fmt" - "slices" "golang.org/x/crypto/cryptobyte" ) @@ -92,22 +89,18 @@ func (e *MTCLogEntry) Marshal() ([]byte, error) { // SPEC: draft-ietf-plants-merkle-tree-certs section 5.2.1. // "The extensions list MUST appear in ascending order by extension_type and // MUST NOT contain two extensions with the same extension_type." - exts := slices.Clone(e.Extensions) - // First, sort extensions. - slices.SortStableFunc(exts, func(a, b MTCLogEntryExtension) int { - return cmp.Compare(a.Type, b.Type) - }) - - // Then, deduplicate extensions, unless duplicate extension - // tags have different data. 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) { - child.SetError(fmt.Errorf("conflicting duplicate extension type %d with differing data", ext.Type)) + for i, ext := range e.Extensions { + if i > 0 { + prevType := e.Extensions[i-1].Type + if ext.Type == prevType { + child.SetError(fmt.Errorf("duplicate extension type %d", ext.Type)) + return + } + if ext.Type < prevType { + child.SetError(fmt.Errorf("extensions out of order: type %d appears after %d", ext.Type, prevType)) return } - continue } child.AddUint16(uint16(ext.Type)) // Each extension data block has its own 16-bit length prefix. diff --git a/cmd/mtc/log/internal/entry/entry_test.go b/cmd/mtc/log/internal/entry/entry_test.go index 3f6c1ccfb..ef179b961 100644 --- a/cmd/mtc/log/internal/entry/entry_test.go +++ b/cmd/mtc/log/internal/entry/entry_test.go @@ -16,16 +16,13 @@ package entry import ( "bytes" - "cmp" "errors" "fmt" - "slices" "testing" "golang.org/x/crypto/cryptobyte" ) - // unmarshal decodes a raw TLS presentation byte stream into an MTCLogEntry structure. // This is kept in the test suite for verifying round-trip serialization of MTCLogEntry. func (e *MTCLogEntry) unmarshal(data []byte) error { @@ -97,30 +94,6 @@ func TestMTCLogEntry_RoundTrip(t *testing.T) { }, }, }, - { - name: "tbs cert entry with unsorted extensions", - entry: MTCLogEntry{ - Type: MTCLogEntryTypeTBSCert, - EntryData: []byte("fake-der-octets"), - Extensions: []MTCLogEntryExtension{ - {Type: 10, Data: []byte("ext-10-data")}, - {Type: 1, Data: []byte("ext-1-data")}, - {Type: 5, Data: []byte("ext-5-data")}, - }, - }, - }, - { - name: "tbs cert entry with identical duplicate extensions", - entry: MTCLogEntry{ - Type: MTCLogEntryTypeTBSCert, - EntryData: []byte("fake-der-octets"), - Extensions: []MTCLogEntryExtension{ - {Type: 5, Data: []byte("ext-5-data")}, - {Type: 1, Data: []byte("ext-1-data")}, - {Type: 5, Data: []byte("ext-5-data")}, - }, - }, - }, } for _, tc := range tests { @@ -142,19 +115,12 @@ func TestMTCLogEntry_RoundTrip(t *testing.T) { t.Errorf("EntryData = %x, want %x", got.EntryData, tc.entry.EntryData) } - wantExts := slices.Clone(tc.entry.Extensions) - slices.SortStableFunc(wantExts, func(a, b MTCLogEntryExtension) int { - return cmp.Compare(a.Type, b.Type) - }) - wantExts = slices.CompactFunc(wantExts, func(a, b MTCLogEntryExtension) bool { - return a.Type == b.Type && bytes.Equal(a.Data, b.Data) - }) - if len(got.Extensions) != len(wantExts) { - t.Fatalf("len(Extensions) = %d, want %d", len(got.Extensions), len(wantExts)) + if len(got.Extensions) != len(tc.entry.Extensions) { + t.Fatalf("len(Extensions) = %d, want %d", len(got.Extensions), len(tc.entry.Extensions)) } for i := range got.Extensions { - if got.Extensions[i].Type != wantExts[i].Type || !bytes.Equal(got.Extensions[i].Data, wantExts[i].Data) { - t.Errorf("Extension[%d] = %+v, want %+v", i, got.Extensions[i], wantExts[i]) + if got.Extensions[i].Type != tc.entry.Extensions[i].Type || !bytes.Equal(got.Extensions[i].Data, tc.entry.Extensions[i].Data) { + t.Errorf("Extension[%d] = %+v, want %+v", i, got.Extensions[i], tc.entry.Extensions[i]) } } }) @@ -176,7 +142,18 @@ func TestMTCLogEntry_MarshalErrors(t *testing.T) { wantErr: true, }, { - name: "conflicting duplicate extensions with different data", + name: "duplicate extensions with same data", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + Extensions: []MTCLogEntryExtension{ + {Type: 2, Data: []byte("data-a")}, + {Type: 2, Data: []byte("data-a")}, + }, + }, + wantErr: true, + }, + { + name: "duplicate extensions with different data", entry: MTCLogEntry{ Type: MTCLogEntryTypeTBSCert, Extensions: []MTCLogEntryExtension{ @@ -186,6 +163,17 @@ func TestMTCLogEntry_MarshalErrors(t *testing.T) { }, wantErr: true, }, + { + name: "unsorted extensions", + entry: MTCLogEntry{ + Type: MTCLogEntryTypeTBSCert, + Extensions: []MTCLogEntryExtension{ + {Type: 5, Data: []byte("data-5")}, + {Type: 1, Data: []byte("data-1")}, + }, + }, + wantErr: true, + }, { name: "entry size exceeds tile limit", entry: MTCLogEntry{