search

espressif / esp-azure

SDK to connect ESP8266 and ESP32 to Microsoft Azure IoT services
177 stars 93 forks source link

TLS Server certificate Verification not working #7

Closed sHorst closed 5 years ago

sHorst commented 6 years ago

The TLS Server certificate Verification is not working. Since the tlsio layer does not support this at all. This is a big security issue, since you can have any man in the middle attack.

This also results in the DPS examles not working, since they need client Certificate authentication to work, which is also not supported.

Is there a rodemap, to when the custom tlsio layer will support this? Or is there a workaround (like use a different tlsio layer, which works with the esp?)

markrad commented 6 years ago

Perhaps if two of us mention this it might get some traction. I noticed this too and I am equally concerned about the security hole.

Patrik-Berglund commented 6 years ago

@JetstreamRoySprowl @ustccw

You two have been the main contributors to the ESP32 PAL. Would be great if you could comment ASAP around this issue since we are planning to use the ESP32 in a production environment in the near future.

JetstreamRoySprowl commented 6 years ago

Hi Patrick, I'm afraid I'm not on this project anymore; sorry I can't offer anything.

ustccw commented 6 years ago

@sHorst @markrad this repo ports azure to esp platform, about TLS secure issues, maybe point out a issue at here will be more effective.

sHorst commented 6 years ago

it is not an azure problem. there is a custom tls layer, which is esp custom. (components/azure_iot/pal/src/tlsio_openssl_compact.c)

ustccw commented 6 years ago

@sHorst would this issue reproduce on linux platform?

sHorst commented 6 years ago

if you compile the custom esp layer to an linux platform maybe .. but it is not part of the azure-iot-sdk-c code. it is custom to this repository.

ustccw commented 6 years ago

@dcristoloveanu @jebrando Hi, does azure provide a TLS Server certificate Verification adapter layer for us?

FHFS commented 6 years ago

change line https://github.com/espressif/esp-azure/blob/master/components/azure_iot/pal/src/tlsio_openssl_compact.c#L285 from TLSIO_OPTION_BIT_NONE to TLSIO_OPTION_BIT_TRUSTED_CERTS. This will enable you to call IoTHubClient_LL_SetOption(_iotHubClientHandle, OPTION_TRUSTED_CERT, certificates);

sHorst commented 6 years ago

this will still break the verification, you will get no error, but no certification verification as well..

I looked into this a bit more, and it looks like the create_ssl function does not properly create the ssl context, as it should. I created a patch, which should work, but needs some tweeking .. will come back to this thread..

Patrik-Berglund commented 5 years ago

Ok, so this doesn't affect the security when using SAS tokens? The TLS connection is still secure?

sHorst commented 5 years ago

a attacker with an elevated position in the network (e.g. a starbugs/ free network/ rough ISP) could redirect your traffic to his evil server, and the Azure sdk would not know, that it does not talk to the azure iot hub/dps. Therefore any communication and Tokens etc. could be compromised. The TLS connection is not secure, since the Server Certificates are not testet at all.

Patrik-Berglund commented 5 years ago

@sHorst Seems to be the same issue in the Apple iOS PAL?

https://github.com/Azure/azure-c-shared-utility/blob/master/pal/ios-osx/tlsio_appleios.c

markrad commented 5 years ago

@Patrik-Berglund I don't see why you think that. The issue is that the device does not check the certificate from the server against a trusted root. The stream would be encrypted but you would not know if you were actually connected to an Azure IoT hub. The iOS code you referenced uses Apple's own SSL layer and, as far as I can see, the certificate will be validated. I would be interested to know why you think it has the same issue since I wrote most of that code.

Patrik-Berglund commented 5 years ago

Hi @markrad

I think a more correct phrasing from my side should have been "Do you think the same issue could exist in the Apple iOS PAL?"

Comment was based on the similarities of the two implementations, although using another type of transport. If that had been the case a fix for the ESP-IDF might have possibly progressed faster...

@ustccw It's now two months since this vulnerability surfaced, what's Espressifs standpoint, will you assign resources to have this fixed?

mahavirj commented 5 years ago

@Patrik-Berglund @sHorst (Update) We are looking into this (both issue and PR), hopefully we will try to resolve in next few days.

mahavirj commented 5 years ago

This is now addressed through commit https://github.com/espressif/esp-azure/commit/6e7b860d4fff35229c3d03cdc5acf6169456aa7e. Additional provisioning client based example is also added. Now in both examples server certificate verification is made mandatory.

Going ahead planning to add some more documentation to examples, please feel free to take a look and provide feedback if any.