From 8b04f1fcf3bd83d8638739973980cddf5eea0136 Mon Sep 17 00:00:00 2001 From: Ben Birt Date: Tue, 6 Oct 2026 15:25:59 +0000 Subject: [PATCH] policy: verify every known signature; proof: decode base64 strictly TLogPolicy.Verify used to stop at the first log key whose signature verified, and Satisfied opened the note once per witness, stopping as soon as the quorum was met. Signatures from other known keys were never looked at, so a checkpoint carrying an invalid signature from a known log or witness could still be accepted. c2sp.org/signed-note says that if a signature's key name and ID match a known key, verifiers MUST verify it, and SHOULD reject the whole note if it fails; c2sp.org/tlog-proof likewise says to "verify all cosignatures for witnesses known to the verifier". Verify now opens the checkpoint once with all of the policy's log and witness keys, so note.Open checks every known signature and fails on any invalid one, then applies the log-origin and quorum rules to the verified signatures. Satisfied does the same with the witness keys. Signatures from unknown keys are still ignored. Separately, c2sp.org/tlog-proof requires decoders to reject non-canonical base64, so TLogProof.Unmarshal now decodes the extra data and inclusion hashes with base64.StdEncoding.Strict(), which rejects non-zero padding bits. Other formats (checkpoints, vkeys, signed notes) are left unchanged, as their specs don't require this. --- policy/tlog_policy.go | 124 +++++++++++++++++++++++++++++-------- policy/tlog_policy_test.go | 83 ++++++++++++++++++++++++- proof/tlog_proof.go | 4 +- proof/tlog_proof_test.go | 12 ++++ 4 files changed, 193 insertions(+), 30 deletions(-) diff --git a/policy/tlog_policy.go b/policy/tlog_policy.go index 3c3215a..eebf8ab 100644 --- a/policy/tlog_policy.go +++ b/policy/tlog_policy.go @@ -20,8 +20,10 @@ import ( "bufio" "bytes" "encoding/base64" + "errors" "fmt" "net/url" + "slices" "strconv" "strings" @@ -307,13 +309,98 @@ func (p TLogPolicy) Marshal() []byte { // Satisfied returns true if the checkpoint provided is cosigned by // witnesses according to the policy's quorum rule. -// This will return false if there are insufficient cosignatures, and also -// if the checkpoint cannot be read as a valid note. It is up to the caller -// to ensure that the input value represents a valid note. +// This will return false if there are insufficient cosignatures, if any +// cosignature from a witness in the policy is invalid, and also if the +// checkpoint cannot be read as a valid note. It is up to the caller to +// ensure that the input value represents a valid note. // // Note that Satisfied does not require a log signature; use Verify to // apply the policy's full checkpoint validity rule. func (p TLogPolicy) Satisfied(checkpoint []byte) bool { + if p.Quorum == quorumNone { + return true + } + vs := make([]note.Verifier, 0, len(p.Witnesses)) + for _, w := range p.Witnesses { + vs = append(vs, w.Verifier) + } + n, err := note.Open(checkpoint, note.VerifierList(vs...)) + if err != nil { + return false + } + return p.quorumMet(n.Sigs) +} + +// Verify applies the policy's checkpoint validity rule: the provided +// checkpoint must be a note signed by any one of the policy's logs, with +// an origin line matching that log's key name, and it must be cosigned by +// witnesses according to the quorum rule. The parsed checkpoint is +// returned if it is valid. +// +// As https://c2sp.org/signed-note requires, every signature from a key the +// policy knows, whether a log's or a witness', is verified. The checkpoint +// is rejected if any of them is invalid, even one which the validity rule +// could do without. Signatures from keys the policy does not know are +// ignored. +// +// A policy with an empty set of logs cannot validate any checkpoint; if +// the applicable log is known from other context, verify its signature +// separately and use Satisfied for the quorum rule. +func (p TLogPolicy) Verify(checkpoint []byte) (*f_log.Checkpoint, error) { + if len(p.Logs) == 0 { + return nil, fmt.Errorf("policy defines no logs") + } + vs := make([]note.Verifier, 0, len(p.Logs)+len(p.Witnesses)) + for _, l := range p.Logs { + vs = append(vs, l.Verifier) + } + for _, w := range p.Witnesses { + vs = append(vs, w.Verifier) + } + n, err := note.Open(checkpoint, note.VerifierList(vs...)) + var unverified *note.UnverifiedNoteError + switch { + case errors.As(err, &unverified): + // No signature from any key in the policy. That is not an error in + // itself: carry on with no verified signatures, and let the rules + // below report which requirement is unmet. + n = unverified.Note + case err != nil: + return nil, fmt.Errorf("verifying checkpoint signatures: %w", err) + } + logSigners := p.logSigners(n.Sigs) + if len(logSigners) == 0 { + return nil, fmt.Errorf("checkpoint is not signed by any log in the policy") + } + cp := &f_log.Checkpoint{} + if _, err := cp.Unmarshal([]byte(n.Text)); err != nil { + return nil, fmt.Errorf("parsing checkpoint: %w", err) + } + if !slices.Contains(logSigners, cp.Origin) { + return nil, fmt.Errorf("checkpoint origin %q does not match any log that signed it (%s)", + cp.Origin, strings.Join(logSigners, ", ")) + } + if !p.quorumMet(n.Sigs) { + return nil, fmt.Errorf("checkpoint does not satisfy quorum %q", p.Quorum) + } + return cp, nil +} + +// logSigners returns the key names of the policy logs that have a +// signature in sigs, which must already be verified. +func (p TLogPolicy) logSigners(sigs []note.Signature) []string { + var names []string + for _, l := range p.Logs { + if hasSig(sigs, l.Verifier) { + names = append(names, l.Verifier.Name()) + } + } + return names +} + +// quorumMet reports whether sigs, which must already be verified, satisfy +// the policy's quorum rule. +func (p TLogPolicy) quorumMet(sigs []note.Signature) bool { if p.Quorum == quorumNone { return true } @@ -330,8 +417,7 @@ func (p TLogPolicy) Satisfied(checkpoint []byte) bool { var satisfied func(string) bool satisfied = func(name string) bool { if w, ok := witnesses[name]; ok { - n, err := note.Open(checkpoint, note.VerifierList(w.Verifier)) - return err == nil && len(n.Sigs) == 1 + return hasSig(sigs, w.Verifier) } g, ok := groups[name] // Unknown names and cyclic references (neither of which can occur @@ -355,30 +441,14 @@ func (p TLogPolicy) Satisfied(checkpoint []byte) bool { return satisfied(p.Quorum) } -// Verify applies the policy's checkpoint validity rule: the provided -// checkpoint must be a note signed by any one of the policy's logs, with -// an origin line matching that log's key name, and it must be cosigned by -// witnesses according to the quorum rule. The parsed checkpoint is -// returned if it is valid. -// -// A policy with an empty set of logs cannot validate any checkpoint; if -// the applicable log is known from other context, verify its signature -// separately and use Satisfied for the quorum rule. -func (p TLogPolicy) Verify(checkpoint []byte) (*f_log.Checkpoint, error) { - if len(p.Logs) == 0 { - return nil, fmt.Errorf("policy defines no logs") - } - for _, l := range p.Logs { - cp, _, _, err := f_log.ParseCheckpoint(checkpoint, l.Verifier.Name(), l.Verifier) - if err != nil { - continue - } - if !p.Satisfied(checkpoint) { - return nil, fmt.Errorf("checkpoint does not satisfy quorum %q", p.Quorum) +// hasSig reports whether sigs includes a signature by v's key. +func hasSig(sigs []note.Signature, v note.Verifier) bool { + for _, s := range sigs { + if s.Name == v.Name() && s.Hash == v.KeyHash() { + return true } - return cp, nil } - return nil, fmt.Errorf("checkpoint is not signed by any log in the policy") + return false } // checkCharset returns an error if data contains an octet not permitted by diff --git a/policy/tlog_policy_test.go b/policy/tlog_policy_test.go index 91b689f..3708e5e 100644 --- a/policy/tlog_policy_test.go +++ b/policy/tlog_policy_test.go @@ -459,6 +459,32 @@ func signedCheckpoint(t *testing.T, signers ...note.Signer) []byte { return cp } +// forger signs as the key behind a verifier, but its signatures are all +// zeroes, so they never verify. +type forger struct { + note.Verifier + sigSize int +} + +func (f forger) Sign([]byte) ([]byte, error) { return make([]byte, f.sigSize), nil } + +// forge returns a forger for vkey, whose signatures are sigSize bytes. +func forge(t *testing.T, vkey string, sigSize int) note.Signer { + t.Helper() + v, err := f_note.NewVerifier(vkey) + if err != nil { + t.Fatalf("NewVerifier(%q) failed: %v", vkey, err) + } + return forger{Verifier: v, sigSize: sigSize} +} + +// Signature sizes for forgers: an Ed25519 signature, and a +// cosignature/v1 timestamp plus Ed25519 signature. +const ( + ed25519SigSize = 64 + cosigV1SigSize = 72 +) + func TestSatisfied(t *testing.T) { policy := fmt.Sprintf(`witness w1 %s witness w2 %s @@ -502,6 +528,16 @@ quorum outer signers: []note.Signer{}, expectSatisfied: false, }, + { + desc: "quorum met, with an unknown witness' cosignature too", + signers: []note.Signer{wit1Sign, wit3Sign, forge(t, wit4_vkey, cosigV1SigSize)}, + expectSatisfied: true, + }, + { + desc: "quorum met, but a known witness' cosignature is invalid", + signers: []note.Signer{wit1Sign, wit3Sign, forge(t, wit2_vkey, cosigV1SigSize)}, + expectSatisfied: false, + }, } { t.Run(tc.desc, func(t *testing.T) { cp := signedCheckpoint(t, tc.signers...) @@ -601,7 +637,7 @@ func TestVerify(t *testing.T) { desc: "log key name must match checkpoint origin", policy: fmt.Sprintf("log %s\nwitness w1 %s\nquorum w1\n", otherVkey, wit1_vkey), signers: []note.Signer{otherSign, wit1Sign}, - errStr: "not signed by any log", + errStr: `origin "` + testOrigin + `" does not match any log that signed it (other.example.com/log)`, }, { desc: "policy without logs", @@ -609,6 +645,51 @@ func TestVerify(t *testing.T) { signers: []note.Signer{logSign, wit1Sign}, errStr: "no logs", }, + { + desc: "no signature from any known key", + policy: fmt.Sprintf("log %s\nwitness w1 %s\nquorum w1\n", logVkey, wit1_vkey), + signers: []note.Signer{wit2Sign}, + errStr: "not signed by any log", + }, + { + desc: "signatures from unknown keys are ignored, even if invalid", + policy: fmt.Sprintf("log %s\nwitness w1 %s\nquorum w1\n", logVkey, wit1_vkey), + signers: []note.Signer{logSign, wit1Sign, wit2Sign, forge(t, wit4_vkey, cosigV1SigSize)}, + }, + { + desc: "invalid log signature", + policy: fmt.Sprintf("log %s\nwitness w1 %s\nquorum w1\n", logVkey, wit1_vkey), + signers: []note.Signer{forge(t, logVkey, ed25519SigSize), wit1Sign}, + errStr: "invalid signature", + }, + { + desc: "invalid signature from a second known log", + policy: fmt.Sprintf("log %s\nlog %s\nwitness w1 %s\nquorum w1\n", + logVkey, otherVkey, wit1_vkey), + signers: []note.Signer{logSign, forge(t, otherVkey, ed25519SigSize), wit1Sign}, + errStr: "invalid signature", + }, + { + desc: "invalid cosignature from a known witness outside the quorum", + policy: fmt.Sprintf("log %s\nwitness w1 %s\nwitness w2 %s\nquorum w1\n", + logVkey, wit1_vkey, wit2_vkey), + signers: []note.Signer{logSign, wit1Sign, forge(t, wit2_vkey, cosigV1SigSize)}, + errStr: "invalid signature", + }, + { + desc: "invalid cosignature from a known witness the quorum can do without", + policy: fmt.Sprintf("log %s\nwitness w1 %s\nwitness w2 %s\ngroup g any w1 w2\nquorum g\n", + logVkey, wit1_vkey, wit2_vkey), + signers: []note.Signer{logSign, wit1Sign, forge(t, wit2_vkey, cosigV1SigSize)}, + errStr: "invalid signature", + }, + { + desc: "invalid cosignature from a known witness under quorum none", + policy: fmt.Sprintf("log %s\nwitness w1 %s\nquorum none\n", + logVkey, wit1_vkey), + signers: []note.Signer{logSign, forge(t, wit1_vkey, cosigV1SigSize)}, + errStr: "invalid signature", + }, } { t.Run(tc.desc, func(t *testing.T) { p := &TLogPolicy{} diff --git a/proof/tlog_proof.go b/proof/tlog_proof.go index a40b34b..6061c7c 100644 --- a/proof/tlog_proof.go +++ b/proof/tlog_proof.go @@ -68,7 +68,7 @@ func (p *TLogProof) Unmarshal(data []byte) error { var extra []byte if b.Scan(); strings.HasPrefix(b.Text(), "extra ") { e, _ := strings.CutPrefix(b.Text(), "extra ") - extra, err = base64.StdEncoding.DecodeString(e) + extra, err = base64.StdEncoding.Strict().DecodeString(e) if err != nil { return fmt.Errorf("tlog proof extra data not base64 encoded: %w", err) } @@ -90,7 +90,7 @@ func (p *TLogProof) Unmarshal(data []byte) error { if b.Text() == "" { break } - hash, err := base64.StdEncoding.DecodeString(b.Text()) + hash, err := base64.StdEncoding.Strict().DecodeString(b.Text()) if err != nil { return fmt.Errorf("tlog proof hash not base64 encoded: %w", err) } diff --git a/proof/tlog_proof_test.go b/proof/tlog_proof_test.go index dd3400b..4045742 100644 --- a/proof/tlog_proof_test.go +++ b/proof/tlog_proof_test.go @@ -117,6 +117,18 @@ func TestUnmarshalErrors(t *testing.T) { base64.StdEncoding.EncodeToString(make([]byte, 64)) + "\n\ncheckpoint\n"), wantErrSubstr: "hash length", }, + { + // "aGVsbG8=" ("hello") with non-zero padding bits. + name: "non-canonical extra data encoding", + proof: []byte("c2sp.org/tlog-proof@v1\nextra aGVsbG9=\nindex 0\n\ncheckpoint\n"), + wantErrSubstr: "extra data not base64 encoded", + }, + { + // The encoding of 32 zero bytes, with non-zero padding bits. + name: "non-canonical hash encoding", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\n" + strings.Repeat("A", 42) + "B=\n\ncheckpoint\n"), + wantErrSubstr: "hash not base64 encoded", + }, { name: "scanner error - buffer too large", proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\n" + strings.Repeat("a", 65*1024) + "\n"),