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

ssl.SSLSocket shutdown doesn't behave like socket.shutdown #63080

Open
zielmicha mannequin opened this issue Aug 29, 2013 · 6 comments
Open

ssl.SSLSocket shutdown doesn't behave like socket.shutdown #63080

zielmicha mannequin opened this issue Aug 29, 2013 · 6 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir topic-SSL type-bug An unexpected behavior, bug, or error

Comments

@zielmicha
Copy link
Mannequin

zielmicha mannequin commented Aug 29, 2013

BPO 18880
Nosy @pitrou, @tiran, @alex, @dstufft, @matrixise
Files
  • ssl-shutdown-fail.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 = None
    created_at = <Date 2013-08-29.21:36:58.431>
    labels = ['3.7', 'expert-SSL', 'type-bug', 'library']
    title = "ssl.SSLSocket shutdown doesn't behave like socket.shutdown"
    updated_at = <Date 2017-09-05.22:55:28.333>
    user = 'https://bugs.python.org/zielmicha'

    bugs.python.org fields:

    activity = <Date 2017-09-05.22:55:28.333>
    actor = 'christian.heimes'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)', 'SSL']
    creation = <Date 2013-08-29.21:36:58.431>
    creator = 'zielmicha'
    dependencies = []
    files = ['31514']
    hgrepos = []
    issue_num = 18880
    keywords = ['patch']
    message_count = 6.0
    messages = ['196494', '272351', '277423', '301389', '301390', '301392']
    nosy_count = 7.0
    nosy_names = ['janssen', 'pitrou', 'christian.heimes', 'alex', 'dstufft', 'matrixise', 'zielmicha']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18880'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

    @zielmicha
    Copy link
    Mannequin Author

    zielmicha mannequin commented Aug 29, 2013

    SSLSocket documentation mentions shutdown as analogue to socket.shutdown. However, instead of forbidding communication, it removes SSL wrapper from socket. For example, the following script doesn't work and returns garbage:

        import socket
        import ssl
    
        s = socket.socket()
        s.connect(('google.com', 443))
        client = ssl.wrap_socket(s)
        client.sendall(b'GET / HTTP/1.0\nConnection: close\n\n')
        client.shutdown(socket.SHUT_WR)
    
        print(repr(client.recv(40)))

    Attached patch makes shutdown raise exception if how != SHUT_RDWR, as closing one side of socket over SSL doesn't make sense (unless I'm missing something).

    @zielmicha zielmicha mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Aug 29, 2013
    @matrixise
    Copy link
    Member

    Christian,

    What do you think about this issue ?

    1. Fix for 3.5 and 3.6
    2. Maybe for 2.7 ?

    @vstinner vstinner added type-security A security issue and removed type-bug An unexpected behavior, bug, or error labels Aug 17, 2016
    @tiran
    Copy link
    Member

    tiran commented Sep 26, 2016

    Sounds fine, but it's not a security issue. I'm re-targeting the bug for 3.7.

    @tiran tiran added the 3.7 (EOL) end of life label Sep 26, 2016
    @tiran tiran removed their assignment Sep 26, 2016
    @tiran tiran added type-bug An unexpected behavior, bug, or error and removed type-security A security issue labels Sep 26, 2016
    @tiran
    Copy link
    Member

    tiran commented Sep 5, 2017

    Sounds like a good idea.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 5, 2017

    This will needlessly break code which until now accepts both kinds of sockets.

    By the way, socket.shutdown() doesn't specify that *only* one direction is shut down when using SHUT_RD or SHUT_WR; what is guaranteed is that *at least* the given direction will shut down. But there may be socket types where unidirectional shutdown is not supported and both directions will be shut down. This is (approximately) what SSLSocket does -- though the SSL unwrapping part is a bit unintuitive as well.

    @tiran
    Copy link
    Member

    tiran commented Sep 5, 2017

    I agree with Antoine. I tried to test your patch and found out that is not compatible with socketserver. The socketserver module shuts down the connection with SHUT_WR.

    We could either ignore the problem or ignore the how and use SHUT_RDWR in all cases.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir topic-SSL type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants