LinuxLists.cc - Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>
>>>> ------------------
>>>>
>>>> From: Steve French <[email protected]>
>>>>
>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>
>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>
>>>> See kernel bugzilla bug 197311
>>>>
>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>> Signed-off-by: Steve French <[email protected]>
>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>
>>>> ---
>>>> fs/cifs/smb2pdu.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> --- a/fs/cifs/smb2pdu.c
>>>> +++ b/fs/cifs/smb2pdu.c
>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>> } else
>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>> cifs_small_buf_release(req);
>>>>
>>>>
>>>>
>>>
>>> This one needs to be backported to all stable kernels as the commit that
>>> introduced the regression:
>>> '
>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>
>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>
>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>> apply it :)
>>
>> Can you provide me with a working backport?
>>
>
> Hi Steve,
>
> Is there a version of this fix available for stable kernels?
>

Any thoughts on this?

Regards,
Srivatsa

> I tried applying this patch to 4.4.109 (and a similar one for 4.9.74),
> but it didn't fix the problem. Instead, I actually got a NULL pointer
> dereference when I tried to mount an SMB3 share.
>
> Here is the patch I tried on 4.4.109:
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index f2ff60e..3963bd2 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1559,6 +1559,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> } else
> iov[0].iov_len = get_rfc1002_length(req) + 4;
>
> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> + req->hdr.Flags |= SMB2_FLAGS_SIGNED;
>
> rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
> rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
>
>
> This results in the following NULL pointer dereference when I try
> mounting:
>
> # mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
>
> [ 53.073057] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
> [ 53.073511] IP: [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0
> [ 53.073973] PGD 0
> [ 53.074427] Oops: 0000 [#1] SMP
> [ 53.074946] Modules linked in: arc4(E) ecb(E) md4(E) cifs(E) dns_resolver(E) vmw_vsock_vmci_transport(E) vsock(E) hid_generic(E) usbhid(E) hid(E) xt_conntrack(E) mousedev(E) iptable_nat(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) nf_nat_ipv4(E) nf_nat(E) iptable_filter(E) ip_tables(E) crc32c_intel(E) xt_LOG(E) nf_conntrack(E) jitterentropy_rng(E) hmac(E) sha256_ssse3(E) sha256_generic(E) drbg(E) vmw_balloon(E) ansi_cprng(E) aesni_intel(E) aes_x86_64(E) glue_helper(E) lrw(E) gf128mul(E) ablk_helper(E) cryptd(E) psmouse(E) evdev(E) uhci_hcd(E) ehci_pci(E) ehci_hcd(E) usbcore(E) intel_agp(E) usb_common(E) vmw_vmci(E) i2c_piix4(E) intel_gtt(E) nfit(E) battery(E) tpm_tis(E) tpm(E) ac(E) button(E) sch_fq_codel(E) autofs4(E)
> [ 53.079435] CPU: 3 PID: 829 Comm: mount.cifs Tainted: G E 4.4.109-possible-fix1+ #21
> [ 53.079983] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016
> [ 53.081086] task: ffff8800b4f41940 ti: ffff8800b92ac000 task.ti: ffff8800b92ac000
> [ 53.081667] RIP: 0010:[<ffffffff8138ee9a>] [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0
> [ 53.082247] RSP: 0018:ffff8800b92af9a8 EFLAGS: 00010282
> [ 53.082604] systemd-journald[284]: Compressed data object 721 -> 468 using XZ
> [ 53.083419] RAX: ffff8800af5943c0 RBX: ffff8800b484a800 RCX: 00000000ffff0ec7
> [ 53.084001] RDX: 0000000000000010 RSI: ffff8800b900af18 RDI: 0000000000000000
> [ 53.084602] RBP: ffff8800b92af9e0 R08: ffff8800b92afb64 R09: 0000000000000000
> [ 53.085184] R10: 3031322e3030312e R11: 00000000000007f5 R12: 0000000000000002
> [ 53.085755] R13: 0000000000000000 R14: ffff8800b900af18 R15: 0000000000000010
> [ 53.086333] FS: 00007fb659b45740(0000) GS:ffff88013fcc0000(0000) knlGS:0000000000000000
> [ 53.086907] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 53.087480] CR2: 0000000000000050 CR3: 00000000b7970000 CR4: 00000000001606e0
> [ 53.088107] Stack:
> [ 53.088681] ffff8800bba5d8c0 ffff8800b92afa08 ffff8800b484a800 0000000000000002
> [ 53.089281] ffff8800b92afac8 000008012400007d ffff8800b484a800 ffff8800b92afa50
> [ 53.089871] ffffffffa02194a6 ffff8800b92afb70 ffff8800af5943c0 ffff8800b7a2f800
> [ 53.090457] Call Trace:
> [ 53.091054] [<ffffffffa02194a6>] smb3_calc_signature+0xb6/0x290 [cifs]
> [ 53.091650] [<ffffffffa0218b5b>] smb2_sign_rqst+0x2b/0x40 [cifs]
> [ 53.092244] [<ffffffffa0219981>] smb2_setup_request+0xd1/0x170 [cifs]
> [ 53.092838] [<ffffffffa02082a7>] SendReceive2+0xc7/0x450 [cifs]
> [ 53.093435] [<ffffffffa02057d5>] ? cifs_small_buf_get+0x15/0x30 [cifs]
> [ 53.094030] [<ffffffffa021b83f>] ? small_smb2_init+0xdf/0x200 [cifs]
> [ 53.094616] [<ffffffffa021d6d7>] SMB2_ioctl+0x147/0x310 [cifs]
> [ 53.095203] [<ffffffffa021d99e>] smb3_validate_negotiate+0xfe/0x2d0 [cifs]
> [ 53.095792] [<ffffffffa021b196>] SMB2_tcon+0x296/0x500 [cifs]
> [ 53.096362] [<ffffffff817d7b49>] ? _raw_spin_unlock_irqrestore+0x9/0x10
> [ 53.096930] [<ffffffffa01efe0b>] cifs_get_tcon+0x1bb/0x560 [cifs]
> [ 53.097486] [<ffffffffa01f2b10>] cifs_mount+0x690/0xde0 [cifs]
> [ 53.098032] [<ffffffff817d7b49>] ? _raw_spin_unlock_irqrestore+0x9/0x10
> [ 53.098570] [<ffffffffa01de6eb>] cifs_do_mount+0xcb/0x5a0 [cifs]
> [ 53.099089] [<ffffffff81193ef7>] ? alloc_pages_current+0x87/0x110
> [ 53.099598] [<ffffffff811b7b03>] mount_fs+0x33/0x160
> [ 53.100091] [<ffffffff811d1b62>] vfs_kern_mount+0x62/0x100
> [ 53.100574] [<ffffffff811d3f1b>] do_mount+0x21b/0xd30
> [ 53.101050] [<ffffffff81193ef7>] ? alloc_pages_current+0x87/0x110
> [ 53.101511] [<ffffffff811d4d47>] SyS_mount+0x87/0xd0
> [ 53.101959] [<ffffffff817d806e>] entry_SYSCALL_64_fastpath+0x12/0x71
> [ 53.102400] Code: 89 e5 8b 12 e8 78 d2 04 00 31 c0 5d c3 0f 1f 40 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fd 53 49 89 f6 41 89 d7 48 83 ec 10 <4c> 8b 67 50 41 8b 5c 24 2c 48 85 de 75 14 41 ff 54 24 e8 48 83
> [ 53.103820] RIP [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0
> [ 53.104288] RSP <ffff8800b92af9a8>
> [ 53.104745] CR2: 0000000000000050
> [ 53.105225] ---[ end trace fc2de0ad7f229314 ]---
>
>
> The CIFS config options enabled are:
>
> CONFIG_CIFS=m
> CONFIG_CIFS_STATS=y
> CONFIG_CIFS_STATS2=y
> CONFIG_CIFS_WEAK_PW_HASH=y
> CONFIG_CIFS_UPCALL=y
> CONFIG_CIFS_XATTR=y
> CONFIG_CIFS_POSIX=y
> CONFIG_CIFS_ACL=y
> CONFIG_CIFS_DEBUG=y
> CONFIG_CIFS_DEBUG2=y
> CONFIG_CIFS_DFS_UPCALL=y
> CONFIG_CIFS_SMB2=y
> # CONFIG_CIFS_SMB311 is not set
> # CONFIG_CIFS_FSCACHE is not set
>
>
> The problem seems to be that crypto_shash_setkey() is called without
> calling smb3_crypto_shash_allocate() first. So I looked up how mainline
> avoids this issue, and it looks like the following commit makes a call
> to generate_signingkey() even when server->sign == false, and this path
> eventually calls smb3_crytpto_shash_allocate()), thus avoiding the NULL
> pointer dereference.
>
> cabfb3680f78 (CIFS: Enable encryption during session setup phase)
>
>
> So, I adopted this change, and now my resulting patch looks like this:
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index f2ff60e..19cc92c 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -833,7 +833,7 @@ ssetup_exit:
>
> if (!rc) {
> mutex_lock(&server->srv_mutex);
> - if (server->sign && server->ops->generate_signingkey) {
> + if (server->ops->generate_signingkey) {
> rc = server->ops->generate_signingkey(ses);
> kfree(ses->auth_key.response);
> ses->auth_key.response = NULL;
> @@ -1559,6 +1559,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> } else
> iov[0].iov_len = get_rfc1002_length(req) + 4;
>
> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> + req->hdr.Flags |= SMB2_FLAGS_SIGNED;
>
> rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
> rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
>
>
>
> This fixes the NULL pointer dereference, but the mount still fails, but
> this time for a different reason -- due to STATUS_ACCESS_DENIED:
>
>
> # mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
>
> mount.cifs kernel mount options: ip=<ip_addr>,unc=\\<ip_addr>\TestSMB,vers=3.0,user=srivatsa,pass=********
> mount error(5): Input/output error
> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
>
> Here is the dmesg output:
>
> [ 48.192141] fs/cifs/cifsfs.c: Devname: //<ip_addr>/TestSMB/ flags: 0
> [ 48.192178] address conversion returned 1 for <ip_addr>
> [ 48.192205] fs/cifs/connect.c: Username: srivatsa
> [ 48.192222] fs/cifs/connect.c: file mode: 0x1ed dir mode: 0x1ed
> [ 48.192280] fs/cifs/connect.c: CIFS VFS: in cifs_mount as Xid: 0 with uid: 0
> [ 48.192302] fs/cifs/connect.c: UNC: \\<ip_addr>\TestSMB
> [ 48.192335] fs/cifs/connect.c: Socket created
> [ 48.192349] fs/cifs/connect.c: sndbuf 16384 rcvbuf 87380 rcvtimeo 0x6d6
> [ 48.193453] fs/cifs/connect.c: CIFS VFS: in cifs_get_smb_ses as Xid: 1 with uid: 0
> [ 48.193462] fs/cifs/connect.c: Demultiplex PID: 829
> [ 48.193492] fs/cifs/connect.c: Existing smb sess not found
> [ 48.193510] fs/cifs/smb2pdu.c: Negotiate protocol
> [ 48.193531] fs/cifs/transport.c: Sending smb: smb_len=102
> [ 48.194301] fs/cifs/connect.c: RFC1002 header 0xaa
> [ 48.194321] fs/cifs/smb2misc.c: smb2_check_message length: 0xae, smb_buf_length: 0xaa
> [ 48.194349] fs/cifs/smb2misc.c: SMB2 data length 42 offset 128
> [ 48.194367] fs/cifs/smb2misc.c: SMB2 len 174
> [ 48.194393] fs/cifs/transport.c: cifs_sync_mid_result: cmd=0 mid=0 state=4
> [ 48.194415] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> [ 48.194436] fs/cifs/smb2pdu.c: mode 0x1
> [ 48.194448] fs/cifs/smb2pdu.c: negotiated smb3.0 dialect
> [ 48.194466] fs/cifs/asn1.c: OID len = 10 oid = 0x1 0x3 0x6 0x1
> [ 48.194484] fs/cifs/asn1.c: OID len = 10 oid = 0x1 0x3 0x6 0x1
> [ 48.194502] fs/cifs/connect.c: Security Mode: 0x1 Capabilities: 0x300007 TimeAdjust: 0
> [ 48.194525] fs/cifs/smb2pdu.c: Session Setup
> [ 48.194539] fs/cifs/transport.c: Sending smb: smb_len=120
> [ 48.194817] fs/cifs/connect.c: RFC1002 header 0x136
> [ 48.194836] fs/cifs/smb2misc.c: smb2_check_message length: 0x13a, smb_buf_length: 0x136
> [ 48.194859] fs/cifs/smb2misc.c: SMB2 data length 238 offset 72
> [ 48.195306] fs/cifs/smb2misc.c: SMB2 len 314
> [ 48.195740] fs/cifs/transport.c: cifs_sync_mid_result: cmd=1 mid=1 state=4
> [ 48.196174] Status code returned 0xc0000016 STATUS_MORE_PROCESSING_REQUIRED
> [ 48.196605] fs/cifs/smb2maperror.c: Mapping SMB2 status code -1073741802 to POSIX err -5
> [ 48.197043] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> [ 48.210008] fs/cifs/transport.c: Sending smb: smb_len=412
> [ 48.211625] fs/cifs/connect.c: RFC1002 header 0x48
> [ 48.212060] fs/cifs/smb2misc.c: smb2_check_message length: 0x4c, smb_buf_length: 0x48
> [ 48.212494] fs/cifs/smb2misc.c: SMB2 data length 0 offset 72
> [ 48.212919] fs/cifs/smb2misc.c: SMB2 len 77
> [ 48.213364] fs/cifs/smb2misc.c: Calculated size 77 length 76 mismatch mid 2
> [ 48.213807] fs/cifs/transport.c: cifs_sync_mid_result: cmd=1 mid=2 state=4
> [ 48.214242] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> [ 48.219385] fs/cifs/smb2pdu.c: SMB2/3 session established successfully
> [ 48.219831] fs/cifs/connect.c: CIFS VFS: leaving cifs_get_smb_ses (xid = 1) rc = 0
> [ 48.220276] fs/cifs/connect.c: CIFS VFS: in cifs_get_tcon as Xid: 2 with uid: 0
> [ 48.220724] fs/cifs/smb2pdu.c: TCON
> [ 48.221182] fs/cifs/transport.c: Sending smb: smb_len=122
> [ 48.221830] fs/cifs/connect.c: RFC1002 header 0x50
> [ 48.222280] fs/cifs/smb2misc.c: smb2_check_message length: 0x54, smb_buf_length: 0x50
> [ 48.222734] fs/cifs/smb2misc.c: SMB2 len 84
> [ 48.223199] fs/cifs/transport.c: cifs_sync_mid_result: cmd=3 mid=3 state=4
> [ 48.223656] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> [ 48.224107] fs/cifs/smb2pdu.c: connection to disk share
> [ 48.224560] fs/cifs/smb2pdu.c: validate negotiate
> [ 48.225015] fs/cifs/smb2pdu.c: SMB2 IOCTL
> [ 48.225456] fs/cifs/transport.c: Sending smb: smb_len=146
> [ 48.226049] fs/cifs/connect.c: RFC1002 header 0x49
> [ 48.226498] fs/cifs/smb2misc.c: smb2_check_message length: 0x4d, smb_buf_length: 0x49
> [ 48.226951] fs/cifs/smb2misc.c: SMB2 data length 0 offset 0
> [ 48.227408] fs/cifs/smb2misc.c: SMB2 len 77
> [ 48.227863] fs/cifs/transport.c: cifs_sync_mid_result: cmd=11 mid=4 state=4
> [ 48.228318] Status code returned 0xc0000022 STATUS_ACCESS_DENIED
> [ 48.228780] fs/cifs/smb2maperror.c: Mapping SMB2 status code -1073741790 to POSIX err -13
> [ 48.229265] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release
> [ 48.229732] CIFS VFS: validate protocol negotiate failed: -13
> [ 48.230197] fs/cifs/connect.c: CIFS VFS: leaving cifs_get_tcon (xid = 2) rc = -5
> [ 48.230681] fs/cifs/connect.c: Tcon rc = -5
> [ 48.231150] fs/cifs/connect.c: build_unc_path_to_root: full_path=\\<ip_addr>\TestSMB
> [ 48.231634] fs/cifs/connect.c: cifs_put_smb_ses: ses_count=1
> [ 48.232101] fs/cifs/connect.c: CIFS VFS: in cifs_put_smb_ses as Xid: 3 with uid: 0
> [ 48.232569] fs/cifs/smb2pdu.c: disconnect session ffff8800b9189e00
> [ 48.233053] fs/cifs/transport.c: Sending smb: smb_len=68
> [ 48.233651] fs/cifs/connect.c: RFC1002 header 0x44
> [ 48.234116] fs/cifs/smb2misc.c: smb2_check_message length: 0x48, smb_buf_length: 0x44
> [ 48.234585] fs/cifs/smb2misc.c: SMB2 len 72
> [ 48.235063] fs/cifs/transport.c: cifs_sync_mid_result: cmd=2 mid=5 state=4
> [ 48.235541] SendRcvNoRsp flags 64 rc 0
> [ 48.236075] fs/cifs/connect.c: CIFS VFS: leaving cifs_mount (xid = 0) rc = -5
> [ 48.236556] CIFS VFS: cifs_mount failed w/return code = -5
>
>
> Any thoughts on what is the right fix for stable kernels? Mounting SMB3
> shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if
> I pass the sec=ntlmsspi option to the mount command (as opposed to the
> default: sec=ntlmssp). Please let me know if you need any other info.
>
> Thank you!
>
> Regards,
> Srivatsa
>

2018-01-19 13:24:11

by Aurélien Aptel

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

Hi,

"Srivatsa S. Bhat" <[email protected]> writes:
>> Any thoughts on what is the right fix for stable kernels? Mounting SMB3
>> shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if
>> I pass the sec=ntlmsspi option to the mount command (as opposed to the
>> default: sec=ntlmssp). Please let me know if you need any other info.

Make sure you have (in that order):

db3b5474f462 ("CIFS: Fix NULL pointer deref on SMB2_tcon() failure")
fe83bebc0522 ("SMB: fix leak of validate negotiate info response buffer")
a2d9daad1d2d ("SMB: fix validate negotiate info uninitialised memory use")
4587eee04e2a ("SMB3: Validate negotiate request must always be signed")
a821df3f1af7 ("cifs: fix NULL deref in SMB2_read")

Does enabling CIFS_SMB311 changes anything?

I also suspect some things assume encryption patches are in.

--
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)

2018-01-30 03:34:35

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

Hi Aurélien,

On 1/19/18 5:23 AM, Aurélien Aptel wrote:
> Hi,
>
> "Srivatsa S. Bhat" <[email protected]> writes:
>>> Any thoughts on what is the right fix for stable kernels? Mounting SMB3
>>> shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if
>>> I pass the sec=ntlmsspi option to the mount command (as opposed to the
>>> default: sec=ntlmssp). Please let me know if you need any other info.
>
> Make sure you have (in that order):
>
> db3b5474f462 ("CIFS: Fix NULL pointer deref on SMB2_tcon() failure")
> fe83bebc0522 ("SMB: fix leak of validate negotiate info response buffer")
> a2d9daad1d2d ("SMB: fix validate negotiate info uninitialised memory use")
> 4587eee04e2a ("SMB3: Validate negotiate request must always be signed")
> a821df3f1af7 ("cifs: fix NULL deref in SMB2_read")
>
> Does enabling CIFS_SMB311 changes anything?
>

Thank you for looking into this. I tried applying these patches on top
of 4.4.113 and 4.9.78, but that didn't fix the problem on either kernel,
with or without CONFIG_CIFS_SMB311 enabled.

(By the way, shouldn't these patches be applied to stable kernels anyway?
I was a bit surprised that none of them are present in 4.4.113 and 4.9.78).

> I also suspect some things assume encryption patches are in.
>

Do you happen to know which patches they might be? In any case, I'm using
the latest (unmodified) 4.4 and 4.9 stable kernels, so I hope the necessary
support is already present in them.

The 5 patches you suggested above needed a bit of fixup by hand for 4.4.113,
so I have shared my combined patch below for reference, which applies
cleanly on top of 4.4.113. (The same patch applies on 4.9.78 as well, with
some minor line-number differences).

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index f2ff60e..92abb8b9 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -519,7 +519,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
{
int rc = 0;
struct validate_negotiate_info_req vneg_inbuf;
- struct validate_negotiate_info_rsp *pneg_rsp;
+ struct validate_negotiate_info_rsp *pneg_rsp = NULL;
u32 rsplen;

cifs_dbg(FYI, "validate negotiate\n");
@@ -575,8 +575,9 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
rsplen);

/* relax check since Mac returns max bufsize allowed on ioctl */
- if (rsplen > CIFSMaxBufSize)
- return -EIO;
+ if ((rsplen > CIFSMaxBufSize)
+ || (rsplen < sizeof(struct validate_negotiate_info_rsp)))
+ goto err_rsp_free;
}

/* check validate negotiate info response matches what we got earlier */
@@ -595,10 +596,13 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)

/* validate negotiate successful */
cifs_dbg(FYI, "validate negotiate info successful\n");
+ kfree(pneg_rsp);
return 0;

vneg_out:
cifs_dbg(VFS, "protocol revalidation - security settings mismatch\n");
+err_rsp_free:
+ kfree(pneg_rsp);
return -EIO;
}

@@ -1042,7 +1046,7 @@ tcon_exit:
return rc;

tcon_error_exit:
- if (rsp->hdr.Status == STATUS_BAD_NETWORK_NAME) {
+ if (rsp && rsp->hdr.Status == STATUS_BAD_NETWORK_NAME) {
cifs_dbg(VFS, "BAD_NETWORK_NAME: %s\n", tree);
}
goto tcon_exit;
@@ -1559,6 +1563,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
} else
iov[0].iov_len = get_rfc1002_length(req) + 4;

+ /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
+ if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
+ req->hdr.Flags |= SMB2_FLAGS_SIGNED;

rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
@@ -2159,23 +2166,22 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms,

rsp = (struct smb2_read_rsp *)iov[0].iov_base;

- if (rsp->hdr.Status == STATUS_END_OF_FILE) {
+ if (rc) {
+ if (rc != -ENODATA) {
+ cifs_stats_fail_inc(io_parms->tcon, SMB2_READ_HE);
+ cifs_dbg(VFS, "Send error in read = %d\n", rc);
+ }
free_rsp_buf(resp_buftype, iov[0].iov_base);
- return 0;
+ return rc == -ENODATA ? 0 : rc;
}

- if (rc) {
- cifs_stats_fail_inc(io_parms->tcon, SMB2_READ_HE);
- cifs_dbg(VFS, "Send error in read = %d\n", rc);
- } else {
- *nbytes = le32_to_cpu(rsp->DataLength);
- if ((*nbytes > CIFS_MAX_MSGSIZE) ||
- (*nbytes > io_parms->length)) {
- cifs_dbg(FYI, "bad length %d for count %d\n",
- *nbytes, io_parms->length);
- rc = -EIO;
- *nbytes = 0;
- }
+ *nbytes = le32_to_cpu(rsp->DataLength);
+ if ((*nbytes > CIFS_MAX_MSGSIZE) ||
+ (*nbytes > io_parms->length)) {
+ cifs_dbg(FYI, "bad length %d for count %d\n",
+ *nbytes, io_parms->length);
+ rc = -EIO;
+ *nbytes = 0;
}

if (*buf) {

With this patch (and CONFIG_CIFS_SMB311 enabled), the 4.4.113 kernel crashes as
shown below when I try:

# mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir

[ 14.638907] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
[ 14.638940] IP: [<ffffffff8139221a>] crypto_shash_setkey+0x1a/0xc0
[ 14.638964] PGD 0
[ 14.638972] Oops: 0000 [#1] SMP
[ 14.638985] Modules linked in: arc4(E) ecb(E) md4(E) cifs(E) dns_resolver(E) vmw_vsock_vmci_transport(E) vsock(E) xt_conntrack(E) iptable_nat(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) nf_nat_ipv4(E) nf_nat(E) iptable_filter(E) ip_tables(E) xt_LOG(E) nf_conntrack(E) hid_generic(E) usbhid(E) hid(E) mousedev(E) crc32c_intel(E) jitterentropy_rng(E) hmac(E) sha256_ssse3(E) sha256_generic(E) uhci_hcd(E) drbg(E) ansi_cprng(E) aesni_intel(E) ehci_pci(E) aes_x86_64(E) glue_helper(E) ehci_hcd(E) lrw(E) gf128mul(E) usbcore(E) ablk_helper(E) psmouse(E) cryptd(E) vmw_balloon(E) evdev(E) intel_agp(E) vmw_vmci(E) usb_common(E) i2c_piix4(E) intel_gtt(E) nfit(E) battery(E) tpm_tis(E) tpm(E) ac(E) button(E) sch_fq_codel(E) autofs4(E)
[ 14.639237] CPU: 0 PID: 841 Comm: mount.cifs Tainted: G E 4.4.113-fixes-smb311+ #33
[ 14.639263] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016
[ 14.639294] task: ffff8800ae811440 ti: ffff8800b9d4c000 task.ti: ffff8800b9d4c000
[ 14.639315] RIP: 0010:[<ffffffff8139221a>] [<ffffffff8139221a>] crypto_shash_setkey+0x1a/0xc0
[ 14.639343] RSP: 0018:ffff8800b9d4f9a8 EFLAGS: 00010282
[ 14.639358] RAX: ffff88013305d580 RBX: ffff8800ba2ed000 RCX: 00000000fffee93f
[ 14.639379] RDX: 0000000000000010 RSI: ffff8800b9f58d18 RDI: 0000000000000000
[ 14.639399] RBP: ffff8800b9d4f9e0 R08: ffff8800b9d4fb64 R09: 0000000000000000
[ 14.639420] R10: 3036312e3130312e R11: 424d53747365545c R12: 0000000000000002
[ 14.639440] R13: 0000000000000000 R14: ffff8800b9f58d18 R15: 0000000000000010
[ 14.639461] FS: 00007f02bcb74740(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000
[ 14.639484] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 14.639501] CR2: 0000000000000050 CR3: 00000000ae9f8000 CR4: 0000000000160670
[ 14.639558] Stack:
[ 14.639566] ffff8800b66789c0 ffff8800b9d4fa08 ffff8800ba2ed000 0000000000000002
[ 14.639592] ffff8800b9d4fac8 00000c0094000029 ffff8800ba2ed000 ffff8800b9d4fa50
[ 14.639618] ffffffffa02594f6 ffff8800b9d4fb70 ffff88013305d580 0000000000000002
[ 14.639644] Call Trace:
[ 14.639669] [<ffffffffa02594f6>] smb3_calc_signature+0xb6/0x290 [cifs]
[ 14.639699] [<ffffffffa0258bab>] smb2_sign_rqst+0x2b/0x40 [cifs]
[ 14.639726] [<ffffffffa02599d1>] smb2_setup_request+0xd1/0x170 [cifs]
[ 14.640347] [<ffffffffa0248be7>] SendReceive2+0xc7/0x450 [cifs]
[ 14.640958] [<ffffffffa02461b5>] ? cifs_small_buf_get+0x15/0x30 [cifs]
[ 14.641582] [<ffffffffa025b89f>] ? small_smb2_init+0xdf/0x200 [cifs]
[ 14.642172] [<ffffffffa025d867>] SMB2_ioctl+0x147/0x310 [cifs]
[ 14.642753] [<ffffffffa025db37>] smb3_validate_negotiate+0x107/0x2e0 [cifs]
[ 14.643336] [<ffffffffa025b1eb>] SMB2_tcon+0x29b/0x510 [cifs]
[ 14.643921] [<ffffffffa0230c5b>] cifs_get_tcon+0x1bb/0x560 [cifs]
[ 14.644501] [<ffffffffa02335f0>] cifs_mount+0x690/0xde0 [cifs]
[ 14.645061] [<ffffffffa021f6eb>] cifs_do_mount+0xcb/0x5a0 [cifs]
[ 14.645618] [<ffffffff81196057>] ? alloc_pages_current+0x87/0x110
[ 14.646149] [<ffffffff811baa83>] mount_fs+0x33/0x160
[ 14.646663] [<ffffffff811d4b22>] vfs_kern_mount+0x62/0x100
[ 14.647163] [<ffffffff811d6edb>] do_mount+0x21b/0xd30
[ 14.647653] [<ffffffff81196057>] ? alloc_pages_current+0x87/0x110
[ 14.648128] [<ffffffff811d7d07>] SyS_mount+0x87/0xd0
[ 14.648591] [<ffffffff817db31b>] entry_SYSCALL_64_fastpath+0x18/0x93
[ 14.649047] Code: 89 e5 8b 12 e8 a8 cd 04 00 31 c0 5d c3 0f 1f 40 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fd 53 49 89 f6 41 89 d7 48 83 ec 10 <4c> 8b 67 50 41 8b 5c 24 2c 48 85 de 75 14 41 ff 54 24 e8 48 83
[ 14.650496] RIP [<ffffffff8139221a>] crypto_shash_setkey+0x1a/0xc0
[ 14.650953] RSP <ffff8800b9d4f9a8>
[ 14.651397] CR2: 0000000000000050
[ 14.651861] ---[ end trace c98f651d4ccb0d7d ]---

Regards,
Srivatsa

2018-02-27 03:45:41

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

2018-02-27 08:57:32

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
> > On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
> >> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
> >>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
> >>>> 4.13-stable review patch. If anyone has any objections, please let me know.
> >>>>
> >>>> ------------------
> >>>>
> >>>> From: Steve French <[email protected]>
> >>>>
> >>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
> >>>>
> >>>> According to MS-SMB2 3.2.55 validate_negotiate request must
> >>>> always be signed. Some Windows can fail the request if you send it unsigned
> >>>>
> >>>> See kernel bugzilla bug 197311
> >>>>
> >>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
> >>>> Signed-off-by: Steve French <[email protected]>
> >>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >>>>
> >>>> ---
> >>>> fs/cifs/smb2pdu.c | 3 +++
> >>>> 1 file changed, 3 insertions(+)
> >>>>
> >>>> --- a/fs/cifs/smb2pdu.c
> >>>> +++ b/fs/cifs/smb2pdu.c
> >>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
> >>>> } else
> >>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
> >>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> >>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> >>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
> >>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
> >>>> cifs_small_buf_release(req);
> >>>>
> >>>>
> >>>>
> >>>
> >>> This one needs to be backported to all stable kernels as the commit that
> >>> introduced the regression:
> >>> '
> >>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
> >>> SMB: Validate negotiate (to protect against downgrade) even if signing off
> >>>
> >>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
> >>
> >> Oh wait, it breaks the builds on older kernels, that's why I didn't
> >> apply it :)
> >>
> >> Can you provide me with a working backport?
> >>
> >
> > Hi Steve,
> >
> > Is there a version of this fix available for stable kernels?
> >
>
> Hi Greg,
>
> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
> due to the issues that I have described in detail on this mail thread.
>
> Since there is no apparent fix for this bug on stable kernels, could
> you please consider reverting the original commit that caused this
> regression?
>
> That commit was intended to enhance security, which is probably why it
> was backported to stable kernels in the first place; but instead it
> ends up breaking basic functionality itself (mounting). So in the
> absence of a proper fix, I don't see much of an option but to revert
> that commit.
>
> So, please consider reverting the following:
>
> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
> against downgrade) even if signing off" on 4.4.118
>
> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
> against downgrade) even if signing off" on 4.9.84
>
> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
> upstream. Both these patches should revert cleanly.

Do you still have this same problem on 4.14 and 4.15? If so, the issue
needs to get fixed there, not papered-over by reverting these old
changes, as you will hit the issue again in the future when you update
to a newer kernel version.

thanks,

greg k-h

2018-02-27 09:41:09

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>
>>>>>> ------------------
>>>>>>
>>>>>> From: Steve French <[email protected]>
>>>>>>
>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>
>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>
>>>>>> See kernel bugzilla bug 197311
>>>>>>
>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>
>>>>>> ---
>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>> 1 file changed, 3 insertions(+)
>>>>>>
>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>> } else
>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>> cifs_small_buf_release(req);
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>> introduced the regression:
>>>>> '
>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>
>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>
>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>> apply it :)
>>>>
>>>> Can you provide me with a working backport?
>>>>
>>>
>>> Hi Steve,
>>>
>>> Is there a version of this fix available for stable kernels?
>>>
>>
>> Hi Greg,
>>
>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>> due to the issues that I have described in detail on this mail thread.
>>
>> Since there is no apparent fix for this bug on stable kernels, could
>> you please consider reverting the original commit that caused this
>> regression?
>>
>> That commit was intended to enhance security, which is probably why it
>> was backported to stable kernels in the first place; but instead it
>> ends up breaking basic functionality itself (mounting). So in the
>> absence of a proper fix, I don't see much of an option but to revert
>> that commit.
>>
>> So, please consider reverting the following:
>>
>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>> against downgrade) even if signing off" on 4.4.118
>>
>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>> against downgrade) even if signing off" on 4.9.84
>>
>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>> upstream. Both these patches should revert cleanly.
>
> Do you still have this same problem on 4.14 and 4.15? If so, the issue
> needs to get fixed there, not papered-over by reverting these old
> changes, as you will hit the issue again in the future when you update
> to a newer kernel version.
>

4.14 and 4.15 work great! (I had mentioned this is in my original bug
report but forgot to summarize it here, sorry).

Thank you!

Regards,
Srivatsa

2018-02-27 12:41:57

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
> > On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
> >> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
> >>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
> >>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
> >>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
> >>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
> >>>>>>
> >>>>>> ------------------
> >>>>>>
> >>>>>> From: Steve French <[email protected]>
> >>>>>>
> >>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
> >>>>>>
> >>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
> >>>>>> always be signed. Some Windows can fail the request if you send it unsigned
> >>>>>>
> >>>>>> See kernel bugzilla bug 197311
> >>>>>>
> >>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
> >>>>>> Signed-off-by: Steve French <[email protected]>
> >>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >>>>>>
> >>>>>> ---
> >>>>>> fs/cifs/smb2pdu.c | 3 +++
> >>>>>> 1 file changed, 3 insertions(+)
> >>>>>>
> >>>>>> --- a/fs/cifs/smb2pdu.c
> >>>>>> +++ b/fs/cifs/smb2pdu.c
> >>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
> >>>>>> } else
> >>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
> >>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
> >>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
> >>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
> >>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
> >>>>>> cifs_small_buf_release(req);
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> This one needs to be backported to all stable kernels as the commit that
> >>>>> introduced the regression:
> >>>>> '
> >>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
> >>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
> >>>>>
> >>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
> >>>>
> >>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
> >>>> apply it :)
> >>>>
> >>>> Can you provide me with a working backport?
> >>>>
> >>>
> >>> Hi Steve,
> >>>
> >>> Is there a version of this fix available for stable kernels?
> >>>
> >>
> >> Hi Greg,
> >>
> >> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
> >> due to the issues that I have described in detail on this mail thread.
> >>
> >> Since there is no apparent fix for this bug on stable kernels, could
> >> you please consider reverting the original commit that caused this
> >> regression?
> >>
> >> That commit was intended to enhance security, which is probably why it
> >> was backported to stable kernels in the first place; but instead it
> >> ends up breaking basic functionality itself (mounting). So in the
> >> absence of a proper fix, I don't see much of an option but to revert
> >> that commit.
> >>
> >> So, please consider reverting the following:
> >>
> >> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
> >> against downgrade) even if signing off" on 4.4.118
> >>
> >> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
> >> against downgrade) even if signing off" on 4.9.84
> >>
> >> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
> >> upstream. Both these patches should revert cleanly.
> >
> > Do you still have this same problem on 4.14 and 4.15? If so, the issue
> > needs to get fixed there, not papered-over by reverting these old
> > changes, as you will hit the issue again in the future when you update
> > to a newer kernel version.
> >
>
> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
> report but forgot to summarize it here, sorry).

Then what is the bugfix that should be applied here in order to keep
things working with these patches applied?

thanks,

greg k-h

2018-02-27 17:47:28

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
> On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
>> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
>>> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>>>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>>>
>>>>>>>> ------------------
>>>>>>>>
>>>>>>>> From: Steve French <[email protected]>
>>>>>>>>
>>>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>>>
>>>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>>>
>>>>>>>> See kernel bugzilla bug 197311
>>>>>>>>
>>>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>>>
>>>>>>>> ---
>>>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>>>> } else
>>>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>>>> cifs_small_buf_release(req);
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>>>> introduced the regression:
>>>>>>> '
>>>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>>>
>>>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>>>
>>>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>>>> apply it :)
>>>>>>
>>>>>> Can you provide me with a working backport?
>>>>>>
>>>>>
>>>>> Hi Steve,
>>>>>
>>>>> Is there a version of this fix available for stable kernels?
>>>>>
>>>>
>>>> Hi Greg,
>>>>
>>>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>>>> due to the issues that I have described in detail on this mail thread.
>>>>
>>>> Since there is no apparent fix for this bug on stable kernels, could
>>>> you please consider reverting the original commit that caused this
>>>> regression?
>>>>
>>>> That commit was intended to enhance security, which is probably why it
>>>> was backported to stable kernels in the first place; but instead it
>>>> ends up breaking basic functionality itself (mounting). So in the
>>>> absence of a proper fix, I don't see much of an option but to revert
>>>> that commit.
>>>>
>>>> So, please consider reverting the following:
>>>>
>>>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>>>> against downgrade) even if signing off" on 4.4.118
>>>>
>>>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>>>> against downgrade) even if signing off" on 4.9.84
>>>>
>>>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>> upstream. Both these patches should revert cleanly.
>>>
>>> Do you still have this same problem on 4.14 and 4.15? If so, the issue
>>> needs to get fixed there, not papered-over by reverting these old
>>> changes, as you will hit the issue again in the future when you update
>>> to a newer kernel version.
>>>
>>
>> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
>> report but forgot to summarize it here, sorry).
>
>
> Then what is the bugfix that should be applied here in order to keep
> things working with these patches applied?
>

That would be the one mentioned in the subject line of this thread :)
However, a working backport of that fix is not available for 4.4 and
4.9, hence the trouble.

It looks like we are reconstructing elements of this email thread all
over again, so let me quickly summarize the status so far:

In 4.14/4.15/mainline,
- commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against
downgrade) even if signing off) caused mount regression with SMB v3.

- commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must
always be signed) fixed the issue.

- [ There was a lot of code churn in the CIFS/SMB codebase between
these two commits in mainline. ]

In this email thread, you backported the fix to stable 4.13. Thomas
noticed that the problematic commit had also made it to stable series
such as 4.4 and 4.9, and requested a backport of the fix to those
trees as well. However, a straight-forward backport of the fix to 4.4
and 4.9 breaks the build, so no fix was available for those kernels.

I investigated this and tried to produce a working backport of the fix
to 4.4 and 4.9, but didn't succeed, despite trying several variations
as well as suggestions from Aurelien [1][2]. So, given that there is
still no known fix for the mount regression on 4.4 and 4.9 stable
series at this point, I decided to request a revert of the problematic
commit that caused the regression in those kernels.

[1]. https://lkml.org/lkml/2018/1/3/892
[2]. https://lkml.org/lkml/2018/1/29/1009

Regards,
Srivatsa

2018-02-27 17:57:57

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

This shouldn't be too hard to figure out if willing to backport a
slightly larger set of fixes to the older stable, but I don't have a
system running 4.9 stable.

Is this the correct stable tree branch?
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-4.9.y

On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat
<[email protected]> wrote:
> On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
>> On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
>>> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
>>>> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>>>>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>>>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>>>>
>>>>>>>>> ------------------
>>>>>>>>>
>>>>>>>>> From: Steve French <[email protected]>
>>>>>>>>>
>>>>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>>>>
>>>>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>>>>
>>>>>>>>> See kernel bugzilla bug 197311
>>>>>>>>>
>>>>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>>
>>>>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>>>>> } else
>>>>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>>>>> cifs_small_buf_release(req);
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>>>>> introduced the regression:
>>>>>>>> '
>>>>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>>>>
>>>>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>>>>
>>>>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>>>>> apply it :)
>>>>>>>
>>>>>>> Can you provide me with a working backport?
>>>>>>>
>>>>>>
>>>>>> Hi Steve,
>>>>>>
>>>>>> Is there a version of this fix available for stable kernels?
>>>>>>
>>>>>
>>>>> Hi Greg,
>>>>>
>>>>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>>>>> due to the issues that I have described in detail on this mail thread.
>>>>>
>>>>> Since there is no apparent fix for this bug on stable kernels, could
>>>>> you please consider reverting the original commit that caused this
>>>>> regression?
>>>>>
>>>>> That commit was intended to enhance security, which is probably why it
>>>>> was backported to stable kernels in the first place; but instead it
>>>>> ends up breaking basic functionality itself (mounting). So in the
>>>>> absence of a proper fix, I don't see much of an option but to revert
>>>>> that commit.
>>>>>
>>>>> So, please consider reverting the following:
>>>>>
>>>>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>>>>> against downgrade) even if signing off" on 4.4.118
>>>>>
>>>>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>>>>> against downgrade) even if signing off" on 4.9.84
>>>>>
>>>>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>> upstream. Both these patches should revert cleanly.
>>>>
>>>> Do you still have this same problem on 4.14 and 4.15? If so, the issue
>>>> needs to get fixed there, not papered-over by reverting these old
>>>> changes, as you will hit the issue again in the future when you update
>>>> to a newer kernel version.
>>>>
>>>
>>> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
>>> report but forgot to summarize it here, sorry).
>>
>>
>> Then what is the bugfix that should be applied here in order to keep
>> things working with these patches applied?
>>
>
> That would be the one mentioned in the subject line of this thread :)
> However, a working backport of that fix is not available for 4.4 and
> 4.9, hence the trouble.
>
> It looks like we are reconstructing elements of this email thread all
> over again, so let me quickly summarize the status so far:
>
> In 4.14/4.15/mainline,
> - commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against
> downgrade) even if signing off) caused mount regression with SMB v3.
>
> - commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must
> always be signed) fixed the issue.
>
> - [ There was a lot of code churn in the CIFS/SMB codebase between
> these two commits in mainline. ]
>
> In this email thread, you backported the fix to stable 4.13. Thomas
> noticed that the problematic commit had also made it to stable series
> such as 4.4 and 4.9, and requested a backport of the fix to those
> trees as well. However, a straight-forward backport of the fix to 4.4
> and 4.9 breaks the build, so no fix was available for those kernels.
>
> I investigated this and tried to produce a working backport of the fix
> to 4.4 and 4.9, but didn't succeed, despite trying several variations
> as well as suggestions from Aurelien [1][2]. So, given that there is
> still no known fix for the mount regression on 4.4 and 4.9 stable
> series at this point, I decided to request a revert of the problematic
> commit that caused the regression in those kernels.
>
> [1]. https://lkml.org/lkml/2018/1/3/892
> [2]. https://lkml.org/lkml/2018/1/29/1009
>
> Regards,
> Srivatsa

--
Thanks,

Steve

2018-02-27 18:35:45

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 2/27/18 9:56 AM, Steve French wrote:
> This shouldn't be too hard to figure out if willing to backport a
> slightly larger set of fixes to the older stable, but I don't have a
> system running 4.9 stable.
>

If you have the proposed patches that apply on 4.9, I'd be happy to
try them out!

[ I would have offered to backport the patches myself, but actually I
already tried doing that with a larger set of patches from mainline
(picking those commits between the regression and the fix that seemed
relevant), but I felt quite out-of-depth trying to adapt them to 4.9
and 4.4, as I'm not that familiar with the internals of SMB/CIFS. ]

> Is this the correct stable tree branch?
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-4.9.y
>

Yep!

Regards,
Srivatsa

> On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat
> <[email protected]> wrote:
>> On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
>>> On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
>>>> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
>>>>> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>>>>>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>>>>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>>>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>>>>>
>>>>>>>>>> ------------------
>>>>>>>>>>
>>>>>>>>>> From: Steve French <[email protected]>
>>>>>>>>>>
>>>>>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>>>>>
>>>>>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>>>>>
>>>>>>>>>> See kernel bugzilla bug 197311
>>>>>>>>>>
>>>>>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>>>
>>>>>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>>>>>> } else
>>>>>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>>>>>> cifs_small_buf_release(req);
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>>>>>> introduced the regression:
>>>>>>>>> '
>>>>>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>>>>>
>>>>>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>>>>>
>>>>>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>>>>>> apply it :)
>>>>>>>>
>>>>>>>> Can you provide me with a working backport?
>>>>>>>>
>>>>>>>
>>>>>>> Hi Steve,
>>>>>>>
>>>>>>> Is there a version of this fix available for stable kernels?
>>>>>>>
>>>>>>
>>>>>> Hi Greg,
>>>>>>
>>>>>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>>>>>> due to the issues that I have described in detail on this mail thread.
>>>>>>
>>>>>> Since there is no apparent fix for this bug on stable kernels, could
>>>>>> you please consider reverting the original commit that caused this
>>>>>> regression?
>>>>>>
>>>>>> That commit was intended to enhance security, which is probably why it
>>>>>> was backported to stable kernels in the first place; but instead it
>>>>>> ends up breaking basic functionality itself (mounting). So in the
>>>>>> absence of a proper fix, I don't see much of an option but to revert
>>>>>> that commit.
>>>>>>
>>>>>> So, please consider reverting the following:
>>>>>>
>>>>>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>>>>>> against downgrade) even if signing off" on 4.4.118
>>>>>>
>>>>>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>>>>>> against downgrade) even if signing off" on 4.9.84
>>>>>>
>>>>>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>> upstream. Both these patches should revert cleanly.
>>>>>
>>>>> Do you still have this same problem on 4.14 and 4.15? If so, the issue
>>>>> needs to get fixed there, not papered-over by reverting these old
>>>>> changes, as you will hit the issue again in the future when you update
>>>>> to a newer kernel version.
>>>>>
>>>>
>>>> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
>>>> report but forgot to summarize it here, sorry).
>>>
>>>
>>> Then what is the bugfix that should be applied here in order to keep
>>> things working with these patches applied?
>>>
>>
>> That would be the one mentioned in the subject line of this thread :)
>> However, a working backport of that fix is not available for 4.4 and
>> 4.9, hence the trouble.
>>
>> It looks like we are reconstructing elements of this email thread all
>> over again, so let me quickly summarize the status so far:
>>
>> In 4.14/4.15/mainline,
>> - commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against
>> downgrade) even if signing off) caused mount regression with SMB v3.
>>
>> - commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must
>> always be signed) fixed the issue.
>>
>> - [ There was a lot of code churn in the CIFS/SMB codebase between
>> these two commits in mainline. ]
>>
>> In this email thread, you backported the fix to stable 4.13. Thomas
>> noticed that the problematic commit had also made it to stable series
>> such as 4.4 and 4.9, and requested a backport of the fix to those
>> trees as well. However, a straight-forward backport of the fix to 4.4
>> and 4.9 breaks the build, so no fix was available for those kernels.
>>
>> I investigated this and tried to produce a working backport of the fix
>> to 4.4 and 4.9, but didn't succeed, despite trying several variations
>> as well as suggestions from Aurelien [1][2]. So, given that there is
>> still no known fix for the mount regression on 4.4 and 4.9 stable
>> series at this point, I decided to request a revert of the problematic
>> commit that caused the regression in those kernels.
>>
>> [1]. https://lkml.org/lkml/2018/1/3/892
>> [2]. https://lkml.org/lkml/2018/1/29/1009
>>
>> Regards,
>> Srivatsa
>
>
>

2018-03-01 20:13:59

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

So far I haven't been able to reproduce this on the current 4.9 stable
tree with vers=3.0 or with default (vers=1.0 for these older kernels).

On Tue, Feb 27, 2018 at 11:56 AM, Steve French <[email protected]> wrote:
> This shouldn't be too hard to figure out if willing to backport a
> slightly larger set of fixes to the older stable, but I don't have a
> system running 4.9 stable.
>
> Is this the correct stable tree branch?
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-4.9.y
>
> On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat
> <[email protected]> wrote:
>> On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
>>> On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
>>>> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
>>>>> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>>>>>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>>>>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>>>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>>>>>
>>>>>>>>>> ------------------
>>>>>>>>>>
>>>>>>>>>> From: Steve French <[email protected]>
>>>>>>>>>>
>>>>>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>>>>>
>>>>>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>>>>>
>>>>>>>>>> See kernel bugzilla bug 197311
>>>>>>>>>>
>>>>>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>>>
>>>>>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>>>>>> } else
>>>>>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>>>>>> cifs_small_buf_release(req);
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>>>>>> introduced the regression:
>>>>>>>>> '
>>>>>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>>>>>
>>>>>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>>>>>
>>>>>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>>>>>> apply it :)
>>>>>>>>
>>>>>>>> Can you provide me with a working backport?
>>>>>>>>
>>>>>>>
>>>>>>> Hi Steve,
>>>>>>>
>>>>>>> Is there a version of this fix available for stable kernels?
>>>>>>>
>>>>>>
>>>>>> Hi Greg,
>>>>>>
>>>>>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>>>>>> due to the issues that I have described in detail on this mail thread.
>>>>>>
>>>>>> Since there is no apparent fix for this bug on stable kernels, could
>>>>>> you please consider reverting the original commit that caused this
>>>>>> regression?
>>>>>>
>>>>>> That commit was intended to enhance security, which is probably why it
>>>>>> was backported to stable kernels in the first place; but instead it
>>>>>> ends up breaking basic functionality itself (mounting). So in the
>>>>>> absence of a proper fix, I don't see much of an option but to revert
>>>>>> that commit.
>>>>>>
>>>>>> So, please consider reverting the following:
>>>>>>
>>>>>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>>>>>> against downgrade) even if signing off" on 4.4.118
>>>>>>
>>>>>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>>>>>> against downgrade) even if signing off" on 4.9.84
>>>>>>
>>>>>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>> upstream. Both these patches should revert cleanly.
>>>>>
>>>>> Do you still have this same problem on 4.14 and 4.15? If so, the issue
>>>>> needs to get fixed there, not papered-over by reverting these old
>>>>> changes, as you will hit the issue again in the future when you update
>>>>> to a newer kernel version.
>>>>>
>>>>
>>>> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
>>>> report but forgot to summarize it here, sorry).
>>>
>>>
>>> Then what is the bugfix that should be applied here in order to keep
>>> things working with these patches applied?
>>>
>>
>> That would be the one mentioned in the subject line of this thread :)
>> However, a working backport of that fix is not available for 4.4 and
>> 4.9, hence the trouble.
>>
>> It looks like we are reconstructing elements of this email thread all
>> over again, so let me quickly summarize the status so far:
>>
>> In 4.14/4.15/mainline,
>> - commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against
>> downgrade) even if signing off) caused mount regression with SMB v3.
>>
>> - commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must
>> always be signed) fixed the issue.
>>
>> - [ There was a lot of code churn in the CIFS/SMB codebase between
>> these two commits in mainline. ]
>>
>> In this email thread, you backported the fix to stable 4.13. Thomas
>> noticed that the problematic commit had also made it to stable series
>> such as 4.4 and 4.9, and requested a backport of the fix to those
>> trees as well. However, a straight-forward backport of the fix to 4.4
>> and 4.9 breaks the build, so no fix was available for those kernels.
>>
>> I investigated this and tried to produce a working backport of the fix
>> to 4.4 and 4.9, but didn't succeed, despite trying several variations
>> as well as suggestions from Aurelien [1][2]. So, given that there is
>> still no known fix for the mount regression on 4.4 and 4.9 stable
>> series at this point, I decided to request a revert of the problematic
>> commit that caused the regression in those kernels.
>>
>> [1]. https://lkml.org/lkml/2018/1/3/892
>> [2]. https://lkml.org/lkml/2018/1/29/1009
>>
>> Regards,
>> Srivatsa
>
>
>
> --
> Thanks,
>
> Steve

--
Thanks,

Steve

2018-03-01 20:53:01

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 3/1/18 12:12 PM, Steve French wrote:
> So far I haven't been able to reproduce this on the current 4.9 stable
> tree with vers=3.0 or with default (vers=1.0 for these older kernels).
>

Maybe the problem also depends on the particular version of Windows
that hosts the SMB shares? I'm using Windows Server 2016 (Version
1607, OS Build 14393.693). With vers=3.0, the issue is reproducible
every time, but vers=1.0 works fine.

Regards,
Srivatsa

> On Tue, Feb 27, 2018 at 11:56 AM, Steve French <[email protected]> wrote:
>> This shouldn't be too hard to figure out if willing to backport a
>> slightly larger set of fixes to the older stable, but I don't have a
>> system running 4.9 stable.
>>
>> Is this the correct stable tree branch?
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-4.9.y
>>
>> On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat
>> <[email protected]> wrote:
>>> On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
>>>> On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
>>>>> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
>>>>>> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>>>>>>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>>>>>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>>>>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>>>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>>>>>>
>>>>>>>>>>> ------------------
>>>>>>>>>>>
>>>>>>>>>>> From: Steve French <[email protected]>
>>>>>>>>>>>
>>>>>>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>>>>>>
>>>>>>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>>>>>>
>>>>>>>>>>> See kernel bugzilla bug 197311
>>>>>>>>>>>
>>>>>>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>>>>>>> } else
>>>>>>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>>>>>>> cifs_small_buf_release(req);
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>>>>>>> introduced the regression:
>>>>>>>>>> '
>>>>>>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>>>>>>
>>>>>>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>>>>>>
>>>>>>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>>>>>>> apply it :)
>>>>>>>>>
>>>>>>>>> Can you provide me with a working backport?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Steve,
>>>>>>>>
>>>>>>>> Is there a version of this fix available for stable kernels?
>>>>>>>>
>>>>>>>
>>>>>>> Hi Greg,
>>>>>>>
>>>>>>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>>>>>>> due to the issues that I have described in detail on this mail thread.
>>>>>>>
>>>>>>> Since there is no apparent fix for this bug on stable kernels, could
>>>>>>> you please consider reverting the original commit that caused this
>>>>>>> regression?
>>>>>>>
>>>>>>> That commit was intended to enhance security, which is probably why it
>>>>>>> was backported to stable kernels in the first place; but instead it
>>>>>>> ends up breaking basic functionality itself (mounting). So in the
>>>>>>> absence of a proper fix, I don't see much of an option but to revert
>>>>>>> that commit.
>>>>>>>
>>>>>>> So, please consider reverting the following:
>>>>>>>
>>>>>>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>>>>>>> against downgrade) even if signing off" on 4.4.118
>>>>>>>
>>>>>>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>>>>>>> against downgrade) even if signing off" on 4.9.84
>>>>>>>
>>>>>>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>> upstream. Both these patches should revert cleanly.
>>>>>>
>>>>>> Do you still have this same problem on 4.14 and 4.15? If so, the issue
>>>>>> needs to get fixed there, not papered-over by reverting these old
>>>>>> changes, as you will hit the issue again in the future when you update
>>>>>> to a newer kernel version.
>>>>>>
>>>>>
>>>>> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
>>>>> report but forgot to summarize it here, sorry).
>>>>
>>>>
>>>> Then what is the bugfix that should be applied here in order to keep
>>>> things working with these patches applied?
>>>>
>>>
>>> That would be the one mentioned in the subject line of this thread :)
>>> However, a working backport of that fix is not available for 4.4 and
>>> 4.9, hence the trouble.
>>>
>>> It looks like we are reconstructing elements of this email thread all
>>> over again, so let me quickly summarize the status so far:
>>>
>>> In 4.14/4.15/mainline,
>>> - commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against
>>> downgrade) even if signing off) caused mount regression with SMB v3.
>>>
>>> - commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must
>>> always be signed) fixed the issue.
>>>
>>> - [ There was a lot of code churn in the CIFS/SMB codebase between
>>> these two commits in mainline. ]
>>>
>>> In this email thread, you backported the fix to stable 4.13. Thomas
>>> noticed that the problematic commit had also made it to stable series
>>> such as 4.4 and 4.9, and requested a backport of the fix to those
>>> trees as well. However, a straight-forward backport of the fix to 4.4
>>> and 4.9 breaks the build, so no fix was available for those kernels.
>>>
>>> I investigated this and tried to produce a working backport of the fix
>>> to 4.4 and 4.9, but didn't succeed, despite trying several variations
>>> as well as suggestions from Aurelien [1][2]. So, given that there is
>>> still no known fix for the mount regression on 4.4 and 4.9 stable
>>> series at this point, I decided to request a revert of the problematic
>>> commit that caused the regression in those kernels.
>>>
>>> [1]. https://lkml.org/lkml/2018/1/3/892
>>> [2]. https://lkml.org/lkml/2018/1/29/1009
>>>
>>> Regards,
>>> Srivatsa
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>
>
>

2018-03-12 02:39:27

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

Just got a wireshark trace - this is a fairly trivial issue (missing
the validate negotiate must be signed patch) - I had some trouble
getting this version of the kernel running (unrelated issue) and on
systems with access to Windows 2016...

On Tue, Feb 27, 2018 at 10:33 AM, Srivatsa S. Bhat
<[email protected]> wrote:
> On 2/27/18 9:56 AM, Steve French wrote:
>> This shouldn't be too hard to figure out if willing to backport a
>> slightly larger set of fixes to the older stable, but I don't have a
>> system running 4.9 stable.
>>
>
> If you have the proposed patches that apply on 4.9, I'd be happy to
> try them out!
>
> [ I would have offered to backport the patches myself, but actually I
> already tried doing that with a larger set of patches from mainline
> (picking those commits between the regression and the fix that seemed
> relevant), but I felt quite out-of-depth trying to adapt them to 4.9
> and 4.4, as I'm not that familiar with the internals of SMB/CIFS. ]
>
>> Is this the correct stable tree branch?
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-4.9.y
>>
>
> Yep!
>
> Regards,
> Srivatsa
>
>> On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat
>> <[email protected]> wrote:
>>> On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
>>>> On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
>>>>> On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
>>>>>> On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
>>>>>>> On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
>>>>>>>> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
>>>>>>>>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
>>>>>>>>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
>>>>>>>>>>> 4.13-stable review patch. If anyone has any objections, please let me know.
>>>>>>>>>>>
>>>>>>>>>>> ------------------
>>>>>>>>>>>
>>>>>>>>>>> From: Steve French <[email protected]>
>>>>>>>>>>>
>>>>>>>>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
>>>>>>>>>>>
>>>>>>>>>>> According to MS-SMB2 3.2.55 validate_negotiate request must
>>>>>>>>>>> always be signed. Some Windows can fail the request if you send it unsigned
>>>>>>>>>>>
>>>>>>>>>>> See kernel bugzilla bug 197311
>>>>>>>>>>>
>>>>>>>>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
>>>>>>>>>>> Signed-off-by: Steve French <[email protected]>
>>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>>>>>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>> fs/cifs/smb2pdu.c | 3 +++
>>>>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> --- a/fs/cifs/smb2pdu.c
>>>>>>>>>>> +++ b/fs/cifs/smb2pdu.c
>>>>>>>>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc
>>>>>>>>>>> } else
>>>>>>>>>>> iov[0].iov_len = get_rfc1002_length(req) + 4;
>>>>>>>>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
>>>>>>>>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
>>>>>>>>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
>>>>>>>>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov);
>>>>>>>>>>> cifs_small_buf_release(req);
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This one needs to be backported to all stable kernels as the commit that
>>>>>>>>>> introduced the regression:
>>>>>>>>>> '
>>>>>>>>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>>>>> SMB: Validate negotiate (to protect against downgrade) even if signing off
>>>>>>>>>>
>>>>>>>>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
>>>>>>>>>
>>>>>>>>> Oh wait, it breaks the builds on older kernels, that's why I didn't
>>>>>>>>> apply it :)
>>>>>>>>>
>>>>>>>>> Can you provide me with a working backport?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Steve,
>>>>>>>>
>>>>>>>> Is there a version of this fix available for stable kernels?
>>>>>>>>
>>>>>>>
>>>>>>> Hi Greg,
>>>>>>>
>>>>>>> Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84
>>>>>>> due to the issues that I have described in detail on this mail thread.
>>>>>>>
>>>>>>> Since there is no apparent fix for this bug on stable kernels, could
>>>>>>> you please consider reverting the original commit that caused this
>>>>>>> regression?
>>>>>>>
>>>>>>> That commit was intended to enhance security, which is probably why it
>>>>>>> was backported to stable kernels in the first place; but instead it
>>>>>>> ends up breaking basic functionality itself (mounting). So in the
>>>>>>> absence of a proper fix, I don't see much of an option but to revert
>>>>>>> that commit.
>>>>>>>
>>>>>>> So, please consider reverting the following:
>>>>>>>
>>>>>>> commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect
>>>>>>> against downgrade) even if signing off" on 4.4.118
>>>>>>>
>>>>>>> commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect
>>>>>>> against downgrade) even if signing off" on 4.9.84
>>>>>>>
>>>>>>> They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9
>>>>>>> upstream. Both these patches should revert cleanly.
>>>>>>
>>>>>> Do you still have this same problem on 4.14 and 4.15? If so, the issue
>>>>>> needs to get fixed there, not papered-over by reverting these old
>>>>>> changes, as you will hit the issue again in the future when you update
>>>>>> to a newer kernel version.
>>>>>>
>>>>>
>>>>> 4.14 and 4.15 work great! (I had mentioned this is in my original bug
>>>>> report but forgot to summarize it here, sorry).
>>>>
>>>>
>>>> Then what is the bugfix that should be applied here in order to keep
>>>> things working with these patches applied?
>>>>
>>>
>>> That would be the one mentioned in the subject line of this thread :)
>>> However, a working backport of that fix is not available for 4.4 and
>>> 4.9, hence the trouble.
>>>
>>> It looks like we are reconstructing elements of this email thread all
>>> over again, so let me quickly summarize the status so far:
>>>
>>> In 4.14/4.15/mainline,
>>> - commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against
>>> downgrade) even if signing off) caused mount regression with SMB v3.
>>>
>>> - commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must
>>> always be signed) fixed the issue.
>>>
>>> - [ There was a lot of code churn in the CIFS/SMB codebase between
>>> these two commits in mainline. ]
>>>
>>> In this email thread, you backported the fix to stable 4.13. Thomas
>>> noticed that the problematic commit had also made it to stable series
>>> such as 4.4 and 4.9, and requested a backport of the fix to those
>>> trees as well. However, a straight-forward backport of the fix to 4.4
>>> and 4.9 breaks the build, so no fix was available for those kernels.
>>>
>>> I investigated this and tried to produce a working backport of the fix
>>> to 4.4 and 4.9, but didn't succeed, despite trying several variations
>>> as well as suggestions from Aurelien [1][2]. So, given that there is
>>> still no known fix for the mount regression on 4.4 and 4.9 stable
>>> series at this point, I decided to request a revert of the problematic
>>> commit that caused the regression in those kernels.
>>>
>>> [1]. https://lkml.org/lkml/2018/1/3/892
>>> [2]. https://lkml.org/lkml/2018/1/29/1009
>>>
>>> Regards,
>>> Srivatsa
>>
>>
>>
>

--
Thanks,

Steve

2018-03-13 09:22:43

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On Sun, Mar 11, 2018 at 07:37:55PM -0700, Steve French wrote:
> Just got a wireshark trace - this is a fairly trivial issue (missing
> the validate negotiate must be signed patch) - I had some trouble
> getting this version of the kernel running (unrelated issue) and on
> systems with access to Windows 2016...

Ok, I have no idea what this means, or what I should do here because of
it :(

Any hints for what to do with the stable tree?

totally confused,

greg k-h

2018-03-13 15:24:12

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

There will be a fix needed to correct an oops in calc_signature,
besides the easy patch (smb3 validate negotiate patch).

On Tue, Mar 13, 2018 at 4:21 AM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Sun, Mar 11, 2018 at 07:37:55PM -0700, Steve French wrote:
>> Just got a wireshark trace - this is a fairly trivial issue (missing
>> the validate negotiate must be signed patch) - I had some trouble
>> getting this version of the kernel running (unrelated issue) and on
>> systems with access to Windows 2016...
>
> Ok, I have no idea what this means, or what I should do here because of
> it :(
>
> Any hints for what to do with the stable tree?
>
> totally confused,
>
> greg k-h

--
Thanks,

Steve

2018-03-16 13:34:11

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On Tue, Mar 13, 2018 at 10:21:45AM -0500, Steve French wrote:
> There will be a fix needed to correct an oops in calc_signature,
> besides the easy patch (smb3 validate negotiate patch).

Ok, I still have no idea how to parse this for a stable tree submission.

So can someone please just send me a simple "apply these git ids to tree
X.X.y so we can fix the problem", otherwise I'm not going to do anything
here as I'm really confused,

greg k-h

2018-03-22 02:03:59

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

Found a patch which solves the dependency issue. In my testing (on
4.9, with Windows 2016, and also to Samba) as Pavel suggested this
appears to fix the problem, but I will let Srivatsa confirm that it
also fixes it for him. The two attached patches for 4.9 should work.

As an aside which may help some in testing stable true problems (as a
point of comparison or alternative), I did a complete backport of all
relevant CIFS/SMB3 patches (ie all patches to cifs.ko that are not
dependent on a VFS changes or global kernel API changes) for kernels
4.9 through 4.15
https://github.com/smfrench/smb3-cifs-linux-stable-backports

The individual patches that were included (and in a distinct directory
all cifs patches that were rejected due to global/VFS dependencies)
are also checked in -
https://github.com/smfrench/smb3-backported-patches.

Given the focus on security, these two git trees may be useful for
those who want a cifs.ko which includes all security and functional
improvements and fixes that more closely matches mainline cifs.ko

Srivatsa,
Let us know if those two patches fix your issue as expected.

On Fri, Mar 16, 2018 at 8:32 AM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Tue, Mar 13, 2018 at 10:21:45AM -0500, Steve French wrote:
>> There will be a fix needed to correct an oops in calc_signature,
>> besides the easy patch (smb3 validate negotiate patch).
>
> Ok, I still have no idea how to parse this for a stable tree submission.
>
> So can someone please just send me a simple "apply these git ids to tree
> X.X.y so we can fix the problem", otherwise I'm not going to do anything
> here as I'm really confused,
>
> greg k-h

--
Thanks,

Steve

Attachments:

0001-SMB3-Validate-negotiate-request-must-always-be-signe.patch (1.19 kB)
0002-CIFS-Enable-encryption-during-session-setup-phase.patch (3.23 kB)
Download all attachments

2018-03-22 05:13:34

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 3/21/18 7:02 PM, Steve French wrote:
> Found a patch which solves the dependency issue. In my testing (on
> 4.9, with Windows 2016, and also to Samba) as Pavel suggested this
> appears to fix the problem, but I will let Srivatsa confirm that it
> also fixes it for him. The two attached patches for 4.9 should work.
>

Indeed, those two patches fix the problem for me on 4.9. Thanks a lot
Steve, Pavel and Aurelien for all your efforts in fixing this!

I was also interested in getting this fixed on 4.4, so I modified the
patches to apply on 4.4.88 and verified that they fix the mount
failure. I have attached my patches for 4.4 with this mail.

Steve, Pavel, could you kindly double-check the second patch for 4.4,
especially around the keygen_exit error path?

Thank you very much!

Regards,
Srivatsa
VMware Photon OS

Attachments:

v4.4-0001-SMB3-Validate-negotiate-request-must-always-be-signe.patch (1.29 kB)
v4.4-0002-CIFS-Enable-encryption-during-session-setup-phase.patch (3.06 kB)
Download all attachments

2018-03-22 05:17:23

[permalink] [raw]

Subject: Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

On 3/21/18 10:12 PM, Srivatsa S. Bhat wrote:
> On 3/21/18 7:02 PM, Steve French wrote:
>> Found a patch which solves the dependency issue. In my testing (on
>> 4.9, with Windows 2016, and also to Samba) as Pavel suggested this
>> appears to fix the problem, but I will let Srivatsa confirm that it
>> also fixes it for him. The two attached patches for 4.9 should work.
>>
>
> Indeed, those two patches fix the problem for me on 4.9. Thanks a lot
> Steve, Pavel and Aurelien for all your efforts in fixing this!
>
> I was also interested in getting this fixed on 4.4, so I modified the
> patches to apply on 4.4.88 and verified that they fix the mount

I meant to say 4.4.122 there (the latest stable 4.4 version at the moment).

Regards,
Srivatsa
VMware Photon OS

2018-03-22 10:35:07