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

test_ssl failures with OpenSSL 1.0.0 #52569

Closed
pitrou opened this issue Apr 5, 2010 · 10 comments
Closed

test_ssl failures with OpenSSL 1.0.0 #52569

pitrou opened this issue Apr 5, 2010 · 10 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@pitrou
Copy link
Member

pitrou commented Apr 5, 2010

BPO 8322
Nosy @pitrou, @vstinner, @giampaolo, @benjaminp, @florentx
Files
  • ciphers.patch
  • ciphers2.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 2010-04-17.17:41:14.597>
    created_at = <Date 2010-04-05.20:45:34.750>
    labels = ['type-bug', 'library']
    title = 'test_ssl failures with OpenSSL 1.0.0'
    updated_at = <Date 2010-04-17.17:41:14.595>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2010-04-17.17:41:14.595>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-04-17.17:41:14.597>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2010-04-05.20:45:34.750>
    creator = 'pitrou'
    dependencies = []
    files = ['16953', '16954']
    hgrepos = []
    issue_num = 8322
    keywords = ['patch']
    message_count = 10.0
    messages = ['102409', '102440', '102441', '102446', '102757', '102782', '102783', '103357', '103363', '103413']
    nosy_count = 8.0
    nosy_names = ['janssen', 'pitrou', 'vstinner', 'giampaolo.rodola', 'benjamin.peterson', 'rpetrov', 'Arfrever', 'flox']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue8322'
    versions = ['Python 2.7', 'Python 3.2']

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 5, 2010

    When I compile and link against a local build of OpenSSL 1.0.0 (vanilla), I get the following errors in test_ssl:

    ======================================================================
    ERROR: testProtocolSSL2 (test.test_ssl.ThreadedTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 869, in testProtocolSSL2
        tryProtocolCombo(ssl.PROTOCOL_SSLv2, ssl.PROTOCOL_SSLv23, True)
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 736, in tryProtocolCombo
        CERTFILE, CERTFILE, client_protocol, chatty=False)
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 688, in serverParamsTest
        raise test_support.TestFailed("Unexpected exception:  " + str(x))
    TestFailed: Unexpected exception:  [Errno 104] Connection reset by peer

    ======================================================================
    ERROR: testProtocolSSL3 (test.test_ssl.ThreadedTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 903, in testProtocolSSL3
        tryProtocolCombo(ssl.PROTOCOL_SSLv3, ssl.PROTOCOL_SSLv23, False)
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 745, in tryProtocolCombo
        ssl.get_protocol_name(server_protocol)))
    TestFailed: Client protocol SSLv23 succeeded with server protocol SSLv3!

    ======================================================================
    ERROR: testProtocolTLS1 (test.test_ssl.ThreadedTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 914, in testProtocolTLS1
        tryProtocolCombo(ssl.PROTOCOL_TLSv1, ssl.PROTOCOL_SSLv23, False)
      File "/home/antoine/cpython/newssl/Lib/test/test_ssl.py", line 745, in tryProtocolCombo
        ssl.get_protocol_name(server_protocol)))
    TestFailed: Client protocol SSLv23 succeeded with server protocol TLSv1!

    @pitrou pitrou added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Apr 5, 2010
    @vstinner
    Copy link
    Member

    vstinner commented Apr 6, 2010

    Extract of SSL_CTX_new manual page:

    SSLv23_method(void), SSLv23_server_method(void), SSLv23_client_method(void)

    A TLS/SSL connection established with these methods will understand
    the SSLv2, SSLv3, and TLSv1 protocol. A client will send out SSLv2
    client hello messages and will indicate that it also understands
    SSLv3 and TLSv1. A server will understand SSLv2, SSLv3, and TLSv1
    client hello messages. This is the best choice when compatibility
    is a concern.

    I don't think that "Client protocol SSLv23 succeeded with server protocol SSLv3" or "Client protocol SSLv23 succeeded with server protocol TLSv1" are errors.

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 6, 2010

    It begs the question of why the tests succeed with previous OpenSSL versions.
    The only possibly relevant entry I could find in the OpenSSL changelog (but I'm not an expert) is the following:

    *) If no SSLv2 ciphers are used don't use an SSLv2 compatible client hello:
    this allows the use of compression and extensions. Change default cipher
    string to remove SSLv2 ciphersuites. This effectively avoids ancient SSLv2
    by default unless an application cipher string requests it.
    [Steve Henson]

    @vstinner
    Copy link
    Member

    vstinner commented Apr 6, 2010

    All errors concern SSLv23 at server side, and another protocol at client side (SSLv23 for client and server works as expected).

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 9, 2010

    After some investigation, the error does occur because of the aforementioned changelog entry (SSLv2 weak ciphers are now disabled by default). To check it I just added the following line to newPySSLObject():

     SSL_CTX_set_cipher_list(self->ctx, "ALL");
    

    Of course this isn't desirable: we shouldn't blindly enable weak ciphers. Instead we could simply add an argument to configure allowed ciphers, and use "ALL" in our tests. Or we could add a separate method to configure ciphers.

    (this begs the question of whether this is suitable post-beta1)

    What do you think?

    @rpetrov
    Copy link
    Mannequin

    rpetrov mannequin commented Apr 10, 2010

    Antoine Pitrou wrote:

    Antoine Pitrou<pitrou@free.fr> added the comment:

    After some investigation, the error does occur because of the aforementioned changelog entry (SSLv2 weak ciphers are now disabled by default). To check it I just added the following line to newPySSLObject():

      SSL_CTX_set_cipher_list(self-\>ctx, "ALL");
    

    Of course this isn't desirable: we shouldn't blindly enable weak ciphers. Instead we could simply add an argument to configure allowed ciphers, and use "ALL" in our tests. Or we could add a separate method to configure ciphers.

    (this begs the question of whether this is suitable post-beta1)

    What do you think?

    List of ciphers is application dependent and configure script can't
    limit users(applications) .

    Roumen

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 10, 2010

    List of ciphers is application dependent and configure script can't
    limit users(applications) .

    I was obviously talking about a runtime argument to ssl.wrap_socket().

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 16, 2010

    A set_ciphers() method turns out to be highly impractical, since the underlying SSL objects are created lazily when we actually connect the socket.
    Therefore, I have instead opted for a new "ciphers" argument to the SSL constructor(s). Patch attached.

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 16, 2010

    New patch updating docs.

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 17, 2010

    Committed in r80151 (trunk), r80154 (py3k).

    @pitrou pitrou closed this as completed Apr 17, 2010
    @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-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants