[go] crypto/rsa: add a test walking through every key size
116 views
Skip to first unread message
Filippo Valsorda (Gerrit)
Oct 15, 2022, 6:11:45 AM10/15/22
to goph...@pubsubhelper.golang.org, Filippo Valsorda, golang-co...@googlegroups.com
Filippo Valsorda has uploaded this change for review.
crypto/rsa: add a test walking through every key size
We already had some tests for special cases such as PSS with 513 bit
keys. The upcoming backend rewrite also happened to crash at 63 and 504
bits for different reasons. Might as well be systematic about it.
Also, make sure SignPSS returns ErrMessageTooLong like SignPKCS1v15 when
the key is too small, instead of panicking or returning an unnamed error.
-all takes a couple minutes on my M1.
Change-Id: I656239a00d0831fa7d187a6d3bb30341d41602f7
---
M src/crypto/rsa/pss.go
M src/crypto/rsa/rsa.go
M src/crypto/rsa/rsa_test.go
3 files changed, 172 insertions(+), 4 deletions(-)
diff --git a/src/crypto/rsa/pss.go b/src/crypto/rsa/pss.go
index 29e79bd..b20dc61 100644
--- a/src/crypto/rsa/pss.go
+++ b/src/crypto/rsa/pss.go
@@ -49,7 +49,7 @@
// 3. If emLen < hLen + sLen + 2, output "encoding error" and stop.
if emLen < hLen+sLen+2 {
- return nil, errors.New("crypto/rsa: key size too small for PSS signature")
+ return nil, ErrMessageTooLong
}
em := make([]byte, emLen)
diff --git a/src/crypto/rsa/rsa.go b/src/crypto/rsa/rsa.go
index c941124..64f8a13 100644
--- a/src/crypto/rsa/rsa.go
+++ b/src/crypto/rsa/rsa.go
@@ -415,9 +415,10 @@
}
}
-// ErrMessageTooLong is returned when attempting to encrypt a message which is
-// too large for the size of the public key.
-var ErrMessageTooLong = errors.New("crypto/rsa: message too long for RSA public key size")
+// ErrMessageTooLong is returned when attempting to encrypt or sign a message
+// which is too large for the size of the key. When using SignPSS, this can also
+// be returned if the size of the salt is too large.
+var ErrMessageTooLong = errors.New("crypto/rsa: message too long for RSA key size")
func encrypt(c *big.Int, pub *PublicKey, m *big.Int) *big.Int {
boring.Unreachable()
diff --git a/src/crypto/rsa/rsa_test.go b/src/crypto/rsa/rsa_test.go
index bc2a32c..5e4964d 100644
--- a/src/crypto/rsa/rsa_test.go
+++ b/src/crypto/rsa/rsa_test.go
@@ -14,6 +14,7 @@
"crypto/sha256"
"crypto/x509"
"encoding/pem"
+ "flag"
"fmt"
"math/big"
"strings"
@@ -135,6 +136,154 @@
}
}
+var allFlag = flag.Bool("all", false, "test all key sizes up to 8192")
+
+func TestEverything(t *testing.T) {
+ min := 32
+ max := 560 // any smaller than this and not all tests will run
+ if testing.Short() {
+ min = max
+ }
+ if *allFlag {
+ max = 2048
+ }
+ for size := min; size <= max; size++ {
+ t.Run(fmt.Sprintf("%d", size), func(t *testing.T) {
+ t.Parallel()
+ priv, err := GenerateKey(rand.Reader, size)
+ if err != nil {
+ t.Errorf("GenerateKey(%d): %v", size, err)
+ }
+ if bits := priv.N.BitLen(); bits != size {
+ t.Errorf("key too short (%d vs %d)", bits, size)
+ }
+ testEverything(t, priv)
+ })
+ }
+}
+
+func testEverything(t *testing.T, priv *PrivateKey) {
+ if err := priv.Validate(); err != nil {
+ t.Errorf("Validate() failed: %s", err)
+ }
+
+ msg := []byte("test")
+ enc, err := EncryptPKCS1v15(rand.Reader, &priv.PublicKey, msg)
+ if err == ErrMessageTooLong {
+ t.Log("key too small for EncryptPKCS1v15")
+ } else if err != nil {
+ t.Errorf("EncryptPKCS1v15: %v", err)
+ }
+ if err == nil {
+ dec, err := DecryptPKCS1v15(nil, priv, enc)
+ if err != nil {
+ t.Errorf("DecryptPKCS1v15: %v", err)
+ }
+ err = DecryptPKCS1v15SessionKey(nil, priv, enc, make([]byte, 4))
+ if err != nil {
+ t.Errorf("DecryptPKCS1v15SessionKey: %v", err)
+ }
+ if !bytes.Equal(dec, msg) {
+ t.Errorf("got:%x want:%x (%+v)", dec, msg, priv)
+ }
+ }
+
+ label := []byte("label")
+ enc, err = EncryptOAEP(sha256.New(), rand.Reader, &priv.PublicKey, msg, label)
+ if err == ErrMessageTooLong {
+ t.Log("key too small for EncryptOAEP")
+ } else if err != nil {
+ t.Errorf("EncryptOAEP: %v", err)
+ }
+ if err == nil {
+ dec, err := DecryptOAEP(sha256.New(), nil, priv, enc, label)
+ if err != nil {
+ t.Errorf("DecryptOAEP: %v", err)
+ }
+ if !bytes.Equal(dec, msg) {
+ t.Errorf("got:%x want:%x (%+v)", dec, msg, priv)
+ }
+ }
+
+ hash := sha256.Sum256(msg)
+ sig, err := SignPKCS1v15(nil, priv, crypto.SHA256, hash[:])
+ if err == ErrMessageTooLong {
+ t.Log("key too small for SignPKCS1v15")
+ } else if err != nil {
+ t.Errorf("SignPKCS1v15: %v", err)
+ }
+ if err == nil {
+ err = VerifyPKCS1v15(&priv.PublicKey, crypto.SHA256, hash[:], sig)
+ if err != nil {
+ t.Errorf("VerifyPKCS1v15: %v", err)
+ }
+ sig[1] ^= 0x80
+ err = VerifyPKCS1v15(&priv.PublicKey, crypto.SHA256, hash[:], sig)
+ if err == nil {
+ t.Errorf("VerifyPKCS1v15 success for tampered signature")
+ }
+ sig[1] ^= 0x80
+ hash[1] ^= 0x80
+ err = VerifyPKCS1v15(&priv.PublicKey, crypto.SHA256, hash[:], sig)
+ if err == nil {
+ t.Errorf("VerifyPKCS1v15 success for tampered message")
+ }
+ hash[1] ^= 0x80
+ }
+
+ opts := &PSSOptions{SaltLength: PSSSaltLengthAuto}
+ sig, err = SignPSS(rand.Reader, priv, crypto.SHA256, hash[:], opts)
+ if err == ErrMessageTooLong {
+ t.Log("key too small for SignPSS with PSSSaltLengthAuto")
+ } else if err != nil {
+ t.Errorf("SignPSS: %v", err)
+ }
+ if err == nil {
+ err = VerifyPSS(&priv.PublicKey, crypto.SHA256, hash[:], sig, opts)
+ if err != nil {
+ t.Errorf("VerifyPSS: %v", err)
+ }
+ sig[1] ^= 0x80
+ err = VerifyPSS(&priv.PublicKey, crypto.SHA256, hash[:], sig, opts)
+ if err == nil {
+ t.Errorf("VerifyPSS success for tampered signature")
+ }
+ sig[1] ^= 0x80
+ hash[1] ^= 0x80
+ err = VerifyPSS(&priv.PublicKey, crypto.SHA256, hash[:], sig, opts)
+ if err == nil {
+ t.Errorf("VerifyPSS success for tampered message")
+ }
+ hash[1] ^= 0x80
+ }
+
+ opts.SaltLength = PSSSaltLengthEqualsHash
+ sig, err = SignPSS(rand.Reader, priv, crypto.SHA256, hash[:], opts)
+ if err == ErrMessageTooLong {
+ t.Log("key too small for SignPSS with PSSSaltLengthEqualsHash")
+ } else if err != nil {
+ t.Errorf("SignPSS: %v", err)
+ }
+ if err == nil {
+ err = VerifyPSS(&priv.PublicKey, crypto.SHA256, hash[:], sig, opts)
+ if err != nil {
+ t.Errorf("VerifyPSS: %v", err)
+ }
+ sig[1] ^= 0x80
+ err = VerifyPSS(&priv.PublicKey, crypto.SHA256, hash[:], sig, opts)
+ if err == nil {
+ t.Errorf("VerifyPSS success for tampered signature")
+ }
+ sig[1] ^= 0x80
+ hash[1] ^= 0x80
+ err = VerifyPSS(&priv.PublicKey, crypto.SHA256, hash[:], sig, opts)
+ if err == nil {
+ t.Errorf("VerifyPSS success for tampered message")
+ }
+ hash[1] ^= 0x80
+ }
+}
+
func testingKey(s string) string { return strings.ReplaceAll(s, "TESTING KEY", "PRIVATE KEY") }
func parseKey(s string) *PrivateKey {
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Nov 10, 2022, 1:12:46 PM11/10/22
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Gopher Robot, Adam Langley, golang-co...@googlegroups.com
Attention is currently required from: Filippo Valsorda.
Patch set 3:Code-Review +2
1 comment:
File src/crypto/rsa/rsa_test.go:
Mismatch between max here, and max indicated in the flag help text?
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
Nov 14, 2022, 6:40:29 PM11/14/22
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Filippo Valsorda, Roland Shoemaker.
Filippo Valsorda uploaded patch set #4 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Filippo Valsorda, TryBot-Result+1 by Gopher Robot
crypto/rsa: add a test walking through every key size
We already had some tests for special cases such as PSS with 513 bit
keys. The upcoming backend rewrite also happened to crash at 63 and 504
bits for different reasons. Might as well be systematic about it.
Also, make sure SignPSS returns ErrMessageTooLong like SignPKCS1v15 when
the key is too small, instead of panicking or returning an unnamed error.
-all takes a couple minutes on my M1.
Change-Id: I656239a00d0831fa7d187a6d3bb30341d41602f7
---
M src/crypto/rsa/pss.go
M src/crypto/rsa/rsa.go
M src/crypto/rsa/rsa_test.go
3 files changed, 172 insertions(+), 4 deletions(-)
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
Nov 14, 2022, 6:40:29 PM11/14/22
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Roland Shoemaker, Gopher Robot, Adam Langley, golang-co...@googlegroups.com
Attention is currently required from: Filippo Valsorda, Roland Shoemaker.
1 comment:
File src/crypto/rsa/rsa_test.go:
Mismatch between max here, and max indicated in the flag help text?
Ah yep thank you
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
Nov 14, 2022, 6:42:34 PM11/14/22
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Roland Shoemaker, Gopher Robot, Adam Langley, golang-co...@googlegroups.com
Attention is currently required from: Roland Shoemaker.
Patch set 4:Run-TryBot +1Auto-Submit +1
1 comment:
File src/crypto/rsa/rsa_test.go:
Ah yep thank you
(Resolving)
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Joedian Reid (Gerrit)
Nov 15, 2022, 12:33:36 PM11/15/22
to Filippo Valsorda, goph...@pubsubhelper.golang.org, Gopher Robot, Roland Shoemaker, Adam Langley, golang-co...@googlegroups.com
Attention is currently required from: Filippo Valsorda, Roland Shoemaker.
Patch set 5:Code-Review +1
Gopher Robot (Gerrit)
Nov 15, 2022, 12:33:49 PM11/15/22
to Filippo Valsorda, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Joedian Reid, Roland Shoemaker, Adam Langley, golang-co...@googlegroups.com
Gopher Robot submitted this change.
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/crypto/rsa/rsa_test.go
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Approvals:
Roland Shoemaker: Looks good to me, approved
Joedian Reid: Looks good to me, but someone else must approve
Filippo Valsorda: Run TryBots; Automatically submit change
Gopher Robot: TryBots succeeded
crypto/rsa: add a test walking through every key size
We already had some tests for special cases such as PSS with 513 bit
keys. The upcoming backend rewrite also happened to crash at 63 and 504
bits for different reasons. Might as well be systematic about it.
Also, make sure SignPSS returns ErrMessageTooLong like SignPKCS1v15 when
the key is too small, instead of panicking or returning an unnamed error.
-all takes a couple minutes on my M1.
Change-Id: I656239a00d0831fa7d187a6d3bb30341d41602f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/443195
Reviewed-by: Roland Shoemaker <rol...@golang.org>
Run-TryBot: Filippo Valsorda <fil...@golang.org>
Reviewed-by: Joedian Reid <joe...@golang.org>
Auto-Submit: Filippo Valsorda <fil...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/crypto/rsa/pss.go
M src/crypto/rsa/rsa.go
M src/crypto/rsa/rsa_test.go
3 files changed, 178 insertions(+), 4 deletions(-)
diff --git a/src/crypto/rsa/pss.go b/src/crypto/rsa/pss.go
index a3e9bfc..fd9fc2e 100644
--- a/src/crypto/rsa/pss.go
+++ b/src/crypto/rsa/pss.go
@@ -49,7 +49,7 @@
// 3. If emLen < hLen + sLen + 2, output "encoding error" and stop.
if emLen < hLen+sLen+2 {
- return nil, errors.New("crypto/rsa: key size too small for PSS signature")
+ return nil, ErrMessageTooLong
}
em := make([]byte, emLen)
diff --git a/src/crypto/rsa/rsa.go b/src/crypto/rsa/rsa.go
index 9c57595..34218e5 100644
--- a/src/crypto/rsa/rsa.go
+++ b/src/crypto/rsa/rsa.go
@@ -424,9 +424,10 @@
}
}
-// ErrMessageTooLong is returned when attempting to encrypt a message which is
-// too large for the size of the public key.
-var ErrMessageTooLong = errors.New("crypto/rsa: message too long for RSA public key size")
+// ErrMessageTooLong is returned when attempting to encrypt or sign a message
+// which is too large for the size of the key. When using SignPSS, this can also
+// be returned if the size of the salt is too large.
+var ErrMessageTooLong = errors.New("crypto/rsa: message too long for RSA key size")
func encrypt(c *big.Int, pub *PublicKey, m *big.Int) *big.Int {
boring.Unreachable()
diff --git a/src/crypto/rsa/rsa_test.go b/src/crypto/rsa/rsa_test.go
index 400f40a..0b1c8fb 100644
--- a/src/crypto/rsa/rsa_test.go
+++ b/src/crypto/rsa/rsa_test.go
@@ -14,6 +14,7 @@
"crypto/sha256"
"crypto/x509"
"encoding/pem"
+ "flag"
"fmt"
"math/big"
"strings"
@@ -135,6 +136,154 @@
}
}
+var allFlag = flag.Bool("all", false, "test all key sizes up to 2048")To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Cherry Mui (Gerrit)
Nov 15, 2022, 2:01:25 PM11/15/22
to Gopher Robot, Filippo Valsorda, goph...@pubsubhelper.golang.org, Joedian Reid, Roland Shoemaker, Adam Langley, golang-co...@googlegroups.com
1 comment:
Patchset:
This CL breaks the builders. Sent CL 450738 to fix. Thanks.
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Nov 15, 2022, 2:25:06 PM11/15/22
to Gopher Robot, Filippo Valsorda, goph...@pubsubhelper.golang.org, Bryan Mills, Cherry Mui, Joedian Reid, Roland Shoemaker, Adam Langley, golang-co...@googlegroups.com
1 comment:
Patchset:
This CL breaks the builders. Sent CL 450738 to fix. Thanks.
FWIW, it appears that the TryBots missed this because the check was enabled in CL 450435, which landed after the last rebase of this CL.
To view, visit change 443195. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages