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.

Sign up for GitHub

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

Jump to bottom

Add support for NFLOG groups and "Operation not permitted" error #1169

Merged
merged 8 commits into from
Aug 2, 2023
Merged

Add support for NFLOG groups and "Operation not permitted" error #1169

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c31bdc2
Add support for NFLOG groups. Add condition for double open for send …
Jul 26, 2023
f8d2ac9
Add extra config to doxy
Jul 27, 2023
2e39ef1
Fix type
Jul 27, 2023
61b9304
Fix trailing white space
Jul 30, 2023
314d626
Change attribute name from extra to nflogGroup
Jul 31, 2023
954bf95
Change attribute name from extra to nflogGroup
Jul 31, 2023
277daa5
Fix variable name
Jul 31, 2023
d1ea3b0
Add brackets
Aug 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Pcap++/header/PcapLiveDevice.h
View file
Open in desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a more general question: is it possible to add tests for this feature? Our Linux CI currently uses docker containers so I guess this would be hard because nflog is driven by the kernel, am I wrong? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is possible to add tests for this feature. But we would need a few things:

  • Run with elevated privileges so we can open nflog

nflog events are generated by netfilter when an IPTables rule is triggered:

  • Create an IPTables rule inside the container
  • Trigger the rule

I am not familiar with the testing infrastructure in the project, do you think it would be possible to implement the aforementioned steps in the CI workflow?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PcapPlusPlus uses GitHub Actions as the CI platform. You can see the workflow file here: https://github.com/seladb/PcapPlusPlus/blob/master/.github/workflows/build_and_test.yml

Currently all Linux jobs run on a docker container, but maybe we can add a new job that runs in a VM and not in a docker container. This should be pretty easy, here are some docs that explain how to do it. When we create this job we can add the IP table rules etc. This should be the easy part...

The more complex part is how to run the test only in the new "nflog" job and not in the other tests. In order to do that you need to understand the PcapPlusPlus testing infrastructure. Each test has 1 or more "tags" that can be included or excluded in test runs. You can see these tags here: https://github.com/seladb/PcapPlusPlus/blob/master/Tests/Pcap%2B%2BTest/main.cpp
What we can do is add a new "nflog" tag for this test, and run all other tests with -x "nflog". Then in this job we can run it with -t "nflog".

As I think more about it, it seems like a lot of work... maybe we can skip tests for now and keep it as a future enhancement. @egecetin @MrPeck what do you think?

Copy link
Collaborator

@egecetin egecetin Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think separate PR for tests is a better idea. So, we can merge this PR to fix error. Since it also needs configuration of VM, testing the workflow locally is not easy, it will probably take some time, you have to push and wait again, again and again...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that keeping tests for the future would be a better idea. And I would gladly take that task upon myself whenever it comes up :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks! 👍

Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ namespace pcpp
*/
int snapshotLength;

/**
* Set extra information. For some types of device, extra information might be required. Such a case is when opening a NFLOG
* device and setting a different NFLOG group. If no extra information is required, this configuration is ignored.
*/
unsigned int extra;
MrPeck marked this conversation as resolved.
Show resolved Hide resolved

/**
* A c'tor for this struct
* @param[in] mode The mode to open the device: promiscuous or non-promiscuous. Default value is promiscuous
Expand All @@ -224,15 +230,18 @@ namespace pcpp
* A snapshot length of 262144 should be big enough for maximum-size Linux loopback packets (65549) and some USB packets
* captured with USBPcap (> 131072, < 262144). A snapshot length of 65535 should be sufficient, on most if not all networks,
* to capture all the data available from the packet.
* @param[in] extra Extra information for specific devices. Default value is 0. This value is used when a device needs to be
* provided with some extra information. If such information is not required, this configuration shall be ignored.
*/
explicit DeviceConfiguration(DeviceMode mode = Promiscuous, int packetBufferTimeoutMs = 0, int packetBufferSize = 0,
PcapDirection direction = PCPP_INOUT, int snapshotLength = 0)
PcapDirection direction = PCPP_INOUT, int snapshotLength = 0, unsigned int extra = 0)
MrPeck marked this conversation as resolved.
Show resolved Hide resolved
{
this->mode = mode;
this->packetBufferTimeoutMs = packetBufferTimeoutMs;
this->packetBufferSize = packetBufferSize;
this->direction = direction;
this->snapshotLength = snapshotLength;
this->extra = extra;
}
};

Expand Down
20 changes: 17 additions & 3 deletions Pcap++/src/PcapLiveDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#define LIBPCAP_OPEN_LIVE_TIMEOUT -1
#endif

static const char *NFLOG_IFACE = "nflog";
seladb marked this conversation as resolved.
Show resolved Hide resolved
static const int DEFAULT_SNAPLEN = 9000;

namespace pcpp
Expand Down Expand Up @@ -202,7 +203,14 @@ void PcapLiveDevice::statsThreadMain()
pcap_t* PcapLiveDevice::doOpen(const DeviceConfiguration& config)
{
char errbuf[PCAP_ERRBUF_SIZE] = {'\0'};
pcap_t* pcap = pcap_create(m_Name.c_str(), errbuf);
std::string device_name = m_Name;

if (device_name == NFLOG_IFACE)
{
device_name += ":" + std::to_string(config.extra & 0xffff);
seladb marked this conversation as resolved.
Show resolved Hide resolved
}

pcap_t* pcap = pcap_create(device_name.c_str(), errbuf);
if (!pcap)
{
PCPP_LOG_ERROR(errbuf);
Expand Down Expand Up @@ -306,8 +314,14 @@ bool PcapLiveDevice::open(const DeviceConfiguration& config)
}

m_PcapDescriptor = doOpen(config);
m_PcapSendDescriptor = doOpen(config);
if (m_PcapDescriptor == nullptr || m_PcapSendDescriptor == nullptr)

// It's not possible to have two open instances of the same NFLOG device:group
if (m_Name == NFLOG_IFACE)
m_PcapSendDescriptor = nullptr;
else
m_PcapSendDescriptor = doOpen(config);
MrPeck marked this conversation as resolved.
Show resolved Hide resolved

if (m_PcapDescriptor == nullptr || (m_Name != NFLOG_IFACE && m_PcapSendDescriptor == nullptr))
{
m_DeviceOpened = false;
return false;
Expand Down