From 2c9e1454fe421de5fa2668ac1dfcc43a9f369909 Mon Sep 17 00:00:00 2001 From: Viktor Krykun Date: Tue, 5 Jun 2012 23:24:37 +0300 Subject: [PATCH] various bug fixes in libzrtp * fixed bug with remote hello-hash buffer being too small * removed unused hello-hash storage in zrtp stream context * fixed bug with libzrtp rendered "empty" SAS hash from incoming SasRelay packet * incremented libzrtp version number to 1.15 Signed-off-by: Travis Cross --- libs/libzrtp/include/zrtp_types.h | 16 ++++------ libs/libzrtp/include/zrtp_version.h | 6 ++-- libs/libzrtp/projects/gnu/configure.in | 2 +- libs/libzrtp/src/zrtp.c | 8 ++--- libs/libzrtp/src/zrtp_pbx.c | 44 ++++++++++++++++++-------- libs/libzrtp/src/zrtp_utils.c | 2 +- 6 files changed, 46 insertions(+), 32 deletions(-) diff --git a/libs/libzrtp/include/zrtp_types.h b/libs/libzrtp/include/zrtp_types.h index a38d118591..c5c7654d68 100644 --- a/libs/libzrtp/include/zrtp_types.h +++ b/libs/libzrtp/include/zrtp_types.h @@ -527,7 +527,12 @@ typedef struct zrtp_stream_mescache_t zrtp_retry_task_t errorack_task; zrtp_retry_task_t sasrelay_task; - zrtp_string16_t signaling_hash; + /*! + * Hash pre-image of the remote party Hello retrieved from Signaling. When + * user calls zrtp_signaling_hash_set() libzrtp stores hash value in this + * variable and checks all incoming Hello-s to prevent DOS attacks. + */ + zrtp_string64_t signaling_hash; } zrtp_stream_mescache_t; @@ -722,14 +727,7 @@ struct zrtp_stream_t * crypto sources and performs traffic encryption/decryption. */ zrtp_protocol_t *protocol; - - /*! - * Hash pre-image of the remote party Hello retrieved from Signaling. When - * user calls zrtp_signaling_hash_set() libzrtp stores hash value in this - * variable and checks all incoming Hellos to prevent DOS attacks. - */ - zrtp_string128_t signaling_hash; - + /*!< Holder for RTP/ZRTP media stream options. */ zrtp_media_context_t media_ctx; diff --git a/libs/libzrtp/include/zrtp_version.h b/libs/libzrtp/include/zrtp_version.h index 5afc3b76af..e666e42cb9 100644 --- a/libs/libzrtp/include/zrtp_version.h +++ b/libs/libzrtp/include/zrtp_version.h @@ -12,8 +12,8 @@ #define LIBZRTP_VERSION_MAJOR 1 -#define LIBZRTP_VERSION_MINOR 13 -#define LIBZRTP_VERSION_BUILD 604 -#define LIBZRTP_VERSION_STR "v1.13 604" +#define LIBZRTP_VERSION_MINOR 15 +#define LIBZRTP_VERSION_BUILD 607 +#define LIBZRTP_VERSION_STR "v1.15 607" #endif /*__ZRTP_VERSION_H__*/ diff --git a/libs/libzrtp/projects/gnu/configure.in b/libs/libzrtp/projects/gnu/configure.in index d25d41771a..7d97319a76 100644 --- a/libs/libzrtp/projects/gnu/configure.in +++ b/libs/libzrtp/projects/gnu/configure.in @@ -32,7 +32,7 @@ case $target_os in esac -AM_INIT_AUTOMAKE([libzrtp], [1.14]) +AM_INIT_AUTOMAKE([libzrtp], [1.15]) AX_PREFIX_CONFIG_H(../../include/zrtp_config_unix.h,ZRTP,config/config.h) CFLAGS="$CFLAGS -Wno-unused-parameter -fno-strict-aliasing -fPIC -DZRTP_AUTOMAKE=1" diff --git a/libs/libzrtp/src/zrtp.c b/libs/libzrtp/src/zrtp.c index d0002a7d3d..c69b5e9692 100644 --- a/libs/libzrtp/src/zrtp.c +++ b/libs/libzrtp/src/zrtp.c @@ -418,8 +418,6 @@ zrtp_status_t zrtp_stream_attach(zrtp_session_t *session, zrtp_stream_t** stream return zrtp_status_alloc_fail; } - ZSTR_SET_EMPTY(new_stream->signaling_hash); - /* * Initialize the private data stream with default initial values */ @@ -437,6 +435,8 @@ zrtp_status_t zrtp_stream_attach(zrtp_session_t *session, zrtp_stream_t** stream ZSTR_SET_EMPTY(new_stream->cc.zrtp_key); ZSTR_SET_EMPTY(new_stream->cc.peer_zrtp_key); + ZSTR_SET_EMPTY(new_stream->messages.signaling_hash); + new_stream->dh_cc.initialized_with = ZRTP_COMP_UNKN; bnBegin(&new_stream->dh_cc.peer_pv); ZSTR_SET_EMPTY(new_stream->dh_cc.dhss); @@ -638,8 +638,8 @@ zrtp_status_t zrtp_signaling_hash_set( zrtp_stream_t* ctx, ctx->messages.signaling_hash.length = ZRTP_MESSAGE_HASH_SIZE; { - char buff[66]; - ZRTP_LOG(3, (_ZTU_,"SIGNALLING HAS was ADDED for the comparing. ID=%u\n", ctx->id)); + char buff[64]; + ZRTP_LOG(3, (_ZTU_,"SIGNALLING HAS was ADDED for the comparision. ID=%u\n", ctx->id)); ZRTP_LOG(3, (_ZTU_,"Hash=%s.\n", hex2str(hash_buff, hash_buff_length, buff, sizeof(buff)))); } diff --git a/libs/libzrtp/src/zrtp_pbx.c b/libs/libzrtp/src/zrtp_pbx.c index 3567d4a47b..cc25c46b3e 100644 --- a/libs/libzrtp/src/zrtp_pbx.c +++ b/libs/libzrtp/src/zrtp_pbx.c @@ -128,13 +128,15 @@ zrtp_status_t _zrtp_machine_process_sasrelay(zrtp_stream_t *stream, zrtp_rtp_inf zrtp_status_t s = zrtp_status_fail; zrtp_string128_t hmac = ZSTR_INIT_EMPTY(hmac); char zerosashash[32]; + unsigned sas_scheme_did_change = 0; + unsigned sas_hash_did_change = 0; /* (padding + sig_len + flags) + SAS scheme and SAS hash */ const uint8_t encrypted_body_size = (2 + 1 + 1) + 4 + 32; zrtp_memset(zerosashash, 0, sizeof(zerosashash)); - /* Check if the remote endpoint is assiggneed to relay the SAS values */ + /* Check if the remote endpoint is assigned to relay the SAS values */ if (!stream->peer_mitm_flag) { ZRTP_LOG(2,(_ZTU_, ZRTP_RELAYED_SAS_FROM_NONMITM_STR)); return zrtp_status_fail; @@ -157,7 +159,7 @@ zrtp_status_t _zrtp_machine_process_sasrelay(zrtp_stream_t *stream, zrtp_rtp_inf return zrtp_status_fail; } - ZRTP_LOG(3,(_ZTU_, "\tHMAC value for the SASRELAY is correct - decryptiong...\n")); + ZRTP_LOG(3,(_ZTU_, "\tHMAC value for the SASRELAY is correct - decrypting...\n")); /* Then we need to decrypt Confirm body */ do @@ -217,9 +219,14 @@ zrtp_status_t _zrtp_machine_process_sasrelay(zrtp_stream_t *stream, zrtp_rtp_inf _zrtp_machine_enter_initiatingerror(stream, zrtp_error_invalid_packet, 1); return zrtp_status_fail; } - session->sasscheme = zrtp_comp_find(ZRTP_CC_SAS, rendering_id, session->zrtp ); - ZRTP_LOG(3,(_ZTU_,"\tSasrelay: New Rendering scheme %.4s.\n", session->sasscheme->base.type)); + /* Check is SAS rendering did change */ + if (rendering_id != session->sasscheme->base.id) { + session->sasscheme = zrtp_comp_find(ZRTP_CC_SAS, rendering_id, session->zrtp ); + + sas_scheme_did_change = 1; + ZRTP_LOG(3,(_ZTU_,"\tSasrelay: Rendering scheme was updated to %.4s.\n", session->sasscheme->base.type)); + } if (session->secrets.matches & ZRTP_BIT_PBX) { if ( ( ((uint32_t) *sasrelay->sas_scheme) != (uint32_t)0x0L ) && @@ -231,7 +238,8 @@ zrtp_status_t _zrtp_machine_process_sasrelay(zrtp_stream_t *stream, zrtp_rtp_inf zrtp_memcpy(session->sasbin.buffer, sasrelay->sashash, session->sasbin.length); stream->mitm_mode = ZRTP_MITM_MODE_RECONFIRM_CLIENT; - ZRTP_LOG(3,(_ZTU_,"\tSasRelay: SAS value was updated bin=%s.\n", + sas_hash_did_change = 1; + ZRTP_LOG(3,(_ZTU_,"\tSasRelay: SAS value was updated to bin=%s.\n", hex2str(buff, sizeof(buff), session->sasbin.buffer, session->sasbin.length))); } } else if (0 != zrtp_memcmp(sasrelay->sashash, zerosashash, sizeof(sasrelay->sashash))) { @@ -242,16 +250,24 @@ zrtp_status_t _zrtp_machine_process_sasrelay(zrtp_stream_t *stream, zrtp_rtp_inf ZRTP_LOG(1,(_ZTU_, "\rERROR! For SasRelay Other secret doesn't match. ID=%u\n", stream->id)); } - s = session->sasscheme->compute(session->sasscheme, stream, session->hash, 1); - if (zrtp_status_ok != s) { - _zrtp_machine_enter_initiatingerror(stream, zrtp_error_software, 1); - return s; - } - ZRTP_LOG(3,(_ZTU_,"\tSasRelay: Updated SAS is <%s> <%s>.\n", session->sas1.buffer, session->sas2.buffer)); + /* Generate new SAS if hash or rendering scheme did change. + * Note: latest libzrtp may send "empty" SasRelay with the same SAS rendering + * scheme and empty Hello hash for consistency reasons, we should ignore + * such packets. + */ + if (sas_scheme_did_change || sas_hash_did_change) { + s = session->sasscheme->compute(session->sasscheme, stream, session->hash, 1); + if (zrtp_status_ok != s) { + _zrtp_machine_enter_initiatingerror(stream, zrtp_error_software, 1); + return s; + } - if (session->zrtp->cb.event_cb.on_zrtp_protocol_event) { - session->zrtp->cb.event_cb.on_zrtp_protocol_event(stream, ZRTP_EVENT_LOCAL_SAS_UPDATED); + ZRTP_LOG(3,(_ZTU_,"\tSasRelay: Updated SAS is <%s> <%s>.\n", session->sas1.buffer, session->sas2.buffer)); + + if (session->zrtp->cb.event_cb.on_zrtp_protocol_event) { + session->zrtp->cb.event_cb.on_zrtp_protocol_event(stream, ZRTP_EVENT_LOCAL_SAS_UPDATED); + } } return zrtp_status_ok; @@ -492,7 +508,7 @@ zrtp_status_t zrtp_update_remote_options( zrtp_stream_t* stream, return zrtp_status_bad_param; } - /* Don't allow to transfer the SAS if the library wasn't initalized as MiTM endpoint */ + /* Don't allow to transfer the SAS if the library wasn't initialized as MiTM endpoint */ if (!stream->zrtp->is_mitm) { ZRTP_LOG(3,(_ZTU_,"\tERROR! The endpoint can't transfer SAS values to other endpoints" " without introducing itself by M-flag in Hello. see zrtp_init().\n")); diff --git a/libs/libzrtp/src/zrtp_utils.c b/libs/libzrtp/src/zrtp_utils.c index 7b61aac146..45b5290d85 100644 --- a/libs/libzrtp/src/zrtp_utils.c +++ b/libs/libzrtp/src/zrtp_utils.c @@ -488,7 +488,7 @@ zrtp_status_t _zrtp_packet_preparse( zrtp_stream_t* stream, (const char*) info->message, zrtp_ntoh16(((zrtp_packet_Hello_t*) info->message)->hdr.length)*4, ZSTR_GV(hash_str) ); - if (!zrtp_memcmp(stream->messages.signaling_hash.buffer, hash_str.buffer, ZRTP_MESSAGE_HASH_SIZE)) { + if (zrtp_memcmp(stream->messages.signaling_hash.buffer, hash_str.buffer, ZRTP_MESSAGE_HASH_SIZE)) { if (stream->zrtp->cb.event_cb.on_zrtp_security_event) { stream->zrtp->cb.event_cb.on_zrtp_security_event(stream, ZRTP_EVENT_WRONG_SIGNALING_HASH); }