This PR (HEAD: acd10d4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/203480 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

Message from Filippo Valsorda:

Patch Set 1: Run-TryBot+1

Interesting, given how our signature verification works (by just comparing the prefix), has this never worked, and no one noticed? How did you notice?

We'll definitely need a test for this, with details about how it was generated, and ideally a better reference than an OpenPGP RFC.

Please don’t reply on this GitHub thread. Visit golang.org/cl/203480.
After addressing review feedback, remember to publish your drafts!

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=8f5e1dcb

Please don’t reply on this GitHub thread. Visit golang.org/cl/203480.
After addressing review feedback, remember to publish your drafts!

Message from Gobot Gobot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/203480.
After addressing review feedback, remember to publish your drafts!

Yeah, this is difficult... I'm not finding a good source. My assumption is that it's always been incorrect.
I've noticed in the community in general that the "go" version is more or less limited to golang projects:
https://github.com/search?q=%220x30%2C+0x20%2C+0x30%2C+0x08%2C+0x06%2C+0x06%2C+0x28%2C+0xcf%2C+0x06%2C+0x03%2C+0x00%2C+0x31%2C+0x04%2C+0x14%22&type=Code

And the "corrected" version is more widespread:
https://github.com/search?q=%220x30%2C+0x21%2C+0x30%2C+0x09%2C+0x06%2C+0x05%2C+0x2B%2C+0x24%2C+0x03%2C+0x02%2C+0x01%2C+0x05%2C+0x00%2C+0x04%2C+0x14%22&type=Code

I realize this is hardly enough to advocate for changing :)
I'll keep looking for something more definitive.

Message from Emmanuel Odeke:

Patch Set 1:

(1 comment)

Thank you for this catch Scott, much appreciated!

Here is a regression test to seed a test, please
place it in pkcs1v15_test.go:

// Issue 35495: ensure that the hash prefixes from
// RFC 4880 Section 5.2.2 correctly match what we have.
func TestCheckHashPrefixes(t *testing.T) {
want := []struct {
name string
hash crypto.Hash
wantPrefix []byte
}{
{"MD5", crypto.MD5, []byte{0x30, 0x20, 0x30, 0x0C, 0x06, 0x08, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x05, 0x05, 0x00, 0x04, 0x10}},
{"SHA1", crypto.SHA1, []byte{0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2b, 0x0E, 0x03, 0x02, 0x1A, 0x05, 0x00, 0x04, 0x14}},
{"SHA224", crypto.SHA224, []byte{0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04, 0x05, 0x00, 0x04, 0x1c}},
{"SHA256", crypto.SHA256, []byte{0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20}},
{"SHA384", crypto.SHA384, []byte{0x30, 0x41, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, 0x05, 0x00, 0x04, 0x30}},
{"SHA512", crypto.SHA512, []byte{0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, 0x05, 0x00, 0x04, 0x40}},
{"RIPEMD-160", crypto.RIPEMD160, []byte{0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2B, 0x24, 0x03, 0x02, 0x01, 0x05, 0x00, 0x04, 0x14}},
}

    for _, tt := range want {
            t.Run(tt.name, func(t *testing.T) {
                    gotPrefix := hashPrefixes[tt.hash]
                    if g, w := gotPrefix, tt.wantPrefix; !bytes.Equal(g, w) {
                            t.Fatalf("Hash prefix mismatch:\nGot:  % x\nWant: % x", g, w)
                    }
            })
    }

}

and currently it'll print:
$ go test -run=HashPrefixes
--- FAIL: TestHashPrefixes (0.00s)
--- FAIL: TestHashPrefixes/RIPEMD-160 (0.00s)
pkcs1v15_test.go:321: Hash prefix mismatch:
Got: 30 20 30 08 06 06 28 cf 06 03 00 31 04 14
Want: 30 21 30 09 06 05 2b 24 03 02 01 05 00 04 14
FAIL
exit status 1
FAIL crypto/rsa 0.006s

Please don’t reply on this GitHub thread. Visit golang.org/cl/203480.
After addressing review feedback, remember to publish your drafts!

This PR is being closed because golang.org/cl/203480 has been abandoned.

See #35495