2010-09-06 03:35:08

by Bian Naimeng

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

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

2010-09-06 03:36:34

by Bian Naimeng

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

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 <[email protected]>

----

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

2010-09-07 18:36:11

by Trond Myklebust

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

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 <[email protected]>
>
>
> ---
> 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

2010-09-06 03:38:18

by Bian Naimeng

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

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 <[email protected]>


---
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

2010-09-07 18:35:07

by Trond Myklebust

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

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 <[email protected]>
>
> ----
>
> 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