View previous topic :: View next topic |
Author |
Message |
bcolenj
Joined: 26 Nov 2007 Posts: 1
|
Posted: Mon Nov 26, 2007 3:41 pm Post subject: Redundancy in pf rules |
|
|
Code: | $ sudo pfctl -a miniupnpd -s nat
rdr pass on fxp0 inet proto udp from any to any port = 3074 label "Xbox (192.168.1.112:3074) 3074 UDP" -> 192.168.1.112 port 3074
rdr pass on fxp0 inet proto udp from any to any port = 11252 label "Xbox (192.168.1.106:11252) 11252 UDP" -> 192.168.1.106 port 11252
$ sudo pfctl -a miniupnpd -s rules
pass in quick on fxp0 inet proto udp from any to any port = 3074 keep state label "Xbox (192.168.1.112:3074) 3074 UDP"
pass in quick on fxp0 inet proto udp from any to any port = 11252 keep state label "Xbox (192.168.1.106:11252) 11252 UDP" |
From my understanding of pf, when using rdr pass, the packets are immediately passed without traversing through the rest of the rules. This makes the miniupnpd anchor along with the pass rules useless. For pf systems, the whole filter adding functions can be removed along with the need to use the extra anchor miniupnpd. Correct me if I am wrong.
Unless I am wrong, this should be changed as it will reduce the size of the daemon and the number of rules in pf. |
|
Back to top |
|
|
miniupnp Site Admin
Joined: 14 Apr 2007 Posts: 1589
|
Posted: Mon Nov 26, 2007 4:04 pm Post subject: Re: Redundancy in pf rules |
|
|
bcolenj wrote: | Code: | $ sudo pfctl -a miniupnpd -s nat
rdr pass on fxp0 inet proto udp from any to any port = 3074 label "Xbox (192.168.1.112:3074) 3074 UDP" -> 192.168.1.112 port 3074
rdr pass on fxp0 inet proto udp from any to any port = 11252 label "Xbox (192.168.1.106:11252) 11252 UDP" -> 192.168.1.106 port 11252
$ sudo pfctl -a miniupnpd -s rules
pass in quick on fxp0 inet proto udp from any to any port = 3074 keep state label "Xbox (192.168.1.112:3074) 3074 UDP"
pass in quick on fxp0 inet proto udp from any to any port = 11252 keep state label "Xbox (192.168.1.106:11252) 11252 UDP" |
From my understanding of pf, when using rdr pass, the packets are immediately passed without traversing through the rest of the rules. This makes the miniupnpd anchor along with the pass rules useless. For pf systems, the whole filter adding functions can be removed along with the need to use the extra anchor miniupnpd. Correct me if I am wrong.
Unless I am wrong, this should be changed as it will reduce the size of the daemon and the number of rules in pf. |
I think you are right ! I'm going to #ifdef this code as someone may want to to use "rdr" instead of "rdr pass" _________________ Main miniUPnP author.
https://miniupnp.tuxfamily.org/ |
|
Back to top |
|
|
miniupnp Site Admin
Joined: 14 Apr 2007 Posts: 1589
|
Posted: Tue Dec 18, 2007 9:54 am Post subject: |
|
|
since version 1.0-RC12, PF_ENABLE_FILTER_RULES must be defined in order to compile with filter rules code. _________________ Main miniUPnP author.
https://miniupnp.tuxfamily.org/ |
|
Back to top |
|
|
arnal
Joined: 26 Dec 2007 Posts: 4
|
Posted: Wed Dec 26, 2007 10:15 am Post subject: |
|
|
Hi,
I have made a small change to the obsdrdr.c like this if you use PF_ENABLE_FILTER_RULES then the miniupnpd generate the rdr (only not the rdr pass) and the rule, like this you can monitor the activity of the miniupnpd in pftop.
Regards
# diff obsdrdr.c obsdrdr2.c
99c99,103
< pcr.rule.natpass = 1;
---
> #ifndef PF_ENABLE_FILTER_RULES
> pcr.rule.natpass = 1;
> #else
> pcr.rule.natpass = 0;
> #endif
# |
|
Back to top |
|
|
arnal
Joined: 26 Dec 2007 Posts: 4
|
Posted: Wed Dec 26, 2007 2:11 pm Post subject: |
|
|
The diff doesn't work because of ??? so after reading the FAQ of pf here is what I found :
the nat pass statement put a keep state for the pass condition wich implie a S/SA flags.
the diff :
# diff obsdrdr.c obsdrdr.c.old
12d11
< #include <netinet/tcp.h>
100,104c99
< #ifndef PF_ENABLE_FILTER_RULES
< pcr.rule.natpass = 1;
< #else
< pcr.rule.natpass = 0;
< #endif
---
> pcr.rule.natpass = 1;
195,196d189
< pcr.rule.flags = TH_SYN;
< pcr.rule.flagset = (TH_SYN|TH_ACK);
Now I prefer to keep the PF_ENABLE_FILTER_RULES on because I can add more argument to the pass rule than to the rdr pass "rule".
cf. OpenBSD FAQ :
The only exception to this rule is when the pass keyword is used within the rdr rule. In this case, the redirected packets will pass statefully right through the filtering engine: the filter rules won't be evaluated against these packets. This is a handy shortcut to avoid adding pass filter rules for each redirection rule. Think of it as a normal rdr rule (with no pass keyword) associated to a pass filter rule with the keep state keyword. However, if you want to enable more specific filtering options such as synproxy, modulate state, etc. you'll still have to use a dedicate pass rule as these options don't fit into redirection rules.
I want to make some more minor change to the code to enable the ability of having an option like this :
-q queue
Create rules with queue queue appended, so that data connections can be queued.
This will preserve my bandwidth when I make huge download
Regards |
|
Back to top |
|
|
miniupnp Site Admin
Joined: 14 Apr 2007 Posts: 1589
|
|
Back to top |
|
|
arnal
Joined: 26 Dec 2007 Posts: 4
|
Posted: Fri Dec 28, 2007 10:13 pm Post subject: |
|
|
My pleasure.
Chose promise chose dûe :
Here it is the patch against the miniupnpd-20071220 release to add the queue option (I made a miniupnpd-20071228 release) :
It make sense only if you don't use the rdr pass ability so you have to uncomment the PF_ENABLE_FILTER_RULES for it to work and I have to add some #ifndef etc ... to the code ... but here is to you if you want to test.
diff -r miniupnpd-20071228/miniupnpd.1 miniupnpd-20071220/miniupnpd.1
42,44d41
< .B \-q queue
< set ALTQ queue in pf
< .TP
diff -r miniupnpd-20071228/miniupnpd.c miniupnpd-20071220/miniupnpd.c
418,420d417
< case UPNPQUEUE:
< queue = ary_options[i].value;
< break;
484,489d480
< case 'q':
< if(i+1 < argc)
< queue = argv[++i];
< else
< fprintf(stderr, "Option -%c takes one argument.\n", argv[i][1]);
< break;
566c557
< "\t\t[-B down up] [-w url] [-q queue]\n"
---
> "\t\t[-B down up] [-w url]\n"
572d562
< "\t-q sets the ALTQ queue in pf.\n"
diff -r miniupnpd-20071228/miniupnpd.conf miniupnpd-20071220/miniupnpd.conf
21,23d20
< # ALTQ queue in OpenBSD
< #queue=q_def
<
diff -r miniupnpd-20071228/options.c miniupnpd-20071220/options.c
39,40c39
< { UPNPCLEANINTERVAL, "clean_ruleset_interval"},
< { UPNPQUEUE, "queue"}
---
> { UPNPCLEANINTERVAL, "clean_ruleset_interval"}
diff -r miniupnpd-20071228/options.h miniupnpd-20071220/options.h
29,30c29
< UPNPCLEANINTERVAL, /* clean_ruleset_interval */
< UPNPQUEUE /* queue */
---
> UPNPCLEANINTERVAL /* clean_ruleset_interval */
diff -r miniupnpd-20071228/pf/obsdrdr.c miniupnpd-20071220/pf/obsdrdr.c
12d11
< #include <netinet/tcp.h>
100,104c99
< #ifndef PF_ENABLE_FILTER_RULES
< pcr.rule.natpass = 1;
< #else
< pcr.rule.natpass = 0;
< #endif
---
> pcr.rule.natpass = 1;
155c150
< int proto, const char * desc, const char * queue)
---
> int proto, const char * desc)
195,196d189
< pcr.rule.flags = TH_SYN;
< pcr.rule.flagset = (TH_SYN|TH_ACK);
202,203d194
< if (queue != NULL)
< strlcpy(pcr.rule.qname, queue, sizeof pcr.rule.qname);
diff -r miniupnpd-20071228/pf/obsdrdr.h miniupnpd-20071220/pf/obsdrdr.h
27c27
< int proto, const char * desc, const char * queue);
---
> int proto, const char * desc);
diff -r miniupnpd-20071228/upnpglobalvars.c miniupnpd-20071220/upnpglobalvars.c
16,18d15
< /* queue */
< const char * queue = 0;
<
diff -r miniupnpd-20071228/upnpglobalvars.h miniupnpd-20071220/upnpglobalvars.h
17,19d16
< /* queue */
< const char * queue;
<
diff -r miniupnpd-20071228/upnpredirect.c miniupnpd-20071220/upnpredirect.c
108c108
< if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc, queue) < 0)
---
> if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc) < 0)
136c136
< if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc, queue) < 0)
---
> if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc) < 0)
This add the option -q queue or in config file queue=queue_name for OpenBSD
Tested against my OpenBSD 4.2 snapshot of the 07/12/2007 |
|
Back to top |
|
|
miniupnp Site Admin
Joined: 14 Apr 2007 Posts: 1589
|
Posted: Sun Dec 30, 2007 9:29 pm Post subject: |
|
|
could you give the patches given in unified diff format please ? diff -u
Thanks _________________ Main miniUPnP author.
https://miniupnp.tuxfamily.org/ |
|
Back to top |
|
|
arnal
Joined: 26 Dec 2007 Posts: 4
|
Posted: Mon Dec 31, 2007 9:44 am Post subject: |
|
|
#diff -u miniupnpd-20071228 miniupnpd-20071220 > patchMiniUPnPd
# cat patchMiniUPnPd
Common subdirectories: miniupnpd-20071228/bsd and miniupnpd-20071220/bsd
Common subdirectories: miniupnpd-20071228/ipf and miniupnpd-20071220/ipf
Common subdirectories: miniupnpd-20071228/linux and miniupnpd-20071220/linux
diff -u miniupnpd-20071228/miniupnpd.1 miniupnpd-20071220/miniupnpd.1
--- miniupnpd-20071228/miniupnpd.1 Thu Dec 20 16:07:58 2007
+++ miniupnpd-20071220/miniupnpd.1 Mon Oct 8 19:25:15 2007
@@ -39,9 +39,6 @@
.B \-L
set packet log in pf on
.TP
-.B \-q queue
-set ALTQ queue in pf
-.TP
.B \-U
report system uptime instead of daemon uptime to clients.
.TP
diff -u miniupnpd-20071228/miniupnpd.c miniupnpd-20071220/miniupnpd.c
--- miniupnpd-20071228/miniupnpd.c Thu Dec 20 16:06:22 2007
+++ miniupnpd-20071220/miniupnpd.c Thu Dec 13 17:36:51 2007
@@ -415,9 +415,6 @@
case UPNPCLEANINTERVAL:
v->clean_ruleset_interval = atoi(ary_options[i].value);
break;
- case UPNPQUEUE:
- queue = ary_options[i].value;
- break;
default:
fprintf(stderr, "Unknown option in file %s\n",
optionsfile);
@@ -481,12 +478,6 @@
else
fprintf(stderr, "Option -%c takes one argument.\n", argv[i][1]);
break;
- case 'q':
- if(i+1 < argc)
- queue = argv[++i];
- else
- fprintf(stderr, "Option -%c takes one argument.\n", argv[i][1]);
- break;
case 'p':
if(i+1 < argc)
v->port = atoi(argv[++i]);
@@ -563,13 +554,12 @@
/*"[-l logfile] " not functionnal */
"\t\t[-u uuid] [-s serial] [-m model_number] \n"
"\t\t[-t notify_interval] [-P pid_filename]\n"
- "\t\t[-B down up] [-w url] [-q queue]\n"
+ "\t\t[-B down up] [-w url]\n"
"\nNotes:\n\tThere can be one or several listening_ips.\n"
"\tNotify interval is in seconds. Default is 30 seconds.\n"
"\tDefault pid file is %s.\n"
"\tWith -d miniupnpd will run as a standard program.\n"
"\t-L sets packet log in pf on.\n"
- "\t-q sets the ALTQ queue in pf.\n"
"\t-U causes miniupnpd to report system uptime instead "
"of daemon uptime.\n"
"\t-B sets bitrates reported by daemon in bits per second.\n"
diff -u miniupnpd-20071228/miniupnpd.conf miniupnpd-20071220/miniupnpd.conf
--- miniupnpd-20071228/miniupnpd.conf Thu Dec 20 16:02:58 2007
+++ miniupnpd-20071220/miniupnpd.conf Thu Oct 25 14:51:30 2007
@@ -18,9 +18,6 @@
bitrate_up=1000000
bitrate_down=10000000
-# ALTQ queue in OpenBSD
-#queue=q_def
-
# default presentation url is http address on port 80
#presentation_url=http://www.mylan/index.php
Common subdirectories: miniupnpd-20071228/netfilter and miniupnpd-20071220/netfilter
diff -u miniupnpd-20071228/options.c miniupnpd-20071220/options.c
--- miniupnpd-20071228/options.c Thu Dec 20 15:21:07 2007
+++ miniupnpd-20071220/options.c Mon Sep 24 19:15:57 2007
@@ -36,8 +36,7 @@
{ UPNPSERIAL, "serial"},
{ UPNPMODEL_NUMBER, "model_number"},
{ UPNPCLEANTHRESHOLD, "clean_ruleset_threshold"},
- { UPNPCLEANINTERVAL, "clean_ruleset_interval"},
- { UPNPQUEUE, "queue"}
+ { UPNPCLEANINTERVAL, "clean_ruleset_interval"}
};
int
diff -u miniupnpd-20071228/options.h miniupnpd-20071220/options.h
--- miniupnpd-20071228/options.h Thu Dec 20 15:21:13 2007
+++ miniupnpd-20071220/options.h Mon Sep 24 19:15:57 2007
@@ -26,8 +26,7 @@
UPNPSERIAL, /* serial */
UPNPMODEL_NUMBER, /* model_number */
UPNPCLEANTHRESHOLD, /* clean_ruleset_threshold */
- UPNPCLEANINTERVAL, /* clean_ruleset_interval */
- UPNPQUEUE /* queue */
+ UPNPCLEANINTERVAL /* clean_ruleset_interval */
};
/* readoptionsfile()
Common subdirectories: miniupnpd-20071228/pf and miniupnpd-20071220/pf
Common subdirectories: miniupnpd-20071228/solaris and miniupnpd-20071220/solaris
diff -u miniupnpd-20071228/upnpglobalvars.c miniupnpd-20071220/upnpglobalvars.c
--- miniupnpd-20071228/upnpglobalvars.c Thu Dec 20 15:21:45 2007
+++ miniupnpd-20071220/upnpglobalvars.c Wed Dec 5 01:03:58 2007
@@ -13,9 +13,6 @@
/* network interface for internet */
const char * ext_if_name = 0;
-/* queue */
-const char * queue = 0;
-
/* forced ip address to use for this interface
* when NULL, getifaddr() is used */
const char * use_ext_ip_addr = 0;
diff -u miniupnpd-20071228/upnpglobalvars.h miniupnpd-20071220/upnpglobalvars.h
--- miniupnpd-20071228/upnpglobalvars.h Thu Dec 20 15:25:47 2007
+++ miniupnpd-20071220/upnpglobalvars.h Wed Dec 5 01:03:58 2007
@@ -14,9 +14,6 @@
/* name of the network interface used to acces internet */
extern const char * ext_if_name;
-/* queue */
-const char * queue;
-
/* forced ip address to use for this interface
* when NULL, getifaddr() is used */
extern const char * use_ext_ip_addr;
diff -u miniupnpd-20071228/upnpredirect.c miniupnpd-20071220/upnpredirect.c
--- miniupnpd-20071228/upnpredirect.c Thu Dec 20 15:22:05 2007
+++ miniupnpd-20071220/upnpredirect.c Fri Nov 2 23:58:04 2007
@@ -105,7 +105,7 @@
syslog(LOG_INFO, "creating pass rule to %s:%hu protocol %s for: %s",
iaddr, iport, protocol, desc);
- if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc, queue) < 0)
+ if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc) < 0)
{
/* clean up the redirect rule */
#if !defined(__linux__)
@@ -133,7 +133,7 @@
/* syslog(LOG_INFO, "creating pass rule to %s:%hu protocol %s for: %s",
iaddr, iport, protocol, desc);*/
- if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc, queue) < 0)
+ if(add_filter_rule2(ext_if_name, iaddr, eport, iport, proto, desc) < 0)
{
/* clean up the redirect rule */
#if !defined(__linux__)
# |
|
Back to top |
|
|
miniupnp Site Admin
Joined: 14 Apr 2007 Posts: 1589
|
|
Back to top |
|
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
© 2007 Thomas Bernard, author of MiniUPNP.
|