espressif / esp-now

A connectionless Wi-Fi communication protocol
Apache License 2.0
528 stars 93 forks source link

Please clarify security capabilities of esp-now (AEGHB-38) #40

Closed chenlijun99 closed 1 year ago

chenlijun99 commented 2 years ago

Hi, from what I understand esp-now does not support encrypted multicast/broadcast communication. In fact, from the documentation of ESP-IDF, I read:

Encrypting multicast vendor-specific action frame is not supported.

However, this repository seems to be a series of application level components that are developed on-top of the low-level esp-now primitives provided in ESP-IDF and, in particular, it seems to me that the security component implements its own application-level security, that apparently doesn't make use of the PMK/LMK mechanism of the low-level esp-now. It seems also that this application level security supports broadcast/multicast encrypted communication. Could you please confirm this?

LJYSP commented 2 years ago

@chenlijun99 Yes,you are right. The security component in ESP-NOW project is used to protect application data since the project mainly uses broadcast to transmit and esp-now does not support encrypted multicast/broadcast communication.

chenlijun99 commented 2 years ago

Many thanks for the quick response!

chenlijun99 commented 2 years ago

Hi! Testing the system and reading the source code I noticed that the initialization vector is extracted from the exchanged APP KEY and then never modified.

https://github.com/espressif/esp-now/blob/43fb37dc953ad2649732a17f6b8ac11dde40fecc/components/security/src/espnow_security.c#L68

This means that as for now there no protection against replay attacks. Is that correct?

LJYSP commented 2 years ago

Yes,that‘s right. Thanks for your question, we will consider the situation later.

chenlijun99 commented 2 years ago

Thanks for the confirmation.

lhespress commented 1 year ago

@chenlijun99 We have fix it on master branch, thanks for your question.

chenlijun99 commented 1 year ago

Hi! Thanks for the heads-up. However, I tried to look at the most recent commit on master (2b2f9392946d62e7af86ef89485c5f2a607132f5) and I don't see how can the problem be fixed.

It seems to be that even on master the initialization vector is extracted from the exchanged APP KEY and then never modified.

https://github.com/espressif/esp-now/blob/2b2f9392946d62e7af86ef89485c5f2a607132f5/components/security/src/espnow_security.c#L68

Could you please give me some information on how replay-attack is prevented now?

lhespress commented 1 year ago

@chenlijun99 Thanks for your feedback, i'll check it again.

lhespress commented 1 year ago

@chenlijun99 The APP_KEY is generate random firstly which have 32 bytes and changed for each handshake. You can think that it's a protection against replay attacks.

chenlijun99 commented 1 year ago

Hi @lhespress. I'm definitely no security expert, but from my shallow understanding, the initialization vector (a.k.a. nonce) should be different for each frame. So, unless a handshake is performed for each frame, I don't see how currently esp-now protects against replay attacks. Please correct me if I'm wrong. Thanks!

grodansparadis commented 1 year ago

I second this. The IV should preferably be generated new on each send and be sent along the frame.

lhespress commented 1 year ago

@chenlijun99 @pedrominatel I also agree with you, current method only be a low-level. It need add interaction if changed for each frame sent.

grodansparadis commented 1 year ago

@lhespress What do you mean with interaction? Is't it as easy as adding a 16-byte random IV (in clear text) at the end of the payload which was used together with the secret key to encrypt the frame. The receiving side use this IV in the frame + the secret key to decrypt the frame. You loose 16-byte of payload but gain a lot of security.

lhespress commented 1 year ago

@grodansparadis @chenlijun99 Please add the patch as the attachment which use random IV at the end of the payload.

clarify_security.zip

grodansparadis commented 1 year ago

@lhespress Works perfect here. Thanks a lot for this one a truly fast patch indeed. Impressed!

chenlijun99 commented 1 year ago

Hi @grodansparadis! Perhaps I'm missing something here, but how does the approach suggested in https://github.com/espressif/esp-now/issues/40#issuecomment-1508431667 prevent replay attacks? The attacker can just sniff the packet and replay it. Unless the receiver side keeps a (ever-growing) list of previously received initialization vectors and then checks for each received frame whether the IV is already in that list, I don't see how random IV can work.

grodansparadis commented 1 year ago

@chenlijun99 no but it prevent the key from being exposed in a much better way then to use a static IV. I guess one must have some sort of time synchronization between the nodes to prevent what you describe. That would have been very nice to of course. Sorry for hijacking your thread btw.

chenlijun99 commented 1 year ago

@grodansparadis No worries. I should thank you for your contribution to the thread. While not fully protecting against replay attacks, it is certainly a good improvement to have. I think besides time synchronization also an incremental IV could work. Of course that makes the receiver also stateful, just like keeping a list of previously received random IV, but at lower cost (just the last received counter needs to be kept). Anyway, with the stateful approach I think one has to either handshake a new key after each re-boot (due to state-loss) or persist securely the state in non-volatile storage.

grodansparadis commented 1 year ago

@chenlijun99 It's always also a matter to say how far you should go. esp-now is very nice in it's simplicity and used for not critical tasks I guess it is OK security wise. People with higher demands go for ZigBee or similar. But annoying of course if someone replay "turn-on", "turn-off" lamp command(s) or resend invalid sensor values and similar of course. But the re-player at least don't know what he/she do without some further knowledge of the system. That can be bad enough of course.

To keep track of a node state gives me goose bumps. Not sure if it should... :)

Personally use another protocol (VSCP) as payload in esp-now and I think I can solve this problem on that level. But glad you brought it up because I never thought about it as a problem here actually.

In the original esp-now with two keys (pmk and lmk), one unique on each node, there was a way to ensure an event came from the source it was said to originate from. This is not the case anymore. But again enough security for my needs with the originating node knowing the key and therefore being trusted.

chenlijun99 commented 1 year ago

Surely everything depends on the threat model that one what's to adopt for their project. Chasing perfection in security, as in anything else, is an unsustainable endevaour. I think it really boils down to the security capabilities that the maintainers of esp-now intend to provide and to clearly document such capabilities. That's why I titled this issue "Please clarify security capabilities of esp-now" and not "Protect against replay attacks" :).

lhespress commented 1 year ago

@chenlijun99 Thanks for you brought up this topic. Let me clarify it as much as i can.

The original design was to have a higher level of security, but also transfer more data, so static IV was adopted.

There is 6-byte random for prevent replay attacks, both 4-byte at the protocol and 2-byte at the application. They are stored in a list and checked for duplicates when received. The code is this: https://github.com/espressif/esp-now/blob/master/src/espnow/src/espnow.c#L253. Of course, the capabilities is not perfection, but again enough for most scenarios.

We have considered using these random to update IV, but it only update fewer bytes and has limited improvement in security, thanks @grodansparadis idea which have a better improvement to security.

svdrummer commented 1 year ago

Hi @grodansparadis! Perhaps I'm missing something here, but how does the approach suggested in #40 (comment) prevent replay attacks? The attacker can just sniff the packet and replay it. Unless the receiver side keeps a (ever-growing) list of previously received initialization vectors and then checks for each received frame whether the IV is already in that list, I don't see how random IV can work.

I use an incremental counter. 1) Increment counter at the sender 2) Receiver decodes and stores the count. the first message is ignored due to count error. Apart from the very first count, the receiver check the current counter is higher than the last count ie, someone storing the message cant re-send as that message is no longer valid.

This is how a lot of commercial systems work, car alarm rolling key etc

i use uint8_t / uint16_t variables , and allow in the code for rollover.

chenlijun99 commented 1 year ago

Hi @lhespress. Thanks for the explanation and clarification. To me then the security capabilities of esp-now is clear. Feel free to close this issue.