[linux-cifs-client] Kerberos5 support in cifs pathset [PATCH: 2/4] enables extended security in NEG... and SESSION_SETUP... requests when mounting with krb5i option

[Jeff Layton]

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


> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/asn1.c linux-2.6.22.9/fs/cifs.krb5/asn1.c
> --- linux-2.6.22.9/fs/cifs/asn1.c	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/asn1.c	2007-10-18 13:37:36.000000000 +0400
> @@ -77,8 +77,12 @@
>  
>  #define SPNEGO_OID_LEN 7
>  #define NTLMSSP_OID_LEN  10
> +#define KRB5_OID_LEN  7
> +#define MSKRB5_OID_LEN  7
>  static unsigned long SPNEGO_OID[7] = { 1, 3, 6, 1, 5, 5, 2 };
>  static unsigned long NTLMSSP_OID[10] = { 1, 3, 6, 1, 4, 1, 311, 2, 2, 10 };
> +static unsigned long KRB5_OID[10] = { 1, 2, 840, 113554, 1, 2, 2 };
> +static unsigned long MSKRB5_OID[10] = { 1, 2, 840, 48018, 1, 2, 2 };
>  
>  /* 
>   * ASN.1 context.
> @@ -458,6 +462,7 @@
>  	unsigned long *oid = NULL;
>  	unsigned int cls, con, tag, oidlen, rc;
>  	int use_ntlmssp = FALSE;
> +	int use_kerberos = FALSE;
>  
>  	*secType = NTLM; /* BB eventually make Kerberos or NLTMSSP the default */
>  
> @@ -552,9 +557,20 @@
>  					   *(oid + 3)));
>  					rc = compare_oid(oid, oidlen, NTLMSSP_OID,
>  						 NTLMSSP_OID_LEN);
> -					kfree(oid);
>  					if (rc)
>  						use_ntlmssp = TRUE;
> +					rc = compare_oid(oid, oidlen, MSKRB5_OID,
> +						 MSKRB5_OID_LEN);
> +					if (rc)
> +						use_kerberos = TRUE;
> +					rc = compare_oid(oid, oidlen, KRB5_OID,
> +						 KRB5_OID_LEN);
> +					if (rc)
> +						use_kerberos = TRUE;
> +
> +
> +
> +					kfree(oid);
>  				}
>  			} else {
>  				cFYI(1,("This should be an oid what is going on? "));
> @@ -606,10 +622,9 @@
>  		cFYI(1, ("Need to call asn1_octets_decode() function for this %s", ctx.pointer));	/* is this UTF-8 or ASCII? */
>  	}
>  
> -	/* if (use_kerberos) 
> -	   *secType = Kerberos 
> -	   else */
> -	if (use_ntlmssp) {
> +	if (use_kerberos) {
> +	   *secType = Kerberos;
> +	} else if (use_ntlmssp) {
>  		*secType = NTLMSSP;
>  	}
>  
> 

Our current thinking is that we should offload all of the ASN1 parsing to
userspace. There's no way to reasonably handle kerberos tickets directly
from kernel space. Since we're going to have to upcall anyway, we might
as well just ship everything there.

Still, there might be a place to do some simple parsing of ASN1 blobs. It
might help avoid upcalls when we're just looking for something trivial, or
for NTLMSSP.

Steve, thoughts?

> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/cifssmb.c linux-2.6.22.9/fs/cifs.krb5/cifssmb.c
> --- linux-2.6.22.9/fs/cifs/cifssmb.c	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/cifssmb.c	2007-10-18 13:37:11.000000000 +0400
> @@ -434,7 +434,7 @@
>  
>  	pSMB->hdr.Mid = GetNextMid(server);
>  	pSMB->hdr.Flags2 |= (SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS);
> -	if ((secFlags & CIFSSEC_MUST_KRB5) == CIFSSEC_MUST_KRB5)
> +	if ((secFlags & CIFSSEC_MAY_KRB5) == CIFSSEC_MAY_KRB5)
>  		pSMB->hdr.Flags2 |= SMBFLG2_EXT_SEC;
>  	

Not sure that that is right...MAY_KRB5 can be set in conjunction with
other MAY flags.

>  	count = 0;
> @@ -569,7 +569,9 @@
>  		server->secType = NTLM;
>  	else if(secFlags & CIFSSEC_MAY_NTLMV2)
>  		server->secType = NTLMv2;
> -	/* else krb5 ... any others ... */
> +	else if(secFlags & CIFSSEC_MAY_KRB5)
> +		server->secType = Kerberos;
> +	/* else any others ... */
>  

Already upstream...

>  	/* one byte, so no need to convert this or EncryptionKeyLen from
>  	   little endian */
> @@ -621,8 +623,7 @@
>  						 count - 16,
>  						 &server->secType);
>  			if(rc == 1) {
> -			/* BB Need to fill struct for sessetup here */
> -				rc = -EOPNOTSUPP;
> +				rc = 0;
>  			} else {
>  				rc = -EINVAL;
>  			}
> 

Looks sane (the patch I have so far does something similar).

> diff -b -B -N -u -r linux-2.6.22.9/fs/cifs/connect.c linux-2.6.22.9/fs/cifs.krb5/connect.c
> --- linux-2.6.22.9/fs/cifs/connect.c	2007-09-26 22:03:01.000000000 +0400
> +++ linux-2.6.22.9/fs/cifs.krb5/connect.c	2007-10-22 19:49:48.000000000 +0400
> @@ -3227,6 +3227,10 @@
>  		}
>  	}
>  
> +	if( ses->server->secType == Kerberos ) {
> +		smb_buffer->Flags2 |= SMBFLG2_EXT_SEC;
> +	}
> +
>  	if(ses->server->secMode & 
>  			(SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
>  		smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
> @@ -3412,6 +3416,12 @@
>  		else if (extended_security
>  				&& (pSesInfo->capabilities 
>  					& CAP_EXTENDED_SECURITY)
> +				&& (pSesInfo->server->secType == Kerberos)) {
> +			rc = CIFS_SessSetup(xid, pSesInfo,
> +					    first_time, nls_info);
> +		} else if (extended_security
> +				&& (pSesInfo->capabilities 
> +					& CAP_EXTENDED_SECURITY)
>  				&& (pSesInfo->server->secType == NTLMSSP)) {
>  			rc = -EOPNOTSUPP;
>  		} else if (extended_security
> 
> 


Not a comment on your patch, but we seem to have a lot of redundant code
in this file. When experimEnabled < 2, we only call CIFS_SessSetup, so
the above hunk will only come into effect if someone resets
/proc/fs/cifs/Experimental . It's not clear to me whether some of this
code is an effort to break up CIFS_SessSetup, or vestigial code that
should probably be removed...

-- 
Jeff Layton <jlayton at redhat.com>