[patch] pf PPTP nat passthrough patch.

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

Re: [patch] pf PPTP nat passthrough patch.

Stefan Sperling-5
On Tue, Mar 18, 2008 at 10:11:24AM -0700, patrick keshishian wrote:
> 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.

Are you talking about Girish's implementation or Ermal's?
Because Girish's implementation tries to put as little code into
the kernel as possible...

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

IMHO "correctness and perfection" does not equal "can't be used to
do NAT at large conferences where multiple attendees might want to
use pptp." Rather, it means "can be used to do NAT at large conferences
where multiple attendees might want to use pptp and still uses a clever
way of doing so."

You might as well dismiss pf's ability to do NAT altogether because
NAT is an imperfect short-term workaround for IPv4 address space
shortage, and instead tell people to finally switch to IPv6 already
because this is the more correct and elegant solution. You would be
right, but in reality NAT and pptp are in widespread use and therefore
OpenBSD benefits from supporting them both because it makes it yet a bit
more applicable to real-world scenarios.

--
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.

patrick keshishian
On Tue, Mar 18, 2008 at 10:42 AM, Stefan Sperling <[hidden email]> wrote:

> On Tue, Mar 18, 2008 at 10:11:24AM -0700, patrick keshishian wrote:
>  > 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.
>
>  Are you talking about Girish's implementation or Ermal's?
>  Because Girish's implementation tries to put as little code into
>  the kernel as possible...

Both.


>  > > As always OpenBSD should do things in the best manner possible.
>  > > Always place correctness and perfection before anything else.
>
>  IMHO "correctness and perfection" does not equal "can't be used to
>  do NAT at large conferences where multiple attendees might want to
>  use pptp." Rather, it means "can be used to do NAT at large conferences
>  where multiple attendees might want to use pptp and still uses a clever
>  way of doing so."

The point you are making here is that pf is imperfect because it
fails to accommodate NAT for multiple PPTP clients connecting to
a single PPTP server;  PPTP arguably having a design flaw.

So you are saying it is OK to work into OpenBSD kernel provisions
to handle a PPTP quirk that only manifests itself in a very
specific (potentially extremely rare) use-cases, and by doing so
inherit the pitfalls I mentioned in my previous email: maintenance,
readability and potentially security?

--patrick

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 17:43:49 Mar 18, Ermal Lu?i wrote:
> 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.

I hear that Marco Pfafbatcher is developing a sip-proxy(8). Speak to
him.

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.

Stefan Sperling-5
In reply to this post by patrick keshishian
On Tue, Mar 18, 2008 at 04:26:51PM -0700, patrick keshishian wrote:
> The point you are making here is that pf is imperfect because it
> fails to accommodate NAT for multiple PPTP clients connecting to
> a single PPTP server;

Yes.

> PPTP arguably having a design flaw.

Yes.

> So you are saying it is OK to work into OpenBSD kernel provisions
> to handle a PPTP quirk that only manifests itself in a very
> specific (potentially extremely rare) use-cases, and by doing so
> inherit the pitfalls I mentioned in my previous email: maintenance,
> readability and potentially security?

If someone proposed a design that does everything outside the kernel
I would not object. I don't care much about performance of the
pptp NAT use case, I'd just like it to work.

Anyway, let's nail down what code your concerns are about:

The patch under review is adding three non-trivial functions to
the kernel, in terms of size, namely pf_match_translation_gre,
pf_get_translation_gre and pf_test_state_gre. The rest seems
to be glue code that calls these functions.

I trust henning and reyk in reviewing those functions, but would
not object helping with the review if necessary (not that I have
much experience in pf coding and code auditing, but I would not
object to making an effort if it helps getting the patch in).

--
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.

Stefan Sperling-5
In reply to this post by Girish Venkatachalam-2
On Sun, Mar 16, 2008 at 05:45:52AM +0530, Girish Venkatachalam 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.

Hi Girish (and others interested),

I finally got around to testing the pptp nat passthrough patch.

I'm just gonna post my test results here on tech@, I hate having to maintain
tons of addresses in a Cc:, and also I think it's good to have discussions
like this in the archives, and on the list for others to read (this being
open source and all...)

> 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).

It isn't a cinch to compile and run though, see below :)

> So there you go.
>
> Please find the tgz here.
>
> http://sirsasana.org/misc/pptp-patch.tgz

To test this thing I used 3 boxes, all running recent OpenBSD-current.
The setup looks like this:

                    inet
                     |
                     |
                    xl0 (217.197.84.188)
                 +-------+
                 | doyle |
                 +-------+
                    de0 (10.0.0.1)
                     |
                     |
   +------+       +--------+      +-----+
   | jack |-------| switch |------| len |
   +------+       +--------+      +-----+
   (10.0.0.2)                     (10.0.0.3)

jack and len will try to connect to the same pptp server (where I have
two different accounts), with their packets being NATted by doyle.

pf.conf on doyle:
---------------
# $OpenBSD: pf.conf,v 1.35 2008/02/29 17:04:55 reyk Exp $
#
# See pf.conf(5) and /usr/share/pf for syntax and examples.
# Remember to set net.inet.ip.forwarding=1 and/or net.inet6.ip6.forwarding=1
# in /etc/sysctl.conf if packets are to be forwarded between interfaces.

ext_if="xl0"
int_if="de0"

set skip on lo
scrub in

nat on $ext_if from !($ext_if) -> ($ext_if:0)
block in log
pass out

pass quick on $int_if no state
antispoof quick for { lo $int_if }

pass in on $ext_if proto icmp to ($ext_if)
pass in on $ext_if proto icmp6 to ($ext_if)
pass in on $ext_if proto tcp to ($ext_if) port ssh
---------------


/etc/ppp/ppp.conf on jack (see the pptp(8) man page installed by net/pptp):
---------------
default:
        set log Phase Chat LCP IPCP CCP tun command
test1:
        set device "!/usr/local/sbin/pptp --nolaunchpppd pptp.in-berlin.de"
        set authname user1
        set authkey pw1
        set mppe 128 stateless
        disable ipv6cp
---------------


/etc/ppp/ppp.conf on len:
---------------
default:
        set log Phase Chat LCP IPCP CCP tun command
test2:
        set device "!/usr/local/sbin/pptp --nolaunchpppd pptp.in-berlin.de"
        set authname user2
        set authkey pw2
        set mppe 128 stateless
        disable ipv6cp
---------------


Connecting a single host from behind the NAT works fine:

root@jack [~] # ppp -foreground -unit0 test1
Working in foreground mode
Using interface: tun0

Other shell:

root@jack [~] # ifconfig tun0
tun0: flags=8051<UP,POINTOPOINT,RUNNING,MULTICAST> mtu 1498
        groups: tun
        inet 217.197.85.95 --> 192.109.42.188 netmask 0xffffffff
root@jack [~] # ping -c 4 192.109.42.188
PING 192.109.42.188 (192.109.42.188): 56 data bytes
64 bytes from 192.109.42.188: icmp_seq=0 ttl=64 time=29.424 ms
64 bytes from 192.109.42.188: icmp_seq=1 ttl=64 time=36.390 ms
64 bytes from 192.109.42.188: icmp_seq=3 ttl=64 time=64.302 ms
--- 192.109.42.188 ping statistics ---
4 packets transmitted, 3 packets received, 25.0% packet loss
round-trip min/avg/max/std-dev = 29.424/43.372/64.302/15.070 ms


And packets pass through the tunnel OK:

root@jack [~] # route add -host www.de.openbsd.org 192.109.42.188
add host www.de.openbsd.org: gateway 192.109.42.188

Traceroute to www.de.openbsd.org goes through the tunnel:

                             My traceroute  [v0.72]
jack.stsp.name (0.0.0.0)                               Sat Mar 22 17:49:44
2008
Keys:  Help   Display mode   Restart statistics   Order of fields   quit
                                       Packets               Pings
 Host                                Loss%   Snt   Last   Avg  Best  Wrst
StDev
 1. vpn-gw.in-berlin.de               0.0%     2   23.9  23.8  23.7  23.9
0.1
 2. octalus.in-berlin.de              0.0%     2   24.8  24.9  24.8  25.0
0.1
 3. dfn.bcix.de                       0.0%     2   26.8  26.1  25.4  26.8
0.9
 4. zr-pot1-te0-0-0-5.x-win.dfn.de    0.0%     2   72.7  50.0  27.3  72.7
32.1
 5. zr-erl1-te0-0-0-0.x-win.dfn.de    0.0%     2   40.7  39.0  37.4  40.7
2.3
 6. xr-erl1-te2-1.x-win.dfn.de        0.0%     1   37.4  37.4  37.4  37.4
0.0
 7. ???
 8. enterprise.gate.uni-erlangen.de   0.0%     1   44.3  44.3  44.3  44.3
0.0
 9. constellation.gate.uni-erlangen.  0.0%     1   35.7  35.7  35.7  35.7
0.0
10. reliant.gate.uni-erlangen.de      0.0%     1   44.2  44.2  44.2  44.2
0.0
11. openbsd.informatik.uni-erlangen.  0.0%     1   38.4  38.4  38.4  38.4
0.0


Traceroute to www.openbsd.org (which has no explicit hostroute)
still goes through the NAT:

                             My traceroute  [v0.72]
jack.stsp.name (0.0.0.0)                               Sat Mar 22 17:50:29
2008
Keys:  Help   Display mode   Restart statistics   Order of fields   quit
                                       Packets               Pings
 Host                                Loss%   Snt   Last   Avg  Best  Wrst
StDev
 1. 10.0.0.1                          0.0%     4    0.3   0.3   0.3   0.4
0.0
 2. dougal.stsp.name                  0.0%     4    0.5   0.5   0.5   0.5
0.0
 3. vpn-gw.in-berlin.de               0.0%     4   23.0  23.9  21.1  26.4
2.3
 4. octalus.in-berlin.de              0.0%     4   23.2  27.4  23.2  37.6
6.9
 5. octalus-2-farluet.in-berlin.de   25.0%     4   22.9  23.3  22.9  23.4
0.3
 6. vl6.farjun.all.de                 0.0%     4   23.0  23.5  23.0  23.9
0.4
 7. gi4-0.221.core01.ber01.atlas.cog  0.0%     4   23.1  26.8  23.1  34.9
5.6
 8. po0-2.core01.ham01.atlas.cogentc  0.0%     4   41.6  31.6  27.3  41.6
6.8
 9. po2-0.core01.dus01.atlas.cogentc  0.0%     4   35.4  37.8  32.5  46.1
5.9
10. te1-4.ccr01.dus01.atlas.cogentco  0.0%     4   34.3  56.8  33.3 126.5
46.4
11. te7-1.ccr01.ams03.atlas.cogentco  0.0%     4   37.1  38.0  37.1  39.4
1.1
12. te8-1.mpd02.lon01.atlas.cogentco  0.0%     4   49.0  71.2  47.2 136.2
43.4
13. te2-2.ccr04.jfk02.atlas.cogentco  0.0%     4  119.4 119.6 116.4 123.0
2.7
14. vl3495.mpd01.jfk05.atlas.cogentc 50.0%     4  124.3 122.2 120.1 124.3
2.9
15. gi0-0-0.core01.jfk05.atlas.cogen  0.0%     4  147.3 128.3 118.0 147.3
16.5
16. telus.jfk05.atlas.cogentco.com    0.0%     3  120.5 122.7 120.5 126.9
3.6
17. edtnabxmdr00.bb.telus.com         0.0%     3  180.5 177.7 175.0 180.5
3.8
18. ???
19. gsb175-c6509-3-129.backbone.ualb  0.0%     3  185.5 180.9 177.0 185.5
4.3
20. 129.128.3.201                    33.3%     3  175.7 177.4 175.7 179.1
2.4
21. openbsd.sunsite.ualberta.ca       0.0%     3  193.4 181.8 173.5 193.4
10.3


Viewing both websites in lynx works fine. So far, so good.

Without your patch, I cannot connect len at the same time as jack:

Mar 24 22:51:39 len pptp[13953]: anon warn[decaps_hdlc:pptp_gre.c:223]: short
read (0): Invalid argument
Mar 24 22:51:39 len ppp[12752]: tun0: Phase: deflink: Connect time: 16 secs: 0
octets in, 270 octets out
Mar 24 22:51:39 len pptp[10521]: anon log[callmgr_main:pptp_callmgr.c:231]:
Closing connection (unhandled)
Mar 24 22:51:39 len ppp[12752]: tun0: Phase: deflink: 0 packets in, 5 packets
out
Mar 24 22:51:39 len pptp[10521]: anon log[ctrlp_rep:pptp_ctrl.c:251]: Sent
control packet type is 12 'Call-Clear-Request'
Mar 24 22:51:39 len ppp[12752]: tun0: Phase:  total 16 bytes/sec, peak 21
bytes/sec on Mon Mar 24 22:51:27 2008
Mar 24 22:51:39 len pptp[10521]: anon log[call_callback:pptp_callmgr.c:78]:
Closing connection (call state)
Mar 24 22:51:39 len ppp[12752]: tun0: Phase: deflink: HUPing 13953
Mar 24 22:51:39 len ppp[12752]: tun0: Phase: deflink: hangup -> closed
Mar 24 22:51:39 len ppp[12752]: tun0: Phase: bundle: Dead
Mar 24 22:51:39 len ppp[12752]: tun0: Phase: PPP Terminated (normal).

So I applied your patches, copied the pptp-proxy directory to
/usr/src/usr.sbin,
compiled a new GENERIC kernel, installed it, and ran:

cd /usr/src && make includes
cd /usr/src/sbin/pfctl && make obj depend && make && make install
cd /usr/src/usr.sbin/pptp-proxy && make obj depend && make

Here I got:

/usr/src/usr.sbin/pptp-proxy/obj -> /usr/obj/usr.sbin/pptp-proxy
mkdep -a -I/usr/src/usr.sbin/pptp-proxy pptp-proxy.c
cc -O2 -pipe  -I/usr/src/usr.sbin/pptp-proxy -Wall -Wstrict-prototypes
-Wmissing-prototypes -Wpointer-arith  -Wno-uninitialized   -c
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c: In function `pptp_dieout':
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:292: error: `next' undeclared (first
use in this function)
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:292: error: (Each undeclared
identifier is reported only once
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:292: error: for each function it
appears in.)
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c: In function `pptp_gotsignal':
/usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:314: error: `next' undeclared (first
use in this function)
*** Error code 1

Stop in /usr/src/usr.sbin/pptp-proxy (line 92 of /usr/share/mk/sys.mk).

:/
Did you not compile this before asking people to test it?
Or don't you get that error for some reason?

Anyway, I downloaded your pptp-patch tarball again, and you had indeed
updated it in the mean time (with no version number or date in the
tarball filename though...). But there wasn't a fix for this in there,
you had only removed a redundant .backup file, and made comment and
whitespace changes. So I stayed with the version I had, which I got
right when you posted the link to this thread in the mail I'm replying to.

This made it compile for me, not sure if it's really correct,
but looking at queue(3) I think it should be the correct way of doing this.
Correct me if I'm wrong on this one.

--- /tmp/pptp-patch/pptp-proxy/pptp-proxy.c Tue Feb 12 17:23:36 2008
+++ /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c Mon Mar 24 23:53:14 2008
@@ -289,7 +289,7 @@
  pptp_logmsg(LOG_INFO, "%s exiting after cleanup %d", __progname);

  for ( p = LIST_FIRST(&pptp_conn_head); p !=
-   LIST_END(&pptp_conn_head); p = next) {
+   LIST_END(&pptp_conn_head); p = tmp) {
  tmp = LIST_NEXT(p, next);
  pptp_remove_entry(pf_m, p);
  }
@@ -311,7 +311,7 @@
  pptp_logmsg(LOG_ERR, "%s exiting on signal %d", __progname, sig);

  for ( p = LIST_FIRST(&pptp_conn_head); p !=
-   LIST_END(&pptp_conn_head); p = next) {
+   LIST_END(&pptp_conn_head); p = tmp) {
  tmp = LIST_NEXT(p, next);
  pptp_remove_entry(pf_m, p);
  }


This now worked:

cd /usr/src/usr.sbin/pptp-proxy && make && make install

OK, now I should have the kernel patched, pfctl patched, and
pptp-proxy installed. I didn't bother installing the modified pf.conf.5,
you're not adding much information on how to actually use this, just BNF.

The pptp-proxy(8) manual page is nice though :)
Reading that manual page, I concluded that I needed to add
a 'pptp-proxy' user, so I did that:

pptp-proxy:*:92:92:PPTP Proxy:/var/empty:/sbin/nologin

(Why doesn't it have an underscore prefix like other system users?)

Now I rebooted doyle to the new kernel.

Now I changed pf.conf on doyle to this, as per the pptp-proxy(8) manual page:

---------------
# $OpenBSD: pf.conf,v 1.35 2008/02/29 17:04:55 reyk Exp $
#
# See pf.conf(5) and /usr/share/pf for syntax and examples.
# Remember to set net.inet.ip.forwarding=1 and/or net.inet6.ip6.forwarding=1
# in /etc/sysctl.conf if packets are to be forwarded between interfaces.

ext_if="xl0"
int_if="de0"

set skip on lo
scrub in

nat on $ext_if from !($ext_if) -> ($ext_if:0)
rdr-anchor "pptp-proxy/*"
rdr pass on $int_if proto tcp from $int_if:network to any port pptp -> \
        127.0.0.1 port 8723

block in log
pass out

pass quick on $int_if no state
antispoof quick for { lo $int_if }

pass in on $ext_if proto icmp to ($ext_if)
pass in on $ext_if proto icmp6 to ($ext_if)
pass in on $ext_if proto tcp to ($ext_if) port ssh
---------------

You should add the lines below marked with '+' to the filter ruleset
example in the manual page, otherwise the example is confusing.

+  Assuming the proxy connected to the PPTP server using
+  the $proxy source address:

   pass out proto tcp from $proxy to any port 1723

A similar bit can be found in ftp-proxy(8), that's where I
found out what should be specified for $proxy (after some research --
it really is a waste of time having to look around other man pages
to understand simple stuff like this).

I ended up not using that line anyway, as it's covered by the 'pass out'
rule in my rule set.

Now I did this, without errors:

pfctl -f /etc/pf.conf
/usr/sbin/pptp-proxy -d 7 -f


Next I started the pptp client on len, set a host route to www.de.openbsd.org
through the tunnel, verified with traceroute that the route was going through
the tunnel, and started pinging www.de.openbsd.org.


Then I let jack connect as well.
root@jack [~] # ppp -foreground -unit0 test1

Here is what pptp-proxy did on doyle:

root@doyle [~] # pptp-proxy -d 7 -f
pptp proxy listening on 127.0.0.1 port 8723
<#>1 accepted connection from 10.0.0.3
<#>1 PPTP TCP session 1/400 started: client 10.0.0.3 to server 192.109.42.32
via proxy 217.197.84.188
Inited rdr rule
<#>1 Adding rdr rule
<#>2 accepted connection from 10.0.0.2
<#>2 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32
via proxy 217.197.84.188
Inited rdr rule
<#>2 Adding rdr rule
Inited rdr rule
<#>2 removing PPTP rdr anchor as part of session cleanup
<#>16781312 pptp client close
<#>16781312 pptp server close
<#>3 accepted connection from 10.0.0.2
<#>3 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32
via proxy 217.197.84.188
Inited rdr rule
<#>3 Adding rdr rule
Inited rdr rule
<#>3 removing PPTP rdr anchor as part of session cleanup
<#>16781312 pptp client close
<#>16781312 pptp server close
Segmentation fault

Mmmh... Is that due to my compile fix being wrong?

Oddly enough, quite some time passed between this line being printed:
<#>16781312 pptp server close

and this line being printed:
Segmentation fault

As you can see from the log, I had to restart the pptp client on jack,
as it didn't manage to connect the first time and bailed out.
In fact it never connected.

Now let's try to find out where that segfault comes from:

(gdb) run -f -d7
Starting program: /usr/sbin/pptp-proxy -f -d7
pptp proxy listening on 127.0.0.1 port 8723
<#>1 accepted connection from 10.0.0.3
<#>1 PPTP TCP session 1/400 started: client 10.0.0.3 to server 192.109.42.32
via proxy 217.197.84.188
Inited rdr rule
<#>1 Adding rdr rule
<#>2 accepted connection from 10.0.0.2
<#>2 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32
via proxy 217.197.84.188
Inited rdr rule
<#>2 Adding rdr rule
Inited rdr rule
<#>2 removing PPTP rdr anchor as part of session cleanup
<#>16781312 pptp client close
<#>16781312 pptp server close
<#>3 accepted connection from 10.0.0.2
<#>3 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32
via proxy 217.197.84.188
Inited rdr rule
<#>3 Adding rdr rule
Inited rdr rule
<#>3 removing PPTP rdr anchor as part of session cleanup
<#>16781312 pptp client close
<#>16781312 pptp server close

Program received signal SIGSEGV, Segmentation fault.
bufferevent_write (bufev=0x0, data=0x87e10048, size=16)
    at /usr/src/lib/libevent/evbuffer.c:297
297             res = evbuffer_add(bufev->output, data, size);
(gdb) bt
#0  bufferevent_write (bufev=0x0, data=0x87e10048, size=16)
    at /usr/src/lib/libevent/evbuffer.c:297
#1  0x1c0018f8 in ?? ()
#2  0x00000000 in ?? ()
(gdb) info threads
(gdb) info frame
Stack level 0, frame at 0xcfbf5f50:
 eip = 0xbaddc9a in bufferevent_write (/usr/src/lib/libevent/evbuffer.c:297);
    saved eip 0x1c0018f8
 called by frame at 0xcfbf5f54
 source language c.
 Arglist at 0xcfbf5f48, args: bufev=0x0, data=0x87e10048, size=16
 Locals at 0xcfbf5f48, Previous frame's sp is 0xcfbf5f50
 Saved registers:
  ebx at 0xcfbf5f3c, ebp at 0xcfbf5f48, esi at 0xcfbf5f40, edi at 0xcfbf5f44,
  eip at 0xcfbf5f4c
(gdb)

Again, the segfault in this run came sort of out of the blue, timing-wise.

So, hmmm, corrupt stack. Looks like we got a nasy bug here, right? :)

It looks like bufferevent_write is not prepared to deal with
the bufev being NULL, so we probably shouldn't be passing NULL there...

int
bufferevent_write(struct bufferevent *bufev, void *data, size_t size)
{
        int res;

        res = evbuffer_add(bufev->output, data, size);
                          ^^^^^^^^^^^^^^^^


I will stop testing here for now, it's already past midnight here.

I hope you can reproduce the crash, if not it's probably a fault
in my compile fix or setup, but even if I did something totally
wrong (apart from the compile fix being totally wrong) I don't
think pptp-proxy should crash like that.

Let me know if you need more information.

Cheers,
--
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 01:15:23 Mar 25, Stefan Sperling wrote:
>
> Hi Girish (and others interested),
>
> I finally got around to testing the pptp nat passthrough patch.
>

Hello Stefan,

Amazing!

Many thanks for this Herculean effort.

I call it this because I know how hard it can be to test this thing.

> I'm just gonna post my test results here on tech@, I hate having to maintain
> tons of addresses in a Cc:, and also I think it's good to have discussions
> like this in the archives, and on the list for others to read (this being
> open source and all...)
>

Good thing. ;)

> It isn't a cinch to compile and run though, see below :)
>  

Quite right. The linked list double free was a grievous error on my
part.

Sorry about that.

> To test this thing I used 3 boxes, all running recent OpenBSD-current.
> The setup looks like this:

[...]

>
>                     inet
>                      |
>                      |
>                     xl0 (217.197.84.188)
>                  +-------+
>                  | doyle |
>                  +-------+
>                     de0 (10.0.0.1)
>                      |
>                      |
>    +------+       +--------+      +-----+
>    | jack |-------| switch |------| len |
>    +------+       +--------+      +-----+
>    (10.0.0.2)                     (10.0.0.3)
>

[...]

 

> So I applied your patches, copied the pptp-proxy directory to /usr/src/usr.sbin,
> compiled a new GENERIC kernel, installed it, and ran:
>
> cd /usr/src && make includes
> cd /usr/src/sbin/pfctl && make obj depend && make && make install
> cd /usr/src/usr.sbin/pptp-proxy && make obj depend && make
>
> Here I got:
>
> /usr/src/usr.sbin/pptp-proxy/obj -> /usr/obj/usr.sbin/pptp-proxy
> mkdep -a -I/usr/src/usr.sbin/pptp-proxy pptp-proxy.c
> cc -O2 -pipe  -I/usr/src/usr.sbin/pptp-proxy -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith  -Wno-uninitialized   -c /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c
> /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c: In function `pptp_dieout':
> /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:292: error: `next' undeclared (first use in this function)
> /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:292: error: (Each undeclared identifier is reported only once
> /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:292: error: for each function it appears in.)
> /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c: In function `pptp_gotsignal':
> /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c:314: error: `next' undeclared (first use in this function)
> *** Error code 1
>
> Stop in /usr/src/usr.sbin/pptp-proxy (line 92 of /usr/share/mk/sys.mk).
>
> :/
> Did you not compile this before asking people to test it?
> Or don't you get that error for some reason?
>

It was a linked list free issue. Since this patch was generated on Dec
15, 2007 I don't quite recollect what went wrong.

My guess is that I "fixed" the linked list freeing problem without
testing it. My bad.

Sorry for putting you thro' this.

However now I have come up with excellent code for freeing the list.

I got this from a post from Claudio few weeks ago to tech@.

Moreover the man page has clear examples for this stuff.

If only I read the man pages carefully enough...Anyway I fixed the problem for
good.

Please test once again.

> Anyway, I downloaded your pptp-patch tarball again, and you had indeed
> updated it in the mean time (with no version number or date in the
> tarball filename though...). But there wasn't a fix for this in there,
> you had only removed a redundant .backup file, and made comment and
> whitespace changes.

Right. I incorporated Stuart's comment on using leading tabs instead of spaces.

A style(8) issue.

> So I stayed with the version I had, which I got
> right when you posted the link to this thread in the mail I'm replying to.
>

Okay.

> This made it compile for me, not sure if it's really correct,

My code is the culprit. It was a silly issue but suicidal nevertheless.

> but looking at queue(3) I think it should be the correct way of doing this.
> Correct me if I'm wrong on this one.
>

Check out the new code. That is the best way to free a list.

> --- /tmp/pptp-patch/pptp-proxy/pptp-proxy.c Tue Feb 12 17:23:36 2008
> +++ /usr/src/usr.sbin/pptp-proxy/pptp-proxy.c Mon Mar 24 23:53:14 2008
> @@ -289,7 +289,7 @@
>   pptp_logmsg(LOG_INFO, "%s exiting after cleanup %d", __progname);
>  
>   for ( p = LIST_FIRST(&pptp_conn_head); p !=
> -   LIST_END(&pptp_conn_head); p = next) {
> +   LIST_END(&pptp_conn_head); p = tmp) {
>   tmp = LIST_NEXT(p, next);
>   pptp_remove_entry(pf_m, p);
>   }
> @@ -311,7 +311,7 @@
>   pptp_logmsg(LOG_ERR, "%s exiting on signal %d", __progname, sig);
>  
>   for ( p = LIST_FIRST(&pptp_conn_head); p !=
> -   LIST_END(&pptp_conn_head); p = next) {
> +   LIST_END(&pptp_conn_head); p = tmp) {
>   tmp = LIST_NEXT(p, next);
>   pptp_remove_entry(pf_m, p);
>   }
>
>
> This now worked:
>

But there is a better way to free a list.

 while (LIST_EMPTY(&pptp_conn_head))
                pptp_remove_entry(pf_m, LIST_FIRST(&pptp_conn_head));
   
Neat, short and cute. ;)

> cd /usr/src/usr.sbin/pptp-proxy && make && make install
>
> OK, now I should have the kernel patched, pfctl patched, and
> pptp-proxy installed. I didn't bother installing the modified pf.conf.5,
> you're not adding much information on how to actually use this, just BNF.
>
> The pptp-proxy(8) manual page is nice though :)

Thanks for the compliment.

> Reading that manual page, I concluded that I needed to add
> a 'pptp-proxy' user, so I did that:
>
> pptp-proxy:*:92:92:PPTP Proxy:/var/empty:/sbin/nologin
>
> (Why doesn't it have an underscore prefix like other system users?)
>

It sure should. I fixed it. Thanks.

Anyway I have modified the code and man page as per your suggestion.

Now, instead of "pptp-proxy" the user is called "_pptp".

> Now I rebooted doyle to the new kernel.
>
> Now I changed pf.conf on doyle to this, as per the pptp-proxy(8) manual page:
>
> ---------------
> # $OpenBSD: pf.conf,v 1.35 2008/02/29 17:04:55 reyk Exp $
> #
> # See pf.conf(5) and /usr/share/pf for syntax and examples.
> # Remember to set net.inet.ip.forwarding=1 and/or net.inet6.ip6.forwarding=1
> # in /etc/sysctl.conf if packets are to be forwarded between interfaces.
>
> ext_if="xl0"
> int_if="de0"
>
> set skip on lo
> scrub in
>
> nat on $ext_if from !($ext_if) -> ($ext_if:0)
> rdr-anchor "pptp-proxy/*"
> rdr pass on $int_if proto tcp from $int_if:network to any port pptp -> \
> 127.0.0.1 port 8723
>
> block in log
> pass out
>
> pass quick on $int_if no state
> antispoof quick for { lo $int_if }
>
> pass in on $ext_if proto icmp to ($ext_if)
> pass in on $ext_if proto icmp6 to ($ext_if)
> pass in on $ext_if proto tcp to ($ext_if) port ssh
> ---------------
>
> You should add the lines below marked with '+' to the filter ruleset
> example in the manual page, otherwise the example is confusing.
>
> +  Assuming the proxy connected to the PPTP server using
> +  the $proxy source address:
>
>    pass out proto tcp from $proxy to any port 1723
>
> A similar bit can be found in ftp-proxy(8), that's where I
> found out what should be specified for $proxy (after some research --
> it really is a waste of time having to look around other man pages
> to understand simple stuff like this).

What are you trying to say?

How can I fix this?

I think this can be easily handled in the install process. The default
/etc/pf.conf file can have entries pertinent for ftp-proxy(8) and
pptp-proxy(8) commented out.

That will be best way to solve this.


> I ended up not using that line anyway, as it's covered by the 'pass out'
> rule in my rule set.
>
> Now I did this, without errors:
>
> pfctl -f /etc/pf.conf
> /usr/sbin/pptp-proxy -d 7 -f
>

Right.

> Next I started the pptp client on len, set a host route to www.de.openbsd.org
> through the tunnel, verified with traceroute that the route was going through
> the tunnel, and started pinging www.de.openbsd.org.
>
>
> Then I let jack connect as well.
> root@jack [~] # ppp -foreground -unit0 test1
>
> Here is what pptp-proxy did on doyle:
>
> root@doyle [~] # pptp-proxy -d 7 -f
> pptp proxy listening on 127.0.0.1 port 8723
> <#>1 accepted connection from 10.0.0.3
> <#>1 PPTP TCP session 1/400 started: client 10.0.0.3 to server 192.109.42.32 via proxy 217.197.84.188
> Inited rdr rule
> <#>1 Adding rdr rule
> <#>2 accepted connection from 10.0.0.2
> <#>2 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32 via proxy 217.197.84.188
> Inited rdr rule
> <#>2 Adding rdr rule
> Inited rdr rule
> <#>2 removing PPTP rdr anchor as part of session cleanup
> <#>16781312 pptp client close
> <#>16781312 pptp server close
> <#>3 accepted connection from 10.0.0.2
> <#>3 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32 via proxy 217.197.84.188
> Inited rdr rule
> <#>3 Adding rdr rule
> Inited rdr rule
> <#>3 removing PPTP rdr anchor as part of session cleanup
> <#>16781312 pptp client close
> <#>16781312 pptp server close
> Segmentation fault
>

This is because of my "fix" for the linked list freeing using queue(3)
macros. Since I was misty about queue(3) functions, this error crept in.

I have fixed it in the new code.

I appreciate your testing and it will be really great if you could test
this new patch since I hope your test setup is still intact.

I know how hard it is to get a test setup for PPTP up and running.

So kindly do me this favor and test this out.

Thanks.

> Mmmh... Is that due to my compile fix being wrong?
>

No, I am the culprit...

> Oddly enough, quite some time passed between this line being printed:
> <#>16781312 pptp server close
>
> and this line being printed:
> Segmentation fault
>

I dunno how double free(3) bugs manifest themselves...

> As you can see from the log, I had to restart the pptp client on jack,
> as it didn't manage to connect the first time and bailed out.
> In fact it never connected.
>
> Now let's try to find out where that segfault comes from:
>
> (gdb) run -f -d7
> Starting program: /usr/sbin/pptp-proxy -f -d7
> pptp proxy listening on 127.0.0.1 port 8723
> <#>1 accepted connection from 10.0.0.3
> <#>1 PPTP TCP session 1/400 started: client 10.0.0.3 to server 192.109.42.32 via proxy 217.197.84.188
> Inited rdr rule
> <#>1 Adding rdr rule
> <#>2 accepted connection from 10.0.0.2
> <#>2 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32 via proxy 217.197.84.188
> Inited rdr rule
> <#>2 Adding rdr rule
> Inited rdr rule
> <#>2 removing PPTP rdr anchor as part of session cleanup
> <#>16781312 pptp client close
> <#>16781312 pptp server close
> <#>3 accepted connection from 10.0.0.2
> <#>3 PPTP TCP session 2/400 started: client 10.0.0.2 to server 192.109.42.32 via proxy 217.197.84.188
> Inited rdr rule
> <#>3 Adding rdr rule
> Inited rdr rule
> <#>3 removing PPTP rdr anchor as part of session cleanup
> <#>16781312 pptp client close
> <#>16781312 pptp server close
>
> Program received signal SIGSEGV, Segmentation fault.
> bufferevent_write (bufev=0x0, data=0x87e10048, size=16)
>     at /usr/src/lib/libevent/evbuffer.c:297
> 297             res = evbuffer_add(bufev->output, data, size);

The memory corruption leads to SEGFAULT in some unrelated portion of the
code. Once the stack is corrupted all bets are off. In this case it
might be the heap, but the issue remains.

> (gdb) bt
> #0  bufferevent_write (bufev=0x0, data=0x87e10048, size=16)
>     at /usr/src/lib/libevent/evbuffer.c:297
> #1  0x1c0018f8 in ?? ()
> #2  0x00000000 in ?? ()
> (gdb) info threads
> (gdb) info frame
> Stack level 0, frame at 0xcfbf5f50:
>  eip = 0xbaddc9a in bufferevent_write (/usr/src/lib/libevent/evbuffer.c:297);
>     saved eip 0x1c0018f8
>  called by frame at 0xcfbf5f54
>  source language c.
>  Arglist at 0xcfbf5f48, args: bufev=0x0, data=0x87e10048, size=16
>  Locals at 0xcfbf5f48, Previous frame's sp is 0xcfbf5f50
>  Saved registers:
>   ebx at 0xcfbf5f3c, ebp at 0xcfbf5f48, esi at 0xcfbf5f40, edi at 0xcfbf5f44,
>   eip at 0xcfbf5f4c
> (gdb)
>
> Again, the segfault in this run came sort of out of the blue, timing-wise.
>
> So, hmmm, corrupt stack. Looks like we got a nasy bug here, right? :)
>

No, a simple double free(3). ;)


> It looks like bufferevent_write is not prepared to deal with
> the bufev being NULL, so we probably shouldn't be passing NULL there...
>

No no...

Please test once again.

Thanks.

> int
> bufferevent_write(struct bufferevent *bufev, void *data, size_t size)
> {
>         int res;
>
>         res = evbuffer_add(bufev->output, data, size);
>                           ^^^^^^^^^^^^^^^^
>
>
> I will stop testing here for now, it's already past midnight here.
>

Great help Stefan.

> I hope you can reproduce the crash, if not it's probably a fault
> in my compile fix or setup, but even if I did something totally
> wrong (apart from the compile fix being totally wrong) I don't
> think pptp-proxy should crash like that.

Bah!

No code should ever crash.

Either like this or any other way for that matter. ;)

>
> Let me know if you need more information.
>

Yes. I need you to test this patch.

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

$ sha1 pptp-patch.tgz                                                                                
SHA1 (pptp-patch.tgz) = 83da35964835e3a53f177bd5ea13a3e9a34e361b

Please let me know if you find any bugs.

Crashes are a big no no of course. But any other inputs are also
welcome.

Also see if there are any memory leaks or hangs.

I shall be waiting for your reports/results.

Thanks for testing once again.

-Girish

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 11:21:11 Mar 25, Girish Venkatachalam wrote:
 
> But there is a better way to free a list.
>
>  while (LIST_EMPTY(&pptp_conn_head))
>                 pptp_remove_entry(pf_m, LIST_FIRST(&pptp_conn_head));
>    

Wrong. It should be

 while (!LIST_EMPTY(&pptp_conn_head))
          pptp_remove_entry(pf_m, LIST_FIRST(&pptp_conn_head));
         

Fixed in this patch set.

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

$ sha1 pptp-patch.tgz
SHA1 (pptp-patch.tgz) = fddc092e63536960a15e28bf3d3cb8b94a8cee08

Thanks.

-Girish

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Jonny Borg
In reply to this post by Ermal Luçi-2
Hi,

I read in this thread that there are work ongoing on pptp, are there any news? Is this patch implemented in 4.3?

Best regards
Jonny Borg

Ermal Luçi-2 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.

Stefan Sperling-5
On Mon, May 12, 2008 at 08:11:44AM -0700, Jonny Borg wrote:
> Hi,
>
> I read in this thread that there are work ongoing on pptp, are there any
> news? Is this patch implemented in 4.3?

It's still work in progress. Please help test it if you can.

Thanks,
Stefan

[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.

Jonny Borg
On Mon, May 12, 2008 at 5:21 PM, Stefan Sperling <[hidden email]> wrote:

> On Mon, May 12, 2008 at 08:11:44AM -0700, Jonny Borg wrote:
> > Hi,
> >
> > I read in this thread that there are work ongoing on pptp, are there any
> > news? Is this patch implemented in 4.3?
>
> It's still work in progress. Please help test it if you can.
>
> Thanks,
> Stefan
>

I would be glad to help out, the patch at
http://sirsasana.org/misc/pptp-patch.tgz
gives me 404, is there a change I can test this on 4.3?

// Jonny
<http://sirsasana.org/misc/pptp-patch.tgz>

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Stefan Sperling-5
On Mon, May 12, 2008 at 05:28:34PM +0200, Jonny Borg wrote:
> On Mon, May 12, 2008 at 5:21 PM, Stefan Sperling <[hidden email]> wrote:
>
> > On Mon, May 12, 2008 at 08:11:44AM -0700, Jonny Borg wrote:
> > > Hi,
> > >
> > > I read in this thread that there are work ongoing on pptp, are there
any

> > > news? Is this patch implemented in 4.3?
> >
> > It's still work in progress. Please help test it if you can.
> >
> > Thanks,
> > Stefan
> >
>
> I would be glad to help out, the patch at
> http://sirsasana.org/misc/pptp-patch.tgz
> gives me 404, is there a change I can test this on 4.3?
>
> // Jonny
> <http://sirsasana.org/misc/pptp-patch.tgz>

Let's see if Girish can fix this.

I have an updated pptp-proxy version in my mailbox somewhere
(by claudio@), I hope Girish will include that in his new tarball.

I have some tarball here but I don't know if it's the
latest version, it's probably the broken one I already tested.
I don't know what the latest version is either because Girish never
uses version numbers when naming his tarballs, he just posts his
tarballs' hashes and let's people look through their mail box to see
which of his mails he posted a hash in has the latest date stamp.
I'm too lazy to do that right now.

Stefan

[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.

Federico Schwindt-2
On Mon, May 12, 2008 at 05:42:53PM +0200, Stefan Sperling wrote:

> On Mon, May 12, 2008 at 05:28:34PM +0200, Jonny Borg wrote:
> > On Mon, May 12, 2008 at 5:21 PM, Stefan Sperling <[hidden email]> wrote:
> >
> > > On Mon, May 12, 2008 at 08:11:44AM -0700, Jonny Borg wrote:
> > > > Hi,
> > > >
> > > > I read in this thread that there are work ongoing on pptp, are there
> any
> > > > news? Is this patch implemented in 4.3?
> > >
> > > It's still work in progress. Please help test it if you can.
> > >
> > > Thanks,
> > > Stefan
> > >
> >
> > I would be glad to help out, the patch at
> > http://sirsasana.org/misc/pptp-patch.tgz
> > gives me 404, is there a change I can test this on 4.3?
> >
> > // Jonny
> > <http://sirsasana.org/misc/pptp-patch.tgz>
>
> Let's see if Girish can fix this.
>
> I have an updated pptp-proxy version in my mailbox somewhere
> (by claudio@), I hope Girish will include that in his new tarball.
>
> I have some tarball here but I don't know if it's the
> latest version, it's probably the broken one I already tested.
> I don't know what the latest version is either because Girish never
> uses version numbers when naming his tarballs, he just posts his
> tarballs' hashes and let's people look through their mail box to see
> which of his mails he posted a hash in has the latest date stamp.
> I'm too lazy to do that right now.

  I've started to work on this some time ago but never finished. If my
memory is right, I don't believe this is correct. The code should check
for gre packets instead.

  f.-

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
In reply to this post by Stefan Sperling-5
On 17:42:53 May 12, Stefan Sperling wrote:
> > <http://sirsasana.org/misc/pptp-patch.tgz>
>

The URL has changed.

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

> Let's see if Girish can fix this.
>
> I have an updated pptp-proxy version in my mailbox somewhere
> (by claudio@), I hope Girish will include that in his new tarball.
>

It is already included.

> I have some tarball here but I don't know if it's the
> latest version, it's probably the broken one I already tested.
> I don't know what the latest version is either because Girish never
> uses version numbers when naming his tarballs, he just posts his
> tarballs' hashes and let's people look through their mail box to see
> which of his mails he posted a hash in has the latest date stamp.
> I'm too lazy to do that right now.

I don't want to maintain a website or versioning. It will go into the
tree soon. Just wait until it is fully tested.

There is no point in releasing broken versions since the code is
very complex with lot of dependency on the kernel and pf.
(Which keeps changing every now and then)

Right now anyway you cannot test since the hackathon in Japan has
changed the NAT code completely. So I have to rewrite the kernel bits.

Please be patient. And use it once it is imported into -current.

And this will not happen before Jun 7 anyway.

Thanks.

-Girish

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Jonny Borg
On Mon, May 12, 2008 at 6:15 PM, Girish Venkatachalam <
[hidden email]> wrote:

> On 17:42:53 May 12, Stefan Sperling wrote:
> > > <http://sirsasana.org/misc/pptp-patch.tgz>
> >
>
> The URL has changed.
>
> http://sirsasana.org/.misc/pptp-patch.tgz
>
> > Let's see if Girish can fix this.
> >
> > I have an updated pptp-proxy version in my mailbox somewhere
> > (by claudio@), I hope Girish will include that in his new tarball.
> >
>
> It is already included.
>
> > I have some tarball here but I don't know if it's the
> > latest version, it's probably the broken one I already tested.
> > I don't know what the latest version is either because Girish never
> > uses version numbers when naming his tarballs, he just posts his
> > tarballs' hashes and let's people look through their mail box to see
> > which of his mails he posted a hash in has the latest date stamp.
> > I'm too lazy to do that right now.
>
> I don't want to maintain a website or versioning. It will go into the
> tree soon. Just wait until it is fully tested.
>
>
Thats great news :)

Is it possible to run this code on 4.3 or just on current?

// Jonny Borg

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Girish Venkatachalam-2
On 19:13:12 May 12, Jonny Borg wrote:
> Thats great news :)
>
> Is it possible to run this code on 4.3 or just on current?

Hold your breath till mid June and you won't have to do any patching as
it will be in the tree.

And it won't work with 4.3. Only 4.4. ;)

And ever after of course. ;)

-Girish

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Henning Brauer-5
In reply to this post by Girish Venkatachalam-2
* Girish Venkatachalam <[hidden email]> [2008-05-12 18:18]:
> Right now anyway you cannot test since the hackathon in Japan has
> changed the NAT code completely. So I have to rewrite the kernel bits.

but before we actually have to complete and commit that :)

--
Henning Brauer, [hidden email], [hidden email]
BS Web Services, http://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting - Hamburg & Amsterdam

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Ermal Luçi-2
On Tue, May 13, 2008 at 11:41 AM, Henning Brauer
<[hidden email]> wrote:
> * Girish Venkatachalam <[hidden email]> [2008-05-12 18:18]:
>
> > Right now anyway you cannot test since the hackathon in Japan has
>  > changed the NAT code completely. So I have to rewrite the kernel bits.
>
>  but before we actually have to complete and commit that :)
>

Can i have a preview of the changess.
I am interested since am customizing shaping code of pfSense but do
not want to have the hassle of difficult maintaining it in the long
run.

Regards,
Ermal

>  --
>  Henning Brauer, [hidden email], [hidden email]
>  BS Web Services, http://bsws.de
>  Full-Service ISP - Secure Hosting, Mail and DNS Services
>  Dedicated Servers, Rootservers, Application Hosting - Hamburg & Amsterdam

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Henning Brauer-5
* Ermal Lugi <[hidden email]> [2008-05-13 18:24]:

> On Tue, May 13, 2008 at 11:41 AM, Henning Brauer
> <[hidden email]> wrote:
> > * Girish Venkatachalam <[hidden email]> [2008-05-12 18:18]:
> >
> > > Right now anyway you cannot test since the hackathon in Japan has
> >  > changed the NAT code completely. So I have to rewrite the kernel bits.
> >
> >  but before we actually have to complete and commit that :)
> >
>
> Can i have a preview of the changess.
> I am interested since am customizing shaping code of pfSense but do
> not want to have the hassle of difficult maintaining it in the long
> run.

why don't you guys just wait for a few days.

it's not like we develop and maintain shit out of tree for years.

--
Henning Brauer, [hidden email], [hidden email]
BS Web Services, http://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting - Hamburg & Amsterdam

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Ryan McBride-3
On Wed, May 14, 2008 at 03:36:32AM +0200, Henning Brauer wrote:
> * Ermal Lugi <[hidden email]> [2008-05-13 18:24]:
> > Can i have a preview of the changess.
> > I am interested since am customizing shaping code of pfSense but do
> > not want to have the hassle of difficult maintaining it in the long
> > run.
>
> why don't you guys just wait for a few days.

We regularly email significant "previews of changes" to this list for
testing, and rarely do we ever hear any comments. On the other hand,
those who reply to such requests with good test reports will also likely
recieve other stuff to try in advance of commit.

Reply | Threaded
Open this post in threaded view
|

Re: [patch] pf PPTP nat passthrough patch.

Johan Borch
In reply to this post by Ermal Luçi-2
Ermal LuC'i <ermal.luci <at> gmail.com> writes:

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


Hi!

Any news regarding pptp-proxy for openbsd?

Regards
Johan

123