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.send(b"") fails #75892

Closed
joernheissler mannequin opened this issue Oct 6, 2017 · 8 comments
Closed

ssl.SSLSocket.send(b"") fails #75892

joernheissler mannequin opened this issue Oct 6, 2017 · 8 comments
Labels
3.10 only security fixes docs Documentation in the Doc dir topic-SSL type-bug An unexpected behavior, bug, or error

Comments

@joernheissler
Copy link
Mannequin

joernheissler mannequin commented Oct 6, 2017

BPO 31711
Nosy @tiran, @alex, @njsmith, @slingamn, @dstufft, @joernheissler
PRs
  • bpo-31711: Fix for calling SSLSocket.send with empty input. #7559
  • bpo-31711: On SSLObject.write method, added assert that data has content.  #17671
  • Superseder
  • bpo-42854: OpenSSL 1.1.1: use SSL_write_ex() and SSL_read_ex()
  • Files
  • client.py
  • 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 2021-04-19.04:01:15.793>
    created_at = <Date 2017-10-06.09:43:00.138>
    labels = ['expert-SSL', 'type-bug', '3.10', 'docs']
    title = 'ssl.SSLSocket.send(b"") fails'
    updated_at = <Date 2021-04-19.04:01:15.791>
    user = 'https://github.com/joernheissler'

    bugs.python.org fields:

    activity = <Date 2021-04-19.04:01:15.791>
    actor = 'christian.heimes'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2021-04-19.04:01:15.793>
    closer = 'christian.heimes'
    components = ['Documentation', 'SSL']
    creation = <Date 2017-10-06.09:43:00.138>
    creator = 'joernheissler'
    dependencies = []
    files = ['47194']
    hgrepos = []
    issue_num = 31711
    keywords = ['patch']
    message_count = 8.0
    messages = ['303810', '312898', '312903', '312908', '312956', '346525', '358830', '391357']
    nosy_count = 8.0
    nosy_names = ['janssen', 'christian.heimes', 'alex', 'njs', 'docs@python', 'slingamn', 'dstufft', 'joernheissler']
    pr_nums = ['7559', '17671']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = '42854'
    type = 'behavior'
    url = 'https://bugs.python.org/issue31711'
    versions = ['Python 3.10']

    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Oct 6, 2017

    Traceback (most recent call last):
      File "client.py", line 10, in <module>
        conn.send(b'')
      File "/usr/lib/python3.6/ssl.py", line 941, in send
        return self._sslobj.write(data)
      File "/usr/lib/python3.6/ssl.py", line 642, in write
        return self._sslobj.write(data)
    ssl.SSLEOFError: EOF occurred in violation of protocol (_ssl.c:2074)

    This error is not what I expected. I expected a noop instead.

    My guess is, that python calls SSL_write (3.6 branch, _ssl.c:2038) with that empty buffer.
    The manpage states: "When calling SSL_write() with num=0 bytes to be sent the behaviour is undefined."

    This undefined behaviour should either be documented in python, or defined to either raise an exception (ValueError?) or defined as a noop. I'd prefer the latter.

    @joernheissler joernheissler mannequin assigned tiran Oct 6, 2017
    @joernheissler joernheissler mannequin added topic-SSL type-bug An unexpected behavior, bug, or error labels Oct 6, 2017
    @tiran
    Copy link
    Member

    tiran commented Feb 26, 2018

    It's a bit too late to change the behavior of send(). Let's document the issue instead.

    @tiran tiran added docs Documentation in the Doc dir 3.7 (EOL) end of life 3.8 only security fixes labels Feb 26, 2018
    @tiran tiran assigned docspython and unassigned tiran Feb 26, 2018
    @njsmith
    Copy link
    Contributor

    njsmith commented Feb 26, 2018

    If openssl says the behavior is undefined, then don't we have to first make it defined before we can document it?

    And if we're going to detect this case and guarantee some behavior, making it a no-op like it is on regular sockets seems the way to go...

    @tiran
    Copy link
    Member

    tiran commented Feb 26, 2018

    The message "EOF occurred in violation of protocol" is set by Python. Python maps SSL_ERROR_SYSCALL with SSL error code == 0 and len == 0 to that error message.

    https://github.com/python/cpython/blob/master/Modules/_ssl.c#L682-L689
    https://github.com/python/cpython/blob/master/Modules/_ssl.c#L2263

    I don't know why the code was implemented that way.

    @njsmith
    Copy link
    Contributor

    njsmith commented Feb 26, 2018

    My point is that SSL_write(3ssl) says "WARNING: When calling SSL_write() with num=0 bytes to be sent the behaviour is undefined."

    Apparently on the particular openssl you're looking at, it gives SSL_ERROR_SYSCALL with error code == 0 and len == 0, but the openssl devs claim this is some arbitrary thing that shouldn't be depended on.

    Just as a general principle it would be nice if performing ordinary operations on an SSLSocket from Python did not invoke undefined behavior :-)

    @slingamn
    Copy link
    Mannequin

    slingamn mannequin commented Jun 25, 2019

    Are there any possible next steps on this?

    This issue is very counterintuitive and challenging to debug --- it commonly presents as a nondeterministic edge case, and it appears to be a failed system call but doesn't show up in strace.

    Thanks for your time.

    @joernheissler
    Copy link
    Mannequin Author

    joernheissler mannequin commented Dec 23, 2019

    Manpage (openssl 1.1.1d) now states:

    You should not call SSL_write() with num=0, it will return an error. SSL_write_ex() can be called with num=0, but will not send application data to the peer. SSL_write_ex was added in 1.1.1

    So it looks like openssl cleaned up another mistake by defining SSL_write_ex's behaviour to be a noop.

    @tiran
    Copy link
    Member

    tiran commented Apr 19, 2021

    Thanks to PEP-644 the issue will be fixed in 3.10 by using SSL_read_ex and SSL_write_ex() functions. I couldn't use the functions earlier because Python had to support older OpenSSL versions and LibreSSL.

    See #25468 and bpo-42854

    @tiran tiran added 3.10 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Apr 19, 2021
    @tiran tiran closed this as completed Apr 19, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    WillChilds-Klein added a commit to WillChilds-Klein/aws-lc that referenced this issue Nov 20, 2023
    The primary motivation of this change is to match OpenSSL's behavior for
    these functions. Relevant links:
    
    - [SSL_read_ex docs][1]
    - [SSL_write_ex docs][2]
    - [CPython update accounting for this behavior][3]
    
    [1]: https://www.openssl.org/docs/man1.1.1/man3/SSL_read_ex.html
    [2]: https://www.openssl.org/docs/man1.1.1/man3/SSL_write_ex.html
    [3]: python/cpython#75892 (comment)
    WillChilds-Klein added a commit to WillChilds-Klein/aws-lc that referenced this issue Nov 22, 2023
    The primary motivation of this change is to match OpenSSL's behavior for
    these functions. Relevant links:
    
    - [SSL_read_ex docs][1]
    - [SSL_write_ex docs][2]
    - [CPython update accounting for this behavior][3]
    
    [1]: https://www.openssl.org/docs/man1.1.1/man3/SSL_read_ex.html
    [2]: https://www.openssl.org/docs/man1.1.1/man3/SSL_write_ex.html
    [3]: python/cpython#75892 (comment)
    WillChilds-Klein added a commit to aws/aws-lc that referenced this issue Nov 27, 2023
    The primary motivation of this change is to match OpenSSL's behavior for
    these functions. Relevant links:
    
    - [SSL_read_ex docs][1]
    - [SSL_write_ex docs][2]
    - [CPython update accounting for this behavior][3]
    
    [1]: https://www.openssl.org/docs/man1.1.1/man3/SSL_read_ex.html
    [2]: https://www.openssl.org/docs/man1.1.1/man3/SSL_write_ex.html
    [3]: python/cpython#75892 (comment)
    
    ---------
    
    Co-authored-by: Samuel Chiang <samuelchungchiang@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes docs Documentation in the Doc dir topic-SSL type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants