pcap_lookupdev returns Unicode adapter name on Windows, should be ANSI.

[hsluoyz]

Hi @guyharris , a Npcap user reports to me a bug that running: WinDump.exe with Npcap will get error:

J:\github_repos\WinDump\win32\prj\Win32\Debug>WinDump.exe
WinDump.exe: listening on \Device\NPF_{44DB6B7A-661D-4FA3-925E-6287EA48D3F6}
WinDump.exe: \: The interface name has not been specified in the source string.

The responsible code in WinDump is: (the TCPdump maintained by Libpcap also has this issue)

        if (device == NULL) {
            device = pcap_lookupdev(ebuf);
            if (device == NULL)
                error("%s", ebuf);
        }
#ifdef WIN32
        if(strlen(device) == 1) //we assume that an ASCII string is always longer than 1 char
        {                       //a Unicode string has a \0 as second byte (so strlen() is 1)
            fprintf(stderr, "%s: listening on %ws\n", program_name, device);
        }
        else
        {
            fprintf(stderr, "%s: listening on %s\n", program_name, device);
        }

        fflush(stderr); 
#endif /* WIN32 */
        *ebuf = '\0';
        // pd = pcap_open_live(device, snaplen, !pflag, 1000, ebuf);
        pd = pcap_open(device, snaplen, 0, 1000, NULL, ebuf); //PCAP_OPENFLAG_NOCAPTURE_LOCAL
        if (pd == NULL)
            error("%s", ebuf);
        else if (*ebuf)
            warning("%s", ebuf);

I found that the adapter name (aka device) returned by pcap_lookupdev is Unicode, but it then becomes the input argument of pcap_open, which only accepts ANSI adapter name. So Libpcap recognizes the adapter name as \ instead of \Device\NPF_{44DB6B7A-661D-4FA3-925E-6287EA48D3F6} and fails the call.

I found this commit actually converts the output of pcap_lookupdev from ANSI to Unicode. I want to know why? AFAIK, all Libpcap interface is ANSI, and the Linux alternative seems to also return ANSI.

BTW, I also found some legacy Win95, 98 code which should be removed here: https://github.com/the-tcpdump-group/libpcap/commit/6831542ec753e8d13d7de2f383aebcc2df8b5a98#commitcomment-21039809

[guyharris]

The commit in question was made by Fulvio Risso back in 2003; you'd have to ask him for the official reason why, but my guess is that an older version of WinPcap returned Unicode strings (at least on Windows NT), and the change was made for binary compatibility reasons, even if it means source incompatibility between libpcap and WInPcap.

Yes, this is a mess (especially given that I think it returns ASCII strings on Windows OT). It is why, if you're trying to pick a default device on which to capture, I recommend using pcap_findalldevs() and using the first entry in the list, unless you have to make sure your software will work with older versions of libpcap or WinPcap that lack pcap_findalldevs().

[hsluoyz]

Hi @guyharris ,

To make things simple, I think I will just switch to pcap_findalldevs() and use the first adapter. Thanks for the support!

[guyharris]

To make things simple, I think I will just switch to pcap_findalldevs() and use the first adapter.

"Switch to pcap_findalldevs()" in a program, or "switch to pcap_findalldevs()" in pcap_lookupdev() on Windows?

If the latter, then 1) that does run the risk of breaking some programs that expect Unicode but 2) would mean you could use the same code on UN*X and Windows for pcap_lookupdev() (the UN*X version already calls pcap_findalldevs() and uses the first adapter (unless the first adapter is a loopback adapter, in which case it fails - that's compatible with what pcap_lookupdev() did in older versions of libpcap without pcap_findalldevs() and use the first adapter).

[hsluoyz]

"Switch to pcap_findalldevs()" in a program, or "switch to pcap_findalldevs()" in pcap_lookupdev() on Windows?

I only switched the call in my program for now.

I won't do the switch in the libpcap submodule of Npcap. The submodule should only use the release version of this official repo. I don't want to maintain a custom libpcap only for Npcap. Any intended change will be made as a PR to this official repo.

About this issue, It's basically your call whether to switch it in libpcap. Personally, I'm OK with either.

[Trolldemorted]

You might want to reopen this though, as this is not solved (i ran into it today while using npcap).

Imo you should at least change the return value to a wchar* (since right now the return value does not match the type in the header) - it was very consing to receive a utf16 string while the return value was supposed to be an ascii string. Nevertheless, i'd vote for an ascii return value.

[hsluoyz]

Reopened :)

[guyharris]

IMO you should only be using pcap_lookupdev() if your code absolutely MUST work with older versions of libpcap/WinPcap that don't have pcap_findalldevs(); if you can possibly use pcap_findalldevs(), you should use it - call it and, if it finds any devices at all, use the first one on the list. (That's what pcap_lookupdev() does on UN*X.)

pcap_lookupdev(), even on UN*X, isn't a good API, as it's not thread-safe (it uses a static buffer). With WinPcap, not only does it use a static buffer, but its behavior is, as you've discovered, a bit bogus.

The API can't universally be changed to return a wchar *, as that's not what it returns on UN*X or on Windows OT (95/98/Me). It'd have to be wchar * for NPcap and WinPcap-on-NT, and char * for everything else.

I don't know how many programs would break if it were to be changed to return an ASCII string on Windows NT. Personally, I think it might be fun to change it to return ASCII strings and, if anybody complains that their program broke, tell them to start using pcap_findalldevs(), but, then, I'm not the person who would directly get the complaints from WinPcap/NPcap users (although they might end up handed to me anyway).

[Trolldemorted]

I understand that you are reluctant to change the behaviour of the api, but both sides of the coin are ugly. If you start working with libpcap you have no idea that you should not use pcap_lookupdev(). The manpage does not say anything about it being deprecated or not threadsafe, so i assumed it was a normal function. I do know now because i spoke with you, but others won't.

To which windows release do you want to be backwards compatible? It should be impossible that recent versions of libpcap are running on pre-vista win32, since you don't supply native builds and drivers, winpcap is discontinued, and npcap requires vista or above. I am not convinced it makes sense to carry the burdens of 20years windows api changes, if the older windows versions cannot run it anyways.

You don't have to change it universally, it would just be helpful if some includeguard-magic would allow us to see the actual type on the affected systems (the entire windows family, if i understand 1.8's code correctly)

How do you think about removing the function from the public interface?

[guyharris]

How do you think about removing the function from the public interface?

Only in the sense of 1) explicitly deprecating it in the man page (which I'll do shortly) and 2) attaching "this is deprecated" attributes for compilers that support them. Removing its declaration from pcap.h would break source compatibility, and not exporting it from libpcap would break binary compatibility, so that will not happen.

I don't personally have a problem with changing the behavior in WinPcap-on-NT (note: "Windows NT" means "everything from NT 3.1 through Windows 10"), as long as I don't have to field complaints about the change - i.e., as long as, if changing the behavior causes a problem, I don't have to deal with the complaints, or get blamed for the change. (I didn't create this mess, the WinPcap developers did; that behavior dates back at least to WinPcap 3.1.)

I'm not sure how pre-VIsta vs. Vista-and-later enters into this; they're both NT, and both have the same WinPcap API.

[guyharris]

Deprecated in the documentation in 1a50c82211fce5bc5a3e12744b25161999b703d8.

[Trolldemorted]

:+1:

I mentioned pre-vistas because libpcap has code explicitly handling old windows versions which are not capable of running current libpcap releases.

Can we change the declaration to a wchar with macromagic for windows, or does that also break compability? Since wchar and char* have the same size, i assume that would be ok?

[guyharris]

I mentioned pre-vistas because libpcap has code explicitly handling old windows versions which are not capable of running current libpcap releases.

Those tests have nothing to do with Vista; they're checking for Windows OT (95, 98, Me) vs. Windows NT (NT 3.1 through Windows 10).

Can we change the declaration to a wchar* with macromagic for windows, or does that also break compability?

If we don't care about Windows OT, that'd work unless somebody is 1) using pcap_lookupdev(), 2) assigning its result to a char *, and 3) running the build with options that provoke a warning for such an assignment and that turn that warning into an error. I don't know whether anybody's doing that.

[Trolldemorted]

Those tests have nothing to do with Vista; they're checking for Windows OT (95, 98, Me) vs. Windows NT (NT 3.1 through Windows 10).

I know - but the operating system will always be vista or newer, thus never be 95, 98 or Me, that is why the check is superfluous. This check made sense when libpcap was capable of running on 95, 98 or Me, but now it doesn't, unless you plan to support them in the future. Why carry code around that will never be executed?

If we don't care about Windows OT, that'd work unless somebody is 1) using pcap_lookupdev(), 2) assigning its result to a char *, and 3) running the build with options that provoke a warning for such an assignment and that turn that warning into an error. I don't know whether anybody's doing that.

Even if anybody would be doing that, the only thing they'd have to change is the assignment. Since the assignment is incorrect anyways, we should break source compability here.

Though personally i am a fan of returning ascii - imho the conversion should have never been implemented. Changing the type to a void for windows, converting nothing and letting the users deal with whatever windows was returning would have been better in retrospect, because we would now be able to switch to char without hassle. It is insane that operating system details from the last millennium are still influencing apis today.

[infrastation]

Closing -- the recommended solution is to use pcap_findalldevs(), and pcap_lookupdev() has been deprecated.