classification
Title: Python SSL stack doesn't support DH ciphers
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jcea, meador.inge, naif, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2011-12-18 13:06 by naif, last changed 2011-12-22 09:05 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
dh.patch pitrou, 2011-12-19 17:37 review
Messages (12)
msg149749 - (view) Author: naif (naif) Date: 2011-12-18 13:06
Python SSL doesn't support DH ciphers in in all version tested.

This is a serious security issue because it's not possible to use as a server or client Perfect Forward Secrecy [1] security provided by DHE and ECDH ciphers .

In order to enable DH ciphers the SSL implementation the in the file Modules/_ssl.c, it must issue a DH_generate_parameters() if a cipher is DH.

For example PHP handling of DH ciphers, look php-5.3.8/ext/openssl/openssl.c : 

#if !defined(NO_DH)
                        case OPENSSL_KEYTYPE_DH:
                                {
                                        DH *dhpar = DH_generate_parameters(req->priv_key_bits, 2, NULL, NULL);
                                        int codes = 0;

                                        if (dhpar) {
                                                DH_set_method(dhpar, DH_get_default_method());
                                                if (DH_check(dhpar, &codes) && codes == 0 && DH_generate_key(dhpar)) {
                                                        if (EVP_PKEY_assign_DH(req->priv_key, dhpar)) {
                                                                return_val = req->priv_key;
                                                        }
                                                } else {
                                                        DH_free(dhpar);
                                                }
                                        }
                                }
                                break;
#endif
                        default:


An important security fix, to support and enable by default DH ciphers has to be done.

[1] http://en.wikipedia.org/wiki/Perfect_forward_secrecy
msg149759 - (view) Author: naif (naif) Date: 2011-12-18 14:25
Other example for DH and ECC from:
https://github.com/bumptech/stud/blob/master/stud.c

#ifndef OPENSSL_NO_DH
static int init_dh(SSL_CTX *ctx, const char *cert) {
    DH *dh;
    BIO *bio;

    assert(cert);

    bio = BIO_new_file(cert, "r");
    if (!bio) {
      ERR_print_errors_fp(stderr);
      return -1;
    }

    dh = PEM_read_bio_DHparams(bio, NULL, NULL, NULL);
    BIO_free(bio);
    if (!dh) {
        ERR("{core} Note: no DH parameters found in %s\n", cert);
        return -1;
    }

    LOG("{core} Using DH parameters from %s\n", cert);
    SSL_CTX_set_tmp_dh(ctx, dh);
    LOG("{core} DH initialized with %d bit key\n", 8*DH_size(dh));
    DH_free(dh);

#ifdef NID_X9_62_prime256v1
    EC_KEY *ecdh = NULL;
    ecdh = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
    SSL_CTX_set_tmp_ecdh(ctx,ecdh);
    EC_KEY_free(ecdh);
    LOG("{core} ECDH Initialized with NIST P-256\n");
#endif

    return 0;
}
#endif /* OPENSSL_NO_DH */
msg149766 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-18 15:10
The ssl module doesn't directly handle keys, it just gives a PEM file to OpenSSL's ssl functions. So I don't understand what should be done precisely here, or even if something has to be done at all.
msg149770 - (view) Author: naif (naif) Date: 2011-12-18 15:46
Please look at how PHP implement the feature.
It doesn't use any PEM or any Key File, but just initiatlize the DH parameters.

Stud instead, ask the user to generate "offline" the DH parameters and save it into the PEM file.

I think that the PHP approach it's better than the STUD one:
It does not require any file or key to generate DH parameters.

This is the way to have supported ciphers such as DHE-RSA-AES256-SHA (
http://www.openssl.org/docs/apps/ciphers.html ) that now cannot be used because the Python SSL binding doesn't initialize the DH parameters.
msg149772 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-18 16:03
Well the OpenSSL docs say “DH_generate_parameters() may run for several hours before finding a suitable prime”, which sounds like a good reason not to do it every time your program is run.

Anyway, SSL_CTX_set_tmp_dh() should allow us to set DH parameters on a SSL context, PEM_read_DHparams() to read them from a PEM file, and OpenSSL's source tree has a couple of PEM files with "strong" DH parameters for various key sizes.
msg149829 - (view) Author: naif (naif) Date: 2011-12-19 10:37
Wow, i saw your patch for ECC SSL ciphers on http://bugs.python.org/issue13627 .

Do you think we can use the same method/concept as ssl.OP_SINGLE_ECDH_USE but ssl.OP_SINGLE_DH_USE for DH?
msg149833 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-19 10:45
> Wow, i saw your patch for ECC SSL ciphers on http://bugs.python.org/issue13627 .
> 
> Do you think we can use the same method/concept as
> ssl.OP_SINGLE_ECDH_USE but ssl.OP_SINGLE_DH_USE for DH?

Of course.
msg149834 - (view) Author: naif (naif) Date: 2011-12-19 10:48
In the meantime i added two other tickets on security and performance improvements of Python SSL support, to make it really complete and comparable to Apache/Dovecot/PHP in terms of configuration and capability: 

Python SSL stack doesn't support ordering of Ciphers
http://bugs.python.org/issue13635

Python SSL stack doesn't support Compression configuration
http://bugs.python.org/issue13634
msg149880 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-19 17:37
Here is a patch adding the load_dh_params method on SSL contexts, and the OP_SINGLE_DH_USE option flag.
msg150000 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-12-21 15:54
Per the Red Hat problems in issue13627 I just tried this patch on Fedora 16.  Everything built just fine.  However, the patch doesn't apply cleanly to tip an longer:

[meadori@motherbrain cpython]$ patch -p1 < ../patches/dh.patch 
patching file Doc/library/ssl.rst
Hunk #2 succeeded at 715 (offset 27 lines).
patching file Lib/ssl.py
Hunk #1 succeeded at 68 with fuzz 2.
patching file Lib/test/dh512.pem
patching file Lib/test/ssl_servers.py
Hunk #1 succeeded at 180 (offset 1 line).
Hunk #2 succeeded at 194 (offset 1 line).
patching file Lib/test/test_ssl.py
Hunk #2 succeeded at 101 with fuzz 2.
Hunk #3 succeeded at 541 (offset 3 lines).
Hunk #4 FAILED at 1200.
Hunk #5 succeeded at 1858 with fuzz 2 (offset 29 lines).
1 out of 5 hunks FAILED -- saving rejects to file Lib/test/test_ssl.py.rej
patching file Modules/_ssl.c
Hunk #1 succeeded at 1922 (offset 20 lines).
Hunk #2 succeeded at 2082 (offset 22 lines).
Hunk #3 succeeded at 2539 with fuzz 2 (offset 24 lines).

After fixing the unit test hunk everything builds and the SSL unit tests pass.
msg150082 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-22 09:04
New changeset 33dea851f918 by Antoine Pitrou in branch 'default':
Issue #13626: Add support for SSL Diffie-Hellman key exchange, through the
http://hg.python.org/cpython/rev/33dea851f918
msg150083 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-22 09:05
Thank you Meador. I've committed an updated patch.
History
Date User Action Args
2011-12-22 09:05:35pitrousetstatus: open -> closed
resolution: fixed
messages: + msg150083

stage: patch review -> resolved
2011-12-22 09:04:21python-devsetnosy: + python-dev
messages: + msg150082
2011-12-21 15:54:12meador.ingesetnosy: + meador.inge
messages: + msg150000
2011-12-19 17:37:22pitrousetfiles: + dh.patch
keywords: + patch
messages: + msg149880

stage: needs patch -> patch review
2011-12-19 10:48:30naifsetmessages: + msg149834
2011-12-19 10:45:15pitrousetmessages: + msg149833
2011-12-19 10:37:57naifsetmessages: + msg149829
2011-12-19 01:54:52jceasetnosy: + jcea
2011-12-18 16:03:52pitrousetmessages: + msg149772
stage: needs patch
2011-12-18 15:46:24naifsetmessages: + msg149770
2011-12-18 15:10:49pitrousetversions: - Python 2.6, Python 3.1, Python 2.7, Python 3.2, Python 3.4
nosy: + pitrou

messages: + msg149766

type: enhancement
2011-12-18 14:25:02naifsetmessages: + msg149759
2011-12-18 13:06:06naifcreate