Re: diff: fix bpf problem of pipex (was Re: diff: fix LCP keepalive failures on L2TP.)

2012-04-04 Thread mxb

On 04/04/2012 07:34 AM, YASUOKA Masahiko wrote:

On Tue, 31 Jan 2012 13:59:17 +0100
Sebastian Reitenbachsebas...@l00-bugdead-prods.de  wrote:

However, I noted with tcpdump, listening on tun0:

# tcpdump -n -i tun0
tcpdump: listening on tun0, link-type LOOP
13:51:15.354776
tcpdump: WARNING: compensating for unaligned libpcap packets
13:51:15.354795 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:16.334984
13:51:16.334997 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:17.355463

(snip)

The incoming packets look weird, and this is not only true for icmp,
with tcp/udp its the same.

Attached diff will fix the problem.

bpf requires to use different byte order for DLT_LOOP and DLT_NULL on
address family header.  pipex works with both pppx(DLT_NULL) and
tun(DLT_LOOP), so it should switch the byte order.

ok? or comment?

Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.26
diff -u -p -r1.26 pipex.c
--- sys/net/pipex.c 31 Jan 2012 12:04:20 -  1.26
+++ sys/net/pipex.c 8 Feb 2012 05:23:13 -
@@ -1162,8 +1162,13 @@ pipex_ip_input(struct mbuf *m0, struct p
len = m0-m_pkthdr.len;

  #if NBPFILTER  0
-   if (ifp-if_bpf)
-   bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN);
+   if (ifp-if_bpf) {
+   if (ifp-if_type == IFT_TUNNEL)
+   bpf_mtap_af(ifp-if_bpf, htonl(AF_INET), m0,
+   BPF_DIRECTION_IN);
+   else
+   bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN);
+   }
  #endif

s = splnet();
@@ -1239,8 +1244,14 @@ pipex_ip6_input(struct mbuf *m0, struct
len = m0-m_pkthdr.len;

  #if NBPFILTER  0
-   if (ifp-if_bpf)
-   bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN);
+   if (ifp-if_bpf) {
+   if (ifp-if_type == IFT_TUNNEL)
+   bpf_mtap_af(ifp-if_bpf, htonl(AF_INET6), m0,
+   BPF_DIRECTION_IN);
+   else
+   bpf_mtap_af(ifp-if_bpf, AF_INET6, m0,
+   BPF_DIRECTION_IN);
+   }
  #endif

s = splnet();



Breaks nothing for me so far.

//maxim



Re: diff: fix bpf problem of pipex (was Re: diff: fix LCP keepalive failures on L2TP.)

2012-04-04 Thread Claudio Jeker
On Wed, Apr 04, 2012 at 02:34:46PM +0900, Yasuoka Masahiko wrote:
 On Tue, 31 Jan 2012 13:59:17 +0100
 Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote:
  However, I noted with tcpdump, listening on tun0:
  
  # tcpdump -n -i tun0
  tcpdump: listening on tun0, link-type LOOP
  13:51:15.354776 
  tcpdump: WARNING: compensating for unaligned libpcap packets
  13:51:15.354795 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
  13:51:16.334984 
  13:51:16.334997 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
  13:51:17.355463 
 (snip)
  The incoming packets look weird, and this is not only true for icmp,
  with tcp/udp its the same.
 
 Attached diff will fix the problem.
 
 bpf requires to use different byte order for DLT_LOOP and DLT_NULL on
 address family header.  pipex works with both pppx(DLT_NULL) and
 tun(DLT_LOOP), so it should switch the byte order.
 
 ok? or comment?

Since pppx(4) is only used by npppd/pipex shouldn't we change the DLT there
so that we don't need any magic inside pipex?

-- 
:wq Claudio



diff: fix bpf problem of pipex (was Re: diff: fix LCP keepalive failures on L2TP.)

2012-04-03 Thread YASUOKA Masahiko
On Tue, 31 Jan 2012 13:59:17 +0100
Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote:
 However, I noted with tcpdump, listening on tun0:
 
 # tcpdump -n -i tun0
 tcpdump: listening on tun0, link-type LOOP
 13:51:15.354776 
 tcpdump: WARNING: compensating for unaligned libpcap packets
 13:51:15.354795 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
 13:51:16.334984 
 13:51:16.334997 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
 13:51:17.355463 
(snip)
 The incoming packets look weird, and this is not only true for icmp,
 with tcp/udp its the same.

Attached diff will fix the problem.

bpf requires to use different byte order for DLT_LOOP and DLT_NULL on
address family header.  pipex works with both pppx(DLT_NULL) and
tun(DLT_LOOP), so it should switch the byte order.

ok? or comment?

Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.26
diff -u -p -r1.26 pipex.c
--- sys/net/pipex.c 31 Jan 2012 12:04:20 -  1.26
+++ sys/net/pipex.c 8 Feb 2012 05:23:13 -
@@ -1162,8 +1162,13 @@ pipex_ip_input(struct mbuf *m0, struct p
len = m0-m_pkthdr.len;
 
 #if NBPFILTER  0
-   if (ifp-if_bpf)
-   bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN);
+   if (ifp-if_bpf) {
+   if (ifp-if_type == IFT_TUNNEL)
+   bpf_mtap_af(ifp-if_bpf, htonl(AF_INET), m0,
+   BPF_DIRECTION_IN);
+   else
+   bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN);
+   }
 #endif
 
s = splnet();
@@ -1239,8 +1244,14 @@ pipex_ip6_input(struct mbuf *m0, struct 
len = m0-m_pkthdr.len;
 
 #if NBPFILTER  0
-   if (ifp-if_bpf)
-   bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN);
+   if (ifp-if_bpf) {
+   if (ifp-if_type == IFT_TUNNEL)
+   bpf_mtap_af(ifp-if_bpf, htonl(AF_INET6), m0,
+   BPF_DIRECTION_IN);
+   else
+   bpf_mtap_af(ifp-if_bpf, AF_INET6, m0,
+   BPF_DIRECTION_IN);
+   }
 #endif
 
s = splnet();



Re: diff: fix LCP keepalive failures on L2TP.

2012-01-31 Thread Sebastian Reitenbach
On Tuesday, January 31, 2012 10:02 CET, Sebastian Reitenbach 
sebas...@l00-bugdead-prods.de wrote: 
 
 On Monday, January 30, 2012 15:43 CET, YASUOKA Masahiko yasu...@yasuoka.net 
 wrote: 
  
  On Mon, 30 Jan 2012 22:49:22 +0900 (JST)
  YASUOKA Masahiko yasu...@yasuoka.net wrote:
   pipex hook in udp_usrreq() mistakenly assumed that `inp' is
   connected.  The hook could not use the destination address properly,
   so it failed to find the pipex session.  This bug caused LCP keepalive
   failures on L2TP from client that does LCP keepalive and uses sequence
   number on the L2TP data channel (xl2tpd + pppd).
   
   The diff includes kernel header file changes.
   
   ok?
  
  Oops.  Let me update the diff.
  
  The given struct sockaddr object of
  pipex_l2tp_userland_lookup_session() became passed from the userland,
  so its address family must be checked.
  
  ok?
 
 tested IPSec/L2TP with xl2tpd, strongswan client. I did setup the tunnel 
 yesterday 
 evening, and its still alive this morning. Without the patch, the tunnel 
 usually died after
 a couple of minutes.
 
 So this definitely works for me, but I'm probably not the right guy to OK 
 that in this phase of 
 release cycle.
 
 I'll now try with Android phone, since I've seen the same problems with dying 
 tunnel too.

The tunnel established with Android 2.2, now also seems to be stable, at least 
for the last two hours and 15 minutes.
Prior to the patch, like with the Linux client, the tunnel died within a couple 
of minutes, maybe half an hour or hour.

However, I noted with tcpdump, listening on tun0:


# tcpdump -n -i tun0
tcpdump: listening on tun0, link-type LOOP
13:51:15.354776 
tcpdump: WARNING: compensating for unaligned libpcap packets
13:51:15.354795 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:16.334984 
13:51:16.334997 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:17.355463 
13:51:17.355474 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:18.375712 
13:51:18.375722 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:19.354994 
13:51:19.355004 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:20.375274 
13:51:20.375285 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:21.355354 
13:51:21.355364 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:22.334948 
13:51:22.334959 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:23.394820 
13:51:23.394831 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:24.374594 
13:51:24.374605 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:25.394998 
13:51:25.395009 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:26.394767 
13:51:26.394777 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:27.415028 
13:51:27.415039 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
13:51:28.375882 
13:51:28.375892 10.66.66.1  10.66.66.129: icmp: echo reply (DF)

Also when I show the contents with -X:


# tcpdump -n -i tun0 -s 2000 -X
tcpdump: listening on tun0, link-type LOOP
13:55:32.395223 
  : 4500 0054  4000 4001 a1a3 0a42 4281  E..T..@.@BB.
  0010: 0a42 4201 0800 6b54 6063 08bc 43e5 274f  .BB...kT`c..C.'O
  0020: c554 0800 0809 0a0b 0c0d 0e0f 1011 1213  .T..
  0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223   !#
  0040: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233  $%'()*+,-./0123
  0050: 3435 36374567

tcpdump: WARNING: compensating for unaligned libpcap packets
13:55:32.395249 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
  : 8266 4000 ff01 603c 0a42 4201 0a42 4281  .f@...`.BB..BB.
  0010:  7354 6063 08bc 43e5 274f c554 0800  ..sT`c..C.'O.T..
  0020: 0809 0a0b 0c0d 0e0f 1011 1213 1415 1617  
  0030: 1819 1a1b 1c1d 1e1f 2021 2223 2425 2627   !#$%'
  0040: 2829 2a2b 2c2d 2e2f 3031 3233 3435 3637  ()*+,-./01234567
  0050:  

13:55:34.315241 
  : 4500 0054  4000 4001 a1a3 0a42 4281  E..T..@.@BB.
  0010: 0a42 4201 0800 5e54 6063 08bd 44e5 274f  .BB...^T`c..D.'O
  0020: d153 0800 0809 0a0b 0c0d 0e0f 1011 1213  .S..
  0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223   !#
  0040: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233  $%'()*+,-./0123
  0050: 3435 36374567

13:55:34.315253 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
  : cd82 4000 ff01 1520 0a42 4201 0a42 4281  ..@ .BB..BB.
  0010:  6654 6063 08bd 44e5 274f d153 0800  ..fT`c..D.'O.S..
  0020: 0809 0a0b 0c0d 0e0f 1011 1213 1415 1617  
  0030: 1819 1a1b 1c1d 1e1f 2021 2223 2425 2627   !#$%'
  0040: 2829 2a2b 2c2d 2e2f 3031 3233 3435 3637  ()*+,-./01234567
  0050:  

13:55:34.994293 
  : 4500 0054  4000 4001 a1a3 0a42 4281  E..T..@.@BB.
  0010: 0a42 4201 0800 8752 6063 08be 45e5 274f  .BBR`c..E.'O
  0020: a754 0800 0809 0a0b 0c0d 0e0f 1011 1213  .T..
  0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223   

Re: diff: fix LCP keepalive failures on L2TP.

2012-01-31 Thread YASUOKA Masahiko
Hi,

On Tue, 31 Jan 2012 13:59:17 +0100
Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote:
 However, I noted with tcpdump, listening on tun0:
 # tcpdump -n -i tun0
 tcpdump: listening on tun0, link-type LOOP
 13:51:15.354776 
 tcpdump: WARNING: compensating for unaligned libpcap packets
 13:51:15.354795 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
 13:51:16.334984 
 13:51:16.334997 10.66.66.1  10.66.66.129: icmp: echo reply (DF)
 13:51:17.355463 
(snip)
 # tcpdump -n -i tun0 -s 2000 -X
 tcpdump: listening on tun0, link-type LOOP
 13:55:32.395223 
   : 4500 0054  4000 4001 a1a3 0a42 4281  E..T..@.@BB.
   0010: 0a42 4201 0800 6b54 6063 08bc 43e5 274f  .BB...kT`c..C.'O
   0020: c554 0800 0809 0a0b 0c0d 0e0f 1011 1213  .T..
   0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223   !#
   0040: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233  $%'()*+,-./0123
   0050: 3435 36374567

This is a separate issue.  I'll look into this.

Thank you for your tests and reports.

--yasuoka



diff: fix LCP keepalive failures on L2TP.

2012-01-30 Thread YASUOKA Masahiko
pipex hook in udp_usrreq() mistakenly assumed that `inp' is
connected.  The hook could not use the destination address properly,
so it failed to find the pipex session.  This bug caused LCP keepalive
failures on L2TP from client that does LCP keepalive and uses sequence
number on the L2TP data channel (xl2tpd + pppd).

The diff includes kernel header file changes.

ok?

Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.25
diff -u -p -r1.25 pipex.c
--- sys/net/pipex.c 23 Jan 2012 03:36:21 -  1.25
+++ sys/net/pipex.c 30 Jan 2012 12:11:16 -
@@ -2210,7 +2210,7 @@ pipex_l2tp_userland_lookup_session_ipv6(
 }
 #endif
 
-Static struct pipex_session *
+struct pipex_session *
 pipex_l2tp_userland_lookup_session(struct mbuf *m0, struct sockaddr *sa)
 {
struct pipex_l2tp_header l2tp;
Index: sys/net/pipex.h
===
RCS file: /cvs/src/sys/net/pipex.h,v
retrieving revision 1.11
diff -u -p -r1.11 pipex.h
--- sys/net/pipex.h 23 Jan 2012 03:36:21 -  1.11
+++ sys/net/pipex.h 30 Jan 2012 12:11:16 -
@@ -210,6 +210,7 @@ struct pipex_session  *pipex_pptp_lookup
 struct mbuf   *pipex_pptp_input (struct mbuf *, struct pipex_session 
*);
 struct pipex_session  *pipex_pptp_userland_lookup_session_ipv4 (struct mbuf *, 
struct in_addr);
 struct pipex_session  *pipex_pptp_userland_lookup_session_ipv6 (struct mbuf *, 
struct in6_addr);
+struct pipex_session  *pipex_l2tp_userland_lookup_session(struct mbuf *, 
struct sockaddr *);
 struct mbuf   *pipex_pptp_userland_output (struct mbuf *, struct 
pipex_session *);
 struct pipex_session  *pipex_l2tp_lookup_session (struct mbuf *, int);
 struct mbuf   *pipex_l2tp_input (struct mbuf *, int off, struct 
pipex_session *);
Index: sys/net/pipex_local.h
===
RCS file: /cvs/src/sys/net/pipex_local.h,v
retrieving revision 1.14
diff -u -p -r1.14 pipex_local.h
--- sys/net/pipex_local.h   25 Nov 2011 13:05:06 -  1.14
+++ sys/net/pipex_local.h   30 Jan 2012 12:11:16 -
@@ -406,7 +406,6 @@ Static struct pipex_session  *pipex_pptp
 
 #ifdef PIPEX_L2TP
 Static void  pipex_l2tp_output (struct mbuf *, struct 
pipex_session *);
-Static struct pipex_session  *pipex_l2tp_userland_lookup_session(struct mbuf 
*, struct sockaddr *);
 #endif
 
 #ifdef PIPEX_MPPE
Index: sys/netinet/udp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.145
diff -u -p -r1.145 udp_usrreq.c
--- sys/netinet/udp_usrreq.c8 Jul 2011 18:30:17 -   1.145
+++ sys/netinet/udp_usrreq.c30 Jan 2012 12:11:17 -
@@ -1198,6 +1198,12 @@ udp_usrreq(struct socket *so, int req, s
 #ifdef PIPEX
if (inp-inp_pipex) {
struct pipex_session *session;
+
+   if (addr != NULL) 
+   session =
+   pipex_l2tp_userland_lookup_session(m,
+   mtod(addr, struct sockaddr *));
+   else
 #ifdef INET6
if (inp-inp_flags  INP_IPV6)
session =



Re: diff: fix LCP keepalive failures on L2TP.

2012-01-30 Thread YASUOKA Masahiko
On Mon, 30 Jan 2012 22:49:22 +0900 (JST)
YASUOKA Masahiko yasu...@yasuoka.net wrote:
 pipex hook in udp_usrreq() mistakenly assumed that `inp' is
 connected.  The hook could not use the destination address properly,
 so it failed to find the pipex session.  This bug caused LCP keepalive
 failures on L2TP from client that does LCP keepalive and uses sequence
 number on the L2TP data channel (xl2tpd + pppd).
 
 The diff includes kernel header file changes.
 
 ok?

Oops.  Let me update the diff.

The given struct sockaddr object of
pipex_l2tp_userland_lookup_session() became passed from the userland,
so its address family must be checked.

ok?

Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.25
diff -u -p -r1.25 pipex.c
--- sys/net/pipex.c 23 Jan 2012 03:36:21 -  1.25
+++ sys/net/pipex.c 30 Jan 2012 14:37:31 -
@@ -2210,13 +2210,16 @@ pipex_l2tp_userland_lookup_session_ipv6(
 }
 #endif
 
-Static struct pipex_session *
+struct pipex_session *
 pipex_l2tp_userland_lookup_session(struct mbuf *m0, struct sockaddr *sa)
 {
struct pipex_l2tp_header l2tp;
struct pipex_hash_head *list;
struct pipex_session *session;
uint16_t session_id, tunnel_id, flags;
+
+   if (sa-sa_family != AF_INET  sa-sa_family != AF_INET6)
+   return (NULL);
 
/* pullup */
if (m0-m_pkthdr.len  sizeof(l2tp)) {
Index: sys/net/pipex.h
===
RCS file: /cvs/src/sys/net/pipex.h,v
retrieving revision 1.11
diff -u -p -r1.11 pipex.h
--- sys/net/pipex.h 23 Jan 2012 03:36:21 -  1.11
+++ sys/net/pipex.h 30 Jan 2012 14:37:31 -
@@ -210,6 +210,7 @@ struct pipex_session  *pipex_pptp_lookup
 struct mbuf   *pipex_pptp_input (struct mbuf *, struct pipex_session 
*);
 struct pipex_session  *pipex_pptp_userland_lookup_session_ipv4 (struct mbuf *, 
struct in_addr);
 struct pipex_session  *pipex_pptp_userland_lookup_session_ipv6 (struct mbuf *, 
struct in6_addr);
+struct pipex_session  *pipex_l2tp_userland_lookup_session(struct mbuf *, 
struct sockaddr *);
 struct mbuf   *pipex_pptp_userland_output (struct mbuf *, struct 
pipex_session *);
 struct pipex_session  *pipex_l2tp_lookup_session (struct mbuf *, int);
 struct mbuf   *pipex_l2tp_input (struct mbuf *, int off, struct 
pipex_session *);
Index: sys/net/pipex_local.h
===
RCS file: /cvs/src/sys/net/pipex_local.h,v
retrieving revision 1.14
diff -u -p -r1.14 pipex_local.h
--- sys/net/pipex_local.h   25 Nov 2011 13:05:06 -  1.14
+++ sys/net/pipex_local.h   30 Jan 2012 14:37:31 -
@@ -406,7 +406,6 @@ Static struct pipex_session  *pipex_pptp
 
 #ifdef PIPEX_L2TP
 Static void  pipex_l2tp_output (struct mbuf *, struct 
pipex_session *);
-Static struct pipex_session  *pipex_l2tp_userland_lookup_session(struct mbuf 
*, struct sockaddr *);
 #endif
 
 #ifdef PIPEX_MPPE
Index: sys/netinet/udp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.145
diff -u -p -r1.145 udp_usrreq.c
--- sys/netinet/udp_usrreq.c8 Jul 2011 18:30:17 -   1.145
+++ sys/netinet/udp_usrreq.c30 Jan 2012 14:37:32 -
@@ -1198,6 +1198,12 @@ udp_usrreq(struct socket *so, int req, s
 #ifdef PIPEX
if (inp-inp_pipex) {
struct pipex_session *session;
+
+   if (addr != NULL) 
+   session =
+   pipex_l2tp_userland_lookup_session(m,
+   mtod(addr, struct sockaddr *));
+   else
 #ifdef INET6
if (inp-inp_flags  INP_IPV6)
session =