[patch] pf PPTP nat passthrough patch.

classic Classic list List threaded Threaded
41 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[patch] pf PPTP nat passthrough patch.

Ermal Luçi-2
Hello,

after sometime thinking about this i putted together a patch for NAT
that should workaround PPTP NAT passthrough problems.

It is very simple and not intrusive.

For allowing PPTP to work correctly i replace the CALL_ID of PPTP with
the NAT'ed port number choosen by pf(4) for a connection. This should
be most of the time a unique ip/CALL_ID combination.

Your reviews and opinions are appriciated,
Ermal

? pf_pptp_HEAD.diff
Index: pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.567
diff -u -r1.567 pf.c
--- pf.c 20 Feb 2008 23:40:13 -0000 1.567
+++ pf.c 14 Mar 2008 09:54:05 -0000
@@ -3099,6 +3099,12 @@
  pd->nat_rule = nr;
  }
  }
+ if (pd.proto == IPPROTO_TCP) {
+       /* XXX: This are here until a pluggable framework for NAT is
finished */
+         if (ntohs(th->th_dport) == PPTP_CONTROL_PORT_NUMBER
+             || ntohs(th->th_sport) == PPTP_CONTROL_PORT_NUMBER)
+                 pf_get_pptp_translation(pd, th, m, direction, off, nport);
+ }

  while (r != NULL) {
  r->evaluations++;
@@ -4137,6 +4143,11 @@
     &th->th_sum, &(*state)->state_key->lan.addr,
     (*state)->state_key->lan.port, 0, pd->af);
  m_copyback(m, off, sizeof(*th), th);
+                /* XXX: This are here until a pluggable framework for
NAT is finished */
+                if (ntohs(th->th_dport) == PPTP_CONTROL_PORT_NUMBER
+                        || ntohs(th->th_sport) == PPTP_CONTROL_PORT_NUMBER)
+                        pf_get_pptp_translation(pd, th, m, direction, off,
+                                direction == PF_OUT ?
(*state)->gwy.port : (*state)->lan.port);
  } else if (copyback) {
  /* Copyback sequence modulation or stateful scrub changes */
  m_copyback(m, off, sizeof(*th), th);
Index: pf_pptp.c
===================================================================
RCS file: pf_pptp.c
diff -N pf_pptp.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ pf_pptp.c 14 Mar 2008 09:54:05 -0000
@@ -0,0 +1,179 @@
+/*
+ * Copyright (c) 2000 Whistle Communications, Inc.
+ * Copyright (c) 2008 Ermal Lugi
+ * All rights reserved.
+ *
+ * Subject to the following obligations and disclaimer of warranty, use and
+ * redistribution of this software, in source or object code forms, with or
+ * without modifications are expressly permitted by Whistle Communications;
+ * provided, however, that:
+ * 1. Any and all reproductions of the source or object code must include
the
+ *    copyright notice above and the following disclaimer of warranties; and
+ * 2. No rights are granted, in any manner or form, to use Whistle
+ *    Communications, Inc. trademarks, including the mark "WHISTLE
+ *    COMMUNICATIONS" on advertising, endorsements, or otherwise except as
+ *    such appears in the above copyright notice or in the software.
+ *
+ * THIS SOFTWARE IS BEING PROVIDED BY WHISTLE COMMUNICATIONS "AS IS", AND
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, WHISTLE COMMUNICATIONS MAKES NO
+ * REPRESENTATIONS OR WARRANTIES, EXPRESS OR IMPLIED, REGARDING THIS
SOFTWARE,
+ * INCLUDING WITHOUT LIMITATION, ANY AND ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, OR NON-INFRINGEMENT.
+ * WHISTLE COMMUNICATIONS DOES NOT WARRANT, GUARANTEE, OR MAKE ANY
+ * REPRESENTATIONS REGARDING THE USE OF, OR THE RESULTS OF THE USE OF THIS
+ * SOFTWARE IN TERMS OF ITS CORRECTNESS, ACCURACY, RELIABILITY OR OTHERWISE.
+ * IN NO EVENT SHALL WHISTLE COMMUNICATIONS BE LIABLE FOR ANY DAMAGES
+ * RESULTING FROM OR ARISING OUT OF ANY USE OF THIS SOFTWARE, INCLUDING
+ * WITHOUT LIMITATION, ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ * PUNITIVE, OR CONSEQUENTIAL DAMAGES, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES, LOSS OF USE, DATA OR PROFITS, HOWEVER CAUSED AND UNDER ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF WHISTLE COMMUNICATIONS IS ADVISED OF THE
POSSIBILITY
+ * OF SUCH DAMAGE.
+ */
+
+/* XXX: Some headers might not be needed. */
+
+#include <sys/param.h>
+#include <sys/systm.h>
+#include <sys/mbuf.h>
+#include <sys/filio.h>
+#include <sys/socket.h>
+#include <sys/socketvar.h>
+#include <sys/kernel.h>
+
+#include <net/if.h>
+#include <net/if_types.h>
+#include <net/bpf.h>
+#include <net/route.h>
+
+#include <netinet/in.h>
+#include <netinet/in_var.h>
+#include <netinet/in_systm.h>
+#include <netinet/ip.h>
+#include <netinet/ip_var.h>
+#include <netinet/tcp.h>
+#include <netinet/tcp_seq.h>
+
+#include <dev/rndvar.h>
+#include <net/pfvar.h>
+
+/*
+ * The data structures here and some of the logic in the code is based
+ * on alias_pptp.c of libalias.
+ */
+#define PPTP_MAGIC              0x1a2b3c4d
+#define PPTP_CTRL_MSG_TYPE      1
+
+enum {
+        PPTP_StartCtrlConnRequest = 1,
+        PPTP_StartCtrlConnReply = 2,
+        PPTP_StopCtrlConnRequest = 3,
+        PPTP_StopCtrlConnReply = 4,
+        PPTP_EchoRequest = 5,
+        PPTP_EchoReply = 6,
+        PPTP_OutCallRequest = 7,
+        PPTP_OutCallReply = 8,
+        PPTP_InCallRequest = 9,
+        PPTP_InCallReply = 10,
+        PPTP_InCallConn = 11,
+        PPTP_CallClearRequest = 12,
+        PPTP_CallDiscNotify = 13,
+        PPTP_WanErrorNotify = 14,
+        PPTP_SetLinkInfo = 15
+};
+
+ /* Message structures */
+struct pptpMsgHead {
+        u_int16_t       length; /* total length */
+        u_int16_t       msgType;/* PPTP message type */
+        u_int32_t       magic;  /* magic cookie */
+        u_int16_t       type;   /* control message type */
+        u_int16_t       resv0;  /* reserved */
+};
+
+struct pptpCodes {
+        u_int8_t        resCode;/* Result Code */
+        u_int8_t        errCode;/* Error Code */
+};
+
+struct pptpCallIds {
+        u_int16_t       cid1;   /* Call ID field #1 */
+        u_int16_t       cid2;   /* Call ID field #2 */
+};
+
+#if defined(_NETINET_TCP_H_)
+static __inline void *
+tcp_next(struct tcphdr *tcphdr)
+{
+        char *p = (char *)tcphdr;
+        return (&p[tcphdr->th_off * 4]);
+}
+#endif
+
+void
+pf_get_pptp_translation(struct pf_pdesc *pd, struct tcphdr *th,
struct mbuf *m,
+ int dir, int off, u_int16_t nport)
+{
+ struct pptpCallIds *cptr;
+        struct pptpCode *codes;
+        u_int16_t ctl_type;     /* control message type */
+        struct pptpMsgHead *hptr;
+ u_int16_t *pcall_id;
+
+        /* Verify data length */
+        if (pd->p_len < (int)(sizeof(struct pptpMsgHead) +
sizeof(struct pptpCallIds)))
+                return;
+
+        /* Move up to PPTP message header */
+        hptr = (struct pptpMsgHead *) tcp_next(th);
+
+        /* Return the control message type */
+        ctl_type = ntohs(hptr->type);
+
+        /* Verify PPTP Control Message */
+        if ((ntohs(hptr->msgType) != PPTP_CTRL_MSG_TYPE) ||
+            (ntohl(hptr->magic) != PPTP_MAGIC))
+                return;
+
+        /* Verify data length. */
+        if ((ctl_type == PPTP_OutCallReply || ctl_type == PPTP_InCallReply)
&&
+            (pd->p_len < (int)(sizeof(struct pptpMsgHead) +
sizeof(struct pptpCallIds) +
+                sizeof(struct pptpCodes))))
+                return;
+
+        cptr = (struct pptpCallIds *) (hptr + 1);
+        /* Verify valid PPTP control message */
+        if (cptr == NULL)
+                return;
+
+        /* Modify certain PPTP messages */
+        switch (ctl_type) {
+        case PPTP_OutCallRequest:
+        case PPTP_InCallRequest:
+        case PPTP_CallClearRequest:
+        case PPTP_CallDiscNotify:
+        case PPTP_InCallConn:
+        case PPTP_WanErrorNotify:
+        case PPTP_SetLinkInfo:
+                pcall_id = &cptr->cid1;
+                break;
+        case PPTP_OutCallReply:
+        case PPTP_InCallReply:
+ if (dir == PF_OUT)
+ pcall_id = &cptr->cid1;
+ else
+                 pcall_id = &cptr->cid2;
+                break;
+        default:
+                return;
+        }
+
+ ctl_type = *pcall_id;
+ *pcall_id = nport;
+ th->th_sum = pf_cksum_fixup(th->th_sum, ctl_type, nport, 0);
+ m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead) +
+ sizeof(struct pptpCallIds), sizeof(nport), (caddr_t)&nport);
+ return;
+}
Index: pfvar.h
===================================================================
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.259
diff -u -r1.259 pfvar.h
--- pfvar.h 2 Dec 2007 12:08:04 -0000 1.259
+++ pfvar.h 14 Mar 2008 09:54:09 -0000
@@ -1675,6 +1675,11 @@
 int pfr_ina_define(struct pfr_table *, struct pfr_addr *, int, int *,
     int *, u_int32_t, int);

+/* XXX: This are here until a pluggable framework for NAT is finished */
+#define PPTP_CONTROL_PORT_NUMBER 1723
+void   pf_get_pptp_translation(struct pf_pdesc *, struct tcphdr *,
struct mbuf *,
+                int, int, u_int16_t);
+
 extern struct pfi_kif *pfi_all;

 void pfi_initialize(void);

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Reyk Floeter-2
ok, we have some pf pptp implementations now to look at

On Fri, Mar 14, 2008 at 10:59:28AM +0100, Ermal Lu?i wrote:

> Hello,
>
> after sometime thinking about this i putted together a patch for NAT
> that should workaround PPTP NAT passthrough problems.
>
> It is very simple and not intrusive.
>
> For allowing PPTP to work correctly i replace the CALL_ID of PPTP with
> the NAT'ed port number choosen by pf(4) for a connection. This should
> be most of the time a unique ip/CALL_ID combination.
>
> Your reviews and opinions are appriciated,
> Ermal
>
> ? pf_pptp_HEAD.diff
> Index: pf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.567
> diff -u -r1.567 pf.c
> --- pf.c 20 Feb 2008 23:40:13 -0000 1.567
> +++ pf.c 14 Mar 2008 09:54:05 -0000
> @@ -3099,6 +3099,12 @@
>   pd->nat_rule = nr;
>   }
>   }
> + if (pd.proto == IPPROTO_TCP) {
> +       /* XXX: This are here until a pluggable framework for NAT is
> finished */
> +         if (ntohs(th->th_dport) == PPTP_CONTROL_PORT_NUMBER
> +             || ntohs(th->th_sport) == PPTP_CONTROL_PORT_NUMBER)
> +                 pf_get_pptp_translation(pd, th, m, direction, off, nport);
> + }
>
>   while (r != NULL) {
>   r->evaluations++;
> @@ -4137,6 +4143,11 @@
>      &th->th_sum, &(*state)->state_key->lan.addr,
>      (*state)->state_key->lan.port, 0, pd->af);
>   m_copyback(m, off, sizeof(*th), th);
> +                /* XXX: This are here until a pluggable framework for
> NAT is finished */
> +                if (ntohs(th->th_dport) == PPTP_CONTROL_PORT_NUMBER
> +                        || ntohs(th->th_sport) == PPTP_CONTROL_PORT_NUMBER)
> +                        pf_get_pptp_translation(pd, th, m, direction, off,
> +                                direction == PF_OUT ?
> (*state)->gwy.port : (*state)->lan.port);
>   } else if (copyback) {
>   /* Copyback sequence modulation or stateful scrub changes */
>   m_copyback(m, off, sizeof(*th), th);
> Index: pf_pptp.c
> ===================================================================
> RCS file: pf_pptp.c
> diff -N pf_pptp.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ pf_pptp.c 14 Mar 2008 09:54:05 -0000
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright (c) 2000 Whistle Communications, Inc.
> + * Copyright (c) 2008 Ermal Lugi
> + * All rights reserved.
> + *
> + * Subject to the following obligations and disclaimer of warranty, use and
> + * redistribution of this software, in source or object code forms, with or
> + * without modifications are expressly permitted by Whistle Communications;
> + * provided, however, that:
> + * 1. Any and all reproductions of the source or object code must include
> the
> + *    copyright notice above and the following disclaimer of warranties; and
> + * 2. No rights are granted, in any manner or form, to use Whistle
> + *    Communications, Inc. trademarks, including the mark "WHISTLE
> + *    COMMUNICATIONS" on advertising, endorsements, or otherwise except as
> + *    such appears in the above copyright notice or in the software.
> + *
> + * THIS SOFTWARE IS BEING PROVIDED BY WHISTLE COMMUNICATIONS "AS IS", AND
> + * TO THE MAXIMUM EXTENT PERMITTED BY LAW, WHISTLE COMMUNICATIONS MAKES NO
> + * REPRESENTATIONS OR WARRANTIES, EXPRESS OR IMPLIED, REGARDING THIS
> SOFTWARE,
> + * INCLUDING WITHOUT LIMITATION, ANY AND ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, OR NON-INFRINGEMENT.
> + * WHISTLE COMMUNICATIONS DOES NOT WARRANT, GUARANTEE, OR MAKE ANY
> + * REPRESENTATIONS REGARDING THE USE OF, OR THE RESULTS OF THE USE OF THIS
> + * SOFTWARE IN TERMS OF ITS CORRECTNESS, ACCURACY, RELIABILITY OR OTHERWISE.
> + * IN NO EVENT SHALL WHISTLE COMMUNICATIONS BE LIABLE FOR ANY DAMAGES
> + * RESULTING FROM OR ARISING OUT OF ANY USE OF THIS SOFTWARE, INCLUDING
> + * WITHOUT LIMITATION, ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
> + * PUNITIVE, OR CONSEQUENTIAL DAMAGES, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES, LOSS OF USE, DATA OR PROFITS, HOWEVER CAUSED AND UNDER ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF WHISTLE COMMUNICATIONS IS ADVISED OF THE
> POSSIBILITY
> + * OF SUCH DAMAGE.
> + */
> +
> +/* XXX: Some headers might not be needed. */
> +
> +#include <sys/param.h>
> +#include <sys/systm.h>
> +#include <sys/mbuf.h>
> +#include <sys/filio.h>
> +#include <sys/socket.h>
> +#include <sys/socketvar.h>
> +#include <sys/kernel.h>
> +
> +#include <net/if.h>
> +#include <net/if_types.h>
> +#include <net/bpf.h>
> +#include <net/route.h>
> +
> +#include <netinet/in.h>
> +#include <netinet/in_var.h>
> +#include <netinet/in_systm.h>
> +#include <netinet/ip.h>
> +#include <netinet/ip_var.h>
> +#include <netinet/tcp.h>
> +#include <netinet/tcp_seq.h>
> +
> +#include <dev/rndvar.h>
> +#include <net/pfvar.h>
> +
> +/*
> + * The data structures here and some of the logic in the code is based
> + * on alias_pptp.c of libalias.
> + */
> +#define PPTP_MAGIC              0x1a2b3c4d
> +#define PPTP_CTRL_MSG_TYPE      1
> +
> +enum {
> +        PPTP_StartCtrlConnRequest = 1,
> +        PPTP_StartCtrlConnReply = 2,
> +        PPTP_StopCtrlConnRequest = 3,
> +        PPTP_StopCtrlConnReply = 4,
> +        PPTP_EchoRequest = 5,
> +        PPTP_EchoReply = 6,
> +        PPTP_OutCallRequest = 7,
> +        PPTP_OutCallReply = 8,
> +        PPTP_InCallRequest = 9,
> +        PPTP_InCallReply = 10,
> +        PPTP_InCallConn = 11,
> +        PPTP_CallClearRequest = 12,
> +        PPTP_CallDiscNotify = 13,
> +        PPTP_WanErrorNotify = 14,
> +        PPTP_SetLinkInfo = 15
> +};
> +
> + /* Message structures */
> +struct pptpMsgHead {
> +        u_int16_t       length; /* total length */
> +        u_int16_t       msgType;/* PPTP message type */
> +        u_int32_t       magic;  /* magic cookie */
> +        u_int16_t       type;   /* control message type */
> +        u_int16_t       resv0;  /* reserved */
> +};
> +
> +struct pptpCodes {
> +        u_int8_t        resCode;/* Result Code */
> +        u_int8_t        errCode;/* Error Code */
> +};
> +
> +struct pptpCallIds {
> +        u_int16_t       cid1;   /* Call ID field #1 */
> +        u_int16_t       cid2;   /* Call ID field #2 */
> +};
> +
> +#if defined(_NETINET_TCP_H_)
> +static __inline void *
> +tcp_next(struct tcphdr *tcphdr)
> +{
> +        char *p = (char *)tcphdr;
> +        return (&p[tcphdr->th_off * 4]);
> +}
> +#endif
> +
> +void
> +pf_get_pptp_translation(struct pf_pdesc *pd, struct tcphdr *th,
> struct mbuf *m,
> + int dir, int off, u_int16_t nport)
> +{
> + struct pptpCallIds *cptr;
> +        struct pptpCode *codes;
> +        u_int16_t ctl_type;     /* control message type */
> +        struct pptpMsgHead *hptr;
> + u_int16_t *pcall_id;
> +
> +        /* Verify data length */
> +        if (pd->p_len < (int)(sizeof(struct pptpMsgHead) +
> sizeof(struct pptpCallIds)))
> +                return;
> +
> +        /* Move up to PPTP message header */
> +        hptr = (struct pptpMsgHead *) tcp_next(th);
> +
> +        /* Return the control message type */
> +        ctl_type = ntohs(hptr->type);
> +
> +        /* Verify PPTP Control Message */
> +        if ((ntohs(hptr->msgType) != PPTP_CTRL_MSG_TYPE) ||
> +            (ntohl(hptr->magic) != PPTP_MAGIC))
> +                return;
> +
> +        /* Verify data length. */
> +        if ((ctl_type == PPTP_OutCallReply || ctl_type == PPTP_InCallReply)
> &&
> +            (pd->p_len < (int)(sizeof(struct pptpMsgHead) +
> sizeof(struct pptpCallIds) +
> +                sizeof(struct pptpCodes))))
> +                return;
> +
> +        cptr = (struct pptpCallIds *) (hptr + 1);
> +        /* Verify valid PPTP control message */
> +        if (cptr == NULL)
> +                return;
> +
> +        /* Modify certain PPTP messages */
> +        switch (ctl_type) {
> +        case PPTP_OutCallRequest:
> +        case PPTP_InCallRequest:
> +        case PPTP_CallClearRequest:
> +        case PPTP_CallDiscNotify:
> +        case PPTP_InCallConn:
> +        case PPTP_WanErrorNotify:
> +        case PPTP_SetLinkInfo:
> +                pcall_id = &cptr->cid1;
> +                break;
> +        case PPTP_OutCallReply:
> +        case PPTP_InCallReply:
> + if (dir == PF_OUT)
> + pcall_id = &cptr->cid1;
> + else
> +                 pcall_id = &cptr->cid2;
> +                break;
> +        default:
> +                return;
> +        }
> +
> + ctl_type = *pcall_id;
> + *pcall_id = nport;
> + th->th_sum = pf_cksum_fixup(th->th_sum, ctl_type, nport, 0);
> + m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead) +
> + sizeof(struct pptpCallIds), sizeof(nport), (caddr_t)&nport);
> + return;
> +}
> Index: pfvar.h
> ===================================================================
> RCS file: /cvs/src/sys/net/pfvar.h,v
> retrieving revision 1.259
> diff -u -r1.259 pfvar.h
> --- pfvar.h 2 Dec 2007 12:08:04 -0000 1.259
> +++ pfvar.h 14 Mar 2008 09:54:09 -0000
> @@ -1675,6 +1675,11 @@
>  int pfr_ina_define(struct pfr_table *, struct pfr_addr *, int, int *,
>      int *, u_int32_t, int);
>
> +/* XXX: This are here until a pluggable framework for NAT is finished */
> +#define PPTP_CONTROL_PORT_NUMBER 1723
> +void   pf_get_pptp_translation(struct pf_pdesc *, struct tcphdr *,
> struct mbuf *,
> +                int, int, u_int16_t);
> +
>  extern struct pfi_kif *pfi_all;
>
>  void pfi_initialize(void);

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Ermal Luçi-2
On Fri, Mar 14, 2008 at 1:09 PM, Reyk Floeter <[hidden email]> wrote:
> ok, we have some pf pptp implementations now to look at
Thanks, for the interest in this.

While i should be more careful when sending these mails.
For anybody willing to try this there is an error on rewriting back the packet:
the line
+     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead) +
+             sizeof(struct pptpCallIds), sizeof(nport), (caddr_t)&nport);

should read

+     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead),
+             sizeof(*cptr), (caddr_t)cptr);


>
>
>
>  On Fri, Mar 14, 2008 at 10:59:28AM +0100, Ermal Lu?i wrote:
>  > Hello,
>  >
>  > after sometime thinking about this i putted together a patch for NAT
>  > that should workaround PPTP NAT passthrough problems.
>  >
>  > It is very simple and not intrusive.
>  >
>  > For allowing PPTP to work correctly i replace the CALL_ID of PPTP with
>  > the NAT'ed port number choosen by pf(4) for a connection. This should
>  > be most of the time a unique ip/CALL_ID combination.
>  >
>  > Your reviews and opinions are appriciated,
>  > Ermal
>  >
>  > ? pf_pptp_HEAD.diff
>  > Index: pf.c
>  > ===================================================================
>  > RCS file: /cvs/src/sys/net/pf.c,v
>  > retrieving revision 1.567
>  > diff -u -r1.567 pf.c
>  > --- pf.c      20 Feb 2008 23:40:13 -0000      1.567
>  > +++ pf.c      14 Mar 2008 09:54:05 -0000
>  > @@ -3099,6 +3099,12 @@
>  >                       pd->nat_rule = nr;
>  >               }
>  >       }
>  > +     if (pd.proto == IPPROTO_TCP) {
>  > +            /* XXX: This are here until a pluggable framework for NAT is
>  > finished */
>  > +             if (ntohs(th->th_dport) == PPTP_CONTROL_PORT_NUMBER
>  > +                             || ntohs(th->th_sport) == PPTP_CONTROL_PORT_NUMBER)
>  > +                     pf_get_pptp_translation(pd, th, m, direction, off, nport);
>  > +     }
>  >
>  >       while (r != NULL) {
>  >               r->evaluations++;
>  > @@ -4137,6 +4143,11 @@
>  >                           &th->th_sum, &(*state)->state_key->lan.addr,
>  >                           (*state)->state_key->lan.port, 0, pd->af);
>  >               m_copyback(m, off, sizeof(*th), th);
>  > +                /* XXX: This are here until a pluggable framework for
>  > NAT is finished */
>  > +                if (ntohs(th->th_dport) == PPTP_CONTROL_PORT_NUMBER
>  > +                        || ntohs(th->th_sport) == PPTP_CONTROL_PORT_NUMBER)
>  > +                        pf_get_pptp_translation(pd, th, m, direction, off,
>  > +                                direction == PF_OUT ?
>  > (*state)->gwy.port : (*state)->lan.port);
>  >       } else if (copyback) {
>  >               /* Copyback sequence modulation or stateful scrub changes */
>  >               m_copyback(m, off, sizeof(*th), th);
>  > Index: pf_pptp.c
>  > ===================================================================
>  > RCS file: pf_pptp.c
>  > diff -N pf_pptp.c
>  > --- /dev/null 1 Jan 1970 00:00:00 -0000
>  > +++ pf_pptp.c 14 Mar 2008 09:54:05 -0000
>  > @@ -0,0 +1,179 @@
>  > +/*
>  > + * Copyright (c) 2000 Whistle Communications, Inc.
>  > + * Copyright (c) 2008 Ermal Lugi
>
>
> > + * All rights reserved.
>  > + *
>  > + * Subject to the following obligations and disclaimer of warranty, use and
>  > + * redistribution of this software, in source or object code forms, with or
>  > + * without modifications are expressly permitted by Whistle Communications;
>  > + * provided, however, that:
>  > + * 1. Any and all reproductions of the source or object code must include
>  > the
>  > + *    copyright notice above and the following disclaimer of warranties; and
>  > + * 2. No rights are granted, in any manner or form, to use Whistle
>  > + *    Communications, Inc. trademarks, including the mark "WHISTLE
>  > + *    COMMUNICATIONS" on advertising, endorsements, or otherwise except as
>  > + *    such appears in the above copyright notice or in the software.
>  > + *
>  > + * THIS SOFTWARE IS BEING PROVIDED BY WHISTLE COMMUNICATIONS "AS IS", AND
>  > + * TO THE MAXIMUM EXTENT PERMITTED BY LAW, WHISTLE COMMUNICATIONS MAKES NO
>  > + * REPRESENTATIONS OR WARRANTIES, EXPRESS OR IMPLIED, REGARDING THIS
>  > SOFTWARE,
>  > + * INCLUDING WITHOUT LIMITATION, ANY AND ALL IMPLIED WARRANTIES OF
>  > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, OR NON-INFRINGEMENT.
>  > + * WHISTLE COMMUNICATIONS DOES NOT WARRANT, GUARANTEE, OR MAKE ANY
>  > + * REPRESENTATIONS REGARDING THE USE OF, OR THE RESULTS OF THE USE OF THIS
>  > + * SOFTWARE IN TERMS OF ITS CORRECTNESS, ACCURACY, RELIABILITY OR OTHERWISE.
>  > + * IN NO EVENT SHALL WHISTLE COMMUNICATIONS BE LIABLE FOR ANY DAMAGES
>  > + * RESULTING FROM OR ARISING OUT OF ANY USE OF THIS SOFTWARE, INCLUDING
>  > + * WITHOUT LIMITATION, ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
>  > + * PUNITIVE, OR CONSEQUENTIAL DAMAGES, PROCUREMENT OF SUBSTITUTE GOODS OR
>  > + * SERVICES, LOSS OF USE, DATA OR PROFITS, HOWEVER CAUSED AND UNDER ANY
>  > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>  > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>  > + * THIS SOFTWARE, EVEN IF WHISTLE COMMUNICATIONS IS ADVISED OF THE
>  > POSSIBILITY
>  > + * OF SUCH DAMAGE.
>  > + */
>  > +
>  > +/* XXX: Some headers might not be needed. */
>  > +
>  > +#include <sys/param.h>
>  > +#include <sys/systm.h>
>  > +#include <sys/mbuf.h>
>  > +#include <sys/filio.h>
>  > +#include <sys/socket.h>
>  > +#include <sys/socketvar.h>
>  > +#include <sys/kernel.h>
>  > +
>  > +#include <net/if.h>
>  > +#include <net/if_types.h>
>  > +#include <net/bpf.h>
>  > +#include <net/route.h>
>  > +
>  > +#include <netinet/in.h>
>  > +#include <netinet/in_var.h>
>  > +#include <netinet/in_systm.h>
>  > +#include <netinet/ip.h>
>  > +#include <netinet/ip_var.h>
>  > +#include <netinet/tcp.h>
>  > +#include <netinet/tcp_seq.h>
>  > +
>  > +#include <dev/rndvar.h>
>  > +#include <net/pfvar.h>
>  > +
>  > +/*
>  > + * The data structures here and some of the logic in the code is based
>  > + * on alias_pptp.c of libalias.
>  > + */
>  > +#define PPTP_MAGIC              0x1a2b3c4d
>  > +#define PPTP_CTRL_MSG_TYPE      1
>  > +
>  > +enum {
>  > +        PPTP_StartCtrlConnRequest = 1,
>  > +        PPTP_StartCtrlConnReply = 2,
>  > +        PPTP_StopCtrlConnRequest = 3,
>  > +        PPTP_StopCtrlConnReply = 4,
>  > +        PPTP_EchoRequest = 5,
>  > +        PPTP_EchoReply = 6,
>  > +        PPTP_OutCallRequest = 7,
>  > +        PPTP_OutCallReply = 8,
>  > +        PPTP_InCallRequest = 9,
>  > +        PPTP_InCallReply = 10,
>  > +        PPTP_InCallConn = 11,
>  > +        PPTP_CallClearRequest = 12,
>  > +        PPTP_CallDiscNotify = 13,
>  > +        PPTP_WanErrorNotify = 14,
>  > +        PPTP_SetLinkInfo = 15
>  > +};
>  > +
>  > + /* Message structures */
>  > +struct pptpMsgHead {
>  > +        u_int16_t       length; /* total length */
>  > +        u_int16_t       msgType;/* PPTP message type */
>  > +        u_int32_t       magic;  /* magic cookie */
>  > +        u_int16_t       type;   /* control message type */
>  > +        u_int16_t       resv0;  /* reserved */
>  > +};
>  > +
>  > +struct pptpCodes {
>  > +        u_int8_t        resCode;/* Result Code */
>  > +        u_int8_t        errCode;/* Error Code */
>  > +};
>  > +
>  > +struct pptpCallIds {
>  > +        u_int16_t       cid1;   /* Call ID field #1 */
>  > +        u_int16_t       cid2;   /* Call ID field #2 */
>  > +};
>  > +
>  > +#if defined(_NETINET_TCP_H_)
>  > +static __inline void *
>  > +tcp_next(struct tcphdr *tcphdr)
>  > +{
>  > +        char *p = (char *)tcphdr;
>  > +        return (&p[tcphdr->th_off * 4]);
>  > +}
>  > +#endif
>  > +
>  > +void
>  > +pf_get_pptp_translation(struct pf_pdesc *pd, struct tcphdr *th,
>  > struct mbuf *m,
>  > +     int dir, int off, u_int16_t nport)
>  > +{
>  > +     struct pptpCallIds *cptr;
>  > +        struct pptpCode *codes;
>  > +        u_int16_t ctl_type;     /* control message type */
>  > +        struct pptpMsgHead *hptr;
>  > +     u_int16_t *pcall_id;
>  > +
>  > +        /* Verify data length */
>  > +        if (pd->p_len < (int)(sizeof(struct pptpMsgHead) +
>  > sizeof(struct pptpCallIds)))
>  > +                return;
>  > +
>  > +        /* Move up to PPTP message header */
>  > +        hptr = (struct pptpMsgHead *) tcp_next(th);
>  > +
>  > +        /* Return the control message type */
>  > +        ctl_type = ntohs(hptr->type);
>  > +
>  > +        /* Verify PPTP Control Message */
>  > +        if ((ntohs(hptr->msgType) != PPTP_CTRL_MSG_TYPE) ||
>  > +            (ntohl(hptr->magic) != PPTP_MAGIC))
>  > +                return;
>  > +
>  > +        /* Verify data length. */
>  > +        if ((ctl_type == PPTP_OutCallReply || ctl_type == PPTP_InCallReply)
>  > &&
>  > +            (pd->p_len < (int)(sizeof(struct pptpMsgHead) +
>  > sizeof(struct pptpCallIds) +
>  > +                sizeof(struct pptpCodes))))
>  > +                return;
>  > +
>  > +        cptr = (struct pptpCallIds *) (hptr + 1);
>  > +        /* Verify valid PPTP control message */
>  > +        if (cptr == NULL)
>  > +                return;
>  > +
>  > +        /* Modify certain PPTP messages */
>  > +        switch (ctl_type) {
>  > +        case PPTP_OutCallRequest:
>  > +        case PPTP_InCallRequest:
>  > +        case PPTP_CallClearRequest:
>  > +        case PPTP_CallDiscNotify:
>  > +        case PPTP_InCallConn:
>  > +        case PPTP_WanErrorNotify:
>  > +        case PPTP_SetLinkInfo:
>  > +                pcall_id = &cptr->cid1;
>  > +                break;
>  > +        case PPTP_OutCallReply:
>  > +        case PPTP_InCallReply:
>  > +             if (dir == PF_OUT)
>  > +                     pcall_id = &cptr->cid1;
>  > +             else
>  > +                     pcall_id = &cptr->cid2;
>  > +                break;
>  > +        default:
>  > +                return;
>  > +        }
>  > +
>  > +     ctl_type = *pcall_id;
>  > +     *pcall_id = nport;
>  > +     th->th_sum = pf_cksum_fixup(th->th_sum, ctl_type, nport, 0);
>  > +     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead) +
>  > +             sizeof(struct pptpCallIds), sizeof(nport), (caddr_t)&nport);
>  > +     return;
>  > +}
>  > Index: pfvar.h
>  > ===================================================================
>  > RCS file: /cvs/src/sys/net/pfvar.h,v
>  > retrieving revision 1.259
>  > diff -u -r1.259 pfvar.h
>  > --- pfvar.h   2 Dec 2007 12:08:04 -0000       1.259
>  > +++ pfvar.h   14 Mar 2008 09:54:09 -0000
>  > @@ -1675,6 +1675,11 @@
>  >  int  pfr_ina_define(struct pfr_table *, struct pfr_addr *, int, int *,
>  >           int *, u_int32_t, int);
>  >
>  > +/* XXX: This are here until a pluggable framework for NAT is finished */
>  > +#define PPTP_CONTROL_PORT_NUMBER 1723
>  > +void   pf_get_pptp_translation(struct pf_pdesc *, struct tcphdr *,
>  > struct mbuf *,
>  > +                int, int, u_int16_t);
>  > +
>  >  extern struct pfi_kif                *pfi_all;
>  >
>  >  void          pfi_initialize(void);

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 14:56:31 Mar 14, Ermal Lu?i wrote:
> On Fri, Mar 14, 2008 at 1:09 PM, Reyk Floeter <[hidden email]> wrote:
> > ok, we have some pf pptp implementations now to look at
> Thanks, for the interest in this.
>
> While i should be more careful when sending these mails.
> For anybody willing to try this there is an error on rewriting back the
packet:
> the line
> +     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead) +
> +             sizeof(struct pptpCallIds), sizeof(nport), (caddr_t)&nport);
>
> should read
>
> +     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead),
> +             sizeof(*cptr), (caddr_t)cptr);
>

Firstly PPTP should not be done in the kernel.

Secondly there is already an implementation under review.

Since the tree is locked right now I am not sure when it will make it
into the tree.

Search the archives.

-Girish

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Ermal Luçi-2
On Fri, Mar 14, 2008 at 3:43 PM, Girish Venkatachalam
<[hidden email]> wrote:

> On 14:56:31 Mar 14, Ermal Lu?i wrote:
>  > On Fri, Mar 14, 2008 at 1:09 PM, Reyk Floeter <[hidden email]> wrote:
>  > > ok, we have some pf pptp implementations now to look at
>  > Thanks, for the interest in this.
>  >
>  > While i should be more careful when sending these mails.
>  > For anybody willing to try this there is an error on rewriting back the
>  packet:
>  > the line
>  > +     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead) +
>  > +             sizeof(struct pptpCallIds), sizeof(nport), (caddr_t)&nport);
>  >
>  > should read
>  >
>  > +     m_copyback(m, off + sizeof(*th) + sizeof(struct pptpMsgHead),
>  > +             sizeof(*cptr), (caddr_t)cptr);
>  >
>
>  Firstly PPTP should not be done in the kernel.
>
Well, arguable, since a lot should not be done in kernel but *BSD's
are not a microkernel and life is not that easy.

>  Secondly there is already an implementation under review.
Glad to here and am not trying to conflict with that.

>
>  Since the tree is locked right now I am not sure when it will make it
>  into the tree.
>
>  Search the archives.
I have looked at your patch, but according to my searches there are
two patches i found.
One suggests the userland proxy, pptp-proxy iirc, the same as [t]ftp-proxy
Second is your patch which conflicts with your previous statement.
Anyhow modyfing the packet as i do in this patch is something that
pf(4) already does for icmp and the overhead is minimalistic since
most of the information for doing it is readily available.

Though, as i said, it is not my intention to stump on an already
reviewed implementation but just add more to it.

Ermal

>
>  -Girish
>
>  [demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 16:13:10 Mar 14, Ermal Lu?i wrote:
> Well, arguable, since a lot should not be done in kernel but *BSD's
> are not a microkernel and life is not that easy.
>

It is not arguable at all.

I did my first implementation in kernel and the second in userland. I
agree the userland design is far more complex but that is the right way
to do things.

pf(4) has no business diddling with application level packets.

> Glad to here and am not trying to conflict with that.
>

No problem. I did not mean that either.

> I have looked at your patch, but according to my searches there are
> two patches i found.
> One suggests the userland proxy, pptp-proxy iirc, the same as [t]ftp-proxy
> Second is your patch which conflicts with your previous statement.

Those were the first cut versions. My final patch has been substantially
reworked, the design completely changed and so on.

But you don't find that in the archives.

> Anyhow modyfing the packet as i do in this patch is something that
> pf(4) already does for icmp and the overhead is minimalistic since
> most of the information for doing it is readily available.

It is not a question of performance and comparing PPTP with ICMP IMO is
not a good approach.

> Though, as i said, it is not my intention to stump on an already
> reviewed implementation but just add more to it.
>

That is fine. I did not mean that either.

It is just that it leads to duplication of work.

New ideas are always welcome however.

Perhaps if my code had made it into the tree you would not have had to
reinvent the wheel.

Never mind.

Sorry about this.

-Girish

--
"unix soi qui mal y pense"

UNIX to him who evil thinks

+------------------------------------------------------------------+
| GnuPG key  : 0xC7BBF207  |  http://wwwkeys.nl.pgp.net            |
| Fingerprint: 2AFF C264 20CE C80C DDFF  CC15 AD3E F190 C7BB F207  |
+------------------------------------------------------------------+

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Stefan Sperling-5
On Sat, Mar 15, 2008 at 06:54:58AM +0530, Girish Venkatachalam wrote:
> On 16:13:10 Mar 14, Ermal Lu?i wrote:
> > I have looked at your patch, but according to my searches there are
> > two patches i found.
> > One suggests the userland proxy, pptp-proxy iirc, the same as
[t]ftp-proxy
> > Second is your patch which conflicts with your previous statement.
>
> Those were the first cut versions. My final patch has been substantially
> reworked, the design completely changed and so on.
>
> But you don't find that in the archives.

Why not send it to the list now then so Ermal and others can look at it,
too? I'd like to see it as well (I maintain the net/pptp port).

--
stefan
http://stsp.name                                         PGP Key: 0xF59D25F0

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 09:07:15 Mar 15, Stefan Sperling wrote:

> Why not send it to the list now then so Ermal and others can look at it,
> too? I'd like to see it as well (I maintain the net/pptp port).
>

My pleasure.

The reason I did not think of it is simple. Discussions went over
personal mail with henning@. Later mrbride@, reyk@ and others joined.

Anyway I believe the code is a result of team effort. And I definitely
owe a lot of gratitude towards Camiel Dobbelaar whose excellent
ftp-proxy(8) made it a cinch for me to code pptp-proxy(8).

So there you go.

Please find the tgz here.

http://sirsasana.org/misc/pptp-patch.tgz

# sha1 pptp-patch.tgz
SHA1 (pptp-patch.tgz) = d6e18300c698c392a0f9c60e3c1cfae238c8e332

It contains the following files.

# tar ztf pptp-patch.tgz
pptp-patch
pptp-patch/sys-net-patch.diff
pptp-patch/pfctl-patch.diff
pptp-patch/pf.conf.5.diff
pptp-patch/pptp-proxy
pptp-patch/pptp-proxy/Makefile
pptp-patch/pptp-proxy/pptp-proxy.c
pptp-patch/pptp-proxy/pptp-proxy.8


1) sys-net-patch.diff is the main kernel change. It involves modifying
pf.c for using callid field that is newly introduced for PPTP

2) pfctl-patch.diff contains the relevant changes in pfctl grammar so
that the user gets to specify a callid anchor

3) pf.conf.5.diff merely changes the pf.conf.5 man page to reflect the
addition to grammar. I have not added any description. Perhaps it is
required?

4) The directory pptp-proxy is to be placed under /usr/src/usr.sbin and
is modeled after ftp-proxy(8).


Let me know if you need anything else.

Thanks.

-Girish

--
"unix soi qui mal y pense"

UNIX to him who evil thinks

+------------------------------------------------------------------+
| GnuPG key  : 0xC7BBF207  |  http://wwwkeys.nl.pgp.net            |
| Fingerprint: 2AFF C264 20CE C80C DDFF  CC15 AD3E F190 C7BB F207  |
+------------------------------------------------------------------+

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Ermal Luçi-2
Well i see the reason that OpenBSD developers have about not handling
application layer  in kernel since they do no want special cases in
kernel in an application that should be general enough.

Though the way that you have written this patch it indeed inserts a
special case in kernel. That is GRE.
How about doing this with no special cases but with just a plugin
architecture for nat where let say if i write a rule like:

nat on $interface  from any to any -> $extinterface with-proxy(pptp)

And just check after the nat rule has been found if it has a flag to
handle pptp or other protocols in the run.
And then just call the appropriate function for this.
It is a simple test insturction for users that do not want the
overhead and the users that accept it are served as requested.
IE.
pf_test() has

if (direction == PF_OUT) {
pf_get_translation(...)
do appropriate modifications
} else {
pf_get_translation(...)
do appropriate modifications
}

I just want to add this test  after that code:
if (nr && nr->flags & PF_WITH_PROXY) {
call the proxy rutines to identify what needs to be handled and
continue normally
}

This test would be added after state_translation code too.

This is just unnoticed overhead if you do not want the overhead and it
does not bloat/break ABI.
I would be willing to write such a thing. And i know many people would
be just happy to have something handled transparently than having to
go through the configuration of a proxy.

If you find it interesting we can discuss this more and i am more than
willing in this.

Regrads,
Ermal

P.S. if you can handle ftp without patching pf why cannot PPTP be
handled as such?!
Examples of this are frickin pptp proxy which actually does it job in userland!

On Sun, Mar 16, 2008 at 1:15 AM, Girish Venkatachalam
<[hidden email]> wrote:

> On 09:07:15 Mar 15, Stefan Sperling wrote:
>
>  > Why not send it to the list now then so Ermal and others can look at it,
>  > too? I'd like to see it as well (I maintain the net/pptp port).
>  >
>
>  My pleasure.
>
>  The reason I did not think of it is simple. Discussions went over
>  personal mail with henning@. Later mrbride@, reyk@ and others joined.
>
>  Anyway I believe the code is a result of team effort. And I definitely
>  owe a lot of gratitude towards Camiel Dobbelaar whose excellent
>  ftp-proxy(8) made it a cinch for me to code pptp-proxy(8).
>
>  So there you go.
>
>  Please find the tgz here.
>
>  http://sirsasana.org/misc/pptp-patch.tgz
>
>  # sha1 pptp-patch.tgz
>  SHA1 (pptp-patch.tgz) = d6e18300c698c392a0f9c60e3c1cfae238c8e332
>
>  It contains the following files.
>
>  # tar ztf pptp-patch.tgz
>  pptp-patch
>  pptp-patch/sys-net-patch.diff
>  pptp-patch/pfctl-patch.diff
>  pptp-patch/pf.conf.5.diff
>  pptp-patch/pptp-proxy
>  pptp-patch/pptp-proxy/Makefile
>  pptp-patch/pptp-proxy/pptp-proxy.c
>  pptp-patch/pptp-proxy/pptp-proxy.8
>
>
>  1) sys-net-patch.diff is the main kernel change. It involves modifying
>  pf.c for using callid field that is newly introduced for PPTP
>
>  2) pfctl-patch.diff contains the relevant changes in pfctl grammar so
>  that the user gets to specify a callid anchor
>
>  3) pf.conf.5.diff merely changes the pf.conf.5 man page to reflect the
>  addition to grammar. I have not added any description. Perhaps it is
>  required?
>
>  4) The directory pptp-proxy is to be placed under /usr/src/usr.sbin and
>  is modeled after ftp-proxy(8).
>
>
>  Let me know if you need anything else.
>
>  Thanks.
>
>
>  -Girish
>
>  --
>  "unix soi qui mal y pense"
>
>  UNIX to him who evil thinks
>
>  +------------------------------------------------------------------+
>  | GnuPG key  : 0xC7BBF207  |  http://wwwkeys.nl.pgp.net            |
>  | Fingerprint: 2AFF C264 20CE C80C DDFF  CC15 AD3E F190 C7BB F207  |
>  +------------------------------------------------------------------+
>
>
>
> [demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Stuart Henderson
On 2008/03/16 19:57, Ermal Lugi wrote:
> P.S. if you can handle ftp without patching pf why cannot PPTP be
> handled as such?!

PF already has a method of setting the port number for TCP and UDP.
GRE is a different IP protocol, without any particular support in PF.
call-id can be regarded pretty much like a port number here.

Girish, it looks like you take non-PPTP users of GRE into account
with the kernel changes (i.e. not to touch whatever's in the part of
the packet occupied by callid unless ptype == ETHERTYPE_PPP), have
you had chance to test that works ok?

> Examples of this are frickin pptp proxy which actually does it job in userland!

This is *much* better than frickin.

That's a different (and higher overhead) architecture. This way the
simple bulk data transfers can be handled by some small and relatively
safe support in-kernel, and the inherently more dangerous PPTP protocol
handling is done by jailed unprivileged userland code.

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 21:46:22 Mar 16, Stuart Henderson wrote:
> Girish, it looks like you take non-PPTP users of GRE into account
> with the kernel changes (i.e. not to touch whatever's in the part of
> the packet occupied by callid unless ptype == ETHERTYPE_PPP), have
> you had chance to test that works ok?
>

It will be great if someone can do this big favor for me.

I have not been able to get gre(4) tunnels working for arbitrary
traffic. Or the easier way would be to simply run WCCP or some such
common GRE protocol with this patch.

As Stuart pointed out, my code has the correct hooks to handle this but
it surely needs testing.


> This is *much* better than frickin.
>
> That's a different (and higher overhead) architecture. This way the
> simple bulk data transfers can be handled by some small and relatively
> safe support in-kernel, and the inherently more dangerous PPTP protocol
> handling is done by jailed unprivileged userland code.
>

Please refer to my other replies for a detailed explanation.

-Girish

--
"unix soi qui mal y pense"

UNIX to him who evil thinks

+------------------------------------------------------------------+
| GnuPG key  : 0xC7BBF207  |  http://wwwkeys.nl.pgp.net            |
| Fingerprint: 2AFF C264 20CE C80C DDFF  CC15 AD3E F190 C7BB F207  |
+------------------------------------------------------------------+

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
In reply to this post by Ermal Luçi-2
On 19:57:10 Mar 16, Ermal Lu?i wrote:
> Well i see the reason that OpenBSD developers have about not handling
> application layer  in kernel since they do no want special cases in
> kernel in an application that should be general enough.
>

The idea is this.

pf(4) is concerned only about layer 4 and below. It only looks
at headers. Never the payload.

The only argument for doing PPTP in kernel would be for avoiding the
performance overhead of a user-land proxy. In fact this is what prompted
me to do an in kernel implementation the first time.
Moreover it was much easier.

There is a *huge* performance hit in a user-land PPTP proxy  since PPTP
is very different from ftp(1). PPTP is VPN technology and if every single
PPTP packet is to unnecessary travel all the way to user-land and back ,
then I am sure it will end up into something as bad as something like
SSLVPN. :)

If you watch carefully this design has no performance impact at all.

More explanation below.

> Though the way that you have written this patch it indeed inserts a
> special case in kernel. That is GRE.

This is necessary. No way out. If you have to make an omelette you have
to break eggs.

> How about doing this with no special cases but with just a plugin
> architecture for nat where let say if i write a rule like:
>
> nat on $interface  from any to any -> $extinterface with-proxy(pptp)
>
> And just check after the nat rule has been found if it has a flag to
> handle pptp or other protocols in the run.
> And then just call the appropriate function for this.
> It is a simple test insturction for users that do not want the
> overhead and the users that accept it are served as requested.

Users do not want overhead?

But we have *no* overhead here. It is as efficient as an in kernel
implementation.

See below.

> IE.
> pf_test() has
>
> if (direction == PF_OUT) {
> pf_get_translation(...)
> do appropriate modifications
> } else {
> pf_get_translation(...)
> do appropriate modifications
> }
>
> I just want to add this test  after that code:
> if (nr && nr->flags & PF_WITH_PROXY) {
> call the proxy rutines to identify what needs to be handled and
> continue normally
> }
>
> This test would be added after state_translation code too.
>

I have not understood what you are trying to say here.

If you really want to help OpenBSD then you should perhaps go ahead and
write something similar for RPC protocols. Contact reyk@ for further
details.

If you can write an rpc-proxy(8) that is modeled after pptp-proxy(8)
that rewrites the XID field, then that will definitely be useful.

Please bear in mind that there are several brain-dead protocols out
there.

In the case of PPTP, it is the stupid decision to choose 0 (zero) as the
callid.

Whereas if you take protocols like SIP or NFS or any RPC mechanism and
of course our famously broken FTP, they all suffer from the same
problem.

Firewalls are forced to do deep packet inspection.

How to do this without overhead?

Obviously there are minor nuances between these protocols. We can
perhaps live with an overhead for FTP but what about NFS?

And the icing on the cake is this.

By using the power of pf(4) redirection and a user-land proxy, the
unavoidable "fixing the brokenness" part happens in a jailed user-land
process. And the pf(4) and kernel work as always doing exactly what they
are supposed to do.

> This is just unnoticed overhead if you do not want the overhead and it
> does not bloat/break ABI.

Once again as I said before, there is no overhead here.

> I would be willing to write such a thing. And i know many people would
> be just happy to have something handled transparently than having to
> go through the configuration of a proxy.

Perhaps if I explain the genesis of PPTP NAT traversal solution for
OpenBSD all your above questions will get answered smoothly and you will
see that eventually everything falls in place like a beautiful painting.

The artist adds a lot of fine strokes to perfect a painting. After a lot
of touches he finally reaches a stage when he intuitively feels that the
painting cannot be improved, only spoilt with further diddling.

At that point he stops and decides that the work is complete.

PPTP has been a very hard problem for various reasons and the solution
has evolved through multiple iterations and a deep thought process.

I love evolution much more than revolution because it leads to no
bloodshed, no jerks, no shocks-- only simple smooth motion.

Let me get to the point straight.

The first time I did this implementation more than a year and a half
ago, I dismissed the frickin user-land approach as being completely
unacceptable.

It must be stupid to take packets all the way to user-land and back in a
VPN. C'mon there must be a better way.

I arrived at the in kernel payload manipulation approach. I knew
instinctively that something was definitely wrong somewhere but I was
glad that at least that made things easy for me and also avoid the much
dreaded overhead of useless context switches and packet copying.

Then the next step in evolution came about.

Henning tossed the idea of using the callID field in the pf(4) rule.

This idea never occurred to me since as far as I was concerned pf(4)
grammar was carved in stone and I was more or less sure that PPTP cannot
change pf(4) operation.

It took me a little while for the idea to sink in. But once I realized
that I could have the best of both worlds with a single change in the
design; that of augmenting pf(4) grammar to redirect packets based on
callID in addition to destination port numbers I knew it was Eureka.

The feeling that I could avoid all of the overhead of the frickin
approach and also play by the rules of doing payload manipulation in
user-land with this approach made me feel very satisfied with the beauty
of the solution. The fact that this happens in jailed code in /var/empty
makes it even sweeter.

As always OpenBSD should do things in the best manner possible. Always
place correctness and perfection before anything else.

We should be able to give the highest performance and at the same time
have an elegant design. It is easy to compromise one for the other.

The fact that ftp-proxy(8) was already available that did most of what I
wanted was the icing on the cake.

PPTP is a great deal messier than FTP but then it is also simpler in few
respects since the PPTP handshake is short lived and small.

Hope this puts things in perspective.

As you can see I thoroughly enjoyed the beauty and art of this fine work
of craftsmanship.

Review comments are welcome as always.

-Girish

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

patrick keshishian
Hi,

I should state that I don't have any vested interested in
discussed implementations in this thread. I'm just curious
and hoping to learn something here.


On Sun, Mar 16, 2008 at 9:50 PM, Girish Venkatachalam
<[hidden email]> wrote:

> On 19:57:10 Mar 16, Ermal Lu?i wrote:
>  > Well i see the reason that OpenBSD developers have about not handling
>  > application layer  in kernel since they do no want special cases in
>  > kernel in an application that should be general enough.
>  >
>
>  The idea is this.
>
>  pf(4) is concerned only about layer 4 and below. It only looks
>  at headers. Never the payload.

Maybe I'm misunderstanding something here, but it looks like you
will be contradicting yourself below where you mention using the
PPTP call-id:

>  Henning tossed the idea of using the callID field in the pf(4) rule.
>
>  This idea never occurred to me since as far as I was concerned pf(4)
>  grammar was carved in stone and I was more or less sure that PPTP cannot
>  change pf(4) operation.


(sorry for rearranging your comment "timeline")


>  > Though the way that you have written this patch it indeed inserts a
>  > special case in kernel. That is GRE.
>
>  This is necessary. No way out. If you have to make an omelette you have
>  to break eggs.
...
>  In the case of PPTP, it is the stupid decision to choose 0 (zero) as the
>  callid.
>
>  Whereas if you take protocols like SIP or NFS or any RPC mechanism and
>  of course our famously broken FTP, they all suffer from the same
>  problem.
>
>  Firewalls are forced to do deep packet inspection.

Just to be clear here, this is only for the NAT case, right?


>  The feeling that I could avoid all of the overhead of the frickin
>  approach and also play by the rules of doing payload manipulation in
>  user-land with this approach made me feel very satisfied with the beauty
>  of the solution. The fact that this happens in jailed code in /var/empty
>  makes it even sweeter.

However, you have introduced very "application" specific data
components into 'struct gre_h'.


>  As always OpenBSD should do things in the best manner possible. Always
>  place correctness and perfection before anything else.

Agreed.  Hence, my curiosity over your choice to augment gre_h
to solve a very specific problem case.



>  Review comments are welcome as always.
>
>  -Girish


Best regards,
--patrick

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 02:00:32 Mar 17, patrick keshishian wrote:
> >  The idea is this.
> >
> >  pf(4) is concerned only about layer 4 and below. It only looks
> >  at headers. Never the payload.
>
> Maybe I'm misunderstanding something here, but it looks like you
> will be contradicting yourself below where you mention using the
> PPTP call-id:

PPTP callid is in the GRE header. It is in layer 4.

PPTP callid manipulation happens inside TCP negotiations over port 1723.
Those are application level packets and pf(4) should not even know what
is inside.

PPTP has two parts. Read the man page in my patch or better read the
relevant RFC - 2637.

> >  Firewalls are forced to do deep packet inspection.
>
> Just to be clear here, this is only for the NAT case, right?
>

No. For every case.

> >  The feeling that I could avoid all of the overhead of the frickin
> >  approach and also play by the rules of doing payload manipulation in
> >  user-land with this approach made me feel very satisfied with the beauty
> >  of the solution. The fact that this happens in jailed code in /var/empty
> >  makes it even sweeter.
>
> However, you have introduced very "application" specific data
> components into 'struct gre_h'.
>

No. Only added fields for PPTP callid manipulation.

> >  As always OpenBSD should do things in the best manner possible. Always
> >  place correctness and perfection before anything else.
>
> Agreed.  Hence, my curiosity over your choice to augment gre_h
> to solve a very specific problem case.
>

There is no other way to achieve the goal. ;)

Without adding the said fields it is impossible to do this thing.

-Girish

--
"unix soi qui mal y pense"

UNIX to him who evil thinks

+------------------------------------------------------------------+
| GnuPG key  : 0xC7BBF207  |  http://wwwkeys.nl.pgp.net            |
| Fingerprint: 2AFF C264 20CE C80C DDFF  CC15 AD3E F190 C7BB F207  |
+------------------------------------------------------------------+

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

patrick keshishian
Hi again,

On Mon, Mar 17, 2008 at 3:38 AM, Girish Venkatachalam
<[hidden email]> wrote:

>  > >  Firewalls are forced to do deep packet inspection.
>  >
>  > Just to be clear here, this is only for the NAT case, right?
>
>  No. For every case.

Every case? I'm not sure if we are talking about the same thing
here or maybe my understanding of the "PPTP issue" that is being
addressed here is not complete.

Correct me if I'm wrong, but I believe the issue here is PPTP
connection from multiple clients to the same PPTP "server",
where all the "clients" are behind a NAT-ing pf firewall.  Or
more precisely you run into problems when source IP, destination
IP and call-id tripple is not unique across tunnels.

The more I think about this I can't help but wonder how many
OpenBSD users are facing this problem of running multiple
NAT-ed PPTP clients connecting to the same PPTP server?
Are they a majority or a minority?

--patrick

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 20:02:30 Mar 17, patrick keshishian wrote:
> >  No. For every case.
>
> Every case? I'm not sure if we are talking about the same thing
> here or maybe my understanding of the "PPTP issue" that is being
> addressed here is not complete.

I was saying that firewalls never look into payloads. Regardless of
whether there is NAT or not. Or was your question something else?

> Correct me if I'm wrong, but I believe the issue here is PPTP
> connection from multiple clients to the same PPTP "server",
> where all the "clients" are behind a NAT-ing pf firewall.  Or
> more precisely you run into problems when source IP, destination
> IP and call-id tripple is not unique across tunnels.


Correct. So far so good.

> The more I think about this I can't help but wonder how many
> OpenBSD users are facing this problem of running multiple
> NAT-ed PPTP clients connecting to the same PPTP server?
> Are they a majority or a minority?

I have no idea but I know that this feature has been pending for years.

-Girish

--
"unix soi qui mal y pense"

UNIX to him who evil thinks

+------------------------------------------------------------------+
| GnuPG key  : 0xC7BBF207  |  http://wwwkeys.nl.pgp.net            |
| Fingerprint: 2AFF C264 20CE C80C DDFF  CC15 AD3E F190 C7BB F207  |
+------------------------------------------------------------------+

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Stefan Sperling-5
In reply to this post by patrick keshishian
On Mon, Mar 17, 2008 at 08:02:30PM -0700, patrick keshishian wrote:
> The more I think about this I can't help but wonder how many
> OpenBSD users are facing this problem of running multiple
> NAT-ed PPTP clients connecting to the same PPTP server?

A typical situation where an OpenBSD firewall needs to NAT multiple
pptp sessions correctly is where students are sharing a flat and
they all want to connect to their university's pptp-based vpn
simultaneously from their laptops through an OpenBSD-based router.

I know that such setups exist in homes of fellow students at my
university, albeit they are using Linux instead of OpenBSD on their
home routers, one of the obvious reasons being OpenBSD's inability
to NAT pptp.

And yes, they could terminate a single tunnel on their router
and share that, but many non-techies don't even want to start
thinking about this possibility if VPN already works for them somehow.

And in any case, an OpenBSD router serving a large network behind NAT
(think conferences with private IP address space) will benefit
from this feature if only more than one user on the network starts
a pptp session. Being able to NAT pptp may well be a determining
factor whether OpenBSD can be used as a router on such occasions.

I myself sometimes tend to use the pptp-based VPN at my uni as a
fallback in case I am at a conference with open wifi. If that does
not work I'm confined to connecting to my home network via openvpn
and access the internet from there, with noticeable lag and low bandwidth.

And there are many people who don't even know what pptp is even though
they use it all the time (especially on conferences) and expect it to
just work (i.e. suits with their windows laptops).

So yes, quite a few people need this functionality, even though
many of them may not be direct users of OpenBSD like you and me.

> Are they a majority or a minority?

Why does that matter?

If support is being implemented, it means that at least one person can
make use of it, namely the developer, and even if only as an exercise
in writing good quality code that passes thorough review of OpenBSD
developers.


I would toss pptp out the window any second if I could, but the whole
world hasn't yet switched to ipsec, openvpn or similar much more sane
VPN solutions :(

--
stefan
http://stsp.name                                         PGP Key: 0xF59D25F0

[demime 1.01d removed an attachment of type application/pgp-signature]

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Ermal Luçi-2
In reply to this post by Girish Venkatachalam-2
>  There is a *huge* performance hit in a user-land PPTP proxy  since PPTP
>  is very different from ftp(1). PPTP is VPN technology and if every single
>  PPTP packet is to unnecessary travel all the way to user-land and back ,
>  then I am sure it will end up into something as bad as something like
>  SSLVPN. :)
>
>  If you watch carefully this design has no performance impact at all.
>
I wonder if this performance hit can be justified for a real-time
constraint protocol like SIP!
Which surely gets hit by this overhead.

Just playing the devil advocate here.

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

patrick keshishian
In reply to this post by Girish Venkatachalam-2
On Mon, Mar 17, 2008 at 9:36 PM, Girish Venkatachalam
<[hidden email]> wrote:

> On 20:02:30 Mar 17, patrick keshishian wrote:
>  > Correct me if I'm wrong, but I believe the issue here is PPTP
>  > connection from multiple clients to the same PPTP "server",
>  > where all the "clients" are behind a NAT-ing pf firewall.  Or
>  > more precisely you run into problems when source IP, destination
>  > IP and call-id tripple is not unique across tunnels.
>
>  Correct. So far so good.
>
>
>  > The more I think about this I can't help but wonder how many
>  > OpenBSD users are facing this problem of running multiple
>  > NAT-ed PPTP clients connecting to the same PPTP server?
>  > Are they a majority or a minority?
>
>  I have no idea but I know that this feature has been pending for years.

I am questioning whether it makes sense to introduce such changes
into the kernel and pf just to solve such a specific use-case.  I
would argue that it probably does not make all that much sense to
introduce so many changes into the kernel, complicated the code
with respect to maintenance, readability and security for the
benefit  of a very small use-case issue.

To quote you:

> As always OpenBSD should do things in the best manner possible.
> Always place correctness and perfection before anything else.

--patrick

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Claudio Jeker
In reply to this post by Ermal Luçi-2
On Tue, Mar 18, 2008 at 05:43:49PM +0100, Ermal Lugi wrote:

> >  There is a *huge* performance hit in a user-land PPTP proxy  since PPTP
> >  is very different from ftp(1). PPTP is VPN technology and if every single
> >  PPTP packet is to unnecessary travel all the way to user-land and back ,
> >  then I am sure it will end up into something as bad as something like
> >  SSLVPN. :)
> >
> >  If you watch carefully this design has no performance impact at all.
> >
> I wonder if this performance hit can be justified for a real-time
> constraint protocol like SIP!
> Which surely gets hit by this overhead.
>

What is so special with SIP? SIP is just the session initation protocol
the VoIP traffic uses a seperate RTP UDP connection which does not need to
be passed to userland.

> Just playing the devil advocate here.
>

--
:wq Claudio

123