[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

[Q (Igor Mammedov)]

Jeff Layton wrote:
  > 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.

Simple pre parsing not only helps to decide whether do upcall or not, 
but can help to
determine if negotiation has been accepted by server or we need continue 
negotiation.

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

Upstream has more correct for doing this. Besides CIFSSEC_MUST_KRB5 
isn't used in code at all. Looks like I was the first who tried to use it.

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

Yes, It's not necessary code now and with experimEnabled < 2 it will 
work just fine. But finely we could end up with separate session setup 
function for kerberos if we will consider implementation of multi-stage 
negotiation.

-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com