Skip to content

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

Closed

Description

@colesbury

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

Activity

  1. added 2 commits that reference this issue on Apr 2, 2024
  2. added a commit that references this issue on Apr 3, 2024
  3. erlend-aasland commented on Apr 3, 2024

    @erlend-aasland
    Contributor

    Interesting. Could it be related to #26893?

  4. encukou commented on Apr 3, 2024

    @encukou
    Member

    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.

  5. keepworking commented on Apr 5, 2024

    @keepworking
    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.

  6. added a commit that references this issue on Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions