Bug 194292
| Summary: | Patch for adding firewall_myservices_tcp and firewall_myservices_udp support to rc.conf | ||
|---|---|---|---|
| Product: | Base System | Reporter: | olivier |
| Component: | conf | Assignee: | Hiroki Sato <hrs> |
| Status: | Closed FIXED | ||
| Severity: | Affects Some People | CC: | adrian, gonzo, hiren, hrs, info, swhetzel |
| Priority: | --- | Flags: | bugmeister:
mfc-stable10?
bugmeister: mfc-stable9? bugmeister: mfc-stable8? |
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
| Attachments: | |||
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 |
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).