Re: pfctl: stricter redirect specs

2014-06-24 Thread Mike Belopuhov
On Tue, Jun 24, 2014 at 15:07 +0200, Mike Belopuhov wrote:
 I have carefully tested that and do not expect any unrelated
 fallout.  And for the reasons stated above I don't believe
 anyone is using this since it's largely error prone.
 

and a regress chunk that avoids using such combination.

note that this diff basically finishes previous attemts at
modifying tests to match present syntax.

diff --git regress/sbin/pfctl/pf48.in regress/sbin/pfctl/pf48.in
index 540711a..a0dd143 100644
--- regress/sbin/pfctl/pf48.in
+++ regress/sbin/pfctl/pf48.in
@@ -1,12 +1,12 @@
 table  regress  { 1.2.3.4 !5.6.7.8 10/8 lo0 }
 table regress.1 const { ::1 fe80::/64 }
 table regress.a { 1.2.3.4 !5.6.7.8 } { ::1 ::2 ::3 } file /dev/null const 
{ 4.3.2.1 }
-match out on lo0 from  regress.1 to regress.2 nat-to lo0:0
+match out on lo0 inet from  regress.1 to regress.2 nat-to lo0:0
 match out on !lo0 inet from !regress.1  to regress.2 nat-to lo0:0
 match in on lo0 inet6 from regress.1 to regress.2 rdr-to lo0:0
-match in on !lo0 from ! regress.1  to regress.2 rdr-to lo0:0
+match in on !lo0 inet6 from ! regress.1  to regress.2 rdr-to lo0:0
 match in from { regress.1 !regress.2 } to any
 match out from any to { !regress.1, regress.2 }
 pass in from regress to any
 pass out from any to regress 
 pass in from { regress.1 regress.2 } to any
diff --git regress/sbin/pfctl/pf48.loaded regress/sbin/pfctl/pf48.loaded
index 68b9854..cfdc8a2 100644
--- regress/sbin/pfctl/pf48.loaded
+++ regress/sbin/pfctl/pf48.loaded
@@ -1,7 +1,7 @@
-@0 match out on lo0 inet6 from regress.1:2 to regress.2:* nat-to ::1
-  [ Skip steps: d=2 p=end da=4 sp=end dp=end ]
+@0 match out on lo0 inet from regress.1:2 to regress.2:* nat-to 127.0.0.1
+  [ Skip steps: d=2 f=2 p=end da=4 sp=end dp=end ]
   [ queue: qname= qid=0 pqname= pqid=0 ]
   [ Evaluations: 0 Packets: 0 Bytes: 0   States: 0 
]
 @1 match out on ! lo0 inet from ! regress.1:2 to regress.2:* nat-to 
127.0.0.1
   [ Skip steps: p=end da=4 sp=end dp=end ]
   [ queue: qname= qid=0 pqname= pqid=0 ]
diff --git regress/sbin/pfctl/pf48.ok regress/sbin/pfctl/pf48.ok
index 5f6e6ab..aeccfcf 100644
--- regress/sbin/pfctl/pf48.ok
+++ regress/sbin/pfctl/pf48.ok
@@ -1,9 +1,9 @@
 table regress { 1.2.3.4 !5.6.7.8 10.0.0.0/8 ::1 fe80::1 127.0.0.1 }
 table regress.1 const { ::1 fe80::/64 }
 table regress.a const { 1.2.3.4 !5.6.7.8 ::1 ::2 ::3 } file /dev/null { 
4.3.2.1 }
-match out on lo0 inet6 from regress.1 to regress.2 nat-to ::1
+match out on lo0 inet from regress.1 to regress.2 nat-to 127.0.0.1
 match out on ! lo0 inet from ! regress.1 to regress.2 nat-to 127.0.0.1
 match in on lo0 inet6 from regress.1 to regress.2 rdr-to ::1
 match in on ! lo0 inet6 from ! regress.1 to regress.2 rdr-to ::1
 match in from regress.1 to any
 match in from ! regress.2 to any
diff --git regress/sbin/pfctl/pf48.optimized regress/sbin/pfctl/pf48.optimized
index 63309a7..90e2036 100644
--- regress/sbin/pfctl/pf48.optimized
+++ regress/sbin/pfctl/pf48.optimized
@@ -1,7 +1,7 @@
-@0 match out on lo0 inet6 from regress.1:2 to regress.2:* nat-to ::1
-  [ Skip steps: d=2 p=end da=4 sp=end dp=end ]
+@0 match out on lo0 inet from regress.1:2 to regress.2:* nat-to 127.0.0.1
+  [ Skip steps: d=2 f=2 p=end da=4 sp=end dp=end ]
   [ queue: qname= qid=0 pqname= pqid=0 ]
   [ Evaluations: 0 Packets: 0 Bytes: 0   States: 0 
]
 @1 match out on ! lo0 inet from ! regress.1:2 to regress.2:* nat-to 
127.0.0.1
   [ Skip steps: p=end da=4 sp=end dp=end ]
   [ queue: qname= qid=0 pqname= pqid=0 ]
diff --git regress/sbin/pfctl/pf98.in regress/sbin/pfctl/pf98.in
index a18a491..a224f9e 100644
--- regress/sbin/pfctl/pf98.in
+++ regress/sbin/pfctl/pf98.in
@@ -1,4 +1,4 @@
 # Test rule order processing should pass (require-order no longer required)
 pass in on lo100 all
-match out on lo0 all nat-to lo0
+match out on lo0 inet6 all nat-to lo0
 



Re: pfctl: stricter redirect specs

2014-06-24 Thread Stuart Henderson
On 2014/06/24 15:07, Mike Belopuhov wrote:
 I propose to avoid the confusion by flagging such situations as
 errors, e.g.:
 
  % echo 'pass out nat-to { ::1 1.1.1.1 }' | ./obj/pfctl -o none -vnf - 
  stdin:1: translation spec contains addresses with different address families
  stdin:1: skipping rule due to errors
  stdin:1: rule expands to no valid combination
 
 While previously it would pick only one of them (the first one):

I agree with this change, it does need a warning in current.html to
avoid users being bitten by it (mostly this is likely when using interface
names e.g. echo 'pass out nat-to lo0' | pfctl -vnf -), however the
recent change to disabling ipv6 by default should reduce the impact of
this.