From 59d26251efd8a2a08abd9029018194430f7f25ca Mon Sep 17 00:00:00 2001
From: Paul Kehrer <paul.l.kehrer@gmail.com>
Date: Thu, 20 Jul 2017 10:45:54 +0200
Subject: (EC)DSA signature fix (#670)

* Write a test - signatures with EC keys (#609)

* Ask for signature length before allocating a buffer.

This fixes a potential heap buffer overflow that may happen when a signature
is longer than the private key, as with X9.62 ECDSA (#609).

* change approach to EVP_PKEY_size and add changelog

* add a small assert
---
 CHANGELOG.rst         |  2 ++
 src/OpenSSL/crypto.py |  7 ++++---
 tests/test_crypto.py  | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 738ceab..ba9a124 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -27,6 +27,8 @@ Changes:
 
 - Fixed a bug causing ``Context.set_default_verify_paths()`` to not work with cryptography ``manylinux1`` wheels on Python 3.x.
   `#665 <https://github.com/pyca/pyopenssl/pull/665>`_
+- Fixed a crash with (EC)DSA signatures in some cases.
+  `#670 <https://github.com/pyca/pyopenssl/pull/670>`_
 
 
 ----
diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py
index 20cf183..4f7e4d8 100644
--- a/src/OpenSSL/crypto.py
+++ b/src/OpenSSL/crypto.py
@@ -2801,9 +2801,10 @@ def sign(pkey, data, digest):
     _lib.EVP_SignInit(md_ctx, digest_obj)
     _lib.EVP_SignUpdate(md_ctx, data, len(data))
 
-    pkey_length = (PKey.bits(pkey) + 7) // 8
-    signature_buffer = _ffi.new("unsigned char[]", pkey_length)
-    signature_length = _ffi.new("unsigned int*")
+    length = _lib.EVP_PKEY_size(pkey._pkey)
+    _openssl_assert(length > 0)
+    signature_buffer = _ffi.new("unsigned char[]", length)
+    signature_length = _ffi.new("unsigned int *")
     final_result = _lib.EVP_SignFinal(
         md_ctx, signature_buffer, signature_length, pkey._pkey)
     _openssl_assert(final_result == 1)
diff --git a/tests/test_crypto.py b/tests/test_crypto.py
index 47a914d..2f8a70d 100644
--- a/tests/test_crypto.py
+++ b/tests/test_crypto.py
@@ -544,6 +544,29 @@ zo0MUVPQgwJ3aJtNM1QMOQUayCrRwfklg+D/rFSUwEUqtZh7fJDiFqz3
 -----END PRIVATE KEY-----
 """
 
+ec_root_key_pem = b"""-----BEGIN EC PRIVATE KEY-----
+MIGlAgEBBDEAz/HOBFPYLB0jLWeTpJn4Yc4m/C4mdWymVHBjOmnwiPHKT326iYN/
+ZhmSs+RM94RsoAcGBSuBBAAioWQDYgAEwE5vDdla/nLpWAPAQ0yFGqwLuw4BcN2r
+U+sKab5EAEHzLeceRa8ffncYdCXNoVsBcdob1y66CFZMEWLetPTmGapyWkBAs6/L
+8kUlkU9OsE+7IVo4QQJkgV5gM+Dim1XE
+-----END EC PRIVATE KEY-----
+"""
+
+ec_root_cert_pem = b"""-----BEGIN CERTIFICATE-----
+MIICLTCCAbKgAwIBAgIMWW/hwTl6ufz6/WkCMAoGCCqGSM49BAMDMFgxGDAWBgNV
+BAMTD1Rlc3RpbmcgUm9vdCBDQTEQMA4GA1UEChMHVGVzdGluZzEQMA4GA1UEBxMH
+Q2hpY2FnbzELMAkGA1UECBMCSUwxCzAJBgNVBAYTAlVTMCAXDTE3MDcxOTIyNDgz
+M1oYDzk5OTkxMjMxMjM1OTU5WjBYMRgwFgYDVQQDEw9UZXN0aW5nIFJvb3QgQ0Ex
+EDAOBgNVBAoTB1Rlc3RpbmcxEDAOBgNVBAcTB0NoaWNhZ28xCzAJBgNVBAgTAklM
+MQswCQYDVQQGEwJVUzB2MBAGByqGSM49AgEGBSuBBAAiA2IABMBObw3ZWv5y6VgD
+wENMhRqsC7sOAXDdq1PrCmm+RABB8y3nHkWvH353GHQlzaFbAXHaG9cuughWTBFi
+3rT05hmqclpAQLOvy/JFJZFPTrBPuyFaOEECZIFeYDPg4ptVxKNDMEEwDwYDVR0T
+AQH/BAUwAwEB/zAPBgNVHQ8BAf8EBQMDBwQAMB0GA1UdDgQWBBSoTrF0H2m8RDzB
+MnY2KReEPfz7ZjAKBggqhkjOPQQDAwNpADBmAjEA3+G1oVCxGjYX4iUN93QYcNHe
+e3fJQJwX9+KsHRut6qNZDUbvRbtO1YIAwB4UJZjwAjEAtXCPURS5A4McZHnSwgTi
+Td8GMrwKz0557OxxtKN6uVVy4ACFMqEw0zN/KJI1vxc9
+-----END CERTIFICATE-----"""
+
 
 @pytest.fixture
 def x509_data():
@@ -3697,6 +3720,24 @@ class TestSignVerify(object):
                     WARNING_TYPE_EXPECTED
                 ) == str(w[-1].message))
 
+    def test_sign_verify_ecdsa(self):
+        """
+        `sign` generates a cryptographic signature which `verify` can check.
+        ECDSA Signatures in the X9.62 format may have variable length,
+        different from the length of the private key.
+        """
+        content = (
+            b"It was a bright cold day in April, and the clocks were striking "
+            b"thirteen. Winston Smith, his chin nuzzled into his breast in an "
+            b"effort to escape the vile wind, slipped quickly through the "
+            b"glass doors of Victory Mansions, though not quickly enough to "
+            b"prevent a swirl of gritty dust from entering along with him."
+        ).decode("ascii")
+        priv_key = load_privatekey(FILETYPE_PEM, ec_root_key_pem)
+        cert = load_certificate(FILETYPE_PEM, ec_root_cert_pem)
+        sig = sign(priv_key, content, "sha1")
+        verify(cert, sig, content, "sha1")
+
     def test_sign_nulls(self):
         """
         `sign` produces a signature for a string with embedded nulls.
-- 
cgit v1.2.1