Skip to content

test_ssl.test_wrong_cert_tls13 should accept "Broken pipe" as valid error #117483

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.

Sign up for GitHub

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

Jump to bottom
Closed
colesbury opened this issue Apr 2, 2024 · 3 comments
Closed

test_ssl.test_wrong_cert_tls13 should accept "Broken pipe" as valid error #117483

colesbury opened this issue Apr 2, 2024 · 3 comments
Labels
OS-mac tests Tests in the Lib/test dir topic-free-threading topic-SSL

Comments

@colesbury
Copy link
Contributor

colesbury commented Apr 2, 2024 •
edited by bedevere-app bot
Loading

The test_wrong_cert_tls13 unit tests checks the behavior when the server rejects the client's ceritficate. On macOS, this can sometimes lead to a "Broken pipe" on the client instead of a "Connection reset by peer" when the connection is closed during the s.write() call.

This happens frequently in the free-threaded build, but can also be reproduced on the default (with GIL) build by adding a short time.sleep(0.1) immediately before the s.write(b'data').

cpython/Lib/test/test_ssl.py

Lines 3153 to 3178 in 8eda146

@requires_tls_version('TLSv1_3')
def test_wrong_cert_tls13(self):
client_context, server_context, hostname = testing_context()
# load client cert that is not signed by trusted CA
client_context.load_cert_chain(CERTFILE)
server_context.verify_mode = ssl.CERT_REQUIRED
server_context.minimum_version = ssl.TLSVersion.TLSv1_3
client_context.minimum_version = ssl.TLSVersion.TLSv1_3
server = ThreadedEchoServer(
context=server_context, chatty=True, connectionchatty=True,
)
with server, \
client_context.wrap_socket(socket.socket(),
server_hostname=hostname,
suppress_ragged_eofs=False) as s:
s.connect((HOST, server.port))
with self.assertRaisesRegex(
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')
s.read(1000)
s.write(b'should have failed already')
s.read(1000)

Linked PRs
colesbury added a commit to colesbury/cpython that referenced this issue Apr 2, 2024
@colesbury
pythongh-117483: Accept "Broken pipe" as valid error message in `test…
db1b208 
…_wrong_cert_tls13`

On macOS, the closed connection can lead to a "Broken pipe" error instead of
a "Connection reset by peer" error.
@bedevere-app bedevere-app bot mentioned this issue Apr 2, 2024
Merged
colesbury added a commit to colesbury/cpython that referenced this issue Apr 2, 2024
@colesbury
Merge branch 'pythongh-117483-ssl-error' into nogil-integration
62234c9
encukou pushed a commit that referenced this issue Apr 3, 2024
@colesbury
gh-117483: Accept "Broken pipe" as valid error message in `test_wrong…
a214f55 
…_cert_tls13` (GH-117484)

On macOS, the closed connection can lead to a "Broken pipe" error instead of
a "Connection reset by peer" error.
@erlend-aasland
Copy link
Contributor

Interesting. Could it be related to #26893?

@encukou
Copy link
Member

encukou commented Apr 3, 2024

IMO it's related to #115628. And the fact that SSL tests are too strict about the exact wording of the messages.

I checked that adding the time.sleep(0.1) doesn't break tests on 3.12, so it looks like a backport isn't needed.

@keepworking
Copy link
Contributor

@encukou I am agreed with your opinion
Before the error was handled with assertRaisesRegex, it was being processed collectively as except OSError as e:.
You can refer to #26520.
As the types of errors that can occur again since #115628 are increasing, it may be necessary to consider a method of collectively dealing with except OSError as e:.
Or, I think it is necessary to investigate what is possible about OSError that may occur in that situation.
I'll look into it over the weekend.

diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
@colesbury @diegorusso
pythongh-117483: Accept "Broken pipe" as valid error message in `test…
e76d078 
…_wrong_cert_tls13` (pythonGH-117484)

On macOS, the closed connection can lead to a "Broken pipe" error instead of
a "Connection reset by peer" error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
OS-mac tests Tests in the Lib/test dir topic-free-threading topic-SSL
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@encukou @colesbury @erlend-aasland @keepworking