go.crypto/openpgp: fix hash presence checks.
At some point in the distant past, crypto.Hash.New() changed from
returning nil when a hash function wasn't provided, to panicing. Some
of the code in openpgp predates this and was still using the nil check.
LGTM=bradfitz
R=sburford, bradfitz
CC=golang-codereviews
https://golang.org/cl/104850045
diff --git a/openpgp/clearsign/clearsign.go b/openpgp/clearsign/clearsign.go
index a753bcf..4f7bc2c 100644
--- a/openpgp/clearsign/clearsign.go
+++ b/openpgp/clearsign/clearsign.go
@@ -296,10 +296,10 @@
return nil, errors.UnsupportedError("unknown hash type: " + strconv.Itoa(int(hashType)))
}
- h := hashType.New()
- if h == nil {
+ if !hashType.Available() {
return nil, errors.UnsupportedError("unsupported hash type: " + strconv.Itoa(int(hashType)))
}
+ h := hashType.New()
buffered := bufio.NewWriter(w)
// start has a \n at the beginning that we don't want here.
diff --git a/openpgp/packet/public_key_v3.go b/openpgp/packet/public_key_v3.go
index 8028996..0897bcc 100644
--- a/openpgp/packet/public_key_v3.go
+++ b/openpgp/packet/public_key_v3.go
@@ -237,9 +237,10 @@
// userIdSignatureV3Hash returns a Hash of the message that needs to be signed
// to assert that pk is a valid key for id.
func userIdSignatureV3Hash(id string, pk signingKey, hfn crypto.Hash) (h hash.Hash, err error) {
- if h = hfn.New(); h == nil {
+ if !hfn.Available() {
return nil, errors.UnsupportedError("hash function")
}
+ h = hfn.New()
// RFC 4880, section 5.2.4
pk.SerializeSignaturePrefix(h)
diff --git a/openpgp/read.go b/openpgp/read.go
index da59dcc..94c4e6a 100644
--- a/openpgp/read.go
+++ b/openpgp/read.go
@@ -276,10 +276,10 @@
// returns two hashes. The second should be used to hash the message itself and
// performs any needed preprocessing.
func hashForSignature(hashId crypto.Hash, sigType packet.SignatureType) (hash.Hash, hash.Hash, error) {
- h := hashId.New()
- if h == nil {
+ if !hashId.Available() {
return nil, nil, errors.UnsupportedError("hash not available: " + strconv.Itoa(int(hashId)))
}
+ h := hashId.New()
switch sigType {
case packet.SigTypeBinary:
diff --git a/openpgp/read_test.go b/openpgp/read_test.go
index 3c90837..b08469d 100644
--- a/openpgp/read_test.go
+++ b/openpgp/read_test.go
@@ -11,6 +11,7 @@
"encoding/hex"
"io"
"io/ioutil"
+ "strings"
"testing"
)
@@ -285,6 +286,33 @@
testDetachedSignature(t, kring, readerFromHex(detachedSignatureDSAHex), signedInput, "binary", testKey3KeyId)
}
+func testHashFunctionError(t *testing.T, signatureHex string) {
+ kring, _ := ReadKeyRing(readerFromHex(testKeys1And2Hex))
+ _, err := CheckDetachedSignature(kring, nil, readerFromHex(signatureHex))
+ if err == nil {
+ t.Fatal("Packet with bad hash type was correctly parsed")
+ }
+ unsupported, ok := err.(errors.UnsupportedError)
+ if !ok {
+ t.Fatalf("Unexpected class of error: %s", err)
+ }
+ if !strings.Contains(string(unsupported), "hash ") {
+ t.Fatalf("Unexpected error: %s", err)
+ }
+}
+
+func TestUnknownHashFunction(t *testing.T) {
+ // unknownHashFunctionHex contains a signature packet with hash
+ // function type 153 (which isn't a real hash function id).
+ testHashFunctionError(t, unknownHashFunctionHex)
+}
+
+func TestMissingHashFunction(t *testing.T) {
+ // missingHashFunctionHex contains a signature packet that uses
+ // RIPEMD160, which isn't compiled in.
+ testHashFunctionError(t, missingHashFunctionHex)
+}
+
func TestReadingArmoredPrivateKey(t *testing.T) {
el, err := ReadArmoredKeyRing(bytes.NewBufferString(armoredPrivateKeyBlock))
if err != nil {
@@ -373,3 +401,7 @@
-----END PGP PRIVATE KEY BLOCK-----`
const dsaKeyWithSHA512 = `9901a2044f04b07f110400db244efecc7316553ee08d179972aab87bb1214de7692593fcf5b6feb1c80fba268722dd464748539b85b81d574cd2d7ad0ca2444de4d849b8756bad7768c486c83a824f9bba4af773d11742bdfb4ac3b89ef8cc9452d4aad31a37e4b630d33927bff68e879284a1672659b8b298222fc68f370f3e24dccacc4a862442b9438b00a0ea444a24088dc23e26df7daf8f43cba3bffc4fe703fe3d6cd7fdca199d54ed8ae501c30e3ec7871ea9cdd4cf63cfe6fc82281d70a5b8bb493f922cd99fba5f088935596af087c8d818d5ec4d0b9afa7f070b3d7c1dd32a84fca08d8280b4890c8da1dde334de8e3cad8450eed2a4a4fcc2db7b8e5528b869a74a7f0189e11ef097ef1253582348de072bb07a9fa8ab838e993cef0ee203ff49298723e2d1f549b00559f886cd417a41692ce58d0ac1307dc71d85a8af21b0cf6eaa14baf2922d3a70389bedf17cc514ba0febbd107675a372fe84b90162a9e88b14d4b1c6be855b96b33fb198c46f058568817780435b6936167ebb3724b680f32bf27382ada2e37a879b3d9de2abe0c3f399350afd1ad438883f4791e2e3b4184453412068617368207472756e636174696f6e207465737488620413110a002205024f04b07f021b03060b090807030206150802090a0b0416020301021e01021780000a0910ef20e0cefca131581318009e2bf3bf047a44d75a9bacd00161ee04d435522397009a03a60d51bd8a568c6c021c8d7cf1be8d990d6417b0020003`
+
+const unknownHashFunctionHex = `8a00000040040001990006050253863c24000a09103b4fe6acc0b21f32ffff01010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101`
+
+const missingHashFunctionHex = `8a00000040040001030006050253863c24000a09103b4fe6acc0b21f32ffff01010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101`
diff --git a/openpgp/s2k/s2k.go b/openpgp/s2k/s2k.go
index 33462cc..aab3041 100644
--- a/openpgp/s2k/s2k.go
+++ b/openpgp/s2k/s2k.go
@@ -91,10 +91,10 @@
if !ok {
return nil, errors.UnsupportedError("hash for S2K function: " + strconv.Itoa(int(buf[1])))
}
- h := hash.New()
- if h == nil {
+ if !hash.Available() {
return nil, errors.UnsupportedError("hash not available: " + strconv.Itoa(int(hash)))
}
+ h := hash.New()
switch buf[0] {
case 0:
