diff mbox series

cifs: connect to servername instead of IP for IPC$ share

Message ID FC6E4373-3884-453F-845C-CF6748E40A04@geo.uzh.ch
State New
Headers show
Series cifs: connect to servername instead of IP for IPC$ share | expand

Commit Message

Thomas Werschlein Aug. 30, 2018, 4:29 p.m. UTC
This change corresponds to the buffer size for the UNC (Aurélien Aptel), prevents authentication to be forced down to NTLM (Tom Talpey) and allows access to a Microsoft fileserver failover cluster behind a 1:1 NAT firewall.

Signed-off-by: Thomas Werschlein <thomas.werschlein@geo.uzh.ch>
---
 fs/cifs/connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Talpey Aug. 30, 2018, 5:05 p.m. UTC | #1
> -----Original Message-----
> From: Thomas Werschlein <thomas.werschlein@geo.uzh.ch>
> Sent: Thursday, August 30, 2018 12:29 PM
> To: CIFS <linux-cifs@vger.kernel.org>
> Cc: Steve French <smfrench@gmail.com>; Aurélien Aptel <aaptel@suse.com>;
> Tom Talpey <ttalpey@microsoft.com>
> Subject: [PATCH] cifs: connect to servername instead of IP for IPC$ share
> 
> This change corresponds to the buffer size for the UNC (Aurélien Aptel),
> prevents authentication to be forced down to NTLM (Tom Talpey) and allows

Well, sort of. "Prevents" isn’t the right word here. If the server only supports NTLM,
then you get what you get. And many servers, if forced down to NTLM, will refuse the
auth.

Now I think about it again, there's also the question of the server handling of the
sharename. MS-SMB2 section 3.3.5.7 and MS-SRVS 3.1.6.8 cover that. The numeric
address may not match the target share.

So I guess I'd suggest a more general "provides stronger context for authentication
and share connection".

Tom.


> access to a Microsoft fileserver failover cluster behind a 1:1 NAT firewall.
> 
> Signed-off-by: Thomas Werschlein <thomas.werschlein@geo.uzh.ch>
> ---
>  fs/cifs/connect.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index c832a8a1970a..7aa08dba4719 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2547,7 +2547,7 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol
> *volume_info)
>  	if (tcon == NULL)
>  		return -ENOMEM;
> 
> -	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
> +	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->server->hostname);
> 
>  	/* cannot fail */
>  	nls_codepage = load_nls_default();
> --
> 2.18.0
Thomas Werschlein Aug. 31, 2018, 8:21 a.m. UTC | #2
Tom, sorry for the wrong wording. I wanted to give credit, not putting words in your mouth.
Your suggestion is definitely better.

Regards
Thomas

> On 30 Aug 2018, at 19:05, Tom Talpey <ttalpey@microsoft.com> wrote:
> 
>> -----Original Message-----
>> From: Thomas Werschlein <thomas.werschlein@geo.uzh.ch>
>> Sent: Thursday, August 30, 2018 12:29 PM
>> To: CIFS <linux-cifs@vger.kernel.org>
>> Cc: Steve French <smfrench@gmail.com>; Aurélien Aptel <aaptel@suse.com>;
>> Tom Talpey <ttalpey@microsoft.com>
>> Subject: [PATCH] cifs: connect to servername instead of IP for IPC$ share
>> 
>> This change corresponds to the buffer size for the UNC (Aurélien Aptel),
>> prevents authentication to be forced down to NTLM (Tom Talpey) and allows
> 
> Well, sort of. "Prevents" isn’t the right word here. If the server only supports NTLM,
> then you get what you get. And many servers, if forced down to NTLM, will refuse the
> auth.
> 
> Now I think about it again, there's also the question of the server handling of the
> sharename. MS-SMB2 section 3.3.5.7 and MS-SRVS 3.1.6.8 cover that. The numeric
> address may not match the target share.
> 
> So I guess I'd suggest a more general "provides stronger context for authentication
> and share connection".
> 
> Tom.
> 
> 
>> access to a Microsoft fileserver failover cluster behind a 1:1 NAT firewall.
>> 
>> Signed-off-by: Thomas Werschlein <thomas.werschlein@geo.uzh.ch>
>> ---
>> fs/cifs/connect.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index c832a8a1970a..7aa08dba4719 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2547,7 +2547,7 @@ cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol
>> *volume_info)
>> 	if (tcon == NULL)
>> 		return -ENOMEM;
>> 
>> -	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
>> +	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->server->hostname);
>> 
>> 	/* cannot fail */
>> 	nls_codepage = load_nls_default();
>> --
>> 2.18.0
Aurélien Aptel Aug. 31, 2018, 12:21 p.m. UTC | #3
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

Steve, you might want to edit the message to something simpler and wrap
the long line.

Cheers,
Steve French Sept. 1, 2018, 2:44 a.m. UTC | #4
Updated commit description wording and added cc: stable

Let me know if you see any problems with it.

On Fri, Aug 31, 2018 at 7:21 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>
> Steve, you might want to edit the message to something simpler and wrap
> the long line.
>
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
diff mbox series

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index c832a8a1970a..7aa08dba4719 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2547,7 +2547,7 @@  cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol *volume_info)
 	if (tcon == NULL)
 		return -ENOMEM;
 
-	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
+	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->server->hostname);
 
 	/* cannot fail */
 	nls_codepage = load_nls_default();