From 0ceea40eacb63297c390032824f2b487ba36e058 Mon Sep 17 00:00:00 2001 From: yevgeny Date: Sun, 18 Feb 2024 18:39:30 +0900 Subject: [PATCH 01/10] gh-115627: Fix PySSL_SetError handling SSL_ERROR_SYSCALL --- Lib/test/test_ssl.py | 25 ++++++++++++++----------- Modules/_ssl.c | 8 ++------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 3fa806ddc2cde7..17ba7672d3b2eb 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2429,16 +2429,19 @@ def run(self): self.write(msg.lower()) except OSError as e: # handles SSLError and socket errors + if isinstance(e, ConnectionError): + # OpenSSL 1.1.1 sometimes raises + # ConnectionResetError when connection is not + # shut down gracefully. + print( + f" Connection reset by peer: {self.addr}" + ) + + self.close() + self.running = False + return if self.server.chatty and support.verbose: - if isinstance(e, ConnectionError): - # OpenSSL 1.1.1 sometimes raises - # ConnectionResetError when connection is not - # shut down gracefully. - print( - f" Connection reset by peer: {self.addr}" - ) - else: - handle_error("Test server failure:\n") + handle_error("Test server failure:\n") try: self.write(b"ERROR\n") except OSError: @@ -4532,8 +4535,8 @@ def msg_cb(conn, direction, version, content_type, msg_type, data): # test sometimes fails with EOF error. Test passes as long as # server aborts connection with an error. with self.assertRaisesRegex( - ssl.SSLError, - '(certificate required|EOF occurred)' + (ssl.SSLError, OSError), + '(certificate required|EOF occurred|closed by the remote host)' ): # receive CertificateRequest data = s.recv(1024) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index d00f407b569fb6..0b3c58b2a57ad2 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -646,11 +646,11 @@ PySSL_SetError(PySSLSocket *sslsock, int ret, const char *filename, int lineno) { if (e == 0) { PySocketSockObject *s = GET_SOCKET(sslsock); - if (ret == 0 || (((PyObject *)s) == Py_None)) { + if (((PyObject *)s) == Py_None) { p = PY_SSL_ERROR_EOF; type = state->PySSLEOFErrorObject; errstr = "EOF occurred in violation of protocol"; - } else if (s && ret == -1) { + } else { /* underlying BIO reported an I/O error */ ERR_clear_error(); #ifdef MS_WINDOWS @@ -667,10 +667,6 @@ PySSL_SetError(PySSLSocket *sslsock, int ret, const char *filename, int lineno) type = state->PySSLEOFErrorObject; errstr = "EOF occurred in violation of protocol"; } - } else { /* possible? */ - p = PY_SSL_ERROR_SYSCALL; - type = state->PySSLSyscallErrorObject; - errstr = "Some I/O error occurred"; } } else { if (ERR_GET_LIB(e) == ERR_LIB_SSL && From 84df935b0f2e2d4c4490490530176b249fd225d4 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 18 Feb 2024 09:50:32 +0000 Subject: [PATCH 02/10] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst diff --git a/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst b/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst new file mode 100644 index 00000000000000..86d98db0469de2 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst @@ -0,0 +1,2 @@ +Fix :c:func:`PySSL_SetError` : Modify retval handling logic for handling +SSL_ERROR_SYSCALL. From a254edc9a187fe75d3e40214126c2192553d9b60 Mon Sep 17 00:00:00 2001 From: yevgeny Date: Tue, 20 Feb 2024 00:56:49 +0900 Subject: [PATCH 03/10] Fix PySSL_SetError function argument (remove int ret) --- Modules/_ssl.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 0b3c58b2a57ad2..4fb56d72f8f841 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -599,7 +599,7 @@ PySSL_ChainExceptions(PySSLSocket *sslsock) { } static PyObject * -PySSL_SetError(PySSLSocket *sslsock, int ret, const char *filename, int lineno) +PySSL_SetError(PySSLSocket *sslsock, const char *filename, int lineno) { PyObject *type; char *errstr = NULL; @@ -612,7 +612,6 @@ PySSL_SetError(PySSLSocket *sslsock, int ret, const char *filename, int lineno) _sslmodulestate *state = get_state_sock(sslsock); type = state->PySSLErrorObject; - assert(ret <= 0); e = ERR_peek_last_error(); if (sslsock->ssl != NULL) { @@ -1026,7 +1025,7 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self) err.ssl == SSL_ERROR_WANT_WRITE); Py_XDECREF(sock); if (ret < 1) - return PySSL_SetError(self, ret, __FILE__, __LINE__); + return PySSL_SetError(self, __FILE__, __LINE__); if (PySSL_ChainExceptions(self) < 0) return NULL; Py_RETURN_NONE; @@ -2433,7 +2432,7 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b) Py_XDECREF(sock); if (retval == 0) - return PySSL_SetError(self, retval, __FILE__, __LINE__); + return PySSL_SetError(self, __FILE__, __LINE__); if (PySSL_ChainExceptions(self) < 0) return NULL; return PyLong_FromSize_t(count); @@ -2463,7 +2462,7 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self) self->err = err; if (count < 0) - return PySSL_SetError(self, count, __FILE__, __LINE__); + return PySSL_SetError(self, __FILE__, __LINE__); else return PyLong_FromLong(count); } @@ -2586,7 +2585,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, err.ssl == SSL_ERROR_WANT_WRITE); if (retval == 0) { - PySSL_SetError(self, retval, __FILE__, __LINE__); + PySSL_SetError(self, __FILE__, __LINE__); goto error; } if (self->exc != NULL) @@ -2712,7 +2711,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) } if (ret < 0) { Py_XDECREF(sock); - PySSL_SetError(self, ret, __FILE__, __LINE__); + PySSL_SetError(self, __FILE__, __LINE__); return NULL; } if (self->exc != NULL) From a33ec18846884e7f007a51896e6aa9da17d163ad Mon Sep 17 00:00:00 2001 From: yevgeny Date: Tue, 20 Feb 2024 00:57:46 +0900 Subject: [PATCH 04/10] Fix PySSL_SetError function for handling socket variable (able to handle s == NULL) --- Modules/_ssl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 4fb56d72f8f841..abecb506594a41 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -649,7 +649,7 @@ PySSL_SetError(PySSLSocket *sslsock, const char *filename, int lineno) p = PY_SSL_ERROR_EOF; type = state->PySSLEOFErrorObject; errstr = "EOF occurred in violation of protocol"; - } else { + } else if (s) { /* underlying BIO reported an I/O error */ ERR_clear_error(); #ifdef MS_WINDOWS @@ -666,6 +666,10 @@ PySSL_SetError(PySSLSocket *sslsock, const char *filename, int lineno) type = state->PySSLEOFErrorObject; errstr = "EOF occurred in violation of protocol"; } + } else { /* possible? */ + p = PY_SSL_ERROR_SYSCALL; + type = state->PySSLSyscallErrorObject; + errstr = "Some I/O error occurred"; } } else { if (ERR_GET_LIB(e) == ERR_LIB_SSL && From 3bd789051c87f7277589cdd9cb96642369140dce Mon Sep 17 00:00:00 2001 From: yevgeny Date: Tue, 20 Feb 2024 00:59:20 +0900 Subject: [PATCH 05/10] Fix NEWS.d (make it user visible) --- .../Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst b/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst index 86d98db0469de2..263cf5d0c20690 100644 --- a/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst +++ b/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst @@ -1,2 +1,2 @@ -Fix :c:func:`PySSL_SetError` : Modify retval handling logic for handling -SSL_ERROR_SYSCALL. +FIX the ssl module error handling of connection terminate by peer. +it throws OSError instead of EOFError. From 5d5dfa898405d817cdc465e68127ef30a1b8704e Mon Sep 17 00:00:00 2001 From: yevgeny Date: Tue, 20 Feb 2024 02:48:53 +0900 Subject: [PATCH 06/10] Fix test_ssl.py print server log only verbose mode --- Lib/test/test_ssl.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 17ba7672d3b2eb..220b85e1fb03c3 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2433,9 +2433,8 @@ def run(self): # OpenSSL 1.1.1 sometimes raises # ConnectionResetError when connection is not # shut down gracefully. - print( - f" Connection reset by peer: {self.addr}" - ) + if self.server.chatty and support.verbose: + print(f" Connection reset by peer: {self.addr}") self.close() self.running = False From cea4b13768fc20712a9578b77c2af18e13d29889 Mon Sep 17 00:00:00 2001 From: yevgeny Date: Tue, 20 Feb 2024 02:53:12 +0900 Subject: [PATCH 07/10] Fix PySSL_SetError function do not reference socketobject for handling error --- Modules/_ssl.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index abecb506594a41..f5a79dcb1ac57d 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -644,32 +644,21 @@ PySSL_SetError(PySSLSocket *sslsock, const char *filename, int lineno) case SSL_ERROR_SYSCALL: { if (e == 0) { - PySocketSockObject *s = GET_SOCKET(sslsock); - if (((PyObject *)s) == Py_None) { + /* underlying BIO reported an I/O error */ + ERR_clear_error(); +#ifdef MS_WINDOWS + if (err.ws) { + return PyErr_SetFromWindowsErr(err.ws); + } +#endif + if (err.c) { + errno = err.c; + return PyErr_SetFromErrno(PyExc_OSError); + } + else { p = PY_SSL_ERROR_EOF; type = state->PySSLEOFErrorObject; errstr = "EOF occurred in violation of protocol"; - } else if (s) { - /* underlying BIO reported an I/O error */ - ERR_clear_error(); -#ifdef MS_WINDOWS - if (err.ws) { - return PyErr_SetFromWindowsErr(err.ws); - } -#endif - if (err.c) { - errno = err.c; - return PyErr_SetFromErrno(PyExc_OSError); - } - else { - p = PY_SSL_ERROR_EOF; - type = state->PySSLEOFErrorObject; - errstr = "EOF occurred in violation of protocol"; - } - } else { /* possible? */ - p = PY_SSL_ERROR_SYSCALL; - type = state->PySSLSyscallErrorObject; - errstr = "Some I/O error occurred"; } } else { if (ERR_GET_LIB(e) == ERR_LIB_SSL && From d97a8d10df4f87c32b4351b2f9556fee4e07fbdb Mon Sep 17 00:00:00 2001 From: yevgeny hong Date: Tue, 20 Feb 2024 02:57:24 +0900 Subject: [PATCH 08/10] Update Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst Co-authored-by: Serhiy Storchaka --- .../Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst b/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst index 263cf5d0c20690..75d926ab59d557 100644 --- a/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst +++ b/Misc/NEWS.d/next/Library/2024-02-18-09-50-31.gh-issue-115627.HGchj0.rst @@ -1,2 +1,2 @@ -FIX the ssl module error handling of connection terminate by peer. -it throws OSError instead of EOFError. +Fix the :mod:`ssl` module error handling of connection terminate by peer. +It now throws an OSError with the appropriate error code instead of an EOFError. From 567fbcb846e3098c0565e86deac84920585c4209 Mon Sep 17 00:00:00 2001 From: yevgeny hong Date: Fri, 22 Mar 2024 22:45:21 +0900 Subject: [PATCH 09/10] Apply suggestions from code review - merge ssl.SSLError and OSError as OSError - append assertRaisesRegex for test_wrong_cert_tls13 ('Connection reset by peer') Co-authored-by: Petr Viktorin --- Lib/test/test_ssl.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 220b85e1fb03c3..be59d3186eafd6 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3168,8 +3168,8 @@ def test_wrong_cert_tls13(self): suppress_ragged_eofs=False) as s: s.connect((HOST, server.port)) with self.assertRaisesRegex( - ssl.SSLError, - 'alert unknown ca|EOF occurred|TLSV1_ALERT_UNKNOWN_CA' + OSError, + 'alert unknown ca|EOF occurred|TLSV1_ALERT_UNKNOWN_CA|closed by the remote host|Connection reset by peer' ): # TLS 1.3 perform client cert exchange after handshake s.write(b'data') @@ -4534,7 +4534,7 @@ def msg_cb(conn, direction, version, content_type, msg_type, data): # test sometimes fails with EOF error. Test passes as long as # server aborts connection with an error. with self.assertRaisesRegex( - (ssl.SSLError, OSError), + OSError, '(certificate required|EOF occurred|closed by the remote host)' ): # receive CertificateRequest From 1a304667cfbd105a71c3d608780a558e9bc75d01 Mon Sep 17 00:00:00 2001 From: yevgeny Date: Fri, 22 Mar 2024 22:57:55 +0900 Subject: [PATCH 10/10] Modify assertRaisesRegex for ThreadedEchoServer (append Connection reset by peer) --- Lib/test/test_ssl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index be59d3186eafd6..bd831ac22419af 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4535,7 +4535,7 @@ def msg_cb(conn, direction, version, content_type, msg_type, data): # server aborts connection with an error. with self.assertRaisesRegex( OSError, - '(certificate required|EOF occurred|closed by the remote host)' + 'certificate required|EOF occurred|closed by the remote host|Connection reset by peer' ): # receive CertificateRequest data = s.recv(1024)