forked from osmocom/wireshark
ssl: fix TLS renegotiation, add test for this
A handshake starts a new session, be sure to clear the previous state to avoid creating a decoder with wrong secrets. Renegotiations are also kind of transparant to the application layer, so be sure to re-use an existing SslFlow. This fixes the Follow SSL stream functionality which would previously ignore everything except for the first session. The capture file contains a crafted HTTP request/response over TLS 1.2, interleaved with renegotiations. The HTTP response contains the Python script used to generate the traffic. Surprise! Change-Id: I0110ce76893d4a79330845e53e47e10f1c79e47e Reviewed-on: https://code.wireshark.org/review/17480 Petri-Dish: Peter Wu <peter@lekensteyn.nl> Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org> Reviewed-by: Peter Wu <peter@lekensteyn.nl>
This commit is contained in:
parent
36c8065cc3
commit
7a674c006b
|
@ -2780,7 +2780,6 @@ ssl_create_decoder(const SslCipherSuite *cipher_suite, gint compression,
|
|||
}
|
||||
dec->seq = 0;
|
||||
dec->decomp = ssl_create_decompressor(compression);
|
||||
dec->flow = ssl_create_flow();
|
||||
|
||||
/* TODO this does nothing as dec->evp is always NULL. */
|
||||
if (dec->evp)
|
||||
|
@ -3222,6 +3221,10 @@ create_decoders:
|
|||
goto fail;
|
||||
}
|
||||
|
||||
/* Continue the SSL stream after renegotiation with new keys. */
|
||||
ssl_session->client_new->flow = ssl_session->client ? ssl_session->client->flow : ssl_create_flow();
|
||||
ssl_session->server_new->flow = ssl_session->server ? ssl_session->server->flow : ssl_create_flow();
|
||||
|
||||
ssl_debug_printf("%s: client seq %d, server seq %d\n",
|
||||
G_STRFUNC, ssl_session->client_new->seq, ssl_session->server_new->seq);
|
||||
g_free(key_block.data);
|
||||
|
@ -4109,6 +4112,48 @@ ssl_get_session(conversation_t *conversation, dissector_handle_t ssl_handle)
|
|||
return ssl_session;
|
||||
}
|
||||
|
||||
/* Resets the decryption parameters for the next decoder. */
|
||||
static void ssl_reset_session(SslSession *session, SslDecryptSession *ssl, gboolean is_client)
|
||||
{
|
||||
if (ssl) {
|
||||
/* Ensure that secrets are not restored using stale identifiers. Split
|
||||
* between client and server in case the packets somehow got out of order. */
|
||||
gint clear_flags = SSL_HAVE_SESSION_KEY | SSL_MASTER_SECRET | SSL_PRE_MASTER_SECRET;
|
||||
|
||||
if (is_client) {
|
||||
clear_flags |= SSL_CLIENT_EXTENDED_MASTER_SECRET;
|
||||
ssl->session_id.data_len = 0;
|
||||
ssl->session_ticket.data_len = 0;
|
||||
ssl->master_secret.data_len = 0;
|
||||
ssl->client_random.data_len = 0;
|
||||
} else {
|
||||
clear_flags |= SSL_SERVER_EXTENDED_MASTER_SECRET | SSL_NEW_SESSION_TICKET;
|
||||
ssl->server_random.data_len = 0;
|
||||
ssl->pre_master_secret.data_len = 0;
|
||||
#if defined(HAVE_LIBGNUTLS) && defined(HAVE_LIBGCRYPT)
|
||||
ssl->private_key = NULL;
|
||||
#endif
|
||||
ssl->psk.data_len = 0;
|
||||
}
|
||||
|
||||
if (ssl->state & clear_flags) {
|
||||
ssl_debug_printf("%s detected renegotiation, clearing 0x%02x (%s side)\n",
|
||||
G_STRFUNC, ssl->state & clear_flags, is_client ? "client" : "server");
|
||||
ssl->state &= ~clear_flags;
|
||||
}
|
||||
}
|
||||
|
||||
/* These flags might be used for non-decryption purposes and may affect the
|
||||
* dissection, so reset them as well. */
|
||||
if (is_client) {
|
||||
session->client_cert_type = 0;
|
||||
} else {
|
||||
session->compression = 0;
|
||||
session->server_cert_type = 0;
|
||||
/* session->is_session_resumed is already handled in the ServerHello dissection. */
|
||||
}
|
||||
}
|
||||
|
||||
static guint32
|
||||
ssl_starttls(dissector_handle_t ssl_handle, packet_info *pinfo,
|
||||
dissector_handle_t app_handle, guint32 last_nontls_frame)
|
||||
|
@ -5441,12 +5486,16 @@ ssl_dissect_hnd_hello_ext_cert_type(ssl_common_dissect_t *hf, tvbuff_t *tvb,
|
|||
static gint
|
||||
ssl_dissect_hnd_hello_common(ssl_common_dissect_t *hf, tvbuff_t *tvb,
|
||||
proto_tree *tree, guint32 offset,
|
||||
SslDecryptSession *ssl, gboolean from_server)
|
||||
SslSession *session, SslDecryptSession *ssl,
|
||||
gboolean from_server)
|
||||
{
|
||||
nstime_t gmt_unix_time;
|
||||
guint8 sessid_length;
|
||||
proto_tree *rnd_tree;
|
||||
|
||||
/* Prepare for renegotiation by resetting the state. */
|
||||
ssl_reset_session(session, ssl, !from_server);
|
||||
|
||||
if (ssl) {
|
||||
StringInfo *rnd;
|
||||
if (from_server)
|
||||
|
@ -5778,7 +5827,7 @@ ssl_dissect_hnd_cli_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
|
|||
offset += 2;
|
||||
|
||||
/* dissect fields that are also present in ClientHello */
|
||||
offset = ssl_dissect_hnd_hello_common(hf, tvb, tree, offset, ssl, FALSE);
|
||||
offset = ssl_dissect_hnd_hello_common(hf, tvb, tree, offset, session, ssl, FALSE);
|
||||
|
||||
/* fields specific for DTLS (cookie_len, cookie) */
|
||||
if (dtls_hfs != NULL) {
|
||||
|
@ -5898,7 +5947,7 @@ ssl_dissect_hnd_srv_hello(ssl_common_dissect_t *hf, tvbuff_t *tvb,
|
|||
offset += 2;
|
||||
|
||||
/* dissect fields that are also present in ClientHello */
|
||||
offset = ssl_dissect_hnd_hello_common(hf, tvb, tree, offset, ssl, TRUE);
|
||||
offset = ssl_dissect_hnd_hello_common(hf, tvb, tree, offset, session, ssl, TRUE);
|
||||
|
||||
if (ssl) {
|
||||
/* store selected cipher suite for decryption */
|
||||
|
|
Binary file not shown.
|
@ -245,6 +245,22 @@ decryption_step_ssl_master_secret() {
|
|||
test_step_ok
|
||||
}
|
||||
|
||||
# TLS 1.2 with renegotiation
|
||||
decryption_step_ssl_renegotiation() {
|
||||
TEST_KEYS_FILE="$TESTS_DIR/keys/rsasnakeoil2.key"
|
||||
if [ "$WS_SYSTEM" == "Windows" ] ; then
|
||||
TEST_KEYS_FILE="`cygpath -w $TEST_KEYS_FILE`"
|
||||
fi
|
||||
output=$($TESTS_DIR/run_and_catch_crashes env $TS_DC_ENV $TSHARK $TS_DC_ARGS -Tfields -e http.content_length \
|
||||
-o ssl.keys_list:"0.0.0.0,4433,http,$TEST_KEYS_FILE" \
|
||||
-r "$CAPTURE_DIR/tls-renegotiation.pcap" -Y http)
|
||||
if [[ "$output" != 0*2151* ]]; then
|
||||
test_step_failed "Failed to decrypt SSL with renegotiation"
|
||||
return
|
||||
fi
|
||||
test_step_ok
|
||||
}
|
||||
|
||||
# ZigBee
|
||||
# https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7022
|
||||
decryption_step_zigbee() {
|
||||
|
@ -492,6 +508,7 @@ tshark_decryption_suite() {
|
|||
test_step_add "SSL Decryption (RSA private key with p smaller than q)" decryption_step_ssl_rsa_pq
|
||||
test_step_add "SSL Decryption (private key with password)" decryption_step_ssl_with_password
|
||||
test_step_add "SSL Decryption (master secret)" decryption_step_ssl_master_secret
|
||||
test_step_add "SSL Decryption (renegotiation)" decryption_step_ssl_renegotiation
|
||||
test_step_add "ZigBee Decryption" decryption_step_zigbee
|
||||
test_step_add "ANSI C12.22 Decryption" decryption_step_c1222
|
||||
test_step_add "DVB-CI Decryption" decryption_step_dvb_ci
|
||||
|
|
Loading…
Reference in New Issue