espressif / esp-now

A connectionless Wi-Fi communication protocol
Apache License 2.0
486 stars 91 forks source link

Being able to send clear data (AEGHB-115) #63

Closed grodansparadis closed 1 year ago

grodansparadis commented 1 year ago

I notices that setting espnow_frame_head_t .security to false and sending espnow data still encrypts the data. This is due to

https://github.com/espressif/esp-now/blob/master/src/espnow/src/espnow.c#L505

Should't this be if (data_head->security && g_espnow_config->sec_enable && type != ESPNOW_DATA_TYPE_ACK && type != ESPNOW_DATA_TYPE_FORWARD && type != ESPNOW_DATA_TYPE_SECURITY_STATUS && type != ESPNOW_DATA_TYPE_SECURITY) {

lhespress commented 1 year ago

@grodansparadis Please replace the patch as the attachment: security.patch

grodansparadis commented 1 year ago

@lhespress Great. I am away today. Will test on Sunday.

grodansparadis commented 1 year ago

@lhespress Works only if I set security to false for

diff --git a/src/security/src/espnow_security_initiator.c b/src/security/src/espnow_security_initiator.c
index 71cdfbb..367e688 100644
--- a/src/security/src/espnow_security_initiator.c
+++ b/src/security/src/espnow_security_initiator.c
@@ -146,6 +146,7 @@ esp_err_t espnow_sec_initiator_scan(espnow_sec_responder_t **info_list, size_t *
     espnow_frame_head_t frame_head = {
         .retransmit_count = 10,
         .broadcast        = true,
+        .security         = false,
         .magic            = esp_random(),
         .filter_adjacent_channel = true,
         .forward_ttl      = CONFIG_ESPNOW_SEC_SEND_FORWARD_TTL,
@@ -248,6 +249,7 @@ static esp_err_t protocomm_espnow_initiator_start(const protocomm_security_t *pr
     uint8_t *outbuf = NULL;
     int32_t session_id = 0;
     espnow_frame_head_t frame_head = {
+        .security         = false,
         .retransmit_count = CONFIG_ESPNOW_SEC_SEND_RETRY_NUM,
         .filter_adjacent_channel = true,
         .forward_ttl      = CONFIG_ESPNOW_SEC_SEND_FORWARD_TTL,
diff --git a/src/security/src/espnow_security_responder.c b/src/security/src/espnow_security_responder.c
index 859c3a9..7057d27 100644
--- a/src/security/src/espnow_security_responder.c
+++ b/src/security/src/espnow_security_responder.c
@@ -34,17 +34,19 @@ static const char* TAG = "espnow_sec_resp";
 static uint8_t app_key[APP_KEY_LEN] = { 0 };
 static protocomm_t *g_espnow_pc = NULL;
 static espnow_sec_info_t g_sec_info = { 0 };
-static espnow_frame_head_t g_frame_config = { 0 };

 static esp_err_t espnow_sec_info(const uint8_t *src_addr)
 {
     esp_err_t ret = ESP_OK;
     size_t size = sizeof(espnow_sec_info_t);
     espnow_sec_info_t *info = &g_sec_info;
+    espnow_frame_head_t frame_head = {
+        .security         = false,
+    };

     info->type = ESPNOW_SEC_TYPE_INFO;

-    ret = espnow_send(ESPNOW_DATA_TYPE_SECURITY_STATUS, src_addr, info, size, &g_frame_config, portMAX_DELAY);
+    ret = espnow_send(ESPNOW_DATA_TYPE_SECURITY_STATUS, src_addr, info, size, &frame_head, portMAX_DELAY);

     ESP_ERROR_RETURN(ret != ESP_OK, ret, "espnow_write");

@@ -76,6 +78,7 @@ static esp_err_t espnow_sec_handle(const char *ep_name, uint8_t resp_type, const
     espnow_frame_head_t frame_head = {
         .retransmit_count = 1,
         .broadcast        = false,
+        .security         = false,
         .filter_adjacent_channel = true,
         .forward_ttl      = 0,
     };
grodansparadis commented 1 year ago

Also espnow_log_send_task is now broken.

lhespress commented 1 year ago

@grodansparadis You're right, i overlooked the security handshake must work on clear data. Sorry for your job.

    if (g_espnow_config->sec_enable && data_head->security
        && type != ESPNOW_DATA_TYPE_ACK && type != ESPNOW_DATA_TYPE_FORWARD
        && type != ESPNOW_DATA_TYPE_SECURITY_STATUS && type != ESPNOW_DATA_TYPE_SECURITY) {
grodansparadis commented 1 year ago

@lhespress No problem att all

grodansparadis commented 1 year ago

Just for your information I ended up with this

if (g_espnow_config->sec_enable && ((NULL == data_head) ? g_espnow_frame_head_default.security : data_head->security) &&

in the end as data_head was passed as NULL from logging.