Skip to content

Commit

Permalink
crypto/dsa: prevent bad public keys from causing panic
Browse files Browse the repository at this point in the history
dsa.Verify might currently use a nil s inverse in a
multiplication if the public key contains a non-prime Q,
causing a panic. Change this to check that the mod
inverse exists before using it.

Fixes CVE-2019-17596

Fixes #34960

Change-Id: I94d5f3cc38f1b5d52d38dcb1d253c71b7fd1cae7
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/572809
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/205441
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
  • Loading branch information
katiehockman committed Nov 5, 2019
1 parent 7e71c9c commit 552987f
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/crypto/dsa/dsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ func Verify(pub *PublicKey, hash []byte, r, s *big.Int) bool {
}

w := new(big.Int).ModInverse(s, pub.Q)
if w == nil {
return false
}

n := pub.Q.BitLen()
if n%8 != 0 {
Expand Down
15 changes: 15 additions & 0 deletions src/crypto/dsa/dsa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,21 @@ func TestSignAndVerify(t *testing.T) {
testSignAndVerify(t, 0, &priv)
}

func TestSignAndVerifyWithBadPublicKey(t *testing.T) {
pub := PublicKey{
Parameters: Parameters{
P: fromHex("A9B5B793FB4785793D246BAE77E8FF63CA52F442DA763C440259919FE1BC1D6065A9350637A04F75A2F039401D49F08E066C4D275A5A65DA5684BC563C14289D7AB8A67163BFBF79D85972619AD2CFF55AB0EE77A9002B0EF96293BDD0F42685EBB2C66C327079F6C98000FBCB79AACDE1BC6F9D5C7B1A97E3D9D54ED7951FEF"),
Q: fromHex("FA"),
G: fromHex("634364FC25248933D01D1993ECABD0657CC0CB2CEED7ED2E3E8AECDFCDC4A25C3B15E9E3B163ACA2984B5539181F3EFF1A5E8903D71D5B95DA4F27202B77D2C44B430BB53741A8D59A8F86887525C9F2A6A5980A195EAA7F2FF910064301DEF89D3AA213E1FAC7768D89365318E370AF54A112EFBA9246D9158386BA1B4EEFDA"),
},
Y: fromHex("32969E5780CFE1C849A1C276D7AEB4F38A23B591739AA2FE197349AEEBD31366AEE5EB7E6C6DDB7C57D02432B30DB5AA66D9884299FAA72568944E4EEDC92EA3FBC6F39F53412FBCC563208F7C15B737AC8910DBC2D9C9B8C001E72FDC40EB694AB1F06A5A2DBD18D9E36C66F31F566742F11EC0A52E9F7B89355C02FB5D32D2"),
}

if Verify(&pub, []byte("testing"), fromHex("2"), fromHex("4")) {
t.Errorf("Verify unexpected success with non-existant mod inverse of Q")
}
}

func TestSigningWithDegenerateKeys(t *testing.T) {
// Signing with degenerate private keys should not cause an infinite
// loop.
Expand Down

0 comments on commit 552987f

Please sign in to comment.