Skip to content
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.

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

Different return value from s2n_connection_get_client_protocol_version depending on security_policy configured #4158

Closed
raycoll opened this issue Aug 21, 2023 · 2 comments · Fixed by #4249

Comments

@raycoll
Copy link
Contributor

raycoll commented Aug 21, 2023

Problem:

s2n_connection_get_client_protocol_version returns S2N_TLS12 when a client connects that supports TLS1.3 if the s2n_connection is configured with a TLS1.2 max security policy.

When a TLS1.3 max security policy is configured, s2n_connection_get_client_protocol_version returns S2N_TLS13 as expected.

Solution:

Ideally, s2n_connection_get_client_protocol_version should return the same value regardless of the server configuration used.

  • Does this change what S2N sends over the wire? No
  • Does this change any public APIs? Yes, to make the return value of s2n_connection_get_client_protocol_version consistent.
  • Which versions of TLS will this impact?

Requirements / Acceptance Criteria:

I would expect s2n_connection_get_client_protocol_version to always return S2N_TLS13 when a client connects that supports TLS1.3.

@raycoll
Copy link
Contributor Author

raycoll commented Aug 21, 2023

repros using s2nd/openssl s_client:

  • s2nd reports S2N_TLS12 despite s_client enabling TLS1.3
> DYLD_LIBRARY_PATH=/opt/homebrew/Cellar/openssl@3/3.1.2/lib:/Volumes/workplace/s2n-tls/build/lib/lib ./build/bin/s2nd 0.0.0.0 10000
Listening on 0.0.0.0:10000
Client hello version: 33
Client protocol version: 33
Server protocol version: 33
Actual protocol version: 33
Curve: secp256r1
KEM: NONE
KEM Group: NONE
Cipher negotiated: ECDHE-RSA-AES128-GCM-SHA256
Server signature negotiated: RSA+SHA256


> openssl s_client -min_protocol 'TLSv1' -max_protocol 'TLSv1.3' 127.0.0.1:10000
...
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : ECDHE-RSA-AES128-GCM-SHA256
...
  • s2nd reports S2N_TLS13 with the same client but using a 1.3 security policy:
> DYLD_LIBRARY_PATH=/opt/homebrew/Cellar/openssl@3/3.1.2/lib:/Volumes/workplace/s2n-tls/build/lib/lib ./build/bin/s2nd -c default_tls13 0.0.0.0 10000
Listening on 0.0.0.0:10000
the conn->client_protocol_version 34
the conn->client_protocol_version 34
CONNECTED:
Handshake: NEGOTIATED|FULL_HANDSHAKE|MIDDLEBOX_COMPAT
Client hello version: 33
Client protocol version: 34
Server protocol version: 34
Actual protocol version: 34
Curve: x25519
KEM: NONE
KEM Group: NONE
Cipher negotiated: TLS_AES_128_GCM_SHA256

>  openssl s_client -min_protocol 'TLSv1' -max_protocol 'TLSv1.3' 127.0.0.1:10000
...
SSL-Session:
    Protocol  : TLSv1.3
    Cipher    : TLS_AES_128_GCM_SHA256

@maddeleine
Copy link
Contributor

Note that this specifically an issue when this API is called on the server-side. This behavior is likely due to the fact that we don't process the supported_versions extension if the server doesn't support TLS1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants