2008-02-13 13:17:50

by Patrick McHardy

[permalink] [raw]
Subject: HIFN+IPsec crashes in current -git

I'm getting crashes when using HIFN and IPsec (ESP with
AES + MD5) in the current -git tree. I didn't capture the
Oops, but there seem to be a number of problems:

- hifn_setup_session walks over the scatterlist, subtracting
the scatterlist element size from nbytes until nbytes
reaches zero. In my case nbytes is 12 byte smaller than
the scatterlist, so nbytes underflows and it oopses when
walking over the of the scatterlist.

- similar problem in ablkcipher_walk

- After adding a hack to only walk the correct amount of
bytes, I got another crash because the return value
of ablkcipher_walk is not checked for errors, which
can cause sg_num to take a very large value.

- After adding more hacks to work around the crash,
I got "bad page state" on resume and a refcount
underflow in dst_release() called by xfrm_input(),
but that may very well be caused by my hacks.

I couldn't figure out where in the crypto code the
nbytes decrement by 12 bytes compared to the length
seen when setting up the crypto operation happens
or I might have tried to properly fix it myself.
I'll happily test patches in case someone more
familiar with the code does a proper fix.


2008-02-14 02:13:29

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Hi Patric.

On Wed, Feb 13, 2008 at 02:17:45PM +0100, Patrick McHardy ([email protected]) wrote:
> I'm getting crashes when using HIFN and IPsec (ESP with
> AES + MD5) in the current -git tree. I didn't capture the
> Oops, but there seem to be a number of problems:
>
> - hifn_setup_session walks over the scatterlist, subtracting
> the scatterlist element size from nbytes until nbytes
> reaches zero. In my case nbytes is 12 byte smaller than
> the scatterlist, so nbytes underflows and it oopses when
> walking over the of the scatterlist.

How is it possible? If I understood correctly ablkcipher_request->nbytes
has to have value equal to number of bytes placed into underlying
scatterlists, so if they do not match, hifn driver will not work at all.

> I couldn't figure out where in the crypto code the
> nbytes decrement by 12 bytes compared to the length
> seen when setting up the crypto operation happens
> or I might have tried to properly fix it myself.
> I'll happily test patches in case someone more
> familiar with the code does a proper fix.

Any chance you can apply following patch and check output for correct
and broken cases (it will produce 2 or 3 debug strings for each crypto
operation)?

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..b8b088d 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1558,6 +1558,23 @@ err_out_unmap:
return err;
}

+static void hifn_dump_req(struct ablkcipher_request *req, const char *prefix)
+{
+ int nbytes = (signed)req->nbytes;
+ struct scatterlist *src, *dst;
+ int idx = 0;
+
+ printk("%s: nbytes: %u, ", prefix, nbytes);
+ while (nbytes > 0) {
+ src = &req->src[idx];
+ dst = &req->dst[idx];
+
+ printk("%u/%u ", src->length, dst->length);
+ nbytes -= src->length;
+ }
+ printk("\n");
+}
+
static int hifn_setup_session(struct ablkcipher_request *req)
{
struct hifn_context *ctx = crypto_tfm_ctx(req->base.tfm);
@@ -1572,6 +1589,8 @@ static int hifn_setup_session(struct ablkcipher_request *req)
unsigned alignmask =
crypto_ablkcipher_alignmask(crypto_ablkcipher_reqtfm(req));

+ hifn_dump_req(req, __func__);
+
if (ctx->iv && !ctx->ivsize && ctx->mode != ACRYPTO_MODE_ECB)
goto err_out_exit;

@@ -2182,6 +2201,8 @@ static int hifn_process_queue(struct hifn_device *dev)
ctx = crypto_tfm_ctx(async_req->tfm);
req = container_of(async_req, struct ablkcipher_request, base);

+ hifn_dump_req(req, __func__);
+
err = hifn_handle_req(req);
if (err)
break;
@@ -2196,6 +2217,8 @@ static int hifn_setup_crypto(struct ablkcipher_request *req, u8 op,
int err;
struct hifn_context *ctx = crypto_tfm_ctx(req->base.tfm);
struct hifn_device *dev = ctx->dev;
+
+ hifn_dump_req(req, __func__);

err = hifn_setup_crypto_req(req, op, type, mode);
if (err)


--
Evgeniy Polyakov

2008-02-14 09:31:12

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Hi Patrick.

On Wed, Feb 13, 2008 at 05:44:42PM +0300, Evgeniy Polyakov ([email protected]) wrote:
> Any chance you can apply following patch and check output for correct
> and broken cases (it will produce 2 or 3 debug strings for each crypto
> operation)?
>
> diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
> index dfbf24c..b8b088d 100644
> --- a/drivers/crypto/hifn_795x.c
> +++ b/drivers/crypto/hifn_795x.c
> @@ -1558,6 +1558,23 @@ err_out_unmap:
> return err;
> }
>
> +static void hifn_dump_req(struct ablkcipher_request *req, const char *prefix)
> +{
> + int nbytes = (signed)req->nbytes;
> + struct scatterlist *src, *dst;
> + int idx = 0;
> +
> + printk("%s: nbytes: %u, ", prefix, nbytes);
> + while (nbytes > 0) {
> + src = &req->src[idx];
> + dst = &req->dst[idx];
> +
> + printk("%u/%u ", src->length, dst->length);
> + nbytes -= src->length;

Ouch, forgot idx++;

--
Evgeniy Polyakov

2008-02-19 06:23:30

by Herbert Xu

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Patrick McHardy <[email protected]> wrote:
>
> I couldn't figure out where in the crypto code the
> nbytes decrement by 12 bytes compared to the length
> seen when setting up the crypto operation happens
> or I might have tried to properly fix it myself.
> I'll happily test patches in case someone more
> familiar with the code does a proper fix.

The usual ICV is 12 bytes long so that could be where it's coming
from.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-02-19 16:11:12

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Evgeniy Polyakov wrote:
> Hi Patrick.
>
> On Wed, Feb 13, 2008 at 05:44:42PM +0300, Evgeniy Polyakov ([email protected]) wrote:
>> Any chance you can apply following patch and check output for correct
>> and broken cases (it will produce 2 or 3 debug strings for each crypto
>> operation)?
>>
>> diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
>> index dfbf24c..b8b088d 100644
>> --- a/drivers/crypto/hifn_795x.c
>> +++ b/drivers/crypto/hifn_795x.c
>> @@ -1558,6 +1558,23 @@ err_out_unmap:
>> return err;
>> }
>>
>> +static void hifn_dump_req(struct ablkcipher_request *req, const char *prefix)
>> +{
>> + int nbytes = (signed)req->nbytes;
>> + struct scatterlist *src, *dst;
>> + int idx = 0;
>> +
>> + printk("%s: nbytes: %u, ", prefix, nbytes);
>> + while (nbytes > 0) {
>> + src = &req->src[idx];
>> + dst = &req->dst[idx];
>> +
>> + printk("%u/%u ", src->length, dst->length);
>> + nbytes -= src->length;
>
> Ouch, forgot idx++;


Unfortunately I'm unable to boot current -git, this time it
hangs while trying to mount dm-crypt devices. The last output
I get is:

[ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.148790] hifn_setup_session: nbytes: 512, 512/512
[ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.148790] hifn_setup_session: nbytes: 512, 512/512
[ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.148790] hifn_setup_session: nbytes: 512, 512/512
[ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.148837] hifn_setup_session: nbytes: 512, 512/512
[ 15.148958] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.149085] hifn_setup_session: nbytes: 512, 512/512
[ 15.149206] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.149332] hifn_setup_session: nbytes: 512, 512/512
[ 15.149455] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.149582] hifn_setup_session: nbytes: 512, 512/512
[ 15.149705] hifn_setup_crypto: nbytes: 512, 512/512
[ 15.149871] hifn_setup_session: nbytes: 512, 512/512

I'll try to narrow it down.

2008-02-19 16:14:48

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Patrick McHardy wrote:
> Evgeniy Polyakov wrote:
>> Hi Patrick.
>>
>> On Wed, Feb 13, 2008 at 05:44:42PM +0300, Evgeniy Polyakov
>> ([email protected]) wrote:
>>> Any chance you can apply following patch and check output for correct
>>> and broken cases (it will produce 2 or 3 debug strings for each crypto
>>> operation)?
>>>
>>> diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
>>> index dfbf24c..b8b088d 100644
>>> --- a/drivers/crypto/hifn_795x.c
>>> +++ b/drivers/crypto/hifn_795x.c
>>> @@ -1558,6 +1558,23 @@ err_out_unmap:
>>> return err;
>>> }
>>>
>>> +static void hifn_dump_req(struct ablkcipher_request *req, const char
>>> *prefix)
>>> +{
>>> + int nbytes = (signed)req->nbytes;
>>> + struct scatterlist *src, *dst;
>>> + int idx = 0;
>>> +
>>> + printk("%s: nbytes: %u, ", prefix, nbytes);
>>> + while (nbytes > 0) {
>>> + src = &req->src[idx];
>>> + dst = &req->dst[idx];
>>> +
>>> + printk("%u/%u ", src->length, dst->length);
>>> + nbytes -= src->length;
>>
>> Ouch, forgot idx++;
>
>
> Unfortunately I'm unable to boot current -git, this time it
> hangs while trying to mount dm-crypt devices. The last output
> I get is:
>
> [ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.148790] hifn_setup_session: nbytes: 512, 512/512
> [ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.148790] hifn_setup_session: nbytes: 512, 512/512
> [ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.148790] hifn_setup_session: nbytes: 512, 512/512
> [ 15.148790] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.148837] hifn_setup_session: nbytes: 512, 512/512
> [ 15.148958] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.149085] hifn_setup_session: nbytes: 512, 512/512
> [ 15.149206] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.149332] hifn_setup_session: nbytes: 512, 512/512
> [ 15.149455] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.149582] hifn_setup_session: nbytes: 512, 512/512
> [ 15.149705] hifn_setup_crypto: nbytes: 512, 512/512
> [ 15.149871] hifn_setup_session: nbytes: 512, 512/512
>
> I'll try to narrow it down.

BTW, I also get this just before the HIFN debug output:

[ 14.695659] device-mapper: uevent: version 1.0.3
[ 14.697257] device-mapper: ioctl: 4.13.0-ioctl (2007-10-18)
initialised: [email protected]
[ 15.137123] device-mapper: crypt: Selected cipher does not support IVs

Not sure if its related, but I don't get this when using software
crypto.

2008-02-19 16:27:41

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Herbert Xu wrote:
> Patrick McHardy <[email protected]> wrote:
>> I couldn't figure out where in the crypto code the
>> nbytes decrement by 12 bytes compared to the length
>> seen when setting up the crypto operation happens
>> or I might have tried to properly fix it myself.
>> I'll happily test patches in case someone more
>> familiar with the code does a proper fix.
>
> The usual ICV is 12 bytes long so that could be where it's coming
> from.


Yes, probably. So I take it the assumption in HIFN that the
sg-list length matches req->nbytes is incorrect?

2008-02-20 00:53:31

by Herbert Xu

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

On Tue, Feb 19, 2008 at 05:27:25PM +0100, Patrick McHardy wrote:
>
> Yes, probably. So I take it the assumption in HIFN that the
> sg-list length matches req->nbytes is incorrect?

Where were you seeing the discrepancy? If it's at the point of
entry into the HIFN code then the bug is further up. If it's
within the HIFN code then I don't know where 12 came from either
since the ICV is not visible to it.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-02-20 12:33:40

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Herbert Xu wrote:
> On Tue, Feb 19, 2008 at 05:27:25PM +0100, Patrick McHardy wrote:
>> Yes, probably. So I take it the assumption in HIFN that the
>> sg-list length matches req->nbytes is incorrect?
>
> Where were you seeing the discrepancy? If it's at the point of
> entry into the HIFN code then the bug is further up. If it's
> within the HIFN code then I don't know where 12 came from either
> since the ICV is not visible to it.


I saw the discrepancy between the elen value used
for aead_request_set_crypt() in esp_input() and the
req->nbytes value seen in hifn_setup_session().

2008-02-20 13:19:50

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Hi Patrick.

On Wed, Feb 20, 2008 at 01:33:22PM +0100, Patrick McHardy ([email protected]) wrote:
> I saw the discrepancy between the elen value used
> for aead_request_set_crypt() in esp_input() and the
> req->nbytes value seen in hifn_setup_session().

What iv generation scheme do you use? It looks like only gcm and ccm add
16 bytes to cryptlen and set nbytes to them. Although they both install
two scatterlists for crypto operation: one for data and one for above
addition block of 16 bytes.

Btw, Herbert, I've found a tiny typo in both ccm and gcm modules, which
do not set correct cra_aead.geniv name.

Signed-off-by: Evgeniy Polyakov <[email protected]>

diff --git a/crypto/ccm.c b/crypto/ccm.c
index 7cf7e5a..118b6f5 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -821,7 +821,7 @@ static struct crypto_instance *crypto_rfc4309_alloc(struct rtattr **tb)
inst->alg.cra_aead.encrypt = crypto_rfc4309_encrypt;
inst->alg.cra_aead.decrypt = crypto_rfc4309_decrypt;

- inst->alg.cra_aead.geniv = "seqiv";
+ inst->alg.cra_aead.geniv = "ccm";

out:
return inst;
diff --git a/crypto/gcm.c b/crypto/gcm.c
index e70afd0..058de64 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -754,7 +754,7 @@ static struct crypto_instance *crypto_rfc4106_alloc(struct rtattr **tb)
inst->alg.cra_aead.encrypt = crypto_rfc4106_encrypt;
inst->alg.cra_aead.decrypt = crypto_rfc4106_decrypt;

- inst->alg.cra_aead.geniv = "seqiv";
+ inst->alg.cra_aead.geniv = "gcm";

out:
return inst;


--
Evgeniy Polyakov

2008-02-20 17:26:22

by Herbert Xu

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

On Wed, Feb 20, 2008 at 01:33:22PM +0100, Patrick McHardy wrote:
>
> I saw the discrepancy between the elen value used
> for aead_request_set_crypt() in esp_input() and the
> req->nbytes value seen in hifn_setup_session().

OK that is normal. esp_input is now using the AEAD interface
so the size for the request includes the ICV. By the time the
request gets to hifn it no longer has the ICV so it would be
12 bytes elss.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-02-20 17:30:04

by Herbert Xu

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

On Wed, Feb 20, 2008 at 04:19:03PM +0300, Evgeniy Polyakov wrote:
>
> What iv generation scheme do you use? It looks like only gcm and ccm add
> 16 bytes to cryptlen and set nbytes to them. Although they both install
> two scatterlists for crypto operation: one for data and one for above
> addition block of 16 bytes.
>
> Btw, Herbert, I've found a tiny typo in both ccm and gcm modules, which
> do not set correct cra_aead.geniv name.

Actually rfc4309/rfc4106 do need to use the seqiv generator.
Also ccm/gcm are not IV generators so they can't be used in
the geniv field. The geniv field is meant to contain an IV
generator template that takes a block cipher which may lack a
givencrypt function and produce a new block cipher with a
givencrypt function.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-02-21 09:10:58

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

On Thu, Feb 21, 2008 at 01:26:20AM +0800, Herbert Xu ([email protected]) wrote:
> On Wed, Feb 20, 2008 at 01:33:22PM +0100, Patrick McHardy wrote:
> >
> > I saw the discrepancy between the elen value used
> > for aead_request_set_crypt() in esp_input() and the
> > req->nbytes value seen in hifn_setup_session().
>
> OK that is normal. esp_input is now using the AEAD interface
> so the size for the request includes the ICV. By the time the
> request gets to hifn it no longer has the ICV so it would be
> 12 bytes elss.

But req->nbytes should or should not correspond to number of bytes to be
encrypted? HIFN driver assumes so, it can be changed to run through
scatterlists (btw, how will it determine how many of them were
provided?) and get full size of the request from them, but I suspected
that req->nbytes should always be equal to number of bytes to be
encrypted by the driver, otherwise where to get that info from?

--
Evgeniy Polyakov

2008-02-21 14:10:29

by Herbert Xu

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

On Thu, Feb 21, 2008 at 12:10:12PM +0300, Evgeniy Polyakov wrote:
>
> But req->nbytes should or should not correspond to number of bytes to be
> encrypted? HIFN driver assumes so, it can be changed to run through
> scatterlists (btw, how will it determine how many of them were
> provided?) and get full size of the request from them, but I suspected
> that req->nbytes should always be equal to number of bytes to be
> encrypted by the driver, otherwise where to get that info from?

Yes by the time the request gets to hifn req->nbytes would have
been decremented by 12 bytes which means that it is now the exact
amount of data that needs to be encrypted.

However, if you follow the scatterlist then you will see more data
available which is normal.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-02-21 14:18:56

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Hi Herbert.

On Thu, Feb 21, 2008 at 10:10:13PM +0800, Herbert Xu ([email protected]) wrote:
> Yes by the time the request gets to hifn req->nbytes would have
> been decremented by 12 bytes which means that it is now the exact
> amount of data that needs to be encrypted.
>
> However, if you follow the scatterlist then you will see more data
> available which is normal.

Argh, I see.

Then following patch should help.

Signed-off-by: Evgeniy Polyakov <[email protected]>

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..81b8f2f 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1544,7 +1544,10 @@ static int ablkcipher_walk(struct ablkcipher_request *req,

kunmap_atomic(daddr, KM_SOFTIRQ0);
} else {
- nbytes -= src->length;
+ if (src->length > nbytes)
+ nbytes = 0;
+ else
+ nbytes -= src->length;
idx++;
}


--
Evgeniy Polyakov

2008-02-21 14:21:06

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Evgeniy Polyakov wrote:
> Hi Herbert.
>
> On Thu, Feb 21, 2008 at 10:10:13PM +0800, Herbert Xu ([email protected]) wrote:
>> Yes by the time the request gets to hifn req->nbytes would have
>> been decremented by 12 bytes which means that it is now the exact
>> amount of data that needs to be encrypted.
>>
>> However, if you follow the scatterlist then you will see more data
>> available which is normal.
>
> Argh, I see.
>
> Then following patch should help.
>
> Signed-off-by: Evgeniy Polyakov <[email protected]>
>
> diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
> index dfbf24c..81b8f2f 100644
> --- a/drivers/crypto/hifn_795x.c
> +++ b/drivers/crypto/hifn_795x.c
> @@ -1544,7 +1544,10 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
>
> kunmap_atomic(daddr, KM_SOFTIRQ0);
> } else {
> - nbytes -= src->length;
> + if (src->length > nbytes)
> + nbytes = 0;
> + else
> + nbytes -= src->length;
> idx++;
> }
>

Almost I guess :) There are similar loops in hifn_setup_session().
Additionally we need to check that the return value of ablkcipher_walk()
is not a negative errno code.

2008-02-21 14:38:08

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

On Thu, Feb 21, 2008 at 03:20:45PM +0100, Patrick McHardy ([email protected]) wrote:
> Almost I guess :) There are similar loops in hifn_setup_session().
> Additionally we need to check that the return value of ablkcipher_walk()
> is not a negative errno code.

Yep. Kind of this one:

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..c88b4d9 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1544,7 +1544,10 @@ static int ablkcipher_walk(struct ablkcipher_request *req,

kunmap_atomic(daddr, KM_SOFTIRQ0);
} else {
- nbytes -= src->length;
+ if (src->length >= nbytes)
+ nbytes = 0;
+ else
+ nbytes -= src->length;
idx++;
}

@@ -1588,7 +1591,10 @@ static int hifn_setup_session(struct ablkcipher_request *req)
ctx->walk.flags |= ASYNC_FLAGS_MISALIGNED;
}

- nbytes -= src->length;
+ if (src->length > nbytes)
+ nbytes = 0;
+ else
+ nbytes -= src->length;
idx++;
}

@@ -1602,6 +1608,10 @@ static int hifn_setup_session(struct ablkcipher_request *req)
idx = 0;

sg_num = ablkcipher_walk(req, &ctx->walk);
+ if (sg_num < 0) {
+ err = sg_num;
+ goto err_out_exit;
+ }

atomic_set(&ctx->sg_num, sg_num);

@@ -1640,7 +1650,10 @@ static int hifn_setup_session(struct ablkcipher_request *req)
if (err)
goto err_out;

- nbytes -= len;
+ if (len > nbytes)
+ nbytes = 0;
+ else
+ nbytes -= len;
}

dev->active = HIFN_DEFAULT_ACTIVE_NUM;
@@ -1803,7 +1816,10 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
sg_page(dst), dst->length, nbytes);

if (!t->length) {
- nbytes -= dst->length;
+ if (dst->length > nbytes)
+ nbytes = 0;
+ else
+ nbytes -= dst->length;
idx++;
continue;
}



--
Evgeniy Polyakov

2008-02-21 14:42:10

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Evgeniy Polyakov wrote:
> On Thu, Feb 21, 2008 at 03:20:45PM +0100, Patrick McHardy ([email protected]) wrote:
>> Almost I guess :) There are similar loops in hifn_setup_session().
>> Additionally we need to check that the return value of ablkcipher_walk()
>> is not a negative errno code.
>
> Yep. Kind of this one:

Thanks, I'll give it a try. It includes one chunk I missed when
trying this myself, which might explain the problems I saw
afterwards.

2008-02-21 15:30:16

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Patrick McHardy wrote:
> Evgeniy Polyakov wrote:
>> On Thu, Feb 21, 2008 at 03:20:45PM +0100, Patrick McHardy
>> ([email protected]) wrote:
>>> Almost I guess :) There are similar loops in hifn_setup_session().
>>> Additionally we need to check that the return value of ablkcipher_walk()
>>> is not a negative errno code.
>>
>> Yep. Kind of this one:
>
> Thanks, I'll give it a try. It includes one chunk I missed when
> trying this myself, which might explain the problems I saw
> afterwards.

Unfortunately still no luck. I got an error from ablkcipher_add()
because of this condition:

if (drest < size || size > nbytes)

with size=124 any nbytes=112. After changing ablkcipher_walk():

- unsigned slen = src->length - offset
+ unsigned slen = min(src->length, nbytes) - offset;

the error went away and I got a silent crash (at least
nothing was logged over netconsole).

It also looks like at least two more changes are needed:

- hifn_setup_session does:

if (src->length & (blocksize - 1) ||
..
dst->length & (blocksize - 1) ||

ctx->walk.flags |= ASYNC_FLAGS_MISALIGNED;

which looks like it should use min(length, nbytes).

- further down it uses dst->length in the last while-loop,
which seems to need a similar change.

2008-02-21 15:32:12

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Patrick McHardy wrote:
> It also looks like at least two more changes are needed:
> ...

One more thing: I got lots of "printk: 4 messages suppressed"
without any real messages. The reason is a printk_ratelimit()
before a dprintk(). This probably wants either an #ifdef or
a real printk.

2008-02-22 02:34:01

by Loc Ho

[permalink] [raw]
Subject: Test AES-CCM mode via IPSec (NETKEY)

Hi,

I am trying to test AES-CCM mode via IPSec. Setkey doesn't seem to support
"aes-ccm" mode. Anyone try this yet? Or should I checkout OpenSwan or
racoon?

-Loc

2008-02-22 05:45:36

by Herbert Xu

[permalink] [raw]
Subject: Re: Test AES-CCM mode via IPSec (NETKEY)

Loc Ho <[email protected]> wrote:
>
> I am trying to test AES-CCM mode via IPSec. Setkey doesn't seem to support
> "aes-ccm" mode. Anyone try this yet? Or should I checkout OpenSwan or
> racoon?

Sorry, I forgot to submit the iproute2 patch for this.

See if this one helps.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index 51aa042..b0c4cc8 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -96,6 +96,13 @@ struct xfrm_algo {
char alg_key[0];
};

+struct xfrm_algo_aead {
+ char alg_name[64];
+ int alg_key_len; /* in bits */
+ int alg_icv_len; /* in bits */
+ char alg_key[0];
+};
+
struct xfrm_stats {
__u32 replay_window;
__u32 replay;
@@ -114,6 +121,7 @@ enum
XFRM_POLICY_IN = 0,
XFRM_POLICY_OUT = 1,
XFRM_POLICY_FWD = 2,
+ XFRM_POLICY_MASK = 3,
XFRM_POLICY_MAX = 3
};

@@ -269,6 +277,7 @@ enum xfrm_attr_type_t {
XFRMA_LASTUSED,
XFRMA_POLICY_TYPE, /* struct xfrm_userpolicy_type */
XFRMA_MIGRATE,
+ XFRMA_ALG_AEAD, /* struct xfrm_algo_aead */
__XFRMA_MAX

#define XFRMA_MAX (__XFRMA_MAX - 1)
@@ -328,6 +337,7 @@ struct xfrm_usersa_info {
#define XFRM_STATE_DECAP_DSCP 2
#define XFRM_STATE_NOPMTUDISC 4
#define XFRM_STATE_WILDRECV 8
+#define XFRM_STATE_ICMP 16
};

struct xfrm_usersa_id {
@@ -362,6 +372,8 @@ struct xfrm_userpolicy_info {
#define XFRM_POLICY_BLOCK 1
__u8 flags;
#define XFRM_POLICY_LOCALOK 1 /* Allow user to override global policy */
+ /* Automatically expand selector to include matching ICMP payloads. */
+#define XFRM_POLICY_ICMP 2
__u8 share;
};

@@ -414,12 +426,14 @@ struct xfrm_user_migrate {
__u16 new_family;
};

+#ifndef __KERNEL__
/* backwards compatibility for userspace */
#define XFRMGRP_ACQUIRE 1
#define XFRMGRP_EXPIRE 2
#define XFRMGRP_SA 4
#define XFRMGRP_POLICY 8
#define XFRMGRP_REPORT 0x20
+#endif

enum xfrm_nlgroups {
XFRMNLGRP_NONE,
diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 80dbb52..0a7d39a 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -154,7 +154,8 @@ const char *strxf_xfrmproto(__u8 proto)

static const struct typeent algo_types[]= {
{ "enc", XFRMA_ALG_CRYPT }, { "auth", XFRMA_ALG_AUTH },
- { "comp", XFRMA_ALG_COMP }, { NULL, -1 }
+ { "comp", XFRMA_ALG_COMP }, { "aead", XFRMA_ALG_AEAD },
+ { NULL, -1 }
};

int xfrm_algotype_getbyname(char *name)
@@ -525,8 +526,8 @@ void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
fprintf(fp, "%s", _SL_);
}

-static void xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
- FILE *fp, const char *prefix)
+static void __xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
+ FILE *fp, const char *prefix, int newline)
{
int keylen;
int i;
@@ -558,6 +559,32 @@ static void xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
fprintf(fp, " (%d bits)", algo->alg_key_len);

fin:
+ if (newline)
+ fprintf(fp, "%s", _SL_);
+}
+
+static inline void xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
+ FILE *fp, const char *prefix)
+{
+ return __xfrm_algo_print(algo, type, len, fp, prefix, 1);
+}
+
+static void xfrm_aead_print(struct xfrm_algo_aead *algo, int len,
+ FILE *fp, const char *prefix)
+{
+ struct {
+ struct xfrm_algo algo;
+ char key[algo->alg_key_len / 8];
+ } base;
+
+ memcpy(base.algo.alg_name, algo->alg_name, sizeof(base.algo.alg_name));
+ base.algo.alg_key_len = algo->alg_key_len;
+ memcpy(base.algo.alg_key, algo->alg_key, algo->alg_key_len / 8);
+
+ __xfrm_algo_print(&base.algo, XFRMA_ALG_AEAD, len, fp, prefix, 0);
+
+ fprintf(fp, " %d", algo->alg_icv_len);
+
fprintf(fp, "%s", _SL_);
}

@@ -635,6 +662,12 @@ void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
XFRMA_ALG_AUTH, RTA_PAYLOAD(rta), fp, prefix);
}

+ if (tb[XFRMA_ALG_AEAD]) {
+ struct rtattr *rta = tb[XFRMA_ALG_AEAD];
+ xfrm_aead_print((struct xfrm_algo_aead *)RTA_DATA(rta),
+ RTA_PAYLOAD(rta), fp, prefix);
+ }
+
if (tb[XFRMA_ALG_CRYPT]) {
struct rtattr *rta = tb[XFRMA_ALG_CRYPT];
xfrm_algo_print((struct xfrm_algo *) RTA_DATA(rta),
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index f51e8b6..0748a61 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -88,8 +88,10 @@ static void usage(void)
fprintf(stderr, "ENCAP-TYPE := espinudp | espinudp-nonike\n");

fprintf(stderr, "ALGO-LIST := [ ALGO-LIST ] | [ ALGO ]\n");
- fprintf(stderr, "ALGO := ALGO_TYPE ALGO_NAME ALGO_KEY\n");
+ fprintf(stderr, "ALGO := ALGO_TYPE ALGO_NAME ALGO_KEY "
+ "[ ALGO_ICV_LEN ]\n");
fprintf(stderr, "ALGO_TYPE := [ ");
+ fprintf(stderr, "%s | ", strxf_algotype(XFRMA_ALG_AEAD));
fprintf(stderr, "%s | ", strxf_algotype(XFRMA_ALG_CRYPT));
fprintf(stderr, "%s | ", strxf_algotype(XFRMA_ALG_AUTH));
fprintf(stderr, "%s ", strxf_algotype(XFRMA_ALG_COMP));
@@ -112,7 +114,7 @@ static void usage(void)
}

static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
- char *name, char *key, int max)
+ char *name, char *key, char *buf, int max)
{
int len;
int slen = strlen(key);
@@ -152,7 +154,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
if (get_u8(&val, vbuf, 16))
invarg("\"ALGOKEY\" is invalid", key);

- alg->alg_key[j] = val;
+ buf[j] = val;
}
} else {
len = slen;
@@ -160,7 +162,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
if (len > max)
invarg("\"ALGOKEY\" makes buffer overflow\n", key);

- strncpy(alg->alg_key, key, len);
+ strncpy(buf, key, len);
}
}

@@ -233,6 +235,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
char buf[RTA_BUF_SIZE];
} req;
char *idp = NULL;
+ char *aeadop = NULL;
char *ealgop = NULL;
char *aalgop = NULL;
char *calgop = NULL;
@@ -316,20 +319,31 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
/* try to assume ALGO */
int type = xfrm_algotype_getbyname(*argv);
switch (type) {
+ case XFRMA_ALG_AEAD:
case XFRMA_ALG_CRYPT:
case XFRMA_ALG_AUTH:
case XFRMA_ALG_COMP:
{
/* ALGO */
struct {
- struct xfrm_algo alg;
+ union {
+ struct xfrm_algo alg;
+ struct xfrm_algo_aead aead;
+ } u;
char buf[XFRM_ALGO_KEY_BUF_SIZE];
- } alg;
+ } alg = {};
int len;
+ __u32 icvlen;
char *name;
char *key;
+ char *buf;

switch (type) {
+ case XFRMA_ALG_AEAD:
+ if (aeadop)
+ duparg("ALGOTYPE", *argv);
+ aeadop = *argv;
+ break;
case XFRMA_ALG_CRYPT:
if (ealgop)
duparg("ALGOTYPE", *argv);
@@ -360,11 +374,27 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
NEXT_ARG();
key = *argv;

- memset(&alg, 0, sizeof(alg));
+ buf = alg.u.alg.alg_key;
+ len = sizeof(alg.u.alg);
+
+ if (type != XFRMA_ALG_AEAD)
+ goto parse_algo;
+
+ if (!NEXT_ARG_OK())
+ missarg("ALGOICVLEN");
+ NEXT_ARG();
+ if (get_u32(&icvlen, *argv, 0))
+ invarg("\"aead\" ICV length is invalid",
+ *argv);
+ alg.u.aead.alg_icv_len = icvlen;
+
+ buf = alg.u.aead.alg_key;
+ len = sizeof(alg.u.aead);

+parse_algo:
xfrm_algo_parse((void *)&alg, type, name, key,
- sizeof(alg.buf));
- len = sizeof(struct xfrm_algo) + alg.alg.alg_key_len;
+ buf, sizeof(alg.buf));
+ len += alg.u.alg.alg_key_len;

addattr_l(&req.n, sizeof(req.buf), type,
(void *)&alg, len);
@@ -417,7 +447,7 @@ static int xfrm_state_modify(int cmd, unsigned flags, int argc, char **argv)
break;
}

- if (ealgop || aalgop || calgop) {
+ if (aeadop || ealgop || aalgop || calgop) {
if (!xfrm_xfrmproto_is_ipsec(req.xsinfo.id.proto)) {
fprintf(stderr, "\"ALGO\" is invalid with proto=%s\n",
strxf_xfrmproto(req.xsinfo.id.proto));

2008-02-22 12:43:08

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Hi Patrick.

On Thu, Feb 21, 2008 at 04:29:54PM +0100, Patrick McHardy ([email protected]) wrote:
> Unfortunately still no luck. I got an error from ablkcipher_add()
> because of this condition:

...

> - further down it uses dst->length in the last while-loop,
> which seems to need a similar change.

Does this patch (on top of unpatched tree) helps?
I can not test it with real hardware, since I'm sicking at home with
laptop only, but I already know how to make a test case for this problem
without complex setup, so if it does not work, I will investigate it
further as soon as be able to move to office with testing machine :)

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..7807ca9 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1464,7 +1464,7 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
crypto_ablkcipher_alignmask(crypto_ablkcipher_reqtfm(req));
struct scatterlist *src, *dst, *t;
void *daddr;
- unsigned int nbytes = req->nbytes, offset, copy, diff;
+ unsigned int nbytes = req->nbytes, offset, copy, diff, nb;
int idx, tidx, err;

tidx = idx = 0;
@@ -1475,11 +1475,13 @@ static int ablkcipher_walk(struct ablkcipher_request *req,

src = &req->src[idx];
dst = &req->dst[idx];
+
+ nb = min(nbytes, src->length);

dprintk("\n%s: slen: %u, dlen: %u, soff: %u, doff: %u, offset: %u, "
- "blocksize: %u, nbytes: %u.\n",
+ "blocksize: %u, nbytes: %u, nb: %u.\n",
__func__, src->length, dst->length, src->offset,
- dst->offset, offset, blocksize, nbytes);
+ dst->offset, offset, blocksize, nbytes, nb);

if (src->length & (blocksize - 1) ||
src->offset & (alignmask - 1) ||
@@ -1492,7 +1494,7 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
t = &w->cache[idx];

daddr = kmap_atomic(sg_page(t), KM_SOFTIRQ0);
- err = ablkcipher_add(daddr, &dlen, src, slen, &nbytes);
+ err = ablkcipher_add(daddr, &dlen, src, nb, &nbytes);
if (err < 0)
goto err_out_unmap;

@@ -1501,7 +1503,7 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
copy = slen & ~(blocksize - 1);
diff = slen & (blocksize - 1);

- if (dlen < nbytes) {
+ if (dlen < nb) {
/*
* Destination page does not have enough space
* to put there additional blocksized chunk,
@@ -1510,17 +1512,17 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
* t->length = (slen & ~(blocksize - 1));
* and increase number of bytes to be processed
* in next chunk:
- * nbytes += diff;
+ * nb += diff;
*/
- nbytes += diff;
+ nb += diff;

/*
* Temporary of course...
* Kick author if you will catch this one.
*/
printk(KERN_ERR "%s: dlen: %u, nbytes: %u,"
- "slen: %u, offset: %u.\n",
- __func__, dlen, nbytes, slen, offset);
+ "slen: %u, offset: %u, nb: %u.\n",
+ __func__, dlen, nbytes, slen, offset, nb);
printk(KERN_ERR "%s: please contact author to fix this "
"issue, generally you should not catch "
"this path under any condition but who "
@@ -1528,11 +1530,11 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
"Thank you.\n", __func__);
BUG();
} else {
- copy += diff + nbytes;
+ copy += diff + nb;

src = &req->src[idx];

- err = ablkcipher_add(daddr + slen, &dlen, src, nbytes, &nbytes);
+ err = ablkcipher_add(daddr + slen, &dlen, src, nb, &nbytes);
if (err < 0)
goto err_out_unmap;

@@ -1544,7 +1546,7 @@ static int ablkcipher_walk(struct ablkcipher_request *req,

kunmap_atomic(daddr, KM_SOFTIRQ0);
} else {
- nbytes -= src->length;
+ nbytes -= nb;
idx++;
}

@@ -1564,7 +1566,7 @@ static int hifn_setup_session(struct ablkcipher_request *req)
struct hifn_device *dev = ctx->dev;
struct page *spage, *dpage;
unsigned long soff, doff, flags;
- unsigned int nbytes = req->nbytes, idx = 0, len;
+ unsigned int nbytes = req->nbytes, idx = 0, len, nb;
int err = -EINVAL, sg_num;
struct scatterlist *src, *dst, *t;
unsigned blocksize =
@@ -1581,6 +1583,8 @@ static int hifn_setup_session(struct ablkcipher_request *req)
src = &req->src[idx];
dst = &req->dst[idx];

+ nb = min(src->length, nbytes);
+
if (src->length & (blocksize - 1) ||
src->offset & (alignmask - 1) ||
dst->length & (blocksize - 1) ||
@@ -1588,7 +1592,7 @@ static int hifn_setup_session(struct ablkcipher_request *req)
ctx->walk.flags |= ASYNC_FLAGS_MISALIGNED;
}

- nbytes -= src->length;
+ nbytes -= nb;
idx++;
}

@@ -1602,6 +1606,10 @@ static int hifn_setup_session(struct ablkcipher_request *req)
idx = 0;

sg_num = ablkcipher_walk(req, &ctx->walk);
+ if (sg_num < 0) {
+ err = sg_num;
+ goto err_out_exit;
+ }

atomic_set(&ctx->sg_num, sg_num);

@@ -1632,10 +1640,12 @@ static int hifn_setup_session(struct ablkcipher_request *req)

len = dst->length;
}
+
+ nb = min(len, nbytes);

idx++;

- err = hifn_setup_dma(dev, spage, soff, dpage, doff, nbytes,
+ err = hifn_setup_dma(dev, spage, soff, dpage, doff, nb,
req, ctx);
if (err)
goto err_out;
@@ -1652,7 +1662,7 @@ err_out:
spin_unlock_irqrestore(&dev->lock, flags);
err_out_exit:
if (err && printk_ratelimit())
- dprintk("%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u, "
+ printk("%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u, "
"type: %u, err: %d.\n",
dev->name, ctx->iv, ctx->ivsize,
ctx->key, ctx->keysize,
@@ -1786,7 +1796,7 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
BUG();

if (atomic_dec_and_test(&ctx->sg_num)) {
- unsigned int nbytes = req->nbytes;
+ unsigned int nbytes = req->nbytes, nb;
int idx = 0, err;
struct scatterlist *dst, *t;
void *saddr;
@@ -1796,6 +1806,8 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
t = &ctx->walk.cache[idx];
dst = &req->dst[idx];

+ nb = min(nbytes, dst->length);
+
dprintk("\n%s: sg_page(t): %p, t->length: %u, "
"sg_page(dst): %p, dst->length: %u, "
"nbytes: %u.\n",
@@ -1803,7 +1815,7 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
sg_page(dst), dst->length, nbytes);

if (!t->length) {
- nbytes -= dst->length;
+ nbytes -= nb;
idx++;
continue;
}
@@ -1811,7 +1823,7 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
saddr = kmap_atomic(sg_page(t), KM_IRQ1);

err = ablkcipher_get(saddr, &t->length, t->offset,
- dst, nbytes, &nbytes);
+ dst, nb, &nbytes);
if (err < 0) {
kunmap_atomic(saddr, KM_IRQ1);
break;


--
Evgeniy Polyakov

2008-02-22 13:53:32

by Patrick McHardy

[permalink] [raw]
Subject: Re: HIFN+IPsec crashes in current -git

Evgeniy Polyakov wrote:
> Does this patch (on top of unpatched tree) helps?
> I can not test it with real hardware, since I'm sicking at home with
> laptop only, but I already know how to make a test case for this problem
> without complex setup, so if it does not work, I will investigate it
> further as soon as be able to move to office with testing machine :)


It still crashed with this patch. Unfortunately I only got
the first line of the Oops over netconsole, and I can't
crash that box too often. I'll see if I can capture the
full oops using serial console sometime tonight.

2008-03-13 17:44:04

by Loc Ho

[permalink] [raw]
Subject: RE: Test AES-CCM mode via IPSec (NETKEY)

Hi,

I had tried this and it works after I manual performed the patch to
iproute2.

It is possible to use program "ip" to setup algorithm non-combined mode,
such as "authenc(cbc(aes),hmac(md5))"? If so, how?

-Loc

-----Original Message-----
From: [email protected]
[mailto:[email protected]] On Behalf Of Herbert Xu
Sent: Thursday, February 21, 2008 9:46 PM
To: Loc Ho
Cc: [email protected]
Subject: Re: Test AES-CCM mode via IPSec (NETKEY)

Loc Ho <[email protected]> wrote:
>
> I am trying to test AES-CCM mode via IPSec. Setkey doesn't seem to
> support "aes-ccm" mode. Anyone try this yet? Or should I checkout
> OpenSwan or racoon?

Sorry, I forgot to submit the iproute2 patch for this.

See if this one helps.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]> Home Page:
http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-03-14 01:12:50

by Herbert Xu

[permalink] [raw]
Subject: Re: Test AES-CCM mode via IPSec (NETKEY)

On Thu, Mar 13, 2008 at 10:34:32AM -0700, Loc Ho wrote:
> Hi,
>
> I had tried this and it works after I manual performed the patch to
> iproute2.
>
> It is possible to use program "ip" to setup algorithm non-combined mode,
> such as "authenc(cbc(aes),hmac(md5))"? If so, how?

You need to specify two algorithms on the command-line, one for
type enc and one for type auth. Otherwise the format is the same
as for aead, i.e., type name key.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-04-04 22:49:31

by Joy Latten

[permalink] [raw]
Subject: RE: Test AES-CCM mode via IPSec (NETKEY)

I was wondering if you could post what you passed to ip command for ccm
to get it to work with ipsec. I have not had much luck so far.

thanks!!

regards,
Joy

On Thu, 2008-03-13 at 10:34 -0700, Loc Ho wrote:
> Hi,
>
> I had tried this and it works after I manual performed the patch to
> iproute2.
>
> It is possible to use program "ip" to setup algorithm non-combined mode,
> such as "authenc(cbc(aes),hmac(md5))"? If so, how?
>
> -Loc
>
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Herbert Xu
> Sent: Thursday, February 21, 2008 9:46 PM
> To: Loc Ho
> Cc: [email protected]
> Subject: Re: Test AES-CCM mode via IPSec (NETKEY)
>
> Loc Ho <[email protected]> wrote:
> >
> > I am trying to test AES-CCM mode via IPSec. Setkey doesn't seem to
> > support "aes-ccm" mode. Anyone try this yet? Or should I checkout
> > OpenSwan or racoon?
>
> Sorry, I forgot to submit the iproute2 patch for this.
>
> See if this one helps.
>


2008-04-04 23:53:30

by Loc Ho

[permalink] [raw]
Subject: RE: Test AES-CCM mode via IPSec (NETKEY)

Hi,

Try these scripts with proper IP address. In addition, you must patch
iproute2 manually using the patch from Herbert's email:

[lho@svdclab161 sec]$ cat ip-start-transport-ccm
#!/bin/sh

NODE=$1

echo "Starting IPSec transport mode using CCM..."

./ip xfrm policy flush
./ip xfrm state flush
#
# SA
./ip xfrm state add src 10.66.21.164 dst 10.66.21.166 proto esp spi
0x201 mode transport aead "rfc4309(ccm(aes))"
0x0102037aeaca3f87d060a12f4a4487d5a5c335 96
./ip xfrm state add src 10.66.21.166 dst 10.66.21.164 proto esp spi
0x301 mode transport aead "rfc4309(ccm(aes))"
0x010203f6ddb555acfd9d77b03ea3843f265325 96
#
# Policy
if [ "${NODE}" = "A" ]; then
./ip xfrm policy add dir out src 10.66.21.164 dst 10.66.21.166
tmpl proto esp mode transport
./ip xfrm policy add dir in src 10.66.21.166 dst 10.66.21.164
tmpl proto esp mode transport
fi
if [ "${NODE}" = "B" ]; then
./ip xfrm policy add dir in src 10.66.21.164 dst 10.66.21.166
tmpl proto esp mode transport
./ip xfrm policy add dir out src 10.66.21.166 dst 10.66.21.164
tmpl proto esp mode transport
fi

[lho@svdclab161 sec]$ cat ip-start-transport-gcm
#!/bin/sh

NODE=$1

echo "Starting IPSec transport mode using GCM..."

./ip xfrm policy flush
./ip xfrm state flush
#
# SA
./ip xfrm state add src 10.66.21.164 dst 10.66.21.166 proto esp spi
0x201 mode transport aead "rfc4106(gcm(aes))"
0x010203047aeaca3f87d060a12f4a4487d5a5c335 96
./ip xfrm state add src 10.66.21.166 dst 10.66.21.164 proto esp spi
0x301 mode transport aead "rfc4106(gcm(aes))"
0x01020304f6ddb555acfd9d77b03ea3843f265325 96
#
# Policy
if [ "${NODE}" = "A" ]; then
./ip xfrm policy add dir out src 10.66.21.164 dst 10.66.21.166
tmpl proto esp mode transport
./ip xfrm policy add dir in src 10.66.21.166 dst 10.66.21.164
tmpl proto esp mode transport
fi
if [ "${NODE}" = "B" ]; then
./ip xfrm policy add dir in src 10.66.21.164 dst 10.66.21.166
tmpl proto esp mode transport
./ip xfrm policy add dir out src 10.66.21.166 dst 10.66.21.164
tmpl proto esp mode transport
fi

-Loc

2008-04-07 21:26:51

by Joy Latten

[permalink] [raw]
Subject: RE: Test AES-CCM mode via IPSec (NETKEY)

>Hi,
>
>Try these scripts with proper IP address. In addition, you must patch
>iproute2 manually using the patch from Herbert's email:
>
>[lho@svdclab161 sec]$ cat ip-start-transport-ccm
>#!/bin/sh
>
>NODE=$1
>
>echo "Starting IPSec transport mode using CCM..."
>
>./ip xfrm policy flush
>./ip xfrm state flush
>#
># SA
>./ip xfrm state add src 10.66.21.164 dst 10.66.21.166 proto esp spi
>0x201 mode transport aead "rfc4309(ccm(aes))"
>0x0102037aeaca3f87d060a12f4a4487d5a5c335 96
>./ip xfrm state add src 10.66.21.166 dst 10.66.21.164 proto esp spi
>0x301 mode transport aead "rfc4309(ccm(aes))"
>0x010203f6ddb555acfd9d77b03ea3843f265325 96
>#
># Policy
>if [ "${NODE}" = "A" ]; then
> ./ip xfrm policy add dir out src 10.66.21.164 dst 10.66.21.166
>tmpl proto esp mode transport
> ./ip xfrm policy add dir in src 10.66.21.166 dst 10.66.21.164
>tmpl proto esp mode transport
>fi
>if [ "${NODE}" = "B" ]; then
> ./ip xfrm policy add dir in src 10.66.21.164 dst 10.66.21.166
>tmpl proto esp mode transport
> ./ip xfrm policy add dir out src 10.66.21.166 dst 10.66.21.164
>tmpl proto esp mode transport
>fi
>
>[lho@svdclab161 sec]$ cat ip-start-transport-gcm
>#!/bin/sh
>
>NODE=$1
>
>echo "Starting IPSec transport mode using GCM..."
>
>./ip xfrm policy flush
>./ip xfrm state flush
>#
># SA
>./ip xfrm state add src 10.66.21.164 dst 10.66.21.166 proto esp spi
>0x201 mode transport aead "rfc4106(gcm(aes))"
>0x010203047aeaca3f87d060a12f4a4487d5a5c335 96
>./ip xfrm state add src 10.66.21.166 dst 10.66.21.164 proto esp spi
>0x301 mode transport aead "rfc4106(gcm(aes))"
>0x01020304f6ddb555acfd9d77b03ea3843f265325 96
>#
># Policy
>if [ "${NODE}" = "A" ]; then
> ./ip xfrm policy add dir out src 10.66.21.164 dst 10.66.21.166
>tmpl proto esp mode transport
> ./ip xfrm policy add dir in src 10.66.21.166 dst 10.66.21.164
>tmpl proto esp mode transport
>fi
>if [ "${NODE}" = "B" ]; then
> ./ip xfrm policy add dir in src 10.66.21.164 dst 10.66.21.166
>tmpl proto esp mode transport
> ./ip xfrm policy add dir out src 10.66.21.166 dst 10.66.21.164
>tmpl proto esp mode transport
>fi
>

Thank you!! Your instructions were perfect and I had it working
in no time.

regards,
Joy