secdev / scapy

Scapy: the Python-based interactive packet manipulation program & library.
https://scapy.net
GNU General Public License v2.0
10.83k stars 2.04k forks source link

Some advice about Zigbee and Dot15d4 improvements #2992

Closed rben-dev closed 3 years ago

rben-dev commented 3 years ago

Dear Scapy developers,

First of all, thanks for developing and maintaining such a great and powerful tool :-)

This is more a request for advice than an issue per se. The topic covers the Zigbee and 802.15.4 layers architecture and extensibility, in order to properly prepare future contributions to the Scapy project. Please forgive the approach if this is not the place to ask for such advice!

For some internal needs, we have developed new features on top of the existing layers: 1- Packet encryption and decryption both at the Zigbee and 802.15.4 layers 2- Some ZCL (Zigbee Cluster Library) [1] extensions that improve Zigbee packets dissection

The big issue we face here is that the ZCL standard (that is one of the possible application layers of Zigbee) is really huge and covers many possible clusters. We have implemented most of the general clusters, as well as the OTA and the Commissioning clusters, and more might come in the future as we need them. So here comes the first issue: the Zigbee file in scapy/layers might slowly become unreadable and difficult to maintain. What would be the best way to contribute without killing readability and maintainability? One possible solution would be to split some parts of Zigbee in another file, e.g. a zigbee_zcl.py file. If such a solution is conceivable, where would this file go? (in layers, in contrib, elsewhere?).

A related issue concerns the packet encryption and decryption routines (for both Zigbee and Dot15d4): for now, we have put these routines in their respective layer files (along with an implementation of the AES-CCM* core cryptographic primitive). These routines take a packet as input, and encrypt or decrypt it to provide a new packet when possible. Is this way of dealing with encryption/decryption suits the Scapy philosophy or are there better paradigms to handle this? Finally, encryption and decryption raise another question: where "default keys", or default values in general, can be handled? We have found that the conf class already implements such global values, but is it the cleanest way to deal with this?

Thanks in advance for your precious advice and feedback on this matter, Regards,

[1] https://zigbeealliance.org/wp-content/uploads/2019/12/07-5123-06-zigbee-cluster-library-specification.pdf

gpotter2 commented 3 years ago

Hi & thanks for your interest in Scapy.

The encryption and decryption can very likely go along the definitions in their layer files. This is what was done for macsecfor instance.

It is a good idea to split the ZCL in a separate file. About whether it should go in layers or contrib, it depends whether it should be loaded by default or not (and the amount of support we maintainer will provide). If you think these kind of packets are more uncommon, you would rather put them in contrib where they'll need to be loaded manually.

You can use conf.contribs to store contrib-specific configuration keys. For instance conf.contribs["zigbee"] (or any key). Just be careful to not overwrite it if already present on loading.

Do you want to add anything @guedou ?

guedou commented 3 years ago

IMHO, we should keep the zigbee layer in scapy/layers/ to avoid breaking code that rely on these layers. The zigbee.py file could be splitted into smaller files as done in scapy/layer/tls/. Following your example, you could use scapy/layers/zigbee/zcl.py. The current code could be moved to scapy/layers/zigbee/layers.py to start with.

Concerning keys, there is no unique way of storing them. For example, we already have conf.wepkey & conf.tls_sessions. It is OK to use conf.zigbee_keys if that makes sense.

rben-dev commented 3 years ago

Thanks to both of you for your time and valuable advice,

Since ZCL seems to be a prevalent Zigbee class in common IoT related frames, putting the zigbee.py and zcl.py files in a scapy/layers/zigbee new folder might indeed be the reasonable way to go! I will prepare a PR with a proposal for this split and submit it.

Thanks again, Regards,