diff options
author | Jack Lloyd <[email protected]> | 2016-10-27 10:31:52 -0400 |
---|---|---|
committer | Jack Lloyd <[email protected]> | 2016-10-27 10:31:52 -0400 |
commit | 8c96d4d64f469bf710b7c6f599d0c53291e23631 (patch) | |
tree | 34a3df26df5b43785d1bf9afad7902077c6bd4e7 /src/lib/tls | |
parent | 1b9d13aed71152d61fab7e0ba016d1951909bac5 (diff) |
Fix TLS resumption bugs
The client would attempt to resume a session, even if the session was
for a version other than what it wanted to offer. If the server
resumed with the original version, the client would then reject the
'incorrect' version. Instead, if the session is for a version other
than what we want to offer, just start a fresh handshake.
Fix resuming in the EtM case - even if the policy says otherwise,
always resume EtM sessions as EtM. Servers are required to reject a
MtE resumption on an EtM session.
The new client hello already ordered extensions to prevent an empty
extension from ever being last (working around a bug in some dumb
stack somewhere), but this was not true for the resume case. Fix that.
Beef up tests a bit - test ECDSA suites, alerts, and sqlite3 session db.
Sharing the session state across all the tests is what tipped me off
on the resumption bugs in the first place - as usual, what is not tested
does not work correctly.
Diffstat (limited to 'src/lib/tls')
-rw-r--r-- | src/lib/tls/msg_client_hello.cpp | 18 | ||||
-rw-r--r-- | src/lib/tls/tls_client.cpp | 29 |
2 files changed, 27 insertions, 20 deletions
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp index 36335e7ce..50c83c10c 100644 --- a/src/lib/tls/msg_client_hello.cpp +++ b/src/lib/tls/msg_client_hello.cpp @@ -84,7 +84,7 @@ Client_Hello::Client_Hello(Handshake_IO& io, "Our policy accepts the version we are offering"); /* - * Place all empty extensions in front to avoid a bug in some sytems + * Place all empty extensions in front to avoid a bug in some systems * which reject hellos when the last extension in the list is empty. */ m_extensions.add(new Extended_Master_Secret); @@ -170,14 +170,7 @@ Client_Hello::Client_Hello(Handshake_IO& io, m_extensions.add(new Supported_Point_Formats()); } - if(m_version.supports_negotiable_signature_algorithms()) - m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(), - policy.allowed_signature_methods())); - - if(reneg_info.empty() && !next_protocols.empty()) - m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols)); - - if(policy.negotiate_encrypt_then_mac()) + if(session.supports_encrypt_then_mac()) m_extensions.add(new Encrypt_then_MAC); #if defined(BOTAN_HAS_SRP6) @@ -189,6 +182,13 @@ Client_Hello::Client_Hello(Handshake_IO& io, } #endif + if(m_version.supports_negotiable_signature_algorithms()) + m_extensions.add(new Signature_Algorithms(policy.allowed_signature_hashes(), + policy.allowed_signature_methods())); + + if(reneg_info.empty() && !next_protocols.empty()) + m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols)); + hash.update(io.send(*this)); } diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp index 0e72b9a28..183886c66 100644 --- a/src/lib/tls/tls_client.cpp +++ b/src/lib/tls/tls_client.cpp @@ -149,18 +149,25 @@ void Client::send_client_hello(Handshake_State& state_base, Session session_info; if(session_manager().load_from_server_info(m_info, session_info)) { - if(srp_identifier == "" || session_info.srp_identifier() == srp_identifier) + /* + Ensure that the session protocol type matches what we want to use + If not skip the resume and establish a new session + */ + if(version == session_info.version()) { - state.client_hello(new Client_Hello( - state.handshake_io(), - state.hash(), - policy(), - rng(), - secure_renegotiation_data_for_client_hello(), - session_info, - next_protocols)); - - state.resume_master_secret = session_info.master_secret(); + if(srp_identifier == "" || session_info.srp_identifier() == srp_identifier) + { + state.client_hello( + new Client_Hello(state.handshake_io(), + state.hash(), + policy(), + rng(), + secure_renegotiation_data_for_client_hello(), + session_info, + next_protocols)); + + state.resume_master_secret = session_info.master_secret(); + } } } } |