Bug 235879
| Summary: | www/e2guardian: update to 5.3.1 | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | marcellocoutinho | ||||||||||||||||||||||||||||
| Component: | Individual Port(s) | Assignee: | Renato Botelho <garga> | ||||||||||||||||||||||||||||
| Status: | Closed FIXED | ||||||||||||||||||||||||||||||
| Severity: | Affects Only Me | CC: | garga, marcellocoutinho, w.schwarzenfeld | ||||||||||||||||||||||||||||
| Priority: | --- | Flags: | marcellocoutinho:
maintainer-feedback+
|
||||||||||||||||||||||||||||
| Version: | Latest | ||||||||||||||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||||||||||||||
| OS: | Any | ||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||
Comment on attachment 202182 [details]
e2guardian 5.3.1 port update
Can you please submit unified diffs for port updates? They are
easier to review and work with than shell archives for updates since
they show what has actually changed in the port.
+DEFAULT_VERSIONS+=ssl=openssl
This is wrong. Ports can never set DEFAULT_VERSIONS. DEFAULT_VERSIONS
is a global setting. What version of SSL to use is for users to
decide. It's not up to individual ports.
What problem does this solve?
+GH_ACCOUNT= e2guardian
Remove this. It's set to the default value.
+PORTVERSION= 5.3
...
+GH_TAGNAME= v5.3.1
Why not actually set PORTVERSION=5.3.1 ?
Waiting for unified diff with fixes Removed the ssl default option from makefile and GH_ACCOUNT as its the same name as the package name. The GH_TAGNAME is specified because 5.3.1 is a tag, not a branch on e2guardian repo Created attachment 202425 [details]
e2guardian 5.3.1 port update after changes pointed
(In reply to marcellocoutinho from comment #3) Please submit a unified diff instead of a shar file. Created attachment 202426 [details]
e2guardian 5.3.1 diff port update
(In reply to marcellocoutinho from comment #6) The best way to create a diff to submit is using subversion. Regarding the patch I have a few comments: - PORTVERSION is 5.3 but GH_TAGNAME is v5.3.1. You should use PORTVERSION=5.3.1 then you can delete GH_TAGNAME line - Keep USES list alphabetically sorted Created attachment 202443 [details]
e2guardian 5.3.1 diff port update
(In reply to marcellocoutinho from comment #8) - You added 'iconv' to general USES, please remove it from NTLM_USES because now it's redundant. - You added 'ssl' to general USES, please remove SSL from options since it's now mandatory Port is broken when all OPTIONS are UNSET Making install in src /bin/mkdir -p '/wrkdirs/usr/ports/www/e2guardian/work/stage/usr/local/sbin' install -s -m 555 e2guardian '/wrkdirs/usr/ports/www/e2guardian/work/stage/usr/local/sbin' mv: rename /wrkdirs/usr/ports/www/e2guardian/work/stage/usr/local/etc/e2guardian/lists/exceptionvirusextensionlist.sample to /wrkdirs/usr/ports/www/e2guardian/work/stage/usr/local/etc/e2guardian/lists/contentscanners/: No such file or directory *** Error code 1 Stop. make: stopped in /usr/ports/www/e2guardian build of www/e2guardian | e2guardian-5.3.1 ended at Thu Feb 28 13:07:10 -03 2019 build time: 00:00:43 !!! build failure encountered !!! [00:00:58] Error: Build failed in phase: stage [00:00:58] Cleaning up [00:00:58] Unmounting file systems And it's broken with all OPTIONS SET =========================================================================== ====> Running Q/A tests (stage-qa) ====> Checking for pkg-plist issues (check-plist) ===> Parsing plist ===> Checking for items in STAGEDIR missing from pkg-plist Error: Orphaned: %%ETCDIR%%/contentscanners/avastdscan.conf.sample ===> Checking for items in pkg-plist which are not in STAGEDIR Error: Missing: %%ETCDIR%%/contentscanners/avastd.conf.sample ===> Error: Plist issues found. *** Error code 1 without ssl in options it does not compile Warning: you need USES=ssl Error: /usr/local/sbin/e2guardian is linked to /usr/local/lib/libssl.so.9 from security/openssl but it is not declared as a dependency Warning: you need USES=ssl Error: /usr/local/sbin/e2guardian is linked to /usr/local/lib/libcrypto.so.9 from security/openssl but it is not declared as a dependency Warning: you need USES=ssl (In reply to marcellocoutinho from comment #12) Yes, I understand SSL is now mandatory. But in this case the port doesn't need to have an option called SSL in OPTIONS_DEFINE Created attachment 202445 [details]
e2guardian 5.3.1 diff port update
(In reply to marcellocoutinho from comment #14) With all OPTIONS set and using AVASTD on AV RADIO pkg-plist is still broken ===> Checking for items in STAGEDIR missing from pkg-plist Error: Orphaned: %%ETCDIR%%/contentscanners/avastdscan.conf.sample ===> Checking for items in pkg-plist which are not in STAGEDIR Error: Missing: %%ETCDIR%%/contentscanners/avastd.conf.sample ===> Error: Plist issues found. configure also complains about unrecognized options: configure: WARNING: unrecognized options: --enable-dgdebug, --enable-trickledm Please test it with all options disabled, with all options enabled, and with 3 possible AV options You should note on `./configure --help` output that it doesn't use that notation of --enable-foo versus --disable-foo, instead they have --enable-foo=yes or --enable-foo=no.
and some options are --with-foo=on --with-foo=off
To make ports options to work with that you will need to use
${OPTION_NAME}_CONFIGURE_ON= --enable-foo
${OPTION_NAME}_CONFIGURE_OFF= --enable-foo=no
${OPTION_NAME}_CONFIGURE_ON= --with-foo
${OPTION_NAME}_CONFIGURE_OFF= --with-foo=off
Also note that configure output doesn't mention the trickledm option, it's only mentioned on INSTALL file so you need to remove it because it will have no effect.
---------------------------------------
Optional Features:
--disable-option-checking ignore unrecognized --enable/--with options
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
--enable-silent-rules less verbose build output (undo: "make V=1")
--disable-silent-rules verbose build output (undo: "make V=0")
--enable-dependency-tracking
do not reject slow dependency extractors
--disable-dependency-tracking
speeds up one-time build
--enable-static-zlib[=no]
Enable static linking of zlib
--enable-pcre[=yes] Enable support for the PCRE library
--enable-lfs[=yes] Enable large file support on 32 bit systems
--enable-orig-ip[=no] Enable support for checking the client's original
destination IP address against HTTP request details
when deployed as a transparent proxy (US-CERT
VU#435052). Currently only works on Linux.
--enable-clamd[=no] Enable support for the ClamD content scanner
--enable-avastd[=no] Enable support for the AvastD content scanner
--enable-icap[=no] Enable support for ICAP AV server content scanner
--enable-kavd[=no] Enable support for the Kaspersky AV daemon content
scanner
--enable-commandline[=no]
Enable support for command-line content scanners
--enable-ntlm[=no] Enable support for the NTLM auth plugin
--enable-sslmitm[=no] Enable support for the SSL MITM plugin
--enable-dnsauth[=no] Enable support for the DNS auth plugin
--enable-email[=no] Enable support for email reporting functionality
Optional Packages:
--with-PACKAGE[=ARG] use PACKAGE [ARG=yes]
--without-PACKAGE do not use PACKAGE (same as --with-PACKAGE=no)
--with-zlib[=NONE] non-standard search path for zlib library
--with-dgdebug[=off] switch on debug build mode
--with-newdebug[=off] switch off debuglevel build mode
--with-netdebug[=off] switch on netdebug build mode
--with-redebug[=off] switch on redebug build mode
--with-proxyuser[=nobody]
name of proxy user
--with-proxygroup[=nobody]
name of proxy group
--with-piddir[=${localstatedir}/run]
path for pid file
--with-logdir[=${localstatedir}/log/${PACKAGE_NAME}]
path for log files
--with-libiconv[=NONE] Specify search path on a system which requires an
external iconv library (only used in conjunction
with NTLM auth plugin).
--with-sysconfsubdir[=e2guardian]
subdirectory under sysconfdir in which to place
config files
(In reply to Renato Botelho from comment #16) also note what it says about iconv option --with-libiconv[=NONE] Specify search path on a system which requires an external iconv library (only used in conjunction with NTLM auth plugin). so the way it is today, adding iconv as a dependency only when NTLM option is selected makes sense. Probably you just need to rework the way options define configure parameters and it will be good. (In reply to Renato Botelho from comment #16) just ignore my --enable/--disable comment. I was wrong, sorry Created attachment 202447 [details]
e2guardian 5.3.1 patch
Here is a patch that passes all tests on amd64 for your approval. I'll test other archs now
Comment on attachment 202447 [details]
e2guardian 5.3.1 patch
replacing
SSL_LDFLAGS= -lssl -lcrypto
SSL_CFLAGS= -D__SSLMITM -D__SSLCERT
with
SSL_CONFIGURE_ENV= OPENSSL_LIBS="-lssl -lcrypto"
OPENSSL_CFLAGS="-I${OPENSSLINC} -L${OPENSSLLIB}"
prevents ssl interception compiling
I get this message at the end of make messages:
Warning: you may not need USES=ssl
Also, the patch is not including Tricke download manager
OPTIONS_DEFINE= CLISCAN ICAP NTLM DNS EMAIL DEBUG DOCS SSL TRICKLE
OPTIONS_DEFAULT=CLAMD DNS TRICKLE
TRICKLE_DESC= Enable the trickle download manager
TRICKLE_CONFIGURE_ENABLE= trickledm
Created attachment 202454 [details]
e2guardian 5.3.1 port update
Take a look on this one. It includes garga's and mine as well.
(In reply to marcellocoutinho from comment #20) Regarding SSL MITM what kind of issues did you see? Could you attach a full log? On my local tests it works perfectly Configure says: checking for SSL MITM support... yes checking for OPENSSL... yes Regarding Tricke download manager, that parameter --enable-trickledm is not valid. There is no mention of the word trickle on configure script: ❯ grep -i trickle configure ❯ ./configure --help |& grep -i trickle ❯ And it makes it an invalid option. I can't get the reason to add it this way. I've also checked their repo and based on these 2 commits fancy and trickle download managers were disabled: https://github.com/e2guardian/e2guardian/commit/3f0b8ebfa8184dba21f7e65858762f9b427ce76a#diff-67e997bcfdac55191033d57a16d1408a https://github.com/e2guardian/e2guardian/commit/ca543af883f253c51e620c6ddaf24f582d10b62d#diff-67e997bcfdac55191033d57a16d1408a Created attachment 202460 [details] make output with ssl_config_env options on makefile (In reply to Renato Botelho from comment #22) this is the make output with ssl_config_env options you added and the alert at the end about ssl. I've tested removing the ssl_use = ssl and ssl selected on make config. It compiles without ssl as it does not complain about missing ssl options. Created attachment 202461 [details] make output with current compile options I use since e2guardian v3 (In reply to Renato Botelho from comment #22) make output with current compile options I use since e2guardian v3 Created attachment 202462 [details] e2guardian 5.3.1 port update another try (In reply to Renato Botelho from comment #22) Created attachment 202463 [details] e2guardian 5.3.1 port update another try (In reply to Renato Botelho from comment #22) (In reply to marcellocoutinho from comment #23) Both outputs you sent have this on configure: checking for SSL MITM support... no Are you sure you have the following line defined? SSL_CONFIGURE_ENABLE=sslmitm When it's defined, --enable-sslmitm will be passed to configure script and it will end up adding the same defines current version of the port are doing now. Can you please send the output of following commands on the port with my patch applied, in the environment you are able to reproduce a problem? # uname -a # cat Makefile # make WITH=SSL -V CONFIGURE_ARGS # make WITH=SSL -V CONFIGURE_ENV # make WITH=SSL -V configure Then attach the output of these commands and also ./work/e2guardian-5.3.1/config.log I'm just trying to figure out if upstream code has a bug in this regard, and if so I would like to submit them a patch. And just a side note, I tested that patch on FreeBSD 12 (i386 and amd64), 11 (amd64) and all of them worked as expected detecting OpenSSL, linking binary with libssl and libcrypto. Created attachment 202481 [details]
5.3.1 final patch
Final patch after a private discussion with Marcello. It also changes SSL option name to SSL_MITM since it makes more sense.
The problem on Marcello's test was a typo on OPTION configure clause that made it to end up missing a configure parameter
A commit references this bug: Author: garga Date: Fri Mar 1 16:14:14 UTC 2019 New revision: 494302 URL: https://svnweb.freebsd.org/changeset/ports/494302 Log: www/e2guardian: Update to 5.3.1 - Update to 5.3.1 - Create a RADIO option for AV items - Rename SSL option to SSL_MITM to reflect what it really does - Fix DEBUG option to pass expected parameter to configure - Use more standard way to enable SSL_MITM - Fix install when no AV options are selected PR: 235879 Submitted by: Marcello Coutinho <marcellocoutinho@gmail.com> (based on) Approved by: Marcello Coutinho <marcellocoutinho@gmail.com> (maintainer) Changes: head/www/e2guardian/Makefile head/www/e2guardian/distinfo head/www/e2guardian/pkg-plist Hello.
When I try to build e2guardian5.3.1 without debug option, it builds with debug support.
Details available below:
/usr/ports/www/e2guardian % make showconfig
===> The following configuration options are available for e2guardian-5.3.1:
CLISCAN=off: Enable support for CLI content scanners
DEBUG=off: Enable debug build mode
DNS=on: Include DNS authetication plugin
DOCS=on: Build and/or install documentation
EMAIL=off: Enable e-mail reporting support
ICAP=off: Enable ICAP AV content scanner support
NTLM=off: Include NTLM authentication plugin
SSL_MITM=off: Enable support for the SSL MITM plugin
====> Options available for the radio AV: you can only select none or one of them
AVAST=off: Enable AvastD content scanner
CLAMD=on: Enable ClamD AV content scanner
KAV=off: Enable Kaspersky AV support
===> Use 'make config' to modify these settings
When I run:
% /usr/local/sbin/e2guardian
Running in debug mode...
...
Reopening due to regression report Created attachment 202686 [details]
e2guardian fix debug
configure script doesn't deal with --without-dgdebug, it needs --with=dgdebug=off to really disable it. While here also disable --with-newdebug when option is not selected.
A commit references this bug: Author: garga Date: Thu Mar 7 15:45:58 UTC 2019 New revision: 494959 URL: https://svnweb.freebsd.org/changeset/ports/494959 Log: www/e2guardian: Fix DEBUG option e2guardian configure script doesn't respect --without-dgdebug option and end up building binary with debug support since it was upgraded to 5.3.1. Pass --with-dgdebug=off explicit when DEBUG option is not set. While here, include --with-newdebug parameter to make sure all debug will be disabled. PR: 235879 Approved by: Marcello Coutinho <marcellocoutinho@gmail.com> (maintainer) Sponsored by: Rubicon Communications, LLC (Netgate) Changes: head/www/e2guardian/Makefile Fixed in 5.3.1_1 |
Created attachment 202182 [details] e2guardian 5.3.1 port update e2guardian 5.3.1 port update