From af1bd1472ce7bd7eeec0fb372ffa259ccd52f16a Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 10 Mar 2025 09:58:49 +0530 Subject: [PATCH] fix(provisioning): fix incorrect AES-GCM IV usage in security2 scheme Using same IV in AES-GCM across multiple invocation of encryption/decryption operations can pose a security risk. It can help to reveal co-relation between different plaintexts. This commit introduces a change to use part of IV as a monotonic counter, which must be incremented after every AES-GCM invocation on both the client and the device side. Concept of patch version for a security scheme has been introduced here which can help to differentiate a protocol behavior for the provisioning entity. The security patch version will be available in the JSON response for `proto-ver` endpoint request with the field `sec_patch_ver`. Please refer to documentation for more details on the changes required on the provisioning entity side (e.g., PhoneApps). --- .../protocomm/include/common/protocomm.h | 17 ++++- .../include/security/protocomm_security.h | 7 +- components/protocomm/src/common/protocomm.c | 14 +++- components/protocomm/src/security/security2.c | 72 +++++++++++++++++-- components/wifi_provisioning/src/manager.c | 39 +++++----- tools/esp_prov/esp_prov.py | 34 +++++++-- tools/esp_prov/security/security2.py | 47 ++++++++---- 7 files changed, 183 insertions(+), 47 deletions(-) diff --git a/components/protocomm/include/common/protocomm.h b/components/protocomm/include/common/protocomm.h index 5056f78771..c3e19cf587 100644 --- a/components/protocomm/include/common/protocomm.h +++ b/components/protocomm/include/common/protocomm.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -260,6 +260,21 @@ esp_err_t protocomm_set_version(protocomm_t *pc, const char *ep_name, */ esp_err_t protocomm_unset_version(protocomm_t *pc, const char *ep_name); +/** + * @brief Get the security version of the protocomm instance + * + * This API will return the security version of the protocomm instance. + * + * @param[in] pc Pointer to the protocomm instance + * @param[out] sec_ver Pointer to the security version + * @param[out] sec_patch_ver Pointer to the security patch version + * + * @return + * - ESP_OK : Success + * - ESP_ERR_INVALID_ARG : Null instance/name arguments + */ +esp_err_t protocomm_get_sec_version(protocomm_t *pc, int *sec_ver, uint8_t *sec_patch_ver); + #ifdef __cplusplus } #endif diff --git a/components/protocomm/include/security/protocomm_security.h b/components/protocomm/include/security/protocomm_security.h index fb3c491497..38c6a70e55 100644 --- a/components/protocomm/include/security/protocomm_security.h +++ b/components/protocomm/include/security/protocomm_security.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -86,6 +86,11 @@ typedef struct protocomm_security { */ int ver; + /** + * Patch version number of security implementation + */ + uint8_t patch_ver; + /** * Function for initializing/allocating security * infrastructure diff --git a/components/protocomm/src/common/protocomm.c b/components/protocomm/src/common/protocomm.c index c8e0012db5..1a54826790 100644 --- a/components/protocomm/src/common/protocomm.c +++ b/components/protocomm/src/common/protocomm.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -426,3 +426,15 @@ esp_err_t protocomm_unset_version(protocomm_t *pc, const char *ep_name) return protocomm_remove_endpoint(pc, ep_name); } + +esp_err_t protocomm_get_sec_version(protocomm_t *pc, int *sec_ver, uint8_t *sec_patch_ver) +{ + if (pc == NULL || sec_ver == NULL || sec_patch_ver == NULL) { + return ESP_ERR_INVALID_ARG; + } + + *sec_ver = pc->sec->ver; + *sec_patch_ver = pc->sec->patch_ver; + + return ESP_OK; +} diff --git a/components/protocomm/src/security/security2.c b/components/protocomm/src/security/security2.c index 113fe361cc..326bb65d7f 100644 --- a/components/protocomm/src/security/security2.c +++ b/components/protocomm/src/security/security2.c @@ -14,7 +14,8 @@ #include #include -#include +#include +#include #include #include @@ -24,6 +25,7 @@ #include "constants.pb-c.h" #include "esp_srp.h" +#include "endian.h" static const char *TAG = "security2"; @@ -33,13 +35,21 @@ ESP_EVENT_DEFINE_BASE(PROTOCOMM_SECURITY_SESSION_EVENT); #define PUBLIC_KEY_LEN (384) #define CLIENT_PROOF_LEN (64) #define AES_GCM_KEY_LEN (256) -#define AES_GCM_IV_SIZE (16) +#define AES_GCM_IV_SIZE (12) #define AES_GCM_TAG_LEN (16) +#define SESSION_ID_LEN (8) #define SESSION_STATE_CMD0 0 /* Session is not setup: Initial State*/ #define SESSION_STATE_CMD1 1 /* Session is not setup: Cmd0 done */ #define SESSION_STATE_DONE 2 /* Session setup successful */ +typedef struct aes_gcm_iv { + uint8_t session_id[SESSION_ID_LEN]; + uint32_t counter; +} aes_gcm_iv_t; + +static_assert(sizeof(aes_gcm_iv_t) == AES_GCM_IV_SIZE, "Invalid size of AES GCM IV"); + typedef struct session { /* Session data */ uint32_t id; @@ -65,6 +75,12 @@ static void hexdump(const char *msg, char *buf, int len) ESP_LOG_BUFFER_HEX_LEVEL(TAG, buf, len, ESP_LOG_DEBUG); } +static inline void sec2_gcm_iv_counter_increment(uint8_t *iv_buf) +{ + aes_gcm_iv_t *iv = (aes_gcm_iv_t *) iv_buf; + iv->counter = htobe32(be32toh(iv->counter) + 1); +} + static esp_err_t sec2_new_session(protocomm_security_handle_t handle, uint32_t session_id); static esp_err_t handle_session_command0(session_t *cur_session, @@ -226,15 +242,35 @@ static esp_err_t handle_session_command1(session_t *cur_session, } hexdump("Device proof", device_proof, CLIENT_PROOF_LEN); - /* Initialize crypto context */ - mbedtls_gcm_init(&cur_session->ctx_gcm); + mbedtls_entropy_context entropy; + mbedtls_ctr_drbg_context ctr_drbg; - /* Considering the protocomm component is only used after RF ( Wifi/Bluetooth ) is enabled. - * Hence, we can be sure that the RNG generates true random numbers */ - esp_fill_random(&cur_session->iv, AES_GCM_IV_SIZE); + mbedtls_entropy_init(&entropy); + mbedtls_ctr_drbg_init(&ctr_drbg); + + int ret; + ret = mbedtls_ctr_drbg_seed(&ctr_drbg, mbedtls_entropy_func, &entropy, NULL, 0); + if (ret != 0) { + ESP_LOGE(TAG, "Failed to seed random number generator"); + free(device_proof); + return ESP_FAIL; + } + + aes_gcm_iv_t *iv = (aes_gcm_iv_t *) cur_session->iv; + ret = mbedtls_ctr_drbg_random(&ctr_drbg, iv->session_id, SESSION_ID_LEN); + if (ret != 0) { + ESP_LOGE(TAG, "Failed to generate random number"); + free(device_proof); + return ESP_FAIL; + } + /* Initialize counter value to 1 */ + iv->counter = htobe32(0x1); hexdump("Initialization vector", (char *)cur_session->iv, AES_GCM_IV_SIZE); + /* Initialize crypto context */ + mbedtls_gcm_init(&cur_session->ctx_gcm); + mbed_err = mbedtls_gcm_setkey(&cur_session->ctx_gcm, MBEDTLS_CIPHER_ID_AES, (unsigned char *)cur_session->session_key, AES_GCM_KEY_LEN); if (mbed_err != 0) { ESP_LOGE(TAG, "Failure at mbedtls_gcm_setkey_enc with error code : -0x%x", -mbed_err); @@ -431,6 +467,13 @@ static esp_err_t sec2_encrypt(protocomm_security_handle_t handle, return ESP_ERR_INVALID_STATE; } + aes_gcm_iv_t *iv = (aes_gcm_iv_t *) cur_session->iv; + if (be32toh(iv->counter) == 0) { + ESP_LOGE(TAG, "Invalid counter value, restart session"); + return ESP_ERR_INVALID_STATE; + } + hexdump("Encrypt IV", (char *)cur_session->iv, AES_GCM_IV_SIZE); + *outlen = inlen + AES_GCM_TAG_LEN; *outbuf = (uint8_t *) malloc(*outlen); if (!*outbuf) { @@ -448,6 +491,9 @@ static esp_err_t sec2_encrypt(protocomm_security_handle_t handle, } memcpy(*outbuf + inlen, gcm_tag, AES_GCM_TAG_LEN); + /* Increment counter value for next operation */ + sec2_gcm_iv_counter_increment(cur_session->iv); + return ESP_OK; } @@ -471,6 +517,13 @@ static esp_err_t sec2_decrypt(protocomm_security_handle_t handle, return ESP_ERR_INVALID_STATE; } + aes_gcm_iv_t *iv = (aes_gcm_iv_t *) cur_session->iv; + if (be32toh(iv->counter) == 0) { + ESP_LOGE(TAG, "Invalid counter value, restart session"); + return ESP_ERR_INVALID_STATE; + } + hexdump("Decrypt IV", (char *)cur_session->iv, AES_GCM_IV_SIZE); + *outlen = inlen - AES_GCM_TAG_LEN; *outbuf = (uint8_t *) malloc(*outlen); if (!*outbuf) { @@ -484,6 +537,10 @@ static esp_err_t sec2_decrypt(protocomm_security_handle_t handle, ESP_LOGE(TAG, "Failed at mbedtls_gcm_auth_decrypt : %d", ret); return ESP_FAIL; } + + /* Increment counter value for next operation */ + sec2_gcm_iv_counter_increment(cur_session->iv); + return ESP_OK; } @@ -545,6 +602,7 @@ static esp_err_t sec2_req_handler(protocomm_security_handle_t handle, const protocomm_security_t protocomm_security2 = { .ver = 2, + .patch_ver = 1, .init = sec2_init, .cleanup = sec2_cleanup, .new_transport_session = sec2_new_session, diff --git a/components/wifi_provisioning/src/manager.c b/components/wifi_provisioning/src/manager.c index c3b0538306..559832ee9e 100644 --- a/components/wifi_provisioning/src/manager.c +++ b/components/wifi_provisioning/src/manager.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -156,7 +156,7 @@ static struct wifi_prov_mgr_ctx *prov_ctx; /* This executes registered app_event_callback for a particular event * - * NOTE : By the time this fucntion returns, it is possible that + * NOTE : By the time this function returns, it is possible that * the manager got de-initialized due to a call to wifi_prov_mgr_deinit() * either inside the event callbacks or from another thread. Therefore * post execution of execute_event_cb(), the validity of prov_ctx must @@ -257,7 +257,15 @@ static cJSON* wifi_prov_get_info_json(void) /* Version field */ cJSON_AddStringToObject(prov_info_json, "ver", prov_ctx->mgr_info.version); + /* Security field */ + int sec_ver = 0; + uint8_t sec_patch_ver = 0; + + protocomm_get_sec_version(prov_ctx->pc, &sec_ver, &sec_patch_ver); + assert(sec_ver == prov_ctx->security); cJSON_AddNumberToObject(prov_info_json, "sec_ver", prov_ctx->security); + cJSON_AddNumberToObject(prov_info_json, "sec_patch_ver", sec_patch_ver); + /* Capabilities field */ cJSON_AddItemToObject(prov_info_json, "cap", prov_capabilities); @@ -304,19 +312,6 @@ static esp_err_t wifi_prov_mgr_start_service(const char *service_name, const cha return ret; } - /* Set version information / capabilities of provisioning service and application */ - cJSON *version_json = wifi_prov_get_info_json(); - char *version_str = cJSON_Print(version_json); - ret = protocomm_set_version(prov_ctx->pc, "proto-ver", version_str); - free(version_str); - cJSON_Delete(version_json); - if (ret != ESP_OK) { - ESP_LOGE(TAG, "Failed to set version endpoint"); - scheme->prov_stop(prov_ctx->pc); - protocomm_delete(prov_ctx->pc); - return ret; - } - /* Set protocomm security type for endpoint */ if (prov_ctx->security == 0) { #ifdef CONFIG_ESP_PROTOCOMM_SUPPORT_SECURITY_VERSION_0 @@ -353,6 +348,19 @@ static esp_err_t wifi_prov_mgr_start_service(const char *service_name, const cha return ret; } + /* Set version information / capabilities of provisioning service and application */ + cJSON *version_json = wifi_prov_get_info_json(); + char *version_str = cJSON_Print(version_json); + ret = protocomm_set_version(prov_ctx->pc, "proto-ver", version_str); + free(version_str); + cJSON_Delete(version_json); + if (ret != ESP_OK) { + ESP_LOGE(TAG, "Failed to set version endpoint"); + scheme->prov_stop(prov_ctx->pc); + protocomm_delete(prov_ctx->pc); + return ret; + } + prov_ctx->wifi_prov_handlers = malloc(sizeof(wifi_prov_config_handlers_t)); ret = get_wifi_prov_handlers(prov_ctx->wifi_prov_handlers); if (ret != ESP_OK) { @@ -1609,7 +1617,6 @@ esp_err_t wifi_prov_mgr_start_provisioning(wifi_prov_security_t security, const #endif prov_ctx->security = security; - esp_timer_create_args_t wifi_connect_timer_conf = { .callback = wifi_connect_timer_cb, .arg = NULL, diff --git a/tools/esp_prov/esp_prov.py b/tools/esp_prov/esp_prov.py index 5b1600f0a0..0d869842d5 100644 --- a/tools/esp_prov/esp_prov.py +++ b/tools/esp_prov/esp_prov.py @@ -1,9 +1,8 @@ #!/usr/bin/env python # -# SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 # - import argparse import asyncio import json @@ -38,9 +37,9 @@ def on_except(err): print(err) -def get_security(secver, username, password, pop='', verbose=False): +def get_security(secver, sec_patch_ver, username, password, pop='', verbose=False): if secver == 2: - return security.Security2(username, password, verbose) + return security.Security2(sec_patch_ver, username, password, verbose) elif secver == 1: return security.Security1(pop, verbose) elif secver == 0: @@ -148,6 +147,27 @@ async def get_version(tp): return response +async def get_sec_patch_ver(tp, verbose=False): + response = await get_version(tp) + + if verbose: + print('proto-ver response : ', response) + + try: + # Interpret this as JSON structure containing + # information with security version information + info = json.loads(response) + try: + sec_patch_ver = info['prov']['sec_patch_ver'] + except KeyError: + sec_patch_ver = 0 + return sec_patch_ver + + except ValueError: + # If decoding as JSON fails, we assume default patch level + return 0 + + async def establish_session(tp, sec): try: response = None @@ -415,6 +435,7 @@ async def main(): raise RuntimeError('Failed to establish connection') try: + sec_patch_ver = 0 # If security version not specified check in capabilities if args.secver is None: # First check if capabilities are supported or not @@ -436,13 +457,14 @@ async def main(): args.sec1_pop = '' if (args.secver == 2): + sec_patch_ver = await get_sec_patch_ver(obj_transport) if len(args.sec2_usr) == 0: args.sec2_usr = input('Security Scheme 2 - SRP6a Username required: ') if len(args.sec2_pwd) == 0: prompt_str = 'Security Scheme 2 - SRP6a Password required: ' args.sec2_pwd = getpass(prompt_str) - obj_security = get_security(args.secver, args.sec2_usr, args.sec2_pwd, args.sec1_pop, args.verbose) + obj_security = get_security(args.secver, sec_patch_ver, args.sec2_usr, args.sec2_pwd, args.sec1_pop, args.verbose) if obj_security is None: raise ValueError('Invalid Security Version') @@ -459,7 +481,7 @@ async def main(): print('==== Session Established ====') if args.reset: - print('==== Reseting WiFi====') + print('==== Resetting WiFi====') await reset_wifi(obj_transport, obj_security) sys.exit() diff --git a/tools/esp_prov/security/security2.py b/tools/esp_prov/security/security2.py index fc428c68e4..9d427f1d44 100644 --- a/tools/esp_prov/security/security2.py +++ b/tools/esp_prov/security/security2.py @@ -1,18 +1,19 @@ -# SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 - # APIs for interpreting and creating protobuf packets for # protocomm endpoint with security type protocomm_security2 - - -from typing import Any, Type +import struct +from typing import Any +from typing import Type import proto from cryptography.hazmat.primitives.ciphers.aead import AESGCM -from utils import long_to_bytes, str_to_bytes +from utils import long_to_bytes +from utils import str_to_bytes from .security import Security -from .srp6a import Srp6a, generate_salt_and_verifier +from .srp6a import generate_salt_and_verifier +from .srp6a import Srp6a AES_KEY_LEN = 256 // 8 @@ -30,17 +31,18 @@ def sec2_gen_salt_verifier(username: str, password: str, salt_len: int) -> Any: salt_str = ', '.join([format(b, '#04x') for b in salt]) salt_c_arr = '\n '.join(salt_str[i: i + 96] for i in range(0, len(salt_str), 96)) - print(f'static const char sec2_salt[] = {{\n {salt_c_arr}\n}};\n') + print(f'static const char sec2_salt[] = {{\n {salt_c_arr}\n}};\n') # noqa E702 verifier_str = ', '.join([format(b, '#04x') for b in verifier]) verifier_c_arr = '\n '.join(verifier_str[i: i + 96] for i in range(0, len(verifier_str), 96)) - print(f'static const char sec2_verifier[] = {{\n {verifier_c_arr}\n}};\n') + print(f'static const char sec2_verifier[] = {{\n {verifier_c_arr}\n}};\n') # noqa E702 class Security2(Security): - def __init__(self, username: str, password: str, verbose: bool) -> None: + def __init__(self, sec_patch_ver:int, username: str, password: str, verbose: bool) -> None: # Initialize state of the security2 FSM self.session_state = security_state.REQUEST1 + self.sec_patch_ver = sec_patch_ver self.username = username self.password = password self.verbose = verbose @@ -49,7 +51,7 @@ class Security2(Security): self.cipher: Type[AESGCM] self.client_pop_key = None - self.nonce = None + self.nonce = bytearray() Security.__init__(self, self.security2_session) @@ -75,7 +77,7 @@ class Security2(Security): def _print_verbose(self, data: str) -> None: if (self.verbose): - print(f'\x1b[32;20m++++ {data} ++++\x1b[0m') + print(f'\x1b[32;20m++++ {data} ++++\x1b[0m') # noqa E702 def setup0_request(self) -> Any: # Form SessionCmd0 request packet using client public key @@ -148,7 +150,7 @@ class Security2(Security): self._print_verbose(f'Session Key:\t0x{session_key.hex()}') # 96-bit nonce - self.nonce = setup_resp.sec2.sr1.device_nonce + self.nonce = bytearray(setup_resp.sec2.sr1.device_nonce) if self.nonce is None: raise RuntimeError('Received invalid nonce from device!') self._print_verbose(f'Nonce:\t0x{self.nonce.hex()}') @@ -158,8 +160,23 @@ class Security2(Security): if self.cipher is None: raise RuntimeError('Failed to initialize AES-GCM cryptographic engine!') + def _increment_nonce(self) -> None: + """Increment the last 4 bytes of nonce (big-endian counter).""" + if self.sec_patch_ver == 1: + counter = struct.unpack('>I', self.nonce[8:])[0] # Read last 4 bytes as big-endian integer + counter += 1 # Increment counter + if counter > 0xFFFFFFFF: # Check for overflow + raise RuntimeError('Nonce counter overflow') + self.nonce[8:] = struct.pack('>I', counter) # Store back as big-endian + def encrypt_data(self, data: bytes) -> Any: - return self.cipher.encrypt(self.nonce, data, None) + self._print_verbose(f'Nonce:\t0x{self.nonce.hex()}') + ciphertext = self.cipher.encrypt(self.nonce, data, None) + self._increment_nonce() + return ciphertext def decrypt_data(self, data: bytes) -> Any: - return self.cipher.decrypt(self.nonce, data, None) + self._print_verbose(f'Nonce:\t0x{self.nonce.hex()}') + plaintext = self.cipher.decrypt(self.nonce, data, None) + self._increment_nonce() + return plaintext