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 <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/crypto/rsa/pss.go b/src/crypto/rsa/pss.go
index e996e7aaa36b9c265abf579b16ae67a680573107..5716c464ca0a339704ebe35c694425fd455e102b 100644 (file)
--- 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 7e908d4389d506e92763df8711586ce8ba916df2..637d07e18cff2e125963d4d328ef1bbc43756f3d 100644 (file)
--- 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)
+       }
+}