scapy icon indicating copy to clipboard operation
scapy copied to clipboard

Fix in KNXnet/IP layer (KNX addresses handling)

Open claire-lex opened this issue 10 months ago • 5 comments

This PR introduces a few changes to the KNXnet/IP layer, last updated in 2021:

  • Fix validation and support issues on KNX individual addresses and KNX group addresses (see below)
  • Add a few recognized KNX codes and supported types for MultipleTypeFields in complex packets
  • Add basic and KNX address-related unit tests

I made the PR mainly to fix issues when building packets containing individual address (format 1.1.1) or group address (format 1/1/1) fields. The layer only supported either individual address or group address in a field, but some fields (for instance, in cEMI blocks) can take both formats. For instance, the code below used to raise a ValueError and is now valid.

test_addr = CEMI(message_code=0x11) # L_Data.req
test_addr.cemi_data.address_type = 0 # Individual address
test_addr.cemi_data.destination_address = "1.1.1"

claire-lex avatar Feb 11 '25 12:02 claire-lex

Thanks for the PR. Could you fix the flake8 issues at https://github.com/secdev/scapy/actions/runs/13262979518/job/37023594039?pr=4663 ?

guedou avatar Feb 16 '25 15:02 guedou

Codecov Report

Attention: Patch coverage is 71.87500% with 9 lines in your changes missing coverage. Please review.

Project coverage is 82.14%. Comparing base (c15a670) to head (ebb4e97). Report is 61 commits behind head on master.

Files with missing lines Patch % Lines
scapy/contrib/knx.py 71.87% 9 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4663      +/-   ##
==========================================
+ Coverage   81.55%   82.14%   +0.59%     
==========================================
  Files         359      361       +2     
  Lines       86557    86820     +263     
==========================================
+ Hits        70592    71321     +729     
+ Misses      15965    15499     -466     
Files with missing lines Coverage Δ
scapy/contrib/knx.py 90.52% <71.87%> (+8.02%) :arrow_up:

... and 39 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 17 '25 08:03 codecov[bot]

Done, it should be ok now! Sorry for the delay. Thank you :)

claire-lex avatar Mar 17 '25 08:03 claire-lex

Hi @claire-lex, (btw great SSTIC presentation, if I guessed the username right).

Just a small bump regarding this PR. There are still a bunch of unaddressed changes (see my previous comments) that prevent us from going forward with this. After a while, we might have to mark it as "draft" or close it, unless someone else picks it up.

Thanks

gpotter2 avatar Sep 03 '25 17:09 gpotter2

Hi, yes it was me, thank you! Your presentation was really nice too :)

I will fix it but I still did not have the time to do it (properly) yet. I'm sorry for keeping this PR stuck, it was not supposed to stay like this so long... As I will still not be able to do it for the next month or so, if you prefer to move it to drafts (or even close it) you can, and I will eventually ressucitate (or reopen) it. Sorry again for the delay. Claire

claire-lex avatar Sep 05 '25 15:09 claire-lex