From ac60dfc680deafe5e0e26fc18e6b71b61f647760 Mon Sep 17 00:00:00 2001 From: Christoph Hagen Date: Sun, 8 Oct 2017 02:31:13 +0900 Subject: [PATCH 1/3] Another magic number removed --- src/curve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/curve.c b/src/curve.c index dbeb37e1..eb3b9c74 100644 --- a/src/curve.c +++ b/src/curve.c @@ -597,7 +597,7 @@ int curve_verify_vrf_signature(signal_context *context, return SG_ERR_INVAL; } - if(!message_data || !signature_data || signature_len != 96) { + if(!message_data || !signature_data || signature_len != VRF_SIGNATURE_LEN) { signal_log(context, SG_LOG_ERROR, "Invalid message or signature format"); return SG_ERR_VRF_SIG_VERIF_FAILED; } From f03ee80c9f181f2f4f4989cf357978f392c33671 Mon Sep 17 00:00:00 2001 From: Christoph Hagen Date: Sun, 8 Oct 2017 02:32:05 +0900 Subject: [PATCH 2/3] Revert "Another magic number removed" This reverts commit ac60dfc680deafe5e0e26fc18e6b71b61f647760. --- src/curve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/curve.c b/src/curve.c index eb3b9c74..dbeb37e1 100644 --- a/src/curve.c +++ b/src/curve.c @@ -597,7 +597,7 @@ int curve_verify_vrf_signature(signal_context *context, return SG_ERR_INVAL; } - if(!message_data || !signature_data || signature_len != VRF_SIGNATURE_LEN) { + if(!message_data || !signature_data || signature_len != 96) { signal_log(context, SG_LOG_ERROR, "Invalid message or signature format"); return SG_ERR_VRF_SIG_VERIF_FAILED; } From 010556926074e2d9f8203b8de3f2b0fb1cfad54d Mon Sep 17 00:00:00 2001 From: Christoph Hagen Date: Tue, 5 Dec 2017 14:24:11 +0100 Subject: [PATCH 3/3] remove pending key exchange session_pending_key_exchange is never used inside session_state.c or anywhere else, and while it is exposed via session_state.h, it is not included in signal_protocol.h --- src/session_state.c | 351 ------------------------------------ src/session_state.h | 10 - tests/test_session_record.c | 74 -------- 3 files changed, 435 deletions(-) diff --git a/src/session_state.c b/src/session_state.c index 94cf9d30..3a6bc607 100644 --- a/src/session_state.c +++ b/src/session_state.c @@ -35,14 +35,6 @@ typedef struct session_state_receiver_chain struct session_state_receiver_chain *prev, *next; } session_state_receiver_chain; -typedef struct session_pending_key_exchange -{ - uint32_t sequence; - ec_key_pair *local_base_key; - ec_key_pair *local_ratchet_key; - ratchet_identity_key_pair *local_identity_key; -} session_pending_key_exchange; - typedef struct session_pending_pre_key { int has_pre_key_id; @@ -67,9 +59,6 @@ struct session_state session_state_receiver_chain *receiver_chain_head; - int has_pending_key_exchange; - session_pending_key_exchange pending_key_exchange; - int has_pending_pre_key; session_pending_pre_key pending_pre_key; @@ -101,21 +90,12 @@ static int session_state_serialize_prepare_message_keys( Textsecure__SessionStructure__Chain__MessageKey *message_key_structure); static void session_state_serialize_prepare_message_keys_free( Textsecure__SessionStructure__Chain__MessageKey *message_key_structure); -static int session_state_serialize_prepare_pending_key_exchange( - session_pending_key_exchange *exchange, - Textsecure__SessionStructure__PendingKeyExchange *exchange_structure); -static void session_state_serialize_prepare_pending_key_exchange_free( - Textsecure__SessionStructure__PendingKeyExchange *exchange_structure); static int session_state_serialize_prepare_pending_pre_key( session_pending_pre_key *pre_key, Textsecure__SessionStructure__PendingPreKey *pre_key_structure); static void session_state_serialize_prepare_pending_pre_key_free( Textsecure__SessionStructure__PendingPreKey *pre_key_structure); -static int session_state_deserialize_protobuf_pending_key_exchange( - session_pending_key_exchange *result_exchange, - Textsecure__SessionStructure__PendingKeyExchange *exchange_structure, - signal_context *global_context); static int session_state_deserialize_protobuf_pending_pre_key( session_pending_pre_key *result_pre_key, Textsecure__SessionStructure__PendingPreKey *pre_key_structure, @@ -321,21 +301,6 @@ int session_state_serialize_prepare(session_state *state, Textsecure__SessionStr } } - if(state->has_pending_key_exchange) { - session_structure->pendingkeyexchange = malloc(sizeof(Textsecure__SessionStructure__PendingKeyExchange)); - if(!session_structure->pendingkeyexchange) { - result = SG_ERR_NOMEM; - goto complete; - } - textsecure__session_structure__pending_key_exchange__init(session_structure->pendingkeyexchange); - result = session_state_serialize_prepare_pending_key_exchange( - &state->pending_key_exchange, - session_structure->pendingkeyexchange); - if(result < 0) { - goto complete; - } - } - if(state->has_pending_pre_key) { session_structure->pendingprekey = malloc(sizeof(Textsecure__SessionStructure__PendingPreKey)); if(!session_structure->pendingprekey) { @@ -575,76 +540,6 @@ static void session_state_serialize_prepare_message_keys_free( free(message_key_structure); } -static int session_state_serialize_prepare_pending_key_exchange( - session_pending_key_exchange *exchange, - Textsecure__SessionStructure__PendingKeyExchange *exchange_structure) -{ - int result = 0; - - exchange_structure->has_sequence = 1; - exchange_structure->sequence = exchange->sequence; - - if(exchange->local_base_key) { - ec_public_key *public_key = 0; - ec_private_key *private_key = 0; - - public_key = ec_key_pair_get_public(exchange->local_base_key); - result = ec_public_key_serialize_protobuf(&exchange_structure->localbasekey, public_key); - if(result < 0) { - goto complete; - } - exchange_structure->has_localbasekey = 1; - - private_key = ec_key_pair_get_private(exchange->local_base_key); - result = ec_private_key_serialize_protobuf(&exchange_structure->localbasekeyprivate, private_key); - if(result < 0) { - goto complete; - } - exchange_structure->has_localbasekeyprivate = 1; - } - - if(exchange->local_ratchet_key) { - ec_public_key *public_key; - ec_private_key *private_key; - - public_key = ec_key_pair_get_public(exchange->local_ratchet_key); - result = ec_public_key_serialize_protobuf(&exchange_structure->localratchetkey, public_key); - if(result < 0) { - goto complete; - } - exchange_structure->has_localratchetkey = 1; - - private_key = ec_key_pair_get_private(exchange->local_ratchet_key); - result = ec_private_key_serialize_protobuf(&exchange_structure->localratchetkeyprivate, private_key); - if(result < 0) { - goto complete; - } - exchange_structure->has_localratchetkeyprivate = 1; - } - - if(exchange->local_identity_key) { - ec_public_key *public_key; - ec_private_key *private_key; - - public_key = ratchet_identity_key_pair_get_public(exchange->local_identity_key); - result = ec_public_key_serialize_protobuf(&exchange_structure->localidentitykey, public_key); - if(result < 0) { - goto complete; - } - exchange_structure->has_localidentitykey = 1; - - private_key = ratchet_identity_key_pair_get_private(exchange->local_identity_key); - result = ec_private_key_serialize_protobuf(&exchange_structure->localidentitykeyprivate, private_key); - if(result < 0) { - goto complete; - } - exchange_structure->has_localidentitykeyprivate = 1; - } - -complete: - return result; -} - static int session_state_serialize_prepare_pending_pre_key( session_pending_pre_key *pre_key, Textsecure__SessionStructure__PendingPreKey *pre_key_structure) @@ -701,10 +596,6 @@ void session_state_serialize_prepare_free(Textsecure__SessionStructure *session_ free(session_structure->receiverchains); } - if(session_structure->pendingkeyexchange) { - session_state_serialize_prepare_pending_key_exchange_free(session_structure->pendingkeyexchange); - } - if(session_structure->pendingprekey) { session_state_serialize_prepare_pending_pre_key_free(session_structure->pendingprekey); } @@ -743,30 +634,6 @@ static void session_state_serialize_prepare_chain_free( free(chain_structure); } -static void session_state_serialize_prepare_pending_key_exchange_free( - Textsecure__SessionStructure__PendingKeyExchange *exchange_structure) -{ - if(exchange_structure->has_localbasekey) { - free(exchange_structure->localbasekey.data); - } - if(exchange_structure->has_localbasekeyprivate) { - free(exchange_structure->localbasekeyprivate.data); - } - if(exchange_structure->has_localratchetkey) { - free(exchange_structure->localratchetkey.data); - } - if(exchange_structure->has_localratchetkeyprivate) { - free(exchange_structure->localratchetkeyprivate.data); - } - if(exchange_structure->has_localidentitykey) { - free(exchange_structure->localidentitykey.data); - } - if(exchange_structure->has_localidentitykeyprivate) { - free(exchange_structure->localidentitykeyprivate.data); - } - free(exchange_structure); -} - static void session_state_serialize_prepare_pending_pre_key_free( Textsecure__SessionStructure__PendingPreKey *pre_key_structure) { @@ -868,16 +735,6 @@ int session_state_deserialize_protobuf(session_state **state, Textsecure__Sessio } } - if(session_structure->pendingkeyexchange) { - result = session_state_deserialize_protobuf_pending_key_exchange( - &result_state->pending_key_exchange, - session_structure->pendingkeyexchange, global_context); - if(result < 0) { - goto complete; - } - result_state->has_pending_key_exchange = 1; - } - if(session_structure->pendingprekey) { result = session_state_deserialize_protobuf_pending_pre_key( &result_state->pending_pre_key, @@ -923,119 +780,6 @@ int session_state_deserialize_protobuf(session_state **state, Textsecure__Sessio return result; } -static int session_state_deserialize_protobuf_pending_key_exchange( - session_pending_key_exchange *result_exchange, - Textsecure__SessionStructure__PendingKeyExchange *exchange_structure, - signal_context *global_context) -{ - int result = 0; - - ec_key_pair *local_base_key = 0; - ec_public_key *local_base_key_public = 0; - ec_private_key *local_base_key_private = 0; - - ec_key_pair *local_ratchet_key = 0; - ec_public_key *local_ratchet_key_public = 0; - ec_private_key *local_ratchet_key_private = 0; - - ratchet_identity_key_pair *local_identity_key = 0; - ec_public_key *local_identity_key_public = 0; - ec_private_key *local_identity_key_private = 0; - - if(exchange_structure->has_localbasekey && exchange_structure->has_localbasekeyprivate) { - result = curve_decode_point(&local_base_key_public, - exchange_structure->localbasekey.data, - exchange_structure->localbasekey.len, - global_context); - if(result < 0) { - goto complete; - } - - result = curve_decode_private_point(&local_base_key_private, - exchange_structure->localbasekeyprivate.data, - exchange_structure->localbasekeyprivate.len, - global_context); - if(result < 0) { - goto complete; - } - - result = ec_key_pair_create(&local_base_key, - local_base_key_public, local_base_key_private); - if(result < 0) { - goto complete; - } - } - - if(exchange_structure->has_localratchetkey && exchange_structure->has_localratchetkeyprivate) { - result = curve_decode_point(&local_ratchet_key_public, - exchange_structure->localratchetkey.data, - exchange_structure->localratchetkey.len, - global_context); - if(result < 0) { - goto complete; - } - - result = curve_decode_private_point(&local_ratchet_key_private, - exchange_structure->localratchetkeyprivate.data, - exchange_structure->localratchetkeyprivate.len, - global_context); - if(result < 0) { - goto complete; - } - - result = ec_key_pair_create(&local_ratchet_key, - local_ratchet_key_public, local_ratchet_key_private); - if(result < 0) { - goto complete; - } - } - - if(exchange_structure->has_localidentitykey && exchange_structure->has_localidentitykeyprivate) { - result = curve_decode_point(&local_identity_key_public, - exchange_structure->localidentitykey.data, - exchange_structure->localidentitykey.len, - global_context); - if(result < 0) { - goto complete; - } - - result = curve_decode_private_point(&local_identity_key_private, - exchange_structure->localidentitykeyprivate.data, - exchange_structure->localidentitykeyprivate.len, - global_context); - if(result < 0) { - goto complete; - } - - result = ratchet_identity_key_pair_create(&local_identity_key, - local_identity_key_public, - local_identity_key_private); - if(result < 0) { - goto complete; - } - } - - result_exchange->sequence = exchange_structure->sequence; - result_exchange->local_base_key = local_base_key; - result_exchange->local_ratchet_key = local_ratchet_key; - result_exchange->local_identity_key = local_identity_key; - -complete: - SIGNAL_UNREF(local_base_key_public); - SIGNAL_UNREF(local_base_key_private); - SIGNAL_UNREF(local_ratchet_key_public); - SIGNAL_UNREF(local_ratchet_key_private); - SIGNAL_UNREF(local_identity_key_public); - SIGNAL_UNREF(local_identity_key_private); - - if(result < 0) { - SIGNAL_UNREF(local_base_key); - SIGNAL_UNREF(local_ratchet_key); - SIGNAL_UNREF(local_identity_key); - } - - return result; -} static int session_state_deserialize_protobuf_pending_pre_key( session_pending_pre_key *result_pre_key, @@ -1574,90 +1318,6 @@ ratchet_chain_key *session_state_get_receiver_chain_key(session_state *state, ec return result; } -void session_state_set_pending_key_exchange(session_state *state, - uint32_t sequence, - ec_key_pair *our_base_key, ec_key_pair *our_ratchet_key, - ratchet_identity_key_pair *our_identity_key) -{ - assert(state); - assert(our_base_key); - assert(our_ratchet_key); - assert(our_identity_key); - - if(state->pending_key_exchange.local_base_key) { - SIGNAL_UNREF(state->pending_key_exchange.local_base_key); - state->pending_key_exchange.local_base_key = 0; - } - if(state->pending_key_exchange.local_ratchet_key) { - SIGNAL_UNREF(state->pending_key_exchange.local_ratchet_key); - state->pending_key_exchange.local_ratchet_key = 0; - } - if(state->pending_key_exchange.local_identity_key) { - SIGNAL_UNREF(state->pending_key_exchange.local_identity_key); - state->pending_key_exchange.local_identity_key = 0; - } - - SIGNAL_REF(our_base_key); - SIGNAL_REF(our_ratchet_key); - SIGNAL_REF(our_identity_key); - - state->has_pending_key_exchange = 1; - state->pending_key_exchange.sequence = sequence; - state->pending_key_exchange.local_base_key = our_base_key; - state->pending_key_exchange.local_ratchet_key = our_ratchet_key; - state->pending_key_exchange.local_identity_key = our_identity_key; -} - -uint32_t session_state_get_pending_key_exchange_sequence(session_state *state) -{ - assert(state); - if(state->has_pending_key_exchange) { - return state->pending_key_exchange.sequence; - } - else { - return 0; - } -} - -ec_key_pair *session_state_get_pending_key_exchange_base_key(const session_state *state) -{ - assert(state); - if(state->has_pending_key_exchange) { - return state->pending_key_exchange.local_base_key; - } - else { - return 0; - } -} - -ec_key_pair *session_state_get_pending_key_exchange_ratchet_key(const session_state *state) -{ - assert(state); - if(state->has_pending_key_exchange) { - return state->pending_key_exchange.local_ratchet_key; - } - else { - return 0; - } -} - -ratchet_identity_key_pair *session_state_get_pending_key_exchange_identity_key(const session_state *state) -{ - assert(state); - if(state->has_pending_key_exchange) { - return state->pending_key_exchange.local_identity_key; - } - else { - return 0; - } -} - -int session_state_has_pending_key_exchange(const session_state *state) -{ - assert(state); - return state->has_pending_key_exchange; -} - void session_state_set_unacknowledged_pre_key_message(session_state *state, const uint32_t *pre_key_id, uint32_t signed_pre_key_id, ec_public_key *base_key) { @@ -1852,17 +1512,6 @@ void session_state_destroy(signal_type_base *type) } session_state_free_sender_chain(state); session_state_free_receiver_chain(state); - if(state->has_pending_key_exchange) { - if(state->pending_key_exchange.local_base_key) { - SIGNAL_UNREF(state->pending_key_exchange.local_base_key); - } - if(state->pending_key_exchange.local_ratchet_key) { - SIGNAL_UNREF(state->pending_key_exchange.local_ratchet_key); - } - if(state->pending_key_exchange.local_identity_key) { - SIGNAL_UNREF(state->pending_key_exchange.local_identity_key); - } - } if(state->has_pending_pre_key) { if(state->pending_pre_key.base_key) { SIGNAL_UNREF(state->pending_pre_key.base_key); diff --git a/src/session_state.h b/src/session_state.h index ef1ecaed..da4be2bb 100644 --- a/src/session_state.h +++ b/src/session_state.h @@ -49,16 +49,6 @@ int session_state_add_receiver_chain(session_state *state, ec_public_key *sender int session_state_set_receiver_chain_key(session_state *state, ec_public_key *sender_ephemeral, ratchet_chain_key *chain_key); ratchet_chain_key *session_state_get_receiver_chain_key(session_state *state, ec_public_key *sender_ephemeral); -void session_state_set_pending_key_exchange(session_state *state, - uint32_t sequence, - ec_key_pair *our_base_key, ec_key_pair *our_ratchet_key, - ratchet_identity_key_pair *our_identity_key); -uint32_t session_state_get_pending_key_exchange_sequence(session_state *state); -ec_key_pair *session_state_get_pending_key_exchange_base_key(const session_state *state); -ec_key_pair *session_state_get_pending_key_exchange_ratchet_key(const session_state *state); -ratchet_identity_key_pair *session_state_get_pending_key_exchange_identity_key(const session_state *state); -int session_state_has_pending_key_exchange(const session_state *state); - void session_state_set_unacknowledged_pre_key_message(session_state *state, const uint32_t *pre_key_id, uint32_t signed_pre_key_id, ec_public_key *base_key); int session_state_unacknowledged_pre_key_message_has_pre_key_id(const session_state *state); diff --git a/tests/test_session_record.c b/tests/test_session_record.c index 6f3842f1..e267cc58 100644 --- a/tests/test_session_record.c +++ b/tests/test_session_record.c @@ -106,32 +106,6 @@ void fill_test_session_state(session_state *state, ec_public_key *receiver_chain SIGNAL_UNREF(receiver_chain_chain_key2); } - /* Set pending key exchange */ - ec_key_pair *our_base_key; - result = curve_generate_key_pair(global_context, &our_base_key); - ck_assert_int_eq(result, 0); - - ec_key_pair *our_ratchet_key; - result = curve_generate_key_pair(global_context, &our_ratchet_key); - ck_assert_int_eq(result, 0); - - ec_key_pair *our_identity_key_pair; - result = curve_generate_key_pair(global_context, &our_identity_key_pair); - ck_assert_int_eq(result, 0); - - ratchet_identity_key_pair *our_identity_key; - result = ratchet_identity_key_pair_create(&our_identity_key, - ec_key_pair_get_public(our_identity_key_pair), - ec_key_pair_get_private(our_identity_key_pair)); - ck_assert_int_eq(result, 0); - SIGNAL_UNREF(our_identity_key_pair); - - session_state_set_pending_key_exchange(state, 42, - our_base_key, our_ratchet_key, our_identity_key); - SIGNAL_UNREF(our_base_key); - SIGNAL_UNREF(our_ratchet_key); - SIGNAL_UNREF(our_identity_key); - /* Set pending pre-key */ ec_public_key *pending_pre_key_base_key = create_test_ec_public_key(global_context); uint32_t pre_key_id = 1234; @@ -242,54 +216,6 @@ void compare_session_states(session_state *state1, session_state *state2, compare_session_states_receiver_chain(state1, state2, receiver_chain_ratchet_key2); } - /* Compare pending key exchange */ - int has_pending_key_exchange1 = session_state_has_pending_key_exchange(state1); - int has_pending_key_exchange2 = session_state_has_pending_key_exchange(state2); - ck_assert_int_eq(has_pending_key_exchange1, has_pending_key_exchange2); - - if(has_pending_key_exchange1 == 1) { - /* Compare sequence numbers */ - int sequence1 = session_state_get_pending_key_exchange_sequence(state1); - int sequence2 = session_state_get_pending_key_exchange_sequence(state2); - ck_assert_int_eq(sequence1, sequence2); - - /* Compare base keys */ - ec_key_pair *base_key1 = session_state_get_pending_key_exchange_base_key(state1); - ec_key_pair *base_key2 = session_state_get_pending_key_exchange_base_key(state2); - - ec_public_key *base_key_public1 = ec_key_pair_get_public(base_key1); - ec_public_key *base_key_public2 = ec_key_pair_get_public(base_key2); - ck_assert_int_eq(ec_public_key_compare(base_key_public1, base_key_public2), 0); - - ec_private_key *base_key_private1 = ec_key_pair_get_private(base_key1); - ec_private_key *base_key_private2 = ec_key_pair_get_private(base_key2); - ck_assert_int_eq(ec_private_key_compare(base_key_private1, base_key_private2), 0); - - /* Compare ratchet keys */ - ec_key_pair *ratchet_key1 = session_state_get_pending_key_exchange_ratchet_key(state1); - ec_key_pair *ratchet_key2 = session_state_get_pending_key_exchange_ratchet_key(state2); - - ec_public_key *ratchet_key_public1 = ec_key_pair_get_public(ratchet_key1); - ec_public_key *ratchet_key_public2 = ec_key_pair_get_public(ratchet_key2); - ck_assert_int_eq(ec_public_key_compare(ratchet_key_public1, ratchet_key_public2), 0); - - ec_private_key *ratchet_key_private1 = ec_key_pair_get_private(ratchet_key1); - ec_private_key *ratchet_key_private2 = ec_key_pair_get_private(ratchet_key2); - ck_assert_int_eq(ec_private_key_compare(ratchet_key_private1, ratchet_key_private2), 0); - - /* Compare identity keys */ - ratchet_identity_key_pair *identity_key1 = session_state_get_pending_key_exchange_identity_key(state1); - ratchet_identity_key_pair *identity_key2 = session_state_get_pending_key_exchange_identity_key(state2); - - ec_public_key *identity_key_public1 = ratchet_identity_key_pair_get_public(identity_key1); - ec_public_key *identity_key_public2 = ratchet_identity_key_pair_get_public(identity_key2); - ck_assert_int_eq(ec_public_key_compare(identity_key_public1, identity_key_public2), 0); - - ec_private_key *identity_key_private1 = ratchet_identity_key_pair_get_private(identity_key1); - ec_private_key *identity_key_private2 = ratchet_identity_key_pair_get_private(identity_key2); - ck_assert_int_eq(ec_private_key_compare(identity_key_private1, identity_key_private2), 0); - } - /* Compare pending pre-key */ int has_pending_pre_key1 = session_state_has_unacknowledged_pre_key_message(state1); int has_pending_pre_key2 = session_state_has_unacknowledged_pre_key_message(state2);