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

portability.h not installed #560

Closed
woodsb02 opened this issue Feb 18, 2017 · 12 comments
Closed

portability.h not installed #560

woodsb02 opened this issue Feb 18, 2017 · 12 comments

Comments

@woodsb02
Copy link

woodsb02 commented Feb 18, 2017

On FreeBSD 12-current, building the port sysutils/pftop has started to fail since the update to libpcap 1.8.1, producing the following error:

===>  Building for pftop-0.7_8
yacc -d -o sf-grammer.c sf-grammer.y
lex  -osf-scanner.c sf-scanner.l
echo pftop: /usr/lib/libc.a  >> .depend
Warning: Object directory not changed from original /wrkdirs/usr/ports/sysutils/pftop/work/pftop-0.7
cc  -O2 -pipe  -DHAVE_SNPRINTF=1 -DHAVE_VSNPRINTF=1 -DHAVE_FINE_GRAINED_LOCKING=1 -fstack-protector -fno-strict-aliasing -Wall -DOS_LEVEL=45   -MD  -MF.depend.pftop.o -MTpftop.o -std=gnu89 -fstack-protector-strong    -Qunused-arguments  -c pftop.c -o pftop.o
In file included from pftop.c:70:
In file included from ./sf-gencode.h:31:
/usr/include/pcap-int.h:369:10: fatal error: 'portability.h' file not found
#include "portability.h"
         ^
1 error generated.
*** Error code 1

Upon inspection, it appears the portability.h file is not installed in /usr/include/ with the other headers.

Note that portability.h was only recently created in this commit: 7cc3ed6

@guyharris
Copy link
Member

It's not supposed to be installed.

Neither is pcap-int.h.

Those are both internal headers that are part of the libpcap source. If FreeBSD is installing pcap-int.h in /usr/include, then FreeBSD needs to change to install portability.h in /usr/include, but, really, FreeBSD should change to NOT install pcap-int.h in /usr/include and pftop should change not to include pcap-int.h.

@guyharris
Copy link
Member

FreeBSD bug 217219 has been filed against sysutils/pftop, to get the includes of pcap-int.h removed.

@woodsb02
Copy link
Author

FreeBSD bug 217221 has been filed against contrib/libpcap, to get either stop installing pcap-int.h or also install portability.h

uqs pushed a commit to freebsd/freebsd-src that referenced this issue Mar 7, 2017
Reference:	the-tcpdump-group/libpcap#560
PR:		217221


git-svn-id: svn+ssh://svn.freebsd.org/base/head@314863 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
uqs pushed a commit to freebsd/freebsd-src that referenced this issue Mar 7, 2017
bdrewery pushed a commit to bdrewery/freebsd that referenced this issue Mar 9, 2017
Reference:	the-tcpdump-group/libpcap#560
PR:		217221


git-svn-id: svn+ssh://svn.freebsd.org/base/head@314863 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
brooksdavis pushed a commit to CTSRD-CHERI/cheribsd that referenced this issue Mar 12, 2017
uqs pushed a commit to freebsd/freebsd-src that referenced this issue May 31, 2017
r313695:
MFV r313676: libpcap 1.8.1

r313760:
MFV r313759: license change for a few headers (4 clause BSD to 3 clause BSD).

X-MFC-with:	r313695

r314769:
Remove compatibility with old libpcap.

Differential Revision:	https://reviews.freebsd.org/D9606

r314863:
Stop installing pcap-int.h, which is the internal interface for libpcap.

Reference:	the-tcpdump-group/libpcap#560
PR:		217221

r314865:
Bump __FreeBSD_version for removal of pcap-int.h.

PR:		217221

r316125:
MFV r316124: Fix build when WITHOUT_INET6.

Reported by:	Randy Westlund <rwestlun gmail com>
mat813 pushed a commit to mat813/freebsd that referenced this issue Jun 1, 2017
r313695:
MFV r313676: libpcap 1.8.1

r313760:
MFV r313759: license change for a few headers (4 clause BSD to 3 clause BSD).

X-MFC-with:	r313695

r314769:
Remove compatibility with old libpcap.

Differential Revision:	https://reviews.freebsd.org/D9606

r314863:
Stop installing pcap-int.h, which is the internal interface for libpcap.

Reference:	the-tcpdump-group/libpcap#560
PR:		217221

r314865:
Bump __FreeBSD_version for removal of pcap-int.h.

PR:		217221

r316125:
MFV r316124: Fix build when WITHOUT_INET6.

Reported by:	Randy Westlund <rwestlun gmail com>


git-svn-id: https://svn.freebsd.org/base/stable/11@319279 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
@Sashan
Copy link

Sashan commented Oct 4, 2017

Hello,
we hit similar issue on Solaris with pflogd, which comes from OpenBSD. I understand
including pcap-int.h is bad idea, but we just follow upstream. Hence I would like to
ask you to kindly consider patch below:

--- a/pcap-int.h        2016-10-25 17:07:59.000000000 -0700
+++ b/pcap-int.h        2017-10-04 03:57:35.795313428 -0700
@@ -366,7 +366,9 @@
 
 #include <stdarg.h>
 
+#ifdef BUILDING_PCAP
 #include "portability.h"
+#endif
 
 /*
  * Does the packet count argument to a module's read routine say

thanks a lot

@infrastation
Copy link
Member

For posterity, there is now a FAQ entry about this.

@guyharris
Copy link
Member

we hit similar issue on Solaris with pflogd, which comes from OpenBSD. I understand including pcap-int.h is bad idea, but we just follow upstream. Hence I would like to ask you to kindly consider patch below:

Please consider telling upstream not to include pcap-int.h.

@Sashan
Copy link

Sashan commented Dec 28, 2021

we hit similar issue on Solaris with pflogd, which comes from OpenBSD. I understand including pcap-int.h is bad idea, but we just follow upstream. Hence I would like to ask you to kindly consider patch below:

Please consider telling upstream not to include pcap-int.h.

OK. I'll deal with it. With OpenBSD and Solaris things are more colourful, I think. If I'm not mistaken OpenBSD keeps its own fork of libpcap/tcpdump in base tree. However Solaris uses libpcap/tcpdump from primary source. This is at least my understanding of situation.

@guyharris
Copy link
Member

If I'm not mistaken OpenBSD keeps its own fork of libpcap/tcpdump in base tree.

They do, but I think they picked up pcap_open_dead(), so that's not a reason to include pcap-int.h.

Looking at the current state of the OpenBSD CVS repository:

  1. they directly fetch the linktype and snapshot values from a pcap_t, which might, in some extreme cases, even save a whole microsecond over calling pcap_datalink() and pcap_snapshot(), respectively (i.e., no reason to include pcap-int.h, just use the standard APIs);
  2. they directly fetch the tzoff value from a pcap_t, which is always set to zero on a live capture, either with our libpcap or OpenBSD's (i.e., no reason to include pcap-int.h, just set hdr.thiszone to zero);
  3. pflog_read_live() appears to implement something similar to libpcap's BPF activate routine - I'll look at that to see what, if anything, needs to be changed to allow them to just use pcap_create() and pcap_activate(), like normal people;
  4. priv_init_pcap() does internal things, but it looks as if it's a hack, working around the lack of a better way to do privilege separation, so I'll look at that to see whether the pcap_open_live_fd() they speak of would be the right answer or just the answer you get when you ask the wrong question.

@infrastation
Copy link
Member

There is no pcap-int.h in OpenBSD 6.9 and 7.0:

$ find /usr/include/ -name 'pcap*.h'
/usr/include/pcap-namedb.h
/usr/include/pcap.h

@Sashan
Copy link

Sashan commented Dec 29, 2021

I don't remember from top of my head why I include pcap-int.h on Solaris. Pull request is fairly old, I will take a look when will be back at my desk. I agree the header file structure is different on OpenBSD. I have to revisit that stuff to find exact reason. The time frame is around OpenBSD 5.5.

@Sashan
Copy link

Sashan commented Dec 30, 2021

Looks like I can get rid of pcap-int.h on Solaris. There are functions I need to read linktype (pcap_linktype()), read and set snaphot length (pcap_set_snaplen() and pcap_snpashot()). The only remaining piece is how to deal with time zone offset:

421                 hdr.magic = TCPDUMP_MAGIC;
422                 hdr.version_major = PCAP_VERSION_MAJOR;
423                 hdr.version_minor = PCAP_VERSION_MINOR;
424                 hdr.thiszone = hpcap->tzoff; /* ??? is there a way to get time zone offset??? */
425                 hdr.snaplen = pcap_snapshot(hpcap);
426                 hdr.sigfigs = 0;
427                 hdr.linktype = pcap_datalink(hpcap);

snapshot above comes from pflogd.c we keep on Solaris. The same part as found in OpenBSD:

379                 hdr.magic = TCPDUMP_MAGIC;
380                 hdr.version_major = PCAP_VERSION_MAJOR;
381                 hdr.version_minor = PCAP_VERSION_MINOR;
382                 hdr.thiszone = hpcap->tzoff;
383                 hdr.snaplen = hpcap->snapshot;
384                 hdr.sigfigs = 0;
385                 hdr.linktype = hpcap->linktype;

Also OpenBSD base does not include pcap-int.h from /usr/include It uses CFLAGS=-I../../lib/libpcap to grab the file from library, see Makefile

I agree this PR is mostly invalid. Still will appreciate any hints on how to obtain ->tzoff without including pcap-int.h Thanks a lot for your help.

@guyharris
Copy link
Member

The only remaining piece is how to deal with time zone offset

Ignore it.

Seriously.

Just set it to zero in the header of any file you write, just as you do with hdr.sigfigs:

  1. libpcap doesn't supply it, so no program using libpcap uses it. It also writes both of those fields as zero.
  2. Wireshark doesn't use libpcap to read or write capture files, but it doesn't use the time zone offset, either, and it writes both of those fields as zero.
  3. A numeric time zone offset is not always sufficient to let you convert the sort-of-UTC-based (POSIX-time-based) time stamps in pcap and pcapng files into local time in the locale where the file is captured, as it doesn't tell you what the daylight savings time rules are.
  4. The current pcap file format draft describes the two locations in the file header that were used for the time zone offset and significant figures values as reserved (should be written as 0, must be ignored by readers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants