scapy icon indicating copy to clipboard operation
scapy copied to clipboard

Rewrite arch/bpf

Open gpotter2 opened this issue 1 year ago • 5 comments

This rewrites most of the arch/bpf code that handles the loading of conf.ifaces, conf.route and conf.route6, in order to use PF_ROUTE (instead of the current reading involving subprocess, parsing ifconfig, and other kinda crappy stuff).

fixes:

  • https://github.com/secdev/scapy/issues/4498

This should:

  • add support for multiple IPv4 on interfaces
  • add proper support for multicast routes
  • stabilize the API by not relying on ifconfig changes.

(yeah this is a followup to https://github.com/secdev/scapy/pull/4352)

SUPPORTS:

  • FreeBSD 14.1
  • OpenBSD 7.5
  • NetBSD 10.0

TODO:

  • MacOS

Annoyingly it seems every BSD (at least FreeBSD, OpenBSD and NetBSD) have different rt_* structures. Ugh.

gpotter2 avatar Aug 08 '24 23:08 gpotter2

Codecov Report

Attention: Patch coverage is 73.87387% with 116 lines in your changes missing coverage. Please review.

Project coverage is 81.58%. Comparing base (97a49f3) to head (de7ed7a). Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
scapy/arch/bpf/pfroute.py 78.31% 72 Missing :warning:
scapy/arch/libpcap.py 44.11% 19 Missing :warning:
scapy/arch/bpf/core.py 0.00% 15 Missing :warning:
scapy/arch/common.py 72.72% 3 Missing :warning:
scapy/arch/bpf/supersocket.py 0.00% 1 Missing :warning:
scapy/arch/linux/__init__.py 83.33% 1 Missing :warning:
scapy/arch/solaris.py 0.00% 1 Missing :warning:
scapy/layers/dhcp.py 66.66% 1 Missing :warning:
scapy/layers/dhcp6.py 50.00% 1 Missing :warning:
scapy/tools/UTscapy.py 66.66% 1 Missing :warning:
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4497      +/-   ##
==========================================
- Coverage   81.68%   81.58%   -0.11%     
==========================================
  Files         355      356       +1     
  Lines       84936    85207     +271     
==========================================
+ Hits        69384    69519     +135     
- Misses      15552    15688     +136     
Files with missing lines Coverage Δ
scapy/arch/__init__.py 64.51% <100.00%> (+0.74%) :arrow_up:
scapy/arch/linux/rtnetlink.py 93.42% <100.00%> (+0.02%) :arrow_up:
scapy/arch/unix.py 12.39% <100.00%> (-54.82%) :arrow_down:
scapy/arch/windows/__init__.py 66.41% <100.00%> (-0.62%) :arrow_down:
scapy/config.py 80.12% <100.00%> (ø)
scapy/interfaces.py 96.31% <100.00%> (-0.29%) :arrow_down:
scapy/layers/tuntap.py 82.82% <ø> (ø)
scapy/route6.py 89.94% <ø> (+1.54%) :arrow_up:
scapy/arch/bpf/supersocket.py 0.00% <0.00%> (ø)
scapy/arch/linux/__init__.py 83.01% <83.33%> (-0.44%) :arrow_down:
... and 9 more

... and 2 files with indirect coverage changes

codecov[bot] avatar Aug 10 '24 12:08 codecov[bot]

@dhgutteridge Hi & sorry for bothering. I was wondering if you'd have some advices regarding making this PR work on NetBSD.

Unlike OpenBSD and FreeBSD, NetBSD has chosen to versionize the NET_RT_IFLIST const. In your humble opinion, which version should we be using? From what I understand, some are more compatible than others.

Also if you have something to say regarding this PR in general, feel free to give your opinion :)

Thanks for the help, cheers

gpotter2 avatar Aug 13 '24 15:08 gpotter2

@dhgutteridge Hi & sorry for bothering. I was wondering if you'd have some advices regarding making this PR work on NetBSD.

Sure, my pleasure. Thanks for supporting us. :)

Unlike OpenBSD and FreeBSD, NetBSD has chosen to versionize the NET_RT_IFLIST const. In your humble opinion, which version should we be using? From what I understand, some are more compatible than others.

I think you'd want the magic number 6 here, in other words, the current value of NET_RT_IFLIST. The other values are versioned for compatibility with old NetBSD binaries from EOL releases. Since you're tracking NetBSD 10, I believe, you'd want the most current value. (Now, whether that gives you any additional output needed for your purposes is another matter, I suppose. "3" may "just work" for you. Here I don't know how OpenBSD and FreeBSD vary in their content.)

Also if you have something to say regarding this PR in general, feel free to give your opinion :)

I could test it for you on NetBSD and/or gather input for any other questions you have.

Thanks for the help, cheers

dhgutteridge avatar Aug 14 '24 00:08 dhgutteridge

I think you'd want the magic number 6 here, in other words, the current value of NET_RT_IFLIST.

Thanks a lot ! I tried using that in the latest commit. It should now almost support NetBSD.

I could test it for you on NetBSD and/or gather input for any other questions you have.

Thanks !

I've taken inspiration from sbin/route/rtutil.c for some parts. I however don't really understand how the blob of data returned for IPv6 netmask in routes should be interpreted. (see the todo on line 922). rtutil.c seems to do some very weird stuff. If you have an idea I'm open for it, otherwise I'll get back to this eventually :)

If you want to try this PR out BTW, you should be looking at the contents of conf.ifaces, conf.route and conf.route6 in the Scapy CLI. Those are the parts affected by this rewrite.

gpotter2 avatar Aug 14 '24 19:08 gpotter2

I however don't really understand how the blob of data returned for IPv6 netmask in routes should be interpreted. (see the todo on line 922)

Apparently the first 6 octets of the sockaddr data are garbage (always 0xFFFFFFFFFF), followed by the actual IPv6 netmask. As far as my understanding goes at least. The netmask payload generally looks kinda malformed, unless I missed something, as the sockaddr type is also FF. On other BSDs, it was either AF_INET6 or 0.

gpotter2 avatar Aug 17 '24 14:08 gpotter2

@p-l- @guedou @polybassa This is ready for review. Was kind of a nightmare to make, and is probably also a nightmare to review.

It however works in its latest commit on all *BSDs mentionned above.

gpotter2 avatar Sep 01 '24 20:09 gpotter2

/packit build

gpotter2 avatar Sep 02 '24 17:09 gpotter2

Looks good to me.

polybassa avatar Sep 02 '24 18:09 polybassa

Then let's merge that monster and go for 2.6.0.

gpotter2 avatar Sep 02 '24 19:09 gpotter2

I however don't really understand how the blob of data returned for IPv6 netmask in routes should be interpreted. (see the todo on line 922)

Apparently the first 6 octets of the sockaddr data are garbage (always 0xFFFFFFFFFF), followed by the actual IPv6 netmask. As far as my understanding goes at least. The netmask payload generally looks kinda malformed, unless I missed something, as the sockaddr type is also FF. On other BSDs, it was either AF_INET6 or 0.

I have it on my list to look into this for you, just haven't got around to it yet.

dhgutteridge avatar Sep 30 '24 03:09 dhgutteridge