Different callbacks for esp now multicast messages for better security (IDFGH-6954)

[justjustjustus]

Problem

On reception of a esp now message, the attached receive callback is called and the message is delivered with sender mac, data pointer and data length.
The problem is that it is not possible to check whether the message comes from a peer that was previously added (with encryption) or from an (unencrypted) broadcasted message.

If added a peer with encryption before, it is not possible to check if the message came encrypted as peer-to-peer message or unencrypted over the broadcast channel.

IMPORTANT: By setting the mac address manually on a sending device, any sender could then send messages with the mac of another device without knowing the encryption keys as long as the recipent has the broadcast peer added before, bypassing the encryption.

Possible Solution

Add another callback for broadcasted messages to be able to differenciate if the message came from a previously added device or was broadcasted to everyone.

Alternative Solutions

1. Have a different callback for every registered peer and another callback for broadcasts.
2. Add another argument to the esp now callback as bool, wether the message was a broadcasted one or not.
3. Add another argument to signalize (bool) if message was encrypted or not
4. Have a different callback for encrypted messages and not encrypted messages

Additional context

In our Software, we use esp now heavily. Over broadcasted messages two devices get paired and then switch to an encrypted connection with our own implementation of some kind of TCP. With the broadcast peer constantly enabled, we can not differenciate if the message came from the paired device or from another intruder. With this, an attacker could inflitrate messages in the name of the preveiously paired device without to know the encryption keys that were exchanged during pairing.

[justjustjustus]

I found this almost identical issue: espressif/ESP8266_NONOS_SDK#311

[RutgerOlsbergs]

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

[zhangyanjiaoesp]

@justjustjustus After you switch to an encrypted connection with our own implementation of some kind of TCP, do you still want to recv broadcast packets? Or you just want to recv the broadcast packets from the peer ?

[RutgerOlsbergs]

We want to recieve no broadcast packages at all. Broadcasts are by definition not encrypted and we only want to receive encrypted messeges.

[justjustjustus]

@zhangyanjiaoesp Yes we still want this for pairing with additional devices. We always use direct connections in combination with broadcasting.

[zhangyanjiaoesp]

@justjustjustus We have modify the code to provide more info in the recv callback, but the merge will take some time. If you want to use it now, we can provide a patch to you, when the commit is merged, you can update your IDF then.
If you need the patch, please show me the commit ID you are using, thanks.

[zhangyanjiaoesp]

fix in commit 8223a59