Bug 204334 - security/fwknop: Add missing configuration options
Summary: security/fwknop: Add missing configuration options
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Thomas Zander
URL:
Keywords: easy, patch, patch-ready
: 204335 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-11-06 15:26 UTC by Jens Grassel
Modified: 2015-11-23 06:47 UTC (History)
4 users (show)

See Also:
koobs: maintainer-feedback+
riggs: merge-quarterly-


Attachments
Patch file for the Port (750 bytes, patch)
2015-11-06 15:26 UTC, Jens Grassel
no flags Details | Diff
Updated patch file. (750 bytes, patch)
2015-11-09 15:13 UTC, Jens Grassel
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jens Grassel 2015-11-06 15:26:08 UTC
Created attachment 162854 [details]
Patch file for the Port

Hi,

I added options to Makefile to be able to actually choose the firewall implementation that should be used by fwknop. Currently only IPFW and PF are possible.

Regards,

Jens
Comment 1 Jens Grassel 2015-11-06 15:27:35 UTC
*** Bug 204335 has been marked as a duplicate of this bug. ***
Comment 2 sean.greven 2015-11-09 14:49:33 UTC
Comment on attachment 162854 [details]
Patch file for the Port

should 
+PW_CONFIGURE_WITH=	pf=/sbin/pfctl
not read
+PF_CONFIGURE_WITH=	pf=/sbin/pfctl

?
Comment 3 Jens Grassel 2015-11-09 15:12:02 UTC
Yes, you're correct. :-)
Comment 4 Jens Grassel 2015-11-09 15:13:13 UTC
Created attachment 162924 [details]
Updated patch file.

The options for PF were wrong (PW instead of PF).
Comment 5 sean.greven 2015-11-09 20:05:36 UTC
Patch Applies cleanly.
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-10 07:54:56 UTC
@Jens, please mark "Obsolete" the patch you don't want to be referenced. You can do by clicking "Attachment Details -> Edit Details -> [X] Obsolete"

Please also confirm this port passes QA (portlint/poudriere)

@Sean, if/when you're happy with the change, please set maintainer-approval to + on the attachment you approve, or add a comment "Approved attachment <id>"

Thanks!
Comment 7 Jens Grassel 2015-11-10 07:56:21 UTC
I have the patched port running on two servers using a PF firewall without problems. :-)
Comment 8 Jens Grassel 2015-11-10 07:58:51 UTC
Portlint gives one warning:

WARN: Makefile: PF is listed in OPTIONS_DEFINE, but no PORT_OPTIONS:MPF appears.
0 fatal errors and 1 warning found.

But according to the porters handbook PORT_OPTIONS is deprecated / not recommended?
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-10 08:06:28 UTC
(In reply to Jens Grassel from comment #8)

Deprecated/Undesirable only where a relevant options "helper" can be made to replace it. If the porters handbook needs updating to be a bit less ambiguous/unequivocal, let us know in a new documentation issue :)

For Example:

.if ${PORT_OPTIONS:MFOO}
CONFIGURE_ARGS+=--enable-foo
.endif

Can be turned in into:

FOO_CONFIGURE_ENABLE=foo

If you already have some FOO_* options helpers, that's fine, an portlint may need to be taught to detect <OPT>_* options helpers to avoid the false positive (portlint maintainer cc'd).

Regarding run-time test/QA confirmation, that's great, but there can also be subtle packaging issues that don't show up until deinstall/upgrade/etc time :)

If you don't have poudriere available (highly preferable), you may include (as an attachment), the output of the following instead:

make stage && make check-plist && make stage-qa && make package
Comment 10 Jens Grassel 2015-11-10 08:15:54 UTC
Thanks for the info. I have the appropriate options helpers. =)

Here is the output of the make commands:

====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for items in pkg-plist which are not in STAGEDIR
===> No pkg-plist issues found (check-plist)
====> Running Q/A tests (stage-qa)

Although I've come to the conclusion that the config files should be moved from foo.conf to foo.conf.sample to avoid overriding custom stuff. But that'll be another patch.
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-10 08:21:34 UTC
Great work Sean!
Comment 12 sean.greven 2015-11-10 10:36:54 UTC
Comment on attachment 162924 [details]
Updated patch file.

Approved attachmen
Comment 13 Thomas Zander freebsd_committer freebsd_triage 2015-11-23 06:06:55 UTC
No MFH, since PORTVERSION has changed on head since 2015Q4 was branched.
Comment 14 commit-hook freebsd_committer freebsd_triage 2015-11-23 06:21:08 UTC
A commit references this bug:

Author: riggs
Date: Mon Nov 23 06:20:46 UTC 2015
New revision: 402259
URL: https://svnweb.freebsd.org/changeset/ports/402259

Log:
  Allow to select pf instead of default ipfw for firewall backend

  PR:		204334
  Submitted by:	jan0sch@mykolab.com
  Reviewed by:	sean.greven@gmail.com (maintainer)

Changes:
  head/security/fwknop/Makefile