From 9f9b1721e11470e7d01a3673ec902fdf325c8da2 Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 14:43:41 +0200 Subject: [PATCH 1/7] sm: manage global signing counter --- ble/sm.c | 10 ++++++++++ ble/sm.h | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/ble/sm.c b/ble/sm.c index b33d1fed3..88f90eaaf 100644 --- a/ble/sm.c +++ b/ble/sm.c @@ -137,6 +137,8 @@ static derived_key_generation_t dkg_state; // derived from sm_persistent_er // .. +static uint32_t sm_signing_counter = 0; + // random address update static random_address_update_t rau_state; static bd_addr_t sm_random_address; @@ -2225,6 +2227,14 @@ void sm_set_ir(sm_key_t ir){ memcpy(sm_persistent_ir, ir, 16); } +void sm_set_signing_counter(uint32_t signing_counter){ + sm_signing_counter = signing_counter; +} + +uint32_t sm_signing_counter_next(void){ + return sm_signing_counter++; +} + // Testing support only void sm_test_set_irk(sm_key_t irk){ memcpy(sm_persistent_irk, irk, 16); diff --git a/ble/sm.h b/ble/sm.h index 41c973a56..80b8ca0da 100644 --- a/ble/sm.h +++ b/ble/sm.h @@ -147,6 +147,14 @@ void sm_set_er(sm_key_t er); */ void sm_set_ir(sm_key_t ir); +/** + * @brief Set signning counnter used with Connection Signature Resolving Key (CSRK) + * @note Needs to be stored in persistent memory as a signed writes are invalid if a higher counter was + * recevied before + * @param signing counter + */ +void sm_set_signing_counter(uint32_t signing_counter); + /** * * @brief Registers OOB Data Callback. The callback should set the oob_data and return 1 if OOB data is availble @@ -263,6 +271,11 @@ void sm_authorization_grant(uint8_t addr_type, bd_addr_t address); int sm_cmac_ready(void); void sm_cmac_start(sm_key_t k, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])); +/** + * @brief Get global signing counter and increment it + */ +uint32_t sm_signing_counter_next(void); + /* * @brief Match address against bonded devices * @return 0 if successfully added to lookup queue From 7b8f2e5c7ae654db39c7c7122d06129ac2f99d7b Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 15:03:15 +0200 Subject: [PATCH 2/7] sm: use IRK prefix for address resolution state --- ble/sm.c | 20 ++++++++++---------- src/hci.h | 14 +++++++------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ble/sm.c b/ble/sm.c index 88f90eaaf..a60bbab1b 100644 --- a/ble/sm.c +++ b/ble/sm.c @@ -918,7 +918,7 @@ static void sm_address_resolution_handle_event(address_resolution_event_t event) sm_connection = (sm_connection_t *) context; switch (event){ case ADDRESS_RESOLUTION_SUCEEDED: - sm_connection->sm_csrk_lookup_state = CSRK_LOOKUP_SUCCEEDED; + sm_connection->sm_irk_lookup_state = IRK_LOOKUP_SUCCEEDED; sm_connection->sm_le_db_index = matched_device_id; log_info("ADDRESS_RESOLUTION_SUCEEDED, index %d", sm_connection->sm_le_db_index); if (sm_connection->sm_role) break; @@ -933,7 +933,7 @@ static void sm_address_resolution_handle_event(address_resolution_event_t event) } break; case ADDRESS_RESOLUTION_FAILED: - sm_connection->sm_csrk_lookup_state = CSRK_LOOKUP_FAILED; + sm_connection->sm_irk_lookup_state = IRK_LOOKUP_FAILED; if (sm_connection->sm_role) break; if (!sm_connection->sm_bonding_requested && !sm_connection->sm_security_request_received) break; sm_connection->sm_security_request_received = 0; @@ -1040,10 +1040,10 @@ static void sm_run(void){ while(linked_list_iterator_has_next(&it)){ hci_connection_t * hci_connection = (hci_connection_t *) linked_list_iterator_next(&it); sm_connection_t * sm_connection = &hci_connection->sm_connection; - if (sm_connection->sm_csrk_lookup_state == CSRK_LOOKUP_W4_READY){ + if (sm_connection->sm_irk_lookup_state == IRK_LOOKUP_W4_READY){ // and start lookup sm_address_resolution_start_lookup(sm_connection->sm_peer_addr_type, sm_connection->sm_peer_address, ADDRESS_RESOLUTION_FOR_CONNECTION, sm_connection); - sm_connection->sm_csrk_lookup_state = CSRK_LOOKUP_STARTED; + sm_connection->sm_irk_lookup_state = IRK_LOOKUP_STARTED; break; } } @@ -1746,7 +1746,7 @@ static void sm_event_packet_handler (uint8_t packet_type, uint16_t channel, uint sm_conn->sm_le_db_index = -1; // prepare CSRK lookup (does not involve setup) - sm_conn->sm_csrk_lookup_state = CSRK_LOOKUP_W4_READY; + sm_conn->sm_irk_lookup_state = IRK_LOOKUP_W4_READY; // just connected -> everything else happens in sm_run() if (sm_conn->sm_role){ @@ -1956,11 +1956,11 @@ static void sm_packet_handler(uint8_t packet_type, uint16_t handle, uint8_t *pac sm_pdu_received_in_wrong_state(sm_conn); break; } - if (sm_conn->sm_csrk_lookup_state == CSRK_LOOKUP_FAILED){ + if (sm_conn->sm_irk_lookup_state == IRK_LOOKUP_FAILED){ sm_conn->sm_engine_state = SM_INITIATOR_PH1_W2_SEND_PAIRING_REQUEST; break; } - if (sm_conn->sm_csrk_lookup_state == CSRK_LOOKUP_SUCCEEDED){ + if (sm_conn->sm_irk_lookup_state == IRK_LOOKUP_SUCCEEDED){ uint16_t ediv; le_device_db_encryption_get(sm_conn->sm_le_db_index, &ediv, NULL, NULL, NULL, NULL, NULL); if (ediv){ @@ -2342,11 +2342,11 @@ void sm_request_authorization(uint8_t addr_type, bd_addr_t address){ // used as a trigger to start central/master/initiator security procedures uint16_t ediv; if (sm_conn->sm_engine_state == SM_INITIATOR_CONNECTED){ - switch (sm_conn->sm_csrk_lookup_state){ - case CSRK_LOOKUP_FAILED: + switch (sm_conn->sm_irk_lookup_state){ + case IRK_LOOKUP_FAILED: sm_conn->sm_engine_state = SM_INITIATOR_PH1_W2_SEND_PAIRING_REQUEST; break; - case CSRK_LOOKUP_SUCCEEDED: + case IRK_LOOKUP_SUCCEEDED: le_device_db_encryption_get(sm_conn->sm_le_db_index, &ediv, NULL, NULL, NULL, NULL, NULL); if (ediv){ log_info("sm: Setting up previous ltk/ediv/rand for device index %u", sm_conn->sm_le_db_index); diff --git a/src/hci.h b/src/hci.h index df27f6e6f..7e0f82cdd 100644 --- a/src/hci.h +++ b/src/hci.h @@ -435,12 +435,12 @@ typedef enum { } security_manager_state_t; typedef enum { - CSRK_LOOKUP_IDLE, - CSRK_LOOKUP_W4_READY, - CSRK_LOOKUP_STARTED, - CSRK_LOOKUP_SUCCEEDED, - CSRK_LOOKUP_FAILED -} csrk_lookup_state_t; + IRK_LOOKUP_IDLE, + IRK_LOOKUP_W4_READY, + IRK_LOOKUP_STARTED, + IRK_LOOKUP_SUCCEEDED, + IRK_LOOKUP_FAILED +} irk_lookup_state_t; // Authorization state typedef enum { @@ -469,7 +469,7 @@ typedef struct sm_connection { uint8_t sm_peer_addr_type; bd_addr_t sm_peer_address; security_manager_state_t sm_engine_state; - csrk_lookup_state_t sm_csrk_lookup_state; + irk_lookup_state_t sm_irk_lookup_state; uint8_t sm_connection_encrypted; uint8_t sm_connection_authenticated; // [0..1] uint8_t sm_actual_encryption_key_size; From db894f169ef5151c980d7d74cae1527d09a9e08d Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 15:11:44 +0200 Subject: [PATCH 3/7] Revert "sm: manage global signing counter" This reverts commit 9f9b1721e11470e7d01a3673ec902fdf325c8da2. --- ble/sm.c | 10 ---------- ble/sm.h | 13 ------------- 2 files changed, 23 deletions(-) diff --git a/ble/sm.c b/ble/sm.c index a60bbab1b..39f05d393 100644 --- a/ble/sm.c +++ b/ble/sm.c @@ -137,8 +137,6 @@ static derived_key_generation_t dkg_state; // derived from sm_persistent_er // .. -static uint32_t sm_signing_counter = 0; - // random address update static random_address_update_t rau_state; static bd_addr_t sm_random_address; @@ -2227,14 +2225,6 @@ void sm_set_ir(sm_key_t ir){ memcpy(sm_persistent_ir, ir, 16); } -void sm_set_signing_counter(uint32_t signing_counter){ - sm_signing_counter = signing_counter; -} - -uint32_t sm_signing_counter_next(void){ - return sm_signing_counter++; -} - // Testing support only void sm_test_set_irk(sm_key_t irk){ memcpy(sm_persistent_irk, irk, 16); diff --git a/ble/sm.h b/ble/sm.h index 80b8ca0da..41c973a56 100644 --- a/ble/sm.h +++ b/ble/sm.h @@ -147,14 +147,6 @@ void sm_set_er(sm_key_t er); */ void sm_set_ir(sm_key_t ir); -/** - * @brief Set signning counnter used with Connection Signature Resolving Key (CSRK) - * @note Needs to be stored in persistent memory as a signed writes are invalid if a higher counter was - * recevied before - * @param signing counter - */ -void sm_set_signing_counter(uint32_t signing_counter); - /** * * @brief Registers OOB Data Callback. The callback should set the oob_data and return 1 if OOB data is availble @@ -271,11 +263,6 @@ void sm_authorization_grant(uint8_t addr_type, bd_addr_t address); int sm_cmac_ready(void); void sm_cmac_start(sm_key_t k, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])); -/** - * @brief Get global signing counter and increment it - */ -uint32_t sm_signing_counter_next(void); - /* * @brief Match address against bonded devices * @return 0 if successfully added to lookup queue From 12e4aa3c160a11c2633bcc3321f4e4329eacd25e Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 16:47:23 +0200 Subject: [PATCH 4/7] sm: use CSRK of Peripheral to save persistent memory, fix CMAC calculation --- ble/sm.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ble/sm.c b/ble/sm.c index 39f05d393..82e8dcddc 100644 --- a/ble/sm.c +++ b/ble/sm.c @@ -622,7 +622,7 @@ static inline uint8_t sm_cmac_message_get_byte(int offset){ if (offset < actual_len) { return sm_cmac_message[offset]; } else { - return sm_cmac_message[offset - actual_len]; + return sm_cmac_sign_counter[offset - actual_len]; } } @@ -643,6 +643,8 @@ void sm_cmac_start(sm_key_t k, uint16_t message_len, uint8_t * message, uint32_t sm_cmac_block_count = 1; } + log_info("sm_cmac_start: len %u, block count %u", sm_cmac_message_len, sm_cmac_block_count); + // first, we need to compute l for k1, k2, and m_last sm_cmac_state = CMAC_CALC_SUBKEYS; @@ -732,6 +734,7 @@ static void sm_cmac_handle_encryption_result(sm_key_t data){ sm_cmac_m_last[i] = k2[i]; } } + log_key("last", sm_cmac_m_last); // next @@ -1428,8 +1431,17 @@ static void sm_run(void){ } if (setup->sm_key_distribution_send_set & SM_KEYDIST_FLAG_SIGNING_IDENTIFICATION){ setup->sm_key_distribution_send_set &= ~SM_KEYDIST_FLAG_SIGNING_IDENTIFICATION; + uint8_t buffer[17]; buffer[0] = SM_CODE_SIGNING_INFORMATION; + // optimization: use CSRK of Peripheral if received, to avoid storing two CSRKs in our DB + if (setup->sm_key_distribution_received_set & SM_KEYDIST_FLAG_SIGNING_IDENTIFICATION){ + log_info("sm: mirror CSRK"); + memcpy(setup->sm_local_csrk, setup->sm_peer_csrk, 16); + } else { + log_info("sm: store local CSRK"); + le_device_db_csrk_set(connection->sm_le_db_index, setup->sm_local_csrk); + } swap128(setup->sm_local_csrk, &buffer[1]); l2cap_send_connectionless(connection->sm_handle, L2CAP_CID_SECURITY_MANAGER_PROTOCOL, (uint8_t*) buffer, sizeof(buffer)); sm_timeout_reset(connection); @@ -2155,7 +2167,7 @@ static void sm_packet_handler(uint8_t packet_type, uint16_t handle, uint8_t *pac // store CSRK if (setup->sm_key_distribution_received_set & SM_KEYDIST_FLAG_SIGNING_IDENTIFICATION){ - log_info("sm: set csrk"); + log_info("sm: store remote CSRK"); le_device_db_csrk_set(le_db_index, setup->sm_peer_csrk); le_device_db_remote_counter_set(le_db_index, 0); } @@ -2169,6 +2181,9 @@ static void sm_packet_handler(uint8_t packet_type, uint16_t handle, uint8_t *pac } } + // keep le_db_index + sm_conn->sm_le_db_index = le_db_index; + if (sm_conn->sm_role){ sm_conn->sm_engine_state = SM_RESPONDER_IDLE; sm_done_for_handle(sm_conn->sm_handle); From 03679531e460c1ad6f419e523b95c7cc0d69732f Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 22:39:19 +0200 Subject: [PATCH 5/7] sm: flip message data before calculating CMAC, also store hash result as little endian --- ble/att_server.c | 7 +++++-- ble/gatt_client.c | 5 ++--- ble/sm.c | 27 +++++++++++++++++---------- ble/sm.h | 2 +- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/ble/att_server.c b/ble/att_server.c index aa06a1d5e..d4f701e6d 100644 --- a/ble/att_server.c +++ b/ble/att_server.c @@ -211,7 +211,9 @@ static void att_signed_write_handle_cmac_result(uint8_t hash[8]){ if (att_server_state != ATT_SERVER_W4_SIGNED_WRITE_VALIDATION) return; - if (memcmp(hash, &att_request_buffer[att_request_size-8], 8)){ + uint8_t hash_flipped[8]; + swap64(hash, hash_flipped); + if (memcmp(hash_flipped, &att_request_buffer[att_request_size-8], 8)){ log_info("ATT Signed Write, invalid signature"); att_server_state = ATT_SERVER_IDLE; return; @@ -268,7 +270,8 @@ static void att_run(void){ att_server_state = ATT_SERVER_W4_SIGNED_WRITE_VALIDATION; log_info("Orig Signature: "); hexdump( &att_request_buffer[att_request_size-8], 8); - sm_cmac_start(csrk, att_request_size - 12, att_request_buffer, counter_packet, att_signed_write_handle_cmac_result); + uint16_t attribute_handle = READ_BT_16(att_request_buffer, 1); + sm_cmac_start(csrk, att_request_buffer[0], attribute_handle, att_request_size - 15, &att_request_buffer[3], counter_packet, att_signed_write_handle_cmac_result); return; } // NOTE: fall through for regular commands diff --git a/ble/gatt_client.c b/ble/gatt_client.c index 907c48d0a..52aed181a 100644 --- a/ble/gatt_client.c +++ b/ble/gatt_client.c @@ -293,8 +293,7 @@ static void att_signed_write_request(uint16_t request_type, uint16_t peripheral_ bt_store_16(request, 1, attribute_handle); memcpy(&request[3], value, value_length); bt_store_32(request, 3 + value_length, sign_counter); - memcpy(&request[3 + value_length+4], sgn, 8); - + swap64(sgn, &request[3 + value_length + 4]); l2cap_send_prepared_connectionless(peripheral_handle, L2CAP_CID_ATTRIBUTE_PROTOCOL, 3 + value_length + 12); } @@ -859,7 +858,7 @@ static void gatt_client_run(void){ le_device_db_csrk_get(peripheral->le_device_index, csrk); uint32_t sign_counter = le_device_db_local_counter_get(peripheral->le_device_index); peripheral->gatt_client_state = P_W4_CMAC_RESULT; - sm_cmac_start(csrk, peripheral->attribute_length, peripheral->attribute_value, sign_counter, att_signed_write_handle_cmac_result); + sm_cmac_start(csrk, ATT_SIGNED_WRITE_COMMAND, peripheral->attribute_handle, peripheral->attribute_length, peripheral->attribute_value, sign_counter, att_signed_write_handle_cmac_result); } return; diff --git a/ble/sm.c b/ble/sm.c index 82e8dcddc..bea73c6d9 100644 --- a/ble/sm.c +++ b/ble/sm.c @@ -144,6 +144,7 @@ static bd_addr_t sm_random_address; // CMAC calculation static cmac_state_t sm_cmac_state; static sm_key_t sm_cmac_k; +static uint8_t sm_cmac_header[3]; static uint16_t sm_cmac_message_len; static uint8_t * sm_cmac_message; static uint8_t sm_cmac_sign_counter[4]; @@ -618,19 +619,27 @@ static inline uint8_t sm_cmac_message_get_byte(int offset){ log_error("sm_cmac_message_get_byte. out of bounds, access %u, len %u", offset, sm_cmac_message_len); return 0; } - int actual_len = sm_cmac_message_len - 4; - if (offset < actual_len) { - return sm_cmac_message[offset]; - } else { - return sm_cmac_sign_counter[offset - actual_len]; + + offset = sm_cmac_message_len - 1 - offset; + + // sm_cmac_header[3] | message[] | sm_cmac_sign_counter[4] + if (offset < 3){ + return sm_cmac_header[offset]; } + int actual_message_len_incl_header = sm_cmac_message_len - 4; + if (offset < actual_message_len_incl_header){ + return sm_cmac_message[offset - 3]; + } + return sm_cmac_sign_counter[offset - actual_message_len_incl_header]; } -void sm_cmac_start(sm_key_t k, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])){ +void sm_cmac_start(sm_key_t k, uint8_t opcode, uint16_t handle, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])){ memcpy(sm_cmac_k, k, 16); - sm_cmac_message_len = message_len + 4; // incl. virtually appended sign_counter in LE - sm_cmac_message = message; + sm_cmac_header[0] = opcode; + bt_store_16(sm_cmac_header, 1, handle); bt_store_32(sm_cmac_sign_counter, 0, sign_counter); + sm_cmac_message_len = 3 + message_len + 4; // incl. virtually prepended att opcode, handle and appended sign_counter in LE + sm_cmac_message = message; sm_cmac_done_handler = done_handler; sm_cmac_block_current = 0; memset(sm_cmac_x, 0, 16); @@ -734,8 +743,6 @@ static void sm_cmac_handle_encryption_result(sm_key_t data){ sm_cmac_m_last[i] = k2[i]; } } - log_key("last", sm_cmac_m_last); - // next sm_cmac_state = sm_cmac_block_current < sm_cmac_block_count - 1 ? CMAC_CALC_MI : CMAC_CALC_MLAST; diff --git a/ble/sm.h b/ble/sm.h index 41c973a56..9ee6041f9 100644 --- a/ble/sm.h +++ b/ble/sm.h @@ -261,7 +261,7 @@ void sm_authorization_grant(uint8_t addr_type, bd_addr_t address); * @note Message and result are in little endian to allows passing in ATT PDU without flipping them first. */ int sm_cmac_ready(void); -void sm_cmac_start(sm_key_t k, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])); +void sm_cmac_start(sm_key_t k, uint8_t opcode, uint16_t attribute_handle, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])); /* * @brief Match address against bonded devices From e7409fd6e27b5160417f642926af0e32642fc3d8 Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 22:44:03 +0200 Subject: [PATCH 6/7] sm: document sm_cmac_start result --- ble/sm.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ble/sm.h b/ble/sm.h index 9ee6041f9..49cb5641b 100644 --- a/ble/sm.h +++ b/ble/sm.h @@ -258,7 +258,8 @@ void sm_authorization_grant(uint8_t addr_type, bd_addr_t address); /** * @brief Support for signed writes, used by att_server. - * @note Message and result are in little endian to allows passing in ATT PDU without flipping them first. + * @note Message and result are in little endian to allows passing in ATT PDU without flipping. + * @note calculated hash in done_callback is big endian */ int sm_cmac_ready(void); void sm_cmac_start(sm_key_t k, uint8_t opcode, uint16_t attribute_handle, uint16_t message_len, uint8_t * message, uint32_t sign_counter, void (*done_handler)(uint8_t hash[8])); From d654805b3d925cd69143f253ed390d28d7695c89 Mon Sep 17 00:00:00 2001 From: Matthias Ringwald Date: Wed, 19 Aug 2015 22:50:58 +0200 Subject: [PATCH 7/7] added helper to show packet log --- test/pts/packetlogger.sh | 3 +++ 1 file changed, 3 insertions(+) create mode 100755 test/pts/packetlogger.sh diff --git a/test/pts/packetlogger.sh b/test/pts/packetlogger.sh new file mode 100755 index 000000000..c3cf7ae08 --- /dev/null +++ b/test/pts/packetlogger.sh @@ -0,0 +1,3 @@ +#!/bin/sh +killall PacketLogger +open /tmp/hci_dump.pklg