Skip to content

Conversation

seladb
Copy link

@seladb seladb commented Jun 3, 2021

Description

Add WinPcap/4.1.2 recipe. Please note this is the latest stable version of WinPcap and not the dev version that was previously built from source here: https://github.com/bincrafters/conan-winpcap
This package should be good enough for all Windows users that need WinPcap (since WinPcap is no longer maintained anyway).

Related Issue

N/A

Motivation and Context

I hope this is the first step of migrating https://github.com/bincrafters/conan-winpcap to ConanCenter

How Has This Been Tested?

I wrote a brand new test_package.cpp that reads a packet from a pcap file to make sure linking with WinPcap works

@seladb
Copy link
Author

seladb commented Jun 3, 2021

I'm not sure why it fails in CI on VS 2017/2019.
In my local machine things are working fine...

@seladb
Copy link
Author

seladb commented Jun 4, 2021

The build is still failing. Any idea why? It works well on my local machine...

@seladb seladb requested a review from Croydon June 4, 2021 07:07
@Croydon
Copy link
Member

Croydon commented Jun 5, 2021

No idea. Maybe you can think about a potential difference between your local machine and the CI environment?

@seladb
Copy link
Author

seladb commented Jun 6, 2021

@Croydon I figured out what the problem was: when running something that needs WinPcap you need wpcap.dll and Packet.dll to be installed or present. The library we're linking with is just the WinPcap SDK. I added these 2 files to the test package and CI passes now! 😃
Let me know if there's anything missing in this PR

@Croydon
Copy link
Member

Croydon commented Jun 6, 2021

Good to know what the problem is, but adding libraries directly to the test_package is not a solution

Why are those libraries only missing on Windows? Should they be present in this Conan package?

@seladb
Copy link
Author

seladb commented Jun 6, 2021

Good to know what the problem is, but adding libraries directly to the test_package is not a solution

Why are those libraries only missing on Windows? Should they be present in this Conan package?

WinPcap is only available for Windows. For other platforms libpcap should be used.
In order to use WinPcap and link it with your code you should do 2 things:

This Conan package wraps the WinPcap SDK. It doesn't run the WinPcap installer as I don't think Conan should install anything on the user's system (please correct me if I'm wrong). Adding the wpcap.dll and Packet.dll to the test package emulates what WinPcap installer does (but without installing anything on the user's system).

Please let me know what you think

@seladb
Copy link
Author

seladb commented Jun 9, 2021

@Croydon can you please advise on how we can move forward with this PR?

@seladb
Copy link
Author

seladb commented Jun 17, 2021

@Croydon @solvingj what should we do with this PR?

@solvingj
Copy link
Member

I now remember this nightmare. Yes, indeed, this was the biggest thing that made pcapplusplus and libtins such difficult libraries to package properly for windows.

Let me review the landscape again and consider what the best course of action is. I will say one thing right now for sure... there is no simple solution in this case. There are a few options, and each comes with significant implications and sacrifices.

@seladb
Copy link
Author

seladb commented Jun 17, 2021

Thanks @solvingj , I really appreciate your help with this!
Let me know what you think should be the right approach here

@Croydon
Copy link
Member

Croydon commented Jun 21, 2021

Sorry, for the late response. I don't know much about winpcap, but if it is possible to build wpcap.dll and packet.dll from source, it might be best to add those to this package?

@seladb
Copy link
Author

seladb commented Jun 22, 2021

Sorry, for the late response. I don't know much about winpcap, but if it is possible to build wpcap.dll and packet.dll from source, it might be best to add those to this package?

hi @Croydon I'm not sure that makes a lot of sense because this package installs the WinPcap SDK without building it from source, so building the WinPcap DLLs (which are usually installed from an installer file: https://www.winpcap.org/install/bin/WinPcap_4_1_3.exe) will probably not be a good idea.

@solvingj did you have a chance to take a look at this?

@solvingj
Copy link
Member

So, I completely agree with @Croydon in theory.

wpcap.dll and Packet.dll are open-source components, and can be built from the sources included in https://www.winpcap.org/install/bin/WpdPack_4_1_2.zip. When users call conan install --build=all, they expect all dependencies to be built from source. Why not this one? "It's normally built from an installer" is not a good reason. Using pre-compiled binaries for an open source library just doesn't make sense on the surface, and is not justifiable in almost any case.

However...

What's wrong with just building from source? Well, the reality is that the total complexity involved with re-creating these two .dll files from source is comically high (I laugh to keep from crying). It turns out that WinPCap is one of the most hacked together things I've ever seen. I can't believe it ever worked, let alone went on to become a fundamental runtime dependency in dozens of hugely popular open-source and commercial applications. It's mind boggling. It's not just complex, it's super ugly and of questionable legality. I remember the .zip contains some copies of some headers from the libpcap source code... which is actually kind of a problem from both a licensing and a dependency management perspective. I think they are then patched to work on windows or something, making the problems worse. I think it also requires flex and bison which are Linux applications (I recall we now have a working winflexbison package port for windows, but did not when I was trying to package).

All of this could have been fixed upstream, but sadly the WinPcap project got acquired by a commercial company, and then abandoned... basically freezing it forever in a state of horror.

The situation is so bad, that it actually MIGHT justify making an installer wrapper package and treating like a system_requirement (which is horrible IMO). I gave up before, and I personally would not fight the battle again personally.

But wait... there's hope! We can consider an alternative. As with most popular-but-abandoned OSS projects, someone picks up the baton. The brilliant security folks at nmap.org created an alternative packet driver for Windows called NPCAP, which even has a "WinPCAP mode" making it fully API compatible with WinPCAP, so that it can stand-in for WinPCAP. Almost perfect, so lets just package npcap and build it with winpcap mode by default, and completely avoid the disaster that is Winpcap.

But.... alas... more pain. Even this is not the silver-bullet it could have been. The nmap.org team decided that selling a commercial license for the NPCAP driver would be one of the ways they fund their company and work. So, they wrote their own custom license which has arbitrary provisions and constraints for who can use it for free, and under what circumstances, on how many machines, and how redistribution rights work.

Free and open source software producers are also welcome to contact us for redistribution requests. However, we normally recommend that such authors instead ask your users to download and install Npcap themselves.

In summary, npcap this simply seems too unique and complicated of a licensing model for me to be comfortable re-packaging and redistributing the way we do. They would probably give us permission ( I think i exchanged emails with the author a while back), but it still makes me very uncomfortable. Finally, it still isn't regular old "WinPCAP", and there will always be some users who just want regular old WinPCAP for whatever reason, so is it even worth messing with this port.

Sadly, despite all this history, I do not have good advice for you (@seladb) or us (bincrafters) on how to proceed with this. The only thing that would seemingly make it acceptable for me is a complete backwards-compatible clean-room re-write of WinPCAP with a traditional open-source license model which will never happen.

@Croydon given the complications with building the .dll from source, what are your thoughts?

@seladb
Copy link
Author

seladb commented Jun 23, 2021

@solvingj thanks for your detailed response! WinPcap is indeed very hacky and difficult to build, which is why I suggested using the pre-compiled binaries (for both the .lib as well as .dll files). I do understand that the Conan guidelines say that every package should be built from source, and in that sense WinPcap is probably not suitable to be packaged by Conan. Same goes for Npcap because of its license issues.
However, not having WinPcap/Npcap in Conan blocks other projects that depend on these libraries like the one I'm maintaining: PcapPlusPlus.
As the maintainer of PcapPlusPlus all I want is to make it easy for my users to get and install this project. In macOS they can use homebrew, in Linux they have linuxbrew or the distribution's own package manager, and on Windows they used to have Conan. But since Bintray was deprecated my Windows users don't have the option of using a well-known package manager like Conan to deploy PcapPlusPlus on Windows and I as the maintainer can't update the PcapPlusPlus Conan package to its latest version (v21.05). Unfortunately, all of this happened because of things that are out of my control and my users' control: bintray deprecation, and the fact that WinPcap (which is the most important dependency of PcapPlusPlus on Windows) is a lousy and abandoned project.

I'm not sure how we can take this forward. If you decide not to port WinPcap to the new bincrafters repo (and eventually to ConanCenter) then maybe I should consider building a PcapPlusPlus package for vcpkg which already has a WinPcap package.
To be honest, this is not my preferred option because writing and maintaining Conan packages is much easier comparing to vcpkg packages, and also because I'm already (somewhat) familiar with Conan and have you guys (and the entire bincrafters team) to rely on whenever I have questions or need help.

Let me know what you think

@Croydon
Copy link
Member

Croydon commented Jun 27, 2021

If we are allowed to redistribute the official binaries of the library files then we can add them in a Bincrafters Conan recipe. At Bincrafters, we can be more flexible in what we allow and what we do not allow. That said, we always try to eventually push recipes to CCI and redistribution of binaries will likely be rejected there. However, at that future point in time, there might still be the possibility to convert it to a system package (someone could add winpcap to winget).

Does this .exe installer allow silence installations to a specific directory via parameters? If yes, that might be the best way for now. At least, if the installer is not changing the system, but only extracting files?

@seladb
Copy link
Author

seladb commented Jun 28, 2021

Does this .exe installer allow silence installations to a specific directory via parameters? If yes, that might be the best way for now. At least, if the installer is not changing the system, but only extracting files?

Unfortunately the installer doesn't come with a silent install and that's why I use this version of WinPcap installer in PcapPlusPlus CI.

I've been thinking more about this and was looking at WinPcap recipes from both Bincrafters and vcpkg. Both patch the solution/project files to make the build pass. Maybe we can just use this? We can also build the .dll files and use them for the tests. Please let me know what you think.

@madebr
Copy link
Contributor

madebr commented Jul 15, 2021

Hello,

Would anybody of you please have a look at conan-io/conan-center-index#6348?

On Windows with Visual Studio, it needs the Windows Driver Kit.
I installed Windows 11 WDK,
but winpcap building fails because of a missing include (ndis/types.h inside ndis.h).

Perhaps you have more luck?

@madebr
Copy link
Contributor

madebr commented Jul 15, 2021

@seladb
May I borrow (=steal) your test package in my pr?

@seladb
Copy link
Author

seladb commented Jul 16, 2021

@seladb
May I borrow (=steal) your test package in my pr?

Yes, sure!

@seladb
Copy link
Author

seladb commented Jul 16, 2021

Perhaps you have more luck?

I think @solvingj managed to build WinPcap a few years back, maybe he can help

@Croydon
Copy link
Member

Croydon commented Mar 1, 2023

@seladb Coming back to this PR after a long time...

The CCI PR got closed and no one else tried so far.

What do you want to do with this PR? It doesn't match today's Conan standards anymore, but there is no regression to the current winpcap recipe we have, so I'm willing to merge this. (Though further modernization are obvious welcome)

@seladb
Copy link
Author

seladb commented Mar 2, 2023

Hi @Croydon , thanks for taking a second look at this PR.
WinPcap is definitely missing in ConanCenter, although the successor Npcap is available. I think it'd be nice to merge this PR. However do you think we can merge it into ConanCenter rather than bincrafters?

cmake.build()

def test(self):
for file_name in ["1_packet.pcap", "wpcap.dll", "Packet.dll"]:
Copy link
Member

Choose a reason for hiding this comment

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

Adding binary files is pretty bad. I think the old test_package might be better.

A test_package only needs to check if the Conan package was created correctly, not test internal logic

description = "The WinPcap packet capture library."
homepage = "https://www.winpcap.org/"
url = "https://github.com/bincrafters/community"
license = "Muliple"
Copy link
Member

Choose a reason for hiding this comment

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

I know that this is coming from the current Bincrafters recipe, but what are those licenses? We should list them here

license = ("x1", "x2", "x3")

@seladb
Copy link
Author

seladb commented Mar 3, 2023

@Croydon I can address these comments, but maybe it's better to open a new PR for WinPcap/4.1.2 in conan-center-index?

Or do you think there's no chance they will be willing to merge it?

This package will be more useful in conan-center-index than in bincrafters. Please let me know what you think.

@Croydon
Copy link
Member

Croydon commented Mar 3, 2023

Sure, if you can put in the work to get it in CCI then it will be better. If that does not work we can still merge it in BIncrafters. CCI has stricter policies in some areas.

@seladb seladb closed this Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants