From 8ed18d2cefb91d3e1c6ffed8c465d369587e6ec5 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 19 Sep 2024 09:20:56 -0700 Subject: [PATCH] crypto/rsa: move PSS hash override above boring block The SignPSS hash override happened after the boringcrypto block, meaning if a boringcrypto user passed a hash in the PSSOptions which did not match the hash argument, it wouldn't be overriden. This change moves the check above the boring block to make sure the override is honored. Thanks to Quim Muntal of Microsoft for spotting this issue. Change-Id: I05082a84ccb1863798ac6eae7a15cf4d1e59f12d Reviewed-on: https://go-review.googlesource.com/c/go/+/614276 Reviewed-by: Quim Muntal LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Reviewed-by: David Chase --- src/crypto/rsa/pss.go | 8 ++++---- src/crypto/rsa/pss_test.go | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/crypto/rsa/pss.go b/src/crypto/rsa/pss.go index e996e7aaa3..5716c464ca 100644 --- a/src/crypto/rsa/pss.go +++ b/src/crypto/rsa/pss.go @@ -296,6 +296,10 @@ func SignPSS(rand io.Reader, priv *PrivateKey, hash crypto.Hash, digest []byte, // well-specified number of random bytes is included in the signature, in a // well-specified way. + if opts != nil && opts.Hash != 0 { + hash = opts.Hash + } + if boring.Enabled && rand == boring.RandReader { bkey, err := boringPrivateKey(priv) if err != nil { @@ -305,10 +309,6 @@ func SignPSS(rand io.Reader, priv *PrivateKey, hash crypto.Hash, digest []byte, } boring.UnreachableExceptTests() - if opts != nil && opts.Hash != 0 { - hash = opts.Hash - } - saltLength := opts.saltLength() switch saltLength { case PSSSaltLengthAuto: diff --git a/src/crypto/rsa/pss_test.go b/src/crypto/rsa/pss_test.go index 7e908d4389..637d07e18c 100644 --- a/src/crypto/rsa/pss_test.go +++ b/src/crypto/rsa/pss_test.go @@ -13,6 +13,7 @@ import ( . "crypto/rsa" "crypto/sha1" "crypto/sha256" + "crypto/sha512" "encoding/hex" "math/big" "os" @@ -306,3 +307,22 @@ func TestInvalidPSSSaltLength(t *testing.T) { t.Fatal("VerifyPSS unexpected success") } } + +func TestHashOverride(t *testing.T) { + key, err := GenerateKey(rand.Reader, 1024) + if err != nil { + t.Fatal(err) + } + + digest := sha512.Sum512([]byte("message")) + // opts.Hash overrides the passed hash argument. + sig, err := SignPSS(rand.Reader, key, crypto.SHA256, digest[:], &PSSOptions{Hash: crypto.SHA512}) + if err != nil { + t.Fatalf("SignPSS unexpected error: got %v, want nil", err) + } + + // VerifyPSS has the inverse behavior, opts.Hash is always ignored, check this is true. + if err := VerifyPSS(&key.PublicKey, crypto.SHA512, digest[:], sig, &PSSOptions{Hash: crypto.SHA256}); err != nil { + t.Fatalf("VerifyPSS unexpected error: got %v, want nil", err) + } +} -- 2.48.1