GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||
GeezerVison is showing github.com — gh-148292: Update SSLSocket.read() for OpenSSL 4 by vstinner · Pull Request #148602 · python/cpython Conversation
|
|
@picnixz @gpshead: Would you mind to review this (draft) change? I marked the PR as a draft since I'm not sure if the fix makes sense and is correct. See #148600 (comment) to reproduce the issue and build Python 3.15 with OpenSSL 4.0.0. Note: #148601 (Add Modules/_ssl_data_40.h data) has no effect on |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
See #146217 (comment) for differences between OpenSSL 3 and OpenSSL 4. In short:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I don't think it's the correct change because I need to investigate. The reason why I don't think it's correct is because the code path being taken to trigger "A failure in the SSL library occurred" means that the last OpenSSL error code was not set (either we cleared it accidently or they didn't set it correctly) and this is something that can happen elsewhere. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
FTR, this may be related #148594. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
It's not related. test_urllib2_localnet still fails with this change: diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 4e563379098..d17cd308628 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -2938,6 +2938,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, do { Py_BEGIN_ALLOW_THREADS; + ERR_clear_error(); retval = SSL_read_ex(self->ssl, mem, (size_t)len, &count); err = _PySSL_errno(retval == 0, self->ssl, retval); Py_END_ALLOW_THREADS; |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The 3rd read fails with
Related code in #if defined(SSL_R_UNEXPECTED_EOF_WHILE_READING) /* OpenSSL 3.0 changed transport EOF from SSL_ERROR_SYSCALL with * zero return value to SSL_ERROR_SSL with a special error code. */ if (ERR_GET_LIB(e) == ERR_LIB_SSL && ERR_GET_REASON(e) == SSL_R_UNEXPECTED_EOF_WHILE_READING) { p = PY_SSL_ERROR_EOF; type = state->PySSLEOFErrorObject; errstr = "EOF occurred in violation of protocol"; } #endif The 4th read doesn't seem to set any specific error :-( It only says that it's an "SSL error".
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
How can I debug such issue? According to my previous comment, OpenSSL doesn't set any error. It only says that it's a "SSL error". Even if I check It seems like OpenSSL 4 changed |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This may be a bug in OpenSSL 4. Something changed in their errors:
So.... maybe we may ourselves be doing bad things elsewhere. I honestly do not know if the problem is on our side or their side. It may be on our side because we maybe assume that the stack entry is still there? (idk) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ok, here is a simpler Python reproducer script only using import os.path import socket import ssl import threading CERT = os.path.join('Lib', 'test', 'certdata', 'keycert.pem') HOST = '127.0.0.1' HOSTNAME = 'localhost' class Server(threading.Thread): def __init__(self): super().__init__() self.listening = threading.Event() self.address = None def run(self): context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) context.load_cert_chain(CERT) server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) #server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) #server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1) server_sock.bind((HOST, 0)) server_sock.listen(5) self.address = server_sock.getsockname() self.listening.set() sock, addr = server_sock.accept() sslconn = context.wrap_socket(sock, server_side=True) request = b'' while True: chunk = sslconn.recv(65537) request += chunk if b'\r\n\r\n' in request: break print(f"server got request: {request!r}") print("server sendall") sslconn.sendall( b'HTTP/1.0 200 OK\r\n' b'Server: TestHTTP/ Python/3.15.0a8+\r\n' b'Date: Thu, 16 Apr 2026 12:42:37 GMT\r\n' b'Content-type: text/plain\r\n\r\n') sslconn.sendall(b'we care a bit') print("server shutdown write") sslconn.shutdown(socket.SHUT_WR) print("server close socket") sslconn.close() server_sock.close() def main(): server = Server() server.start() server.listening.wait() port = server.address[1] context = ssl.create_default_context(cafile=CERT) sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) sock.connect((HOST, port)) sslsock = context.wrap_socket(sock, server_hostname=HOSTNAME) sslsock.sendall(b'GET /bizarre HTTP/1.0\r\n\r\n') sslobj = sslsock._sslobj def read(prefix, sslobj): try: data = sslobj.read(1024) result = repr(data) except ssl.SSLError as exc: result = f'<{exc!r}>' print(prefix, result) for i in range(1, 5): read(f"client read #{i}:", sslobj) sslsock.close() server.join() if __name__ == "__main__": main() Output with OpenSSL 3: Output with OpenSSL 4: OpenSSL 4 behaves differently on the last (4th) read (after UNEXPECTED_EOF_WHILE_READING): it fails with a generic |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I went ahead and I created an issue in OpenSSL bug tracker openssl/openssl#30894 to ask if the new behavior was made on purpose or not. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The PR is no longer a draft, it's ready for review.
So in short, yes, the new behavior was made on purpose. After UNEXPECTED_EOF_WHILE_READING, the connection no longer exists, and SSL_read_ex() fails with a generic protocol error. IMO this change is an acceptable solution to get the same behavior on OpenSSL 4 and on OpenSSL 3 (and older). But I'm not sure if the fix is done at the right level. I modified |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I don't have much bandwidth to check this but I think it should be done at the C level instead. AFAIR, the high-level Python part is really just a thin wrapper and there may be users that directly use the C class. Problem is that the SSL code is really large and I can't tell you exactly which places will need to be fixed... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yeah, I agree. So I wrote gh-149102 which moves the logic to the C code (
I searched for |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I close this PR. I merged the PR changing the C code instead (PR gh-149102). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.