diff mbox series

CIFS regression mounting vers=1.0 NTLMSSP when hostname is too long

Message ID e6837098-15d9-acb6-7e34-1923cf8c6fe1@winds.org (mailing list archive)
State New, archived
Headers show
Series CIFS regression mounting vers=1.0 NTLMSSP when hostname is too long | expand

Commit Message

Byron Stanoszek May 3, 2022, 8:36 p.m. UTC
I would like to report a regression in the CIFS fs. Sometime between Linux 4.14
and 5.16, mounting CIFS with option vers=1.0 (and
CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
NTLMSSP stopped working for me. The server side is a Windows 2003 Server.

I found that this behavior depends on the length of the Linux client's
host+domain name (e.g. utsname()->nodename), where the mount works as long as
the name is 16 characters or less. Anything 17 or above returns -EIO, per the
following example:

/etc/fstab entry:

//10.0.0.12/xxxxxxxxx /ext0     cifs    vers=1.0,user=xxxxx,pass=xxxxxxxxxxx,dom=xxxxxxxxxxx,dir_mode=0755,file_mode=0644,noauto 0 0

# hostname 12345678901234567;mount /ext0
mount error(5): Input/output error
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
# hostname 1234567890123456;mount /ext0
#

I implemented a workaround using the following patch:

Signed-off-by: Byron Stanoszek <gandalf@winds.org>
---

I don't know if this patch is correct or will have any real effect outside of
the NTLMSSP session connect sequence, but it worked in my case.

I appended a transcript of the CIFS debug log from Linux 5.17.5 showing this
behavior. Server names are X'd out, and I highlighted the hostname as
"12345678901234567890".

Thanks,
  -Byron

  - - -

CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'source'
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'ip'
CIFS: address conversion returned 1 for 10.0.0.12
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'unc'
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'vers'
Use of the less secure dialect vers=1.0 is not recommended unless required for access to very old servers

CIFS: VFS: Use of the less secure dialect vers=1.0 is not recommended unless required for access to very old servers
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'user'
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'pass'
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'dom'
CIFS: fs/cifs/fs_context.c: Domain name set
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'dir_mode'
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'file_mode'
CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'netbiosname'
CIFS: fs/cifs/cifsfs.c: Devname: \\10.0.0.12\xxxxxxxxx flags: 0
CIFS: fs/cifs/connect.c: Username: xxxxx
CIFS: fs/cifs/connect.c: file mode: 0644  dir mode: 0755
CIFS: fs/cifs/connect.c: VFS: in mount_get_conns as Xid: 104 with uid: 0
CIFS: fs/cifs/connect.c: UNC: \\10.0.0.12\xxxxxxxxx
CIFS: fs/cifs/connect.c: generic_ip_connect: connecting to 10.0.0.12:445
CIFS: fs/cifs/connect.c: Socket created
CIFS: fs/cifs/connect.c: sndbuf 16384 rcvbuf 131072 rcvtimeo 0x1b58
CIFS: fs/cifs/connect.c: cifs_get_tcp_session: next dns resolution scheduled for 600 seconds in the future
CIFS: fs/cifs/connect.c: VFS: in cifs_get_smb_ses as Xid: 105 with uid: 0
CIFS: fs/cifs/connect.c: Existing smb sess not found
CIFS: fs/cifs/cifssmb.c: Requesting extended security
CIFS: fs/cifs/connect.c: Demultiplex PID: 6581
CIFS: fs/cifs/transport.c: wait_for_free_credits: remove 1 credits total=0
CIFS: fs/cifs/transport.c: For smb_command 114
CIFS: fs/cifs/transport.c: Sending smb: smb_len=51
0000 2f00                                .../
53ff 424d 0072 0000 0000 c801 0000 0000  .SMBr...........
0000 0000 0000 0000 0000 19b3 0000 0001  ................
00 0c 00 02 4e 54 20 4c 4d 20 30 2e 31 32 00     ....NT LM 0.12.
CIFS: fs/cifs/connect.c: RFC1002 header 0xb2
0000 b200 53ff 424d 0072 0000 9800 8001  .....SMBr.......
0000 0000 0000 0000 0000 0000 0000 19b3  ................
0000 0001 0011 0300 00fd 0001 8104 0000  ................
0000 0001 e4e2 692b d3fd 8000 79be a08c  ......+i.....y..
5f1b 01d8 012c 6d00 1500 0000 4200 e8c1  ._..,..m.....B..
3625 c892 1069 2669 6056 065b 2b06 0106  %6..i.i&V`[..+..
0505 a002 3051 a04f 3024 0622 2a09 4886  ....Q0O.$0"..*.H
f782 0112 0202 0906 862a 8648 12f7 0201  ........*.H.....
0602 2b0a 0106 0104 3782 0202 a30a 3027  ...+.....7....'0
a025 1b23 xx21 xxxx xxxx xxxx xxxx xxxx  %.#.!xxxxxxxxxxx
4024 xxxx xxxx xxxx xxxx xxxx xxxx xxxx  $@xxxxxxxxxxxxxx
xxxx xxxx xxxx                           xxxxxx
CIFS: fs/cifs/misc.c: checkSMB Length: 0xb6, smb_buf_length: 0xb2
CIFS: fs/cifs/transport.c: cifs_sync_mid_result: cmd=114 mid=1 state=4
0000 b200 53ff 424d 0072 0000 9800 8001  .....SMBr.......
0000 0000 0000 0000 0000 0000 0000 19b3  ................
0000 0001 0011 0300 00fd 0001 8104 0000  ................
0000 0001 e4e2 692b d3fd 8000 79be a08c  ......+i.....y..
5f1b 01d8 012c 6d00 1500 0000 4200 e8c1  ._..,..m.....B..
3625 c892 1069 2669 6056 065b            %6..i.i&V`[.
CIFS: fs/cifs/cifssmb.c: Dialect: 0
CIFS: Max buf = 33028
CIFS: fs/cifs/cifssmb.c: negprot rc 0
CIFS: fs/cifs/connect.c: Security Mode: 0x3 Capabilities: 0x8000d3fd TimeAdjust: 18000
CIFS: fs/cifs/sess.c: sess setup type 2
CIFS: fs/cifs/sess.c: rawntlmssp session setup negotiate phase
CIFS: fs/cifs/transport.c: wait_for_free_credits: remove 1 credits total=252
CIFS: fs/cifs/transport.c: For smb_command 115
CIFS: fs/cifs/transport.c: Sending smb: smb_len=194
0000 be00                                ....
53ff 424d 0073 0000 0000 d801 0000 0000  .SMBs...........
0000 0000 0000 0000 0000 19b3 0000 0002  ................
ff0c 0000 5400 fd40 0100 0000 0000 2400  .....T@........$
00 00 00 00 00 dc d0 00 80 83 00                 ...........
544e 4d4c 5353 0050 0001 0000 8235 e008  NTLMSSP.....5...
0000 0000 0020 0000 0000 0000 0022 0000  .... ......."...
0000 0000                                ....
4c00 6900 6e00 7500 7800 2000 7600 6500  .L.i.n.u.x. .v.e
7200 7300 6900 6f00 6e00 2000 3500 2e00  .r.s.i.o.n. .5..
3100 3700 2e00 3500 0000 4300 4900 4600  .1.7...5...C.I.F
5300 2000 5600 4600 5300 2000 4300 6c00  .S. .V.F.S. .C.l
6900 6500 6e00 7400 2000 6600 6f00 7200  .i.e.n.t. .f.o.r
00 20 00 4c 00 69 00 6e 00 75 00 78 00 00 00     . .L.i.n.u.x...
CIFS: fs/cifs/connect.c: RFC1002 header 0x154
0000 5401 53ff 424d 1673 0000 98c0 c807  ...T.SMBs.......
0000 0000 0000 0000 0000 0000 0000 19b3  ................
0800 0002 ff04 0000 0000 de00 2900 4e01  .............).N
4c54 534d 5053 0200 0000 0e00 0e00 3000  TLMSSP.........0
0000 0500 8982 6560 8fb0 4b2d 8980 002a  ......`e..-K..*.
0000 0000 0000 a000 a000 3e00 0000 XX00  ...........>...X
XX00 XX00 XX00 XX00 XX00 XX00 0200 0e00  .X.X.X.X.X.X....
XX00 XX00 XX00 XX00 XX00 XX00 XX00 0100  .X.X.X.X.X.X.X..
1600 XX00 XX00 XX00 XX00 XX00 XX00 XX00  ...X.X.X.X.X.X.X
XX00 XX00 XX00 XX00 0400 XX00 XX00 XX00  .X.X.X.X...X.X.X
XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
XX00 XX00 0300 XX00 XX00 XX00 XX00 XX00  .X.X...X.X.X.X.X
XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
XX00 XX00 XX00 XX00 0000 0000 0000 0057  .X.X.X.X......W.
0069 006e 0064 006f 0077 0073 0020 0035  i.n.d.o.w.s. .5.
002e 0030 0000 0057 0069 006e 0064 006f  ..0...W.i.n.d.o.
0077 0073 0020 0032 0030 0030 0030 0020  w.s. .2.0.0.0. .
004c 0041 004e 0020 004d 0061 006e 0061  L.A.N. .M.a.n.a.
0067 0065 0072 0000                      g.e.r...
CIFS: fs/cifs/misc.c: checkSMB Length: 0x158, smb_buf_length: 0x154
CIFS: fs/cifs/transport.c: cifs_sync_mid_result: cmd=115 mid=2 state=4
0000 5401 53ff 424d 1673 0000 98c0 c807  ...T.SMBs.......
0000 0000 0000 0000 0000 0000 0000 19b3  ................
0800 0002 ff04 0000 0000 de00 2900 4e01  .............).N
4c54 534d 5053 0200 0000 0e00 0e00 3000  TLMSSP.........0
0000 0500 8982 6560 8fb0 4b2d 8980 002a  ......`e..-K..*.
0000 0000 0000 a000 a000 3e00            ...........>
CIFS: Status code returned 0xc0000016 NT_STATUS_MORE_PROCESSING_REQUIRED
CIFS: fs/cifs/netmisc.c: Mapping smb error code 0xc0000016 to POSIX err -5
CIFS: fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
CIFS: fs/cifs/sess.c: rawntlmssp session setup challenge phase
CIFS: fs/cifs/sess.c: UID = 2048
CIFS: fs/cifs/sess.c: decode_ntlmssp_challenge: negotiate=0xe0088235 challenge=0x60898205
CIFS: fs/cifs/sess.c: rawntlmssp session setup authenticate phase
CIFS: fs/cifs/transport.c: wait_for_free_credits: remove 1 credits total=252
CIFS: fs/cifs/transport.c: For smb_command 115
CIFS: fs/cifs/transport.c: Sending smb: smb_len=514
0000 fe01                                ....
53ff 424d 0073 0000 0000 d801 0000 0000  .SMBs...........
0000 0000 0000 0000 0000 19b3 0800 0003  ................
ff0c 0000 5400 fd40 0100 0000 0000 6400  .....T@........d
01 00 00 00 00 dc d0 00 80 c3 01                 ...........
544e 4d4c 5353 0050 0003 0000 0000 0000  NTLMSSP.........
0040 0000 00cc 00cc 0040 0000 0016 0016  @.......@.......
010c 0000 000a 000a 0122 0000 0028 0028  ........"...(.(.
012c 0000 0010 0010 0154 0000 a205 6089  ,.......T......`
277e 10f2 c522 1143 c4d3 2343 28f2 5b32  ~'..".C...C#.(2[
0101 0000 0000 0000 2278 bb86 5f1b 01d8  ........x"..._..
66a8 9bd0 c591 0e07 0000 0000 0002 000e  .f..............
00XX 00XX 00XX 00XX 00XX 00XX 00XX 0001  X.X.X.X.X.X.X...
0016 00XX 00XX 00XX 00XX 00XX 00XX 00XX  ..X.X.X.X.X.X.X.
00XX 00XX 00XX 00XX 0004 00XX 00XX 00XX  X.X.X.X...X.X.X.
00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
00XX 00XX 0003 00XX 00XX 00XX 00XX 00XX  X.X...@.X.X.X.X.
00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
00XX 00XX 00XX 00XX 0000 0000 00XX 00XX  X.X.X.X.....X.X.
00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
00XX 00XX 00XX 00XX 00XX 00XX 0031 0032  X.X.X.X.X.X.1.2.
0033 0034 0035 0036 0037 0038 0039 0030  3.4.5.6.7.8.9.0.
0031 0032 0033 0034 0035 0036 0037 0038  1.2.3.4.5.6.7.8.
0039 0030 3063 e7c2 9bea b237 7fe3 a91f  9.0.c0....7.....
ac5f e633                                _.3.
4c00 6900 6e00 7500 7800 2000 7600 6500  .L.i.n.u.x. .v.e
7200 7300 6900 6f00 6e00 2000 3500 2e00  .r.s.i.o.n. .5..
3100 3700 2e00 3500 0000 4300 4900 4600  .1.7...5...C.I.F
5300 2000 5600 4600 5300 2000 4300 6c00  .S. .V.F.S. .C.l
6900 6500 6e00 7400 2000 6600 6f00 7200  .i.e.n.t. .f.o.r
00 20 00 4c 00 69 00 6e 00 75 00 78 00 00 00     . .L.i.n.u.x...
CIFS: fs/cifs/connect.c: RFC1002 header 0x23
0000 2300 53ff 424d 1673 0000 88c0 c001  ...#.SMBs.......
0000 0000 0000 0000 0000 0000 0000 19b3  ................
00 00 03 00 00 00 00                             .......
CIFS: fs/cifs/misc.c: checkSMB Length: 0x27, smb_buf_length: 0x23
CIFS: fs/cifs/transport.c: cifs_sync_mid_result: cmd=115 mid=3 state=4
0000 2300 53ff 424d 1673 0000 88c0 c001  ...#.SMBs.......
0000 0000 0000 0000 0000 0000 0000 19b3  ................
00 00 03 00 00 00 00                             .......
CIFS: Status code returned 0xc0000016 NT_STATUS_MORE_PROCESSING_REQUIRED
CIFS: fs/cifs/netmisc.c: Mapping smb error code 0xc0000016 to POSIX err -5
CIFS: fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
CIFS: VFS: \\10.0.0.12 Send error in SessSetup = -5
CIFS: fs/cifs/connect.c: VFS: leaving cifs_get_smb_ses (xid = 105) rc = -5
CIFS: fs/cifs/connect.c: VFS: leaving mount_put_conns (xid = 104) rc = 0
CIFS: VFS: cifs_mount failed w/return code = -5

Comments

Paulo Alcantara May 4, 2022, 1:35 a.m. UTC | #1
Byron Stanoszek <gandalf@winds.org> writes:

> I would like to report a regression in the CIFS fs. Sometime between Linux 4.14
> and 5.16, mounting CIFS with option vers=1.0 (and
> CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
> NTLMSSP stopped working for me. The server side is a Windows 2003 Server.
>
> I found that this behavior depends on the length of the Linux client's
> host+domain name (e.g. utsname()->nodename), where the mount works as long as
> the name is 16 characters or less. Anything 17 or above returns -EIO, per the
> following example:

Looks like your server is expecting the WorkstationName field in
AUTHENTICATE_MESSAGE payload to be 16 bytes long.  That is, NetBIOS name
length as per rfc1001.

> I implemented a workaround using the following patch:
>
> Signed-off-by: Byron Stanoszek <gandalf@winds.org>
> ---
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -101,7 +101,7 @@
>   #define XATTR_DOS_ATTRIB "user.DOSATTRIB"
>   #endif
>
> -#define CIFS_MAX_WORKSTATION_LEN  (__NEW_UTS_LEN + 1)  /* reasonable max for client */
> +#define CIFS_MAX_WORKSTATION_LEN 16
>
>   /*
>    * CIFS vfs client Status information (based on what we know.)
>
> I don't know if this patch is correct or will have any real effect outside of
> the NTLMSSP session connect sequence, but it worked in my case.

Perhaps we should be use TCP_Server_Info::workstation_RFC1001_name in
fs/cifs/sess.c:build_ntlmssp_auth_blob() instead only when connecting to
old servers by using insecure dialects -- like SMB1, in your case.
Steven French May 4, 2022, 5:43 a.m. UTC | #2
makes sense - do you see anything related in the NTLMSSP doc?

Want to spin up a patch for SMB1 for this?

On 5/3/22 20:35, Paulo Alcantara wrote:
> Byron Stanoszek <gandalf@winds.org> writes:
>
>> I would like to report a regression in the CIFS fs. Sometime between Linux 4.14
>> and 5.16, mounting CIFS with option vers=1.0 (and
>> CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
>> NTLMSSP stopped working for me. The server side is a Windows 2003 Server.
>>
>> I found that this behavior depends on the length of the Linux client's
>> host+domain name (e.g. utsname()->nodename), where the mount works as long as
>> the name is 16 characters or less. Anything 17 or above returns -EIO, per the
>> following example:
> Looks like your server is expecting the WorkstationName field in
> AUTHENTICATE_MESSAGE payload to be 16 bytes long.  That is, NetBIOS name
> length as per rfc1001.
>
>> I implemented a workaround using the following patch:
>>
>> Signed-off-by: Byron Stanoszek <gandalf@winds.org>
>> ---
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -101,7 +101,7 @@
>>    #define XATTR_DOS_ATTRIB "user.DOSATTRIB"
>>    #endif
>>
>> -#define CIFS_MAX_WORKSTATION_LEN  (__NEW_UTS_LEN + 1)  /* reasonable max for client */
>> +#define CIFS_MAX_WORKSTATION_LEN 16
>>
>>    /*
>>     * CIFS vfs client Status information (based on what we know.)
>>
>> I don't know if this patch is correct or will have any real effect outside of
>> the NTLMSSP session connect sequence, but it worked in my case.
> Perhaps we should be use TCP_Server_Info::workstation_RFC1001_name in
> fs/cifs/sess.c:build_ntlmssp_auth_blob() instead only when connecting to
> old servers by using insecure dialects -- like SMB1, in your case.
Thorsten Leemhuis May 4, 2022, 7:13 a.m. UTC | #3
[TLDR: I'm adding the regression report below to regzbot, the Linux
kernel regression tracking bot; all text you find below is compiled from
a few templates paragraphs you might have encountered already already
from similar mails.]

Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.

To be sure below issue doesn't fall through the cracks unnoticed, I'm
adding it to regzbot, my Linux kernel regression tracking bot:

#regzbot ^introduced v4.14..v5.16
#regzbot title cifs: regression mounting vers=1.0 NTLMSSP when hostname
is too long
#regzbot ignore-activity

If it turns out this isn't a regression, free free to remove it from the
tracking by sending a reply to this thread containing a paragraph like
"#regzbot invalid: reason why this is invalid" (without the quotes).

Reminder for developers: when fixing the issue, please add a 'Link:'
tags pointing to the report (the mail quoted above) using
lore.kernel.org/r/, as explained in
'Documentation/process/submitting-patches.rst' and
'Documentation/process/5.Posting.rst'. Regzbot needs them to
automatically connect reports with fixes, but they are useful in
general, too.

I'm sending this to everyone that got the initial report, to make
everyone aware of the tracking. I also hope that messages like this
motivate people to directly get at least the regression mailing list and
ideally even regzbot involved when dealing with regressions, as messages
like this wouldn't be needed then. And don't worry, if I need to send
other mails regarding this regression only relevant for regzbot I'll
send them to the regressions lists only (with a tag in the subject so
people can filter them away). With a bit of luck no such messages will
be needed anyway.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.


On 03.05.22 22:36, Byron Stanoszek wrote:
> I would like to report a regression in the CIFS fs. Sometime between
> Linux 4.14
> and 5.16, mounting CIFS with option vers=1.0 (and
> CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
> NTLMSSP stopped working for me. The server side is a Windows 2003 Server.
> 
> I found that this behavior depends on the length of the Linux client's
> host+domain name (e.g. utsname()->nodename), where the mount works as
> long as
> the name is 16 characters or less. Anything 17 or above returns -EIO,
> per the
> following example:
> 
> /etc/fstab entry:
> 
> //10.0.0.12/xxxxxxxxx /ext0     cifs   
> vers=1.0,user=xxxxx,pass=xxxxxxxxxxx,dom=xxxxxxxxxxx,dir_mode=0755,file_mode=0644,noauto
> 0 0
> 
> # hostname 12345678901234567;mount /ext0
> mount error(5): Input/output error
> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel
> log messages (dmesg)
> # hostname 1234567890123456;mount /ext0
> #
> 
> I implemented a workaround using the following patch:
> 
> Signed-off-by: Byron Stanoszek <gandalf@winds.org>
> ---
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -101,7 +101,7 @@
>  #define XATTR_DOS_ATTRIB "user.DOSATTRIB"
>  #endif
> 
> -#define CIFS_MAX_WORKSTATION_LEN  (__NEW_UTS_LEN + 1)  /* reasonable
> max for client */
> +#define CIFS_MAX_WORKSTATION_LEN 16
> 
>  /*
>   * CIFS vfs client Status information (based on what we know.)
> 
> I don't know if this patch is correct or will have any real effect
> outside of
> the NTLMSSP session connect sequence, but it worked in my case.
> 
> I appended a transcript of the CIFS debug log from Linux 5.17.5 showing
> this
> behavior. Server names are X'd out, and I highlighted the hostname as
> "12345678901234567890".
> 
> Thanks,
>  -Byron
> 
>  - - -
> 
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'source'
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'ip'
> CIFS: address conversion returned 1 for 10.0.0.12
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'unc'
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'vers'
> Use of the less secure dialect vers=1.0 is not recommended unless
> required for access to very old servers
> 
> CIFS: VFS: Use of the less secure dialect vers=1.0 is not recommended
> unless required for access to very old servers
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'user'
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'pass'
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'dom'
> CIFS: fs/cifs/fs_context.c: Domain name set
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'dir_mode'
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'file_mode'
> CIFS: fs/cifs/fs_context.c: CIFS: parsing cifs mount option 'netbiosname'
> CIFS: fs/cifs/cifsfs.c: Devname: \\10.0.0.12\xxxxxxxxx flags: 0
> CIFS: fs/cifs/connect.c: Username: xxxxx
> CIFS: fs/cifs/connect.c: file mode: 0644  dir mode: 0755
> CIFS: fs/cifs/connect.c: VFS: in mount_get_conns as Xid: 104 with uid: 0
> CIFS: fs/cifs/connect.c: UNC: \\10.0.0.12\xxxxxxxxx
> CIFS: fs/cifs/connect.c: generic_ip_connect: connecting to 10.0.0.12:445
> CIFS: fs/cifs/connect.c: Socket created
> CIFS: fs/cifs/connect.c: sndbuf 16384 rcvbuf 131072 rcvtimeo 0x1b58
> CIFS: fs/cifs/connect.c: cifs_get_tcp_session: next dns resolution
> scheduled for 600 seconds in the future
> CIFS: fs/cifs/connect.c: VFS: in cifs_get_smb_ses as Xid: 105 with uid: 0
> CIFS: fs/cifs/connect.c: Existing smb sess not found
> CIFS: fs/cifs/cifssmb.c: Requesting extended security
> CIFS: fs/cifs/connect.c: Demultiplex PID: 6581
> CIFS: fs/cifs/transport.c: wait_for_free_credits: remove 1 credits total=0
> CIFS: fs/cifs/transport.c: For smb_command 114
> CIFS: fs/cifs/transport.c: Sending smb: smb_len=51
> 0000 2f00                                .../
> 53ff 424d 0072 0000 0000 c801 0000 0000  .SMBr...........
> 0000 0000 0000 0000 0000 19b3 0000 0001  ................
> 00 0c 00 02 4e 54 20 4c 4d 20 30 2e 31 32 00     ....NT LM 0.12.
> CIFS: fs/cifs/connect.c: RFC1002 header 0xb2
> 0000 b200 53ff 424d 0072 0000 9800 8001  .....SMBr.......
> 0000 0000 0000 0000 0000 0000 0000 19b3  ................
> 0000 0001 0011 0300 00fd 0001 8104 0000  ................
> 0000 0001 e4e2 692b d3fd 8000 79be a08c  ......+i.....y..
> 5f1b 01d8 012c 6d00 1500 0000 4200 e8c1  ._..,..m.....B..
> 3625 c892 1069 2669 6056 065b 2b06 0106  %6..i.i&V`[..+..
> 0505 a002 3051 a04f 3024 0622 2a09 4886  ....Q0O.$0"..*.H
> f782 0112 0202 0906 862a 8648 12f7 0201  ........*.H.....
> 0602 2b0a 0106 0104 3782 0202 a30a 3027  ...+.....7....'0
> a025 1b23 xx21 xxxx xxxx xxxx xxxx xxxx  %.#.!xxxxxxxxxxx
> 4024 xxxx xxxx xxxx xxxx xxxx xxxx xxxx  $@xxxxxxxxxxxxxx
> xxxx xxxx xxxx                           xxxxxx
> CIFS: fs/cifs/misc.c: checkSMB Length: 0xb6, smb_buf_length: 0xb2
> CIFS: fs/cifs/transport.c: cifs_sync_mid_result: cmd=114 mid=1 state=4
> 0000 b200 53ff 424d 0072 0000 9800 8001  .....SMBr.......
> 0000 0000 0000 0000 0000 0000 0000 19b3  ................
> 0000 0001 0011 0300 00fd 0001 8104 0000  ................
> 0000 0001 e4e2 692b d3fd 8000 79be a08c  ......+i.....y..
> 5f1b 01d8 012c 6d00 1500 0000 4200 e8c1  ._..,..m.....B..
> 3625 c892 1069 2669 6056 065b            %6..i.i&V`[.
> CIFS: fs/cifs/cifssmb.c: Dialect: 0
> CIFS: Max buf = 33028
> CIFS: fs/cifs/cifssmb.c: negprot rc 0
> CIFS: fs/cifs/connect.c: Security Mode: 0x3 Capabilities: 0x8000d3fd
> TimeAdjust: 18000
> CIFS: fs/cifs/sess.c: sess setup type 2
> CIFS: fs/cifs/sess.c: rawntlmssp session setup negotiate phase
> CIFS: fs/cifs/transport.c: wait_for_free_credits: remove 1 credits
> total=252
> CIFS: fs/cifs/transport.c: For smb_command 115
> CIFS: fs/cifs/transport.c: Sending smb: smb_len=194
> 0000 be00                                ....
> 53ff 424d 0073 0000 0000 d801 0000 0000  .SMBs...........
> 0000 0000 0000 0000 0000 19b3 0000 0002  ................
> ff0c 0000 5400 fd40 0100 0000 0000 2400  .....T@........$
> 00 00 00 00 00 dc d0 00 80 83 00                 ...........
> 544e 4d4c 5353 0050 0001 0000 8235 e008  NTLMSSP.....5...
> 0000 0000 0020 0000 0000 0000 0022 0000  .... ......."...
> 0000 0000                                ....
> 4c00 6900 6e00 7500 7800 2000 7600 6500  .L.i.n.u.x. .v.e
> 7200 7300 6900 6f00 6e00 2000 3500 2e00  .r.s.i.o.n. .5..
> 3100 3700 2e00 3500 0000 4300 4900 4600  .1.7...5...C.I.F
> 5300 2000 5600 4600 5300 2000 4300 6c00  .S. .V.F.S. .C.l
> 6900 6500 6e00 7400 2000 6600 6f00 7200  .i.e.n.t. .f.o.r
> 00 20 00 4c 00 69 00 6e 00 75 00 78 00 00 00     . .L.i.n.u.x...
> CIFS: fs/cifs/connect.c: RFC1002 header 0x154
> 0000 5401 53ff 424d 1673 0000 98c0 c807  ...T.SMBs.......
> 0000 0000 0000 0000 0000 0000 0000 19b3  ................
> 0800 0002 ff04 0000 0000 de00 2900 4e01  .............).N
> 4c54 534d 5053 0200 0000 0e00 0e00 3000  TLMSSP.........0
> 0000 0500 8982 6560 8fb0 4b2d 8980 002a  ......`e..-K..*.
> 0000 0000 0000 a000 a000 3e00 0000 XX00  ...........>...X
> XX00 XX00 XX00 XX00 XX00 XX00 0200 0e00  .X.X.X.X.X.X....
> XX00 XX00 XX00 XX00 XX00 XX00 XX00 0100  .X.X.X.X.X.X.X..
> 1600 XX00 XX00 XX00 XX00 XX00 XX00 XX00  ...X.X.X.X.X.X.X
> XX00 XX00 XX00 XX00 0400 XX00 XX00 XX00  .X.X.X.X...X.X.X
> XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
> XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
> XX00 XX00 0300 XX00 XX00 XX00 XX00 XX00  .X.X...X.X.X.X.X
> XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
> XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
> XX00 XX00 XX00 XX00 XX00 XX00 XX00 XX00  .X.X.X.X.X.X.X.X
> XX00 XX00 XX00 XX00 0000 0000 0000 0057  .X.X.X.X......W.
> 0069 006e 0064 006f 0077 0073 0020 0035  i.n.d.o.w.s. .5.
> 002e 0030 0000 0057 0069 006e 0064 006f  ..0...W.i.n.d.o.
> 0077 0073 0020 0032 0030 0030 0030 0020  w.s. .2.0.0.0. .
> 004c 0041 004e 0020 004d 0061 006e 0061  L.A.N. .M.a.n.a.
> 0067 0065 0072 0000                      g.e.r...
> CIFS: fs/cifs/misc.c: checkSMB Length: 0x158, smb_buf_length: 0x154
> CIFS: fs/cifs/transport.c: cifs_sync_mid_result: cmd=115 mid=2 state=4
> 0000 5401 53ff 424d 1673 0000 98c0 c807  ...T.SMBs.......
> 0000 0000 0000 0000 0000 0000 0000 19b3  ................
> 0800 0002 ff04 0000 0000 de00 2900 4e01  .............).N
> 4c54 534d 5053 0200 0000 0e00 0e00 3000  TLMSSP.........0
> 0000 0500 8982 6560 8fb0 4b2d 8980 002a  ......`e..-K..*.
> 0000 0000 0000 a000 a000 3e00            ...........>
> CIFS: Status code returned 0xc0000016 NT_STATUS_MORE_PROCESSING_REQUIRED
> CIFS: fs/cifs/netmisc.c: Mapping smb error code 0xc0000016 to POSIX err -5
> CIFS: fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> CIFS: fs/cifs/sess.c: rawntlmssp session setup challenge phase
> CIFS: fs/cifs/sess.c: UID = 2048
> CIFS: fs/cifs/sess.c: decode_ntlmssp_challenge: negotiate=0xe0088235
> challenge=0x60898205
> CIFS: fs/cifs/sess.c: rawntlmssp session setup authenticate phase
> CIFS: fs/cifs/transport.c: wait_for_free_credits: remove 1 credits
> total=252
> CIFS: fs/cifs/transport.c: For smb_command 115
> CIFS: fs/cifs/transport.c: Sending smb: smb_len=514
> 0000 fe01                                ....
> 53ff 424d 0073 0000 0000 d801 0000 0000  .SMBs...........
> 0000 0000 0000 0000 0000 19b3 0800 0003  ................
> ff0c 0000 5400 fd40 0100 0000 0000 6400  .....T@........d
> 01 00 00 00 00 dc d0 00 80 c3 01                 ...........
> 544e 4d4c 5353 0050 0003 0000 0000 0000  NTLMSSP.........
> 0040 0000 00cc 00cc 0040 0000 0016 0016  @.......@.......
> 010c 0000 000a 000a 0122 0000 0028 0028  ........"...(.(.
> 012c 0000 0010 0010 0154 0000 a205 6089  ,.......T......`
> 277e 10f2 c522 1143 c4d3 2343 28f2 5b32  ~'..".C...C#.(2[
> 0101 0000 0000 0000 2278 bb86 5f1b 01d8  ........x"..._..
> 66a8 9bd0 c591 0e07 0000 0000 0002 000e  .f..............
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 0001  X.X.X.X.X.X.X...
> 0016 00XX 00XX 00XX 00XX 00XX 00XX 00XX  ..X.X.X.X.X.X.X.
> 00XX 00XX 00XX 00XX 0004 00XX 00XX 00XX  X.X.X.X...X.X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
> 00XX 00XX 0003 00XX 00XX 00XX 00XX 00XX  X.X...@.X.X.X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
> 00XX 00XX 00XX 00XX 0000 0000 00XX 00XX  X.X.X.X.....X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 00XX 00XX  X.X.X.X.X.X.X.X.
> 00XX 00XX 00XX 00XX 00XX 00XX 0031 0032  X.X.X.X.X.X.1.2.
> 0033 0034 0035 0036 0037 0038 0039 0030  3.4.5.6.7.8.9.0.
> 0031 0032 0033 0034 0035 0036 0037 0038  1.2.3.4.5.6.7.8.
> 0039 0030 3063 e7c2 9bea b237 7fe3 a91f  9.0.c0....7.....
> ac5f e633                                _.3.
> 4c00 6900 6e00 7500 7800 2000 7600 6500  .L.i.n.u.x. .v.e
> 7200 7300 6900 6f00 6e00 2000 3500 2e00  .r.s.i.o.n. .5..
> 3100 3700 2e00 3500 0000 4300 4900 4600  .1.7...5...C.I.F
> 5300 2000 5600 4600 5300 2000 4300 6c00  .S. .V.F.S. .C.l
> 6900 6500 6e00 7400 2000 6600 6f00 7200  .i.e.n.t. .f.o.r
> 00 20 00 4c 00 69 00 6e 00 75 00 78 00 00 00     . .L.i.n.u.x...
> CIFS: fs/cifs/connect.c: RFC1002 header 0x23
> 0000 2300 53ff 424d 1673 0000 88c0 c001  ...#.SMBs.......
> 0000 0000 0000 0000 0000 0000 0000 19b3  ................
> 00 00 03 00 00 00 00                             .......
> CIFS: fs/cifs/misc.c: checkSMB Length: 0x27, smb_buf_length: 0x23
> CIFS: fs/cifs/transport.c: cifs_sync_mid_result: cmd=115 mid=3 state=4
> 0000 2300 53ff 424d 1673 0000 88c0 c001  ...#.SMBs.......
> 0000 0000 0000 0000 0000 0000 0000 19b3  ................
> 00 00 03 00 00 00 00                             .......
> CIFS: Status code returned 0xc0000016 NT_STATUS_MORE_PROCESSING_REQUIRED
> CIFS: fs/cifs/netmisc.c: Mapping smb error code 0xc0000016 to POSIX err -5
> CIFS: fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> CIFS: VFS: \\10.0.0.12 Send error in SessSetup = -5
> CIFS: fs/cifs/connect.c: VFS: leaving cifs_get_smb_ses (xid = 105) rc = -5
> CIFS: fs/cifs/connect.c: VFS: leaving mount_put_conns (xid = 104) rc = 0
> CIFS: VFS: cifs_mount failed w/return code = -5
Paulo Alcantara May 4, 2022, 7:15 p.m. UTC | #4
Hi Steve,

Steven French <sfrench@samba.org> writes:

> makes sense - do you see anything related in the NTLMSSP doc?

I'll quote some relevant parts from MS-NLMP which make sense to me:

	3.1.5.1.2 Client Receives a CHALLENGE_MESSAGE from the Server
	...
	If the NTLMSSP_NEGOTIATE_VERSION flag is set by the client application,
	the Version field MUST be set to the current version (section 2.2.2.10),
	and the Workstation field MUST be set to NbMachineName.
	
	3.2.1.1 Variables Internal to the Protocol
	...
	NbMachineName: A string that indicates the NetBIOS machine name of the
	server.
	
	2.2.2.1 AV_PAIR
	...
	MsvAvNbComputerName: The server's NetBIOS computer name. The name MUST
	be in Unicode, and is not null-terminated. This type of information MUST
	be present in the AV_pair list.

and indeed we set NTLMSSP_NEGOTIATE_VERSION in
fs/cifs/sess.c:build_ntlmssp_smb3_negotiate_blob().

Unless I didn't miss anything obvious, I think we should be sending
NetBIOS name or simply truncate utsname()->nodename to 16 bytes as
previously proposed by Byron regardless what protocol version is being
used.

Tom, what is your opinion on that?
Tom Talpey May 4, 2022, 8:18 p.m. UTC | #5
On 5/4/2022 3:15 PM, Paulo Alcantara wrote:
> Hi Steve,
> 
> Steven French <sfrench@samba.org> writes:
> 
>> makes sense - do you see anything related in the NTLMSSP doc?
> 
> I'll quote some relevant parts from MS-NLMP which make sense to me:
> 
> 	3.1.5.1.2 Client Receives a CHALLENGE_MESSAGE from the Server
> 	...
> 	If the NTLMSSP_NEGOTIATE_VERSION flag is set by the client application,
> 	the Version field MUST be set to the current version (section 2.2.2.10),
> 	and the Workstation field MUST be set to NbMachineName.
> 	
> 	3.2.1.1 Variables Internal to the Protocol
> 	...
> 	NbMachineName: A string that indicates the NetBIOS machine name of the
> 	server.
> 	
> 	2.2.2.1 AV_PAIR
> 	...
> 	MsvAvNbComputerName: The server's NetBIOS computer name. The name MUST
> 	be in Unicode, and is not null-terminated. This type of information MUST
> 	be present in the AV_pair list.
> 
> and indeed we set NTLMSSP_NEGOTIATE_VERSION in
> fs/cifs/sess.c:build_ntlmssp_smb3_negotiate_blob().
> 
> Unless I didn't miss anything obvious, I think we should be sending
> NetBIOS name or simply truncate utsname()->nodename to 16 bytes as
> previously proposed by Byron regardless what protocol version is being
> used.
> 
> Tom, what is your opinion on that?

I think the most conservative and spec-compliant choice should be made.
SMB1 should not be pushing the envelope of interoperability, in this day
and age.

I believe the NetBIOS name is a fixed array of 16 octets, right? So, if
the nodename is shorter, it needs to be padded with 0's.

Did this code change recently? Why???

Tom.
Paulo Alcantara May 4, 2022, 8:58 p.m. UTC | #6
Tom Talpey <tom@talpey.com> writes:

> I think the most conservative and spec-compliant choice should be made.
> SMB1 should not be pushing the envelope of interoperability, in this day
> and age.

OK.

> I believe the NetBIOS name is a fixed array of 16 octets, right? So, if
> the nodename is shorter, it needs to be padded with 0's.

Right.

> Did this code change recently? Why???

We used to not send the WorkstationName during NTLMSSP until recent
patch from Shyam:

	commit 49bd49f983b5026e4557d31c5d737d9657c4113e
	Author: Shyam Prasad N <sprasad@microsoft.com>
	Date:   Fri Nov 5 19:03:57 2021 +0000
	
	    cifs: send workstation name during ntlmssp session setup
	
	    During the ntlmssp session setup (authenticate phases)
	    send the client workstation info. This can make debugging easier on
	    servers.
	
	    Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
	    Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
	    Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
	    Signed-off-by: Steve French <stfrench@microsoft.com>

Unfortunately some servers did not seem to enforce it to be 16 bytes
long, so the reason why we didn't catch it earlier.

Steve, Shyam, let me know if it does make sense to you and then I can
work on a patch to fix it properly.
Steve French May 6, 2022, 2:03 a.m. UTC | #7
Yes - this makes sense.  Patch would be appreciated (just got back
from LSF/MM, so catching up after travel)

On Thu, May 5, 2022 at 8:59 PM Paulo Alcantara <pc@cjr.nz> wrote:
>
> Tom Talpey <tom@talpey.com> writes:
>
> > I think the most conservative and spec-compliant choice should be made.
> > SMB1 should not be pushing the envelope of interoperability, in this day
> > and age.
>
> OK.
>
> > I believe the NetBIOS name is a fixed array of 16 octets, right? So, if
> > the nodename is shorter, it needs to be padded with 0's.
>
> Right.
>
> > Did this code change recently? Why???
>
> We used to not send the WorkstationName during NTLMSSP until recent
> patch from Shyam:
>
>         commit 49bd49f983b5026e4557d31c5d737d9657c4113e
>         Author: Shyam Prasad N <sprasad@microsoft.com>
>         Date:   Fri Nov 5 19:03:57 2021 +0000
>
>             cifs: send workstation name during ntlmssp session setup
>
>             During the ntlmssp session setup (authenticate phases)
>             send the client workstation info. This can make debugging easier on
>             servers.
>
>             Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
>             Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>             Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
>             Signed-off-by: Steve French <stfrench@microsoft.com>
>
> Unfortunately some servers did not seem to enforce it to be 16 bytes
> long, so the reason why we didn't catch it earlier.
>
> Steve, Shyam, let me know if it does make sense to you and then I can
> work on a patch to fix it properly.
ronnie sahlberg May 6, 2022, 2:19 a.m. UTC | #8
On Fri, 6 May 2022 at 11:59, Paulo Alcantara <pc@cjr.nz> wrote:
>
> Tom Talpey <tom@talpey.com> writes:
>
> > I think the most conservative and spec-compliant choice should be made.
> > SMB1 should not be pushing the envelope of interoperability, in this day
> > and age.
>
> OK.
>
> > I believe the NetBIOS name is a fixed array of 16 octets, right? So, if
> > the nodename is shorter, it needs to be padded with 0's.
>
> Right.
>
> > Did this code change recently? Why???
>
> We used to not send the WorkstationName during NTLMSSP until recent
> patch from Shyam:
>
>         commit 49bd49f983b5026e4557d31c5d737d9657c4113e
>         Author: Shyam Prasad N <sprasad@microsoft.com>
>         Date:   Fri Nov 5 19:03:57 2021 +0000
>
>             cifs: send workstation name during ntlmssp session setup
>
>             During the ntlmssp session setup (authenticate phases)
>             send the client workstation info. This can make debugging easier on
>             servers.
>
>             Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
>             Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>             Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
>             Signed-off-by: Steve French <stfrench@microsoft.com>
>
> Unfortunately some servers did not seem to enforce it to be 16 bytes
> long, so the reason why we didn't catch it earlier.
>
> Steve, Shyam, let me know if it does make sense to you and then I can
> work on a patch to fix it properly.

This regression should be easy to fix, but maybe we should not have
done the initial change in the first place.
If things is broken and do not work under SMB1, that is a good thing.
Instead of adding features or fixing
missing parts to SMB1 we should just tell people to switch to SMB2 instead.

I think if things do not work correctly or things are missing in smb1,
that is a GOOD THING.
:-)
Paulo Alcantara May 6, 2022, 2:05 p.m. UTC | #9
ronnie sahlberg <ronniesahlberg@gmail.com> writes:

> This regression should be easy to fix, but maybe we should not have
> done the initial change in the first place.
> If things is broken and do not work under SMB1, that is a good thing.
> Instead of adding features or fixing
> missing parts to SMB1 we should just tell people to switch to SMB2 instead.
>
> I think if things do not work correctly or things are missing in smb1,
> that is a GOOD THING.
> :-)

LOL - couldn't agree more :-)
Paulo Alcantara May 17, 2022, 8:37 p.m. UTC | #10
Hi Byron,

Byron Stanoszek <gandalf@winds.org> writes:

> I would like to report a regression in the CIFS fs. Sometime between Linux 4.14
> and 5.16, mounting CIFS with option vers=1.0 (and
> CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
> NTLMSSP stopped working for me. The server side is a Windows 2003 Server.
>
> I found that this behavior depends on the length of the Linux client's
> host+domain name (e.g. utsname()->nodename), where the mount works as long as
> the name is 16 characters or less. Anything 17 or above returns -EIO, per the
> following example:
>
> /etc/fstab entry:
>
> //10.0.0.12/xxxxxxxxx /ext0     cifs    vers=1.0,user=xxxxx,pass=xxxxxxxxxxx,dom=xxxxxxxxxxx,dir_mode=0755,file_mode=0644,noauto 0 0
>
> # hostname 12345678901234567;mount /ext0
> mount error(5): Input/output error
> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
> # hostname 1234567890123456;mount /ext0
> #

Could you please try below patch?

Let me know if I missed something else.  Thanks.

From bf63fb30ac90c06f45e40acbd3bbd2284d8ffffb Mon Sep 17 00:00:00 2001
From: Paulo Alcantara <pc@cjr.nz>
Date: Tue, 17 May 2022 17:23:23 -0300
Subject: [PATCH] cifs: fix ntlmssp on old servers

Some older servers seem to require the workstation name during ntlmssp
to be at most 15 chars (RFC1001 name length), so truncate it before
sending when using insecure dialects.

Link: https://lore.kernel.org/r/e6837098-15d9-acb6-7e34-1923cf8c6fe1@winds.org
Reported-by: Byron Stanoszek <gandalf@winds.org>
Fixes: 49bd49f983b5 ("cifs: send workstation name during ntlmssp session setup")
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
 fs/cifs/cifsglob.h   | 15 ++++++++++++++-
 fs/cifs/connect.c    | 22 ++++------------------
 fs/cifs/fs_context.c | 29 ++++-------------------------
 fs/cifs/fs_context.h |  2 +-
 fs/cifs/misc.c       |  1 -
 fs/cifs/sess.c       |  6 +++---
 6 files changed, 26 insertions(+), 49 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8de977c359b1..5024b6792dab 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -944,7 +944,7 @@ struct cifs_ses {
 				   and after mount option parsing we fill it */
 	char *domainName;
 	char *password;
-	char *workstation_name;
+	char workstation_name[CIFS_MAX_WORKSTATION_LEN];
 	struct session_key auth_key;
 	struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
 	enum securityEnum sectype; /* what security flavor was specified? */
@@ -1979,4 +1979,17 @@ static inline bool cifs_is_referral_server(struct cifs_tcon *tcon,
 	return is_tcon_dfs(tcon) || (ref && (ref->flags & DFSREF_REFERRAL_SERVER));
 }
 
+static inline size_t ntlmssp_workstation_name_size(const struct cifs_ses *ses)
+{
+	if (WARN_ON_ONCE(!ses || !ses->server))
+		return 0;
+	/*
+	 * Make workstation name no more than 15 chars when using insecure dialects as some legacy
+	 * servers do require it during NTLMSSP.
+	 */
+	if (ses->server->dialect <= SMB20_PROT_ID)
+		return min_t(size_t, sizeof(ses->workstation_name), RFC1001_NAME_LEN_WITH_NULL);
+	return sizeof(ses->workstation_name);
+}
+
 #endif	/* _CIFS_GLOB_H */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 42e14f408856..6ae5193fb562 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2037,18 +2037,7 @@ cifs_set_cifscreds(struct smb3_fs_context *ctx, struct cifs_ses *ses)
 		}
 	}
 
-	ctx->workstation_name = kstrdup(ses->workstation_name, GFP_KERNEL);
-	if (!ctx->workstation_name) {
-		cifs_dbg(FYI, "Unable to allocate memory for workstation_name\n");
-		rc = -ENOMEM;
-		kfree(ctx->username);
-		ctx->username = NULL;
-		kfree_sensitive(ctx->password);
-		ctx->password = NULL;
-		kfree(ctx->domainname);
-		ctx->domainname = NULL;
-		goto out_key_put;
-	}
+	strscpy(ctx->workstation_name, ses->workstation_name, sizeof(ctx->workstation_name));
 
 out_key_put:
 	up_read(&key->sem);
@@ -2157,12 +2146,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
 		if (!ses->domainName)
 			goto get_ses_fail;
 	}
-	if (ctx->workstation_name) {
-		ses->workstation_name = kstrdup(ctx->workstation_name,
-						GFP_KERNEL);
-		if (!ses->workstation_name)
-			goto get_ses_fail;
-	}
+
+	strscpy(ses->workstation_name, ctx->workstation_name, sizeof(ses->workstation_name));
+
 	if (ctx->domainauto)
 		ses->domainAuto = ctx->domainauto;
 	ses->cred_uid = ctx->cred_uid;
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index a92e9eec521f..fbb0e98c7d2c 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -312,7 +312,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
 	new_ctx->password = NULL;
 	new_ctx->server_hostname = NULL;
 	new_ctx->domainname = NULL;
-	new_ctx->workstation_name = NULL;
 	new_ctx->UNC = NULL;
 	new_ctx->source = NULL;
 	new_ctx->iocharset = NULL;
@@ -327,7 +326,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
 	DUP_CTX_STR(UNC);
 	DUP_CTX_STR(source);
 	DUP_CTX_STR(domainname);
-	DUP_CTX_STR(workstation_name);
 	DUP_CTX_STR(nodename);
 	DUP_CTX_STR(iocharset);
 
@@ -766,8 +764,7 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
 		cifs_errorf(fc, "can not change domainname during remount\n");
 		return -EINVAL;
 	}
-	if (new_ctx->workstation_name &&
-	    (!old_ctx->workstation_name || strcmp(new_ctx->workstation_name, old_ctx->workstation_name))) {
+	if (strcmp(new_ctx->workstation_name, old_ctx->workstation_name)) {
 		cifs_errorf(fc, "can not change workstation_name during remount\n");
 		return -EINVAL;
 	}
@@ -814,7 +811,6 @@ static int smb3_reconfigure(struct fs_context *fc)
 	STEAL_STRING(cifs_sb, ctx, username);
 	STEAL_STRING(cifs_sb, ctx, password);
 	STEAL_STRING(cifs_sb, ctx, domainname);
-	STEAL_STRING(cifs_sb, ctx, workstation_name);
 	STEAL_STRING(cifs_sb, ctx, nodename);
 	STEAL_STRING(cifs_sb, ctx, iocharset);
 
@@ -1467,22 +1463,15 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 
 int smb3_init_fs_context(struct fs_context *fc)
 {
-	int rc;
 	struct smb3_fs_context *ctx;
 	char *nodename = utsname()->nodename;
 	int i;
 
 	ctx = kzalloc(sizeof(struct smb3_fs_context), GFP_KERNEL);
-	if (unlikely(!ctx)) {
-		rc = -ENOMEM;
-		goto err_exit;
-	}
+	if (unlikely(!ctx))
+		return -ENOMEM;
 
-	ctx->workstation_name = kstrdup(nodename, GFP_KERNEL);
-	if (unlikely(!ctx->workstation_name)) {
-		rc = -ENOMEM;
-		goto err_exit;
-	}
+	strscpy(ctx->workstation_name, nodename, sizeof(ctx->workstation_name));
 
 	/*
 	 * does not have to be perfect mapping since field is
@@ -1555,14 +1544,6 @@ int smb3_init_fs_context(struct fs_context *fc)
 	fc->fs_private = ctx;
 	fc->ops = &smb3_fs_context_ops;
 	return 0;
-
-err_exit:
-	if (ctx) {
-		kfree(ctx->workstation_name);
-		kfree(ctx);
-	}
-
-	return rc;
 }
 
 void
@@ -1588,8 +1569,6 @@ smb3_cleanup_fs_context_contents(struct smb3_fs_context *ctx)
 	ctx->source = NULL;
 	kfree(ctx->domainname);
 	ctx->domainname = NULL;
-	kfree(ctx->workstation_name);
-	ctx->workstation_name = NULL;
 	kfree(ctx->nodename);
 	ctx->nodename = NULL;
 	kfree(ctx->iocharset);
diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
index e54090d9ef36..3a156c143925 100644
--- a/fs/cifs/fs_context.h
+++ b/fs/cifs/fs_context.h
@@ -170,7 +170,7 @@ struct smb3_fs_context {
 	char *server_hostname;
 	char *UNC;
 	char *nodename;
-	char *workstation_name;
+	char workstation_name[CIFS_MAX_WORKSTATION_LEN];
 	char *iocharset;  /* local code page for mapping to and from Unicode */
 	char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name */
 	char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name */
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index afaf59c22193..114810e563a9 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -95,7 +95,6 @@ sesInfoFree(struct cifs_ses *buf_to_free)
 	kfree_sensitive(buf_to_free->password);
 	kfree(buf_to_free->user_name);
 	kfree(buf_to_free->domainName);
-	kfree(buf_to_free->workstation_name);
 	kfree_sensitive(buf_to_free->auth_key.response);
 	kfree(buf_to_free->iface_list);
 	kfree_sensitive(buf_to_free);
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 32f478c7a66d..1a0995bb5d90 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -714,9 +714,9 @@ static int size_of_ntlmssp_blob(struct cifs_ses *ses, int base_size)
 	else
 		sz += sizeof(__le16);
 
-	if (ses->workstation_name)
+	if (ses->workstation_name[0])
 		sz += sizeof(__le16) * strnlen(ses->workstation_name,
-			CIFS_MAX_WORKSTATION_LEN);
+					       ntlmssp_workstation_name_size(ses));
 	else
 		sz += sizeof(__le16);
 
@@ -960,7 +960,7 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer,
 
 	cifs_security_buffer_from_str(&sec_blob->WorkstationName,
 				      ses->workstation_name,
-				      CIFS_MAX_WORKSTATION_LEN,
+				      ntlmssp_workstation_name_size(ses),
 				      *pbuffer, &tmp,
 				      nls_cp);
Steven French May 22, 2022, 4:40 a.m. UTC | #11
Yes - please let us know if this worked.

On 5/17/22 15:37, Paulo Alcantara wrote:
> Hi Byron,
>
> Byron Stanoszek <gandalf@winds.org> writes:
>
>> I would like to report a regression in the CIFS fs. Sometime between Linux 4.14
>> and 5.16, mounting CIFS with option vers=1.0 (and
>> CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
>> NTLMSSP stopped working for me. The server side is a Windows 2003 Server.
>>
>> I found that this behavior depends on the length of the Linux client's
>> host+domain name (e.g. utsname()->nodename), where the mount works as long as
>> the name is 16 characters or less. Anything 17 or above returns -EIO, per the
>> following example:
>>
>> /etc/fstab entry:
>>
>> //10.0.0.12/xxxxxxxxx /ext0     cifs    vers=1.0,user=xxxxx,pass=xxxxxxxxxxx,dom=xxxxxxxxxxx,dir_mode=0755,file_mode=0644,noauto 0 0
>>
>> # hostname 12345678901234567;mount /ext0
>> mount error(5): Input/output error
>> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
>> # hostname 1234567890123456;mount /ext0
>> #
> Could you please try below patch?
>
> Let me know if I missed something else.  Thanks.
>
>  From bf63fb30ac90c06f45e40acbd3bbd2284d8ffffb Mon Sep 17 00:00:00 2001
> From: Paulo Alcantara <pc@cjr.nz>
> Date: Tue, 17 May 2022 17:23:23 -0300
> Subject: [PATCH] cifs: fix ntlmssp on old servers
>
> Some older servers seem to require the workstation name during ntlmssp
> to be at most 15 chars (RFC1001 name length), so truncate it before
> sending when using insecure dialects.
>
> Link: https://lore.kernel.org/r/e6837098-15d9-acb6-7e34-1923cf8c6fe1@winds.org
> Reported-by: Byron Stanoszek <gandalf@winds.org>
> Fixes: 49bd49f983b5 ("cifs: send workstation name during ntlmssp session setup")
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>   fs/cifs/cifsglob.h   | 15 ++++++++++++++-
>   fs/cifs/connect.c    | 22 ++++------------------
>   fs/cifs/fs_context.c | 29 ++++-------------------------
>   fs/cifs/fs_context.h |  2 +-
>   fs/cifs/misc.c       |  1 -
>   fs/cifs/sess.c       |  6 +++---
>   6 files changed, 26 insertions(+), 49 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 8de977c359b1..5024b6792dab 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -944,7 +944,7 @@ struct cifs_ses {
>   				   and after mount option parsing we fill it */
>   	char *domainName;
>   	char *password;
> -	char *workstation_name;
> +	char workstation_name[CIFS_MAX_WORKSTATION_LEN];
>   	struct session_key auth_key;
>   	struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
>   	enum securityEnum sectype; /* what security flavor was specified? */
> @@ -1979,4 +1979,17 @@ static inline bool cifs_is_referral_server(struct cifs_tcon *tcon,
>   	return is_tcon_dfs(tcon) || (ref && (ref->flags & DFSREF_REFERRAL_SERVER));
>   }
>   
> +static inline size_t ntlmssp_workstation_name_size(const struct cifs_ses *ses)
> +{
> +	if (WARN_ON_ONCE(!ses || !ses->server))
> +		return 0;
> +	/*
> +	 * Make workstation name no more than 15 chars when using insecure dialects as some legacy
> +	 * servers do require it during NTLMSSP.
> +	 */
> +	if (ses->server->dialect <= SMB20_PROT_ID)
> +		return min_t(size_t, sizeof(ses->workstation_name), RFC1001_NAME_LEN_WITH_NULL);
> +	return sizeof(ses->workstation_name);
> +}
> +
>   #endif	/* _CIFS_GLOB_H */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 42e14f408856..6ae5193fb562 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2037,18 +2037,7 @@ cifs_set_cifscreds(struct smb3_fs_context *ctx, struct cifs_ses *ses)
>   		}
>   	}
>   
> -	ctx->workstation_name = kstrdup(ses->workstation_name, GFP_KERNEL);
> -	if (!ctx->workstation_name) {
> -		cifs_dbg(FYI, "Unable to allocate memory for workstation_name\n");
> -		rc = -ENOMEM;
> -		kfree(ctx->username);
> -		ctx->username = NULL;
> -		kfree_sensitive(ctx->password);
> -		ctx->password = NULL;
> -		kfree(ctx->domainname);
> -		ctx->domainname = NULL;
> -		goto out_key_put;
> -	}
> +	strscpy(ctx->workstation_name, ses->workstation_name, sizeof(ctx->workstation_name));
>   
>   out_key_put:
>   	up_read(&key->sem);
> @@ -2157,12 +2146,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
>   		if (!ses->domainName)
>   			goto get_ses_fail;
>   	}
> -	if (ctx->workstation_name) {
> -		ses->workstation_name = kstrdup(ctx->workstation_name,
> -						GFP_KERNEL);
> -		if (!ses->workstation_name)
> -			goto get_ses_fail;
> -	}
> +
> +	strscpy(ses->workstation_name, ctx->workstation_name, sizeof(ses->workstation_name));
> +
>   	if (ctx->domainauto)
>   		ses->domainAuto = ctx->domainauto;
>   	ses->cred_uid = ctx->cred_uid;
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index a92e9eec521f..fbb0e98c7d2c 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -312,7 +312,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
>   	new_ctx->password = NULL;
>   	new_ctx->server_hostname = NULL;
>   	new_ctx->domainname = NULL;
> -	new_ctx->workstation_name = NULL;
>   	new_ctx->UNC = NULL;
>   	new_ctx->source = NULL;
>   	new_ctx->iocharset = NULL;
> @@ -327,7 +326,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx
>   	DUP_CTX_STR(UNC);
>   	DUP_CTX_STR(source);
>   	DUP_CTX_STR(domainname);
> -	DUP_CTX_STR(workstation_name);
>   	DUP_CTX_STR(nodename);
>   	DUP_CTX_STR(iocharset);
>   
> @@ -766,8 +764,7 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
>   		cifs_errorf(fc, "can not change domainname during remount\n");
>   		return -EINVAL;
>   	}
> -	if (new_ctx->workstation_name &&
> -	    (!old_ctx->workstation_name || strcmp(new_ctx->workstation_name, old_ctx->workstation_name))) {
> +	if (strcmp(new_ctx->workstation_name, old_ctx->workstation_name)) {
>   		cifs_errorf(fc, "can not change workstation_name during remount\n");
>   		return -EINVAL;
>   	}
> @@ -814,7 +811,6 @@ static int smb3_reconfigure(struct fs_context *fc)
>   	STEAL_STRING(cifs_sb, ctx, username);
>   	STEAL_STRING(cifs_sb, ctx, password);
>   	STEAL_STRING(cifs_sb, ctx, domainname);
> -	STEAL_STRING(cifs_sb, ctx, workstation_name);
>   	STEAL_STRING(cifs_sb, ctx, nodename);
>   	STEAL_STRING(cifs_sb, ctx, iocharset);
>   
> @@ -1467,22 +1463,15 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
>   
>   int smb3_init_fs_context(struct fs_context *fc)
>   {
> -	int rc;
>   	struct smb3_fs_context *ctx;
>   	char *nodename = utsname()->nodename;
>   	int i;
>   
>   	ctx = kzalloc(sizeof(struct smb3_fs_context), GFP_KERNEL);
> -	if (unlikely(!ctx)) {
> -		rc = -ENOMEM;
> -		goto err_exit;
> -	}
> +	if (unlikely(!ctx))
> +		return -ENOMEM;
>   
> -	ctx->workstation_name = kstrdup(nodename, GFP_KERNEL);
> -	if (unlikely(!ctx->workstation_name)) {
> -		rc = -ENOMEM;
> -		goto err_exit;
> -	}
> +	strscpy(ctx->workstation_name, nodename, sizeof(ctx->workstation_name));
>   
>   	/*
>   	 * does not have to be perfect mapping since field is
> @@ -1555,14 +1544,6 @@ int smb3_init_fs_context(struct fs_context *fc)
>   	fc->fs_private = ctx;
>   	fc->ops = &smb3_fs_context_ops;
>   	return 0;
> -
> -err_exit:
> -	if (ctx) {
> -		kfree(ctx->workstation_name);
> -		kfree(ctx);
> -	}
> -
> -	return rc;
>   }
>   
>   void
> @@ -1588,8 +1569,6 @@ smb3_cleanup_fs_context_contents(struct smb3_fs_context *ctx)
>   	ctx->source = NULL;
>   	kfree(ctx->domainname);
>   	ctx->domainname = NULL;
> -	kfree(ctx->workstation_name);
> -	ctx->workstation_name = NULL;
>   	kfree(ctx->nodename);
>   	ctx->nodename = NULL;
>   	kfree(ctx->iocharset);
> diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
> index e54090d9ef36..3a156c143925 100644
> --- a/fs/cifs/fs_context.h
> +++ b/fs/cifs/fs_context.h
> @@ -170,7 +170,7 @@ struct smb3_fs_context {
>   	char *server_hostname;
>   	char *UNC;
>   	char *nodename;
> -	char *workstation_name;
> +	char workstation_name[CIFS_MAX_WORKSTATION_LEN];
>   	char *iocharset;  /* local code page for mapping to and from Unicode */
>   	char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name */
>   	char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name */
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index afaf59c22193..114810e563a9 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -95,7 +95,6 @@ sesInfoFree(struct cifs_ses *buf_to_free)
>   	kfree_sensitive(buf_to_free->password);
>   	kfree(buf_to_free->user_name);
>   	kfree(buf_to_free->domainName);
> -	kfree(buf_to_free->workstation_name);
>   	kfree_sensitive(buf_to_free->auth_key.response);
>   	kfree(buf_to_free->iface_list);
>   	kfree_sensitive(buf_to_free);
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 32f478c7a66d..1a0995bb5d90 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -714,9 +714,9 @@ static int size_of_ntlmssp_blob(struct cifs_ses *ses, int base_size)
>   	else
>   		sz += sizeof(__le16);
>   
> -	if (ses->workstation_name)
> +	if (ses->workstation_name[0])
>   		sz += sizeof(__le16) * strnlen(ses->workstation_name,
> -			CIFS_MAX_WORKSTATION_LEN);
> +					       ntlmssp_workstation_name_size(ses));
>   	else
>   		sz += sizeof(__le16);
>   
> @@ -960,7 +960,7 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer,
>   
>   	cifs_security_buffer_from_str(&sec_blob->WorkstationName,
>   				      ses->workstation_name,
> -				      CIFS_MAX_WORKSTATION_LEN,
> +				      ntlmssp_workstation_name_size(ses),
>   				      *pbuffer, &tmp,
>   				      nls_cp);
>
Byron Stanoszek May 23, 2022, 2:32 a.m. UTC | #12
On Sat, 21 May 2022, Steven French wrote:

> Yes - please let us know if this worked.

I was on a business trip all last week and didn't get the email till now. I'll
try this out first thing tomorrow morning.

Thanks,
  -Byron

>
> On 5/17/22 15:37, Paulo Alcantara wrote:
>>  Hi Byron,
>>
>>  Byron Stanoszek <gandalf@winds.org> writes:
>>
>>>  I would like to report a regression in the CIFS fs. Sometime between
>>>  Linux 4.14
>>>  and 5.16, mounting CIFS with option vers=1.0 (and
>>>  CONFIG_CIFS_ALLOW_INSECURE_LEGACY=y set appropriately) with security type
>>>  NTLMSSP stopped working for me. The server side is a Windows 2003 Server.
>>>
>>>  I found that this behavior depends on the length of the Linux client's
>>>  host+domain name (e.g. utsname()->nodename), where the mount works as
>>>  long as
>>>  the name is 16 characters or less. Anything 17 or above returns -EIO, per
>>>  the
>>>  following example:
>>>
>>>  /etc/fstab entry:
>>>
>>>  //10.0.0.12/xxxxxxxxx /ext0     cifs
>>>  vers=1.0,user=xxxxx,pass=xxxxxxxxxxx,dom=xxxxxxxxxxx,dir_mode=0755,file_mode=0644,noauto
>>>  0 0
>>>
>>>  # hostname 12345678901234567;mount /ext0
>>>  mount error(5): Input/output error
>>>  Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel
>>>  log messages (dmesg)
>>> #  hostname 1234567890123456;mount /ext0
>>> #
>>  Could you please try below patch?
>>
>>  Let me know if I missed something else.  Thanks.
>>
>>  From bf63fb30ac90c06f45e40acbd3bbd2284d8ffffb Mon Sep 17 00:00:00 2001
>>  From: Paulo Alcantara <pc@cjr.nz>
>>  Date: Tue, 17 May 2022 17:23:23 -0300
>>  Subject: [PATCH] cifs: fix ntlmssp on old servers
>>
>>  Some older servers seem to require the workstation name during ntlmssp
>>  to be at most 15 chars (RFC1001 name length), so truncate it before
>>  sending when using insecure dialects.
>>
>>  Link:
>>  https://lore.kernel.org/r/e6837098-15d9-acb6-7e34-1923cf8c6fe1@winds.org
>>  Reported-by: Byron Stanoszek <gandalf@winds.org>
>>  Fixes: 49bd49f983b5 ("cifs: send workstation name during ntlmssp session
>>  setup")
>>  Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>>  ---
>>    fs/cifs/cifsglob.h   | 15 ++++++++++++++-
>>    fs/cifs/connect.c    | 22 ++++------------------
>>    fs/cifs/fs_context.c | 29 ++++-------------------------
>>    fs/cifs/fs_context.h |  2 +-
>>    fs/cifs/misc.c       |  1 -
>>    fs/cifs/sess.c       |  6 +++---
>>    6 files changed, 26 insertions(+), 49 deletions(-)
>>
>>  diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>  index 8de977c359b1..5024b6792dab 100644
>>  --- a/fs/cifs/cifsglob.h
>>  +++ b/fs/cifs/cifsglob.h
>>  @@ -944,7 +944,7 @@ struct cifs_ses {
>>     			   and after mount option parsing we fill it */
>>     char *domainName;
>>     char *password;
>>  -	char *workstation_name;
>>  +	char workstation_name[CIFS_MAX_WORKSTATION_LEN];
>>     struct session_key auth_key;
>>     struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
>>     enum securityEnum sectype; /* what security flavor was specified? */
>>  @@ -1979,4 +1979,17 @@ static inline bool cifs_is_referral_server(struct
>>  cifs_tcon *tcon,
>>    	return is_tcon_dfs(tcon) || (ref && (ref->flags &
>>    DFSREF_REFERRAL_SERVER));
>>    }
>>
>>  +static inline size_t ntlmssp_workstation_name_size(const struct cifs_ses
>>  *ses)
>>  +{
>>  +	if (WARN_ON_ONCE(!ses || !ses->server))
>>  +		return 0;
>>  +	/*
>>  +	 * Make workstation name no more than 15 chars when using insecure
>>  dialects as some legacy
>>  +	 * servers do require it during NTLMSSP.
>>  +	 */
>>  +	if (ses->server->dialect <= SMB20_PROT_ID)
>>  +		return min_t(size_t, sizeof(ses->workstation_name),
>>  RFC1001_NAME_LEN_WITH_NULL);
>>  +	return sizeof(ses->workstation_name);
>>  +}
>>  +
>>    #endif	/* _CIFS_GLOB_H */
>>  diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>  index 42e14f408856..6ae5193fb562 100644
>>  --- a/fs/cifs/connect.c
>>  +++ b/fs/cifs/connect.c
>>  @@ -2037,18 +2037,7 @@ cifs_set_cifscreds(struct smb3_fs_context *ctx,
>>  struct cifs_ses *ses)
>>     	}
>>     }
>>    -	ctx->workstation_name = kstrdup(ses->workstation_name, GFP_KERNEL);
>>  -	if (!ctx->workstation_name) {
>>  -		cifs_dbg(FYI, "Unable to allocate memory for
>>  workstation_name\n");
>>  -		rc = -ENOMEM;
>>  -		kfree(ctx->username);
>>  -		ctx->username = NULL;
>>  -		kfree_sensitive(ctx->password);
>>  -		ctx->password = NULL;
>>  -		kfree(ctx->domainname);
>>  -		ctx->domainname = NULL;
>>  -		goto out_key_put;
>>  -	}
>>  +	strscpy(ctx->workstation_name, ses->workstation_name,
>>  sizeof(ctx->workstation_name));
>>
>>    out_key_put:
>>    	up_read(&key->sem);
>>  @@ -2157,12 +2146,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server,
>>  struct smb3_fs_context *ctx)
>>      if (!ses->domainName)
>>     		goto get_ses_fail;
>>    	}
>>  -	if (ctx->workstation_name) {
>>  -		ses->workstation_name = kstrdup(ctx->workstation_name,
>>  -						GFP_KERNEL);
>>  -		if (!ses->workstation_name)
>>  -			goto get_ses_fail;
>>  -	}
>>  +
>>  +	strscpy(ses->workstation_name, ctx->workstation_name,
>>  sizeof(ses->workstation_name));
>>  +
>>     if (ctx->domainauto)
>>     	ses->domainAuto = ctx->domainauto;
>>    	ses->cred_uid = ctx->cred_uid;
>>  diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
>>  index a92e9eec521f..fbb0e98c7d2c 100644
>>  --- a/fs/cifs/fs_context.c
>>  +++ b/fs/cifs/fs_context.c
>>  @@ -312,7 +312,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx,
>>  struct smb3_fs_context *ctx
>>     new_ctx->password = NULL;
>>     new_ctx->server_hostname = NULL;
>>     new_ctx->domainname = NULL;
>>  -	new_ctx->workstation_name = NULL;
>>     new_ctx->UNC = NULL;
>>     new_ctx->source = NULL;
>>     new_ctx->iocharset = NULL;
>>  @@ -327,7 +326,6 @@ smb3_fs_context_dup(struct smb3_fs_context *new_ctx,
>>  struct smb3_fs_context *ctx
>>     DUP_CTX_STR(UNC);
>>     DUP_CTX_STR(source);
>>     DUP_CTX_STR(domainname);
>>  -	DUP_CTX_STR(workstation_name);
>>     DUP_CTX_STR(nodename);
>>     DUP_CTX_STR(iocharset);
>>    @@ -766,8 +764,7 @@ static int smb3_verify_reconfigure_ctx(struct
>>  fs_context *fc,
>>      cifs_errorf(fc, "can not change domainname during remount\n");
>>      return -EINVAL;
>>    	}
>>  -	if (new_ctx->workstation_name &&
>>  -	    (!old_ctx->workstation_name || strcmp(new_ctx->workstation_name,
>>  old_ctx->workstation_name))) {
>>  +	if (strcmp(new_ctx->workstation_name, old_ctx->workstation_name)) {
>>      cifs_errorf(fc, "can not change workstation_name during remount\n");
>>      return -EINVAL;
>>    	}
>>  @@ -814,7 +811,6 @@ static int smb3_reconfigure(struct fs_context *fc)
>>     STEAL_STRING(cifs_sb, ctx, username);
>>     STEAL_STRING(cifs_sb, ctx, password);
>>     STEAL_STRING(cifs_sb, ctx, domainname);
>>  -	STEAL_STRING(cifs_sb, ctx, workstation_name);
>>     STEAL_STRING(cifs_sb, ctx, nodename);
>>     STEAL_STRING(cifs_sb, ctx, iocharset);
>>    @@ -1467,22 +1463,15 @@ static int smb3_fs_context_parse_param(struct
>>  fs_context *fc,
>>
>>    int smb3_init_fs_context(struct fs_context *fc)
>>    {
>>  -	int rc;
>>     struct smb3_fs_context *ctx;
>>     char *nodename = utsname()->nodename;
>>     int i;
>>
>>    	ctx = kzalloc(sizeof(struct smb3_fs_context), GFP_KERNEL);
>>  -	if (unlikely(!ctx)) {
>>  -		rc = -ENOMEM;
>>  -		goto err_exit;
>>  -	}
>>  +	if (unlikely(!ctx))
>>  +		return -ENOMEM;
>>    -	ctx->workstation_name = kstrdup(nodename, GFP_KERNEL);
>>  -	if (unlikely(!ctx->workstation_name)) {
>>  -		rc = -ENOMEM;
>>  -		goto err_exit;
>>  -	}
>>  +	strscpy(ctx->workstation_name, nodename,
>>  sizeof(ctx->workstation_name));
>>
>>     /*
>>    	 * does not have to be perfect mapping since field is
>>  @@ -1555,14 +1544,6 @@ int smb3_init_fs_context(struct fs_context *fc)
>>     fc->fs_private = ctx;
>>     fc->ops = &smb3_fs_context_ops;
>>     return 0;
>>  -
>>  -err_exit:
>>  -	if (ctx) {
>>  -		kfree(ctx->workstation_name);
>>  -		kfree(ctx);
>>  -	}
>>  -
>>  -	return rc;
>>    }
>>
>>    void
>>  @@ -1588,8 +1569,6 @@ smb3_cleanup_fs_context_contents(struct
>>  smb3_fs_context *ctx)
>>     ctx->source = NULL;
>>     kfree(ctx->domainname);
>>     ctx->domainname = NULL;
>>  -	kfree(ctx->workstation_name);
>>  -	ctx->workstation_name = NULL;
>>     kfree(ctx->nodename);
>>     ctx->nodename = NULL;
>>     kfree(ctx->iocharset);
>>  diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
>>  index e54090d9ef36..3a156c143925 100644
>>  --- a/fs/cifs/fs_context.h
>>  +++ b/fs/cifs/fs_context.h
>>  @@ -170,7 +170,7 @@ struct smb3_fs_context {
>>     char *server_hostname;
>>     char *UNC;
>>     char *nodename;
>>  -	char *workstation_name;
>>  +	char workstation_name[CIFS_MAX_WORKSTATION_LEN];
>>     char *iocharset;  /* local code page for mapping to and from Unicode */
>>     char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name
>>     */
>>     char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name
>>     */
>>  diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
>>  index afaf59c22193..114810e563a9 100644
>>  --- a/fs/cifs/misc.c
>>  +++ b/fs/cifs/misc.c
>>  @@ -95,7 +95,6 @@ sesInfoFree(struct cifs_ses *buf_to_free)
>>     kfree_sensitive(buf_to_free->password);
>>     kfree(buf_to_free->user_name);
>>     kfree(buf_to_free->domainName);
>>  -	kfree(buf_to_free->workstation_name);
>>     kfree_sensitive(buf_to_free->auth_key.response);
>>     kfree(buf_to_free->iface_list);
>>     kfree_sensitive(buf_to_free);
>>  diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
>>  index 32f478c7a66d..1a0995bb5d90 100644
>>  --- a/fs/cifs/sess.c
>>  +++ b/fs/cifs/sess.c
>>  @@ -714,9 +714,9 @@ static int size_of_ntlmssp_blob(struct cifs_ses *ses,
>>  int base_size)
>>     else
>>      sz += sizeof(__le16);
>>    -	if (ses->workstation_name)
>>  +	if (ses->workstation_name[0])
>>    		sz += sizeof(__le16) * strnlen(ses->workstation_name,
>>  -			CIFS_MAX_WORKSTATION_LEN);
>>  +
>>  ntlmssp_workstation_name_size(ses));
>>     else
>>      sz += sizeof(__le16);
>>    @@ -960,7 +960,7 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer,
>>
>>     cifs_security_buffer_from_str(&sec_blob->WorkstationName,
>>    				      ses->workstation_name,
>>  -				      CIFS_MAX_WORKSTATION_LEN,
>>  +				      ntlmssp_workstation_name_size(ses),
>>              *pbuffer, &tmp,
>>              nls_cp);
>> 
>
Byron Stanoszek May 25, 2022, 3:17 a.m. UTC | #13
On Tue, 17 May 2022, Paulo Alcantara wrote:

> Could you please try below patch?
>
> Let me know if I missed something else.  Thanks.
>
>> From bf63fb30ac90c06f45e40acbd3bbd2284d8ffffb Mon Sep 17 00:00:00 2001
> From: Paulo Alcantara <pc@cjr.nz>
> Date: Tue, 17 May 2022 17:23:23 -0300
> Subject: [PATCH] cifs: fix ntlmssp on old servers
>
> Some older servers seem to require the workstation name during ntlmssp
> to be at most 15 chars (RFC1001 name length), so truncate it before
> sending when using insecure dialects.
>
> Link: https://lore.kernel.org/r/e6837098-15d9-acb6-7e34-1923cf8c6fe1@winds.org
> Reported-by: Byron Stanoszek <gandalf@winds.org>
> Fixes: 49bd49f983b5 ("cifs: send workstation name during ntlmssp session setup")
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
> fs/cifs/cifsglob.h   | 15 ++++++++++++++-
> fs/cifs/connect.c    | 22 ++++------------------
> fs/cifs/fs_context.c | 29 ++++-------------------------
> fs/cifs/fs_context.h |  2 +-
> fs/cifs/misc.c       |  1 -
> fs/cifs/sess.c       |  6 +++---
> 6 files changed, 26 insertions(+), 49 deletions(-)

Hi Paulo,

I confirm that the patch worked for me (against Linux 5.16.13).

Regards,
  -Byron
diff mbox series

Patch

--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -101,7 +101,7 @@ 
  #define XATTR_DOS_ATTRIB "user.DOSATTRIB"
  #endif

-#define CIFS_MAX_WORKSTATION_LEN  (__NEW_UTS_LEN + 1)  /* reasonable max for client */
+#define CIFS_MAX_WORKSTATION_LEN 16

  /*
   * CIFS vfs client Status information (based on what we know.)