Re: [PATCH] cifs: send workstation name during ntlmssp session setup

From: Stefan Metzmacher <metze@samba.org>
To: Shyam Prasad N <nspmangalore@gmail.com>,
	Steve French <smfrench@gmail.com>
Cc: Enzo Matsumiya <ematsumiya@suse.de>,
	CIFS <linux-cifs@vger.kernel.org>, Paulo Alcantara <pc@cjr.nz>
Subject: Re: [PATCH] cifs: send workstation name during ntlmssp session setup
Date: Sun, 7 Nov 2021 11:49:34 +0100	[thread overview]
Message-ID: <bf98a745-5feb-c38a-4641-6dd91c364c8e@samba.org> (raw)
In-Reply-To: <CANT5p=qOQDg4xDbx6oZafJc+gnM0pN+aYOgjokU54ZeLXq_uDQ@mail.gmail.com>

Hi Shyam,

>>>> Please review this patch, and let me know what you think.
>>>> Having this info in the workstation field of session setup helps
>>>> server debugging in two ways.
>>>> 1. It helps identify the client by node name.
>>>> 2. It helps get the kernel release running on the client side.
>>>>
>>>> https://github.com/sprasad-microsoft/smb3-kernel-client/commit/d988e704dd9170c19ff94d9421c017e65dbbaac1.patch
>>>
>>> - AFAICS it doesn't consider runtime hostname changes. Is it important
>>>    to keep track of it? Would changing it mid-auth steps break it
>>>    somehow?
> I think that's okay. AFAIK, that's only used for
> debugging/troubleshooting purposes. So it doesn't need to be a 100%
> accurate.

That's not true, the workstation name is used for access checks.

[MS-ADA3] 2.353 Attribute userWorkstations
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-ada3/ec941bac-bc77-48f3-a1cf-79d773a91b6b

>>>
>>> - I didn't understand the purpose of CIFS_DEFAULT_WORKSTATION_NAME. Why
>>>    not simply use utsname()->nodename? Or even init_utsname()->nodename, which
>>>    is supposed to be always valid.
> I initially did not have the utsname changes. That idea was an afterthought.
> Sure. I'll update the patch to fix this.
> 
>>>
>>> - Ditto for CIFS_MAX_WORKSTATION_LEN. utsname()->nodename has at most 65
>>>    bytes (__NEW_UTS_LEN + 1) anyway. Perhaps using MAXHOSTNAMELEN from
>>>    <asm/param.h> would be a more generic approach.
>>>    (btw this is because nodename is the unqualified hostname, sans-domain)
> Noted.
> 
>>>
>>> - Instead of setting workstation_name to "nodename:release", why not
>>>    implement the VERSION structure (MS-NLMP 2.2.2.10)? Then use
>>>    LINUX_VERSION_* from <linux/version.h> or parse utsname()->release.
> That's a good idea. Let me explore that too.

No, it's not this is for windows version numbers.
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-nlmp/a211d894-21bc-4b8b-86ba-b83d0c167b00#Appendix_A_32

If you want to encode something you can use
2.2.2.2 Single_Host_Data

metze