search

espressif / esp-mqtt

ESP32 mqtt component
Apache License 2.0
603 stars 255 forks source link

CN name check (IDFGH-3408) #158

Closed coaxrobotics closed 4 years ago

coaxrobotics commented 4 years ago

Currently the library doesn't seem to allow to bypass server common name (CN) check. It would be convenient if there was a method to either bypass this check or override the CN that mbedTLS matches with the server certificate. This will be useful in connecting to broker on local network where the IP is usually dynamic and the broker certificate's CN field cannot reflect this.

ESP-Marius commented 4 years ago

Hi @coaxrobotics, thanks for the suggestion!

I agree that being able to bypass this can be usual, as long as users are aware of the security implications! There is a MR in our pipeline now to add this as a config option.

AxelLin commented 3 years ago

define MQTT_SUPPORTED_FEATURE_SKIP_CRT_CMN_NAME_CHECK seems in wrong #if ESP_IDF_VERSION guard.

Currently MQTT_SUPPORTED_FEATURE_SKIP_CRT_CMN_NAME_CHECK is only defined when ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 1, 0). But current code put it in ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(3, 3, 0).

I'm not sure if the target is adding the feature to 3.3.0 and 4.0.0 or just a mistake.

ESP-Marius commented 3 years ago

Hmm, not sure what you mean here. I just tested it and MQTT_SUPPORTED_FEATURE_SKIP_CRT_CMN_NAME_CHECK seems to get defined both in release/v4.0 and release/v3.3

It should be available in both those versions.

AxelLin commented 3 years ago

I checked out esp-idf release/v4.0 branch, but I don't find MQTT_SUPPORTED_FEATURE_SKIP_CRT_CMN_NAME_CHECK. In esp-idf release/v4.0, the esp-mqtt latest commit is 6bc94add892437d0fd50f26bfabe78c646648c13.

ESP-Marius commented 3 years ago

Ah you are right! I was looking at our internal repo. Both 3.3 and 4.0 should work when stuff gets synced to Github, sorry for the delay here!