Fwd: [PATCH] Fix default behaviour for empty domains and add domainauto option
Steve French
smfrench at gmail.com
Thu Dec 15 06:23:13 UTC 2016
Any last minute thoughts on this since it points to differences with
smbclient behavior as well (not just cifs.ko)
---------- Forwarded message ----------
From: Pavel Shilovsky <piastryyy at gmail.com>
Date: Wed, Dec 14, 2016 at 2:10 PM
Subject: Re: [PATCH] Fix default behaviour for empty domains and add
domainauto option
To: Germano Percossi <germano.percossi at citrix.com>
Cc: Steve French <sfrench at samba.org>, linux-cifs
<linux-cifs at vger.kernel.org>, Jeff Layton <jlayton at samba.org>, Shirish
Pargaonkar <shirishpargaonkar at gmail.com>
2016-11-24 13:20 GMT-08:00 Germano Percossi <germano.percossi at citrix.com>:
> With commit 2b149f119 many things have been fixed/introduced.
> However, the default behaviour for RawNTLMSSP authentication
> seems to be wrong in case the domain is not passed on the command line.
>
> The main points (see below) of the patch are:
> - It alignes behaviour with Windows clients
> - It fixes backward compatibility
> - It fixes UPN
>
> I compared this behavour with the one from a Windows 10 command line
> client. When no domains are specified on the command line, I traced
> the packets and observed that the client does send an empty
> domain to the server.
> In the linux kernel case, the empty domain is replaced by the
> primary domain communicated by the SMB server.
> This means that, if the credentials are valid against the local server
> but that server is part of a domain, then the kernel module will
> ask to authenticate against that domain and we will get LOGON failure.
>
> I compared the packet trace from the smbclient when no domain is passed
> and, in that case, a default domain from the client smb.conf is taken.
> Apparently, connection succeeds anyway, because when the domain passed
> is not valid (in my case WORKGROUP), then the local one is tried and
> authentication succeeds. I tried with any kind of invalid domain and
> the result was always a connection.
>
> So, trying to interpret what to do and picking a valid domain if none
> is passed, seems the wrong thing to do.
> To this end, a new option "domainauto" has been added in case the
> user wants a mechanism for guessing.
>
> Without this patch, backward compatibility also is broken.
> With kernel 3.10, the default auth mechanism was NTLM.
> One of our testing servers accepted NTLM and, because no
> domains are passed, authentication was local.
>
> Moving to RawNTLMSSP forced us to change our command line
> to add a fake domain to pass to prevent this mechanism to kick in.
>
> For the same reasons, UPN is broken because the domain is specified
> in the username.
> The SMB server will work out the domain from the UPN and authenticate
> against the right server.
> Without the patch, though, given the domain is empty, it gets replaced
> with another domain that could be the wrong one for the authentication.
>
> Signed-off-by: Germano Percossi <germano.percossi at citrix.com>
> ---
> fs/cifs/cifsencrypt.c | 14 +++++++++-----
> fs/cifs/cifsglob.h | 2 ++
> fs/cifs/connect.c | 7 +++++++
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 5eb0412..66bd7fa 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -699,11 +699,15 @@ static int crypto_hmacmd5_alloc(struct TCP_Server_Info *server)
>
> if (ses->server->negflavor == CIFS_NEGFLAVOR_EXTENDED) {
> if (!ses->domainName) {
> - rc = find_domain_name(ses, nls_cp);
> - if (rc) {
> - cifs_dbg(VFS, "error %d finding domain name\n",
> - rc);
> - goto setup_ntlmv2_rsp_ret;
> + if (ses->domainAuto) {
> + rc = find_domain_name(ses, nls_cp);
> + if (rc) {
> + cifs_dbg(VFS, "error %d finding domain name\n",
> + rc);
> + goto setup_ntlmv2_rsp_ret;
> + }
> + } else {
> + ses->domainName = kstrdup("", GFP_KERNEL);
> }
> }
> } else {
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 1f17f6b..4f0d5a1 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -514,6 +514,7 @@ struct smb_vol {
> bool persistent:1;
> bool nopersistent:1;
> bool resilient:1; /* noresilient not required since not fored for CA */
> + bool domainauto:1;
> unsigned int rsize;
> unsigned int wsize;
> bool sockopt_tcp_nodelay:1;
> @@ -827,6 +828,7 @@ struct cifs_ses {
> enum securityEnum sectype; /* what security flavor was specified? */
> bool sign; /* is signing required? */
> bool need_reconnect:1; /* connection reset, uid now invalid */
> + bool domainAuto:1;
> #ifdef CONFIG_CIFS_SMB2
> __u16 session_flags;
> __u8 smb3signingkey[SMB3_SIGN_KEY_SIZE];
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 4547aed..df7eed7 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -88,6 +88,7 @@ enum {
> Opt_multiuser, Opt_sloppy, Opt_nosharesock,
> Opt_persistent, Opt_nopersistent,
> Opt_resilient, Opt_noresilient,
> + Opt_domainauto,
>
> /* Mount options which take numeric value */
> Opt_backupuid, Opt_backupgid, Opt_uid,
> @@ -176,6 +177,7 @@ enum {
> { Opt_nopersistent, "nopersistenthandles"},
> { Opt_resilient, "resilienthandles"},
> { Opt_noresilient, "noresilienthandles"},
> + { Opt_domainauto, "domainauto"},
>
> { Opt_backupuid, "backupuid=%s" },
> { Opt_backupgid, "backupgid=%s" },
> @@ -1499,6 +1501,9 @@ static int cifs_parse_security_flavors(char *value,
> case Opt_noresilient:
> vol->resilient = false; /* already the default */
> break;
> + case Opt_domainauto:
> + vol->domainauto = true;
> + break;
>
> /* Numeric Values */
> case Opt_backupuid:
> @@ -2548,6 +2553,8 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
> if (!ses->domainName)
> goto get_ses_fail;
> }
> + if (volume_info->domainauto)
> + ses->domainAuto = volume_info->domainauto;
> ses->cred_uid = volume_info->cred_uid;
> ses->linux_uid = volume_info->linux_uid;
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks right to me.
Acked-by: Pavel Shilovsky <pshilov at microsoft.com>
--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
More information about the samba-technical
mailing list