[linux-cifs-client] Kerberos5 support in cifs pathset [PATCH: 3/4] upcall handling via KEYS API for getting security blob and session key

Jeff Layton jlayton at redhat.com
Tue Oct 23 18:02:48 GMT 2007


On Mon, 22 Oct 2007 21:09:49 +0400
"Q (Igor Mammedov)" <qwerty0987654321 at mail.ru> wrote:

> 
> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/cifsfs.c linux-2.6.22.9/fs/cifs.krb5/cifsfs.c
> --- linux-2.6.22.9/fs/cifs/cifsfs.c	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/cifsfs.c	2007-10-22 18:45:23.000000000 +0400
> @@ -940,6 +940,11 @@
>  init_cifs(void)
>  {
>  	int rc = 0;
> +
> +#ifdef CONFIG_CIFS_EXPERIMENTAL
> +	register_key_type(&key_type_cifs_spnego);
> +#endif
> +
>  #ifdef CONFIG_PROC_FS
>  	cifs_proc_init();
>  #endif
> @@ -1036,6 +1041,11 @@
>  exit_cifs(void)
>  {
>  	cFYI(0, ("In unregister ie exit_cifs"));
> +
> +#ifdef CONFIG_CIFS_EXPERIMENTAL
> +	unregister_key_type(&key_type_cifs_spnego);
> +#endif
> +
>  #ifdef CONFIG_PROC_FS
>  	cifs_proc_clean();
>  #endif
> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/cifsglob.h linux-2.6.22.9/fs/cifs.krb5/cifsglob.h
> --- linux-2.6.22.9/fs/cifs/cifsglob.h	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/cifsglob.h	2007-10-22 18:33:25.000000000 +0400
> @@ -19,6 +20,12 @@
>  #include <linux/in.h>
>  #include <linux/in6.h>
>  #include "cifs_fs_sb.h"
> +
> +extern struct key_type key_type_cifs_spnego;
> +int cifs_spnego_get_session_keys(const char* princ, char** secglob, 
> +		int* secglob_len, char** skey, int* skey_len);
> +
> +
>  /*
>   * The sizes of various internal tables and strings
>   */
> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/cifs_spnego_upcall.c linux-2.6.22.9/fs/cifs.krb5/cifs_spnego_upcall.c
> --- linux-2.6.22.9/fs/cifs/cifs_spnego_upcall.c	1970-01-01 03:00:00.000000000 +0300
> +++ linux-2.6.22.9/fs/cifs.krb5/cifs_spnego_upcall.c	2007-10-22 19:49:20.000000000 +0400
> @@ -0,0 +1,112 @@
> +#include <linux/key.h>
> +#include "cifsglob.h"
> +#include "cifs_debug.h"
> +
> +
> +#ifdef CONFIG_CIFS_EXPERIMENTAL
> +
> +static void cifs_spnego_describe(const struct key *key, struct seq_file *m)
> +{
> +	seq_puts(m, key->description);
> +	return;
> +}
> +
> +static int cifs_spnego_match(const struct key *key, const void *description)
> +{
> +	return strcmp(key->description, description) == 0;
> +}
> +
> +static int cifs_spnego_instantiate(struct key *key, const void *data, size_t datalen)
> +{
> +	void* payload=NULL;
> +	int rc;
> +
> +	rc = key_payload_reserve(key,datalen);
> +	if( rc == 0 ){
> +		payload = kmalloc(datalen, GFP_KERNEL);
> +		if ( payload ) {
> +			memcpy(payload,data,datalen);
> +			key->payload.data = payload;
> +		} else {
> +			rc = -ENOMEM;
> +		}
> +	}  
> +	return rc;
> +}
> +
> +void cifs_spnego_destroy(struct key *key)
> +{
> +	if ( key->datalen && key->payload.data ){
> +		cFYI(1, ("%s: payload freed", __FUNCTION__ ));
> +		kfree( key->payload.data );
> +		key->payload.data = NULL;
> +		key->datalen = 0;
> +	}
> +
> +	return;
> +}
> +
> +struct key_type key_type_cifs_spnego =
> +{
> +	.name        = "cifs.spnego",
> +	.def_datalen = 0,
> +	.describe    = cifs_spnego_describe,
> +	.instantiate = cifs_spnego_instantiate,
> +	.match       = cifs_spnego_match,
> +	.destroy     = cifs_spnego_destroy,
> +};
> +
> +typedef struct _cifs_spnego_keys {
> +	int     secglob_length;
> +	int     sesskey_length;
> +	char    data;
> +} cifs_spnego_keys_t;
> +

Pretty close to what I have for this.

> +int
> +cifs_spnego_get_session_keys(const char* princ, char** secglob, int* secglob_len, char** skey, int* skey_len)
> +{
> +	int rc = -EAGAIN;
> +	struct key * rkey;
> +	cifs_spnego_keys_t* k;
> +
> +	if ( (!princ) || (!secglob) || (!secglob_len) ) {
> +		return -EINVAL;
> +	}
> +
> +	rkey = request_key(&key_type_cifs_spnego, princ, "");
> +	if ( !IS_ERR(rkey) ) {
> +		k = rkey->payload.data;
> +		*secglob = kmalloc(k->secglob_length, GFP_KERNEL);
> +		if ( !(*secglob) ) {
> +			cERROR(1, ("%s: failed to allocate space for SecurityBlob", 
> +						__FUNCTION__ ));
> +			return -EINVAL;
> +		}
> +
> +		*skey = kmalloc(k->sesskey_length, GFP_KERNEL);
> +		if ( !(*skey) ) {
> +			cERROR(1, ("%s: failed to allocate space for SessionKey", 
> +						__FUNCTION__ ));
> +			kfree(*secglob);
> +			*secglob = NULL;
> +			return -EINVAL;
> +		}
> +
> +		memcpy(*secglob,&(k->data),k->secglob_length);
> +		*secglob_len = k->secglob_length;
> +		memcpy( *skey, &(k->data)+k->secglob_length, k->sesskey_length);
> +		*skey_len = k->sesskey_length;
> +

One thing that's been pointed out to me -- MS' KDC adds authorization
info to the ticket. These blobs can be big (in excess of 60k). We need
to minimize copying them as much as possible since that means we'll
need large allocations.

I took the approach of passing around a key pointer to avoid some of
the copying, but I do the upcall in CIFSSMBNegotiate instead of during
CIFS_SessSetup.

> +		// release used key
> +		key_put(rkey);
> +		rc = 0;
> +	} else {
> +		cERROR(1, ("%s: unable get key (%ld) for: %s", __FUNCTION__, 
> +					PTR_ERR(rkey), princ));
> +	}
> +
> +	return rc;
> +}
> +
> +
> +#endif
> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/Makefile linux-2.6.22.9/fs/cifs.krb5/Makefile
> --- linux-2.6.22.9/fs/cifs/Makefile	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/Makefile	2007-10-22 19:17:57.000000000 +0400
> @@ -3,4 +3,4 @@
>  #
>  obj-$(CONFIG_CIFS) += cifs.o
>  
> -cifs-objs := cifsfs.o cifssmb.o cifs_debug.o connect.o dir.o file.o inode.o link.o misc.o netmisc.o smbdes.o smbencrypt.o transport.o asn1.o md4.o md5.o cifs_unicode.o nterr.o xattr.o cifsencrypt.o fcntl.o readdir.o ioctl.o sess.o export.o
> +cifs-objs := cifsfs.o cifssmb.o cifs_debug.o connect.o dir.o file.o inode.o link.o misc.o netmisc.o smbdes.o smbencrypt.o transport.o asn1.o md4.o md5.o cifs_unicode.o nterr.o xattr.o cifsencrypt.o fcntl.o readdir.o ioctl.o sess.o export.o cifs_spnego_upcall.o
> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/sess.c linux-2.6.22.9/fs/cifs.krb5/sess.c
> --- linux-2.6.22.9/fs/cifs/sess.c	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/sess.c	2007-10-22 19:28:19.000000000 +0400
> @@ -475,12 +475,65 @@
>  			unicode_ssetup_strings(&bcc_ptr, ses, nls_cp);
>  		} else
>  			ascii_ssetup_strings(&bcc_ptr, ses, nls_cp);
> -	} else /* NTLMSSP or SPNEGO */ {
> +	} else if( type ==  Kerberos ) /* SPNEGO Kerberos */ {
> +		char* secglob = NULL;
> +		int   secglob_len = 0;
> +		char* skey = NULL;
> +		int   skey_len = 0;
> +
>  		pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
>  		capabilities |= CAP_EXTENDED_SECURITY;
>  		pSMB->req.Capabilities = cpu_to_le32(capabilities);
> -		/* BB set password lengths */
> +
> +		rc = cifs_spnego_get_session_keys(ses->serverName, 
> +				&secglob, &secglob_len, &skey, &skey_len);
> +		if ( !rc ) {
> +			ses->server->mac_signing_key_len = skey_len;
> +			if( skey_len && skey ) {
> +				/* BB: could session key be more than 
> +				 * mac_signing_key length ? */
> +				memcpy(ses->server->mac_signing_key,
> +						skey,skey_len);
> +				kfree(skey);
> +				skey = NULL;
> +			}
> +
> +			if( secglob ) {
> +				/* BB: probably we need check 
> +				 * for NULL here */
> +				str_area = kmalloc(secglob_len+300, GFP_KERNEL);
> +				bcc_ptr = str_area;
> +				memset(str_area,0,secglob_len+300);

Should probably be a kzalloc instead of kmalloc+memset.

> +				memcpy(bcc_ptr,secglob,secglob_len);
> +				bcc_ptr += secglob_len;
> +				pSMB->req.SecurityBlobLength = cpu_to_le16( secglob_len );
> +				kfree(secglob);
> +				secglob = NULL;
> +			}
> +

Fairly close to what I already have. I decided to take Steve's advice and
just made the secblob a new iovec to pass to SendReceive2. That cuts down
another secblob copy.

> +			if(ses->capabilities & CAP_UNICODE ) {
> +				if ((long) bcc_ptr % 2) {
> +					*bcc_ptr = 0;
> +					bcc_ptr++;
>  	}
> +				bcc_ptr += 2 * cifs_strtoUCS((__le16 *) bcc_ptr,
> +						"Linux version ", 32, nls_cp);
> +				bcc_ptr += 2 * cifs_strtoUCS((__le16 *) bcc_ptr,
> +						utsname()->release, 32, nls_cp);
> +				bcc_ptr += 2;
> +				bcc_ptr += 2 * cifs_strtoUCS((__le16
> *) bcc_ptr,
> +						CIFS_NETWORK_OPSYS,
> 64, nls_cp);
> +				bcc_ptr += 1;

Steve committed a patch recently to break up unicode_ssetup_strings so
that we could call the functions individually, which I think will make
this cleaner.

> +			} else
> +				ascii_ssetup_strings(&bcc_ptr, ses,
> nls_cp);

Interesting. Is this correct? If unicode isn't enabled can we just call
ascii_ssetup_strings here? I've actually not seen any non-unicode
kerberos setups in the wild. Is it even possible?

> +		} else {
> +			rc = -EACCES;
> +		}
> +	}
> +
> +	/*BB:  shouldn't we check if we could procced further? */
> +	/* if(rc)
> +		goto ssetup_exit; */
>  
>  	count = (long) bcc_ptr - (long) str_area;
>  	smb_buf->smb_buf_length += count;

Nice work, Igor. It's definitely given me a good overview of some of
the gaps that I've got in my implementation. I'll leave the userspace
piece for Simo Sorce to review. He volunteered to do it for the
implementation we were working on. I'm thinking that adding it to the
same dir mount.cifs in the samba tree is probably the right thing to do.
It looks like we have some of this code in libsmbclient already.

Thanks again for sending this to the list!

-- 
Jeff Layton <jlayton at redhat.com>


More information about the linux-cifs-client mailing list