Security Bug in esp now without encryption check (IDFGH-6957)

[justjustjustus]

Environment

• Module or chip used: ESP32-WROVER
• IDF version: v4.3.1

Problem Description

Below description is for esp now communication.
Enable esp now encryption on one device A, but do not enable encryption on device B.
If sending an unencrypted message from device B to device A, it is still accepted and handled as it is encrypted, so it is delivered to the application, without notice about that the message was not encrypted.

Additionally, the device A does always receive an ack from device B, even if the message can't be decrypted and given to the application on device B, which may false-flag a sent message as received.

Expected Behavior

If an unencrypted message was sent from device B to device A, i would assume it is dropped as device A should only accept and decrypt encrypted messages, as it is added to the peer list with peer.encrypt = true.

Actual Behavior

The unencrypted message from device B to device A gets delivered to the application like an encrypted message.

Steps to reproduce

1. Init WiFi & ESP NOW on device A and device B
2. Add an encrypted peer to device A with mac of device B
3. Add an unencrypted peer to device B with mac of device A
4. Send (unencrypted) esp now message from device B to device A
5. See the delivery at device B

Code to reproduce this issue

Code GIST

Debug Logs

Device A

I (1613249) MAIN: Send Message: 12:34:56:78:90:02 - 8 bytes:    5b6a9d9acb717b59
I (1613250) MAIN: Send to 12:34:56:78:90:02 done: success
I (1613517) MAIN: Received Message: 12:34:56:78:90:02 - 11 bytes:       a3413444ef4092c03ef44a

Device B

I (1613222) MAIN: Send Message: 12:34:56:78:90:01 - 11 bytes:   a3413444ef4092c03ef44a
I (1613223) MAIN: Send to 12:34:56:78:90:01 done: success

References

This is related to the feature request #8574 and might have already a possible solution.

[justjustjustus]

Reference: espressif/ESP8266_NONOS_SDK#311

[jack0c]

Yes, it looks like an issue. We need to research how to fix this issue. @zhangyanjiaoesp PTAL

[zhangyanjiaoesp]

@justjustjustus
Thanks for reporting this bug, we will solve the encrypted device can receive unencrypted packets issue as soon as possible.
As for this:

Additionally, the device A does always receive an ack from device B, even if the message can't be decrypted and given to the application on device B, which may false-flag a sent message as received.

The ack from device B is MAC ack, you can check whether device B receive the packets in the API layer.

[justjustjustus]

Thank you really much.

The ack from device B is MAC ack, you can check whether device B receive the packets in the API layer.

I already thought this is the case, but still wanted to mention it. Thanks for clarifying.

@zhangyanjiaoesp Please also have a look at #8574 too, as this might still enable this security issue to be exploited as soon as a broadcast peer is added to the peer list.

[negativekelvin]

I have seen this issue mentioned multiple times in different places in the past so good it is getting fixed now

[zhangyanjiaoesp]

OK， I will look into #8574 also.

[RutgerOlsbergs]

Has anything happened with this issue? Just noticed the same behaviour today and it's a major security concern.

[justjustjustus]

@zhangyanjiaoesp Are there any updates regarding the fix for this issue?

[zhangyanjiaoesp]

Please use 63af85f branch to test.

[justjustjustus]

@zhangyanjiaoesp thank you. I looked at the docs/esp_now.h, but could't find a change, only the precompiled library has changed. Is there now a different callback for encrypted/unencrypted or broadcasted/direct messages or an additional parameter to check for encryption? Or are broadcasted and encrypted direct message treated equally still (like described in #8574)?

[zhangyanjiaoesp]

@justjustjustus We use additional parameter to check the encrypted packets in the wifi lib.

[justjustjustus]

@zhangyanjiaoesp yes, but the general security issue still persists. As soon as you add a broadcast peer, all messages can be received by the application layer. If i have two notes with same software-defined mac, one with encryption and the other one without but as broadcast, both notes would be able to send messages to a third one, without the third one knowing which of the both sent the message and if it was encrypted or broadcasted. I described this in #8574. An Attacker could just send broadcasted messages with same mac as the attacked device and so, circumventing the whole encryption, making it useless.

Without a change of the esp_now.h and its functions (e.g. new function for broadcast or encrypted-flag in callback function) this is not fixable.

[negativekelvin]

Yes, esp-now API is too simple, too much information is disappeared in wifi lib. There should be new api with all frame information passed to callback including destination address, flags, rssi, errors -- esp_now_register_verbose_recv_cb. Send call back should give number of retries before packet is acked.

I want application layer to decide whether to drop unencrypted packets or broadcast packets from encrypted peer, not wifi lib.

[zhangyanjiaoesp]

@negativekelvin @justjustjustus OK, we will try to pass more information to the callback, maybe a new API