Re: pf(4) man page: include missing header in example program

2012-08-31 Thread Ted Unangst
On Thu, May 05, 2011 at 23:56, Stuart Henderson wrote:

>> > That change is correct but I'm not sure about keeping this example
>> > code at all. We've had divert-to since OpenBSD 4.4 - when this is used
>> > instead of rdr-to the destination address is preserved, so it can be
>> > fetched with getsockname() without the DIOCNATLOOK dance.

> looking through the ports tree, there are two occasions where
> DIOCNATLOOK is used that can't be replaced with divert-to/getsockname:
> ftpsesame, which needs to lookup addresses gleaned from BPF captured
> connections, and oidentd which needs to lookup in response to ident
> requests. does anyone think it's worth keeping the example for cases
> like these? (personally I don't, but could be persuaded otherwise
> if people feel strongly about it).

Sorry I'm late to the party.  Can we put the DIOCNATLOOK example back?
It was added like 900 years ago in response to a question I asked
dhartmei, and I actually still use it.  I just read the man page again
expecting to find it.  In my case I've never used rdr-to or divert-to,
just plain nat, doing stuff like what oidentd does.



Re: pf(4) man page: include missing header in example program

2011-05-05 Thread Lawrence Teo
On Thu, May 05, 2011 at 11:56:24PM +0100, Stuart Henderson wrote:
> On 2011/05/03 19:51, Mike Belopuhov wrote:
> > On Tue, May 3, 2011 at 9:56 AM, Stuart Henderson  
> > wrote:
> > > On 2011/05/02 22:28, Lawrence Teo wrote:
> > >> The DIOCNATLOOK example program at the end of the pf(4) man page
> > >> uses memset(3), but string.h is not included. The following diff
> > >> fixes this. Any thoughts?
> > >
> > > That change is correct but I'm not sure about keeping this example
> > > code at all. We've had divert-to since OpenBSD 4.4 - when this is used
> > > instead of rdr-to the destination address is preserved, so it can be
> > > fetched with getsockname() without the DIOCNATLOOK dance.
> > >
> > > As a result the code becomes much less complicated, so we don't
> > > really need an example any more, also another big advantage is that
> > > there's no need for access to the privileged /dev/pf device.
> > >
> > > How about this?
> > >
> > 
> > i'm in favor of this change, so ok mikeb
> > 
> 
> slightly tweaked after knowledge gained from converting tftp-proxy;
> specifically, mention the socket options that will be needed for udp
> as they're not easy to track down.
> 
>  This was primarily used to support transparent proxies with rdr-
>  to rules.  New proxies should use divert-to rules instead.  These
>  do not require access to the privileged /dev/pf device and
>  preserve the original destination address for getsockname(2).
>  For SOCK_DGRAM sockets, the ip(4) socket options IP_RECVDSTADDR
>  and IP_RECVDSTPORT can be used to retrieve destination address
>  and port.
> 
> jmc asked about changing the example; if we replace it with a
> code fragment using getsockname it's no longer specific to pf(4),
> and the code is so much simpler than DIOCNATLOOK that IMO a
> sample isn't really needed for this.
> 
> however we could probably usefully add a line or two to
> getsockname(4) explaining its use with divert-to.
> 
> looking through the ports tree, there are two occasions where
> DIOCNATLOOK is used that can't be replaced with divert-to/getsockname:
> ftpsesame, which needs to lookup addresses gleaned from BPF captured
> connections, and oidentd which needs to lookup in response to ident
> requests. does anyone think it's worth keeping the example for cases
> like these? (personally I don't, but could be persuaded otherwise
> if people feel strongly about it).

Way back when I was a new OpenBSD user, I interpreted the
DIOCNATLOOK example to be a simple generic program that demonstrates
how to use PF ioctl commands as a whole. The man page with the
example was very helpful to me as a beginner's guide that shows
"here's the list of PF ioctl commands and their data structures,"
and ends with "and here's an example of how to use one of the PF
ioctl commands."

Since there are now better alternatives to DIOCNATLOOK in most cases
as you pointed out, what do you think about replacing the example
program with another simple program that does a different ioctl
command? Two candidates could be DIOCGETLIMIT or DIOCGETSTATUS;
basically, calls that new users could try without changing or
harming their system. Those two commands will also always work,
since they do not require the user's PF rules to include non-default
features like tables or queues.

I have included a diff that merges your new paragraph with an
example that uses DIOCGETLIMIT instead of DIOCNATLOOK. I also added
a .Pp right before your text so that there is a blank line between
the pfioc_natlook struct declaration and your new text for
readability.

Please let me know what you think.

Thanks,
Lawrence


Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.72
diff -u -p -r1.72 pf.4
--- pf.428 Dec 2010 13:56:11 -  1.72
+++ pf.46 May 2011 02:04:06 -
@@ -314,6 +314,17 @@ struct pfioc_natlook {
u_int8_t direction;
 };
 .Ed
+.Pp
+This was primarily used to support transparent proxies with rdr-to rules.
+New proxies should use divert-to rules instead.
+These do not require access to the privileged
+.Pa /dev/pf
+device and preserve the original destination address for
+.Xr getsockname 2 .
+For SOCK_DGRAM sockets, the
+.Xr ip 4
+socket options IP_RECVDSTADDR and IP_RECVDSTPORT can be used to retrieve
+destination address and port.
 .It Dv DIOCSETDEBUG Fa "u_int32_t *level"
 Set the debug level.
 See the
@@ -990,68 +1001,77 @@ packet filtering device.
 .El
 .Sh EXAMPLES
 The following example demonstrates how to use the
-.Dv DIOCNATLOOK
-command to find the internal host/port of a NATed connection:
+.Dv DIOCGETLIMIT
+command to show the hard limit of a memory pool:
 .Bd -literal
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
-u_int32_t
-read_address(const char *s)
-{
-   

Re: pf(4) man page: include missing header in example program

2011-05-05 Thread Stuart Henderson
On 2011/05/03 19:51, Mike Belopuhov wrote:
> On Tue, May 3, 2011 at 9:56 AM, Stuart Henderson  wrote:
> > On 2011/05/02 22:28, Lawrence Teo wrote:
> >> The DIOCNATLOOK example program at the end of the pf(4) man page
> >> uses memset(3), but string.h is not included. The following diff
> >> fixes this. Any thoughts?
> >
> > That change is correct but I'm not sure about keeping this example
> > code at all. We've had divert-to since OpenBSD 4.4 - when this is used
> > instead of rdr-to the destination address is preserved, so it can be
> > fetched with getsockname() without the DIOCNATLOOK dance.
> >
> > As a result the code becomes much less complicated, so we don't
> > really need an example any more, also another big advantage is that
> > there's no need for access to the privileged /dev/pf device.
> >
> > How about this?
> >
> 
> i'm in favor of this change, so ok mikeb
> 

slightly tweaked after knowledge gained from converting tftp-proxy;
specifically, mention the socket options that will be needed for udp
as they're not easy to track down.

 This was primarily used to support transparent proxies with rdr-
 to rules.  New proxies should use divert-to rules instead.  These
 do not require access to the privileged /dev/pf device and
 preserve the original destination address for getsockname(2).
 For SOCK_DGRAM sockets, the ip(4) socket options IP_RECVDSTADDR
 and IP_RECVDSTPORT can be used to retrieve destination address
 and port.

jmc asked about changing the example; if we replace it with a
code fragment using getsockname it's no longer specific to pf(4),
and the code is so much simpler than DIOCNATLOOK that IMO a
sample isn't really needed for this.

however we could probably usefully add a line or two to
getsockname(4) explaining its use with divert-to.

looking through the ports tree, there are two occasions where
DIOCNATLOOK is used that can't be replaced with divert-to/getsockname:
ftpsesame, which needs to lookup addresses gleaned from BPF captured
connections, and oidentd which needs to lookup in response to ident
requests. does anyone think it's worth keeping the example for cases
like these? (personally I don't, but could be persuaded otherwise
if people feel strongly about it).

(the other ports using DIOCNATLOOK can probably be converted
fairly easily - transproxy, tor, commoncpp, tircproxy).


Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.72
diff -u -p -r1.72 pf.4
--- pf.428 Dec 2010 13:56:11 -  1.72
+++ pf.45 May 2011 22:41:15 -
@@ -314,6 +314,16 @@ struct pfioc_natlook {
u_int8_t direction;
 };
 .Ed
+This was primarily used to support transparent proxies with rdr-to rules.
+New proxies should use divert-to rules instead.
+These do not require access to the privileged
+.Pa /dev/pf
+device and preserve the original destination address for
+.Xr getsockname 2 .
+For SOCK_DGRAM sockets, the
+.Xr ip 4
+socket options IP_RECVDSTADDR and IP_RECVDSTPORT can be used to retrieve
+destination address and port.
 .It Dv DIOCSETDEBUG Fa "u_int32_t *level"
 Set the debug level.
 See the
@@ -988,73 +998,6 @@ Explicitly remove source tracking nodes.
 .It Pa /dev/pf
 packet filtering device.
 .El
-.Sh EXAMPLES
-The following example demonstrates how to use the
-.Dv DIOCNATLOOK
-command to find the internal host/port of a NATed connection:
-.Bd -literal
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-u_int32_t
-read_address(const char *s)
-{
-   int a, b, c, d;
-
-   sscanf(s, "%i.%i.%i.%i", &a, &b, &c, &d);
-   return htonl(a << 24 | b << 16 | c << 8 | d);
-}
-
-void
-print_address(u_int32_t a)
-{
-   a = ntohl(a);
-   printf("%d.%d.%d.%d", a >> 24 & 255, a >> 16 & 255,
-   a >> 8 & 255, a & 255);
-}
-
-int
-main(int argc, char *argv[])
-{
-   struct pfioc_natlook nl;
-   int dev;
-
-   if (argc != 5) {
-   printf("%s\en",
-   argv[0]);
-   return 1;
-   }
-
-   dev = open("/dev/pf", O_RDWR);
-   if (dev == -1)
-   err(1, "open(\e"/dev/pf\e") failed");
-
-   memset(&nl, 0, sizeof(struct pfioc_natlook));
-   nl.saddr.v4.s_addr  = read_address(argv[1]);
-   nl.sport= htons(atoi(argv[2]));
-   nl.daddr.v4.s_addr  = read_address(argv[3]);
-   nl.dport= htons(atoi(argv[4]));
-   nl.af   = AF_INET;
-   nl.proto= IPPROTO_TCP;
-   nl.direction= PF_IN;
-
-   if (ioctl(dev, DIOCNATLOOK, &nl))
-   err(1, "DIOCNATLOOK");
-
-   printf("internal host ");
-   print_address(nl.rsaddr.v4.s_addr);
-   printf(":%u\en", ntohs(nl.rsport));
-   return 0;
-}
-.Ed
 .Sh SEE ALSO
 .Xr ioctl 2 ,

Re: pf(4) man page: include missing header in example program

2011-05-03 Thread Mike Belopuhov
On Tue, May 3, 2011 at 9:56 AM, Stuart Henderson  wrote:
> On 2011/05/02 22:28, Lawrence Teo wrote:
>> The DIOCNATLOOK example program at the end of the pf(4) man page
>> uses memset(3), but string.h is not included. The following diff
>> fixes this. Any thoughts?
>
> That change is correct but I'm not sure about keeping this example
> code at all. We've had divert-to since OpenBSD 4.4 - when this is used
> instead of rdr-to the destination address is preserved, so it can be
> fetched with getsockname() without the DIOCNATLOOK dance.
>
> As a result the code becomes much less complicated, so we don't
> really need an example any more, also another big advantage is that
> there's no need for access to the privileged /dev/pf device.
>
> How about this?
>

i'm in favor of this change, so ok mikeb



Re: pf(4) man page: include missing header in example program

2011-05-03 Thread Stuart Henderson
On 2011/05/02 22:28, Lawrence Teo wrote:
> The DIOCNATLOOK example program at the end of the pf(4) man page
> uses memset(3), but string.h is not included. The following diff
> fixes this. Any thoughts?

That change is correct but I'm not sure about keeping this example
code at all. We've had divert-to since OpenBSD 4.4 - when this is used
instead of rdr-to the destination address is preserved, so it can be
fetched with getsockname() without the DIOCNATLOOK dance.

As a result the code becomes much less complicated, so we don't
really need an example any more, also another big advantage is that
there's no need for access to the privileged /dev/pf device.

How about this?


Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.72
diff -u -p -r1.72 pf.4
--- pf.428 Dec 2010 13:56:11 -  1.72
+++ pf.43 May 2011 07:56:14 -
@@ -314,6 +314,13 @@ struct pfioc_natlook {
u_int8_t direction;
 };
 .Ed
+This was used to support transparent proxies with rdr-to rules.
+New code should use divert-to rules instead.
+These preserve the original destination address for
+.Xr getsockname 2
+and do not require access to the privileged
+.Pa /dev/pf
+device.
 .It Dv DIOCSETDEBUG Fa "u_int32_t *level"
 Set the debug level.
 See the
@@ -988,73 +995,6 @@ Explicitly remove source tracking nodes.
 .It Pa /dev/pf
 packet filtering device.
 .El
-.Sh EXAMPLES
-The following example demonstrates how to use the
-.Dv DIOCNATLOOK
-command to find the internal host/port of a NATed connection:
-.Bd -literal
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-u_int32_t
-read_address(const char *s)
-{
-   int a, b, c, d;
-
-   sscanf(s, "%i.%i.%i.%i", &a, &b, &c, &d);
-   return htonl(a << 24 | b << 16 | c << 8 | d);
-}
-
-void
-print_address(u_int32_t a)
-{
-   a = ntohl(a);
-   printf("%d.%d.%d.%d", a >> 24 & 255, a >> 16 & 255,
-   a >> 8 & 255, a & 255);
-}
-
-int
-main(int argc, char *argv[])
-{
-   struct pfioc_natlook nl;
-   int dev;
-
-   if (argc != 5) {
-   printf("%s\en",
-   argv[0]);
-   return 1;
-   }
-
-   dev = open("/dev/pf", O_RDWR);
-   if (dev == -1)
-   err(1, "open(\e"/dev/pf\e") failed");
-
-   memset(&nl, 0, sizeof(struct pfioc_natlook));
-   nl.saddr.v4.s_addr  = read_address(argv[1]);
-   nl.sport= htons(atoi(argv[2]));
-   nl.daddr.v4.s_addr  = read_address(argv[3]);
-   nl.dport= htons(atoi(argv[4]));
-   nl.af   = AF_INET;
-   nl.proto= IPPROTO_TCP;
-   nl.direction= PF_IN;
-
-   if (ioctl(dev, DIOCNATLOOK, &nl))
-   err(1, "DIOCNATLOOK");
-
-   printf("internal host ");
-   print_address(nl.rsaddr.v4.s_addr);
-   printf(":%u\en", ntohs(nl.rsport));
-   return 0;
-}
-.Ed
 .Sh SEE ALSO
 .Xr ioctl 2 ,
 .Xr bridge 4 ,



pf(4) man page: include missing header in example program

2011-05-02 Thread Lawrence Teo
The DIOCNATLOOK example program at the end of the pf(4) man page
uses memset(3), but string.h is not included. The following diff
fixes this. Any thoughts?

Thanks,
Lawrence


Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.72
diff -u -p -r1.72 pf.4
--- pf.428 Dec 2010 13:56:11 -  1.72
+++ pf.43 May 2011 02:13:35 -
@@ -1003,6 +1003,7 @@ command to find the internal host/port o
 #include 
 #include 
 #include 
+#include 
 
 u_int32_t
 read_address(const char *s)