miniupnp.tuxfamily.org Forum Index miniupnp.tuxfamily.org
The forum about miniupnp and libnatpmp
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

Redundancy in pf rules

 
Post new topic   Reply to topic    miniupnp.tuxfamily.org Forum Index -> miniupnpd Bugs
View previous topic :: View next topic  
Author Message
bcolenj



Joined: 26 Nov 2007
Posts: 1

PostPosted: Mon Nov 26, 2007 3:41 pm    Post subject: Redundancy in pf rules Reply with quote

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
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Mon Nov 26, 2007 4:04 pm    Post subject: Re: Redundancy in pf rules Reply with quote

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
View user's profile Send private message Visit poster's website
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Tue Dec 18, 2007 9:54 am    Post subject: Reply with quote

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
View user's profile Send private message Visit poster's website
arnal



Joined: 26 Dec 2007
Posts: 4

PostPosted: Wed Dec 26, 2007 10:15 am    Post subject: Reply with quote

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
View user's profile Send private message
arnal



Joined: 26 Dec 2007
Posts: 4

PostPosted: Wed Dec 26, 2007 2:11 pm    Post subject: Reply with quote

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 Very Happy

Regards
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Thu Dec 27, 2007 3:20 pm    Post subject: Reply with quote

thanks for that patch !
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
arnal



Joined: 26 Dec 2007
Posts: 4

PostPosted: Fri Dec 28, 2007 10:13 pm    Post subject: Reply with quote

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
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Sun Dec 30, 2007 9:29 pm    Post subject: Reply with quote

could you give the patches given in unified diff format please ? diff -u
Thanks Wink
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
arnal



Joined: 26 Dec 2007
Posts: 4

PostPosted: Mon Dec 31, 2007 9:44 am    Post subject: Reply with quote

#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
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Wed Jan 09, 2008 9:49 am    Post subject: Reply with quote

miniupnpd-1.0-RC13.tar.gz includes your patch. Thanks for it !
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
Display posts from previous:   
Post new topic   Reply to topic    miniupnp.tuxfamily.org Forum Index -> miniupnpd Bugs All times are GMT
Page 1 of 1

 
Jump to:  
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
Protected by Anti-Spam ACP
© 2007 Thomas Bernard, author of MiniUPNP.