[linux-cifs-client] [PATCH 4/5] [CIFS] have CIFSSMBNegotiate handle SPNEGO via upcall

[Jeff Layton]

On Thu, 25 Oct 2007 13:43:47 -0400
Jeff Layton <jlayton at redhat.com> wrote:

> Change args for CIFSSMBNegotiate to take key pointer and a hostname
> string. Have it call cifs_get_spnego_key when the NegotiateProtocol
> response contains a SPNEGO blob.
> 
> Signed-off-by: Jeff Layton <jlayton at redhat.com>
> ---
>  fs/cifs/cifsproto.h |    3 ++-
>  fs/cifs/cifssmb.c   |   41 ++++++++++++++++++++++++++++++-----------
>  fs/cifs/connect.c   |    7 +++++--
>  3 files changed, 37 insertions(+), 14 deletions(-)
>

I just realized that this patch has a significant (and rather
boneheaded) bug. If we call cifs_setup_session on a reconnect, then
we'll end up calling CIFS_SessSetup with a NULL spnego_key. I'm still
looking at the right way to fix it, but anyone testing this should be
aware of this. I'll repost once I have a solution and have tested it.

> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 0f8a99d..41c29c5 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -104,7 +104,8 @@ void cifs_proc_clean(void);
>  
>  extern int cifs_setup_session(unsigned int xid, struct cifsSesInfo
> *pSesInfo, struct nls_table *nls_info, const char *unc);
> -extern int CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo
> *ses); +extern int CIFSSMBNegotiate(unsigned int xid, struct
> cifsSesInfo *ses,
> +			struct key **spnego_key, const char
> *hostname); 
>  extern int CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
>  			const char *tree, struct cifsTconInfo *tcon,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 31686e9..68ac988 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -38,6 +38,7 @@
>  #include "cifsproto.h"
>  #include "cifs_unicode.h"
>  #include "cifs_debug.h"
> +#include "cifs_spnego.h"
>  
>  #ifdef CONFIG_CIFS_POSIX
>  static struct {
> @@ -407,7 +408,8 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
>  	return rc;
>  }
>  int
> -CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)
> +CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses,
> +		 struct key **spnego_key, const char *hostname)
>  {
>  	NEGOTIATE_REQ *pSMB;
>  	NEGOTIATE_RSP *pSMBr;
> @@ -641,19 +643,36 @@ CIFSSMBNegotiate(unsigned int xid, struct
> cifsSesInfo *ses) memcpy(server->server_GUID,
>  			       pSMBr->u.extended_response.GUID, 16);
>  
> -		if (count == 16) {
> +		if (count == 16)
>  			server->secType = RawNTLMSSP;
> -		} else {
> -			rc =
> decode_negTokenInit(pSMBr->u.extended_response.
> -						 SecurityBlob,
> -						 count - 16,
> -						 &server->secType);
> -			if (rc == 1) {
> -			/* BB Need to fill struct for sessetup here
> */
> -				rc = -EOPNOTSUPP;
> -			} else {
> +		else {
> +#ifdef CONFIG_CIFS_UPCALL
> +			struct cifs_spnego_msg *msg;
> +			*spnego_key = cifs_get_spnego_key(ses,
> pSMBr->u.
> +
> extended_response.SecurityBlob,
> +						count - 16,
> hostname);
> +			if (IS_ERR(*spnego_key)) {
> +				rc = PTR_ERR(*spnego_key);
> +				*spnego_key = NULL;
> +				goto neg_err_exit;
> +			}
> +
> +			msg = (struct cifs_spnego_msg *)
> +
> (*spnego_key)->payload.data;
> +			if (msg->flags & CIFS_SPNEGO_KRB5)
> +				server->secType = Kerberos;
> +			else if (msg->flags & CIFS_SPNEGO_NTLMSSP)
> +				server->secType = NTLMSSP;
> +			else {
>  				rc = -EINVAL;
> +				goto neg_err_exit;
>  			}
> +#else /* CONFIG_CIFS_UPCALL */
> +			cERROR(1, ("SPNEGO response received, but "
> +				   "CONFIG_CIFS_UPCALL not
> enabled!"));
> +			rc = -EINVAL;
> +			goto neg_err_exit;
> +#endif /* CONFIG_CIFS_UPCALL */
>  		}
>  	} else
>  		server->capabilities &= ~CAP_EXTENDED_SECURITY;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 057d55c..37e9474 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3546,6 +3546,7 @@ int cifs_setup_session(unsigned int xid, struct
> cifsSesInfo *pSesInfo, char ntlm_session_key[CIFS_SESS_KEY_SIZE];
>  	int ntlmv2_flag = FALSE;
>  	int first_time = 0;
> +	struct key *spnego_key = NULL;
>  	char *hostname;
>  
>  	hostname = extract_hostname_from_unc(unc);
> @@ -3557,9 +3558,9 @@ int cifs_setup_session(unsigned int xid, struct
> cifsSesInfo *pSesInfo, 
>  	/* what if server changes its buffer size after dropping the
> session? */ if (pSesInfo->server->maxBuf == 0) /* no need to send on
> reconnect */ {
> -		rc = CIFSSMBNegotiate(xid, pSesInfo);
> +		rc = CIFSSMBNegotiate(xid, pSesInfo, &spnego_key,
> hostname); if (rc == -EAGAIN) /* retry only once on 1st time
> connection */ {
> -			rc = CIFSSMBNegotiate(xid, pSesInfo);
> +			rc = CIFSSMBNegotiate(xid, pSesInfo,
> &spnego_key, hostname); if (rc == -EAGAIN)
>  				rc = -EHOSTDOWN;
>  		}
> @@ -3666,6 +3667,8 @@ int cifs_setup_session(unsigned int xid, struct
> cifsSesInfo *pSesInfo, }
>  	}
>  ss_err_exit:
> +	if (spnego_key)
> +		key_put(spnego_key);
>  	if (hostname)
>  		kfree(hostname);
>  

-- 
Jeff Layton <jlayton at poochiereds.net>