Skip to content
Commit 09485fc9 authored by Michael Catanzaro's avatar Michael Catanzaro
Browse files

Replace successful_posthandshake_op with successful_read_op

Turns out the current code to guess whether to return
G_TLS_ERROR_CERTIFICATE_REQUIRED is racy. With TLS 1.2, the client would
see a handshake failure if a client certificate was required but not
provided (or not accepted). That's no longer the case with TLS 1.3;
instead, the client will see a successful handshake before it sees the
server close the connection. GnuTLS is no longer able to indicate *why*
the connection was closed unless the server sends
GNUTLS_A_CERTIFICATE_REQUIRED, but we don't support alerts yet, #65. In
the meantime, we're stuck using a heuristic to decide whether to return
G_TLS_ERROR_CERTIFICATE_REQUIRED: if the server requested a certificate,
and we did not provide one, and an operation fails, and we have never
successfully done any operation after the handshake, then assume the
server rejected our certificate and return
G_TLS_ERROR_CERTIFICATE_REQUIRED. This behavior was added in 14be4745.

However, there in a window of time during which the client may see write
operations succeed even though it failed to provide an acceptable
certificate and the connection is about to be closed by the server. If a
write succeeds, then our heuristic to decide whether to return
G_TLS_ERROR_CERTIFICATE_REQUIRED fails. So let's change the heuristic to
check only for reads instead. A read can never succeed because a
properly-implemented server would not write any data before it verifies
that the client certificate is acceptable.

This commit fixes 14be4745. It's sort of covered by existing tests, in
that we have tests that check for G_TLS_ERROR_CERTIFICATE_REQUIRED,
although they don't seem to be tripping this race. We have a libsoup
test /ssl/tls-interaction that is really hitting this race, although
this change won't be enough to fix that test, because that test needs to
be changed to no longer expect the TLS 1.2 behavior.
parent 33176710
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment