Skip to content

Validate 8-char lowercase hex key hashes across note constructors #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mhutchinson wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Validate 8-char lowercase hex key hashes across note constructors #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions note/note.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
39 changes: 33 additions & 6 deletions note/note_cosigv1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"fmt"
"math"
"math/bits"
"strconv"
"strings"
"time"
"unicode"
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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}

Expand All @@ -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())
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -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
Expand All @@ -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 {
Expand Down
62 changes: 61 additions & 1 deletion note/note_cosigv1_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")
Expand Down
7 changes: 7 additions & 0 deletions note/note_rfc6962.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
17 changes: 17 additions & 0 deletions note/note_rfc6962_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
38 changes: 38 additions & 0 deletions note/note_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
10 changes: 6 additions & 4 deletions note/note_verifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"encoding/binary"
"errors"
"fmt"
"strconv"
"strings"

"golang.org/x/mod/sumdb/note"
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading