From de0ffd515316a1994139136929ea26c0e9170d7a Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Wed, 17 Aug 2016 15:18:35 +0200 Subject: [PATCH] properly handle a DTLS handshake failure early DTLS handshake failures would not terminate the read loop and cause the remaining handshake bytes to be feed to packet reader, causing an error assertion. Rework the main read loop to terminate it when the read event is not longer active and stop the read event on handshake failure. Also, make the DTLS handshake erorr message a bit more readable by appending the WolfSSL error message to it. Fixes issue #8. --- src/common/capwap_dtls.c | 6 +++++- src/wtp/wtp_dfa.c | 7 +++++-- src/wtp/wtp_dfa.h | 1 + src/wtp/wtp_dfa_dtls.c | 15 ++++++++++++++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/common/capwap_dtls.c b/src/common/capwap_dtls.c index f008f58..0e7f06c 100644 --- a/src/common/capwap_dtls.c +++ b/src/common/capwap_dtls.c @@ -420,6 +420,8 @@ static int capwap_crypt_handshake(struct capwap_dtls* dtls) { /* */ if (result != SSL_SUCCESS) { + char buffer[WOLFSSL_MAX_ERROR_SZ]; + result = wolfSSL_get_error((WOLFSSL*)dtls->sslsession, 0); if ((result == SSL_ERROR_WANT_READ) || (result == SSL_ERROR_WANT_WRITE)) { /* Incomplete handshake */ @@ -427,6 +429,9 @@ static int capwap_crypt_handshake(struct capwap_dtls* dtls) { return CAPWAP_HANDSHAKE_CONTINUE; } + log_printf(LOG_DEBUG, "Error in DTLS handshake: %s", + wolfSSL_ERR_error_string(result, buffer)); + /* Handshake error */ dtls->action = CAPWAP_DTLS_ACTION_ERROR; return CAPWAP_HANDSHAKE_ERROR; @@ -572,7 +577,6 @@ int capwap_decrypt_packet(struct capwap_dtls* dtls, void* encrybuffer, int size, /* */ if (dtls->action == CAPWAP_DTLS_ACTION_HANDSHAKE) { if (capwap_crypt_handshake(dtls) == CAPWAP_HANDSHAKE_ERROR) { - log_printf(LOG_DEBUG, "Error in DTLS handshake"); result = CAPWAP_ERROR_CLOSE; /* Error handshake */ } else { result = CAPWAP_ERROR_AGAIN; /* Don't parsing DTLS packet */ diff --git a/src/wtp/wtp_dfa.c b/src/wtp/wtp_dfa.c index 236100d..906e5bc 100644 --- a/src/wtp/wtp_dfa.c +++ b/src/wtp/wtp_dfa.c @@ -189,6 +189,9 @@ static void wtp_dfa_process_packet(void *buffer, int buffersize, } else if (oldaction == CAPWAP_DTLS_ACTION_DATA && g_wtp.dtls.action == CAPWAP_DTLS_ACTION_SHUTDOWN) { wtp_teardown_connection(); + } else if (oldaction == CAPWAP_DTLS_ACTION_HANDSHAKE && + g_wtp.dtls.action == CAPWAP_DTLS_ACTION_ERROR) { + wtp_abort_connecting(); } return; /* Next packet */ @@ -335,7 +338,7 @@ static void capwap_control_cb(EV_P_ ev_io *w, int revents) union sockaddr_capwap fromaddr; union sockaddr_capwap toaddr; - while (42) { + do { /* If request wait packet from AC */ do { log_printf(LOG_DEBUG, "Receive CAPWAP Control Channel message"); @@ -366,7 +369,7 @@ static void capwap_control_cb(EV_P_ ev_io *w, int revents) } wtp_dfa_process_packet(&buffer, r, &fromaddr, &toaddr); - } + } while (ev_is_active(w)); } /* Change WTP state machine */ diff --git a/src/wtp/wtp_dfa.h b/src/wtp/wtp_dfa.h index 6d459fc..7cb5e84 100644 --- a/src/wtp/wtp_dfa.h +++ b/src/wtp/wtp_dfa.h @@ -17,6 +17,7 @@ void wtp_free_discovery_response_array(void); /* */ void wtp_teardown_connection(void); +void wtp_abort_connecting(void); /* */ void wtp_socket_io_start(void); diff --git a/src/wtp/wtp_dfa_dtls.c b/src/wtp/wtp_dfa_dtls.c index 3caa3b8..7092f13 100644 --- a/src/wtp/wtp_dfa_dtls.c +++ b/src/wtp/wtp_dfa_dtls.c @@ -34,7 +34,7 @@ void wtp_start_dtlssetup(void) } if (capwap_crypt_open(&g_wtp.dtls) == CAPWAP_HANDSHAKE_ERROR) { - wtp_dfa_change_state(CAPWAP_SULKING_STATE); + wtp_abort_connecting(); } else wtp_dfa_change_state(CAPWAP_DTLS_CONNECT_STATE); } @@ -133,3 +133,16 @@ void wtp_teardown_connection(void) wtp_dfa_change_state(CAPWAP_DTLS_TEARDOWN_STATE); } + +/* abort a possible DTLS connection before the handshake complete */ +void wtp_abort_connecting(void) +{ + /* DTLS Control */ + if (g_wtp.dtls.enable) + capwap_crypt_close(&g_wtp.dtls); + + /* close the control Socket */ + wtp_socket_io_stop(); + capwap_close_sockets(&g_wtp.net); + wtp_dfa_change_state(CAPWAP_SULKING_STATE); +}