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

SSLSocket created from SSLContext.wrap_socket doesn't include cert/keyfile #60561

Closed
mcjeff mannequin opened this issue Oct 29, 2012 · 6 comments
Closed

SSLSocket created from SSLContext.wrap_socket doesn't include cert/keyfile #60561

mcjeff mannequin opened this issue Oct 29, 2012 · 6 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@mcjeff
Copy link
Mannequin

mcjeff mannequin commented Oct 29, 2012

BPO 16357
Nosy @jcea, @pitrou
Files
  • ssl_context.patch
  • ssl_context.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 2012-11-11.00:30:12.111>
    created_at = <Date 2012-10-29.15:53:59.491>
    labels = ['type-bug', 'library']
    title = "SSLSocket created from SSLContext.wrap_socket doesn't include cert/keyfile"
    updated_at = <Date 2012-12-14.01:20:46.924>
    user = 'https://bugs.python.org/mcjeff'

    bugs.python.org fields:

    activity = <Date 2012-12-14.01:20:46.924>
    actor = 'jcea'
    assignee = 'none'
    closed = True
    closed_date = <Date 2012-11-11.00:30:12.111>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2012-10-29.15:53:59.491>
    creator = 'mcjeff'
    dependencies = []
    files = ['27776', '27784']
    hgrepos = []
    issue_num = 16357
    keywords = ['patch']
    message_count = 6.0
    messages = ['174121', '174151', '174155', '174156', '175306', '175307']
    nosy_count = 4.0
    nosy_names = ['jcea', 'pitrou', 'mcjeff', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue16357'
    versions = ['Python 3.2', 'Python 3.3', 'Python 3.4']

    @mcjeff
    Copy link
    Mannequin Author

    mcjeff mannequin commented Oct 29, 2012

    mcjeff@martian:~/cpython$ ./python -V
    Python 3.4.0a0

    When an SSLSocket is created via SSLContext.wrap_socket, it is passed a _context parameter directly. SSLSocket.__init__ sets self.context at this point, but it does not set self.keyfile or self.certfile.

    However, in SSLSocket.accept, both keyfile & certfile are passed when creating a new, wrapped SSLSocket, from socket.accept's newsock.

    The result is an attribute error.
    >>> import ssl
    >>> c = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
    >>> c.load_cert_chain('Lib/test/keycert.pem')        
    >>> import socket
    >>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0)
    >>> s.bind(('127.0.0.1', 5050))
    >>> s.listen(5)
    >>> s.accept()  # nc localhost 5050 in another term.
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/local/google/home/mcjeff/cpython/Lib/ssl.py", line 557, in accept
        keyfile=self.keyfile, certfile=self.certfile,
    AttributeError: 'SSLSocket' object has no attribute 'keyfile'
    >>> 

    Attached one-liner addresses it by passing in the context rather than the keyfile & certfile.

    >>> s.accept()
    (<socket.socket object, fd=4, family=2, type=1, proto=0>, ('127.0.0.1', 37306))
    >>>

    @mcjeff mcjeff mannequin added the stdlib Python modules in the Lib dir label Oct 29, 2012
    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2012

    I don't understand your code snippet: you don't seem to wrap the socket anywhere (paste error?).
    As for the patch, it would be nice to add a corresponding test in Lib/test/test_ssl.py.

    @pitrou pitrou added the type-bug An unexpected behavior, bug, or error label Oct 29, 2012
    @mcjeff
    Copy link
    Mannequin Author

    mcjeff mannequin commented Oct 29, 2012

    Ak! Yes, cut and paste error.

    Python 3.4.0a0 (default:57a33af85407, Oct 27 2012, 21:26:30) 
    [GCC 4.4.3] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import ssl          
    >>> c = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
    >>> c.load_cert_chain('Lib/test/keycert.pem')        
    >>> import socket
    >>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0)
    >>> s.bind(('127.0.0.1', 5050))
    >>> s = c.wrap_socket(s)
    >>> s.listen(5)
    >>> s.accept()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/jeff/cpython/Lib/ssl.py", line 557, in accept
        keyfile=self.keyfile, certfile=self.certfile,
    AttributeError: 'SSLSocket' object has no attribute 'keyfile'
    >>> 

    I'll add a corresponding test, sure thing.

    @mcjeff
    Copy link
    Mannequin Author

    mcjeff mannequin commented Oct 29, 2012

    Updated to pass in the parent context only actually, as it doesn't look like all of the attributes on SSLSocket will be set if a context was initially passed in.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 11, 2012

    New changeset f475332df9b5 by Antoine Pitrou in branch '3.2':
    Issue bpo-16357: fix calling accept() on a SSLSocket created through SSLContext.wrap_socket().
    http://hg.python.org/cpython/rev/f475332df9b5

    New changeset 9510a9641c80 by Antoine Pitrou in branch '3.3':
    Issue bpo-16357: fix calling accept() on a SSLSocket created through SSLContext.wrap_socket().
    http://hg.python.org/cpython/rev/9510a9641c80

    New changeset 5fc30f0277a5 by Antoine Pitrou in branch 'default':
    Issue bpo-16357: fix calling accept() on a SSLSocket created through SSLContext.wrap_socket().
    http://hg.python.org/cpython/rev/5fc30f0277a5

    @pitrou
    Copy link
    Member

    pitrou commented Nov 11, 2012

    I've reworked the patch a bit and committed it. Thank you for reporting this!

    @pitrou pitrou closed this as completed Nov 11, 2012
    @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

    1 participant