Re: Drop IPSec traffic that should be encapsulated but is not

2016-09-02 Thread Mike Belopuhov
On 1 September 2016 at 10:31, Vincent Gross  wrote:
> Our IPSec stack rejects UDP-encapsulated traffic using a non
> encapsulating SA, but not the other way around. This diff adds
> the missing check and the corresponding stat counter.
>
> Ok ?
>

Go for it.  OK mikeb



Re: Drop IPSec traffic that should be encapsulated but is not

2016-09-01 Thread Claer

On Thu, Sep 01 2016 at 46:18, Vincent Gross wrote:
> On Thu, 1 Sep 2016 18:02:14 +0200
> Claer  wrote:
> 
> > Hello,
> > 
> > In some production systems, I'm still using an old patch to isakmpd for
> > Nat-t.  When negociating SAs with ASA peers and OpenBSD is nated, you have
> > issues during negociation. The following discutions explain the issue
> > 
> > http://openbsd.7691.n7.nabble.com/isakmpd-NAT-T-interoperability-td173004.html
> > http://marc.info/?l=openbsd-tech&m=139140140105433&w=2
> > 
> > I think the patch is related to the parts of the code you are working on.
> > 
> 
> Actually it is not. The issue you are referencing is in isakmpd,
> whereas the diff below is in the OpenBSD kernel. Totally different
> stuff. I do not plan to look at isakmpd at the moment, as it only
> supports IKEv1, and its code is nearly twice the size of iked.
> 
> I do not have Cisco gear available to test, is this issue present when
> opening NAT-T tunnels with iked ?

Sorry I overlooked your diff. Unfortunately I didn't have the opportunity
to test with an ASA and iked. I don't have any ASA on hand (usually they are
on the client's side :))

> Cheers
Thanks for your time!

Claer

> > Would you mind looking at this issue also? :)
> > 
> > Thanks!
> > 
> > Claer
> > 
> > On Thu, Sep 01 2016 at 31:10, Vincent Gross wrote:
> > 
> > > Our IPSec stack rejects UDP-encapsulated traffic using a non
> > > encapsulating SA, but not the other way around. This diff adds
> > > the missing check and the corresponding stat counter.
> > > 
> > > Ok ?
> > > 
> > > Index: sys/netinet/ip_esp.h
> > > ===
> > > RCS file: /cvs/src/sys/netinet/ip_esp.h,v
> > > retrieving revision 1.42
> > > diff -u -p -r1.42 ip_esp.h
> > > --- sys/netinet/ip_esp.h  10 Jan 2010 12:43:07 -
> > > 1.42 +++ sys/netinet/ip_esp.h 1 Sep 2016 08:24:15 -
> > > @@ -62,6 +62,7 @@ struct espstat
> > >  u_int32_tesps_udpencin;  /* Input ESP-in-UDP packets */
> > >  u_int32_tesps_udpencout; /* Output ESP-in-UDP packets
> > > */ u_int32_t  esps_udpinval;  /* Invalid input ESP-in-UDP
> > > packets */
> > > +u_int32_tesps_udpneeded; /* Trying to use a ESP-in-UDP
> > > TDB */ };
> > >  
> > >  /*
> > > Index: sys/netinet/ipsec_input.c
> > > ===
> > > RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> > > retrieving revision 1.135
> > > diff -u -p -r1.135 ipsec_input.c
> > > --- sys/netinet/ipsec_input.c 10 Sep 2015 17:52:05
> > > - 1.135 +++ sys/netinet/ipsec_input.c 1 Sep 2016
> > > 08:24:16 - @@ -262,6 +262,16 @@ ipsec_common_input(struct mbuf
> > > *m, int s return EINVAL;
> > >   }
> > >  
> > > + if (!udpencap && (tdbp->tdb_flags & TDBF_UDPENCAP)) {
> > > + splx(s);
> > > + DPRINTF(("ipsec_common_input(): attempted to use
> > > udpencap "
> > > + "SA %s/%08x/%u\n", ipsp_address(&dst_address,
> > > buf,
> > > + sizeof(buf)), ntohl(spi), tdbp->tdb_sproto));
> > > + m_freem(m);
> > > + espstat.esps_udpneeded++;
> > > + return EINVAL;
> > > + }
> > > +
> > >   if (tdbp->tdb_xform == NULL) {
> > >   splx(s);
> > >   DPRINTF(("ipsec_common_input(): attempted to use
> > > uninitialized " Index: usr.bin/netstat/inet.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/netstat/inet.c,v
> > > retrieving revision 1.150
> > > diff -u -p -r1.150 inet.c
> > > --- usr.bin/netstat/inet.c27 Aug 2016 04:13:43 -
> > > 1.150 +++ usr.bin/netstat/inet.c  1 Sep 2016 08:24:16 -
> > > @@ -1073,6 +1073,7 @@ esp_stats(char *name)
> > >   p(esps_udpencin, "\t%u input UDP encapsulated ESP
> > > packet%s\n"); p(esps_udpencout, "\t%u output UDP encapsulated ESP
> > > packet%s\n"); p(esps_udpinval, "\t%u UDP packet%s for
> > > non-encapsulating TDB received\n");
> > > + p(esps_udpneeded, "\t%u raw ESP packet%s for encapsulating
> > > TDB received\n"); p(esps_ibytes, "\t%llu input byte%s\n");
> > >   p(esps_obytes, "\t%llu output byte%s\n");
> > >  
> > >   
> 



Re: Drop IPSec traffic that should be encapsulated but is not

2016-09-01 Thread Vincent Gross
On Thu, 1 Sep 2016 18:02:14 +0200
Claer  wrote:

> Hello,
> 
> In some production systems, I'm still using an old patch to isakmpd
> for Nat-t.
> When negociating SAs with ASA peers and OpenBSD is nated, you have
> issues during negociation. The following discutions explain the issue
> 
> http://openbsd.7691.n7.nabble.com/isakmpd-NAT-T-interoperability-td173004.html
> http://marc.info/?l=openbsd-tech&m=139140140105433&w=2
> 
> I think the patch is related to the parts of the code you are working
> on.
> 

Actually it is not. The issue you are referencing is in isakmpd,
whereas the diff below is in the OpenBSD kernel. Totally different
stuff. I do not plan to look at isakmpd at the moment, as it only
supports IKEv1, and its code is nearly twice the size of iked.

I do not have Cisco gear available to test, is this issue present when
opening NAT-T tunnels with iked ?

Cheers

> Would you mind looking at this issue also? :)
> 
> Thanks!
> 
> Claer
> 
> On Thu, Sep 01 2016 at 31:10, Vincent Gross wrote:
> 
> > Our IPSec stack rejects UDP-encapsulated traffic using a non
> > encapsulating SA, but not the other way around. This diff adds
> > the missing check and the corresponding stat counter.
> > 
> > Ok ?
> > 
> > Index: sys/netinet/ip_esp.h
> > ===
> > RCS file: /cvs/src/sys/netinet/ip_esp.h,v
> > retrieving revision 1.42
> > diff -u -p -r1.42 ip_esp.h
> > --- sys/netinet/ip_esp.h10 Jan 2010 12:43:07 -
> > 1.42 +++ sys/netinet/ip_esp.h   1 Sep 2016 08:24:15 -
> > @@ -62,6 +62,7 @@ struct espstat
> >  u_int32_t  esps_udpencin;  /* Input ESP-in-UDP packets */
> >  u_int32_t  esps_udpencout; /* Output ESP-in-UDP packets
> > */ u_int32_tesps_udpinval;  /* Invalid input ESP-in-UDP
> > packets */
> > +u_int32_t  esps_udpneeded; /* Trying to use a ESP-in-UDP
> > TDB */ };
> >  
> >  /*
> > Index: sys/netinet/ipsec_input.c
> > ===
> > RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> > retrieving revision 1.135
> > diff -u -p -r1.135 ipsec_input.c
> > --- sys/netinet/ipsec_input.c   10 Sep 2015 17:52:05
> > -   1.135 +++ sys/netinet/ipsec_input.c 1 Sep 2016
> > 08:24:16 - @@ -262,6 +262,16 @@ ipsec_common_input(struct mbuf
> > *m, int s return EINVAL;
> > }
> >  
> > +   if (!udpencap && (tdbp->tdb_flags & TDBF_UDPENCAP)) {
> > +   splx(s);
> > +   DPRINTF(("ipsec_common_input(): attempted to use
> > udpencap "
> > +   "SA %s/%08x/%u\n", ipsp_address(&dst_address,
> > buf,
> > +   sizeof(buf)), ntohl(spi), tdbp->tdb_sproto));
> > +   m_freem(m);
> > +   espstat.esps_udpneeded++;
> > +   return EINVAL;
> > +   }
> > +
> > if (tdbp->tdb_xform == NULL) {
> > splx(s);
> > DPRINTF(("ipsec_common_input(): attempted to use
> > uninitialized " Index: usr.bin/netstat/inet.c
> > ===
> > RCS file: /cvs/src/usr.bin/netstat/inet.c,v
> > retrieving revision 1.150
> > diff -u -p -r1.150 inet.c
> > --- usr.bin/netstat/inet.c  27 Aug 2016 04:13:43 -
> > 1.150 +++ usr.bin/netstat/inet.c1 Sep 2016 08:24:16 -
> > @@ -1073,6 +1073,7 @@ esp_stats(char *name)
> > p(esps_udpencin, "\t%u input UDP encapsulated ESP
> > packet%s\n"); p(esps_udpencout, "\t%u output UDP encapsulated ESP
> > packet%s\n"); p(esps_udpinval, "\t%u UDP packet%s for
> > non-encapsulating TDB received\n");
> > +   p(esps_udpneeded, "\t%u raw ESP packet%s for encapsulating
> > TDB received\n"); p(esps_ibytes, "\t%llu input byte%s\n");
> > p(esps_obytes, "\t%llu output byte%s\n");
> >  
> >   



Re: Drop IPSec traffic that should be encapsulated but is not

2016-09-01 Thread Claer
Hello,

In some production systems, I'm still using an old patch to isakmpd
for Nat-t.
When negociating SAs with ASA peers and OpenBSD is nated, you have issues
during negociation. The following discutions explain the issue

http://openbsd.7691.n7.nabble.com/isakmpd-NAT-T-interoperability-td173004.html
http://marc.info/?l=openbsd-tech&m=139140140105433&w=2

I think the patch is related to the parts of the code you are working on.

Would you mind looking at this issue also? :)

Thanks!

Claer

On Thu, Sep 01 2016 at 31:10, Vincent Gross wrote:

> Our IPSec stack rejects UDP-encapsulated traffic using a non
> encapsulating SA, but not the other way around. This diff adds
> the missing check and the corresponding stat counter.
> 
> Ok ?
> 
> Index: sys/netinet/ip_esp.h
> ===
> RCS file: /cvs/src/sys/netinet/ip_esp.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 ip_esp.h
> --- sys/netinet/ip_esp.h  10 Jan 2010 12:43:07 -  1.42
> +++ sys/netinet/ip_esp.h  1 Sep 2016 08:24:15 -
> @@ -62,6 +62,7 @@ struct espstat
>  u_int32_tesps_udpencin;  /* Input ESP-in-UDP packets */
>  u_int32_tesps_udpencout; /* Output ESP-in-UDP packets */
>  u_int32_tesps_udpinval;  /* Invalid input ESP-in-UDP packets */
> +u_int32_tesps_udpneeded; /* Trying to use a ESP-in-UDP TDB */
>  };
>  
>  /*
> Index: sys/netinet/ipsec_input.c
> ===
> RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> retrieving revision 1.135
> diff -u -p -r1.135 ipsec_input.c
> --- sys/netinet/ipsec_input.c 10 Sep 2015 17:52:05 -  1.135
> +++ sys/netinet/ipsec_input.c 1 Sep 2016 08:24:16 -
> @@ -262,6 +262,16 @@ ipsec_common_input(struct mbuf *m, int s
>   return EINVAL;
>   }
>  
> + if (!udpencap && (tdbp->tdb_flags & TDBF_UDPENCAP)) {
> + splx(s);
> + DPRINTF(("ipsec_common_input(): attempted to use udpencap "
> + "SA %s/%08x/%u\n", ipsp_address(&dst_address, buf,
> + sizeof(buf)), ntohl(spi), tdbp->tdb_sproto));
> + m_freem(m);
> + espstat.esps_udpneeded++;
> + return EINVAL;
> + }
> +
>   if (tdbp->tdb_xform == NULL) {
>   splx(s);
>   DPRINTF(("ipsec_common_input(): attempted to use uninitialized "
> Index: usr.bin/netstat/inet.c
> ===
> RCS file: /cvs/src/usr.bin/netstat/inet.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 inet.c
> --- usr.bin/netstat/inet.c27 Aug 2016 04:13:43 -  1.150
> +++ usr.bin/netstat/inet.c1 Sep 2016 08:24:16 -
> @@ -1073,6 +1073,7 @@ esp_stats(char *name)
>   p(esps_udpencin, "\t%u input UDP encapsulated ESP packet%s\n");
>   p(esps_udpencout, "\t%u output UDP encapsulated ESP packet%s\n");
>   p(esps_udpinval, "\t%u UDP packet%s for non-encapsulating TDB 
> received\n");
> + p(esps_udpneeded, "\t%u raw ESP packet%s for encapsulating TDB 
> received\n");
>   p(esps_ibytes, "\t%llu input byte%s\n");
>   p(esps_obytes, "\t%llu output byte%s\n");
>  
> 



Drop IPSec traffic that should be encapsulated but is not

2016-09-01 Thread Vincent Gross
Our IPSec stack rejects UDP-encapsulated traffic using a non
encapsulating SA, but not the other way around. This diff adds
the missing check and the corresponding stat counter.

Ok ?

Index: sys/netinet/ip_esp.h
===
RCS file: /cvs/src/sys/netinet/ip_esp.h,v
retrieving revision 1.42
diff -u -p -r1.42 ip_esp.h
--- sys/netinet/ip_esp.h10 Jan 2010 12:43:07 -  1.42
+++ sys/netinet/ip_esp.h1 Sep 2016 08:24:15 -
@@ -62,6 +62,7 @@ struct espstat
 u_int32_t  esps_udpencin;  /* Input ESP-in-UDP packets */
 u_int32_t  esps_udpencout; /* Output ESP-in-UDP packets */
 u_int32_t  esps_udpinval;  /* Invalid input ESP-in-UDP packets */
+u_int32_t  esps_udpneeded; /* Trying to use a ESP-in-UDP TDB */
 };
 
 /*
Index: sys/netinet/ipsec_input.c
===
RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.135
diff -u -p -r1.135 ipsec_input.c
--- sys/netinet/ipsec_input.c   10 Sep 2015 17:52:05 -  1.135
+++ sys/netinet/ipsec_input.c   1 Sep 2016 08:24:16 -
@@ -262,6 +262,16 @@ ipsec_common_input(struct mbuf *m, int s
return EINVAL;
}
 
+   if (!udpencap && (tdbp->tdb_flags & TDBF_UDPENCAP)) {
+   splx(s);
+   DPRINTF(("ipsec_common_input(): attempted to use udpencap "
+   "SA %s/%08x/%u\n", ipsp_address(&dst_address, buf,
+   sizeof(buf)), ntohl(spi), tdbp->tdb_sproto));
+   m_freem(m);
+   espstat.esps_udpneeded++;
+   return EINVAL;
+   }
+
if (tdbp->tdb_xform == NULL) {
splx(s);
DPRINTF(("ipsec_common_input(): attempted to use uninitialized "
Index: usr.bin/netstat/inet.c
===
RCS file: /cvs/src/usr.bin/netstat/inet.c,v
retrieving revision 1.150
diff -u -p -r1.150 inet.c
--- usr.bin/netstat/inet.c  27 Aug 2016 04:13:43 -  1.150
+++ usr.bin/netstat/inet.c  1 Sep 2016 08:24:16 -
@@ -1073,6 +1073,7 @@ esp_stats(char *name)
p(esps_udpencin, "\t%u input UDP encapsulated ESP packet%s\n");
p(esps_udpencout, "\t%u output UDP encapsulated ESP packet%s\n");
p(esps_udpinval, "\t%u UDP packet%s for non-encapsulating TDB 
received\n");
+   p(esps_udpneeded, "\t%u raw ESP packet%s for encapsulating TDB 
received\n");
p(esps_ibytes, "\t%llu input byte%s\n");
p(esps_obytes, "\t%llu output byte%s\n");