espressif / esp-rainmaker

ESP RainMaker Agent for firmware development
Apache License 2.0
431 stars 145 forks source link

Adding node without scanning QR code (MEGH-4160) #222

Closed jacek12345 closed 1 year ago

jacek12345 commented 1 year ago

I saw Nova Home app, that i think is built on the top of RainMaker? Suppose, there is possible to add device without scanning QR code nor "proof of possesion" PIN (provisioning is started by hardware on the node side, e.g. pushing switch a few times), right?. How can we implement this in our device? I think, this is very common and convenient method used by many (all?) IoT suppliers.

shahpiyushv commented 1 year ago

@jacek12345 , you can skip the Proof of Possession by passing POP_TYPE_NONE to app_wifi_start() in your application's app_main() function. However, we do not recommend it as it is not a good security practice. HomeKit and Matter also have the QR code/Proof of Possession as a mandatory requirement.

jacek12345 commented 1 year ago

Thank You @shahpiyushv Can you confirm that some degree of security is provided due to:

Or would you recommend any additional security features in POP_TYPE_NONE mode?

shahpiyushv commented 1 year ago

@jacek12345 , the above are sufficient for some level of security. CONFIG_APP_WIFI_PROV_TIMEOUT_PERIOD was added especially for POP_TYPE_NONE so that the window for open access gets reduced.

Meanwhile, devices generally boot up in provisioning mode and so, if the user has not provisioned the node, each reboot gives a small window of attack. Wi-Fi password sort of becomes immaterial here, because the attacker can provision to own account and network and be able to control the device. Do you plan to not start provisioning on boot up, but start it only on user action?

jacek12345 commented 1 year ago

Meanwhile, devices generally boot up in provisioning mode and so, if the user has not provisioned the node, each reboot gives a small window of attack.

You right. I'm starting re-provisioning after auser action on device, but didn't demand user action while it reboots (e.g. due to power cycle) while in unprovisioned state. It can be use case, when user power on device and leave it. It stop provisioning after CONFIG_APP_WIFI_PROV_TIMEOUT_PERIOD but after few hours due to power cycle (even maked by attacker) it starts provisioning again. I suppose that should check some flag after reboot, if it was due to user action. Should it be in RTC memory? How can i place variable in RTC memory? Or it is wrong way? If it is good way, what should I do after reboot if detect that it was not due to user action (flag is not set)? I suppose that should go to the same point as after CONFIG_APP_WIFI_PROV_TIMEOUT_PERIOD time's up?

jacek12345 commented 1 year ago

Is there any RAM that can retain reboot? Using ESP32-S3-WROOM-1. How can I locate variable in this memory? What should i do after reboot when need to not start provisioning?

shahpiyushv commented 1 year ago

You can find the reboot reason using esp_reset_reason(). You can also use nvs memory to store some variables, but that would cause flash writes. Another option is using RTC memory.

jacek12345 commented 1 year ago

Thank You @shahpiyushv esp_reset_reason() API seems to be ok for me. I added code to event_handler. Can You look if it is good way?

case APP_WIFI_EVENT_QR_DISPLAY:
      ESP_LOGI(TAG, "Provisioning QR : %s", (char *)event_data);

      //my code
      esp_reset_reason_t res_reason = esp_reset_reason();
      ESP_LOGW(TAG, "Reset cause: %d",res_reason);
      if(res_reason != ESP_RST_SW)
      {
          wifi_prov_mgr_stop_provisioning();
          esp_event_post(APP_WIFI_EVENT, APP_WIFI_EVENT_PROV_TIMEOUT, NULL, 0, portMAX_DELAY);
      }
      //end of my code
      break;

Since i planned to disable generating QR code, it would be better add this code to

if (event_base == WIFI_PROV_EVENT) {
        switch (event_id) {
            case WIFI_PROV_START:
                ESP_LOGI(TAG, "Provisioning started");
                break;

but unfortunatelly can't register event_handler for WIFI_PROV_EVENT. ESP_ERROR_CHECK(esp_event_handler_register(WIFI_PROV_EVENT, WIFI_PROV_START, &event_handler, NULL));

E (1331) wifi_prov_mgr: Failed to register handler for endpoint
E (1341) esp_rmaker_user_mapping: Failed to register user mapping end point.

Why i can't register handler for this event base?

EDIT: It looks like it is not problem with handler registration, but with wifi_prov_mgr_stop_provisioning(); that i can't call in this point? Handler registered properly. Even seems to work. Are this errors cause for concern or should ignore it? Summarising the code is:

} else if (event_base == WIFI_PROV_EVENT) {
        switch(event_id) {
            case WIFI_PROV_START:
                ESP_LOGI(TAG, "Provisioning started");
                esp_reset_reason_t res_reason = esp_reset_reason();
                ESP_LOGW(TAG, "Reset cause: %d",res_reason);
                if(res_reason != ESP_RST_SW)
                    wifi_prov_mgr_stop_provisioning();
                break;
            default:
                  ESP_LOGW(TAG, "Unhandled Wifi Prov Event: %"PRIi32, event_id);
                  break;
                }
}
jacek12345 commented 1 year ago

Is value after provisioning prefix PROV_xxxxxx mean something? Value after "__" changes so guess that it can't be used for device rocognition by user. Is it even necessary for app while POP_TYPE_NONE ? Or can use it whole for hardcoding device type that can be displayed by app while provisioning by BLE ? I saw that can change prefix to longer at the expense of shortening part after "_" sign. Basicaly can even use whole 11 signs for hardcoded prefix.

shahpiyushv commented 1 year ago

@jacek12345 , the value after PROV_ changes only when you erase the complete flash, which also erases the fctry partition. In actual products, since fctry will never be erased, this will be a fixed value which will typically be printed on the device and the box. However, if there is no POP, this suffix won't be much useful.

shahpiyushv commented 1 year ago

@jacek12345 , meanwhile if you are using idf v5.0, you can just call wifi_prov_mgr_is_provisioned() at any time independent of wifi_prov_mgr_init() and then if the node is not provisioned and your reset reason is ESP_RST_SW, skip the call to app_wifi_start() altogether. Would that work for you?

For the other issue, I believe there must have been some other unintended changes, because the internal handler registration is also failing in your case

E (1341) esp_rmaker_user_mapping: Failed to register user mapping end point.
jacek12345 commented 1 year ago

@shahpiyushv thank You for response. I'm using idf v4.4.3 now. Is idf v.5.0 fully compatible with RainMaker? Is it worth to update to v5.0 while in last development stage?

For the other issue, I believe there must have been some other unintended changes, because the internal handler registration is also failing in your case E (1341) esp_rmaker_user_mapping: Failed to register user mapping end point.

This error is gone, when commented wifi_prov_mgr_stop_provisioning(); while handler registration is in code still. So, I believe that this error is related with using wifi_prov_mgr_stop_provisioning(); when something is not initialised yet.

jacek12345 commented 1 year ago

Thank You @shahpiyushv for help