Merge branch 'bugfix/prov_examples_strlcpy_backport_v3.2' into 'release/v3.2'

Various bugfixes in Protocomm / Unified Provisioning (backport v3.2)

See merge request espressif/esp-idf!5372

components/protocomm/src/security/security1.c

@@ -198,7 +198,7 @@ static esp_err_t handle_session_command0(uint32_t session_id,
     mbedtls_ecdh_context     *ctx_server = malloc(sizeof(mbedtls_ecdh_context));
     mbedtls_entropy_context  *entropy    = malloc(sizeof(mbedtls_entropy_context));
     mbedtls_ctr_drbg_context *ctr_drbg   = malloc(sizeof(mbedtls_ctr_drbg_context));
-    if (!ctx_server || !ctx_server || !ctr_drbg) {
+    if (!ctx_server || !entropy || !ctr_drbg) {
         ESP_LOGE(TAG, "Failed to allocate memory for mbedtls context");
         free(ctx_server);
         free(entropy);

components/protocomm/src/transports/protocomm_ble.c

@@ -140,29 +140,42 @@ static void transport_simple_ble_read(esp_gatts_cb_event_t event, esp_gatt_if_t
 static esp_err_t prepare_write_event_env(esp_gatt_if_t gatts_if,
                                          esp_ble_gatts_cb_param_t *param)
 {
-    ESP_LOGD(TAG, "prepare write, handle = %d, value len = %d",
+    ESP_LOGD(TAG, "prepare write, handle = %d, value len = %d, offset = %d",
-             param->write.handle, param->write.len);
+             param->write.handle, param->write.len, param->write.offset);
     esp_gatt_status_t status = ESP_GATT_OK;
-    if (prepare_write_env.prepare_buf == NULL) {
+
-        prepare_write_env.prepare_buf = (uint8_t *) malloc(PREPARE_BUF_MAX_SIZE * sizeof(uint8_t));
+    /* Ensure that write data is not larger than max attribute length */
-        if (prepare_write_env.prepare_buf == NULL) {
-            ESP_LOGE(TAG, "%s , failed tp allocate preparebuf", __func__);
-            status = ESP_GATT_NO_RESOURCES;
-        }
-        /* prepare_write_env.prepare_len = 0; */
-    } else {
     if (param->write.offset > PREPARE_BUF_MAX_SIZE) {
         status = ESP_GATT_INVALID_OFFSET;
     } else if ((param->write.offset + param->write.len) > PREPARE_BUF_MAX_SIZE) {
         status = ESP_GATT_INVALID_ATTR_LEN;
+    } else {
+        /* If prepare buffer is not allocated, then allocate it */
+        if (prepare_write_env.prepare_buf == NULL) {
+            prepare_write_env.prepare_len = 0;
+            prepare_write_env.prepare_buf = (uint8_t *) malloc(PREPARE_BUF_MAX_SIZE * sizeof(uint8_t));
+            if (prepare_write_env.prepare_buf == NULL) {
+                ESP_LOGE(TAG, "%s , failed to allocate prepare buf", __func__);
+                status = ESP_GATT_NO_RESOURCES;
             }
         }
+    }
+
+    /* If prepare buffer is allocated copy incoming data into it */
+    if (status == ESP_GATT_OK) {
         memcpy(prepare_write_env.prepare_buf + param->write.offset,
                param->write.value,
                param->write.len);
         prepare_write_env.prepare_len += param->write.len;
         prepare_write_env.handle = param->write.handle;
+    }
+
+    /* Send write response if needed */
     if (param->write.need_rsp) {
+        esp_err_t response_err;
+        /* If data was successfully appended to prepare buffer
+         * only then have it reflected in the response */
+        if (status == ESP_GATT_OK) {
             esp_gatt_rsp_t gatt_rsp = {0};
             gatt_rsp.attr_value.len = param->write.len;
             gatt_rsp.attr_value.handle = param->write.handle;
@@ -171,9 +184,12 @@ static esp_err_t prepare_write_event_env(esp_gatt_if_t gatts_if,
             if (gatt_rsp.attr_value.len && param->write.value) {
                 memcpy(gatt_rsp.attr_value.value, param->write.value, param->write.len);
             }
-        esp_err_t response_err = esp_ble_gatts_send_response(gatts_if, param->write.conn_id,
+            response_err = esp_ble_gatts_send_response(gatts_if,
-                param->write.trans_id, status,
+                param->write.conn_id, param->write.trans_id, status, &gatt_rsp);
-                &gatt_rsp);
+        } else {
+            response_err = esp_ble_gatts_send_response(gatts_if,
+                param->write.conn_id, param->write.trans_id, status, NULL);
+        }
         if (response_err != ESP_OK) {
             ESP_LOGE(TAG, "Send response error in prep write");
         }
@@ -195,7 +211,7 @@ static void transport_simple_ble_write(esp_gatts_cb_event_t event, esp_gatt_if_t
     ssize_t outlen = 0;
     esp_err_t ret;
 
-    ESP_LOGD(TAG, "Inside write with session - %d on attr handle - %d \nLen -%d IS Prep - %d",
+    ESP_LOGD(TAG, "Inside write with session - %d on attr handle = %d \nlen = %d, is_prep = %d",
              param->write.conn_id, param->write.handle, param->write.len, param->write.is_prep);
 
     if (param->write.is_prep) {

components/protocomm/src/transports/protocomm_httpd.c

@@ -50,9 +50,7 @@ static esp_err_t common_post_handler(httpd_req_t *req)
             if (pc_httpd->sec && pc_httpd->sec->close_transport_session) {
                 ret = pc_httpd->sec->close_transport_session(session_id);
                 if (ret != ESP_OK) {
-                    ESP_LOGE(TAG, "Failed to close session with ID: %d", session_id);
+                    ESP_LOGW(TAG, "Error closing session with ID: %d", session_id);
-                    ret = ESP_FAIL;
-                    goto out;
                 }
             }
             session_id = PROTOCOMM_NO_SESSION_ID;
@@ -241,6 +239,7 @@ esp_err_t protocomm_httpd_start(protocomm_t *pc, const protocomm_httpd_config_t
     pc->add_endpoint    = protocomm_httpd_add_endpoint;
     pc->remove_endpoint = protocomm_httpd_remove_endpoint;
     pc_httpd = pc;
+    session_id = PROTOCOMM_NO_SESSION_ID;
     return ESP_OK;
 }
 
@@ -256,6 +255,7 @@ esp_err_t protocomm_httpd_stop(protocomm_t *pc)
         }
         pc_httpd->priv = NULL;
         pc_httpd = NULL;
+        session_id = PROTOCOMM_NO_SESSION_ID;
         return ESP_OK;
     }
     return ESP_ERR_INVALID_ARG;

components/wifi_provisioning/include/wifi_provisioning/wifi_config.h

@@ -76,7 +76,7 @@ typedef struct {
  */
 typedef struct {
     char    ssid[33];       /*!< SSID of the AP to which the slave is to be connected */
-    char    password[65];   /*!< Password of the AP */
+    char    password[64];   /*!< Password of the AP */
     char    bssid[6];       /*!< BSSID of the AP */
     uint8_t channel;        /*!< Channel of the AP */
 } wifi_prov_config_set_data_t;

components/wifi_provisioning/src/wifi_config.c

@@ -151,6 +151,24 @@ static esp_err_t cmd_set_config_handler(WiFiConfigPayload *req,
 
     wifi_prov_config_set_data_t req_data;
     memset(&req_data, 0, sizeof(req_data));
+
+    /* Check arguments provided in protobuf packet:
+     * - SSID / Passphrase string length must be within the standard limits
+     * - BSSID must either be NULL or have length equal to that imposed by the standard
+     * If any of these conditions are not satisfied, don't invoke the handler and
+     * send error status without closing connection */
+    resp_payload->status = STATUS__InvalidArgument;
+    if (req->cmd_set_config->bssid.len != 0 &&
+        req->cmd_set_config->bssid.len != sizeof(req_data.bssid)) {
+        ESP_LOGD(TAG, "Received invalid BSSID");
+    } else if (req->cmd_set_config->ssid.len >= sizeof(req_data.ssid)) {
+        ESP_LOGD(TAG, "Received invalid SSID");
+    } else if (req->cmd_set_config->passphrase.len >= sizeof(req_data.password)) {
+        ESP_LOGD(TAG, "Received invalid Passphrase");
+    } else {
+        /* The received SSID and Passphrase are not NULL terminated so
+         * we memcpy over zeroed out arrays. Above length checks ensure
+         * that there is atleast 1 extra byte for null termination */
         memcpy(req_data.ssid, req->cmd_set_config->ssid.data,
                req->cmd_set_config->ssid.len);
         memcpy(req_data.password, req->cmd_set_config->passphrase.data,
@@ -160,6 +178,9 @@ static esp_err_t cmd_set_config_handler(WiFiConfigPayload *req,
         req_data.channel = req->cmd_set_config->channel;
         if (h->set_config_handler(&req_data, &h->ctx) == ESP_OK) {
             resp_payload->status = STATUS__Success;
+        } else {
+            resp_payload->status = STATUS__InternalError;
+        }
     }
 
     resp->payload_case = WI_FI_CONFIG_PAYLOAD__PAYLOAD_RESP_SET_CONFIG;
@@ -188,7 +209,7 @@ static esp_err_t cmd_apply_config_handler(WiFiConfigPayload *req,
     if (h->apply_config_handler(&h->ctx) == ESP_OK) {
         resp_payload->status = STATUS__Success;
     } else {
-        resp_payload->status = STATUS__InvalidArgument;
+        resp_payload->status = STATUS__InternalError;
     }
 
     resp->payload_case = WI_FI_CONFIG_PAYLOAD__PAYLOAD_RESP_APPLY_CONFIG;

examples/provisioning/ble_prov/main/app_prov_handlers.c

@@ -98,10 +98,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
+
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }
 

examples/provisioning/console_prov/main/app_prov_handlers.c

@@ -98,10 +98,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
+
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }
 

examples/provisioning/custom_config/main/app_prov.c

@@ -327,13 +327,13 @@ static esp_err_t start_wifi_ap(const char *ssid, const char *pass)
     };
 
     strncpy((char *) wifi_config.ap.ssid, ssid, sizeof(wifi_config.ap.ssid));
-    wifi_config.ap.ssid_len = strlen(ssid);
+    wifi_config.ap.ssid_len = strnlen(ssid, sizeof(wifi_config.ap.ssid));
 
     if (strlen(pass) == 0) {
         memset(wifi_config.ap.password, 0, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_OPEN;
     } else {
-        strncpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
+        strlcpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_WPA_WPA2_PSK;
     }
 

examples/provisioning/custom_config/main/app_prov_handlers.c

@@ -110,10 +110,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
+
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }
 

examples/provisioning/softap_prov/main/app_prov.c

@@ -314,13 +314,13 @@ static esp_err_t start_wifi_ap(const char *ssid, const char *pass)
     };
 
     strncpy((char *) wifi_config.ap.ssid, ssid, sizeof(wifi_config.ap.ssid));
-    wifi_config.ap.ssid_len = strlen(ssid);
+    wifi_config.ap.ssid_len = strnlen(ssid, sizeof(wifi_config.ap.ssid));
 
     if (strlen(pass) == 0) {
         memset(wifi_config.ap.password, 0, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_OPEN;
     } else {
-        strncpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
+        strlcpy((char *) wifi_config.ap.password, pass, sizeof(wifi_config.ap.password));
         wifi_config.ap.authmode = WIFI_AUTH_WPA_WPA2_PSK;
     }
 

examples/provisioning/softap_prov/main/app_prov_handlers.c

@@ -98,10 +98,14 @@ static esp_err_t set_config_handler(const wifi_prov_config_set_data_t *req_data,
 
     ESP_LOGI(TAG, "WiFi Credentials Received : \n\tssid %s \n\tpassword %s",
              req_data->ssid, req_data->password);
-    memcpy((char *) wifi_cfg->sta.ssid, req_data->ssid,
+
-           strnlen(req_data->ssid, sizeof(wifi_cfg->sta.ssid)));
+    /* Using strncpy allows the max SSID length to be 32 bytes (as per 802.11 standard).
-    memcpy((char *) wifi_cfg->sta.password, req_data->password,
+     * But this doesn't guarantee that the saved SSID will be null terminated, because
-           strnlen(req_data->password, sizeof(wifi_cfg->sta.password)));
+     * wifi_cfg->sta.ssid is also 32 bytes long (without extra 1 byte for null character).
+     * Although, this is not a matter for concern because esp_wifi library reads the SSID
+     * upto 32 bytes in absence of null termination */
+    strncpy((char *) wifi_cfg->sta.ssid, req_data->ssid, sizeof(wifi_cfg->sta.ssid));
+    strlcpy((char *) wifi_cfg->sta.password, req_data->password, sizeof(wifi_cfg->sta.password));
     return ESP_OK;
 }