Bug 202950 - [patch] www/squid: TP_IPF build fails on FreeBSD <10
Summary: [patch] www/squid: TP_IPF build fails on FreeBSD <10
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Cy Schubert
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-09-07 16:17 UTC by Lawrence Chen
Modified: 2015-09-21 11:18 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (timp87)


Attachments
proposed patch (967 bytes, patch)
2015-09-07 16:17 UTC, Lawrence Chen
no flags Details | Diff
poudreire testport log (984.80 KB, text/plain)
2015-09-07 16:20 UTC, Lawrence Chen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lawrence Chen 2015-09-07 16:17:16 UTC
Created attachment 160803 [details]
proposed patch

I'm assuming the problem only affects less than FreeBSD 10, since I believe ipf was upgraded to v5.1.2 in 10.  While on my FreeBSD 9.3 it is v4.1.28.

The compile fails here:

 libtool: compile:  c++ -DHAVE_CONFIG_H -I../.. -I../../include -I../../lib -I../../src -I../../include -I../../libltdl -I/usr/include -Wall -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow -Werror -pipe -D_REENTRANT -O2 -pipe -march=nocona -I/usr/include -fstack-protector -I/usr/local/include -MT tools.lo -MD -MP -MF .deps/tools.Tpo -c tools.cc -o tools.o >/dev/null 2>&1
 cc1plus: warnings being treated as errors
 Intercept.cc: In member function 'bool Ip::Intercept::IpfInterception(const Comm::ConnectionPointer&, int)':
 Intercept.cc:208: warning: operation on 'warningLevel' may be undefined
 *** [Intercept.lo] Error code 1


After some investigation, I found that the offending upstream code was add in the recent update:

 diff -u -r -N squid-3.5.7/src/ip/Intercept.cc squid-3.5.8/src/ip/Intercept.cc
 --- squid-3.5.7/src/ip/Intercept.cc	2015-07-31 23:08:17.000000000 -0700
 +++ squid-3.5.8/src/ip/Intercept.cc	2015-09-01 12:52:00.000000000 -0700
 @@ -200,6 +200,19 @@
      // all fields must be set to 0
      memset(&natLookup, 0, sizeof(natLookup));
      // for NAT lookup set local and remote IP:port's
 +    if (newConn->remote.isIPv6()) {
 +#if IPFILTER_VERSION < 5000003
 +        // warn once every 10 at critical level, then push down a level each repeated event
 +        static int warningLevel = DBG_CRITICAL;
 +        debugs(89, warningLevel, "IPF (IPFilter v4) NAT does not support IPv6. Please upgrade to IPFilter v5.1");
 +        warningLevel = ++warningLevel % 10;
 +        return false;
 +#else
 +        natLookup.nl_v = 6;
 +    } else {
 +        natLookup.nl_v = 4;
 +#endif
 +    }
      natLookup.nl_inport = htons(newConn->local.port());
      newConn->local.getInAddr(natLookup.nl_inip);
      natLookup.nl_outport = htons(newConn->remote.port());

My proposed patch is to just comment out the defective nag to upgrade OS.
Comment 1 Lawrence Chen 2015-09-07 16:20:38 UTC
Created attachment 160804 [details]
poudreire testport log
Comment 2 timp87 2015-09-07 17:24:14 UTC
hi!
Sorry, I don't use IPF, so I'm not competent here.
I cc'd cy@, he ported latest IPF and the author of the latest change to www/squid. Let's wait for his comment.
Comment 3 Cy Schubert freebsd_committer 2015-09-07 17:41:47 UTC
I'll take this.
Comment 4 Cy Schubert freebsd_committer 2015-09-07 17:45:33 UTC
(In reply to timp87 from comment #2)
It is suggested we warn just once.
Comment 5 Jason Unovitch freebsd_committer 2015-09-18 03:15:10 UTC
How are things going here?  Bug 203186 covering the pending 3.1.9 release with security fixes is due out soon.  If we can batch up this fix in the MFH to quarterly or 3.1.9 that would be super!
Comment 6 commit-hook freebsd_committer 2015-09-18 05:12:58 UTC
A commit references this bug:

Author: cy
Date: Fri Sep 18 05:12:13 UTC 2015
New revision: 397215
URL: https://svnweb.freebsd.org/changeset/ports/397215

Log:
  Rather than produce a warning message that IPv6 is not supported
  under ipfilter 4 (FreeBSD 9) every tenth time, reduce the message
  to one in a million. This has the effect of displaying the message
  at or shortly after startup with a reminder every blue moon.

  PR:		202950

Changes:
  head/www/squid/Makefile
  head/www/squid/files/patch-src__ip__Intercept.cc
Comment 7 Cy Schubert freebsd_committer 2015-09-18 05:13:53 UTC
A less intrusive approach to produce the message every million times has the same effect as my original suggestion.
Comment 8 Jason Unovitch freebsd_committer 2015-09-20 19:31:42 UTC
(In reply to Cy Schubert from comment #7)
Reopen.

Cy, GCC still doesn't like this patch.  Was it tested?  I still get this:
Intercept.cc:208: warning: operation on 'warningLevel' may be undefined

Do you have any objection changing it to this to get rid of undefined behavior?
warningLevel = (warningLevel + 1) % 1048576;
Comment 9 commit-hook freebsd_committer 2015-09-21 01:51:55 UTC
A commit references this bug:

Author: junovitch
Date: Mon Sep 21 01:51:27 UTC 2015
New revision: 397476
URL: https://svnweb.freebsd.org/changeset/ports/397476

Log:
  www/squid: security update and build fix

  - security update 3.5.8 -> 3.5.9 [1]
  - Fix TP_IPF build on FreeBSD 9 [2]

  PR:		203186 [1]
  PR:		202950 [2]
  Approved by:	Pavel Timofeev <timp87@gmail.com> (maintainer) [1]
  Security:	d3a98c2d-5da1-11e5-9909-002590263bf5
  MFH:		2015Q3
  X-MFH-With:	r391555, r392222, r393602, r396106, r396185, r397215

Changes:
  head/www/squid/Makefile
  head/www/squid/distinfo
  head/www/squid/files/patch-src__ip__Intercept.cc
Comment 10 commit-hook freebsd_committer 2015-09-21 11:08:39 UTC
A commit references this bug:

Author: junovitch
Date: Mon Sep 21 11:08:27 UTC 2015
New revision: 397486
URL: https://svnweb.freebsd.org/changeset/ports/397486

Log:
  MFH r391555, r392222, r393602, r396106, r396185, r397215, r397476

  r391555
  www/squid: Support DragonFly SHM segments

  Out of the box, squid would not run on dragonfly due to its handling
  of SHM segments.  On DragonFly, SHM segments are always treated as files
  but on FreeBSD it depends on whether or not application is inside a jail.

  In any case, the case for DragonFly was no supported, so it has been
  added via patch.  This also requires the return of /var/run/squid
  directory which is where the SHM files are stored (defined by
  localstatedir and supported by RC script).  The RC script would define
  this directory if missing, but let's make sure it is always available.

  PR:		201405
  Submitted by:	marino
  Approved by:	maintainer (timp87/gmail)

  r392222
  www/squid: pkg-list fix

  - add missing pkg-plist entry (SSL_CRTD option)

  PR:                  201463
  Submitted by:        s3erios@gmail.com
  Approved by:         timp87@gmail.com (maintainer)

  r393602
  www/squid: update 3.5.6 -> 3.5.7

  - Fix build with ecap by clang
  - Get rid of useless and always empty /var/squid/logs
  - Rework patches to make portlint a bit happier

  PR:		202053
  Submitted by:	Pavel Timofeev <timp87@gmail.com> (maintainer)

  r396106
  www/squid: update 3.5.7 -> 3.5.8

  PR:		202826
  Submitted by:	Pavel Timofeev <timp87@gmail.com> (maintainer)
  Approved by:	feld (mentor)

  r396185
  Fix TP_IPF build.

  r397215
  Rather than produce a warning message that IPv6 is not supported
  under ipfilter 4 (FreeBSD 9) every tenth time, reduce the message
  to one in a million. This has the effect of displaying the message
  at or shortly after startup with a reminder every blue moon.

  PR:		202950

  r397476
  www/squid: security update and build fix

  - security update 3.5.8 -> 3.5.9 [1]
  - Fix TP_IPF build on FreeBSD 9 [2]

  PR:		203186 [1]
  PR:		202950 [2]
  Approved by:	Pavel Timofeev <timp87@gmail.com> (maintainer) [1]
  Security:	d3a98c2d-5da1-11e5-9909-002590263bf5
  Approved by:	portmgr (erwin)

Changes:
_U  branches/2015Q3/
  branches/2015Q3/www/squid/Makefile
  branches/2015Q3/www/squid/distinfo
  branches/2015Q3/www/squid/files/extra-patch-build-8-9
  branches/2015Q3/www/squid/files/patch-bug4190
  branches/2015Q3/www/squid/files/patch-compat_compat.h
  branches/2015Q3/www/squid/files/patch-compat_shm.cc
  branches/2015Q3/www/squid/files/patch-configure
  branches/2015Q3/www/squid/files/patch-configure_GSSAPI_NONE
  branches/2015Q3/www/squid/files/patch-configure_NIS
  branches/2015Q3/www/squid/files/patch-configure_crypt.h
  branches/2015Q3/www/squid/files/patch-src-cf.data.pre
  branches/2015Q3/www/squid/files/patch-src_DiskIO_Mmapped_MmappedFile.cc
  branches/2015Q3/www/squid/files/patch-src__ip__Intercept.cc
  branches/2015Q3/www/squid/files/patch-src_ipc_mem_Segment.cc
  branches/2015Q3/www/squid/files/patch-src_tools.cc
  branches/2015Q3/www/squid/pkg-plist
Comment 11 commit-hook freebsd_committer 2015-09-21 11:08:42 UTC
A commit references this bug:

Author: junovitch
Date: Mon Sep 21 11:08:27 UTC 2015
New revision: 397486
URL: https://svnweb.freebsd.org/changeset/ports/397486

Log:
  MFH r391555, r392222, r393602, r396106, r396185, r397215, r397476

  r391555
  www/squid: Support DragonFly SHM segments

  Out of the box, squid would not run on dragonfly due to its handling
  of SHM segments.  On DragonFly, SHM segments are always treated as files
  but on FreeBSD it depends on whether or not application is inside a jail.

  In any case, the case for DragonFly was no supported, so it has been
  added via patch.  This also requires the return of /var/run/squid
  directory which is where the SHM files are stored (defined by
  localstatedir and supported by RC script).  The RC script would define
  this directory if missing, but let's make sure it is always available.

  PR:		201405
  Submitted by:	marino
  Approved by:	maintainer (timp87/gmail)

  r392222
  www/squid: pkg-list fix

  - add missing pkg-plist entry (SSL_CRTD option)

  PR:                  201463
  Submitted by:        s3erios@gmail.com
  Approved by:         timp87@gmail.com (maintainer)

  r393602
  www/squid: update 3.5.6 -> 3.5.7

  - Fix build with ecap by clang
  - Get rid of useless and always empty /var/squid/logs
  - Rework patches to make portlint a bit happier

  PR:		202053
  Submitted by:	Pavel Timofeev <timp87@gmail.com> (maintainer)

  r396106
  www/squid: update 3.5.7 -> 3.5.8

  PR:		202826
  Submitted by:	Pavel Timofeev <timp87@gmail.com> (maintainer)
  Approved by:	feld (mentor)

  r396185
  Fix TP_IPF build.

  r397215
  Rather than produce a warning message that IPv6 is not supported
  under ipfilter 4 (FreeBSD 9) every tenth time, reduce the message
  to one in a million. This has the effect of displaying the message
  at or shortly after startup with a reminder every blue moon.

  PR:		202950

  r397476
  www/squid: security update and build fix

  - security update 3.5.8 -> 3.5.9 [1]
  - Fix TP_IPF build on FreeBSD 9 [2]

  PR:		203186 [1]
  PR:		202950 [2]
  Approved by:	Pavel Timofeev <timp87@gmail.com> (maintainer) [1]
  Security:	d3a98c2d-5da1-11e5-9909-002590263bf5
  Approved by:	portmgr (erwin)

Changes:
_U  branches/2015Q3/
  branches/2015Q3/www/squid/Makefile
  branches/2015Q3/www/squid/distinfo
  branches/2015Q3/www/squid/files/extra-patch-build-8-9
  branches/2015Q3/www/squid/files/patch-bug4190
  branches/2015Q3/www/squid/files/patch-compat_compat.h
  branches/2015Q3/www/squid/files/patch-compat_shm.cc
  branches/2015Q3/www/squid/files/patch-configure
  branches/2015Q3/www/squid/files/patch-configure_GSSAPI_NONE
  branches/2015Q3/www/squid/files/patch-configure_NIS
  branches/2015Q3/www/squid/files/patch-configure_crypt.h
  branches/2015Q3/www/squid/files/patch-src-cf.data.pre
  branches/2015Q3/www/squid/files/patch-src_DiskIO_Mmapped_MmappedFile.cc
  branches/2015Q3/www/squid/files/patch-src__ip__Intercept.cc
  branches/2015Q3/www/squid/files/patch-src_ipc_mem_Segment.cc
  branches/2015Q3/www/squid/files/patch-src_tools.cc
  branches/2015Q3/www/squid/pkg-plist
Comment 12 Jason Unovitch freebsd_committer 2015-09-21 11:17:58 UTC
Pavel,
Can you report the changes here to http://bugs.squid-cache.org/show_bug.cgi?id=4302 so that we can get a Squid 3.5.10 that both compiles properly to start and has a more reasonable amount of logging?

Our changes...
https://svnweb.freebsd.org/ports/head/www/squid/files/patch-src__ip__Intercept.cc?revision=397476&view=markup

Thanks!