Issue #13626: Add support for SSL Diffie-Hellman key exchange, through the

SSLContext.load_dh_params() method and the ssl.OP_SINGLE_DH_USE option.

7 files changed, 103 insertions, 6 deletions

diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
index 9b3306c..0525e89 100644
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@ -428,9 +428,17 @@ Constants
 
    .. versionadded:: 3.3
 
+.. data:: OP_SINGLE_DH_USE
+
+   Prevents re-use of the same DH key for distinct SSL sessions.  This
+   improves forward secrecy but requires more computational resources.
+   This option only applies to server sockets.
+
+   .. versionadded:: 3.3
+
 .. data:: OP_SINGLE_ECDH_USE
 
-   Prevents re-use of the same ECDH key for several SSL sessions.  This
+   Prevents re-use of the same ECDH key for distinct SSL sessions.  This
    improves forward secrecy but requires more computational resources.
    This option only applies to server sockets.
 
@@ -707,12 +715,24 @@ to speed up repeated connections from the same clients.
       when connected, the :meth:`SSLSocket.cipher` method of SSL sockets will
       give the currently selected cipher.
 
+.. method:: SSLContext.load_dh_params(dhfile)
+
+   Load the key generation parameters for Diffie-Helman (DH) key exchange.
+   Using DH key exchange improves forward secrecy at the expense of
+   computational resources (both on the server and on the client).
+   The *dhfile* parameter should be the path to a file containing DH
+   parameters in PEM format.
+
+   This setting doesn't apply to client sockets.  You can also use the
+   :data:`OP_SINGLE_DH_USE` option to further improve security.
+
+   .. versionadded:: 3.3
+
 .. method:: SSLContext.set_ecdh_curve(curve_name)
 
-   Set the curve name for Elliptic Curve-based Diffie-Hellman (abbreviated
-   ECDH) key exchange.  Using Diffie-Hellman key exchange improves forward
-   secrecy at the expense of computational resources (both on the server and
-   on the client).  The *curve_name* parameter should be a string describing
+   Set the curve name for Elliptic Curve-based Diffie-Hellman (ECDH) key
+   exchange.  ECDH is significantly faster than regular DH while arguably
+   as secure.  The *curve_name* parameter should be a string describing
    a well-known elliptic curve, for example ``prime256v1`` for a widely
    supported curve.
 
diff --git a/Lib/ssl.py b/Lib/ssl.py
index d43d255..b56a8c8 100644
--- a/Lib/ssl.py
+++ b/Lib/ssl.py
@@ -68,7 +68,7 @@ from _ssl import (
 from _ssl import CERT_NONE, CERT_OPTIONAL, CERT_REQUIRED
 from _ssl import (
     OP_ALL, OP_NO_SSLv2, OP_NO_SSLv3, OP_NO_TLSv1,
-    OP_CIPHER_SERVER_PREFERENCE, OP_SINGLE_ECDH_USE,
+    OP_CIPHER_SERVER_PREFERENCE, OP_SINGLE_DH_USE, OP_SINGLE_ECDH_USE,
     )
 try:
     from _ssl import OP_NO_COMPRESSION
diff --git a/Lib/test/ssl_servers.py b/Lib/test/ssl_servers.py
index becbfab..8686153 100644
--- a/Lib/test/ssl_servers.py
+++ b/Lib/test/ssl_servers.py
@@ -180,6 +180,8 @@ if __name__ == "__main__":
     parser.add_argument('--curve-name', dest='curve_name', type=str,
                         action='store',
                         help='curve name for EC-based Diffie-Hellman')
+    parser.add_argument('--dh', dest='dh_file', type=str, action='store',
+                        help='PEM file containing DH parameters')
     args = parser.parse_args()
 
     support.verbose = args.verbose
@@ -192,6 +194,8 @@ if __name__ == "__main__":
     context.load_cert_chain(CERTFILE)
     if args.curve_name:
         context.set_ecdh_curve(args.curve_name)
+    if args.dh_file:
+        context.load_dh_params(args.dh_file)
 
     server = HTTPSServer(("", args.port), handler_class, context)
     if args.verbose:
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index d549799..a4bcdd0 100644
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -56,6 +56,8 @@ WRONGCERT = data_file("XXXnonexisting.pem")
 BADKEY = data_file("badkey.pem")
 NOKIACERT = data_file("nokia.pem")
 
+DHFILE = data_file("dh512.pem")
+BYTES_DHFILE = os.fsencode(DHFILE)
 
 def handle_error(prefix):
     exc_format = ' '.join(traceback.format_exception(*sys.exc_info()))
@@ -99,6 +101,7 @@ class BasicSocketTests(unittest.TestCase):
         ssl.CERT_OPTIONAL
         ssl.CERT_REQUIRED
         ssl.OP_CIPHER_SERVER_PREFERENCE
+        ssl.OP_SINGLE_DH_USE
         ssl.OP_SINGLE_ECDH_USE
         if ssl.OPENSSL_VERSION_INFO >= (1, 0):
             ssl.OP_NO_COMPRESSION
@@ -538,6 +541,19 @@ class ContextTests(unittest.TestCase):
         # Issue #10989: crash if the second argument type is invalid
         self.assertRaises(TypeError, ctx.load_verify_locations, None, True)
 
+    def test_load_dh_params(self):
+        ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
+        ctx.load_dh_params(DHFILE)
+        if os.name != 'nt':
+            ctx.load_dh_params(BYTES_DHFILE)
+        self.assertRaises(TypeError, ctx.load_dh_params)
+        self.assertRaises(TypeError, ctx.load_dh_params, None)
+        with self.assertRaises(FileNotFoundError) as cm:
+            ctx.load_dh_params(WRONGCERT)
+        self.assertEqual(cm.exception.errno, errno.ENOENT)
+        with self.assertRaisesRegex(ssl.SSLError, "PEM routines"):
+            ctx.load_dh_params(CERTFILE)
+
     @skip_if_broken_ubuntu_ssl
     def test_session_stats(self):
         for proto in PROTOCOLS:
@@ -1802,6 +1818,19 @@ else:
                                        chatty=True, connectionchatty=True)
             self.assertIs(stats['compression'], None)
 
+        def test_dh_params(self):
+            # Check we can get a connection with ephemeral Diffie-Hellman
+            context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
+            context.load_cert_chain(CERTFILE)
+            context.load_dh_params(DHFILE)
+            context.set_ciphers("kEDH")
+            stats = server_params_test(context, context,
+                                       chatty=True, connectionchatty=True)
+            cipher = stats["cipher"][0]
+            parts = cipher.split("-")
+            if "ADH" not in parts and "EDH" not in parts and "DHE" not in parts:
+                self.fail("Non-DH cipher: " + cipher[0])
+
 
 def test_main(verbose=False):
     if support.verbose:
diff --git a/Misc/NEWS b/Misc/NEWS
index 425eedc..f051d20 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -419,6 +419,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #13626: Add support for SSL Diffie-Hellman key exchange, through the
+  SSLContext.load_dh_params() method and the ssl.OP_SINGLE_DH_USE option.
+
 - Issue #11006: Don't issue low level warning in subprocess when pipe2() fails.
 
 - Issue #13620: Support for Chrome browser in webbrowser.py Patch contributed
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index 02fe5f3..3e5ecfd 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -1922,6 +1922,38 @@ load_verify_locations(PySSLContext *self, PyObject *args, PyObject *kwds)
 }
 
 static PyObject *
+load_dh_params(PySSLContext *self, PyObject *filepath)
+{
+    FILE *f;
+    DH *dh;
+
+    f = _Py_fopen(filepath, "rb");
+    if (f == NULL) {
+        if (!PyErr_Occurred())
+            PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
+        return NULL;
+    }
+    errno = 0;
+    PySSL_BEGIN_ALLOW_THREADS
+    dh = PEM_read_DHparams(f, NULL, NULL, NULL);
+    PySSL_END_ALLOW_THREADS
+    if (dh == NULL) {
+        if (errno != 0) {
+            ERR_clear_error();
+            PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
+        }
+        else {
+            _setSSLError(NULL, 0, __FILE__, __LINE__);
+        }
+        return NULL;
+    }
+    if (SSL_CTX_set_tmp_dh(self->ctx, dh) == 0)
+        _setSSLError(NULL, 0, __FILE__, __LINE__);
+    DH_free(dh);
+    Py_RETURN_NONE;
+}
+
+static PyObject *
 context_wrap_socket(PySSLContext *self, PyObject *args, PyObject *kwds)
 {
     char *kwlist[] = {"sock", "server_side", "server_hostname", NULL};
@@ -2050,6 +2082,8 @@ static struct PyMethodDef context_methods[] = {
                     METH_VARARGS, NULL},
     {"load_cert_chain", (PyCFunction) load_cert_chain,
                         METH_VARARGS | METH_KEYWORDS, NULL},
+    {"load_dh_params", (PyCFunction) load_dh_params,
+                       METH_O, NULL},
     {"load_verify_locations", (PyCFunction) load_verify_locations,
                               METH_VARARGS | METH_KEYWORDS, NULL},
     {"session_stats", (PyCFunction) session_stats,
@@ -2505,6 +2539,7 @@ PyInit__ssl(void)
     PyModule_AddIntConstant(m, "OP_NO_TLSv1", SSL_OP_NO_TLSv1);
     PyModule_AddIntConstant(m, "OP_CIPHER_SERVER_PREFERENCE",
                             SSL_OP_CIPHER_SERVER_PREFERENCE);
+    PyModule_AddIntConstant(m, "OP_SINGLE_DH_USE", SSL_OP_SINGLE_DH_USE);
     PyModule_AddIntConstant(m, "OP_SINGLE_ECDH_USE", SSL_OP_SINGLE_ECDH_USE);
 #ifdef SSL_OP_NO_COMPRESSION
     PyModule_AddIntConstant(m, "OP_NO_COMPRESSION",
diff --git a/Python/fileutils.c b/Python/fileutils.c
index 1e71431..8993c8c 100644
--- a/Python/fileutils.c
+++ b/Python/fileutils.c
@@ -310,6 +310,12 @@ _Py_fopen(PyObject *path, const char *mode)
     wchar_t wmode[10];
     int usize;
 
+    if (!PyUnicode_Check(path)) {
+        PyErr_Format(PyExc_TypeError,
+                     "str file path expected under Windows, got %R",
+                     Py_TYPE(path));
+        return NULL;
+    }
     wpath = PyUnicode_AsUnicode(path);
     if (wpath == NULL)
         return NULL;