diff --git a/persistence/sqlite/sql_test.go b/persistence/sqlite/sql_test.go index 4b20efa2..7e7ced2c 100644 --- a/persistence/sqlite/sql_test.go +++ b/persistence/sqlite/sql_test.go @@ -15,14 +15,19 @@ package sqlite import ( - "testing" - + "context" "database/sql" + "encoding/hex" + "testing" + "time" - _ "modernc.org/sqlite" // Load drivers for sqlite3 "github.com/transparency-dev/formats/log" + f_note "github.com/transparency-dev/formats/note" "github.com/transparency-dev/witness/omniwitness" + "github.com/transparency-dev/witness/witness" ptest "github.com/transparency-dev/witness/persistence/testonly" + "golang.org/x/mod/sumdb/note" + _ "modernc.org/sqlite" // Load drivers for sqlite3 ) func TestUpdate(t *testing.T) { @@ -149,3 +154,79 @@ func TestDisabledLogs(t *testing.T) { } } } + +func TestDeadlock(t *testing.T) { + db, cleanup := mustCreateDB(t) // MaxOpenConns(1) + defer func() { _ = cleanup() }() + + p := New(db) + if err := p.Init(t.Context()); err != nil { + t.Fatalf("Init(): %v", err) + } + + mPK := "monkeys+db4d9f7e+AULaJMvTtDLHPUcUrjdDad9vDlh/PTfC2VV60JUtCfWT" + wSK := "PRIVATE+KEY+witness+f13a86db+AaLa/dfyBhyo/m0Z7WCi98ENVZWtrP8pxgRNrx7tIWiA" + + logV, err := note.NewVerifier(mPK) + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + err = p.AddLogs(t.Context(), []omniwitness.Log{ + {Origin: "monkeys", VKey: mPK, Verifier: logV}, + }) + if err != nil { + t.Fatalf("AddLogs: %v", err) + } + + ns, err := f_note.NewSignerForCosignatureV1(wSK) + if err != nil { + t.Fatalf("NewSignerForCosignatureV1: %v", err) + } + + w, err := witness.New(t.Context(), witness.Opts{ + Persistence: p, + Signers: []note.Signer{ns}, + VerifierForLog: func(ctx context.Context, origin string) (note.Verifier, bool, error) { + l, ok, err := p.Log(ctx, origin) + if err != nil || !ok { + return nil, ok, err + } + return l.Verifier, true, nil + }, + }) + if err != nil { + t.Fatalf("witness.New: %v", err) + } + + mInit := []byte("monkeys\n5\n41smjBUiAU70EtKlT6lIOIYtRTYxYXsDB+XHfcvu/BE=\n\n— monkeys 202fftzGl3LVoqjXfwCFZZXs8I+5G22+Ek2K0AOyBuSJ/8/CZawNF+6fNlTKOCd622pbzJNkkJFWuw9DbicZCkEx9AY=\n") + mNext := []byte("monkeys\n8\nV8K9aklZ4EPB+RMOk1/8VsJUdFZR77GDtZUQq84vSbo=\n\n— monkeys 202ffoUEboiQYpHzICeaFmoy3RNviHTpAxYrq/eO4QQVQMvu9UebKBMX2MJC76NLthZaKsnKbCA8GxrjePZhvDCH7Ag=\n") + + dh := func(h string) []byte { + r, err := hex.DecodeString(h) + if err != nil { + t.Fatal(err) + } + return r + } + consProof := [][]byte{ + dh("b9e1d62618f7fee8034e4c5010f727ab24d8e4705cb296c374bf2025a87a10d2"), + dh("aac66cd7a79ce4012d80762fe8eec3a77f22d1ca4145c3f4cee022e7efcd599d"), + dh("89d0f753f66a290c483b39cd5e9eafb12021293395fad3d4a2ad053cfbcfdc9e"), + dh("29e40bb79c966f4c6fe96aff6f30acfce5f3e8d84c02215175d6e018a5dee833"), + } + + // First update (TOFU) - should succeed. + _, _, err = w.Update(t.Context(), 0, mInit, nil) + if err != nil { + t.Fatalf("First Update (TOFU) failed: %v", err) + } + + // Second update (consistent transition) + ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second) + defer cancel() + + _, _, err = w.Update(ctx, 5, mNext, consProof) + if err != nil { + t.Fatalf("Second Update failed (expected success with fix): %v", err) + } +} diff --git a/witness/witness.go b/witness/witness.go index b9e9406c..f31d4e35 100644 --- a/witness/witness.go +++ b/witness/witness.go @@ -24,6 +24,7 @@ import ( "encoding/binary" "errors" "fmt" + "strconv" "strings" "unicode" "unicode/utf8" @@ -112,24 +113,6 @@ func New(ctx context.Context, wo Opts) (*Witness, error) { }, nil } -// verifyCheckpoint verifies the checkpoint under the appropriate key for the origin and returns -// the parsed checkpoint and the note itself. -func (w *Witness) verifyCheckpoint(ctx context.Context, chkptRaw []byte) (*log.Checkpoint, *note.Note, string, error) { - origin, _, found := strings.Cut(string(chkptRaw), "\n") - if !found { - return nil, nil, "", errors.New("invalid checkpoint") - } - v, ok, err := w.VerifierForLog(ctx, origin) - if err != nil { - return nil, nil, "", err - } - if !ok { - return nil, nil, "", ErrUnknownLog - } - cp, _, n, err := log.ParseCheckpoint(chkptRaw, origin, v) - return cp, n, origin, err -} - // GetCheckpoint gets a checkpoint for a given log, which is consistent with all // other checkpoints for the same log signed by this witness. // @@ -153,7 +136,21 @@ func (w *Witness) Update(ctx context.Context, oldSize uint64, nextRaw []byte, cP // // SPEC: The witness MUST verify the checkpoint signature against the public key(s) it trusts for the // checkpoint origin, and it MUST ignore signatures from unknown keys. - next, nextNote, origin, err := w.verifyCheckpoint(ctx, nextRaw) + next, nextNote, origin, err := func() (*log.Checkpoint, *note.Note, string, error) { + origin, _, found := strings.Cut(string(nextRaw), "\n") + if !found { + return nil, nil, "", errors.New("invalid checkpoint") + } + v, ok, err := w.VerifierForLog(ctx, origin) + if err != nil { + return nil, nil, "", err + } + if !ok { + return nil, nil, "", ErrUnknownLog + } + cp, _, n, err := log.ParseCheckpoint(nextRaw, origin, v) + return cp, n, origin, err + }() if err != nil { return nil, 0, err } @@ -179,33 +176,39 @@ func (w *Witness) Update(ctx context.Context, oldSize uint64, nextRaw []byte, cP return signed, nil } - prev, _, _, err := w.verifyCheckpoint(ctx, prevRaw) + // The persistence layer is within the TCB so we assume that whatever we read is a valid log checkpoint. + // Avoiding revalidating this gives us more flexibility to support log key rotation. + prevOrigin, prevSize, prevHash, err := checkpointUnsafe(prevRaw) if err != nil { retSize, retSigs = 0, nil return nil, fmt.Errorf("couldn't parse stored checkpoint: %v", err) } + if prevOrigin != origin { + retSize, retSigs = 0, nil + return nil, fmt.Errorf("origin didn't match during update. prev=%q, next=%q", prevOrigin, origin) + } // SPEC: The old size MUST be equal to or lower than the (submitted) checkpoint size. if oldSize > next.Size { - retSize, retSigs = prev.Size, nil + retSize, retSigs = prevSize, nil return nil, ErrOldSizeInvalid } // SPEC: The witness MUST check that the old size matches the size of the latest checkpoint it cosigned // for the checkpoint's origin (or zero if it never cosigned a checkpoint for that origin) - if oldSize != prev.Size { - retSize, retSigs = prev.Size, nil - return nil, fmt.Errorf("%w (%d != %d)", ErrCheckpointStale, oldSize, prev.Size) + if oldSize != prevSize { + retSize, retSigs = prevSize, nil + return nil, fmt.Errorf("%w (%d != %d)", ErrCheckpointStale, oldSize, prevSize) } // SPEC: The old size MUST be equal to or lower than the checkpoint size. - if next.Size < prev.Size { - retSize, retSigs = prev.Size, nil + if next.Size < prevSize { + retSize, retSigs = prevSize, nil return nil, ErrOldSizeInvalid } // SPEC: If the old size matches the checkpoint size, the witness MUST check that the root hashes are // also identical. - if next.Size == prev.Size { - if !bytes.Equal(next.Hash, prev.Hash) { - klog.Errorf("%s: INCONSISTENT CHECKPOINTS!:\n%v\n%v", origin, prev, next) + if next.Size == prevSize { + if !bytes.Equal(next.Hash, prevHash) { + klog.Errorf("%s: INCONSISTENT CHECKPOINTS!:\n%v\n%v", origin, prevRaw, next) counterInconsistentCheckpoints.Add(ctx, 1, metric.WithAttributes(originKey.String(origin))) retSize, retSigs = 0, nil @@ -216,7 +219,7 @@ func (w *Witness) Update(ctx context.Context, oldSize uint64, nextRaw []byte, cP } // Checkpoints of size 0 are really placeholders and consistency proofs can't be performed. // If we initialized on a tree size of 0, then we simply ratchet forward and effectively TOFU the new checkpoint. - if prev.Size == 0 { + if prevSize == 0 { // SPEC: The proof MUST be empty if the old size is zero. if len(cProof) > 0 { retSize, retSigs = 0, nil @@ -233,7 +236,7 @@ func (w *Witness) Update(ctx context.Context, oldSize uint64, nextRaw []byte, cP // The only remaining option is next.Size > prev.Size. This might be // valid so we verify the consistency proofs. - if err := proof.VerifyConsistency(rfc6962.DefaultHasher, prev.Size, next.Size, cProof, prev.Hash, next.Hash); err != nil { + if err := proof.VerifyConsistency(rfc6962.DefaultHasher, prevSize, next.Size, cProof, prevHash, next.Hash); err != nil { // Complain if the checkpoints aren't consistent. counterInvalidConsistency.Add(ctx, 1, metric.WithAttributes(originKey.String(origin))) return nil, ErrInvalidProof @@ -317,3 +320,25 @@ func (w *Witness) signChkpt(n *note.Note) ([]byte, []byte, error) { func isValidSignerName(name string) bool { return name != "" && utf8.ValidString(name) && strings.IndexFunc(name, unicode.IsSpace) < 0 && !strings.Contains(name, "+") } + +// checkpointUnsafe parses a checkpoint without performing any signature verification. +// This is intended to be as fast as possible, but sacrifices safety because it skips verifying +// the note signature. +func checkpointUnsafe(rawCp []byte) (string, uint64, []byte, error) { + parts := bytes.SplitN(rawCp, []byte{'\n'}, 4) + if want, got := 4, len(parts); want != got { + return "", 0, nil, fmt.Errorf("invalid checkpoint: %q", rawCp) + } + origin := string(parts[0]) + sizeStr := string(parts[1]) + hashStr := string(parts[2]) + size, err := strconv.ParseUint(sizeStr, 10, 64) + if err != nil { + return "", 0, nil, fmt.Errorf("failed to parse checkpoint size of %q into uint64: %v", sizeStr, err) + } + hash, err := base64.StdEncoding.DecodeString(hashStr) + if err != nil { + return "", 0, nil, fmt.Errorf("failed to decode hash: %v", err) + } + return origin, size, hash, nil +}