[PATCH][IPsec] fix process of error from crypto module

[PATCH][IPsec] fix process of error from crypto module

[MIYAZAWA Kazunori]

Hello,

This patch fixes the process under the case that the crypto module
returns error because of its weak keys or etc.


diff -ur a/net/ipv4/esp4.c b/net/ipv4/esp4.c
--- a/net/ipv4/esp4.c 2004-12-25 06:34:58.000000000 +0900
+++ b/net/ipv4/esp4.c 2005-01-26 18:57:18.000000000 +0900
@@ -427,7 +427,8 @@
    goto error;
   get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
  }
- crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len);
+ if (crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len))
+  goto error;
  x->props.header_len = sizeof(struct ip_esp_hdr) + esp->conf.ivlen;
  if (x->props.mode)
   x->props.header_len += sizeof(struct iphdr);
diff -ur a/net/ipv6/esp6.c b/net/ipv6/esp6.c
--- a/net/ipv6/esp6.c 2004-12-25 06:35:01.000000000 +0900
+++ b/net/ipv6/esp6.c 2005-01-26 18:57:04.000000000 +0900
@@ -364,7 +364,8 @@
    goto error;
   get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
  }
- crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len);
+ if (crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len))
+  goto error;
  x->props.header_len = sizeof(struct ipv6_esp_hdr) + esp->conf.ivlen;
  if (x->props.mode)
   x->props.header_len += sizeof(struct ipv6hdr);


--
Kazunori Miyazawa

Re: [PATCH][IPsec] fix process of error from crypto module

[Herbert Xu]

MIYAZAWA Kazunori <kazunori@miyazawa.org> wrote:
> 
> This patch fixes the process under the case that the crypto module
> returns error because of its weak keys or etc.

Good catch.

> diff -ur a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> --- a/net/ipv6/esp6.c 2004-12-25 06:35:01.000000000 +0900
> +++ b/net/ipv6/esp6.c 2005-01-26 18:57:04.000000000 +0900
> @@ -364,7 +364,8 @@
>    goto error;
>   get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
>  }
> - crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len);
> + if (crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len))
> +  goto error;
>  x->props.header_len = sizeof(struct ipv6_esp_hdr) + esp->conf.ivlen;
>  if (x->props.mode)
>   x->props.header_len += sizeof(struct ipv6hdr);

You need to free esp->conf.ivec here.

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

Re: [PATCH][IPsec] fix process of error from crypto module

[Kazunori Miyazawa]

Herbert Xu wrote:
> MIYAZAWA Kazunori <kazunori@miyazawa.org> wrote:
> 
>>This patch fixes the process under the case that the crypto module
>>returns error because of its weak keys or etc.
> 
> 
> Good catch.
> 
> 
>>diff -ur a/net/ipv6/esp6.c b/net/ipv6/esp6.c
>>--- a/net/ipv6/esp6.c 2004-12-25 06:35:01.000000000 +0900
>>+++ b/net/ipv6/esp6.c 2005-01-26 18:57:04.000000000 +0900
>>@@ -364,7 +364,8 @@
>>   goto error;
>>  get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
>> }
>>- crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len);
>>+ if (crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len))
>>+  goto error;
>> x->props.header_len = sizeof(struct ipv6_esp_hdr) + esp->conf.ivlen;
>> if (x->props.mode)
>>  x->props.header_len += sizeof(struct ipv6hdr);
> 
> 
> You need to free esp->conf.ivec here.
> 
> Cheers,

Thank you for your check, Herbert.
I send the fixed patch.
BTW, esp6_destroy seem to be similar to esp_destroy.
Should we export esp_destroy and IPv4 and IPv6 use it?
Do we have any reason to define the functions separately?

diff -ruN a/net/ipv4/esp4.c b/net/ipv4/esp4.c
--- a/net/ipv4/esp4.c	2004-12-25 06:34:58.000000000 +0900
+++ b/net/ipv4/esp4.c	2005-01-26 18:57:18.000000000 +0900
@@ -427,7 +427,8 @@
 			goto error;
 		get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
 	}
-	crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len);
+	if (crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len))
+		goto error;
 	x->props.header_len = sizeof(struct ip_esp_hdr) + esp->conf.ivlen;
 	if (x->props.mode)
 		x->props.header_len += sizeof(struct iphdr);
diff -ruN a/net/ipv6/esp6.c b/net/ipv6/esp6.c
--- a/net/ipv6/esp6.c	2004-12-25 06:35:01.000000000 +0900
+++ b/net/ipv6/esp6.c	2005-01-27 00:55:34.000000000 +0900
@@ -364,7 +364,8 @@
 			goto error;
 		get_random_bytes(esp->conf.ivec, esp->conf.ivlen);
 	}
-	crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len);
+	if (crypto_cipher_setkey(esp->conf.tfm, esp->conf.key, esp->conf.key_len))
+		goto error;
 	x->props.header_len = sizeof(struct ipv6_esp_hdr) + esp->conf.ivlen;
 	if (x->props.mode)
 		x->props.header_len += sizeof(struct ipv6hdr);
@@ -372,15 +373,9 @@
 	return 0;

 error:
-	if (esp) {
-		if (esp->auth.tfm)
-			crypto_free_tfm(esp->auth.tfm);
-		if (esp->auth.work_icv)
-			kfree(esp->auth.work_icv);
-		if (esp->conf.tfm)
-			crypto_free_tfm(esp->conf.tfm);
-		kfree(esp);
-	}
+	x->data = esp;
+	esp6_destroy(x);
+	x->data = NULL;
 	return -EINVAL;
 }

Re: [PATCH][IPsec] fix process of error from crypto module

[Herbert Xu]

On Thu, Jan 27, 2005 at 01:51:52PM +0900, Kazunori Miyazawa wrote:
>
> BTW, esp6_destroy seem to be similar to esp_destroy.
> Should we export esp_destroy and IPv4 and IPv6 use it?
> Do we have any reason to define the functions separately?

esp_destroy is certainly not the only function that can be merged.

One of my dreams is to abstract out the family-dependent code
from ah/esp/ipcomp so that we can have a family-independent code
base.

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

Re: [PATCH][IPsec] fix process of error from crypto module

[David S. Miller]

On Thu, 27 Jan 2005 13:51:52 +0900
Kazunori Miyazawa <kazunori@miyazawa.org> wrote:

> Thank you for your check, Herbert.
> I send the fixed patch.

Patch applied, thank you Miyazawa-san.