diff --git a/log/checkpoint.go b/log/checkpoint.go index c7555b6..a434a8a 100644 --- a/log/checkpoint.go +++ b/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") diff --git a/log/checkpoint_test.go b/log/checkpoint_test.go index 0549b11..6d453cb 100644 --- a/log/checkpoint_test.go +++ b/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) { diff --git a/note/note.go b/note/note.go index 149e2c2..ab415fe 100644 --- a/note/note.go +++ b/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) diff --git a/note/note_cosigv1.go b/note/note_cosigv1.go index ee90db8..db8fc59 100644 --- a/note/note_cosigv1.go +++ b/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 { diff --git a/note/note_cosigv1_test.go b/note/note_cosigv1_test.go index 9dbfa29..ab723b4 100644 --- a/note/note_cosigv1_test.go +++ b/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) + } + } +} diff --git a/note/note_rfc6962.go b/note/note_rfc6962.go index 7ed3a66..ab504f1 100644 --- a/note/note_rfc6962.go +++ b/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") diff --git a/note/note_rfc6962_test.go b/note/note_rfc6962_test.go index 5877fc9..5f9390d 100644 --- a/note/note_rfc6962_test.go +++ b/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") + } +} diff --git a/note/note_test.go b/note/note_test.go index fd380f3..39e52b6 100644 --- a/note/note_test.go +++ b/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) + } + } +} diff --git a/note/note_verifier.go b/note/note_verifier.go index 78efe62..f71c30f 100644 --- a/note/note_verifier.go +++ b/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) diff --git a/note/note_verifier_test.go b/note/note_verifier_test.go index 771468f..54fcdf9 100644 --- a/note/note_verifier_test.go +++ b/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) + } + } +} diff --git a/proof/tlog_proof.go b/proof/tlog_proof.go index 6061c7c..ecb75e4 100644 --- a/proof/tlog_proof.go +++ b/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)) diff --git a/proof/tlog_proof_test.go b/proof/tlog_proof_test.go index 4045742..0a06755 100644 --- a/proof/tlog_proof_test.go +++ b/proof/tlog_proof_test.go @@ -134,6 +134,37 @@ func TestUnmarshalErrors(t *testing.T) { proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\n" + strings.Repeat("a", 65*1024) + "\n"), wantErrSubstr: "scanning tlog proof", }, + { + name: "CRLF on header line", + proof: []byte("c2sp.org/tlog-proof@v1\r\nindex 0\n\ncheckpoint\n"), + wantErrSubstr: "carriage return", + }, + { + name: "CRLF on extra line", + proof: []byte("c2sp.org/tlog-proof@v1\nextra aGVsbG8=\r\nindex 0\n\ncheckpoint\n"), + wantErrSubstr: "carriage return", + }, + { + name: "CRLF on index line", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\r\n\ncheckpoint\n"), + wantErrSubstr: "carriage return", + }, + { + name: "CRLF on hash line", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\n" + + base64.StdEncoding.EncodeToString(make([]byte, 32)) + "\r\n\ncheckpoint\n"), + wantErrSubstr: "carriage return", + }, + { + name: "CRLF on separator line", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\n\r\ncheckpoint\n"), + wantErrSubstr: "carriage return", + }, + { + name: "CRLF in checkpoint", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 0\n\ncheckpoint\r\n"), + wantErrSubstr: "carriage return", + }, } for _, tt := range tests { diff --git a/witness/witness.go b/witness/witness.go index 55f4a90..f9275dd 100644 --- a/witness/witness.go +++ b/witness/witness.go @@ -49,6 +49,9 @@ type policyComponent interface { // but with the difference that the configured witness keys MUST be signature type `0x04` `vkey`s as specified // by C2SP [signed-note](https://github.com/C2SP/C2SP/blob/main/signed-note.md#verifier-keys). func ParsePolicy(p []byte) (Group, error) { + if bytes.IndexByte(p, '\r') >= 0 { + return Group{}, fmt.Errorf("invalid policy: contains carriage return") + } scanner := bufio.NewScanner(bytes.NewBuffer(p)) components := make(map[string]policyComponent) diff --git a/witness/witness_policy_test.go b/witness/witness_policy_test.go index 0b8ffc1..23f3575 100644 --- a/witness/witness_policy_test.go +++ b/witness/witness_policy_test.go @@ -160,6 +160,11 @@ func TestParsePolicy_Errors(t *testing.T) { policy: `group none 1 witness`, errStr: "invalid group name", }, + { + desc: "carriage return in policy", + policy: "witness w1 sigsum.org+e4ade967+AZuUY6B08pW3QVHu8uvsrxWPcAv9nykap2Nb4oxCee+r https://sigsum.org/witness/\r\nquorum w1\r\n", + errStr: "carriage return", + }, } for _, tc := range testCases { diff --git a/witness/witness_test.go b/witness/witness_test.go index ca0b2f6..bdab69b 100644 --- a/witness/witness_test.go +++ b/witness/witness_test.go @@ -224,3 +224,11 @@ func BenchmarkGroupSatisfaction(b *testing.B) { } } } + +func TestNew_RejectsCRLF(t *testing.T) { + for _, vk := range []string{wit1_vkey + "\r\n", wit1_vkey + "\r", wit1_vkey + "\n"} { + if _, err := New(vk, directURL); err == nil { + t.Errorf("New(%q) succeeded, want error", vk) + } + } +}