[PATCH 0/2]gss:gss miss returning error to caller when import security context

[PATCH 0/2]gss:gss miss returning error to caller when import security context

[Bian Naimeng]

Gss miss returning error to caller when import security context, 
it may be return ok though it has failed to import security context.

Bian Naimeng (2)

----
 net/sunrpc/auth_gss/gss_krb5_mech.c  |   10 ++++++++--
 net/sunrpc/auth_gss/gss_spkm3_mech.c |    5 ++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

-- 
Regards
Bian Naimeng

[PATCH 1/2]gss:krb5 miss returning error to caller when import security context

[Bian Naimeng]

krb5 miss returning error to up layer when import security context, 
it may be return ok though it has failed to import security context.

Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>

----

diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c
index 0326446..778e5df 100644
--- a/net/sunrpc/auth_gss/gss_krb5_mech.c
+++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
@@ -237,6 +237,7 @@ get_key(const void *p, const void *end,
 	if (!supported_gss_krb5_enctype(alg)) {
 		printk(KERN_WARNING "gss_kerberos_mech: unsupported "
 			"encryption key algorithm %d\n", alg);
+		p = ERR_PTR(-EINVAL);
 		goto out_err;
 	}
 	p = simple_get_netobj(p, end, &key);
@@ -282,15 +283,19 @@ gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
 	ctx->enctype = ENCTYPE_DES_CBC_RAW;
 
 	ctx->gk5e = get_gss_krb5_enctype(ctx->enctype);
-	if (ctx->gk5e == NULL)
+	if (ctx->gk5e == NULL) {
+		p = ERR_PTR(-EINVAL);
 		goto out_err;
+	}
 
 	/* The downcall format was designed before we completely understood
 	 * the uses of the context fields; so it includes some stuff we
 	 * just give some minimal sanity-checking, and some we ignore
 	 * completely (like the next twenty bytes): */
-	if (unlikely(p + 20 > end || p + 20 < p))
+	if (unlikely(p + 20 > end || p + 20 < p)) {
+		p = ERR_PTR(-EFAULT);
 		goto out_err;
+	}
 	p += 20;
 	p = simple_get_bytes(p, end, &tmp, sizeof(tmp));
 	if (IS_ERR(p))
@@ -619,6 +624,7 @@ gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
 	if (ctx->seq_send64 != ctx->seq_send) {
 		dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n", __func__,
 			(long unsigned)ctx->seq_send64, ctx->seq_send);
+		p = ERR_PTR(-EINVAL);
 		goto out_err;
 	}
 	p = simple_get_bytes(p, end, &ctx->enctype, sizeof(ctx->enctype));

-- 
Regards
Bian Naimeng

Re: [PATCH 1/2]gss:krb5 miss returning error to caller when import security context

[Trond Myklebust]

On Mon, 2010-09-06 at 11:36 +0800, Bian Naimeng wrote:
> krb5 miss returning error to up layer when import security context, 
> it may be return ok though it has failed to import security context.
> 
> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>
> 
> ----
> 
> diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c
> index 0326446..778e5df 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_mech.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
> @@ -237,6 +237,7 @@ get_key(const void *p, const void *end,
>  	if (!supported_gss_krb5_enctype(alg)) {
>  		printk(KERN_WARNING "gss_kerberos_mech: unsupported "
>  			"encryption key algorithm %d\n", alg);
> +		p = ERR_PTR(-EINVAL);
>  		goto out_err;
>  	}
>  	p = simple_get_netobj(p, end, &key);
> @@ -282,15 +283,19 @@ gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
>  	ctx->enctype = ENCTYPE_DES_CBC_RAW;
>  
>  	ctx->gk5e = get_gss_krb5_enctype(ctx->enctype);
> -	if (ctx->gk5e == NULL)
> +	if (ctx->gk5e == NULL) {
> +		p = ERR_PTR(-EINVAL);
>  		goto out_err;
> +	}
>  
>  	/* The downcall format was designed before we completely understood
>  	 * the uses of the context fields; so it includes some stuff we
>  	 * just give some minimal sanity-checking, and some we ignore
>  	 * completely (like the next twenty bytes): */
> -	if (unlikely(p + 20 > end || p + 20 < p))
> +	if (unlikely(p + 20 > end || p + 20 < p)) {
> +		p = ERR_PTR(-EFAULT);
>  		goto out_err;
> +	}
>  	p += 20;
>  	p = simple_get_bytes(p, end, &tmp, sizeof(tmp));
>  	if (IS_ERR(p))
> @@ -619,6 +624,7 @@ gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
>  	if (ctx->seq_send64 != ctx->seq_send) {
>  		dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n", __func__,
>  			(long unsigned)ctx->seq_send64, ctx->seq_send);
> +		p = ERR_PTR(-EINVAL);
>  		goto out_err;
>  	}
>  	p = simple_get_bytes(p, end, &ctx->enctype, sizeof(ctx->enctype));
> 

Those all look good. Applied!

Thanks for spotting them!

  Trond

[PATCH 2/2]gss:spkm3 miss returning error to caller when import security context

[Bian Naimeng]

spkm3 miss returning error to up layer when import security context, 
it may be return ok though it has failed to import security context.

Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>


---
 net/sunrpc/auth_gss/gss_spkm3_mech.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_spkm3_mech.c b/net/sunrpc/auth_gss/gss_spkm3_mech.c
index dc3f1f5..adade3d 100644
--- a/net/sunrpc/auth_gss/gss_spkm3_mech.c
+++ b/net/sunrpc/auth_gss/gss_spkm3_mech.c
@@ -100,6 +100,7 @@ gss_import_sec_context_spkm3(const void *p, size_t len,
 	if (version != 1) {
 		dprintk("RPC:       unknown spkm3 token format: "
 				"obsolete nfs-utils?\n");
+		p = ERR_PTR(-EINVAL);
 		goto out_err_free_ctx;
 	}
 
@@ -135,8 +136,10 @@ gss_import_sec_context_spkm3(const void *p, size_t len,
 	if (IS_ERR(p))
 		goto out_err_free_intg_alg;
 
-	if (p != end)
+	if (p != end) {
+		p = ERR_PTR(-EFAULT);
 		goto out_err_free_intg_key;
+	}
 
 	ctx_id->internal_ctx_id = ctx;
 
-- 
1.7.0


-- 
Regards
Bian Naimeng

Re: [PATCH 2/2]gss:spkm3 miss returning error to caller when import security context

[Trond Myklebust]

On Mon, 2010-09-06 at 11:38 +0800, Bian Naimeng wrote:
> spkm3 miss returning error to up layer when import security context, 
> it may be return ok though it has failed to import security context.
> 
> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com>
> 
> 
> ---
>  net/sunrpc/auth_gss/gss_spkm3_mech.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sunrpc/auth_gss/gss_spkm3_mech.c b/net/sunrpc/auth_gss/gss_spkm3_mech.c
> index dc3f1f5..adade3d 100644
> --- a/net/sunrpc/auth_gss/gss_spkm3_mech.c
> +++ b/net/sunrpc/auth_gss/gss_spkm3_mech.c
> @@ -100,6 +100,7 @@ gss_import_sec_context_spkm3(const void *p, size_t len,
>  	if (version != 1) {
>  		dprintk("RPC:       unknown spkm3 token format: "
>  				"obsolete nfs-utils?\n");
> +		p = ERR_PTR(-EINVAL);
>  		goto out_err_free_ctx;
>  	}
>  
> @@ -135,8 +136,10 @@ gss_import_sec_context_spkm3(const void *p, size_t len,
>  	if (IS_ERR(p))
>  		goto out_err_free_intg_alg;
>  
> -	if (p != end)
> +	if (p != end) {
> +		p = ERR_PTR(-EFAULT);
>  		goto out_err_free_intg_key;
> +	}
>  
>  	ctx_id->internal_ctx_id = ctx;
>  
> -- 
> 1.7.0
> 
> 

Ditto. Although, I'm wondering if we shouldn't just start ripping out
the spkm stuff at this point. It is pretty much dead as far as the IETF
is concerned.

Cheers
  Trond