Merge branch 'fix/esp_srp_fix_coverity_issues' into 'master'

protocommm/esp_srp: Fix small issues reported by coverity scan.

Closes IDF-6040

See merge request espressif/esp-idf!20458

components/protocomm/src/crypto/srp6a/esp_srp.c

@@ -388,7 +388,7 @@ esp_err_t esp_srp_set_salt_verifier(esp_srp_handle_t *hd, const char *salt, int
     return ESP_FAIL;
 }
 
-esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, int *len_key)
+esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, uint16_t *len_key)
 {
     esp_mpi_t *u = NULL;
     esp_mpi_t *vu = NULL;
@@ -524,6 +524,7 @@ esp_err_t esp_srp_exchange_proofs(esp_srp_handle_t *hd, char *username, uint16_t
     ESP_LOG_BUFFER_HEX_LEVEL(TAG, (char *)digest, sizeof(digest), ESP_LOG_DEBUG);
 
     if (memcmp(bytes_user_proof, digest, SHA512_HASH_SZ) != 0) {
+        free(s);
         return ESP_FAIL;
     }
 

components/protocomm/src/crypto/srp6a/esp_srp_mpi.c

@@ -26,6 +26,7 @@ esp_mpi_t *esp_mpi_new_from_hex(const char *hex)
     int ret = mbedtls_mpi_read_string(a, 16, hex);
     if (ret != 0) {
         printf("mbedtls_mpi_read_string() failed, returned %x\n", ret);
+        esp_mpi_free(a);
         return NULL;
     }
     return a;
@@ -41,6 +42,7 @@ esp_mpi_t *esp_mpi_new_from_bin(const char *str, int str_len)
     int ret = mbedtls_mpi_read_binary(a, (unsigned char *)str, str_len);
     if (ret != 0) {
         printf("mbedtls_mpi_read_binary() failed, returned %x\n", ret);
+        esp_mpi_free(a);
         return NULL;
     }
     return a;
@@ -81,6 +83,7 @@ char *esp_mpi_to_bin(esp_mpi_t *bn, int *len)
     int ret = mbedtls_mpi_write_binary(bn, (unsigned char *)p, *len);
     if (ret != 0) {
         printf("mbedtls_mpi_read_string() failed, returned %x\n", ret);
+        free(p);
         return NULL;
     }
     return p;

components/protocomm/src/crypto/srp6a/include/esp_srp.h

@@ -88,7 +88,7 @@ esp_err_t esp_srp_srv_pubkey_from_salt_verifier(esp_srp_handle_t *hd, char **byt
 /* Returns bytes_key
  * *bytes_key MUST NOT BE FREED BY THE CALLER
  */
-esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, int *len_key);
+esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, uint16_t *len_key);
 
 /* Exchange proofs
  * Returns 1 if user's proof is ok. Also 1 when is returned, bytes_host_proof contains our proof.

components/protocomm/src/security/security2.c

@@ -48,8 +48,6 @@ typedef struct session {
     uint16_t salt_len;
     char *verifier;
     uint16_t verifier_len;
-    char *client_pubkey;
-    uint16_t client_pubkey_len;
     char *session_key;
     uint16_t session_key_len;
     uint8_t iv[AES_GCM_IV_SIZE];
@@ -90,23 +88,16 @@ static esp_err_t handle_session_command0(session_t *cur_session,
         return ESP_ERR_INVALID_ARG;
     }
 
-    cur_session->username_len = in->sc0->client_username.len;
-    cur_session->username = calloc(cur_session->username_len, sizeof(char));
-    if (!cur_session->username) {
-        ESP_LOGE(TAG, "Failed to allocate memory!");
-        return ESP_ERR_NO_MEM;
+    if (sv == NULL) {
+        ESP_LOGE(TAG, "Invalid security params");
+        return ESP_ERR_INVALID_ARG;
     }
-    memcpy(cur_session->username, in->sc0->client_username.data, in->sc0->client_username.len);
-    ESP_LOGD(TAG, "Username: %.*s", cur_session->username_len, cur_session->username);
 
-    cur_session->client_pubkey = calloc(PUBLIC_KEY_LEN, sizeof(char));
-    if (!cur_session->client_pubkey ) {
-        ESP_LOGE(TAG, "Failed to allocate memory!");
-        return ESP_ERR_NO_MEM;
-    }
-    memcpy(cur_session->client_pubkey, in->sc0->client_pubkey.data, PUBLIC_KEY_LEN);
-    cur_session->client_pubkey_len = PUBLIC_KEY_LEN;
-    hexdump("Client Public Key", cur_session->client_pubkey, PUBLIC_KEY_LEN);
+
+    ESP_LOGD(TAG, "Username: %.*s", in->sc0->client_username.len, in->sc0->client_username.data);
+
+
+    hexdump("Client Public Key", (char *) in->sc0->client_pubkey.data, PUBLIC_KEY_LEN);
 
     /* Initialize mu srp context */
     cur_session->srp_hd = calloc(1, sizeof(esp_srp_handle_t));
@@ -117,6 +108,7 @@ static esp_err_t handle_session_command0(session_t *cur_session,
 
     if (esp_srp_init(cur_session->srp_hd, ESP_NG_3072) != ESP_OK) {
         ESP_LOGE(TAG, "Failed to initialise security context!");
+        free(cur_session->srp_hd);
         return ESP_FAIL;
     }
 
@@ -129,31 +121,33 @@ static esp_err_t handle_session_command0(session_t *cur_session,
     cur_session->verifier_len = sv->verifier_len;
     ESP_LOGI(TAG, "Using salt and verifier to generate public key...");
 
-    if (sv != NULL && sv->salt != NULL && sv->salt_len != 0 && sv->verifier != NULL && sv->verifier_len != 0) {
+    if (sv->salt != NULL && sv->salt_len != 0 && sv->verifier != NULL && sv->verifier_len != 0) {
         if (esp_srp_set_salt_verifier(cur_session->srp_hd, cur_session->salt, cur_session->salt_len, cur_session->verifier, cur_session->verifier_len) != ESP_OK) {
             ESP_LOGE(TAG, "Failed to set salt and verifier!");
+            free(cur_session->srp_hd);
             return ESP_FAIL;
         }
         if (esp_srp_srv_pubkey_from_salt_verifier(cur_session->srp_hd, &device_pubkey, &device_pubkey_len) != ESP_OK) {
             ESP_LOGE(TAG, "Failed to device public key!");
+            free(cur_session->srp_hd);
             return ESP_FAIL;
         }
     }
 
     hexdump("Device Public Key", device_pubkey, device_pubkey_len);
-
-    if (esp_srp_get_session_key(cur_session->srp_hd, cur_session->client_pubkey, cur_session->client_pubkey_len,
-                                &cur_session->session_key, (int *)&cur_session->session_key_len) != ESP_OK) {
+    if (esp_srp_get_session_key(cur_session->srp_hd, (char *) in->sc0->client_pubkey.data, PUBLIC_KEY_LEN,
+                                &cur_session->session_key, &cur_session->session_key_len) != ESP_OK) {
         ESP_LOGE(TAG, "Failed to generate device session key!");
+        free(cur_session->srp_hd);
         return ESP_FAIL;
     }
     hexdump("Session Key", cur_session->session_key, cur_session->session_key_len);
 
-
     Sec2Payload *out = (Sec2Payload *) malloc(sizeof(Sec2Payload));
     S2SessionResp0 *out_resp = (S2SessionResp0 *) malloc(sizeof(S2SessionResp0));
     if (!out || !out_resp) {
         ESP_LOGE(TAG, "Error allocating memory for response0");
+        free(cur_session->srp_hd);
         free(out);
         free(out_resp);
         return ESP_ERR_NO_MEM;
@@ -178,6 +172,15 @@ static esp_err_t handle_session_command0(session_t *cur_session,
     resp->proto_case = SESSION_DATA__PROTO_SEC2;
     resp->sec2 = out;
 
+    cur_session->username_len = in->sc0->client_username.len;
+    cur_session->username = malloc(cur_session->username_len);
+    if (!cur_session->username) {
+        ESP_LOGE(TAG, "Failed to allocate memory!");
+        free(cur_session->srp_hd);
+        return ESP_ERR_NO_MEM;
+    }
+    memcpy(cur_session->username, in->sc0->client_username.data, in->sc0->client_username.len);
+
     cur_session->state = SESSION_STATE_CMD1;
 
     ESP_LOGD(TAG, "Session setup phase1 done");
@@ -209,6 +212,7 @@ static esp_err_t handle_session_command1(session_t *cur_session,
 
     if (esp_srp_exchange_proofs(cur_session->srp_hd, cur_session->username, cur_session->username_len, (char * ) in->sc1->client_proof.data, device_proof) != ESP_OK) {
         ESP_LOGE(TAG, "Failed to authenticate client proof!");
+        free(device_proof);
         return ESP_FAIL;
     }
     hexdump("Device proof", device_proof, CLIENT_PROOF_LEN);
@@ -225,6 +229,7 @@ static esp_err_t handle_session_command1(session_t *cur_session,
     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);
+        free(device_proof);
         mbedtls_gcm_free(&cur_session->ctx_gcm);
         return ESP_FAIL;
     }
@@ -233,6 +238,7 @@ static esp_err_t handle_session_command1(session_t *cur_session,
     S2SessionResp1 *out_resp = (S2SessionResp1 *) malloc(sizeof(S2SessionResp1));
     if (!out || !out_resp) {
         ESP_LOGE(TAG, "Error allocating memory for response1");
+        free(device_proof);
         free(out);
         free(out_resp);
         mbedtls_gcm_free(&cur_session->ctx_gcm);
@@ -341,7 +347,6 @@ static esp_err_t sec2_close_session(protocomm_security_handle_t handle, uint32_t
     }
 
     free(cur_session->username);
-    free(cur_session->client_pubkey);
 
     if (cur_session->srp_hd) {
         esp_srp_free(cur_session->srp_hd);

components/wifi_provisioning/src/manager.c

@@ -1485,7 +1485,7 @@ esp_err_t wifi_prov_mgr_start_provisioning(wifi_prov_security_t security, const
     /* Initialize app data */
     if (security == WIFI_PROV_SECURITY_0) {
         prov_ctx->mgr_info.capabilities.no_sec = true;
-    } else
+    }
 #endif
 #ifdef CONFIG_ESP_PROTOCOMM_SUPPORT_SECURITY_VERSION_1
     if (security == WIFI_PROV_SECURITY_1) {
@@ -1504,7 +1504,7 @@ esp_err_t wifi_prov_mgr_start_provisioning(wifi_prov_security_t security, const
         } else {
             prov_ctx->mgr_info.capabilities.no_pop = true;
         }
-    } else
+    }
 #endif
 #ifdef CONFIG_ESP_PROTOCOMM_SUPPORT_SECURITY_VERSION_2
     if (security == WIFI_PROV_SECURITY_2) {

examples/provisioning/wifi_prov_mgr/pytest_wifi_prov_mgr.py

@@ -25,11 +25,7 @@
 esp_prov.config_throw_except = True
 
 
-@pytest.mark.esp32
-@pytest.mark.generic
-@pytest.mark.xfail(reason='Runner unable to connect to target over Bluetooth', run=False)
-def test_examples_wifi_prov_mgr(dut: Dut) -> None:
-
+def test_wifi_prov_mgr(dut: Dut,  sec_ver: int) -> None:
     # Check if BT memory is released before provisioning starts
     dut.expect('wifi_prov_scheme_ble: BT memory released', timeout=60)
 
@@ -40,14 +36,22 @@ def test_examples_wifi_prov_mgr(dut: Dut) -> None:
     logging.info('Starting Provisioning')
     verbose = False
     protover = 'v1.1'
-    secver = 1
-    pop = 'abcd1234'
     provmode = 'ble'
     ap_ssid = 'myssid'
     ap_password = 'mypassword'
 
     logging.info('Getting security')
-    security = esp_prov.get_security(secver, pop, verbose)
+    if (sec_ver == 1):
+        pop = 'abcd1234'
+        sec2_username = None
+        sec2_password = None
+        security = esp_prov.get_security(sec_ver, sec2_username, sec2_password, pop, verbose)
+    elif (sec_ver == 2):
+        pop = None
+        sec2_username = 'wifiprov'
+        sec2_password = 'abcd1234'
+        security = esp_prov.get_security(sec_ver, sec2_username, sec2_password, pop, verbose)
+
     if security is None:
         raise RuntimeError('Failed to get security')
 
@@ -85,3 +89,20 @@ def test_examples_wifi_prov_mgr(dut: Dut) -> None:
 
     # Check if BTDM memory is released after provisioning finishes
     dut.expect('wifi_prov_scheme_ble: BTDM memory released', timeout=30)
+
+
+@pytest.mark.esp32
+@pytest.mark.generic
+@pytest.mark.parametrize('config', ['security1',], indirect=True)
+@pytest.mark.xfail(reason='Runner unable to connect to target over Bluetooth', run=False)
+def test_examples_wifi_prov_mgr_sec1(dut: Dut) -> None:
+
+    test_wifi_prov_mgr(dut, 1)
+
+
+@pytest.mark.esp32
+@pytest.mark.generic
+@pytest.mark.xfail(reason='Runner unable to connect to target over Bluetooth', run=False)
+def test_examples_wifi_prov_mgr_sec2(dut: Dut) -> None:
+
+    test_wifi_prov_mgr(dut, 2)

examples/provisioning/wifi_prov_mgr/sdkconfig.ci.security1

@@ -0,0 +1 @@
+CONFIG_EXAMPLE_PROV_SECURITY_VERSION_1=y