Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support IPv6 #53

Closed
coreylane opened this issue Dec 31, 2019 · 10 comments · Fixed by #70
Closed

Support IPv6 #53

coreylane opened this issue Dec 31, 2019 · 10 comments · Fixed by #70
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@coreylane
Copy link

coreylane commented Dec 31, 2019

Great tool! Noticed ipv6 traffic is not being reported by the tool

Using the compiled binary release 0.6.0

# uname -a
Linux docker-compose 3.10.0-1062.1.2.el7.x86_64 #1 SMP Mon Sep 30 14:19:46 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

# cat /etc/redhat-release
CentOS Linux release 7.7.1908 (Core)

# ./bandwhich --version
bandwhich 0.6.0

Test/reproduce issue
wget -6 http://releases.ubuntu.com/18.04.3/ubuntu-18.04.3-desktop-amd64.iso
wget -4 http://releases.ubuntu.com/18.04.3/ubuntu-18.04.3-desktop-amd64.iso

@grishy
Copy link
Contributor

grishy commented Dec 31, 2019

let content = run(&["-n", "-P", "-i4"]);

it says that use ip4 only -i4 for macOS
maybe something similar under linux, but I have not yet found

@coreylane
Copy link
Author

coreylane commented Dec 31, 2019

use ::pnet::packet::ipv4::Ipv4Packet;

use ::std::net::Ipv4Addr;

May be related to #34

@SuperSandro2000
Copy link

I can confirm that #34 is related.

@imsnif
Copy link
Owner

imsnif commented Jan 1, 2020

So, I'm afraid this tool is very much ipv4 centered at the moment on all platforms. It shouldn't be a problem to get it to also work with ipv6, but it will require some work.

Off the top of my head:

  • We should also sniff for ipv6 packets
  • We should adjust our parsing of ipv6 packets to properly get the packet length from them
  • We should adjust the UI to also support stringifying ipv6 addresses to properly display them

Anyone wants to work on this? I'd be happy to provide pointers and guidance.

@imsnif imsnif changed the title missing ipv6 traffic Support IPv6 Jan 1, 2020
@imsnif imsnif added the help wanted Extra attention is needed label Jan 1, 2020
@zhangxp1998
Copy link
Collaborator

zhangxp1998 commented Jan 4, 2020

I will take this.
TODO

  • Update sniffer.rs to extract information from Ipv6 packets
  • Update Socket struct to accommodate Ipv6 addresses.
  • Update UI logic to display Ipv6 addresses
  • Update ui.rs
  • Update lsof_util.rs to include -i6 flag?
  • Update regex here

Anything else needs to be done in order to support this feature?

@zhangxp1998
Copy link
Collaborator

@imsnif

Regex::new(r"([^\s]+).*(TCP|UDP).*:(.*)->(.*):(\d*)(\s|$)").unwrap();

Would you give an example of what this regex is supposed to match? I'm trying to adapt this regex for Ipv6 addresses.

@lnicola
Copy link

lnicola commented Jan 4, 2020

const FULL_RAW_OUTPUT: &str = r#"
, I think.

It might not be a bad idea to replace that regex with hand-written parsing code, though.

@zhangxp1998 zhangxp1998 mentioned this issue Jan 4, 2020
@imsnif
Copy link
Owner

imsnif commented Jan 4, 2020

Hey @zhangxp1998 - @lnicola is right. It would definitely be great to get a more robust parser here if you feel up to it, but it might be out of scope for this feature (up to you).

The points you wrote seem to be a good run down. I think the best thing to do would be to start out and see what's missing.

Which brings me to another very important point: Tests. You could probably duplicate a lot of the ipv4 tests and just expect different results. Bandwhich uses snapshot testing using insta. Let me know if you need a hand with it.

What we need to think about with ipv6 is the length of the addresses and consequently the screen real-estate we have to give them. As things are right now, connection strings get cut off quite often (especially since we added the interface name at the beginning). I want to address this issue, but it will be even harder with ipv6. Do try to give a thought to this when developing, and feel free to consult me and/or write more here so others could also participate in the discussion.

Happy to hear you're taking this up! I saw you started out with a draft PR, ping me there too if you prefer talking about the implementation there.

@lnicola
Copy link

lnicola commented Jan 4, 2020

My thinking was that regular expressions are a bit overkill for this and they have their costs (performance, compile time, binary size).


Sorry for randomly poking around issues like this. I'd love to contribute (and to see how it works), but I'm not sure when/if I'll find some time. One thing I'm curious about is whether BPF and possibly netlink would work instead of sniffing for packets like now.

@imsnif
Copy link
Owner

imsnif commented Jan 4, 2020

@lnicola - nothing to be sorry for. :) I think the discussion is valuable as long as it's on-topic.
As for your suggestion: how about starting by opening a separate issue and writing a little bit more details about your suggestion? Get a discussion going, maybe participate in an implementation - see what works for you.

@imsnif imsnif added the enhancement New feature or request label Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants