[PATCH 15/28] gss_krb5: introduce encryption type framework
Chuck Lever
chuck.lever at oracle.com
Mon Mar 31 12:58:08 EDT 2008
On Mar 31, 2008, at 12:38 PM, Kevin Coffman wrote:
> On Mon, Mar 31, 2008 at 11:43 AM, Chuck Lever
> <chuck.lever at oracle.com> wrote:
>>
>> On Mar 31, 2008, at 10:32 AM, Kevin Coffman wrote:
>>> Add enctype framework and change functions to use the generic
>>> values from it rather than the values hard-coded for des.
>>>
>>> Signed-off-by: Kevin Coffman <kwc at citi.umich.edu>
>>> ---
>>>
>>> include/linux/sunrpc/gss_krb5.h | 29 +++++++++++
>>> net/sunrpc/auth_gss/gss_krb5_crypto.c | 16 +++---
>>> net/sunrpc/auth_gss/gss_krb5_mech.c | 88 ++++++++++++++++++++++
>>> +++++------
>>> net/sunrpc/auth_gss/gss_krb5_seal.c | 48 +++++++++++-------
>>> net/sunrpc/auth_gss/gss_krb5_unseal.c | 12 +++--
>>> net/sunrpc/auth_gss/gss_krb5_wrap.c | 49 +++++++++++-------
>>> 6 files changed, 175 insertions(+), 67 deletions(-)
>>>
>>> diff --git a/include/linux/sunrpc/gss_krb5.h b/include/linux/sunrpc/
>>> gss_krb5.h
>>> index 973a3cc..790f507 100644
>>> --- a/include/linux/sunrpc/gss_krb5.h
>>> +++ b/include/linux/sunrpc/gss_krb5.h
>>> @@ -36,13 +36,42 @@
>>> *
>>> */
>>>
>>> +#include <linux/crypto.h>
>>> #include <linux/sunrpc/auth_gss.h>
>>> #include <linux/sunrpc/gss_err.h>
>>> #include <linux/sunrpc/gss_asn1.h>
>>>
>>> +/* Maximum checksum function output for the supported crypto
>>> algorithms */
>>> +#define GSS_KRB5_MAX_CKSUM_LEN 20
>>> +
>>> +/* Maximum blocksize for the supported crypto algorithms */
>>> +#define GSS_KRB5_MAX_BLOCKSIZE 16
>>> +
>>> +struct gss_krb5_enctype {
>>> + const u32 etype; /* encryption (key)
>>> type */
>>> + const u32 ctype; /* checksum type */
>>> + const char *name; /* "friendly" name */
>>> + const char *encrypt_name; /* crypto encrypt
>>> name */
>>> + const char *cksum_name; /* crypto checksum
>>> name */
>>> + const u16 signalg; /* signing
>>> algorithm */
>>> + const u16 sealalg; /* sealing
>>> algorithm */
>>> + const u32 blocksize; /* encryption
>>> blocksize */
>>> + const u32 cksumlength; /* checksum length */
>>> + const u32 keyed_cksum; /* is it a keyed
>>> cksum? */
>>> + const u32 keybytes; /* raw key len, in
>>> bytes */
>>> + const u32 keylength; /* final key len,
>>> in bytes */
>>> + u32 (*encrypt) (struct crypto_blkcipher *tfm,
>>> + void *iv, void *in, void *out,
>>> + int length); /* encryption
>>> function */
>>> + u32 (*decrypt) (struct crypto_blkcipher *tfm,
>>> + void *iv, void *in, void *out,
>>> + int length); /* decryption
>>> function */
>>> +};
>>> +
>>> struct krb5_ctx {
>>> int initiate; /* 1 = initiating, 0 =
>>> accepting */
>>> u32 enctype;
>>> + struct gss_krb5_enctype *gk5e; /* enctype-specific
>>> info */
>>> struct crypto_blkcipher *enc;
>>> struct crypto_blkcipher *seq;
>>> s32 endtime;
>>> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/
>>> auth_gss/gss_krb5_crypto.c
>>> index 1d52308..39643a7 100644
>>> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
>>> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
>>> @@ -59,13 +59,13 @@ krb5_encrypt(
>>> {
>>> u32 ret = -EINVAL;
>>> struct scatterlist sg[1];
>>> - u8 local_iv[16] = {0};
>>> + u8 local_iv[GSS_KRB5_MAX_BLOCKSIZE] = {0};
>>> struct blkcipher_desc desc = { .tfm = tfm, .info = local_iv };
>>>
>>> if (length % crypto_blkcipher_blocksize(tfm) != 0)
>>> goto out;
>>>
>>> - if (crypto_blkcipher_ivsize(tfm) > 16) {
>>> + if (crypto_blkcipher_ivsize(tfm) > GSS_KRB5_MAX_BLOCKSIZE) {
>>> dprintk("RPC: gss_k5encrypt: tfm iv size too
>>> large %d\n",
>>> crypto_blkcipher_ivsize(tfm));
>>> goto out;
>>> @@ -95,13 +95,13 @@ krb5_decrypt(
>>> {
>>> u32 ret = -EINVAL;
>>> struct scatterlist sg[1];
>>> - u8 local_iv[16] = {0};
>>> + u8 local_iv[GSS_KRB5_MAX_BLOCKSIZE] = {0};
>>> struct blkcipher_desc desc = { .tfm = tfm, .info = local_iv };
>>>
>>> if (length % crypto_blkcipher_blocksize(tfm) != 0)
>>> goto out;
>>>
>>> - if (crypto_blkcipher_ivsize(tfm) > 16) {
>>> + if (crypto_blkcipher_ivsize(tfm) > GSS_KRB5_MAX_BLOCKSIZE) {
>>> dprintk("RPC: gss_k5decrypt: tfm iv size too
>>> large %d\n",
>>> crypto_blkcipher_ivsize(tfm));
>>> goto out;
>>> @@ -164,7 +164,7 @@ out:
>>> EXPORT_SYMBOL(make_checksum);
>>>
>>> struct encryptor_desc {
>>> - u8 iv[8]; /* XXX hard-coded blocksize */
>>> + u8 iv[GSS_KRB5_MAX_BLOCKSIZE];
>>> struct blkcipher_desc desc;
>>> int pos;
>>> struct xdr_buf *outbuf;
>>> @@ -205,7 +205,7 @@ encryptor(struct scatterlist *sg, void *data)
>>> desc->fraglen += sg->length;
>>> desc->pos += sg->length;
>>>
>>> - fraglen = thislen & 7; /* XXX hardcoded blocksize */
>>> + fraglen = thislen & (crypto_blkcipher_blocksize(desc-
>>> >desc.tfm) -
>>> 1);
>>> thislen -= fraglen;
>>>
>>> if (thislen == 0)
>>> @@ -265,7 +265,7 @@ gss_encrypt_xdr_buf(struct crypto_blkcipher
>>> *tfm, struct xdr_buf *buf,
>>> EXPORT_SYMBOL(gss_encrypt_xdr_buf);
>>>
>>> struct decryptor_desc {
>>> - u8 iv[8]; /* XXX hard-coded blocksize */
>>> + u8 iv[GSS_KRB5_MAX_BLOCKSIZE];
>>> struct blkcipher_desc desc;
>>> struct scatterlist frags[4];
>>> int fragno;
>>> @@ -287,7 +287,7 @@ decryptor(struct scatterlist *sg, void *data)
>>> desc->fragno++;
>>> desc->fraglen += sg->length;
>>>
>>> - fraglen = thislen & 7; /* XXX hardcoded blocksize */
>>> + fraglen = thislen & (crypto_blkcipher_blocksize(desc-
>>> >desc.tfm) -
>>> 1);
>>> thislen -= fraglen;
>>>
>>> if (thislen == 0)
>>> diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/
>>> auth_gss/gss_krb5_mech.c
>>> index f0f0749..fe88266 100644
>>> --- a/net/sunrpc/auth_gss/gss_krb5_mech.c
>>> +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
>>> @@ -48,6 +48,50 @@
>>> # define RPCDBG_FACILITY RPCDBG_AUTH
>>> #endif
>>>
>>> +static struct gss_krb5_enctype supported_gss_krb5_enctypes[] = {
>>> + /*
>>> + * DES (All DES enctypes are mapped to the same gss
>>> functionality)
>>> + */
>>> + {
>>> + .etype = ENCTYPE_DES_CBC_RAW,
>>> + .ctype = CKSUMTYPE_RSA_MD5,
>>> + .name = "des-cbc-crc",
>>> + .encrypt_name = "cbc(des)",
>>> + .cksum_name = "md5",
>>> + .encrypt = krb5_encrypt,
>>> + .decrypt = krb5_decrypt,
>>> + .signalg = SGN_ALG_DES_MAC_MD5,
>>> + .sealalg = SEAL_ALG_DES,
>>> + .keybytes = 7,
>>> + .keylength = 8,
>>> + .blocksize = 8,
>>> + .cksumlength = 8,
>>> + },
>>> +};
>>> +
>>> +static int num_supported_enctypes =
>>> + sizeof(supported_gss_krb5_enctypes) / sizeof(struct
>>> gss_krb5_enctype);
>>
>> You could use ARRAY_SIZE() instead of open-coding this calculation.
>
> I'll look at ARRAY_SIZE().
>
>> Not having looked at the subsequent patches, why couldn't
>> num_supported_enctypes be a constant instead of a static variable?
>
> Sure, it just requires updating every time a new enctype is added.
> One more thing to remember.
If you use ARRAY_SIZE(), adding a new enctype entry in the array
makes this automatic.
[ ... snipped ... ]
>>> @@ -234,29 +238,34 @@ gss_unwrap_kerberos_v1(struct krb5_ctx *kctx,
>>> int offset, struct xdr_buf *buf)
>>> /* get the sign and seal algorithms */
>>>
>>> signalg = ptr[0] + (ptr[1] << 8);
>>> - if (signalg != SGN_ALG_DES_MAC_MD5)
>>> + if (signalg != kctx->gk5e->signalg)
>>> return GSS_S_DEFECTIVE_TOKEN;
>>>
>>> sealalg = ptr[2] + (ptr[3] << 8);
>>> - if (sealalg != SEAL_ALG_DES)
>>> + if (sealalg != kctx->gk5e->sealalg)
>>> return GSS_S_DEFECTIVE_TOKEN;
>>>
>>> if ((ptr[4] != 0xff) || (ptr[5] != 0xff))
>>> return GSS_S_DEFECTIVE_TOKEN;
>>>
>>> - if (gss_decrypt_xdr_buf(kctx->enc, buf,
>>> - ptr + 22 - (unsigned char *)buf->head
>>> [0].iov_base))
>>> + /*
>>> + * Data starts after token header and checksum. ptr points
>>> + * two past the beginning of the token header (which is 16
>>> bytes)
>>> + */
>>> + crypt_offset = ptr + (16 - 2 + kctx->gk5e->cksumlength) -
>>> + (unsigned char *)buf->head
>>> [0].iov_base;
>>> + if (gss_decrypt_xdr_buf(kctx->enc, buf, crypt_offset))
>>> return GSS_S_DEFECTIVE_TOKEN;
>>
>> I'm not comfortable with the number of places in all these patches
>> where naked integers are used instead of a macro constant.
>>
>> I prefer the use of macro constants that spell out the purpose of a
>> constant value over adding an explanatory comment, since the macro
>> constants provide an opportunity for the code itself to explain
>> exactly what is going on. The explanation will then never be out of
>> date with the code!
>
> I agree in principle. However, does "ptr + (GSS_TOKEN_V1_HEADER_SIZE
> - TWO_BYTES_WE_ALREADY_SKIPPED + kctx->gk5e->cksumlength)" really
> help?
You seem to use "ptr - 2" in several other places. Why not create
another variable that always points right to the beginning of the
token header?
> btw, I agree a define for GSS_TOKEN_V1_HEADER_SIZE might make
> sense. Bruce?
>
>>> - if (make_checksum("md5", ptr - 2, 8, buf,
>>> - ptr + 22 - (unsigned char *)buf->head[0].iov_base,
>>> &md5cksum))
>>> + if (make_checksum((char *)kctx->gk5e->cksum_name, ptr - 2,
>>> 8, buf,
>>> + crypt_offset,
>>> &md5cksum))
>>> return GSS_S_FAILURE;
>>>
>>> if (krb5_encrypt(kctx->seq, NULL, md5cksum.data,
>>> md5cksum.data, md5cksum.len))
>>> return GSS_S_FAILURE;
>>>
>>> - if (memcmp(md5cksum.data + 8, ptr + 14, 8))
>>> + if (memcmp(md5cksum.data + 8, ptr + 14, 8))/* this is still
>>> DES-
>>> aware */
Sometimes you use "14" and sometimes "16 - 2".
>>> return GSS_S_BAD_SIG;
>>>
>>> /* it got through unscathed. Make sure the context is
>>> unexpired */
>>> @@ -280,7 +289,7 @@ gss_unwrap_kerberos_v1(struct krb5_ctx *kctx,
>>> int offset, struct xdr_buf *buf)
>>> * better to copy and encrypt at the same time. */
>>>
>>> blocksize = crypto_blkcipher_blocksize(kctx->enc);
>>> - data_start = ptr + 22 + blocksize;
>>> + data_start = ptr + (16 - 2 + kctx->gk5e->cksumlength) +
>>> blocksize;
>>> orig_start = buf->head[0].iov_base + offset;
>>> data_len = (buf->head[0].iov_base + buf->head[0].iov_len) -
>>> data_start;
>>> memmove(orig_start, data_start, data_len);
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
More information about the NFSv4
mailing list