policy: verify every known signature; proof: decode base64 strictly

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

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,14 +637,59 @@ 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",
 			policy:  fmt.Sprintf("witness w1 %s\nquorum w1\n", wit1_vkey),
 			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{}

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)
 		}

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"),