[patch 5/6] Implement a new version of lucid spkm3 context.

Trond Myklebust trond.myklebust at fys.uio.no
Mon Oct 30 14:35:32 EST 2006


On Mon, 2006-10-30 at 12:51 -0500, J. Bruce Fields wrote:
> On Mon, Oct 16, 2006 at 08:41:57PM -0400, J. Bruce Fields wrote:
> > On Tue, Oct 17, 2006 at 09:54:06AM +1000, Neil Brown wrote:
> > > On Friday October 13, trond.myklebust at fys.uio.no wrote:
> > > > On Fri, 2006-10-13 at 15:35 -0400, kwc at citi.umich.edu wrote:
> > > > > @@ -51,6 +51,7 @@
> > > > >   *
> > > > >   *	buf->length should be:
> > > > >   *
> > > > > + *      version 4
> > > > 
> > > > How is this versioning useful? There doesn't appear to be any
> > > > provisioning for a fallback to previous versions for kernels that don't
> > > > support version 4. Am I missing something?
> > > 
> > > Ping? I would like to see the answer to this one too.
> 
> By the way, here's the kernel side of that change.  Opinions?
> 
> --b.
> 
> commit f8dc1aad1ffa1ce0eda507be3d7121d109b5bd6b
> Author: Olga Kornievskaia <aglo at citi.umich.edu>
> Date:   Tue Jun 13 13:22:15 2006 -0400
> 
>     rpc: spkm3 update
>     
>     This updates the spkm3 code to bring it up to date with our current
>     understanding of the spkm3 spec.
>     
>     In doing so, we're changing the downcall format used by gssd in the spkm3 case,
>     which will cause an incompatilibity with old userland spkm3 support.  Since the
>     old code a) didn't implement the protocol correctly, and b) was never
>     distributed except in the form of some experimental patches from the citi web
>     site, we're assuming this is OK.
>     
>     We do detect the old downcall format and print warning (and fail).  We also
>     include a version number in the new downcall format, to be used in the
>     future in case any further change is required.
>     
>     In some more detail:
>     
>     	- fix integrity support
>     	- removed dependency on NIDs. instead OIDs are used
>     	- known OID values for algorithms added.
>     	- fixed some context fields and types
>     
>     Signed-off-by: J. Bruce Fields <bfields at citi.umich.edu>
> 
> diff --git a/include/linux/sunrpc/gss_spkm3.h b/include/linux/sunrpc/gss_spkm3.h
> index 2cf3fbb..998fbea 100644
> --- a/include/linux/sunrpc/gss_spkm3.h
> +++ b/include/linux/sunrpc/gss_spkm3.h
> @@ -12,27 +12,27 @@
>  #include <linux/sunrpc/gss_asn1.h>
>  
>  struct spkm3_ctx {
> -	struct xdr_netobj	ctx_id; /* per message context id */
> -	int			qop;         /* negotiated qop */
> +	struct xdr_netobj	ctx_id;  /* per message context id */
> +	int			endtime; /* endtime of the context */
>  	struct xdr_netobj	mech_used;
>  	unsigned int		ret_flags ;
> -	unsigned int		req_flags ;
> -	struct xdr_netobj	share_key;
> -	int			conf_alg;
> -	struct crypto_blkcipher	*derived_conf_key;
> -	int			intg_alg;
> -	struct crypto_blkcipher	*derived_integ_key;
> -	int			keyestb_alg;   /* alg used to get share_key */
> -	int			owf_alg;   /* one way function */
> +	struct xdr_netobj	conf_alg;
> +	struct xdr_netobj	derived_conf_key;
> +	struct xdr_netobj	intg_alg;
> +	struct xdr_netobj 	derived_integ_key;
>  };
>  
> -/* from openssl/objects.h */
> -/* XXX need SEAL_ALG_NONE */
> -#define NID_md5		4
> -#define NID_dhKeyAgreement	28 
> -#define NID_des_cbc		31 
> -#define NID_sha1		64
> -#define NID_cast5_cbc		108
> +/* OIDs declarations for K-ALG, I-ALG, C-ALG, and OWF-ALG */
> +static const struct xdr_netobj dh_oid = {9, "\x2A\x86\x48\x86\xF7\x0D\x01\x03\x01"};
> +static const struct xdr_netobj hmac_md5_oid = { 8, "\x2B\x06\x01\x05\x05\x08\x01\x01"};
> +static const struct xdr_netobj null_mac_oid = { 6, "\x2B\x06\x01\x05\x03\x03"};
> +static const struct xdr_netobj md5_rsa_encryption_oid = {9, "\x2A\x86\x48\x86\xF7\x0D\x01\x01\x04"};
> +static const struct xdr_netobj sha1_rsa_encryption_oid = {9, "\x2A\x86\x48\x86\xF7\x0D\x01\x01\x05"};
> +
> +static const struct xdr_netobj cast5_cbc_oid = {9, "\x2A\x86\x48\x86\xF6\x7D\x07\x42\x0A"};
> +static const struct xdr_netobj des_cbc_oid = {5, "\x2B\x0E\x03\x02\x07"};
> +
> +static const struct xdr_netobj sha1_oid = {5, "\x2B\x0E\x03\x02\x1A"};
>  
>  /* SPKM InnerContext Token types */
>  
> @@ -46,11 +46,12 @@ u32 spkm3_make_token(struct spkm3_ctx *c
>  u32 spkm3_read_token(struct spkm3_ctx *ctx, struct xdr_netobj *read_token, struct xdr_buf *message_buffer, int toktype);
>  
>  #define CKSUMTYPE_RSA_MD5            0x0007
> +#define CKSUMTYPE_HMAC_MD5           0x0008
>  
> -s32 make_checksum(s32 cksumtype, char *header, int hdrlen, struct xdr_buf *body,
> +s32 make_spkm3_checksum(s32 cksumtype, struct xdr_netobj *key, char *header, int hdrlen, struct xdr_buf *body,
>                     int body_offset, struct xdr_netobj *cksum);
>  void asn1_bitstring_len(struct xdr_netobj *in, int *enclen, int *zerobits);
> -int decode_asn1_bitstring(struct xdr_netobj *out, char *in, int enclen, 
> +int decode_asn1_bitstring(struct xdr_netobj *out, char *in, int enclen,
>                     int explen);
>  void spkm3_mic_header(unsigned char **hdrbuf, unsigned int *hdrlen, 
>                     unsigned char *ctxhdr, int elen, int zbit);
> diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> index ac69e55..cb2d858 100644
> --- a/include/linux/sunrpc/xdr.h
> +++ b/include/linux/sunrpc/xdr.h
> @@ -11,6 +11,7 @@
>  
>  #include <linux/uio.h>
>  #include <asm/byteorder.h>
> +#include <linux/scatterlist.h>
>  
>  /*
>   * Buffer adjustment
> @@ -196,6 +197,7 @@ extern void xdr_init_decode(struct xdr_s
>  extern __be32 *xdr_inline_decode(struct xdr_stream *xdr, size_t nbytes);
>  extern void xdr_read_pages(struct xdr_stream *xdr, unsigned int len);
>  extern void xdr_enter_page(struct xdr_stream *xdr, unsigned int len);
> +extern int xdr_process_buf(struct xdr_buf *buf, int offset, int len, int (*actor)(struct scatterlist *, void *), void *data);
>  
>  #endif /* __KERNEL__ */
>  
> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index b36b946..00c4042 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -68,7 +68,7 @@ static struct rpc_credops gss_credops;
>  #define GSS_CRED_SLACK		1024		/* XXX: unused */
>  /* length of a krb5 verifier (48), plus data added before arguments when
>   * using integrity (two 4-byte integers): */
> -#define GSS_VERF_SLACK		56
> +#define GSS_VERF_SLACK		100
>  
>  /* XXX this define must match the gssd define
>  * as it is passed to gssd to signal the use of
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index e11a40b..aa53257 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -43,6 +43,7 @@
>  #include <linux/highmem.h>
>  #include <linux/pagemap.h>
>  #include <linux/sunrpc/gss_krb5.h>
> +#include <linux/sunrpc/xdr.h>
>  
>  #ifdef RPC_DEBUG
>  # define RPCDBG_FACILITY        RPCDBG_AUTH
> @@ -132,72 +133,6 @@ out:
>  EXPORT_SYMBOL(krb5_decrypt);
>  
>  static int
> -process_xdr_buf(struct xdr_buf *buf, int offset, int len,
> -		int (*actor)(struct scatterlist *, void *), void *data)
> -{
> -	int i, page_len, thislen, page_offset, ret = 0;
> -	struct scatterlist	sg[1];
> -
> -	if (offset >= buf->head[0].iov_len) {
> -		offset -= buf->head[0].iov_len;
> -	} else {
> -		thislen = buf->head[0].iov_len - offset;
> -		if (thislen > len)
> -			thislen = len;
> -		sg_set_buf(sg, buf->head[0].iov_base + offset, thislen);
> -		ret = actor(sg, data);
> -		if (ret)
> -			goto out;
> -		offset = 0;
> -		len -= thislen;
> -	}
> -	if (len == 0)
> -		goto out;
> -
> -	if (offset >= buf->page_len) {
> -		offset -= buf->page_len;
> -	} else {
> -		page_len = buf->page_len - offset;
> -		if (page_len > len)
> -			page_len = len;
> -		len -= page_len;
> -		page_offset = (offset + buf->page_base) & (PAGE_CACHE_SIZE - 1);
> -		i = (offset + buf->page_base) >> PAGE_CACHE_SHIFT;
> -		thislen = PAGE_CACHE_SIZE - page_offset;
> -		do {
> -			if (thislen > page_len)
> -				thislen = page_len;
> -			sg->page = buf->pages[i];
> -			sg->offset = page_offset;
> -			sg->length = thislen;
> -			ret = actor(sg, data);
> -			if (ret)
> -				goto out;
> -			page_len -= thislen;
> -			i++;
> -			page_offset = 0;
> -			thislen = PAGE_CACHE_SIZE;
> -		} while (page_len != 0);
> -		offset = 0;
> -	}
> -	if (len == 0)
> -		goto out;
> -
> -	if (offset < buf->tail[0].iov_len) {
> -		thislen = buf->tail[0].iov_len - offset;
> -		if (thislen > len)
> -			thislen = len;
> -		sg_set_buf(sg, buf->tail[0].iov_base + offset, thislen);
> -		ret = actor(sg, data);
> -		len -= thislen;
> -	}
> -	if (len != 0)
> -		ret = -EINVAL;
> -out:
> -	return ret;
> -}
> -
> -static int
>  checksummer(struct scatterlist *sg, void *data)
>  {
>  	struct hash_desc *desc = data;
> @@ -237,7 +172,7 @@ make_checksum(s32 cksumtype, char *heade
>  	err = crypto_hash_update(&desc, sg, hdrlen);
>  	if (err)
>  		goto out;
> -	err = process_xdr_buf(body, body_offset, body->len - body_offset,
> +	err = xdr_process_buf(body, body_offset, body->len - body_offset,
>  			      checksummer, &desc);
>  	if (err)
>  		goto out;
> @@ -335,7 +270,7 @@ gss_encrypt_xdr_buf(struct crypto_blkcip
>  	desc.fragno = 0;
>  	desc.fraglen = 0;
>  
> -	ret = process_xdr_buf(buf, offset, buf->len - offset, encryptor, &desc);
> +	ret = xdr_process_buf(buf, offset, buf->len - offset, encryptor, &desc);
>  	return ret;
>  }
>  
> @@ -401,7 +336,7 @@ gss_decrypt_xdr_buf(struct crypto_blkcip
>  	desc.desc.flags = 0;
>  	desc.fragno = 0;
>  	desc.fraglen = 0;
> -	return process_xdr_buf(buf, offset, buf->len - offset, decryptor, &desc);
> +	return xdr_process_buf(buf, offset, buf->len - offset, decryptor, &desc);
>  }
>  
>  EXPORT_SYMBOL(gss_decrypt_xdr_buf);
> diff --git a/net/sunrpc/auth_gss/gss_spkm3_mech.c b/net/sunrpc/auth_gss/gss_spkm3_mech.c
> index bdedf45..3687f77 100644
> --- a/net/sunrpc/auth_gss/gss_spkm3_mech.c
> +++ b/net/sunrpc/auth_gss/gss_spkm3_mech.c
> @@ -83,133 +83,73 @@ simple_get_netobj(const void *p, const v
>  	return q;
>  }
>  
> -static inline const void *
> -get_key(const void *p, const void *end, struct crypto_blkcipher **res,
> -	int *resalg)
> -{
> -	struct xdr_netobj	key = { 0 };
> -	int			setkey = 0;
> -	char			*alg_name;
> -
> -	p = simple_get_bytes(p, end, resalg, sizeof(*resalg));
> -	if (IS_ERR(p))
> -		goto out_err;
> -	p = simple_get_netobj(p, end, &key);
> -	if (IS_ERR(p))
> -		goto out_err;
> -
> -	switch (*resalg) {
> -		case NID_des_cbc:
> -			alg_name = "cbc(des)";
> -			setkey = 1;
> -			break;
> -		case NID_cast5_cbc:
> -			/* XXXX here in name only, not used */
> -			alg_name = "cbc(cast5)";
> -			setkey = 0; /* XXX will need to set to 1 */
> -			break;
> -		case NID_md5:
> -			if (key.len == 0) {
> -				dprintk("RPC: SPKM3 get_key: NID_md5 zero Key length\n");
> -			}
> -			alg_name = "md5";
> -			setkey = 0;
> -			break;
> -		default:
> -			dprintk("gss_spkm3_mech: unsupported algorithm %d\n", *resalg);
> -			goto out_err_free_key;
> -	}
> -	*res = crypto_alloc_blkcipher(alg_name, 0, CRYPTO_ALG_ASYNC);
> -	if (IS_ERR(*res)) {
> -		printk("gss_spkm3_mech: unable to initialize crypto algorthm %s\n", alg_name);
> -		*res = NULL;
> -		goto out_err_free_key;
> -	}
> -	if (setkey) {
> -		if (crypto_blkcipher_setkey(*res, key.data, key.len)) {
> -			printk("gss_spkm3_mech: error setting key for crypto algorthm %s\n", alg_name);
> -			goto out_err_free_tfm;
> -		}
> -	}
> -
> -	if(key.len > 0)
> -		kfree(key.data);
> -	return p;
> -
> -out_err_free_tfm:
> -	crypto_free_blkcipher(*res);
> -out_err_free_key:
> -	if(key.len > 0)
> -		kfree(key.data);
> -	p = ERR_PTR(-EINVAL);
> -out_err:
> -	return p;
> -}
> -
>  static int
>  gss_import_sec_context_spkm3(const void *p, size_t len,
>  				struct gss_ctx *ctx_id)
>  {
>  	const void *end = (const void *)((const char *)p + len);
>  	struct	spkm3_ctx *ctx;
> +	int	version;
>  
>  	if (!(ctx = kzalloc(sizeof(*ctx), GFP_KERNEL)))
>  		goto out_err;
>  
> +	p = simple_get_bytes(p, end, &version, sizeof(version));
> +	if (IS_ERR(p))
> +		goto out_err_free_ctx;
> +        if (version >= 16 || version <= 0) {

indentation
> +		dprintk("RPC: old nfs-utils software. upgrade to the latest version\n");
> +		goto out_err_free_ctx;
> +	}
> +
>  	p = simple_get_netobj(p, end, &ctx->ctx_id);
>  	if (IS_ERR(p))
>  		goto out_err_free_ctx;
>  
> -	p = simple_get_bytes(p, end, &ctx->qop, sizeof(ctx->qop));
> +	p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx->endtime));
>  	if (IS_ERR(p))
>  		goto out_err_free_ctx_id;
>  
>  	p = simple_get_netobj(p, end, &ctx->mech_used);
>  	if (IS_ERR(p))
> -		goto out_err_free_mech;
> +		goto out_err_free_ctx_id;
>  
>  	p = simple_get_bytes(p, end, &ctx->ret_flags, sizeof(ctx->ret_flags));
>  	if (IS_ERR(p))
>  		goto out_err_free_mech;
>  
> -	p = simple_get_bytes(p, end, &ctx->req_flags, sizeof(ctx->req_flags));
> +	p = simple_get_netobj(p, end, &ctx->conf_alg);
>  	if (IS_ERR(p))
>  		goto out_err_free_mech;
>  
> -	p = simple_get_netobj(p, end, &ctx->share_key);
> -	if (IS_ERR(p))
> -		goto out_err_free_s_key;
> -
> -	p = get_key(p, end, &ctx->derived_conf_key, &ctx->conf_alg);
> +	p = simple_get_netobj(p, end, &ctx->derived_conf_key);
>  	if (IS_ERR(p))
> -		goto out_err_free_s_key;
> +		goto out_err_free_conf_alg;
>  
> -	p = get_key(p, end, &ctx->derived_integ_key, &ctx->intg_alg);
> +	p = simple_get_netobj(p, end, &ctx->intg_alg);
>  	if (IS_ERR(p))
> -		goto out_err_free_key1;
> +		goto out_err_free_conf_key;
>  
> -	p = simple_get_bytes(p, end, &ctx->keyestb_alg, sizeof(ctx->keyestb_alg));
> +	p = simple_get_netobj(p, end, &ctx->derived_integ_key);
>  	if (IS_ERR(p))
> -		goto out_err_free_key2;
> -
> -	p = simple_get_bytes(p, end, &ctx->owf_alg, sizeof(ctx->owf_alg));
> -	if (IS_ERR(p))
> -		goto out_err_free_key2;
> +		goto out_err_free_intg_alg;
>  
>  	if (p != end)
> -		goto out_err_free_key2;
> +		goto out_err_free_intg_key;
>  
>  	ctx_id->internal_ctx_id = ctx;
>  
>  	dprintk("Successfully imported new spkm context.\n");
>  	return 0;
>  
> -out_err_free_key2:
> -	crypto_free_blkcipher(ctx->derived_integ_key);
> -out_err_free_key1:
> -	crypto_free_blkcipher(ctx->derived_conf_key);
> -out_err_free_s_key:
> -	kfree(ctx->share_key.data);
> +out_err_free_intg_key:
> +	kfree(ctx->derived_integ_key.data);
> +out_err_free_intg_alg:
> +	kfree(ctx->intg_alg.data);
> +out_err_free_conf_key:
> +	kfree(ctx->derived_conf_key.data);
> +out_err_free_conf_alg:
> +	kfree(ctx->conf_alg.data);
>  out_err_free_mech:
>  	kfree(ctx->mech_used.data);
>  out_err_free_ctx_id:
> @@ -224,11 +164,13 @@ static void
>  gss_delete_sec_context_spkm3(void *internal_ctx) {
>  	struct spkm3_ctx *sctx = internal_ctx;
>  
> -	crypto_free_blkcipher(sctx->derived_integ_key);
> -	crypto_free_blkcipher(sctx->derived_conf_key);
> -	kfree(sctx->share_key.data);
> -	kfree(sctx->mech_used.data);
> -	kfree(sctx);
> +        kfree(sctx->derived_integ_key.data);
> +        kfree(sctx->intg_alg.data);
> +        kfree(sctx->derived_conf_key.data);
> +        kfree(sctx->conf_alg.data);
> +        kfree(sctx->mech_used.data);
> +        kfree(sctx->ctx_id.data);
> +        kfree(sctx);

The above indentation looks screwy. Why are you changing it?

>  }
>  
>  static u32
> @@ -239,7 +181,6 @@ gss_verify_mic_spkm3(struct gss_ctx		*ct
>  	u32 maj_stat = 0;
>  	struct spkm3_ctx *sctx = ctx->internal_ctx_id;
>  
> -	dprintk("RPC: gss_verify_mic_spkm3 calling spkm3_read_token\n");
>  	maj_stat = spkm3_read_token(sctx, checksum, signbuf, SPKM_MIC_TOK);
>  
>  	dprintk("RPC: gss_verify_mic_spkm3 returning %d\n", maj_stat);
> @@ -254,10 +195,9 @@ gss_get_mic_spkm3(struct gss_ctx	*ctx,
>  	u32 err = 0;
>  	struct spkm3_ctx *sctx = ctx->internal_ctx_id;
>  
> -	dprintk("RPC: gss_get_mic_spkm3\n");
> -
>  	err = spkm3_make_token(sctx, message_buffer,
> -			      message_token, SPKM_MIC_TOK);
> +				message_token, SPKM_MIC_TOK);
> +	dprintk("RPC: gss_get_mic_spkm3 returning %d\n", err);
>  	return err;
>  }
>  
> diff --git a/net/sunrpc/auth_gss/gss_spkm3_seal.c b/net/sunrpc/auth_gss/gss_spkm3_seal.c
> index 18c7862..c83d994 100644
> --- a/net/sunrpc/auth_gss/gss_spkm3_seal.c
> +++ b/net/sunrpc/auth_gss/gss_spkm3_seal.c
> @@ -39,6 +39,9 @@
>  #include <linux/sunrpc/gss_spkm3.h>
>  #include <linux/random.h>
>  #include <linux/crypto.h>
> +#include <linux/pagemap.h>
> +#include <linux/scatterlist.h>
> +#include <linux/sunrpc/xdr.h>
>  
>  #ifdef RPC_DEBUG
>  # define RPCDBG_FACILITY        RPCDBG_AUTH
> @@ -66,8 +69,6 @@ spkm3_make_token(struct spkm3_ctx *ctx,
>  	int			ctxelen = 0, ctxzbit = 0;
>  	int			md5elen = 0, md5zbit = 0;
>  
> -	dprintk("RPC: spkm3_make_token\n");
> -
>  	now = jiffies;
>  
>  	if (ctx->ctx_id.len != 16) {
> @@ -75,21 +76,17 @@ spkm3_make_token(struct spkm3_ctx *ctx,
>  			ctx->ctx_id.len);
>  		goto out_err;
>  	}
> -		
> -	switch (ctx->intg_alg) {
> -		case NID_md5:
> -			checksum_type = CKSUMTYPE_RSA_MD5;
> -			break;
> -		default:
> -			dprintk("RPC: gss_spkm3_seal: ctx->signalg %d not"
> -				" supported\n", ctx->intg_alg);
> -			goto out_err;
> -	}
> -	/* XXX since we don't support WRAP, perhaps we don't care... */
> -	if (ctx->conf_alg != NID_cast5_cbc) {
> -		dprintk("RPC: gss_spkm3_seal: ctx->sealalg %d not supported\n",
> -			ctx->conf_alg);
> -		goto out_err;
> +
> +	if (!g_OID_equal(&ctx->intg_alg, &hmac_md5_oid)) {
> +	        dprintk("RPC: gss_spkm3_seal: unsupported I-ALG algorithm."
> +		        "only support hmac-md5 I-ALG.\n");
> +	        goto out_err;
> +	} else 
> +		checksum_type = CKSUMTYPE_HMAC_MD5;
> +
> +	if (!g_OID_equal(&ctx->conf_alg, &cast5_cbc_oid)) {
> +	        dprintk("RPC: gss_spkm3_seal: unsupported C-ALG algorithm\n");
> +	        goto out_err;
>  	}
>  
>  	if (toktype == SPKM_MIC_TOK) {
> @@ -97,10 +94,10 @@ spkm3_make_token(struct spkm3_ctx *ctx,
>  		asn1_bitstring_len(&ctx->ctx_id, &ctxelen, &ctxzbit);
>  		spkm3_mic_header(&mic_hdr.data, &mic_hdr.len, ctx->ctx_id.data,
>  		                         ctxelen, ctxzbit);
> -
> -		if (make_checksum(checksum_type, mic_hdr.data, mic_hdr.len, 
> -		                             text, 0, &md5cksum))
> -			goto out_err;
> +                if (make_spkm3_checksum(checksum_type, &ctx->derived_integ_key,

indentation

> +					(char *)mic_hdr.data, mic_hdr.len,
> +					text, 0, &md5cksum))
> +                        goto out_err;
>  
>  		asn1_bitstring_len(&md5cksum, &md5elen, &md5zbit);
>  		tokenlen = 10 + ctxelen + 1 + md5elen + 1;
> @@ -121,7 +118,66 @@ spkm3_make_token(struct spkm3_ctx *ctx,
>  
>  	return  GSS_S_COMPLETE;
>  out_err:
> +        if (md5cksum.data)

indentation

> +		kfree(md5cksum.data);
> +
>  	token->data = NULL;
>  	token->len = 0;
>  	return GSS_S_FAILURE;
>  }
> +
> +static int
> +spkm3_checksummer(struct scatterlist *sg, void *data)
> +{
> +	struct hash_desc *desc = data;
> +
> +        return crypto_hash_update(desc, sg, sg->length);

indentation

> +}
> +
> +/* checksum the plaintext data and hdrlen bytes of the token header */
> +s32
> +make_spkm3_checksum(s32 cksumtype, struct xdr_netobj *key, char *header,
> +		    int hdrlen, struct xdr_buf *body,
> +		    int body_offset, struct xdr_netobj *cksum)

hdrlen and body_offset want to be unsigned.
> +{
> +        char                            *cksumname;

indentation

> +	struct hash_desc		desc; /* XXX add to ctx? */
> +        struct scatterlist              sg[1];

indentation

> +	int err;
> +
> +        switch (cksumtype) {
> +                case CKSUMTYPE_HMAC_MD5:
> +                        cksumname = "md5";
> +                        break;
> +                default:
> +                        dprintk("RPC:      spkm3_make_checksum:"
> +                                " unsupported checksum %d", cksumtype);
> +                        return GSS_S_FAILURE;
> +        }

indentation (for all the above lines)

> +
> +	if (key->data == NULL || key->len <= 0) return GSS_S_FAILURE;
> +
> +	desc.tfm = crypto_alloc_hash(cksumname, 0, CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(desc.tfm))
> +		return GSS_S_FAILURE;
> +        cksum->len = crypto_hash_digestsize(desc.tfm);

indentaion

> +	desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +
> +	err = crypto_hash_setkey(desc.tfm, key->data, key->len);
> +	if (err)
> +		goto out;
> +		
> +        sg_set_buf(sg, header, hdrlen);
> +        crypto_hash_update(&desc, sg, 1);

indentation (above 2 lines)

> +
> +        xdr_process_buf(body, body_offset, body->len - body_offset,
> +			      spkm3_checksummer, &desc);
> +        crypto_hash_final(&desc, cksum->data);

indentation (above 3 lines)

> +
> +out:
> +        crypto_free_hash(desc.tfm);
> +
> +        return err ? GSS_S_FAILURE : 0;

indentation (above 3 lines)

> +}
> +
> +EXPORT_SYMBOL(make_spkm3_checksum);
> diff --git a/net/sunrpc/auth_gss/gss_spkm3_token.c b/net/sunrpc/auth_gss/gss_spkm3_token.c
> index 854a983..35188b6 100644
> --- a/net/sunrpc/auth_gss/gss_spkm3_token.c
> +++ b/net/sunrpc/auth_gss/gss_spkm3_token.c
> @@ -172,10 +172,10 @@ spkm3_mic_header(unsigned char **hdrbuf,
>  	*(u8 *)hptr++ = zbit;
>  	memcpy(hptr, ctxdata, elen);
>  	hptr += elen;
> -	*hdrlen = hptr - top; 
> +	*hdrlen = hptr - top;
>  }
> -		
> -/* 
> +
> +/*
>   * spkm3_mic_innercontext_token()
>   *
>   * *tokp points to the beginning of the SPKM_MIC token  described 
> diff --git a/net/sunrpc/auth_gss/gss_spkm3_unseal.c b/net/sunrpc/auth_gss/gss_spkm3_unseal.c
> index 8537f58..fa3a734 100644
> --- a/net/sunrpc/auth_gss/gss_spkm3_unseal.c
> +++ b/net/sunrpc/auth_gss/gss_spkm3_unseal.c
> @@ -54,6 +54,7 @@ spkm3_read_token(struct spkm3_ctx *ctx,
>  		struct xdr_buf *message_buffer, /* signbuf */
>  		int toktype)
>  {
> +	s32			checksum_type;
>  	s32			code;
>  	struct xdr_netobj	wire_cksum = {.len =0, .data = NULL};
>  	char			cksumdata[16];
> @@ -64,8 +65,6 @@ spkm3_read_token(struct spkm3_ctx *ctx,
>  	int			mic_hdrlen;
>  	u32			ret = GSS_S_DEFECTIVE_TOKEN;
>  
> -	dprintk("RPC: spkm3_read_token read_token->len %d\n", read_token->len);
> -
>  	if (g_verify_token_header((struct xdr_netobj *) &ctx->mech_used,
>  					&bodysize, &ptr, read_token->len))
>  		goto out;
> @@ -94,26 +93,27 @@ spkm3_read_token(struct spkm3_ctx *ctx,
>  		*  mic_hdrlen + 2 = length of header piece of checksum
>  		*/
>  		ret = GSS_S_DEFECTIVE_TOKEN;
> -		code = make_checksum(CKSUMTYPE_RSA_MD5, ptr + 2, 
> -					mic_hdrlen + 2, 
> -		                        message_buffer, 0, &md5cksum);
> +		if (!g_OID_equal(&ctx->intg_alg, &hmac_md5_oid)) {
> +			dprintk("RPC: gss_spkm3_seal: unsupported I-ALG algorithm. only support hmac-md5 I-ALG.\n");
> +			goto out;
> +		} else 

'goto out' should ensure that the next line won't be executed in case
the condition is true. Adding an "else" here just confuses.

> +			checksum_type = CKSUMTYPE_HMAC_MD5;
> +
> +		code = make_spkm3_checksum(checksum_type,
> +			&ctx->derived_integ_key, ptr + 2, mic_hdrlen + 2,
> +			message_buffer, 0, &md5cksum);
>  
>  		if (code)
>  			goto out;
>  
> -		dprintk("RPC: spkm3_read_token: digest wire_cksum.len %d:\n", 
> -			wire_cksum.len);
> -		dprintk("          md5cksum.data\n");
> -		print_hexl((u32 *) md5cksum.data, 16, 0);
> -		dprintk("          cksum.data:\n");
> -		print_hexl((u32 *) wire_cksum.data, wire_cksum.len, 0);
> -
>  		ret = GSS_S_BAD_SIG;
>  		code = memcmp(md5cksum.data, wire_cksum.data, wire_cksum.len);
> -		if (code)
> +		if (code) {
> +			dprintk("RPC: bad MIC checksum\n");
>  			goto out;
> +		}
>  
> -	} else { 
> +	} else {
>  		dprintk("RPC: BAD or UNSUPPORTED SPKM3 token type: %d\n",toktype);
>  		goto out;
>  	}

Note: you could change the above condition to

	if (toktype != SPKM_MIC_TOK) {
		dprintk("RPC: BAD or UNSUPPORTED SPKM3 token type: %d\n",toktype);
		goto out;
	}

and get rid of an bunch of unnecessary indentation instead.

> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> index 9022eb8..c0f83e5 100644
> --- a/net/sunrpc/xdr.c
> +++ b/net/sunrpc/xdr.c
> @@ -1020,3 +1020,68 @@ xdr_encode_array2(struct xdr_buf *buf, u
>  
>  	return xdr_xcode_array2(buf, base, desc, 1);
>  }
> +
> +int
> +xdr_process_buf(struct xdr_buf *buf, int offset, int len,
> +                int (*actor)(struct scatterlist *, void *), void *data)
> +{
> +        int i, page_len, thislen, page_offset, ret = 0;

offset, len, page_len, thislen, page_offset really want to be either
size_t or unsigned int. They definitely shouldn't be signed.

> +        struct scatterlist      sg[1];
> +
> +        if (offset >= buf->head[0].iov_len) {
> +                offset -= buf->head[0].iov_len;
> +        } else {                 thislen = buf->head[0].iov_len - offset;

Formatting...

> +                if (thislen > len)
> +                        thislen = len;
> +                sg_set_buf(sg, buf->head[0].iov_base + offset, thislen);
> +                ret = actor(sg, data);
> +                if (ret)
> +                        goto out;
> +                offset = 0;
> +                len -= thislen;
> +        }
> +        if (len == 0)
> +                goto out;
> +
> +        if (offset >= buf->page_len) {
> +                offset -= buf->page_len;
> +        } else {
> +                page_len = buf->page_len - offset;
> +                if (page_len > len)
> +                        page_len = len;
> +                len -= page_len;
> +                page_offset = (offset + buf->page_base) & (PAGE_CACHE_SIZE - 1);                i = (offset + buf->page_base) >> PAGE_CACHE_SHIFT;

Are you trying to hide something from us all? :-) 1 statement per line
please.

> +                thislen = PAGE_CACHE_SIZE - page_offset;
> +                do {
> +                        if (thislen > page_len)
> +                                thislen = page_len;
> +                        sg->page = buf->pages[i];
> +                        sg->offset = page_offset;
> +                        sg->length = thislen;
> +                        ret = actor(sg, data);
> +                        if (ret)
> +                                goto out;
> +                        page_len -= thislen;
> +                        i++;
> +                        page_offset = 0;
> +                        thislen = PAGE_CACHE_SIZE;
> +                } while (page_len != 0);
> +                offset = 0;
> +        }
> +        if (len == 0)
> +                goto out;
> +         if (offset < buf->tail[0].iov_len) {
> +                thislen = buf->tail[0].iov_len - offset;
> +                if (thislen > len)
> +                        thislen = len;
> +                sg_set_buf(sg, buf->tail[0].iov_base + offset, thislen);
> +                ret = actor(sg, data);
> +                len -= thislen;
> +        }
> +        if (len != 0)
> +                ret = -EINVAL;
> +out:
> +        return ret;
> +}

The indentation on the entire function above looks screwed up. Please
use 8-character tabs instead of spaces.

> +EXPORT_SYMBOL(xdr_process_buf);
> +
> _______________________________________________
> NFSv4 mailing list
> NFSv4 at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4



More information about the NFSv4 mailing list