From 33865ad130e6811f5cb9e3d393e509ba3b104e05 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Mon, 7 Apr 2025 14:16:09 +0000 Subject: [PATCH 1/6] return the dir of the log --- internal/scti/handlers_test.go | 38 ++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/internal/scti/handlers_test.go b/internal/scti/handlers_test.go index d358d7d2..fe50fd89 100644 --- a/internal/scti/handlers_test.go +++ b/internal/scti/handlers_test.go @@ -64,6 +64,10 @@ var ( MaskInternalErrors: false, TimeSource: newFixedTimeSource(fakeTime), } + + // POSIX subdirectories + logDir = "log" + issDir = "issuers" ) type fixedTimeSource struct { @@ -81,8 +85,9 @@ func (f *fixedTimeSource) Now() time.Time { } // setupTestLog creates a test TesseraCT log using a POSIX backend. -func setupTestLog(t *testing.T) *log { +func setupTestLog(t *testing.T) (*log, string) { t.Helper() + storageDir := t.TempDir() signer, err := setupSigner(fakeSignature) if err != nil { @@ -100,12 +105,12 @@ func setupTestLog(t *testing.T) *log { rejectUnexpired: false, } - log, err := NewLog(t.Context(), origin, signer, cvOpts, newPosixStorageFunc(t), newFixedTimeSource(fakeTime)) + log, err := NewLog(t.Context(), origin, signer, cvOpts, newPosixStorageFunc(t, storageDir), newFixedTimeSource(fakeTime)) if err != nil { t.Fatalf("newLog(): %v", err) } - return log + return log, storageDir } // setupTestServer creates a test TesseraCT server with a single endpoint at path. @@ -125,10 +130,13 @@ func setupTestServer(t *testing.T, log *log, path string) *httptest.Server { // - a POSIX Tessera storage driver // - a POSIX issuer storage system // - a BBolt deduplication database -func newPosixStorageFunc(t *testing.T) storage.CreateStorage { +// +// It also prepares directories to host the log and the deduplication database. +func newPosixStorageFunc(t *testing.T, root string) storage.CreateStorage { t.Helper() + return func(ctx context.Context, signer note.Signer) (*storage.CTStorage, error) { - driver, err := posixTessera.New(ctx, path.Join(t.TempDir(), "log")) + driver, err := posixTessera.New(ctx, path.Join(root, logDir)) if err != nil { klog.Fatalf("Failed to initialize POSIX Tessera storage driver: %v", err) } @@ -144,12 +152,12 @@ func newPosixStorageFunc(t *testing.T) storage.CreateStorage { klog.Fatalf("Failed to initialize POSIX Tessera appender: %v", err) } - issuerStorage, err := posix.NewIssuerStorage(t.TempDir()) + issuerStorage, err := posix.NewIssuerStorage(path.Join(root, issDir)) if err != nil { klog.Fatalf("failed to initialize InMemory issuer storage: %v", err) } - beDedupStorage, err := bbolt.NewStorage(path.Join(t.TempDir(), "dedup.db")) + beDedupStorage, err := bbolt.NewStorage(path.Join(root, "dedup.db")) if err != nil { klog.Fatalf("Failed to initialize BBolt deduplication database: %v", err) } @@ -193,7 +201,7 @@ func postHandlers(t *testing.T, handlers pathHandlers) pathHandlers { } func TestPostHandlersRejectGet(t *testing.T) { - log := setupTestLog(t) + log, _ := setupTestLog(t) handlers := NewPathHandlers(&hOpts, log) // Anything in the post handler list should reject GET @@ -214,7 +222,7 @@ func TestPostHandlersRejectGet(t *testing.T) { } func TestGetHandlersRejectPost(t *testing.T) { - log := setupTestLog(t) + log, _ := setupTestLog(t) handlers := NewPathHandlers(&hOpts, log) // Anything in the get handler list should reject POST. @@ -247,7 +255,7 @@ func TestPostHandlersFailure(t *testing.T) { {"wrong-chain", strings.NewReader(`{ "chain": [ "test" ] }`), http.StatusBadRequest}, } - log := setupTestLog(t) + log, _ := setupTestLog(t) handlers := NewPathHandlers(&hOpts, log) for path, handler := range postHandlers(t, handlers) { @@ -269,7 +277,7 @@ func TestPostHandlersFailure(t *testing.T) { } func TestNewPathHandlers(t *testing.T) { - log := setupTestLog(t) + log, _ := setupTestLog(t) t.Run("Handlers", func(t *testing.T) { handlers := NewPathHandlers(&HandlerOptions{}, log) // Check each entrypoint has a handler @@ -321,7 +329,7 @@ func TestNewPathHandlers(t *testing.T) { // } func TestGetRoots(t *testing.T) { - log := setupTestLog(t) + log, _ := setupTestLog(t) server := setupTestServer(t, log, path.Join(prefix, rfc6962.GetRootsPath)) defer server.Close() @@ -411,7 +419,7 @@ func TestAddChainWhitespace(t *testing.T) { }, } - log := setupTestLog(t) + log, _ := setupTestLog(t) server := setupTestServer(t, log, path.Join(prefix, rfc6962.AddChainPath)) defer server.Close() @@ -472,7 +480,7 @@ func TestAddChain(t *testing.T) { }, } - log := setupTestLog(t) + log, _ := setupTestLog(t) server := setupTestServer(t, log, path.Join(prefix, rfc6962.AddChainPath)) defer server.Close() @@ -563,7 +571,7 @@ func TestAddPreChain(t *testing.T) { }, } - log := setupTestLog(t) + log, _ := setupTestLog(t) server := setupTestServer(t, log, path.Join(prefix, rfc6962.AddPreChainPath)) defer server.Close() From 0ba13213ee90f0c3f153e0833106e8d2a577fd2d Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Mon, 7 Apr 2025 13:31:03 +0000 Subject: [PATCH 2/6] add TODO --- internal/scti/handlers.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/scti/handlers.go b/internal/scti/handlers.go index a927043a..942c9453 100644 --- a/internal/scti/handlers.go +++ b/internal/scti/handlers.go @@ -393,6 +393,8 @@ func deadlineTime(opts *HandlerOptions) time.Time { // verifyAddChain is used by add-chain and add-pre-chain. It does the checks that the supplied // cert is of the correct type and chains to a trusted root. +// TODO(phbnf): add tests +// TODO(phbnf): move to chain_validation.go func verifyAddChain(log *log, req rfc6962.AddChainRequest, expectingPrecert bool) ([]*x509.Certificate, error) { // We already checked that the chain is not empty so can move on to verification validPath, err := validateChain(req.Chain, log.chainValidationOpts) @@ -452,7 +454,8 @@ func marshalAndWriteAddChainResponse(sct *rfc6962.SignedCertificateTimestamp, w // entryFromChain generates an Entry from a chain and timestamp. // copied from certificate-transparency-go/serialization.go -// TODO(phboneff): move in a different file maybe? +// TODO(phboneff): move to ct.go +// TODO(phboneff): add tests func entryFromChain(chain []*x509.Certificate, isPrecert bool, timestamp uint64) (*ctonly.Entry, error) { leaf := ctonly.Entry{ IsPrecert: isPrecert, From ed7acc1541fc0b3f5298036b07aff99719305a9e Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Mon, 7 Apr 2025 13:49:11 +0000 Subject: [PATCH 3/6] verify that entries in the log are correct # Conflicts: # internal/scti/handlers_test.go # Conflicts: # internal/scti/handlers_test.go --- internal/scti/handlers_test.go | 190 +++++++++++++++++++++------------ 1 file changed, 124 insertions(+), 66 deletions(-) diff --git a/internal/scti/handlers_test.go b/internal/scti/handlers_test.go index fe50fd89..e71030c2 100644 --- a/internal/scti/handlers_test.go +++ b/internal/scti/handlers_test.go @@ -18,6 +18,7 @@ import ( "bufio" "bytes" "context" + "crypto/x509" "encoding/base64" "encoding/hex" "encoding/json" @@ -25,6 +26,7 @@ import ( "io" "net/http" "net/http/httptest" + "os" "path" "strings" "testing" @@ -40,6 +42,8 @@ import ( "github.com/transparency-dev/static-ct/storage" "github.com/transparency-dev/static-ct/storage/bbolt" tessera "github.com/transparency-dev/trillian-tessera" + "github.com/transparency-dev/trillian-tessera/api/layout" + "github.com/transparency-dev/trillian-tessera/ctonly" posixTessera "github.com/transparency-dev/trillian-tessera/storage/posix" "golang.org/x/mod/sumdb/note" "k8s.io/klog/v2" @@ -308,25 +312,23 @@ func TestNewPathHandlers(t *testing.T) { }) } -// TODO(phboneff): use this in followup PR. Leaving here for now to make -// diffs easier to digest in PRs. -// func parseChain(t *testing.T, isPrecert bool, pemChain []string, root *x509.Certificate) (*ctonly.Entry, []*x509.Certificate) { -// t.Helper() -// pool := loadCertsIntoPoolOrDie(t, pemChain) -// leafChain := pool.RawCertificates() -// if !leafChain[len(leafChain)-1].Equal(root) { -// // The submitted chain may not include a root, but the generated LogLeaf will -// fullChain := make([]*x509.Certificate, len(leafChain)+1) -// copy(fullChain, leafChain) -// fullChain[len(leafChain)] = root -// leafChain = fullChain -// } -// entry, err := entryFromChain(leafChain, isPrecert, fakeTimeMillis) -// if err != nil { -// t.Fatalf("failed to create entry") -// } -// return entry, leafChain -// } +func parseChain(t *testing.T, isPrecert bool, pemChain []string, root *x509.Certificate) (*ctonly.Entry, []*x509.Certificate) { + t.Helper() + pool := loadCertsIntoPoolOrDie(t, pemChain) + leafChain := pool.RawCertificates() + if !leafChain[len(leafChain)-1].Equal(root) { + // The submitted chain may not include a root, but the generated LogLeaf will + fullChain := make([]*x509.Certificate, len(leafChain)+1) + copy(fullChain, leafChain) + fullChain[len(leafChain)] = root + leafChain = fullChain + } + entry, err := entryFromChain(leafChain, isPrecert, fakeTimeMillis) + if err != nil { + t.Fatalf("failed to create entry") + } + return entry, leafChain +} func TestGetRoots(t *testing.T) { log, _ := setupTestLog(t) @@ -438,11 +440,12 @@ func TestAddChainWhitespace(t *testing.T) { func TestAddChain(t *testing.T) { var tests = []struct { - descr string - chain []string - want int - wantIdx uint64 - err error + descr string + chain []string + want int + wantIdx uint64 + wantLogSize uint64 + err error }{ { descr: "leaf-only", @@ -455,32 +458,36 @@ func TestAddChain(t *testing.T) { want: http.StatusBadRequest, }, { - descr: "success", - chain: []string{testdata.CertFromIntermediate, testdata.IntermediateFromRoot, testdata.CACertPEM}, - wantIdx: 0, - want: http.StatusOK, + descr: "success", + chain: []string{testdata.CertFromIntermediate, testdata.IntermediateFromRoot, testdata.CACertPEM}, + wantIdx: 0, + wantLogSize: 1, + want: http.StatusOK, }, { - descr: "success-duplicate", - chain: []string{testdata.CertFromIntermediate, testdata.IntermediateFromRoot, testdata.CACertPEM}, - wantIdx: 0, - want: http.StatusOK, + descr: "success-duplicate", + chain: []string{testdata.CertFromIntermediate, testdata.IntermediateFromRoot, testdata.CACertPEM}, + wantIdx: 0, + wantLogSize: 1, + want: http.StatusOK, }, { - descr: "success-not-duplicate", - chain: []string{testdata.TestCertPEM, testdata.CACertPEM}, - wantIdx: 1, - want: http.StatusOK, + descr: "success-not-duplicate", + chain: []string{testdata.TestCertPEM, testdata.CACertPEM}, + wantIdx: 1, + wantLogSize: 2, + want: http.StatusOK, }, { - descr: "success-without-root", - chain: []string{testdata.CertFromIntermediate, testdata.IntermediateFromRoot}, - wantIdx: 0, - want: http.StatusOK, + descr: "success-without-root", + chain: []string{testdata.CertFromIntermediate, testdata.IntermediateFromRoot}, + wantIdx: 0, + wantLogSize: 2, + want: http.StatusOK, }, } - log, _ := setupTestLog(t) + log, dir := setupTestLog(t) server := setupTestServer(t, log, path.Join(prefix, rfc6962.AddChainPath)) defer server.Close() @@ -497,6 +504,8 @@ func TestAddChain(t *testing.T) { t.Errorf("http.Post(%s)=(%d,nil); want (%d,nil)", rfc6962.AddChainPath, got, want) } if test.want == http.StatusOK { + wantEntry, _ := parseChain(t, false, test.chain, log.chainValidationOpts.trustedRoots.RawCertificates()[0]) + var gotRsp rfc6962.AddChainResponse if err := json.NewDecoder(resp.Body).Decode(&gotRsp); err != nil { t.Fatalf("json.Decode()=%v; want nil", err) @@ -513,6 +522,8 @@ func TestAddChain(t *testing.T) { if got, want := hex.EncodeToString(gotRsp.Signature), "040300067369676e6564"; got != want { t.Errorf("resp.Signature=%s; want %s", got, want) } + + // Check that the Extensions contains the expected index. idx, err := staticct.ParseCTExtensions(gotRsp.Extensions) if err != nil { t.Errorf("Failed to parse extensions %q: %v", gotRsp.Extensions, err) @@ -520,8 +531,27 @@ func TestAddChain(t *testing.T) { if got, want := idx, test.wantIdx; got != want { t.Errorf("resp.Extensions.Index=%d; want %d", got, want) } - // TODO(phboneff): read from the log and compare values - // TODO(phboneff): add a test with a backend write failure + + // Check that the leaf bundle contains the expected leaf. + bPath := path.Join(dir, logDir, "tile/data", layout.NWithSuffix(0, test.wantLogSize/256, uint8(test.wantLogSize))) + bundle, err := os.ReadFile(bPath) + if err != nil { + t.Errorf("Failed to read leaf bundle at %q: %v", bPath, err) + } + eBundle := staticct.EntryBundle{} + if err := eBundle.UnmarshalText(bundle); err != nil { + t.Errorf("Failed to parse entry bundle: %v", err) + } + if uint64(len(eBundle.Entries)) < test.wantIdx { + t.Errorf("Got %d entries, want %d", len(eBundle.Entries), test.wantIdx) + } + if got, want := eBundle.Entries[test.wantIdx], wantEntry.LeafData(test.wantIdx); !bytes.Equal(got, want) { + // TODO(phbnf): replace with a more useful error message + t.Errorf("Got entry %q, want %q", got, want) + } + // TODO(phbnf): check the issuer chain fingerprint + // TODO(phbnf): check inclusion proof + // TODO(phbnf): add a test with a backend write failure } }) } @@ -529,11 +559,12 @@ func TestAddChain(t *testing.T) { func TestAddPreChain(t *testing.T) { var tests = []struct { - descr string - chain []string - want int - wantIdx uint64 - err error + descr string + chain []string + want int + wantIdx uint64 + wantLogSize uint64 + err error }{ { descr: "leaf-signed-by-different", @@ -546,32 +577,36 @@ func TestAddPreChain(t *testing.T) { want: http.StatusBadRequest, }, { - descr: "success", - chain: []string{testdata.PrecertPEMValid, testdata.CACertPEM}, - want: http.StatusOK, - wantIdx: 0, + descr: "success", + chain: []string{testdata.PrecertPEMValid, testdata.CACertPEM}, + want: http.StatusOK, + wantIdx: 0, + wantLogSize: 1, }, { - descr: "success-duplicate", - chain: []string{testdata.PrecertPEMValid, testdata.CACertPEM}, - want: http.StatusOK, - wantIdx: 0, + descr: "success-duplicate", + chain: []string{testdata.PrecertPEMValid, testdata.CACertPEM}, + want: http.StatusOK, + wantIdx: 0, + wantLogSize: 1, }, { - descr: "success-with-intermediate", - chain: []string{testdata.PreCertFromIntermediate, testdata.IntermediateFromRoot, testdata.CACertPEM}, - want: http.StatusOK, - wantIdx: 1, + descr: "success-with-intermediate", + chain: []string{testdata.PreCertFromIntermediate, testdata.IntermediateFromRoot, testdata.CACertPEM}, + want: http.StatusOK, + wantIdx: 1, + wantLogSize: 2, }, { - descr: "success-without-root", - chain: []string{testdata.PrecertPEMValid}, - want: http.StatusOK, - wantIdx: 0, + descr: "success-without-root", + chain: []string{testdata.PrecertPEMValid}, + want: http.StatusOK, + wantIdx: 0, + wantLogSize: 2, }, } - log, _ := setupTestLog(t) + log, dir := setupTestLog(t) server := setupTestServer(t, log, path.Join(prefix, rfc6962.AddPreChainPath)) defer server.Close() @@ -588,6 +623,8 @@ func TestAddPreChain(t *testing.T) { t.Errorf("http.Post(%s)=(%d,nil); want (%d,nil)", rfc6962.AddPreChainPath, got, want) } if test.want == http.StatusOK { + wantEntry, _ := parseChain(t, true, test.chain, log.chainValidationOpts.trustedRoots.RawCertificates()[0]) + var gotRsp rfc6962.AddChainResponse if err := json.NewDecoder(resp.Body).Decode(&gotRsp); err != nil { t.Fatalf("json.Decode()=%v; want nil", err) @@ -604,6 +641,8 @@ func TestAddPreChain(t *testing.T) { if got, want := hex.EncodeToString(gotRsp.Signature), "040300067369676e6564"; got != want { t.Errorf("resp.Signature=%s; want %s", got, want) } + + // Check that the Extensions contains the expected index. idx, err := staticct.ParseCTExtensions(gotRsp.Extensions) if err != nil { t.Errorf("Failed to parse extensions %q: %v", gotRsp.Extensions, err) @@ -611,7 +650,26 @@ func TestAddPreChain(t *testing.T) { if got, want := idx, test.wantIdx; got != want { t.Errorf("resp.Extensions.Index=%d; want %d", got, want) } - // TODO(phboneff): read from the log and compare values + + // Check that the leaf bundle contains the expected leaf. + bPath := path.Join(dir, logDir, "tile/data", layout.NWithSuffix(0, test.wantLogSize/256, uint8(test.wantLogSize))) + bundle, err := os.ReadFile(bPath) + if err != nil { + t.Errorf("Failed to read leaf bundle at %q: %v", bPath, err) + } + eBundle := staticct.EntryBundle{} + if err := eBundle.UnmarshalText(bundle); err != nil { + t.Errorf("Failed to parse entry bundle: %v", err) + } + if uint64(len(eBundle.Entries)) < test.wantIdx { + t.Errorf("Got %d entries, want %d", len(eBundle.Entries), test.wantIdx) + } + if got, want := eBundle.Entries[test.wantIdx], wantEntry.LeafData(test.wantIdx); !bytes.Equal(got, want) { + // TODO(phbnf): replace with a more useful error message + t.Errorf("Got entry %q, want %q", got, want) + } + // TODO(phbnf): check the issuer chain fingerprint + // TODO(phbnf): check inclusion proof // TODO(phboneff): add a test with a backend write failure } }) From 67e764493207e0778847d920b531c8b3aca331b2 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Mon, 7 Apr 2025 14:05:29 +0000 Subject: [PATCH 4/6] add a function to parse Entries --- internal/types/staticct/staticct.go | 111 ++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/internal/types/staticct/staticct.go b/internal/types/staticct/staticct.go index 44ca2c88..5e78a897 100644 --- a/internal/types/staticct/staticct.go +++ b/internal/types/staticct/staticct.go @@ -15,6 +15,7 @@ package staticct import ( + "bytes" "encoding/base64" "fmt" "math" @@ -145,3 +146,113 @@ func readUint40(s *cryptobyte.String, out *uint64) bool { *out = uint64(v[0])<<32 | uint64(v[1])<<24 | uint64(v[2])<<16 | uint64(v[3])<<8 | uint64(v[4]) return true } + +// Entry represents a CT log entry. +type Entry struct { + Timestamp uint64 + IsPrecert bool + // Certificate holds different things depending on whether the entry represents a Certificate or a Precertificate submission: + // - IsPrecert == false: the bytes here are the x509 certificate submitted for logging. + // - IsPrecert == true: the bytes here are the TBS certificate extracted from the submitted precert. + Certificate []byte + // Precertificate holds the precertificate to be logged, only used when IsPrecert is true. + Precertificate []byte + IssuerKeyHash []byte + RawFingerprints string + FingerprintsChain [][32]byte + RawExtensions string + LeafIndex uint64 +} + +// UnmarshalText implements encoding/TextUnmarshaler and reads EntryBundles +// which are encoded using the Static CT API spec. +func (t *Entry) UnmarshalText(raw []byte) error { + s := cryptobyte.String(raw) + + entry := []byte{} + var entryType uint16 + var extensions, fingerprints cryptobyte.String + if !s.ReadUint64(&t.Timestamp) || !s.ReadUint16(&entryType) || t.Timestamp > math.MaxInt64 { + return fmt.Errorf("invalid data tile") + } + + bb := []byte{} + b := cryptobyte.NewBuilder(bb) + b.AddUint64(t.Timestamp) + b.AddUint16(entryType) + + switch entryType { + case 0: // x509_entry + t.IsPrecert = false + if !s.ReadUint24LengthPrefixed((*cryptobyte.String)(&entry)) || + !s.ReadUint16LengthPrefixed(&extensions) || + !s.ReadUint16LengthPrefixed(&fingerprints) { + return fmt.Errorf("invalid data tile x509_entry") + } + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(entry) + t.Certificate = bytes.Clone(entry) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(extensions) + t.RawExtensions = string(extensions) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(fingerprints) + t.RawFingerprints = string(fingerprints) + }) + + case 1: // precert_entry + t.IsPrecert = true + IssuerKeyHash := [32]byte{} + var defangedCrt, extensions cryptobyte.String + if !s.CopyBytes(IssuerKeyHash[:]) || + !s.ReadUint24LengthPrefixed(&defangedCrt) || + !s.ReadUint16LengthPrefixed(&extensions) || + !s.ReadUint24LengthPrefixed((*cryptobyte.String)(&entry)) || + !s.ReadUint16LengthPrefixed(&fingerprints) { + return fmt.Errorf("invalid data tile precert_entry") + } + b.AddBytes(IssuerKeyHash[:]) + t.IssuerKeyHash = bytes.Clone(IssuerKeyHash[:]) + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(defangedCrt) + t.Certificate = bytes.Clone(defangedCrt) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(extensions) + t.RawExtensions = string(extensions) + }) + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(entry) + t.Precertificate = bytes.Clone(entry) + }) + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(fingerprints) + t.RawFingerprints = string(fingerprints) + }) + default: + return fmt.Errorf("invalid data tile: unknown type %d", entryType) + } + + var err error + t.LeafIndex, err = ParseCTExtensions(base64.StdEncoding.EncodeToString([]byte(t.RawExtensions))) + if err != nil { + return fmt.Errorf("can't parse extensions: %v", err) + } + + rfp := cryptobyte.String(t.RawFingerprints) + for i := 0; len(rfp) > 0; i++ { + fp := [32]byte{} + if !rfp.CopyBytes(fp[:]) { + return fmt.Errorf("can't extract fingerprint number %d", i) + } + t.FingerprintsChain = append(t.FingerprintsChain, fp) + } + + if len(s) > 0 { + return fmt.Errorf("trailing %d bytes after entry", len(s)) + } + + return nil +} From 2ac3f3f04ce4d222524586689b37cf27bbee32e5 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Mon, 7 Apr 2025 14:42:36 +0000 Subject: [PATCH 5/6] display a more useful message # Conflicts: # internal/scti/handlers_test.go --- internal/scti/handlers_test.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/scti/handlers_test.go b/internal/scti/handlers_test.go index e71030c2..2199f633 100644 --- a/internal/scti/handlers_test.go +++ b/internal/scti/handlers_test.go @@ -327,6 +327,7 @@ func parseChain(t *testing.T, isPrecert bool, pemChain []string, root *x509.Cert if err != nil { t.Fatalf("failed to create entry") } + return entry, leafChain } @@ -504,7 +505,7 @@ func TestAddChain(t *testing.T) { t.Errorf("http.Post(%s)=(%d,nil); want (%d,nil)", rfc6962.AddChainPath, got, want) } if test.want == http.StatusOK { - wantEntry, _ := parseChain(t, false, test.chain, log.chainValidationOpts.trustedRoots.RawCertificates()[0]) + unseqEntry, _ := parseChain(t, false, test.chain, log.chainValidationOpts.trustedRoots.RawCertificates()[0]) var gotRsp rfc6962.AddChainResponse if err := json.NewDecoder(resp.Body).Decode(&gotRsp); err != nil { @@ -545,9 +546,16 @@ func TestAddChain(t *testing.T) { if uint64(len(eBundle.Entries)) < test.wantIdx { t.Errorf("Got %d entries, want %d", len(eBundle.Entries), test.wantIdx) } - if got, want := eBundle.Entries[test.wantIdx], wantEntry.LeafData(test.wantIdx); !bytes.Equal(got, want) { - // TODO(phbnf): replace with a more useful error message - t.Errorf("Got entry %q, want %q", got, want) + gotEntry := staticct.Entry{} + if err := gotEntry.UnmarshalText(eBundle.Entries[test.wantIdx]); err != nil { + t.Errorf("Failed to parse log entry: %v", err) + } + wantEntry := staticct.Entry{} + if err := wantEntry.UnmarshalText(unseqEntry.LeafData(test.wantIdx)); err != nil { + t.Errorf("Failed to parse log entry: %v", err) + } + if diff := cmp.Diff(wantEntry, gotEntry); diff != "" { + t.Errorf("Logged entry mismatch (-want +got):\n%s", diff) } // TODO(phbnf): check the issuer chain fingerprint // TODO(phbnf): check inclusion proof @@ -623,7 +631,7 @@ func TestAddPreChain(t *testing.T) { t.Errorf("http.Post(%s)=(%d,nil); want (%d,nil)", rfc6962.AddPreChainPath, got, want) } if test.want == http.StatusOK { - wantEntry, _ := parseChain(t, true, test.chain, log.chainValidationOpts.trustedRoots.RawCertificates()[0]) + unseqEntry, _ := parseChain(t, true, test.chain, log.chainValidationOpts.trustedRoots.RawCertificates()[0]) var gotRsp rfc6962.AddChainResponse if err := json.NewDecoder(resp.Body).Decode(&gotRsp); err != nil { @@ -664,9 +672,16 @@ func TestAddPreChain(t *testing.T) { if uint64(len(eBundle.Entries)) < test.wantIdx { t.Errorf("Got %d entries, want %d", len(eBundle.Entries), test.wantIdx) } - if got, want := eBundle.Entries[test.wantIdx], wantEntry.LeafData(test.wantIdx); !bytes.Equal(got, want) { - // TODO(phbnf): replace with a more useful error message - t.Errorf("Got entry %q, want %q", got, want) + gotEntry := staticct.Entry{} + if err := gotEntry.UnmarshalText(eBundle.Entries[test.wantIdx]); err != nil { + t.Errorf("Failed to parse log entry: %v", err) + } + wantEntry := staticct.Entry{} + if err := wantEntry.UnmarshalText(unseqEntry.LeafData(test.wantIdx)); err != nil { + t.Errorf("Failed to parse log entry: %v", err) + } + if diff := cmp.Diff(wantEntry, gotEntry); diff != "" { + t.Errorf("Logged entry mismatch (-want +got):\n%s", diff) } // TODO(phbnf): check the issuer chain fingerprint // TODO(phbnf): check inclusion proof From 178f62bab09b45569adb1e00a4826d36868c19b2 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Tue, 8 Apr 2025 16:08:30 +0000 Subject: [PATCH 6/6] Address comments --- internal/scti/handlers_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/scti/handlers_test.go b/internal/scti/handlers_test.go index 2199f633..1648d16a 100644 --- a/internal/scti/handlers_test.go +++ b/internal/scti/handlers_test.go @@ -89,6 +89,8 @@ func (f *fixedTimeSource) Now() time.Time { } // setupTestLog creates a test TesseraCT log using a POSIX backend. +// +// It returns the log and the path to the storage directory. func setupTestLog(t *testing.T) (*log, string) { t.Helper() storageDir := t.TempDir() @@ -109,7 +111,7 @@ func setupTestLog(t *testing.T) (*log, string) { rejectUnexpired: false, } - log, err := NewLog(t.Context(), origin, signer, cvOpts, newPosixStorageFunc(t, storageDir), newFixedTimeSource(fakeTime)) + log, err := NewLog(t.Context(), origin, signer, cvOpts, newPOSIXStorageFunc(t, storageDir), newFixedTimeSource(fakeTime)) if err != nil { t.Fatalf("newLog(): %v", err) } @@ -130,13 +132,13 @@ func setupTestServer(t *testing.T, log *log, path string) *httptest.Server { return httptest.NewServer(handler) } -// newPosixStorageFunc returns a function to create a new storage.CTStorage instance with: +// newPOSIXStorageFunc returns a function to create a new storage.CTStorage instance with: // - a POSIX Tessera storage driver // - a POSIX issuer storage system // - a BBolt deduplication database // // It also prepares directories to host the log and the deduplication database. -func newPosixStorageFunc(t *testing.T, root string) storage.CreateStorage { +func newPOSIXStorageFunc(t *testing.T, root string) storage.CreateStorage { t.Helper() return func(ctx context.Context, signer note.Signer) (*storage.CTStorage, error) { @@ -317,7 +319,7 @@ func parseChain(t *testing.T, isPrecert bool, pemChain []string, root *x509.Cert pool := loadCertsIntoPoolOrDie(t, pemChain) leafChain := pool.RawCertificates() if !leafChain[len(leafChain)-1].Equal(root) { - // The submitted chain may not include a root, but the generated LogLeaf will + // The submitted chain may not include a root, but the generated LogLeaf will. fullChain := make([]*x509.Certificate, len(leafChain)+1) copy(fullChain, leafChain) fullChain[len(leafChain)] = root @@ -325,7 +327,7 @@ func parseChain(t *testing.T, isPrecert bool, pemChain []string, root *x509.Cert } entry, err := entryFromChain(leafChain, isPrecert, fakeTimeMillis) if err != nil { - t.Fatalf("failed to create entry") + t.Fatalf("Failed to create entry") } return entry, leafChain @@ -534,7 +536,7 @@ func TestAddChain(t *testing.T) { } // Check that the leaf bundle contains the expected leaf. - bPath := path.Join(dir, logDir, "tile/data", layout.NWithSuffix(0, test.wantLogSize/256, uint8(test.wantLogSize))) + bPath := path.Join(dir, logDir, "tile/data", layout.NWithSuffix(0, test.wantLogSize/layout.EntryBundleWidth, uint8(test.wantLogSize))) bundle, err := os.ReadFile(bPath) if err != nil { t.Errorf("Failed to read leaf bundle at %q: %v", bPath, err) @@ -660,7 +662,7 @@ func TestAddPreChain(t *testing.T) { } // Check that the leaf bundle contains the expected leaf. - bPath := path.Join(dir, logDir, "tile/data", layout.NWithSuffix(0, test.wantLogSize/256, uint8(test.wantLogSize))) + bPath := path.Join(dir, logDir, "tile/data", layout.NWithSuffix(0, test.wantLogSize/layout.EntryBundleWidth, uint8(test.wantLogSize))) bundle, err := os.ReadFile(bPath) if err != nil { t.Errorf("Failed to read leaf bundle at %q: %v", bPath, err)