diff --git a/note/note.go b/note/note.go index 0098827..c331abf 100644 --- a/note/note.go +++ b/note/note.go @@ -64,7 +64,7 @@ func NewSigner(skey string) (Signer, error) { // This has the benefit that all signers from this package carry a Verifier() getter. // Adapted from sumdb/note/note.go - hash, err := strconv.ParseUint(hash16, 16, 32) + hash, err := parseKeyHash(hash16) if err != nil { return nil, errSignerID } @@ -74,14 +74,14 @@ func NewSigner(skey string) (Signer, error) { s := &signer{ name: name, - hash: uint32(hash), + hash: hash, } if len(key) != 32 { return nil, errSignerID } key = ed25519.NewKeyFromSeed(key) pubkey := append([]byte{algEd25519}, key[32:]...) - if uint32(hash) != keyHashEd25519(name, pubkey) { + if hash != keyHashEd25519(name, pubkey) { return nil, errSignerHash } @@ -137,6 +137,9 @@ func NewVerifier(key string) (note.Verifier, error) { if got, want := len(parts), 3; got != want { return nil, fmt.Errorf("key has %d parts, expected %d: %q", got, want, key) } + if _, err := parseKeyHash(parts[1]); err != nil { + return nil, fmt.Errorf("key has invalid keyhash %q: %v", parts[1], err) + } keyBytes, err := base64.StdEncoding.Strict().DecodeString(parts[2]) if err != nil { return nil, fmt.Errorf("key has invalid base64 %q: %v", parts[2], err) @@ -178,3 +181,21 @@ func (v *verifier) KeyHash() uint32 { func (v *verifier) Verify(msg, sig []byte) bool { return v.v(msg, sig) } + +// parseKeyHash parses a 4-byte key hash encoded as 8 lowercase hexadecimal characters. +func parseKeyHash(hash16 string) (uint32, error) { + if len(hash16) != 8 { + return 0, errInvalidHash + } + for i := 0; i < len(hash16); i++ { + c := hash16[i] + if (c < '0' || c > '9') && (c < 'a' || c > 'f') { + return 0, errInvalidHash + } + } + h, err := strconv.ParseUint(hash16, 16, 32) + if err != nil { + return 0, errInvalidHash + } + return uint32(h), nil +} diff --git a/note/note_cosigv1.go b/note/note_cosigv1.go index 8b91318..4201731 100644 --- a/note/note_cosigv1.go +++ b/note/note_cosigv1.go @@ -15,7 +15,6 @@ import ( "fmt" "math" "math/bits" - "strconv" "strings" "time" "unicode" @@ -64,7 +63,7 @@ func NewMLDSASigner(skey string) (SubtreeSigner, error) { if priv1 != "PRIVATE" || priv2 != "KEY" || len(hash16) != 8 || err != nil || !isValidName(name) || len(key) == 0 { return nil, errSignerID } - hash, err := strconv.ParseUint(hash16, 16, 32) + hash, err := parseKeyHash(hash16) if err != nil { return nil, errInvalidHash } @@ -84,7 +83,7 @@ func NewMLDSASigner(skey string) (SubtreeSigner, error) { if err != nil { return nil, err } - if s.KeyHash() != uint32(hash) { + if s.KeyHash() != hash { return nil, errInvalidHash } return s, nil @@ -154,6 +153,10 @@ func NewMLDSAVerifier(vkey string) (SubtreeVerifier, error) { if len(hash16) != 8 || err != nil || !isValidName(name) || len(keyBytes) != mldsa.MLDSA44PublicKeySize+1 { return nil, errVerifierID } + hash, err := parseKeyHash(hash16) + if err != nil { + return nil, errInvalidHash + } alg, pubKeyBytes := keyBytes[0], keyBytes[1:] if alg != algMLDSA44 { return nil, errVerifierID @@ -163,6 +166,9 @@ func NewMLDSAVerifier(vkey string) (SubtreeVerifier, error) { name: name, keyHash: keyHashMLDSA(name, keyBytes), } + if hash != v.keyHash { + return nil, errInvalidHash + } pubKey, err := mldsa.NewPublicKey(mldsa.MLDSA44(), pubKeyBytes) if err != nil { @@ -194,6 +200,10 @@ func NewSignerForCosignatureV1(skey string) (Signer, error) { if priv1 != "PRIVATE" || priv2 != "KEY" || len(hash16) != 8 || err != nil || !isValidName(name) || len(key) == 0 { return nil, errSignerID } + hash, err := parseKeyHash(hash16) + if err != nil { + return nil, errInvalidHash + } s := &signer{name: name} @@ -207,7 +217,11 @@ func NewSignerForCosignatureV1(skey string) (Signer, error) { return nil, errSignerID } key := ed25519.NewKeyFromSeed(key) - pubkey := append([]byte{algEd25519CosignatureV1}, key.Public().(ed25519.PublicKey)...) + pub := key.Public().(ed25519.PublicKey) + if hash != keyHashEd25519(name, append([]byte{alg}, pub...)) { + return nil, errInvalidHash + } + pubkey := append([]byte{algEd25519CosignatureV1}, pub...) s.hash = keyHashEd25519(name, pubkey) s.sign = func(msg []byte) ([]byte, error) { t := uint64(time.Now().Unix()) @@ -236,6 +250,9 @@ func NewSignerForCosignatureV1(skey string) (Signer, error) { if err != nil { return nil, err } + if hash != stSigner.KeyHash() { + return nil, errInvalidHash + } s.sign = stSigner.Sign s.verify = stSigner.Verifier().Verify s.hash = stSigner.KeyHash() @@ -266,6 +283,10 @@ func NewVerifierForCosignatureV1(vkey string) (note.Verifier, error) { if len(hash16) != 8 || err != nil || !isValidName(name) || len(key) == 0 { return nil, errVerifierID } + hash, err := parseKeyHash(hash16) + if err != nil { + return nil, errInvalidHash + } v := &verifier{ name: name, @@ -280,6 +301,9 @@ func NewVerifierForCosignatureV1(vkey string) (note.Verifier, error) { if len(key) != 32 { return nil, errVerifierID } + if hash != keyHashEd25519(name, append([]byte{alg}, key...)) { + return nil, errInvalidHash + } v.keyHash = keyHashEd25519(name, append([]byte{algEd25519CosignatureV1}, key...)) v.v = verifyEd25519CosigV1(key) @@ -288,6 +312,9 @@ func NewVerifierForCosignatureV1(vkey string) (note.Verifier, error) { return nil, errVerifierID } v.keyHash = keyHashMLDSA(name, append([]byte{algMLDSA44}, key...)) + if hash != v.keyHash { + return nil, errInvalidHash + } pubKey, err := mldsa.NewPublicKey(mldsa.MLDSA44(), key) if err != nil { return nil, err @@ -314,12 +341,12 @@ func VKeyToCosignatureV1(vkey string) (string, error) { if alg != algEd25519 { return "", errVerifierAlg } - hash, err := strconv.ParseUint(hash16, 16, 32) + hash, err := parseKeyHash(hash16) if err != nil { return "", errInvalidHash } - if uint32(hash) != keyHashEd25519(name, algKey) { + if hash != keyHashEd25519(name, algKey) { return "", errInvalidHash } if len(key) != 32 { diff --git a/note/note_cosigv1_test.go b/note/note_cosigv1_test.go index 9276503..725d636 100644 --- a/note/note_cosigv1_test.go +++ b/note/note_cosigv1_test.go @@ -193,7 +193,7 @@ func TestCoSigV1NewVerifier(t *testing.T) { pubK: "remora.n621.de+da77ade7+BOvN63jn/bLvkieywe8R6UYAtVtNbZpXh34x7onlmtw2", }, { name: "works: native algMLDSA44 verifier", - pubK: "test+5893dc2c+BtMcFiao6ZOdU6LZ40tLKbsWpDOU8smRapXBIYI3lXyESm62to+/AeDuWOEtbwUNVzrC9FacZ1q+gXES2hAhp5/C1TPwTiO/G+T9x0iAb8gSGkGwsbDJXmQMhsFM+Ub3tB5Fdujz4o7DF3NqCCUMC1zsD7jMyy9BTCFi6Av1I/ZDRQxJJOKFt31l0cJY6OHFcUGMSGSHcGEo839UikbMBlArWRgYk/Ve4aqW0pRl7G46Qk39pu/yFYwhk3gMYMkush5NKQo7EbvhnHvUlQWK27t2VsIbH2p/l9i73UDtEmHeqIMcqtwhCnFoqT6S7cL9/p7NLwxD1gICM0gCZIi3KrbnMFok+5uovBbrF9vISSXX67R1nprdjiE0MGAwZ3Prtt0ah2xchT5I1WgmUGSA0B1cnEDXWneUaA0axw/TQ47x88+jfKIN0kn8rg5bncI5q71hV2mF1n2xuE4G+WOdBRjMVGLWlt1rZcCh8IredoZe3SxWKx7amrLo00lFN8QL8TAw1bvDiFYRqyAZE4Z5M77H8OmAR2QahuZA+d8Q1SXmTdDtOu1RRXtHq54Nm3d2SbQl48UE7BsWvu7YdqGEti5EpTX3oMVmnnjj0FRH2QjlnpaRn5bE8tiblhL31f6KRz37E9lqIUoLuE19OQ+yYOj2B6avwEY6xWq5SrOOhENQKAODXTjacDYVL4Z1hsJA9+7qbFH5S3JjMCs4VFtZHOa4tkxbpO94lfPNnhqDnuFb5xm7sT51/In+xn0vAyCoaaIpm/rwG0nRFZmR6bafSPBJXcnrocdPy86sQ/C3ma7ldByWwsHuEm8YTNABAqn6hNICNECFuoV8EBwnA0BVkhClyjTkPSnyB1CAUEkzQwd/SW7VgjDoI9r6k4ot1oaxD9ZudZor7309jhMIbQhE1ODPMDxFM2B5XvhlJ6nl6r0JUI/q2uLnZGyzKDMGnL+T+uGDN+AvBrg4kyHEM1X39Dkr1XpJIMRlROY+GayJliTQ8eEdZ1yug5C0JGHJ9bEQu/L2Zf084+/+4BAUrrJ4JtmShbYulaDmMVjm72Osu5a9ld7KeZ2hn5uK9yhiUJqeBDGGLmqqZbbUwIRa9OWZoQVU+dCt2ust5migCq69O3H+83U2YKBj9r3QLv6FG/crj3IUrKagKIqv5fT71eeCEmSmdAGqVP+5hxBQMBcISqp+9rqDsS1NLba2YuUWYgFDlQ7Xq915t+d3N5ouHNWJHs1/2V0ZNsSvID1p73sUWVJlh2M4/B8/QT1uvteCPc+yycsJM80Xaomwv9KTfUpyNn60R860zr+Va432W8v5urT1Teu6QCTWKdq3ivhyYREBLvka07jb7SlIees2PKDfvibKBBanF/EmorgkdPIagG/88kT9GtXtOIQp/HJLw3Ej5QPwEgrLYZ0YY6t53ZC6BmjDt0eSEShPmqte74KC7Qgo2hvA3grqp03vA7RG3cMCmtn/k0PH9ZY4ytTF7eozJCRim9AWa0HudpkGKbv3ijxrkBBhAGTCGQe96Tt5nBiKatheV3z4i2o4TtoRC6SEQZUEnWLszXOWtghhfe/V7QqB8jfMLxHW2qkueUkTz2IatYI+2hlkyTQRIKGPjKrnF8qAKz5/PZcoaL6pBaWcpsUEcpwQs6/aT0tmB2UGZNzrj02lREEgGajjuqHMH/rwmqIQTV38KohBniPDkDV9jdIyU1HVlm5dElah29WMC+RWC6bN2GbOvn1+s6iQwNNLUGU=", + pubK: "test+31236897+BtMcFiao6ZOdU6LZ40tLKbsWpDOU8smRapXBIYI3lXyESm62to+/AeDuWOEtbwUNVzrC9FacZ1q+gXES2hAhp5/C1TPwTiO/G+T9x0iAb8gSGkGwsbDJXmQMhsFM+Ub3tB5Fdujz4o7DF3NqCCUMC1zsD7jMyy9BTCFi6Av1I/ZDRQxJJOKFt31l0cJY6OHFcUGMSGSHcGEo839UikbMBlArWRgYk/Ve4aqW0pRl7G46Qk39pu/yFYwhk3gMYMkush5NKQo7EbvhnHvUlQWK27t2VsIbH2p/l9i73UDtEmHeqIMcqtwhCnFoqT6S7cL9/p7NLwxD1gICM0gCZIi3KrbnMFok+5uovBbrF9vISSXX67R1nprdjiE0MGAwZ3Prtt0ah2xchT5I1WgmUGSA0B1cnEDXWneUaA0axw/TQ47x88+jfKIN0kn8rg5bncI5q71hV2mF1n2xuE4G+WOdBRjMVGLWlt1rZcCh8IredoZe3SxWKx7amrLo00lFN8QL8TAw1bvDiFYRqyAZE4Z5M77H8OmAR2QahuZA+d8Q1SXmTdDtOu1RRXtHq54Nm3d2SbQl48UE7BsWvu7YdqGEti5EpTX3oMVmnnjj0FRH2QjlnpaRn5bE8tiblhL31f6KRz37E9lqIUoLuE19OQ+yYOj2B6avwEY6xWq5SrOOhENQKAODXTjacDYVL4Z1hsJA9+7qbFH5S3JjMCs4VFtZHOa4tkxbpO94lfPNnhqDnuFb5xm7sT51/In+xn0vAyCoaaIpm/rwG0nRFZmR6bafSPBJXcnrocdPy86sQ/C3ma7ldByWwsHuEm8YTNABAqn6hNICNECFuoV8EBwnA0BVkhClyjTkPSnyB1CAUEkzQwd/SW7VgjDoI9r6k4ot1oaxD9ZudZor7309jhMIbQhE1ODPMDxFM2B5XvhlJ6nl6r0JUI/q2uLnZGyzKDMGnL+T+uGDN+AvBrg4kyHEM1X39Dkr1XpJIMRlROY+GayJliTQ8eEdZ1yug5C0JGHJ9bEQu/L2Zf084+/+4BAUrrJ4JtmShbYulaDmMVjm72Osu5a9ld7KeZ2hn5uK9yhiUJqeBDGGLmqqZbbUwIRa9OWZoQVU+dCt2ust5migCq69O3H+83U2YKBj9r3QLv6FG/crj3IUrKagKIqv5fT71eeCEmSmdAGqVP+5hxBQMBcISqp+9rqDsS1NLba2YuUWYgFDlQ7Xq915t+d3N5ouHNWJHs1/2V0ZNsSvID1p73sUWVJlh2M4/B8/QT1uvteCPc+yycsJM80Xaomwv9KTfUpyNn60R860zr+Va432W8v5urT1Teu6QCTWKdq3ivhyYREBLvka07jb7SlIees2PKDfvibKBBanF/EmorgkdPIagG/88kT9GtXtOIQp/HJLw3Ej5QPwEgrLYZ0YY6t53ZC6BmjDt0eSEShPmqte74KC7Qgo2hvA3grqp03vA7RG3cMCmtn/k0PH9ZY4ytTF7eozJCRim9AWa0HudpkGKbv3ijxrkBBhAGTCGQe96Tt5nBiKatheV3z4i2o4TtoRC6SEQZUEnWLszXOWtghhfe/V7QqB8jfMLxHW2qkueUkTz2IatYI+2hlkyTQRIKGPjKrnF8qAKz5/PZcoaL6pBaWcpsUEcpwQs6/aT0tmB2UGZNzrj02lREEgGajjuqHMH/rwmqIQTV38KohBniPDkDV9jdIyU1HVlm5dElah29WMC+RWC6bN2GbOvn1+s6iQwNNLUGU=", }, { name: "wrong number of parts", pubK: "bananas.sigstore.dev+12344556", @@ -815,6 +815,66 @@ func TestCosigV1_RejectNonCanonicalBase64(t *testing.T) { } } +func TestCosigV1_KeyHashValidation(t *testing.T) { + const ( + edSk = "PRIVATE+KEY+Wit1+55ee4561+AeadRiG7XM4XiieCHzD8lxysXMwcViy5nYsoXURWGrlE" + edVk = "Wit1+55ee4561+AVhZSmQj9+SoL+p/nN0Hh76xXmF7QcHfytUrI1XfSClk" + cosigVk = "remora.n621.de+da77ade7+BOvN63jn/bLvkieywe8R6UYAtVtNbZpXh34x7onlmtw2" + mlSk = "PRIVATE+KEY+mldsa+6b9c8425+BqQPO4WbZDu4B20sC1K2ngxNDdGbMCFtlZtqkF5Yw4sT" + mlVk = "witness.example/w1+a0d0cd91+Bhh6t5zW2IMElD4GJw65/1+GnMosGFhPFst8+uCD+C9/lSovRbjzbFPV7YP9rox2T9pHbsqNmvR3GTnwSFLUZIatFUlXBt0CGLU1J75PMs7G1KNj5yL4R8vvInpzaDvlBUoCLjzegUOh7KImnowtDnJRX38puLf1mStewbP61oBYZmIlWhXproauXnBl0VGbqbd8qeLQ96ENJBhFATSuxbQoJZ3dhGwkHnEQUlqHdEPgYXKwuMWfzKz4X+54eDV4c+IOFcU3/Lmis2aqAG0LoAM4X+eE569R0YVmjXdybbqrCx6dc8gwM9DZp7+gUBwWtI6ccaXJoN6kmdPBC6qUv/PHlCaFCNq170DqAbFJauGRUOBKjGyP9mSQaF6yoNrkdEOsSh039J1uaqDD7X+g5KxFWwM+R1X2HibnEW6okJaIM+MaPsAnZeQLPYbaup/2emXDsHj58+b/5S03DRszJJStFlNxHnMTe+aNyj7vbU8/GD5B3T4es6HsvNbF5+w0PRmZ/1Kkkad4+D6aFj7pMU8eVCpMZtdU8zlYcHOyNZssna8mTL11jfq9NpD5wSo+CYAiDZ/hrW9QjUYpqwzUVTr9gN14mBXfax1g9iTxRAId1ZaHBoYqyitNpE2/797iJVEshYXeh49wGPsPUp+GQYUvYVGr0CmtEYEBoXXDYechA+G1dSyKGgeU5R5ufOkLnumvHW060RKxst3eXKMi1uNyEojsTUhHclL44a6irz95MAWE4k7d9E3Tu8S6mct5WVN4TzyFKZ7PPB+mIMhsbg6n5lI62D3gMKGHTJGbGz17iNZwj0FVp5trL4OGiF6qeK2FLXesOlWYOfOFXvfuLZXvNwmry3aSi/ST+65qXHmAGLQ0ip06A8yEUt3zGZ2VI8whagfAXj2CxcFcPVgg9KI+0aJnbldm6KYcaLoI+fs1iyGQ+zgXrejm/hCyGik+JFVfZGlH+hirnNNglxRsJzIYsFT73ss3+29SnAduHZOYFqLs1aNsDytZgEhC37EL8vqIO7jY1MR8ihldBljWiVV+AQiWpyKvseP5SFP82pcfkGi6DbAAQmifLjGGcm9pCqDIUWrXIyiwt/WbtTgzEuQhdtuYEFeK59BSdpXW9UxUAYG8AjlYL+ylVUjbtz9CYGHtaCoi3NxkaRaUZj2Mtx2WQSyDI2uUTUwOpiqVwJEAE3lpOBqHV58hEosDfAxR1A0bw7px4LFqsfuFcapsP1015sb2IIQn8zx8lfIVtTon2jYMyANqACceBA43Vu9+o7ij+wwEVedRsO5d6/F6KZjL+weSiaIzGRQUazSvkMGZr+3rgaol3wgHkBY/AvJRPqxEZA14+e+HVl7rOTR96o20g72AoDnTj+V5LpggrDEd2eJeYR+1SjphxPKE+Y4U4NJ/Zv68Cji9lmIXAo9ohBGxcjkPdw2Ha2HTE0WT3HwROtbBeFfEvKaAkke9VS5fGmPqkLXzuBmZ4OsRM5zwhW0mcZUjWp4anpuSyngVddL3NyTUg2UBoU3sQVAKr/VtboY0Fn6LuSi+jS6IXUhNkTfHEkZXnWN1oYnQs46/iWbJk0NxjPaCIYceU7DZTLpCuPOVRgm31FcF0AZT/T40gbgzAu7isTXhlOtswI0dpT9k2FidQPsrZh9QZr1lZjcyzqt0M2PeYcctUYY/Y75T5ZNBxxmgQ/EDwu6Y1RUAk9xa7ZrIeJ2ZHXGgwIT7yDQoj8c0nlb/k1qqMbqQey/63s4=" + ) + + withKeyHash := func(key string, mutate func(string) string) string { + prefix := "" + if after, ok := strings.CutPrefix(key, "PRIVATE+KEY+"); ok { + prefix = "PRIVATE+KEY+" + key = after + } + name, rest, _ := strings.Cut(key, "+") + hash, b64, _ := strings.Cut(rest, "+") + return prefix + name + "+" + mutate(hash) + "+" + b64 + } + + mutations := []struct { + name string + mutate func(string) string + }{ + {"mismatched", func(string) string { return "00000000" }}, + {"non-hex", func(string) string { return "zzzzzzzz" }}, + {"short", func(h string) string { return h[:7] }}, + {"uppercase", strings.ToUpper}, + } + + for _, target := range []struct { + name string + key string + fn func(string) error + }{ + {"NewSignerForCosignatureV1/Ed25519", edSk, func(k string) error { _, err := NewSignerForCosignatureV1(k); return err }}, + {"NewSignerForCosignatureV1/MLDSA", mlSk, func(k string) error { _, err := NewSignerForCosignatureV1(k); return err }}, + {"NewMLDSASigner", mlSk, func(k string) error { _, err := NewMLDSASigner(k); return err }}, + {"NewVerifierForCosignatureV1/Ed25519", edVk, func(k string) error { _, err := NewVerifierForCosignatureV1(k); return err }}, + {"NewVerifierForCosignatureV1/CosigV1", cosigVk, func(k string) error { _, err := NewVerifierForCosignatureV1(k); return err }}, + {"NewVerifierForCosignatureV1/MLDSA", mlVk, func(k string) error { _, err := NewVerifierForCosignatureV1(k); return err }}, + {"NewMLDSAVerifier", mlVk, func(k string) error { _, err := NewMLDSAVerifier(k); return err }}, + {"VKeyToCosignatureV1", edVk, func(k string) error { _, err := VKeyToCosignatureV1(k); return err }}, + } { + t.Run(target.name, func(t *testing.T) { + if err := target.fn(target.key); err != nil { + t.Fatalf("valid key rejected: %v", err) + } + for _, m := range mutations { + t.Run(m.name, func(t *testing.T) { + badKey := withKeyHash(target.key, m.mutate) + if err := target.fn(badKey); err == nil { + t.Errorf("succeeded on %s key hash (%q), want error", m.name, badKey) + } + }) + } + }) + } +} + func TestCosigV1_RejectCRLF(t *testing.T) { edSk, edVk := mustGenerateEd25519Key(t, "ed25519") mlSk, mlVk := mustGenerateMLDSAKey(t, "mldsa") diff --git a/note/note_rfc6962.go b/note/note_rfc6962.go index bb48036..2d400ec 100644 --- a/note/note_rfc6962.go +++ b/note/note_rfc6962.go @@ -60,6 +60,10 @@ func NewRFC6962Verifier(vkey string) (note.Verifier, error) { if len(hash16) != 8 || err != nil || !isValidName(name) || len(key) == 0 { return nil, errVerifierID } + hash, err := parseKeyHash(hash16) + if err != nil { + return nil, errInvalidHash + } v := &rfc6962Verifer{ name: name, @@ -77,6 +81,9 @@ func NewRFC6962Verifier(vkey string) (note.Verifier, error) { logID := sha256.Sum256(key) v.keyHash = rfc6962Keyhash(name, logID) + if hash != v.keyHash { + return nil, errInvalidHash + } v.v = verifyRFC6962(pubK) return v, nil diff --git a/note/note_rfc6962_test.go b/note/note_rfc6962_test.go index bd63264..e473a30 100644 --- a/note/note_rfc6962_test.go +++ b/note/note_rfc6962_test.go @@ -248,3 +248,20 @@ func TestRFC6962_RejectNonCanonicalBase64(t *testing.T) { t.Error("RFC6962STHTimestamp succeeded on non-canonical base64 signature, want error") } } + +func TestRFC6962_KeyHashValidation(t *testing.T) { + const keyB64 = "BTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAFzPC3fap+uINc1RQ4eRbYMUt84+bKkA8GDLN8KwVdzAgYhYSv4kS8XSheGLAHCWhIJTJbuC3sL88bNMTtrsBM=" + for _, badVK := range []string{ + "rome.ct.filippo.io/2024h1+00000000+" + keyB64, + "rome.ct.filippo.io/2024h1+zzzzzzzz+" + keyB64, + "rome.ct.filippo.io/2024h1+78f4aba+" + keyB64, + "rome.ct.filippo.io/2024h1+78F4ABAE+" + keyB64, + } { + if _, err := NewRFC6962Verifier(badVK); err == nil { + t.Errorf("NewRFC6962Verifier(%q) succeeded, want error", badVK) + } + if _, err := NewVerifier(badVK); err == nil { + t.Errorf("NewVerifier(%q) succeeded, want error", badVK) + } + } +} diff --git a/note/note_test.go b/note/note_test.go index 39e52b6..bf267f3 100644 --- a/note/note_test.go +++ b/note/note_test.go @@ -117,3 +117,41 @@ func TestNewSignerVerifier_RejectCRLF(t *testing.T) { } } } + +func TestNewSignerVerifier_KeyHashValidation(t *testing.T) { + for _, tc := range []struct { + name string + skey string + vkey string + }{ + { + name: "mismatched", + skey: "PRIVATE+KEY+Wit1+00000000+AeadRiG7XM4XiieCHzD8lxysXMwcViy5nYsoXURWGrlE", + vkey: "PeterNeumann+00000000+ARpc2QcUPDhMQegwxbzhKqiBfsVkmqq/LDE4izWy10TW", + }, + { + name: "non-hex", + skey: "PRIVATE+KEY+Wit1+zzzzzzzz+AeadRiG7XM4XiieCHzD8lxysXMwcViy5nYsoXURWGrlE", + vkey: "PeterNeumann+zzzzzzzz+ARpc2QcUPDhMQegwxbzhKqiBfsVkmqq/LDE4izWy10TW", + }, + { + name: "short", + skey: "PRIVATE+KEY+Wit1+55ee456+AeadRiG7XM4XiieCHzD8lxysXMwcViy5nYsoXURWGrlE", + vkey: "PeterNeumann+c74f20a+ARpc2QcUPDhMQegwxbzhKqiBfsVkmqq/LDE4izWy10TW", + }, + { + name: "uppercase", + skey: "PRIVATE+KEY+Wit1+55EE4561+AeadRiG7XM4XiieCHzD8lxysXMwcViy5nYsoXURWGrlE", + vkey: "PeterNeumann+C74F20A3+ARpc2QcUPDhMQegwxbzhKqiBfsVkmqq/LDE4izWy10TW", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := NewSigner(tc.skey); err == nil { + t.Errorf("NewSigner(%q) succeeded, want error", tc.skey) + } + if _, err := NewVerifier(tc.vkey); err == nil { + t.Errorf("NewVerifier(%q) succeeded, want error", tc.vkey) + } + }) + } +} diff --git a/note/note_verifier.go b/note/note_verifier.go index be1421a..1d1863c 100644 --- a/note/note_verifier.go +++ b/note/note_verifier.go @@ -24,7 +24,6 @@ import ( "encoding/binary" "errors" "fmt" - "strconv" "strings" "golang.org/x/mod/sumdb/note" @@ -49,6 +48,9 @@ func NewEd25519SignerVerifier(skey string) (note.Signer, note.Verifier, error) { if parts[0] != "PRIVATE" || parts[1] != "KEY" { return nil, nil, fmt.Errorf("expected first tokens to be [PRIVATE, KEY]") } + if _, err := parseKeyHash(parts[3]); err != nil { + return nil, nil, fmt.Errorf("invalid keyhash: %v", err) + } key, err := base64.StdEncoding.Strict().DecodeString(parts[4]) if err != nil { return nil, nil, fmt.Errorf("failed to decode base64: %v", err) @@ -112,12 +114,12 @@ func NewECDSAVerifier(key string) (note.Verifier, error) { der := keyBytes[1:] kh := keyHashECDSA(der) - khProvided, err := strconv.ParseUint(parts[1], 16, 32) + khProvided, err := parseKeyHash(parts[1]) if err != nil { return nil, fmt.Errorf("invalid key, couldn't parse keyhash: %v", err) } - if uint32(khProvided) != kh { - return nil, fmt.Errorf("invalid keyhash %x, expected %x", khProvided, kh) + if khProvided != kh { + return nil, fmt.Errorf("invalid keyhash %08x, expected %08x", khProvided, kh) } k, err := x509.ParsePKIXPublicKey(der) diff --git a/note/note_verifier_test.go b/note/note_verifier_test.go index 830b4de..5495713 100644 --- a/note/note_verifier_test.go +++ b/note/note_verifier_test.go @@ -49,6 +49,11 @@ func TestSignerVerifierFromEd25519Key(t *testing.T) { key: "logandmap+38581672+Ab/PCr1eCclRPRMBqw/r5An1xO71MCnImLiospEq6b4l", wantErr: true, }, + { + name: "uppercase hex keyhash", + key: "PRIVATE+KEY+Wit1+55EE4561+AeadRiG7XM4XiieCHzD8lxysXMwcViy5nYsoXURWGrlE", + wantErr: true, + }, } { t.Run(test.name, func(t *testing.T) { s, v, err := NewEd25519SignerVerifier(test.key) @@ -138,6 +143,14 @@ func TestNewECDSAVerifier(t *testing.T) { name: "incorrect keyhash", pubK: "rekor.sigstore.dev" + "+" + "00000000" + "+" + sigStoreKeyMaterial, wantErr: true, + }, { + name: "uppercase hex keyhash", + pubK: "rekor.sigstore.dev" + "+" + strings.ToUpper(sigStoreKeyHash) + "+" + sigStoreKeyMaterial, + wantErr: true, + }, { + name: "short keyhash", + pubK: "rekor.sigstore.dev" + "+" + sigStoreKeyHash[1:] + "+" + sigStoreKeyMaterial, + wantErr: true, }, } { t.Run(test.name, func(t *testing.T) { diff --git a/policy/tlog_policy_test.go b/policy/tlog_policy_test.go index a318cd9..cf5e34b 100644 --- a/policy/tlog_policy_test.go +++ b/policy/tlog_policy_test.go @@ -81,15 +81,15 @@ func genVkey(name string) string { } // rewrapVkey rewraps the public key from vkey with the given algorithm -// octet under the given key name (with a placeholder key id, which -// cosignature verifiers do not validate). +// octet under the given key name. func rewrapVkey(vkey string, alg byte, name string) string { key, err := vkeyPublicKey(vkey) if err != nil { panic(fmt.Sprintf("vkeyPublicKey(%q): %v", vkey, err)) } key[0] = alg - return fmt.Sprintf("%s+00000000+%s", name, base64.StdEncoding.EncodeToString(key)) + h := sha256.Sum256(append([]byte(name+"\n"), key...)) + return fmt.Sprintf("%s+%08x+%s", name, h[:4], base64.StdEncoding.EncodeToString(key)) } func TestUnmarshal(t *testing.T) { @@ -306,6 +306,11 @@ func TestUnmarshal_Errors(t *testing.T) { policy: fmt.Sprintf("witness w1 %s\nquorum w1\n", wit1_ed25519_vkey), errStr: "key type 0x01 is not a cosignature key type", }, + { + desc: "witness with mismatched key hash", + policy: fmt.Sprintf("witness w1 %s\nquorum w1\n", "Wit1+00000000+"+strings.SplitN(wit1_vkey, "+", 3)[2]), + errStr: "invalid witness vkey", + }, { desc: "witness named none", policy: fmt.Sprintf("witness none %s\nquorum none\n", wit1_vkey),