Skip to content

Commit

Permalink
Fix signature buffer size for RSA keys
Browse files Browse the repository at this point in the history
When using the pyOpenSSL crypto module to sign data using a large key,
e.g. 8192 bit, a memory allocation error occurs. A test case to show
this, which comes from OpenStack Glance, is:

```
  $ openssl genrsa -out server.key 8192
  $ ...
  $ cat test.py
  from OpenSSL import crypto
  import uuid
  key_file = 'server.key'
  with open(key_file, 'r') as keyfile:
      key_str = keyfile.read()
  key = crypto.load_privatekey(crypto.FILETYPE_PEM, key_str)
  data = str(uuid.uuid4())
  digest = 'sha256'
  crypto.sign(key, data, digest)
  $ python test.py
  *** Error in `python': free(): invalid next size (normal): 0x0000000002879050 ***
  Aborted
```

Other errors that may appear to the user are:

```
  Segmentation Fault
```

```
  *** Error in `python': double free or corruption (!prev): 0x0000000001245300 ***
  Aborted
```

```
  *** Error in `python': munmap_chunk(): invalid pointer: 0x0000000001fde540 ***
  Aborted
```

The reason this happens is that the sign function of the crypto module
hard-codes the size of the signature buffer to 512 bytes (4096 bits).
An RSA key generates a signature that can be up to the size of the
private key modulus, so for an 8192 bit key, a buffer for a 4096 bit
signature is too short and causes a memory allocation error.

Technically the maximum size key this code should be able to handle is
4096 bits, but due to memory allocation alignment the problem only
becomes apparent for keys of at least 4161 bits.

This patch does two things. First, it determines the correct size of
the signature buffer, in bytes, based on the real size of the private
key, and passes that the buffer allocation instead of the static number
512. Second, it no longer passes in a signature length. This is because
the OpenSSL EVP_SignFinal function uses this argument as an output and
completely ignores it as an input[1], so there is no need for us to set
it.

This is only a problem for RSA keys, and this patch only affects RSA
keys. For DSA keys, the key size is restricted to 1024 bits (128
bytes), and the signature a DSA key will generate will be about 46
bytes, so this buffer will still be big enough for DSA signatures.

[1] https://github.com/openssl/openssl/blob/349807608f31b20af01a342d0072bb92e0b036e2/crypto/evp/p_sign.c#L74
  • Loading branch information
cmurphy committed Mar 2, 2016
1 parent 565d3b8 commit e09399b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/OpenSSL/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -2583,9 +2583,9 @@ def sign(pkey, data, digest):
_lib.EVP_SignInit(md_ctx, digest_obj)
_lib.EVP_SignUpdate(md_ctx, data, len(data))

signature_buffer = _ffi.new("unsigned char[]", 512)
pkey_length = (PKey.bits(pkey) + 7) // 8
signature_buffer = _ffi.new("unsigned char[]", pkey_length)
signature_length = _ffi.new("unsigned int*")
signature_length[0] = len(signature_buffer)
final_result = _lib.EVP_SignFinal(
md_ctx, signature_buffer, signature_length, pkey._pkey)

Expand Down
68 changes: 68 additions & 0 deletions tests/test_crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,60 @@ def normalize_privatekey_pem(pem):
RFEIPQsFZRLrtnCAiEhyT8bC2s/Njlu6ly9gtJZWSV46Q3ZjBL4q9sHKqZQ=
-----END CERTIFICATE-----""")

large_key_pem = b("""-----BEGIN RSA PRIVATE KEY-----
MIIJYgIBAAKCAg4AtRua8eIeevRfsj+fkcHr1vmse7Kgb+oX1ssJAvCb1R7JQMnH
hNDjDP6b3vEkZuPUzlDHymP+cNkXvvi4wJ4miVbO3+SeU4Sh+jmsHeHzGIXat9xW
9PFtuPM5FQq8zvkY8aDeRYmYwN9JKu4/neMBCBqostYlTEWg+bSytO/qWnyHTHKh
g0GfaDdqUQPsGQw+J0MgaYIjQOCVASHAPlzbDQLCtuOb587rwTLkZA2GwoHB/LyJ
BwT0HHgBaiObE12Vs6wi2en0Uu11CiwEuK1KIBcZ2XbE6eApaZa6VH9ysEmUxPt7
TqyZ4E2oMIYaLPNRxuvozdwTlj1svI1k1FrkaXGc5MTjbgigPMKjIb0T7b/4GNzt
DhP1LvAeUMnrEi3hJJrcJPXHPqS8/RiytR9xQQW6Sdh4LaA3f9MQm3WSevWage3G
P8YcCLssOVKsArDjuA52NF5LmYuAeUzXprm4ITDi2oO+0iFBpFW6VPEK4A9vO0Yk
M/6Wt6tG8zyWhaSH1zFUTwfQ9Yvjyt5w1lrUaAJuoTpwbMVZaDJaEhjOaXU0dyPQ
jOsePDOQcU6dkeTWsQ3LsHPEEug/X6819TLG5mb3V7bvV9nPFBfTJSCEG794kr90
XgZfIN71FrdByxLerlbuJI21pPs/nZi9SXi9jAWeiS45/azUxMsyYgJArui+gjq7
sV1pWiBm6/orAgMBAAECggINQp5L6Yu+oIXBqcSjgq8tfF9M5hd30pLuf/EheHZf
LA7uAqn2fVGFI2OInIJhXIOT5OxsAXO0xXfltzawZxIFpOFMqajj4F7aYjvSpw9V
J4EdSiJ/zgv8y1qUdbwEZbHVThRZjoSlrtSzilonBoHZAE0mHtqMz7iRFSk1zz6t
GunRrvo/lROPentf3TsvHquVNUYI5yaapyO1S7xJhecMIIYSb8nbsHI54FBDGNas
6mFmpPwI/47/6HTwOEWupnn3NicsjrHzUInOUpaMig4cRR+aP5bjqg/ty8xI8AoN
evEmCytiWTc+Rvbp1ieN+1jpjN18PjUk80/W7qioHUDt4ieLic8uxWH2VD9SCEnX
Mpi9tA/FqoZ+2A/3m1OfrY6jiZVE2g+asi9lCK7QVWL39eK82H4rPvtp0/dyo1/i
ZZz68TXg+m8IgEZcp88hngbkuoTTzpGE73QuPKhGA1uMIimDdqPPB5WP76q+03Oi
IRR5DfZnqPERed49by0enJ7tKa/gFPZizOV8ALKr0Dp+vfAkxGDLPLBLd2A3//tw
xg0Q/wltihHSBujv4nYlDXdc5oYyMYZ+Lhc/VuOghHfBq3tgEQ1ECM/ofqXEIdy7
nVcpZn3Eeq8Jl5CrqxE1ee3NxlzsJHn99yGQpr7mOhW/psJF3XNz80Meg3L4m1T8
sMBK0GbaassuJhdzb5whAoIBBw48sx1b1WR4XxQc5O/HjHva+l16i2pjUnOUTcDF
RWmSbIhBm2QQ2rVhO8+fak0tkl6ZnMWW4i0U/X5LOEBbC7+IS8bO3j3Revi+Vw5x
j96LMlIe9XEub5i/saEWgiz7maCvfzLFU08e1OpT4qPDpP293V400ubA6R7WQTCv
pBkskGwHeu0l/TuKkVqBFFUTu7KEbps8Gjg7MkJaFriAOv1zis/umK8pVS3ZAM6e
8w5jfpRccn8Xzta2fRwTB5kCmfxdDsY0oYGxPLRAbW72bORoLGuyyPp/ojeGwoik
JX9RttErc6FjyZtks370Pa8UL5QskyhMbDhrZW2jFD+RXYM1BrvmZRjbAoIBBwy4
iFJpuDfytJfz1MWtaL5DqEL/kmiZYAXl6hifNhGu5GAipVIIGsDqEYW4i+VC15aa
7kOCwz/I5zsB3vSDW96IRs4wXtqEZSibc2W/bqfVi+xcvPPl1ZhQ2EAwa4D/x035
kyf20ffWOU+1yf2cnijzqs3IzlveUm+meLw5s3Rc+iG7DPWWeCoe1hVwANI1euNc
pqKwKY905yFyjOje2OgiEU2kS4YME4zGeBys8yo7E42hNnN2EPK6xkkUqzdudLLQ
8OUlKRTc8AbIf3XG1rpA4VUpTv3hhxGGwCRy6If8zgZQsNYchgNztRGk72Gcb8Dm
vFSEN3ZtwxU64G3YZzntdcr2WPzxAoIBBw30g6Fgdb/gmVnOpL0//T0ePNDKIMPs
jVJLaRduhoZgB1Bb9qPUPX0SzRzLZtg1tkZSDjBDoHmOHJfhxUaXt+FLCPPbrE4t
+nq9n/nBaMM779w9ClqhqLOyGrwKoxjSmhi+TVEHyIxCbXMvPHVHfX9WzxjbcGrN
ZvRaEVZWo+QlIX8yqdSwqxLk1WtAIRzvlcj7NKum8xBxPed6BNFep/PtgIAmoLT5
L8wb7EWb2iUdc2KbZ4OaY51lDScqpATgXu3WjXfM+Q52G0mX6Wyd0cjlL711Zrjb
yLbiueZT94lgIHHRRKtKc8CEqcjkQV5OzABS3P/gQSfgZXBdLKjOpTnKDUq7IBeH
AoIBBweAOEIAPLQg1QRUrr3xRrYKRwlakgZDii9wJt1l5AgBTICzbTA1vzDJ1JM5
AqSpCV6w9JWyYVcXK+HLdKBRZLaPPNEQDJ5lOxD6uMziWGl2rg8tj+1xNMWfxiPz
aTCjoe4EoBUMoTq2gwzRcM2usEQNikXVhnj9Wzaivsaeb4bJ3GRPW5DkrO6JSEtT
w+gvyMqQM2Hy5k7E7BT46sXVwaj/jZxuqGnebRixXtnp0WixdRIqYWUr1UqLf6hQ
G7WP2BgoxCMaCmNW8+HMD/xuxucEotoIhZ+GgJKBFoNnjl3BX+qxYdSe9RbL/5Tr
4It6Jxtj8uETJXEbv9Cg6v1agWPS9YY8RLTBAoIBBwrU2AsAUts6h1LgGLKK3UWZ
oLH5E+4o+7HqSGRcRodVeN9NBXIYdHHOLeEG6YNGJiJ3bFP5ZQEu9iDsyoFVKJ9O
Mw/y6dKZuxOCZ+X8FopSROg3yWfdOpAm6cnQZp3WqLNX4n/Q6WvKojfyEiPphjwT
0ymrUJELXLWJmjUyPoAk6HgC0Gs28ZnEXbyhx7CSbZNFyCU/PNUDZwto3GisIPD3
le7YjqHugezmjMGlA0sDw5aCXjfbl74vowRFYMO6e3ItApfSRgNV86CDoX74WI/5
AYU/QVM4wGt8XGT2KwDFJaxYGKsGDMWmXY04dS+WPuetCbouWUusyFwRb9SzFave
vYeU7Ab/
-----END RSA PRIVATE KEY-----""")


class X509ExtTests(TestCase):
"""
Expand Down Expand Up @@ -3484,6 +3538,20 @@ def test_sign_nulls(self):
sig = sign(priv_key, content, "sha1")
verify(good_cert, sig, content, "sha1")

def test_sign_with_large_key(self):
"""
:py:obj:`sign` produces a signature for a string when using a long key.
"""
content = b(
"It was a bright cold day in April, and the clocks were striking "
"thirteen. Winston Smith, his chin nuzzled into his breast in an "
"effort to escape the vile wind, slipped quickly through the "
"glass doors of Victory Mansions, though not quickly enough to "
"prevent a swirl of gritty dust from entering along with him.")

priv_key = load_privatekey(FILETYPE_PEM, large_key_pem)
sign(priv_key, content, "sha1")


class EllipticCurveTests(TestCase):
"""
Expand Down

0 comments on commit e09399b

Please sign in to comment.