Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python SSL stack doesn't support DH ciphers #57835

Closed
naif mannequin opened this issue Dec 18, 2011 · 12 comments
Closed

Python SSL stack doesn't support DH ciphers #57835

naif mannequin opened this issue Dec 18, 2011 · 12 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@naif
Copy link
Mannequin

naif mannequin commented Dec 18, 2011

BPO 13626
Nosy @jcea, @pitrou, @meadori
Files
  • dh.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2011-12-22.09:05:35.981>
    created_at = <Date 2011-12-18.13:06:06.785>
    labels = ['type-feature', 'library']
    title = "Python SSL stack doesn't support DH ciphers"
    updated_at = <Date 2011-12-22.09:05:35.981>
    user = 'https://bugs.python.org/naif'

    bugs.python.org fields:

    activity = <Date 2011-12-22.09:05:35.981>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2011-12-22.09:05:35.981>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2011-12-18.13:06:06.785>
    creator = 'naif'
    dependencies = []
    files = ['24053']
    hgrepos = []
    issue_num = 13626
    keywords = ['patch']
    message_count = 12.0
    messages = ['149749', '149759', '149766', '149770', '149772', '149829', '149833', '149834', '149880', '150000', '150082', '150083']
    nosy_count = 5.0
    nosy_names = ['jcea', 'pitrou', 'meador.inge', 'python-dev', 'naif']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue13626'
    versions = ['Python 3.3']

    @naif
    Copy link
    Mannequin Author

    naif mannequin commented Dec 18, 2011

    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

    @naif naif mannequin added the stdlib Python modules in the Lib dir label Dec 18, 2011
    @naif
    Copy link
    Mannequin Author

    naif mannequin commented Dec 18, 2011

    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 */

    @pitrou
    Copy link
    Member

    pitrou commented Dec 18, 2011

    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.

    @pitrou pitrou added the type-feature A feature request or enhancement label Dec 18, 2011
    @naif
    Copy link
    Mannequin Author

    naif mannequin commented Dec 18, 2011

    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.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 18, 2011

    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.

    @naif
    Copy link
    Mannequin Author

    naif mannequin commented Dec 19, 2011

    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?

    @pitrou
    Copy link
    Member

    pitrou commented Dec 19, 2011

    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.

    @naif
    Copy link
    Mannequin Author

    naif mannequin commented Dec 19, 2011

    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

    @pitrou
    Copy link
    Member

    pitrou commented Dec 19, 2011

    Here is a patch adding the load_dh_params method on SSL contexts, and the OP_SINGLE_DH_USE option flag.

    @meadori
    Copy link
    Member

    meadori commented Dec 21, 2011

    Per the Red Hat problems in bpo-13627 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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 22, 2011

    New changeset 33dea851f918 by Antoine Pitrou in branch 'default':
    Issue bpo-13626: Add support for SSL Diffie-Hellman key exchange, through the
    http://hg.python.org/cpython/rev/33dea851f918

    @pitrou
    Copy link
    Member

    pitrou commented Dec 22, 2011

    Thank you Meador. I've committed an updated patch.

    @pitrou pitrou closed this as completed Dec 22, 2011
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants