Created attachment 148177 [details] patch for adding firewall_myservices_udp to rc.conf Settings firewall_myservices in etc/rc.conf permit to support only TCP services. This patch propose to add UDP services by 2 changes: 1. firewall_myservices became a deprecated alias, the new is firewall_myservices_tcp 2. A new firewall_myservices_udp variable is added. Patch to be apply to an up-to-date -current (tested on r272911).
You seem to have a typo (double initial ff) in line 151 of /etc/defaults/rc.conf: ffirewall_myservices_udp=""
Created attachment 148185 [details] patch for adding firewall_myservices_udp to rc.conf fix the double "f" in etc/default/rc.conf
Take.
Instead of defining two new variables, we could expand firewall_myservices to: firewall_myservices="20 80 443:tcp 3128:udp" Then the code in rc.firewall would become: for j in ${firewall_myservices} port=${j%:*} protocol=${j#*:} # Compatibility for when no protocol is specified, default to TCP if [ "x${protocol}" = "x" ] { protocol="tcp" echo "Consider using ${port}:tcp in firewall_myservices" fi case ${protocol} in [Tt][Cc][Pp]) ;; [Uu][Dd][Pp]) ;; *) echo "Protocol ${port}:${protocol} not supported in firewall_myservices" protocol="unsupported" ;; esac if [ "${protocol}" != "unsupported" ] { ${fwcmd} add pass ${protocol} from $i to me ${port} } done
Created attachment 148266 [details] Updated patch to support both protocols in one variable. (In reply to swhetzel from comment #4) > Instead of defining two new variables, we could expand firewall_myservices > to: > > firewall_myservices="20 80 443:tcp 3128:udp" I agree with this idea, but I prefer 443/tcp over 443:tcp. How about the attached patch?
Comment on attachment 148266 [details] Updated patch to support both protocols in one variable. That looks good too.
Created attachment 148267 [details] Updated patch to support both protocols in one variable. A revised one. The patch 148266 included a typo.
Created attachment 148278 [details] Updated patch to support both protocols in one variable Protocol number support has been added. Now it supports the following: 80 = 80/tcp (backward comaptibility) 80/tcp = "allow tcp from $i to me dst-port 80" 80/udp = "allow udp from $i to me dst-port 80" 47/proto = "allow 47 from $i to me" The port or protocol number can be specified in symbols defined in /etc/services and /etc/protocols.
Wow… your improvements exceeded my expectations: Thanks guys !
I will commit the patch once I get a feedback from Adrian.
Looks great! Thanks!
Comment on attachment 148278 [details] Updated patch to support both protocols in one variable Just noticed that when the port is only specified, it is passing it as udp, should be tcp: + echo "Consider using tcp/$j in firewall_myservices." > /dev/stderr + ${fwcmd} add pass udp from $i to me $j Also the error, message should say ..$j/tcp.. Otherwise it looks good to me.
A commit references this bug: Author: hrs Date: Fri Oct 17 00:31:52 UTC 2014 New revision: 273201 URL: https://svnweb.freebsd.org/changeset/base/273201 Log: Add support of "/{udp,tcp,proto}" suffix into $firewall_myservices, which interpreted the listed items as port numbers of TCP services. A service with no suffix still works and recognized as a TCP service for backward compatibility. It should be updated with /tcp suffix. PR: 194292 MFC after: 1 week Changes: head/etc/rc.firewall
(In reply to Scot Hetzel from comment #12) > Comment on attachment 148278 [details] > Updated patch to support both protocols in one variable > > Just noticed that when the port is only specified, it is passing it as udp, > should be tcp: > > + echo "Consider using tcp/$j in firewall_myservices." > /dev/stderr > + ${fwcmd} add pass udp from $i to me $j > > Also the error, message should say ..$j/tcp.. > > Otherwise it looks good to me. Thank you. I fixed this and committed the patch in r273201.
Close PRs that have had a corresponding fix committed.
No, this has not been MFCed yet. Please keep it open until it is done.
hrs@, can this be closed now?
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved. Thanks