From e09399bd21e44255cf327c295dced50ac2dd0b45 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Tue, 1 Mar 2016 17:40:49 -0800 Subject: [PATCH] Fix signature buffer size for RSA keys 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 --- src/OpenSSL/crypto.py | 4 +-- tests/test_crypto.py | 68 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py index 6d78bd76c..9a3a05a1f 100644 --- a/src/OpenSSL/crypto.py +++ b/src/OpenSSL/crypto.py @@ -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) diff --git a/tests/test_crypto.py b/tests/test_crypto.py index 804777964..03b94b9a7 100644 --- a/tests/test_crypto.py +++ b/tests/test_crypto.py @@ -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): """ @@ -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): """