From: Blair Steven <Blair.Steven@alliedtelesis.co.nz>
To: Steffen Klassert <steffen.klassert@secunet.com>,
Herbert Xu <herbert@gondor.apana.org.au>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: esp: Fix ESN generation under UDP encapsulation
Date: Thu, 23 Jun 2016 04:25:21 +0000 [thread overview]
Message-ID: <576B64B1.9030000@alliedtelesis.co.nz> (raw)
In-Reply-To: <20160620105934.GL7698@gauss.secunet.com>
This change tests okay in my setup.
Thanks very much
-Blair
On 06/20/2016 10:59 PM, Steffen Klassert wrote:
> On Sat, Jun 18, 2016 at 01:03:36PM +0800, Herbert Xu wrote:
>> On Fri, Jun 17, 2016 at 12:24:29PM +0200, Steffen Klassert wrote:
>>> On Wed, Jun 15, 2016 at 12:44:54AM +0000, Blair Steven wrote:
>>>> The restoration is happening - but being actioned on the wrong location.
>>>>
>>>> The destination IP address is being saved and restored, and the SPI
>>>> being written directly after the destination IP address. From my
>>>> understanding though, the ESN shuffling should have saved and restored
>>>> the UDP source / dest ports + SPI.
>>> Yes, looks like we copy with a wrong offset if udp encapsulation
>>> is used, skb_transport_header() does not point to the esp header
>>> in this case. Ccing Herbert, he changed this part when switching
>>> to the new AEAD interface with
>>> commit 7021b2e1cddd ("esp4: Switch to new AEAD interface").
>> Thanks for catching this!
>>
>> I think rather than changing the transport header (which isn't
>> quite right because UDP still is the transport protocol), we can
>> just save the offset locally. Something like this:
>>
>> ---8<---
>> Blair Steven noticed that ESN in conjunction with UDP encapsulation
>> is broken because we set the temporary ESP header to the wrong spot.
>>
>> This patch fixes this by first of all using the right spot, i.e.,
>> 4 bytes off the real ESP header, and then saving this information
>> so that after encryption we can restore it properly.
>>
>> Fixes: 7021b2e1cddd ("esp4: Switch to new AEAD interface")
>> Reported-by: Blair Steven <Blair.Steven@alliedtelesis.co.nz>
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> Looks good.
> Blair could you please test this?
>
> Thanks!
next prev parent reply other threads:[~2016-06-23 4:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-12 23:48 [PATCH] IPsec NAT-T issue Blair Steven
2016-06-12 23:48 ` [PATCH] esp: correct offset for ESN when using NAT-T Blair Steven
2016-06-14 21:12 ` David Miller
2016-06-13 10:20 ` [PATCH] IPsec NAT-T issue Steffen Klassert
2016-06-15 0:44 ` Blair Steven
2016-06-17 10:24 ` Steffen Klassert
2016-06-18 5:03 ` esp: Fix ESN generation under UDP encapsulation Herbert Xu
2016-06-20 10:59 ` Steffen Klassert
2016-06-23 4:25 ` Blair Steven [this message]
2016-06-23 10:40 ` Steffen Klassert
2016-06-23 15:52 ` David Miller
2016-06-24 1:52 ` Herbert Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=576B64B1.9030000@alliedtelesis.co.nz \
--to=blair.steven@alliedtelesis.co.nz \
--cc=herbert@gondor.apana.org.au \
--cc=netdev@vger.kernel.org \
--cc=steffen.klassert@secunet.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.