Reject CRLF and embedded newlines across C2SP parsers and signers

log/checkpoint.go

@@ -53,6 +53,9 @@ func (c Checkpoint) Marshal() []byte {
 //
 // Any trailing data after this will be returned.
 func (c *Checkpoint) Unmarshal(data []byte) ([]byte, error) {
+	if bytes.IndexByte(data, '\r') >= 0 {
+		return nil, errors.New("invalid checkpoint - contains carriage return")
+	}
 	l := bytes.SplitN(data, []byte("\n"), 4)
 	if len(l) < 4 {
 		return nil, errors.New("invalid checkpoint - too few newlines")

log/checkpoint_test.go

@@ -134,6 +134,22 @@ func TestUnmarshalLogState(t *testing.T) {
 			desc:    "invalid roothash - not base64",
 			m:       "Log\n123\nThisIsn'tBase64\n",
 			wantErr: true,
+		}, {
+			desc:    "invalid - CRLF on origin",
+			m:       "Log\r\n123\nYmFuYW5hcw==\n",
+			wantErr: true,
+		}, {
+			desc:    "invalid - CRLF on size",
+			m:       "Log\n123\r\nYmFuYW5hcw==\n",
+			wantErr: true,
+		}, {
+			desc:    "invalid - CRLF on roothash",
+			m:       "Log\n123\nYmFuYW5hcw==\r\n",
+			wantErr: true,
+		}, {
+			desc:    "invalid - CRLF in trailing data",
+			m:       "Log\n123\nYmFuYW5hcw==\next\r\n",
+			wantErr: true,
 		},
 	} {
 		t.Run(string(test.desc), func(t *testing.T) {

note/note.go

@@ -44,6 +44,9 @@ var (
 // - 0x04 ed25519 Cosignature/V1
 // - 0x06 ML-DSA-44 Cosignature/V1
 func NewSigner(skey string) (Signer, error) {
+	if strings.ContainsAny(skey, "\r\n") {
+		return nil, errSignerID
+	}
 	priv1, k, _ := strings.Cut(skey, "+")
 	priv2, k, _ := strings.Cut(k, "+")
 	name, k, _ := strings.Cut(k, "+")
@@ -127,6 +130,9 @@ func (s *signer) Verifier() note.Verifier {
 
 // NewVerifier returns a verifier for the given key, if the key's algo is known.
 func NewVerifier(key string) (note.Verifier, error) {
+	if strings.ContainsAny(key, "\r\n") {
+		return nil, fmt.Errorf("key contains newline or carriage return: %q", key)
+	}
 	parts := strings.SplitN(key, "+", 3)
 	if got, want := len(parts), 3; got != want {
 		return nil, fmt.Errorf("key has %d parts, expected %d: %q", got, want, key)

note/note_cosigv1.go

@@ -53,6 +53,9 @@ func GenerateMLDSAKey(name string) (skey string, vkey string, err error) {
 
 // NewMLDSASigner returns a signer for MLDSA cosignature v1.
 func NewMLDSASigner(skey string) (SubtreeSigner, error) {
+	if strings.ContainsAny(skey, "\r\n") {
+		return nil, errSignerID
+	}
 	priv1, skey, _ := strings.Cut(skey, "+")
 	priv2, skey, _ := strings.Cut(skey, "+")
 	name, skey, _ := strings.Cut(skey, "+")
@@ -142,6 +145,9 @@ func NewMLDSASignerFromCrypto(name string, signer crypto.Signer) (SubtreeSigner,
 
 // NewMLDSAVerifier constructs a verifier for MLDSA cosignature v1.
 func NewMLDSAVerifier(vkey string) (SubtreeVerifier, error) {
+	if strings.ContainsAny(vkey, "\r\n") {
+		return nil, errVerifierID
+	}
 	name, vkey, _ := strings.Cut(vkey, "+")
 	hash16, key64, _ := strings.Cut(vkey, "+")
 	keyBytes, err := base64.StdEncoding.DecodeString(key64)
@@ -177,6 +183,9 @@ func NewMLDSAVerifier(vkey string) (SubtreeVerifier, error) {
 //
 // See https://c2sp.org/tlog-cosignature for more details.
 func NewSignerForCosignatureV1(skey string) (Signer, error) {
+	if strings.ContainsAny(skey, "\r\n") {
+		return nil, errSignerID
+	}
 	priv1, skey, _ := strings.Cut(skey, "+")
 	priv2, skey, _ := strings.Cut(skey, "+")
 	name, skey, _ := strings.Cut(skey, "+")
@@ -248,6 +257,9 @@ func NewSignerForCosignatureV1(skey string) (Signer, error) {
 // the returned Verifier has a different key hash from a non-timestamped Ed25519
 // verifier key.
 func NewVerifierForCosignatureV1(vkey string) (note.Verifier, error) {
+	if strings.ContainsAny(vkey, "\r\n") {
+		return nil, errVerifierID
+	}
 	name, vkey, _ := strings.Cut(vkey, "+")
 	hash16, key64, _ := strings.Cut(vkey, "+")
 	key, err := base64.StdEncoding.DecodeString(key64)
@@ -288,6 +300,9 @@ func NewVerifierForCosignatureV1(vkey string) (note.Verifier, error) {
 
 // VKeyToCosignatureV1 converts a standard Ed25519 vkey to an Ed25519CosignatureV1 vkey.
 func VKeyToCosignatureV1(vkey string) (string, error) {
+	if strings.ContainsAny(vkey, "\r\n") {
+		return "", errVerifierID
+	}
 	name, vkey, _ := strings.Cut(vkey, "+")
 	hash16, key64, _ := strings.Cut(vkey, "+")
 	algKey, err := base64.StdEncoding.DecodeString(key64)
@@ -326,6 +341,9 @@ func CoSigV1Timestamp(s note.Signature) (time.Time, error) {
 
 // CosignatureTimestamp returns the timestamp embedded in a signature from a CosignatureV1 signer.
 func CosignatureTimestamp(s note.Signature) (uint64, error) {
+	if strings.ContainsAny(s.Base64, "\r\n") {
+		return 0, errMalformedSig
+	}
 	r, err := base64.StdEncoding.DecodeString(s.Base64)
 	if err != nil {
 		return 0, errMalformedSig
@@ -421,6 +439,9 @@ func formatEd25519CosignatureV1(t uint64, msg []byte) ([]byte, error) {
 	//
 	// See https://c2sp.org/tlog-cosignature for more details.
 
+	if bytes.IndexByte(msg, '\r') >= 0 {
+		return nil, errors.New("cosigned note contains carriage return")
+	}
 	if lines := bytes.Split(msg, []byte("\n")); len(lines) < 3 {
 		return nil, errors.New("cosigned note format invalid")
 	}
@@ -443,7 +464,8 @@ func formatMLDSACosignatureV1(cosignerName string, timestamp uint64, logOrigin s
 	if !isSubtreeValid(start, end) {
 		return nil, errInvalidPayload
 	}
-	if lo, lc := len(logOrigin), len(cosignerName); lo == 0 || lo > 255 || lc == 0 || lc > 255 {
+	if lo, lc := len(logOrigin), len(cosignerName); lo == 0 || lo > 255 || lc == 0 || lc > 255 ||
+		strings.ContainsAny(logOrigin, "\r\n") || strings.ContainsAny(cosignerName, "\r\n") {
 		return nil, errSignerID
 	}
 	if len(root) != sha256.Size {

note/note_cosigv1_test.go

@@ -770,3 +770,96 @@ func mustECDSASigner(t *testing.T) crypto.Signer {
 	}
 	return ecdsaK
 }
+
+func TestCosigV1_RejectCRLF(t *testing.T) {
+	edSk, edVk := mustGenerateEd25519Key(t, "ed25519")
+	mlSk, mlVk := mustGenerateMLDSAKey(t, "mldsa")
+
+	embedCRLF := func(s string) string {
+		return s[:len(s)-4] + "\r\n" + s[len(s)-4:]
+	}
+	for _, sk := range []string{edSk + "\r\n", edSk + "\r", edSk + "\n", embedCRLF(edSk), mlSk + "\r\n", mlSk + "\r", mlSk + "\n", embedCRLF(mlSk)} {
+		if _, err := NewSignerForCosignatureV1(sk); err == nil {
+			t.Errorf("NewSignerForCosignatureV1(%q) succeeded, want error", sk)
+		}
+	}
+	for _, sk := range []string{mlSk + "\r\n", mlSk + "\r", mlSk + "\n", embedCRLF(mlSk)} {
+		if _, err := NewMLDSASigner(sk); err == nil {
+			t.Errorf("NewMLDSASigner(%q) succeeded, want error", sk)
+		}
+	}
+	for _, vk := range []string{edVk + "\r\n", edVk + "\r", edVk + "\n", embedCRLF(edVk), mlVk + "\r\n", mlVk + "\r", mlVk + "\n", embedCRLF(mlVk)} {
+		if _, err := NewVerifierForCosignatureV1(vk); err == nil {
+			t.Errorf("NewVerifierForCosignatureV1(%q) succeeded, want error", vk)
+		}
+	}
+	for _, vk := range []string{mlVk + "\r\n", mlVk + "\r", mlVk + "\n", embedCRLF(mlVk)} {
+		if _, err := NewMLDSAVerifier(vk); err == nil {
+			t.Errorf("NewMLDSAVerifier(%q) succeeded, want error", vk)
+		}
+	}
+	for _, vk := range []string{edVk + "\r\n", edVk + "\r", edVk + "\n", embedCRLF(edVk)} {
+		if _, err := VKeyToCosignatureV1(vk); err == nil {
+			t.Errorf("VKeyToCosignatureV1(%q) succeeded, want error", vk)
+		}
+	}
+
+	validSig := "ZGhGuQAAAABm/qTPeyKXD+R2rzyQsxPiP8mXum7qq/iF0u4vanlqJyocWODBt97w9uL+8qT7S5gxEHWWOworDcFiEBYJXORmnFBOBA=="
+	for _, b64 := range []string{validSig + "\r\n", validSig + "\r", validSig + "\n", embedCRLF(validSig)} {
+		if _, err := CosignatureTimestamp(note.Signature{Base64: b64}); err == nil {
+			t.Errorf("CosignatureTimestamp(%q) succeeded, want error", b64)
+		}
+		if _, err := CoSigV1Timestamp(note.Signature{Base64: b64}); err == nil {
+			t.Errorf("CoSigV1Timestamp(%q) succeeded, want error", b64)
+		}
+	}
+
+	edSigner, err := NewSignerForCosignatureV1(edSk)
+	if err != nil {
+		t.Fatal(err)
+	}
+	mlSigner, err := NewMLDSASigner(mlSk)
+	if err != nil {
+		t.Fatal(err)
+	}
+	validMsg := []byte("test\n123\nf+7CoKgXKE/tNys9TTXcr/ad6U/K3xvznmzew9y6SP0=\n")
+	crlfMsg := []byte("test\n123\nf+7CoKgXKE/tNys9TTXcr/ad6U/K3xvznmzew9y6SP0=\r\n")
+	for _, tc := range []struct {
+		name string
+		sign func([]byte) ([]byte, error)
+		ver  note.Verifier
+	}{
+		{"ed25519", edSigner.Sign, edSigner.Verifier()},
+		{"mldsa44", mlSigner.Sign, mlSigner.Verifier()},
+	} {
+		if _, err := tc.sign(crlfMsg); err == nil {
+			t.Errorf("%s.Sign() succeeded on CRLF checkpoint body, want error", tc.name)
+		}
+		sig, err := tc.sign(validMsg)
+		if err != nil {
+			t.Fatalf("%s.Sign(validMsg): %v", tc.name, err)
+		}
+		if tc.ver.Verify(crlfMsg, sig) {
+			t.Errorf("%s.Verifier().Verify() = true on CRLF checkpoint body, want false", tc.name)
+		}
+	}
+
+	root := make([]byte, 32)
+	validSubtreeSig, err := mlSigner.SignSubtree("test", 0, 10, root)
+	if err != nil {
+		t.Fatalf("SignSubtree: %v", err)
+	}
+	for _, badOrigin := range []string{"test\r", "test\n", "test\r\n"} {
+		if _, err := mlSigner.SignSubtree(badOrigin, 0, 10, root); err == nil {
+			t.Errorf("SignSubtree(%q) succeeded, want error", badOrigin)
+		}
+		if mlSigner.Verifier().VerifySubtree(badOrigin, 0, 10, root, validSubtreeSig) {
+			t.Errorf("VerifySubtree(%q) = true, want false", badOrigin)
+		}
+	}
+	for _, badName := range []string{"wit\r", "wit\n", "wit\r\n"} {
+		if _, err := formatMLDSACosignatureV1(badName, 123, "test", 0, 10, root); err == nil {
+			t.Errorf("formatMLDSACosignatureV1(cosignerName=%q) succeeded, want error", badName)
+		}
+	}
+}

note/note_rfc6962.go

@@ -15,6 +15,7 @@
 package note
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/ecdsa"
 	"crypto/sha256"
@@ -50,6 +51,9 @@ func RFC6962VerifierString(logURL string, pubK crypto.PublicKey) (string, error)
 
 // NewRFC6962Verifier creates a note verifier for Static CT/RFC6962 checkpoint signatures.
 func NewRFC6962Verifier(vkey string) (note.Verifier, error) {
+	if strings.ContainsAny(vkey, "\r\n") {
+		return nil, errVerifierID
+	}
 	name, vkey, _ := strings.Cut(vkey, "+")
 	hash16, key64, _ := strings.Cut(vkey, "+")
 	key, err := base64.StdEncoding.DecodeString(key64)
@@ -113,6 +117,9 @@ func RFC6962STHToCheckpoint(j []byte, v note.Verifier) ([]byte, error) {
 
 // RFC6962STHTimestamp extracts the embedded timestamp from a translated RFC6962 STH signature.
 func RFC6962STHTimestamp(s note.Signature) (time.Time, error) {
+	if strings.ContainsAny(s.Base64, "\r\n") {
+		return time.UnixMilli(0), errMalformedSig
+	}
 	r, err := base64.StdEncoding.DecodeString(s.Base64)
 	if err != nil {
 		return time.UnixMilli(0), errMalformedSig
@@ -224,6 +231,9 @@ func formatRFC6962STH(t uint64, msg []byte) (string, []byte, error) {
 	// origin (schema-less log root url) "\n"
 	// tree size (decimal) "\n"
 	// root hash (b64) "\n"
+	if bytes.IndexByte(msg, '\r') >= 0 {
+		return "", nil, errors.New("checkpoint contains carriage return")
+	}
 	lines := strings.Split(string(msg), "\n")
 	if len(lines) != 4 {
 		return "", nil, errors.New("wrong number of lines")

note/note_rfc6962_test.go

@@ -193,3 +193,34 @@ func mustB64(t *testing.T, s string) []byte {
 	}
 	return b
 }
+
+func TestRFC6962_RejectCRLF(t *testing.T) {
+	vkey := "rome.ct.filippo.io/2024h1+78f4abae+BTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAFzPC3fap+uINc1RQ4eRbYMUt84+bKkA8GDLN8KwVdzAgYhYSv4kS8XSheGLAHCWhIJTJbuC3sL88bNMTtrsBM="
+	embedCRLF := func(s string) string {
+		return s[:len(s)-4] + "\r\n" + s[len(s)-4:]
+	}
+	for _, vk := range []string{vkey + "\r\n", vkey + "\r", vkey + "\n", embedCRLF(vkey)} {
+		if _, err := NewRFC6962Verifier(vk); err == nil {
+			t.Errorf("NewRFC6962Verifier(%q) succeeded, want error", vk)
+		}
+	}
+
+	v, err := NewRFC6962Verifier(vkey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	n, err := note.Open([]byte(romeCP), note.VerifierList(v))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, b64 := range []string{n.Sigs[0].Base64 + "\r\n", n.Sigs[0].Base64 + "\r", n.Sigs[0].Base64 + "\n", embedCRLF(n.Sigs[0].Base64)} {
+		if _, err := RFC6962STHTimestamp(note.Signature{Base64: b64}); err == nil {
+			t.Errorf("RFC6962STHTimestamp(%q) succeeded, want error", b64)
+		}
+	}
+	rawSig := mustB64(t, n.Sigs[0].Base64)[keyHashSize:]
+	crlfBody := []byte("rome.ct.filippo.io/2024h1\n115474666\n2q1K6aiIJR+F7TyhiWOghoWOjY0/3dVBLsBbAvB4xCw=\r\n")
+	if v.Verify(crlfBody, rawSig) {
+		t.Error("RFC6962 Verifier.Verify() = true with CRLF on root hash line, want false")
+	}
+}

note/note_test.go

@@ -92,3 +92,28 @@ func TestNoteInterop(t *testing.T) {
 		}
 	}
 }
+
+func TestNewSignerVerifier_RejectCRLF(t *testing.T) {
+	ns, nv, err := note.GenerateKey(nil, "note")
+	if err != nil {
+		t.Fatalf("note.GenerateKey: %v", err)
+	}
+	ms, mv, err := GenerateMLDSAKey("mldsa")
+	if err != nil {
+		t.Fatalf("GenerateMLDSAKey: %v", err)
+	}
+
+	embedCRLF := func(s string) string {
+		return s[:len(s)-4] + "\r\n" + s[len(s)-4:]
+	}
+	for _, s := range []string{ns + "\r\n", ns + "\r", ns + "\n", embedCRLF(ns), ms + "\r\n", ms + "\r", ms + "\n", embedCRLF(ms)} {
+		if _, err := NewSigner(s); err == nil {
+			t.Errorf("NewSigner(%q) succeeded, want error", s)
+		}
+	}
+	for _, v := range []string{nv + "\r\n", nv + "\r", nv + "\n", embedCRLF(nv), mv + "\r\n", mv + "\r", mv + "\n", embedCRLF(mv)} {
+		if _, err := NewVerifier(v); err == nil {
+			t.Errorf("NewVerifier(%q) succeeded, want error", v)
+		}
+	}
+}

note/note_verifier.go

@@ -34,6 +34,9 @@ import (
 // Ed25519 private key in the standard note-formatted form, e.g.
 // `PRIVATE+KEY+logandmap+38581672+AXJ0FKWOcO2ch6WC8kP705Ed3Gxu7pVtZLhfHAQwp+FE`.
 func NewEd25519SignerVerifier(skey string) (note.Signer, note.Verifier, error) {
+	if strings.ContainsAny(skey, "\r\n") {
+		return nil, nil, errors.New("key contains newline or carriage return")
+	}
 	const algEd25519 = 1
 	s, err := note.NewSigner(skey)
 	if err != nil {
@@ -89,6 +92,9 @@ func NewEd25519SignerVerifier(skey string) (note.Signer, note.Verifier, error) {
 //
 //	"rekor.sigstore.dev+12345678+AjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABNhtmPtrWm3U1eQXBogSMdGvXwBcK5AW5i0hrZLOC96l+smGNM7nwZ4QvFK/4sueRoVj//QP22Ni4Qt9DPfkWLc=
 func NewECDSAVerifier(key string) (note.Verifier, error) {
+	if strings.ContainsAny(key, "\r\n") {
+		return nil, fmt.Errorf("key contains newline or carriage return: %q", key)
+	}
 	parts := strings.SplitN(key, "+", 3)
 	if got, want := len(parts), 3; got != want {
 		return nil, fmt.Errorf("key has %d parts, expected %d: %q", got, want, key)

note/note_verifier_test.go

@@ -176,3 +176,20 @@ func TestECDSAVerifier(t *testing.T) {
 		})
 	}
 }
+
+func TestNoteVerifier_RejectCRLF(t *testing.T) {
+	edPriv := "PRIVATE+KEY+logandmap+38581672+AXJ0FKWOcO2ch6WC8kP705Ed3Gxu7pVtZLhfHAQwp+FE"
+	embedCRLF := func(s string) string {
+		return s[:len(s)-4] + "\r\n" + s[len(s)-4:]
+	}
+	for _, k := range []string{edPriv + "\r\n", edPriv + "\r", edPriv + "\n", embedCRLF(edPriv)} {
+		if _, _, err := NewEd25519SignerVerifier(k); err == nil {
+			t.Errorf("NewEd25519SignerVerifier(%q) succeeded, want error", k)
+		}
+	}
+	for _, k := range []string{sigStoreKey + "\r\n", sigStoreKey + "\r", sigStoreKey + "\n", embedCRLF(sigStoreKey)} {
+		if _, err := NewECDSAVerifier(k); err == nil {
+			t.Errorf("NewECDSAVerifier(%q) succeeded, want error", k)
+		}
+	}
+}

proof/tlog_proof.go

@@ -57,6 +57,9 @@ func (p TLogProof) Marshal() []byte {
 }
 
 func (p *TLogProof) Unmarshal(data []byte) error {
+	if bytes.IndexByte(data, '\r') >= 0 {
+		return fmt.Errorf("tlog proof contains carriage return")
+	}
 	var err error
 	b := bufio.NewScanner(bytes.NewReader(data))