All of lore.kernel.org
 help / color / mirror / Atom feed
* RFC: Revert move default dialect from CIFS to to SMB3"
@ 2017-08-31 21:01 ` Thorsten Leemhuis
  0 siblings, 0 replies; 24+ messages in thread
From: Thorsten Leemhuis @ 2017-08-31 21:01 UTC (permalink / raw)
  To: Steve French
  Cc: Linus Torvalds, Linux Kernel Mailing List,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, Pavel Shilovsky

This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
[SMB3] Improve security, move default dialect to SMB3 from old CIFS), 
as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599

It was a patch to improve security by switching to SMB3 by default and
support SMB1 (aka CIFS) only when explicitly requested, as the latter
is not considered secure anymore (see below for details). This is one of
the rare cases where regressions are unavoidable and accepted in Linux.
But that's bad enough already, so we at least should make it easy for
people to get an idea why something suddenly stopped working with a
newer kernel version. That's not the case, because due to eef914a9eb5e
a mount of a server that only supports CIFS/SMB1 with mount.cifs fails
with a misleading message:

> mount error(112): Host is down > Refer to the mount.cifs(8) manual
> page (e.g. man mount.cifs)

The corresponding message in the kernel log is just as unhelpful:

> CIFS VFS: cifs_mount failed w/return code = -112

This needs to be improved. Hence remove this for now, as the world won't
end suddenly if this gets delayed one or two cycles and resubmitted in
a way that leads to a more helpful error message.

For completeness, here are parts from the original patch description:

> Due to recent publicity about security vulnerabilities in the much
> older CIFS dialect, move the default dialect to the widely accepted
> (and quite secure) SMB3.0 dialect from the old default of the CIFS
> dialect.
>
> We do not want to be encouraging use of less secure dialects, and
> both Microsoft and CERT now strongly recommend not using the older
> CIFS dialect (SMB Security Best Practices "recommends disabling
> SMBv1").
>
> SMB3 is both secure and widely available: in Windows 8 and later,
> Samba and Macs.
>
> Users can still choose to explicitly mount with the less secure
> dialect (for old servers) by choosing "vers=1.0" on the cifs mount

Signed-off-by: Thorsten Leemhuis <linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org>
CC: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: Pavel Shilovsky <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
---
 fs/cifs/connect.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 59647eb..6ab261cd 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1272,9 +1272,9 @@ static int cifs_parse_security_flavors(char *value,
 
 	vol->actimeo = CIFS_DEF_ACTIMEO;
 
-	/* FIXME: add autonegotiation for SMB3 or later rather than just SMB3 */
-	vol->ops = &smb30_operations; /* both secure and accepted widely */
-	vol->vals = &smb30_values;
+	/* FIXME: add autonegotiation -- for now, SMB1 is default */
+	vol->ops = &smb1_operations;
+	vol->vals = &smb1_values;
 
 	vol->echo_interval = SMB_ECHO_INTERVAL_DEFAULT;
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* RFC: Revert move default dialect from CIFS to to SMB3"
@ 2017-08-31 21:01 ` Thorsten Leemhuis
  0 siblings, 0 replies; 24+ messages in thread
From: Thorsten Leemhuis @ 2017-08-31 21:01 UTC (permalink / raw)
  To: Steve French
  Cc: Linus Torvalds, Linux Kernel Mailing List, linux-cifs, Pavel Shilovsky

This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
[SMB3] Improve security, move default dialect to SMB3 from old CIFS), 
as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599

It was a patch to improve security by switching to SMB3 by default and
support SMB1 (aka CIFS) only when explicitly requested, as the latter
is not considered secure anymore (see below for details). This is one of
the rare cases where regressions are unavoidable and accepted in Linux.
But that's bad enough already, so we at least should make it easy for
people to get an idea why something suddenly stopped working with a
newer kernel version. That's not the case, because due to eef914a9eb5e
a mount of a server that only supports CIFS/SMB1 with mount.cifs fails
with a misleading message:

> mount error(112): Host is down > Refer to the mount.cifs(8) manual
> page (e.g. man mount.cifs)

The corresponding message in the kernel log is just as unhelpful:

> CIFS VFS: cifs_mount failed w/return code = -112

This needs to be improved. Hence remove this for now, as the world won't
end suddenly if this gets delayed one or two cycles and resubmitted in
a way that leads to a more helpful error message.

For completeness, here are parts from the original patch description:

> Due to recent publicity about security vulnerabilities in the much
> older CIFS dialect, move the default dialect to the widely accepted
> (and quite secure) SMB3.0 dialect from the old default of the CIFS
> dialect.
>
> We do not want to be encouraging use of less secure dialects, and
> both Microsoft and CERT now strongly recommend not using the older
> CIFS dialect (SMB Security Best Practices "recommends disabling
> SMBv1").
>
> SMB3 is both secure and widely available: in Windows 8 and later,
> Samba and Macs.
>
> Users can still choose to explicitly mount with the less secure
> dialect (for old servers) by choosing "vers=1.0" on the cifs mount

Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
CC: Steve French <smfrench@gmail.com>
CC: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/connect.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 59647eb..6ab261cd 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1272,9 +1272,9 @@ static int cifs_parse_security_flavors(char *value,
 
 	vol->actimeo = CIFS_DEF_ACTIMEO;
 
-	/* FIXME: add autonegotiation for SMB3 or later rather than just SMB3 */
-	vol->ops = &smb30_operations; /* both secure and accepted widely */
-	vol->vals = &smb30_values;
+	/* FIXME: add autonegotiation -- for now, SMB1 is default */
+	vol->ops = &smb1_operations;
+	vol->vals = &smb1_values;
 
 	vol->echo_interval = SMB_ECHO_INTERVAL_DEFAULT;
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-08-31 21:01 ` Thorsten Leemhuis
@ 2017-08-31 21:36     ` Thorsten Leemhuis
  -1 siblings, 0 replies; 24+ messages in thread
From: Thorsten Leemhuis @ 2017-08-31 21:36 UTC (permalink / raw)
  To: Steve French
  Cc: Linus Torvalds, Linux Kernel Mailing List,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, Pavel Shilovsky

Lo! To give a bit more background to this (the mail I reply to was the
first I sent with git send-email and I missed some details): Maybe I'm
over stretching my abilities/position as regression tracker with this
RFC for a revert, but I hope it at least triggers a discussion if such a
revert should be done or not.

The problem described in below patch description is known for a few
weeks already, but the bugzilla entry about it got ignored by the CIFS
developers; the same happened to a mail one affected user sent to
cifs-devel.

I should have send this revert RFC earlier, because I think more users
will be confused by the switch to SMB3, because the error messages they
get when trying to mount a SMB1/CIFS only share are not very helpful. I
myself had one system which regressed and made me look closer. And I
heard at least some widespread NAS boxes only support CIFS/SMB1 by
default :-/

Note: I only light-tested this revert and it worked fine for me (I still
could mount a CIFS/SMB1 only samba share and was still able to mount a
samba share on a different host with SMB3). But I'm not familiar with
the code I modify here. Hence someone who is (Steven?) IMHO should ACK
this before it gets applied.

Ciao, Thorsten


On 31.08.2017 23:01, Thorsten Leemhuis wrote:
> This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
> [SMB3] Improve security, move default dialect to SMB3 from old CIFS), 
> as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599
> 
> It was a patch to improve security by switching to SMB3 by default and
> support SMB1 (aka CIFS) only when explicitly requested, as the latter
> is not considered secure anymore (see below for details). This is one of
> the rare cases where regressions are unavoidable and accepted in Linux.
> But that's bad enough already, so we at least should make it easy for
> people to get an idea why something suddenly stopped working with a
> newer kernel version. That's not the case, because due to eef914a9eb5e
> a mount of a server that only supports CIFS/SMB1 with mount.cifs fails
> with a misleading message:
> 
>> mount error(112): Host is down > Refer to the mount.cifs(8) manual
>> page (e.g. man mount.cifs)
> 
> The corresponding message in the kernel log is just as unhelpful:
> 
>> CIFS VFS: cifs_mount failed w/return code = -112
> 
> This needs to be improved. Hence remove this for now, as the world won't
> end suddenly if this gets delayed one or two cycles and resubmitted in
> a way that leads to a more helpful error message.
> 
> For completeness, here are parts from the original patch description:
> 
>> Due to recent publicity about security vulnerabilities in the much
>> older CIFS dialect, move the default dialect to the widely accepted
>> (and quite secure) SMB3.0 dialect from the old default of the CIFS
>> dialect.
>>
>> We do not want to be encouraging use of less secure dialects, and
>> both Microsoft and CERT now strongly recommend not using the older
>> CIFS dialect (SMB Security Best Practices "recommends disabling
>> SMBv1").
>>
>> SMB3 is both secure and widely available: in Windows 8 and later,
>> Samba and Macs.
>>
>> Users can still choose to explicitly mount with the less secure
>> dialect (for old servers) by choosing "vers=1.0" on the cifs mount
> 
> Signed-off-by: Thorsten Leemhuis <linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org>
> CC: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Pavel Shilovsky <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
> ---
>  fs/cifs/connect.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 59647eb..6ab261cd 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1272,9 +1272,9 @@ static int cifs_parse_security_flavors(char *value,
>  
>  	vol->actimeo = CIFS_DEF_ACTIMEO;
>  
> -	/* FIXME: add autonegotiation for SMB3 or later rather than just SMB3 */
> -	vol->ops = &smb30_operations; /* both secure and accepted widely */
> -	vol->vals = &smb30_values;
> +	/* FIXME: add autonegotiation -- for now, SMB1 is default */
> +	vol->ops = &smb1_operations;
> +	vol->vals = &smb1_values;
>  
>  	vol->echo_interval = SMB_ECHO_INTERVAL_DEFAULT;
>  
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
@ 2017-08-31 21:36     ` Thorsten Leemhuis
  0 siblings, 0 replies; 24+ messages in thread
From: Thorsten Leemhuis @ 2017-08-31 21:36 UTC (permalink / raw)
  To: Steve French
  Cc: Linus Torvalds, Linux Kernel Mailing List, linux-cifs, Pavel Shilovsky

Lo! To give a bit more background to this (the mail I reply to was the
first I sent with git send-email and I missed some details): Maybe I'm
over stretching my abilities/position as regression tracker with this
RFC for a revert, but I hope it at least triggers a discussion if such a
revert should be done or not.

The problem described in below patch description is known for a few
weeks already, but the bugzilla entry about it got ignored by the CIFS
developers; the same happened to a mail one affected user sent to
cifs-devel.

I should have send this revert RFC earlier, because I think more users
will be confused by the switch to SMB3, because the error messages they
get when trying to mount a SMB1/CIFS only share are not very helpful. I
myself had one system which regressed and made me look closer. And I
heard at least some widespread NAS boxes only support CIFS/SMB1 by
default :-/

Note: I only light-tested this revert and it worked fine for me (I still
could mount a CIFS/SMB1 only samba share and was still able to mount a
samba share on a different host with SMB3). But I'm not familiar with
the code I modify here. Hence someone who is (Steven?) IMHO should ACK
this before it gets applied.

Ciao, Thorsten


On 31.08.2017 23:01, Thorsten Leemhuis wrote:
> This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
> [SMB3] Improve security, move default dialect to SMB3 from old CIFS), 
> as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599
> 
> It was a patch to improve security by switching to SMB3 by default and
> support SMB1 (aka CIFS) only when explicitly requested, as the latter
> is not considered secure anymore (see below for details). This is one of
> the rare cases where regressions are unavoidable and accepted in Linux.
> But that's bad enough already, so we at least should make it easy for
> people to get an idea why something suddenly stopped working with a
> newer kernel version. That's not the case, because due to eef914a9eb5e
> a mount of a server that only supports CIFS/SMB1 with mount.cifs fails
> with a misleading message:
> 
>> mount error(112): Host is down > Refer to the mount.cifs(8) manual
>> page (e.g. man mount.cifs)
> 
> The corresponding message in the kernel log is just as unhelpful:
> 
>> CIFS VFS: cifs_mount failed w/return code = -112
> 
> This needs to be improved. Hence remove this for now, as the world won't
> end suddenly if this gets delayed one or two cycles and resubmitted in
> a way that leads to a more helpful error message.
> 
> For completeness, here are parts from the original patch description:
> 
>> Due to recent publicity about security vulnerabilities in the much
>> older CIFS dialect, move the default dialect to the widely accepted
>> (and quite secure) SMB3.0 dialect from the old default of the CIFS
>> dialect.
>>
>> We do not want to be encouraging use of less secure dialects, and
>> both Microsoft and CERT now strongly recommend not using the older
>> CIFS dialect (SMB Security Best Practices "recommends disabling
>> SMBv1").
>>
>> SMB3 is both secure and widely available: in Windows 8 and later,
>> Samba and Macs.
>>
>> Users can still choose to explicitly mount with the less secure
>> dialect (for old servers) by choosing "vers=1.0" on the cifs mount
> 
> Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
> CC: Steve French <smfrench@gmail.com>
> CC: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>  fs/cifs/connect.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 59647eb..6ab261cd 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1272,9 +1272,9 @@ static int cifs_parse_security_flavors(char *value,
>  
>  	vol->actimeo = CIFS_DEF_ACTIMEO;
>  
> -	/* FIXME: add autonegotiation for SMB3 or later rather than just SMB3 */
> -	vol->ops = &smb30_operations; /* both secure and accepted widely */
> -	vol->vals = &smb30_values;
> +	/* FIXME: add autonegotiation -- for now, SMB1 is default */
> +	vol->ops = &smb1_operations;
> +	vol->vals = &smb1_values;
>  
>  	vol->echo_interval = SMB_ECHO_INTERVAL_DEFAULT;
>  
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3"
       [not found] ` <1504213298-27431-1-git-send-email-linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org>
  2017-08-31 21:36     ` Thorsten Leemhuis
@ 2017-09-01  0:03   ` L. A. Walsh
       [not found]     ` <59A8A3E2.40804-gT3AUAsYRbTYtjvyW6yDsg@public.gmane.org>
  1 sibling, 1 reply; 24+ messages in thread
From: L. A. Walsh @ 2017-09-01  0:03 UTC (permalink / raw)
  To: Thorsten Leemhuis; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

Thorsten Leemhuis wrote:
> This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
> [SMB3] Improve security, move default dialect to SMB3 from old CIFS), 
> as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599
>
> It was a patch to improve security by switching to SMB3 by default and
> support SMB1 (aka CIFS) only when explicitly requested, as the latter
> is not considered secure anymore (see below for details). This is one of
> the rare cases where regressions are unavoidable and accepted in Linux.
>   
----
    Why not SMB2.1?  Win7 is still in support and getting security updates.
MS has not issued any updates for Win7 upgrading it to SMB3.0 for any
reason (that I'm aware of) -- including security. 

    If there were security problems in Win7 w/SMB2.1, wouldn't MS
issue patches -- as they did for WinXP just recently for a severe
SMB1 bug?

    Seems like if they are willing to patch "out of support" XP, for
a serious problem, then they would be more likely to patch Win7 for
lesser problems.

    Seems like jumping the default to MS's latest and greatest puts
linux on MS's OS-release schedule -- especially when they haven't declared
SMB2.1 as "bad"...  From what I understand, most of the new security
features in 3.0 when into SMB2.1 or 2.0.

>> SMB3 is both secure and widely available: in Windows 8 and later,
>> Samba and Macs.
>>     
----
    I can't find more recent stats than last Dec, but Win7 had between
2-3X the number of Win8 users AND Win7 had between 40-100% more uses
than Win10.  Win 8 was pretty much a non-starter.
(http://www.zdnet.com/article/windows-10-versus-windows-7-whose-numbers-do-you-trust/)

As of March 2017, another article showed Win7 growing w/r/t Win10:
(https://www.theinquirer.net/inquirer/news/3005602/windows-7-market-share-rises-at-the-expense-of-windows-10)

    I can't say moving the default away from SMB1 seems like a bad thing 
-- especially if the error messages can be improved.  Besides security, its
notably slower, but many home devices still use SMB1 -- which is *fine*,
if they are not exposed to the outside net.  Then again, I've never
put a Windows machine facing the internet -- don't think they are security
enough -- use linux for that.


>
>   

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3"
  2017-08-31 21:01 ` Thorsten Leemhuis
  (?)
  (?)
@ 2017-09-01  0:04 ` L. A. Walsh
  2017-09-01  0:41   ` Steve French
  -1 siblings, 1 reply; 24+ messages in thread
From: L. A. Walsh @ 2017-09-01  0:04 UTC (permalink / raw)
  Cc: Steve French, Linus Torvalds, Linux Kernel Mailing List, Pavel Shilovsky

Thorsten Leemhuis wrote:
> This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
> [SMB3] Improve security, move default dialect to SMB3 from old CIFS), 
> as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599
>
> It was a patch to improve security by switching to SMB3 by default and
> support SMB1 (aka CIFS) only when explicitly requested, as the latter
> is not considered secure anymore (see below for details). This is one of
> the rare cases where regressions are unavoidable and accepted in Linux.
>   
----
    Why not SMB2.1?  Win7 is still in support and getting security updates.
MS has not issued any updates for Win7 upgrading it to SMB3.0 for any
reason (that I'm aware of) -- including security. 

    If there were security problems in Win7 w/SMB2.1, wouldn't MS
issue patches -- as they did for WinXP just recently for a severe
SMB1 bug?

    Seems like if they are willing to patch "out of support" XP, for
a serious problem, then they would be more likely to patch Win7 for
lesser problems.

    Seems like jumping the default to MS's latest and greatest puts
linux on MS's OS-release schedule -- especially when they haven't declared
SMB2.1 as "bad"...  From what I understand, most of the new security
features in 3.0 when into SMB2.1 or 2.0.

>> SMB3 is both secure and widely available: in Windows 8 and later,
>> Samba and Macs.
>>     
----
    I can't find more recent stats than last Dec, but Win7 had between
2-3X the number of Win8 users AND Win7 had between 40-100% more uses
than Win10.  Win 8 was pretty much a non-starter.
(http://www.zdnet.com/article/windows-10-versus-windows-7-whose-numbers-do-you-trust/)

As of March 2017, another article showed Win7 growing w/r/t Win10:
(https://www.theinquirer.net/inquirer/news/3005602/windows-7-market-share-rises-at-the-expense-of-windows-10)

    I can't say moving the default away from SMB1 seems like a bad thing 
-- especially if the error messages can be improved.  Besides security, its
notably slower, but many home devices still use SMB1 -- which is *fine*,
if they are not exposed to the outside net.  Then again, I've never
put a Windows machine facing the internet -- don't think they are security
enough -- use linux for that.


>
>   

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-08-31 21:36     ` Thorsten Leemhuis
  (?)
@ 2017-09-01  0:12     ` Linus Torvalds
  2017-09-01  0:29       ` Steve French
  2017-09-01 18:23       ` L. A. Walsh
  -1 siblings, 2 replies; 24+ messages in thread
From: Linus Torvalds @ 2017-09-01  0:12 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Steve French, Linux Kernel Mailing List, linux-cifs, Pavel Shilovsky

On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux@leemhuis.info> wrote:
> Lo! To give a bit more background to this (the mail I reply to was the
> first I sent with git send-email and I missed some details): Maybe I'm
> over stretching my abilities/position as regression tracker with this
> RFC for a revert, but I hope it at least triggers a discussion if such a
> revert should be done or not.

I don't think that a revert is appropriate.

But perhaps just a single printk() or something if the user does *not*
specify the version explicitly? Just saying something like

  We used to default to 1.0, we now default to 3.0, if you want old
defaults, use "vers=1.0"

Oh, looking at that version parsing code, I think we also need to fix
that legacy "ver=1" thing (ver without the 's') which now silently
ignores "ver=1" as being the "default", even though it's not.

I do *not* believe that "default to version 1" is acceptable.

                Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-01  0:12     ` Linus Torvalds
@ 2017-09-01  0:29       ` Steve French
       [not found]         ` <CAH2r5msWDXzwbFPtUHCKbqHrEBTsvw5eaTayj5RkdgYCLM5nAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2017-09-01 18:23       ` L. A. Walsh
  1 sibling, 1 reply; 24+ messages in thread
From: Steve French @ 2017-09-01  0:29 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Thorsten Leemhuis, Linux Kernel Mailing List, linux-cifs,
	Pavel Shilovsky, Jeff Layton

Yes - updating the parsing slightly and printks as suggested makes sense

Some additional warning messages in the userspace helper (adding Jeff
Layton), mount.cifs can also help.

I also have an experimental set of patches to allow multi-dialect
negotiation with at least three of the acceptable dialects
(smb2.1/smb3/smb3.02) which will help, but complicate secure dialect
validation ("validate negotiate") but that will have to wait till next
release.

On Thu, Aug 31, 2017 at 7:12 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> Lo! To give a bit more background to this (the mail I reply to was the
>> first I sent with git send-email and I missed some details): Maybe I'm
>> over stretching my abilities/position as regression tracker with this
>> RFC for a revert, but I hope it at least triggers a discussion if such a
>> revert should be done or not.
>
> I don't think that a revert is appropriate.
>
> But perhaps just a single printk() or something if the user does *not*
> specify the version explicitly? Just saying something like
>
>   We used to default to 1.0, we now default to 3.0, if you want old
> defaults, use "vers=1.0"
>
> Oh, looking at that version parsing code, I think we also need to fix
> that legacy "ver=1" thing (ver without the 's') which now silently
> ignores "ver=1" as being the "default", even though it's not.
>
> I do *not* believe that "default to version 1" is acceptable.
>
>                 Linus



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3"
  2017-09-01  0:04 ` L. A. Walsh
@ 2017-09-01  0:41   ` Steve French
  0 siblings, 0 replies; 24+ messages in thread
From: Steve French @ 2017-09-01  0:41 UTC (permalink / raw)
  To: L. A. Walsh; +Cc: Linus Torvalds, Linux Kernel Mailing List, Pavel Shilovsky

On Thu, Aug 31, 2017 at 7:04 PM, L. A. Walsh <linux-cifs@tlinx.org> wrote:
> Thorsten Leemhuis wrote:
>>
>> This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
>> [SMB3] Improve security, move default dialect to SMB3 from old CIFS), as
>> it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=196599
>>
>> It was a patch to improve security by switching to SMB3 by default and
>> support SMB1 (aka CIFS) only when explicitly requested, as the latter
>> is not considered secure anymore (see below for details). This is one of
>> the rare cases where regressions are unavoidable and accepted in Linux.
>>
>
> ----
>    Why not SMB2.1?  Win7 is still in support and getting security updates.
> MS has not issued any updates for Win7 upgrading it to SMB3.0 for any
> reason (that I'm aware of) -- including security.
>    If there were security problems in Win7 w/SMB2.1, wouldn't MS
> issue patches -- as they did for WinXP just recently for a severe
> SMB1 bug?

This was discussed at length with Microsoft, others on the Samba team,
and various vendors at the last SMB3 test event, and the general
opinion was that we should move to:

1) multi-dialect negotiate starting at the minimum 'secure-enough'
dialect (distros can already do this by patching the user space tools,
with retry), offering SMB2.1 through SMB3.02 (secure SMB3.1.1 requires
an additional patch that is not ready) but this was not ready for 4.13
due to difficulties with the "validate negotiate" handling.  It is
planned for next release (move from SMB3 default to SMB2.1 through
SMB3.11)

2) In the interim, given the seriousness of security issues around
older dialects, pick the best compromise as the default and SMB3 was
considered more secure (it offers encryption e.g. which is required
for some modern servers like Azure).

There was some urgency in making this change for obvious reasons, but
it should be easier in 4.14 (and backport to stable presumably) when
multidialect support is complete.




-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
       [not found]         ` <CAH2r5msWDXzwbFPtUHCKbqHrEBTsvw5eaTayj5RkdgYCLM5nAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-09-01  2:42           ` Steve French
       [not found]             ` <CAH2r5mv9roEvMX+C-csU=GZFM_HMbqxnHfF11NUp+2yonDVPgA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Steve French @ 2017-09-01  2:42 UTC (permalink / raw)
  To: Pavel Shilovskiy, ronnie sahlberg
  Cc: Thorsten Leemhuis, linux-cifs-u79uwXL29TY76Z2rM5mHXA, Jeff Layton

Any thoughts on this patch to add additional warnings for the user -
logging when using default dialects (or when server returns dialect
not supported), and noting the default dialect change?

See https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=bb86f22eeddbb5879675b55168b8fa8990d74a21

On Thu, Aug 31, 2017 at 7:29 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Yes - updating the parsing slightly and printks as suggested makes sense
>
> Some additional warning messages in the userspace helper (adding Jeff
> Layton), mount.cifs can also help.
>
> I also have an experimental set of patches to allow multi-dialect
> negotiation with at least three of the acceptable dialects
> (smb2.1/smb3/smb3.02) which will help, but complicate secure dialect
> validation ("validate negotiate") but that will have to wait till next
> release.
>
> On Thu, Aug 31, 2017 at 7:12 PM, Linus Torvalds
> <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org> wrote:
>>> Lo! To give a bit more background to this (the mail I reply to was the
>>> first I sent with git send-email and I missed some details): Maybe I'm
>>> over stretching my abilities/position as regression tracker with this
>>> RFC for a revert, but I hope it at least triggers a discussion if such a
>>> revert should be done or not.
>>
>> I don't think that a revert is appropriate.
>>
>> But perhaps just a single printk() or something if the user does *not*
>> specify the version explicitly? Just saying something like
>>
>>   We used to default to 1.0, we now default to 3.0, if you want old
>> defaults, use "vers=1.0"
>>
>> Oh, looking at that version parsing code, I think we also need to fix
>> that legacy "ver=1" thing (ver without the 's') which now silently
>> ignores "ver=1" as being the "default", even though it's not.
>>
>> I do *not* believe that "default to version 1" is acceptable.
>>
>>                 Linus
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
       [not found]             ` <CAH2r5mv9roEvMX+C-csU=GZFM_HMbqxnHfF11NUp+2yonDVPgA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-09-01  3:06               ` ronnie sahlberg
  2017-09-01 11:07               ` Jeff Layton
  2017-09-02 14:25                 ` Thorsten Leemhuis
  2 siblings, 0 replies; 24+ messages in thread
From: ronnie sahlberg @ 2017-09-01  3:06 UTC (permalink / raw)
  To: Steve French
  Cc: Pavel Shilovskiy, Thorsten Leemhuis,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, Jeff Layton

Two missing \n in connect.c

Fix those and you can add a
Reviewed-by: Ronnie Sahlberg <lsahlber-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Sep 1, 2017 at 12:42 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Any thoughts on this patch to add additional warnings for the user -
> logging when using default dialects (or when server returns dialect
> not supported), and noting the default dialect change?
>
> See https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=bb86f22eeddbb5879675b55168b8fa8990d74a21
>
> On Thu, Aug 31, 2017 at 7:29 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Yes - updating the parsing slightly and printks as suggested makes sense
>>
>> Some additional warning messages in the userspace helper (adding Jeff
>> Layton), mount.cifs can also help.
>>
>> I also have an experimental set of patches to allow multi-dialect
>> negotiation with at least three of the acceptable dialects
>> (smb2.1/smb3/smb3.02) which will help, but complicate secure dialect
>> validation ("validate negotiate") but that will have to wait till next
>> release.
>>
>> On Thu, Aug 31, 2017 at 7:12 PM, Linus Torvalds
>> <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>>> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org> wrote:
>>>> Lo! To give a bit more background to this (the mail I reply to was the
>>>> first I sent with git send-email and I missed some details): Maybe I'm
>>>> over stretching my abilities/position as regression tracker with this
>>>> RFC for a revert, but I hope it at least triggers a discussion if such a
>>>> revert should be done or not.
>>>
>>> I don't think that a revert is appropriate.
>>>
>>> But perhaps just a single printk() or something if the user does *not*
>>> specify the version explicitly? Just saying something like
>>>
>>>   We used to default to 1.0, we now default to 3.0, if you want old
>>> defaults, use "vers=1.0"
>>>
>>> Oh, looking at that version parsing code, I think we also need to fix
>>> that legacy "ver=1" thing (ver without the 's') which now silently
>>> ignores "ver=1" as being the "default", even though it's not.
>>>
>>> I do *not* believe that "default to version 1" is acceptable.
>>>
>>>                 Linus
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>
>
>
> --
> Thanks,
>
> Steve

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3"
       [not found]     ` <59A8A3E2.40804-gT3AUAsYRbTYtjvyW6yDsg@public.gmane.org>
@ 2017-09-01  3:11       ` Andrew Bartlett
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Bartlett @ 2017-09-01  3:11 UTC (permalink / raw)
  To: L. A. Walsh, Thorsten Leemhuis; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Thu, 2017-08-31 at 17:03 -0700, L. A. Walsh wrote:
> Thorsten Leemhuis wrote:
> > This reverts commit eef914a9eb5eb83e60eb498315a491cd1edc13a1 (
> > [SMB3] Improve security, move default dialect to SMB3 from old
> > CIFS), 
> > as it confuses users: https://bugzilla.kernel.org/show_bug.cgi?id=1
> > 96599
> > 
> > It was a patch to improve security by switching to SMB3 by default
> > and
> > support SMB1 (aka CIFS) only when explicitly requested, as the
> > latter
> > is not considered secure anymore (see below for details). This is
> > one of
> > the rare cases where regressions are unavoidable and accepted in
> > Linux.
> >   
> 
> ----
>     Why not SMB2.1?  Win7 is still in support and getting security
> updates.
> MS has not issued any updates for Win7 upgrading it to SMB3.0 for any
> reason (that I'm aware of) -- including security. 
> 
>     If there were security problems in Win7 w/SMB2.1, wouldn't MS
> issue patches -- as they did for WinXP just recently for a severe
> SMB1 bug?

To be clear, the issue with SMB1 is lack of integrity protection, in
particular on the negotiation, where a client may come to think it had
to use NTLM(v2) to talk to the server. 

The only protection available on SMB1 is 'smb signing', which is after
the negotiation of the authentication protocols etc.

>     Seems like if they are willing to patch "out of support" XP, for
> a serious problem, then they would be more likely to patch Win7 for
> lesser problems.
> 
>     Seems like jumping the default to MS's latest and greatest puts
> linux on MS's OS-release schedule -- especially when they haven't
> declared
> SMB2.1 as "bad"...  From what I understand, most of the new security
> features in 3.0 when into SMB2.1 or 2.0.

Sadly 'most' turned out not to be good enough to actually secure the
negotiation, which is the main weakness.   If a change is to be made, I
think it should move up to SMB3. 

In terms of Windows versions, Windows 2012R2 is quite popular these
days, even if Windows 8.1 still wasn't a great hit. 

Finally, I do agree that the move from SMB1 is partly on the basis of a
false premise.  It appears that Microsoft's recent declaration of SMB1
as 'bad' is as much on the basis of coding flaws in their own SMB1
implementation as the lack of protection.  It is hoped that more modern
code might have less buffer-overflow style security flaws compared with
the 25 year old stuff.

Both however are good reasons to move off the SMB1 protocol towards
SMB3 on the server, and this is easiest and safest to do once the
clients also move to SMB3 by default.

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
       [not found]             ` <CAH2r5mv9roEvMX+C-csU=GZFM_HMbqxnHfF11NUp+2yonDVPgA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2017-09-01  3:06               ` ronnie sahlberg
@ 2017-09-01 11:07               ` Jeff Layton
  2017-09-02 14:25                 ` Thorsten Leemhuis
  2 siblings, 0 replies; 24+ messages in thread
From: Jeff Layton @ 2017-09-01 11:07 UTC (permalink / raw)
  To: Steve French, Pavel Shilovskiy, ronnie sahlberg
  Cc: Thorsten Leemhuis, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Thu, 2017-08-31 at 21:42 -0500, Steve French wrote:
> Any thoughts on this patch to add additional warnings for the user -
> logging when using default dialects (or when server returns dialect
> not supported), and noting the default dialect change?
> 
> See https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=bb86f22eeddbb5879675b55168b8fa8990d74a21
> 

Breaking backward compatability sucks, but I agree that there's no real
alternative here. SMB1 is just not a safe default these days.

> On Thu, Aug 31, 2017 at 7:29 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Yes - updating the parsing slightly and printks as suggested makes sense
> > 
> > Some additional warning messages in the userspace helper (adding Jeff
> > Layton), mount.cifs can also help.
> > 

What do you suggest here?

> > I also have an experimental set of patches to allow multi-dialect
> > negotiation with at least three of the acceptable dialects
> > (smb2.1/smb3/smb3.02) which will help, but complicate secure dialect
> > validation ("validate negotiate") but that will have to wait till next
> > release.
> > 

That seems like the best way to fix this. If you fail to negotiate any
dialect, throw a warning then.


> > On Thu, Aug 31, 2017 at 7:12 PM, Linus Torvalds
> > <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> > > On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org> wrote:
> > > > Lo! To give a bit more background to this (the mail I reply to was the
> > > > first I sent with git send-email and I missed some details): Maybe I'm
> > > > over stretching my abilities/position as regression tracker with this
> > > > RFC for a revert, but I hope it at least triggers a discussion if such a
> > > > revert should be done or not.
> > > 
> > > I don't think that a revert is appropriate.
> > > 
> > > But perhaps just a single printk() or something if the user does *not*
> > > specify the version explicitly? Just saying something like
> > > 
> > >   We used to default to 1.0, we now default to 3.0, if you want old
> > > defaults, use "vers=1.0"
> > > 
> > > Oh, looking at that version parsing code, I think we also need to fix
> > > that legacy "ver=1" thing (ver without the 's') which now silently
> > > ignores "ver=1" as being the "default", even though it's not.
> > > 
> > > I do *not* believe that "default to version 1" is acceptable.
> > > 
> > >                 Linus
> > 
> > 
> > 
> > --
> > Thanks,
> > 
> > Steve
> 
> 
> 

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-01  0:12     ` Linus Torvalds
  2017-09-01  0:29       ` Steve French
@ 2017-09-01 18:23       ` L. A. Walsh
       [not found]         ` <59A9A59E.6040205-gT3AUAsYRbTYtjvyW6yDsg@public.gmane.org>
  1 sibling, 1 reply; 24+ messages in thread
From: L. A. Walsh @ 2017-09-01 18:23 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Thorsten Leemhuis, Steve French, Linux Kernel Mailing List,
	linux-cifs, Pavel Shilovsky

Linus Torvalds wrote:
> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>   
>> Lo! To give a bit more background to this (the mail I reply to was the
>> first I sent with git send-email and I missed some details): Maybe I'm
>> over stretching my abilities/position as regression tracker with this
>> RFC for a revert, but I hope it at least triggers a discussion if such a
>> revert should be done or not.
>>     
>
> I don't think that a revert is appropriate.
>
> But perhaps just a single printk() or something if the user does *not*
> specify the version explicitly? Just saying something like
>
>   We used to default to 1.0, we now default to 3.0, if you want old
> defaults, use "vers=1.0"
>   
----
    Why be incompatible with the majority of Windows installations?
I.e.  If you really want to up security from 1.0 (not adverse to that),
then why not go to 2.1 as used by Win7?  Win7 is still in support
from MS -- and they haven't indicated a need to upgrade to 3.x for
security reasons.  3.x may have new security features, no argument, but
that doesn't mean 2.1, is insecure.


> I do *not* believe that "default to version 1" is acceptable.
>   
---
    But does it have to jump to 3?  I.e. Why not go a more middle
route of 2.1 -- as it is still security-supported by MS.  Ideally
MS would find some bug in 2.1 and allow 3.x to be an upgrade to Win7,
but until then...

Linda

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-01 18:23       ` L. A. Walsh
@ 2017-09-01 19:45             ` Linus Torvalds
  0 siblings, 0 replies; 24+ messages in thread
From: Linus Torvalds @ 2017-09-01 19:45 UTC (permalink / raw)
  To: L. A. Walsh
  Cc: Thorsten Leemhuis, Steve French, Linux Kernel Mailing List,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, Pavel Shilovsky

On Fri, Sep 1, 2017 at 11:23 AM, L. A. Walsh <linux-cifs-gT3AUAsYRbTYtjvyW6yDsg@public.gmane.org> wrote:
>    Why be incompatible with the majority of Windows installations?
> I.e.  If you really want to up security from 1.0 (not adverse to that),
> then why not go to 2.1 as used by Win7?  Win7 is still in support
> from MS -- and they haven't indicated a need to upgrade to 3.x for
> security reasons.  3.x may have new security features, no argument, but
> that doesn't mean 2.1, is insecure.

I'm certainly ok with changing the default to 2.1 if that helps people.

Is that actually likely to help the people who now see problems with
the existing 3.0 default?

I don't know the exact security issue details with cifs, but I _think_
it was explicitly _only_ smb-1.0, right?

                   Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
@ 2017-09-01 19:45             ` Linus Torvalds
  0 siblings, 0 replies; 24+ messages in thread
From: Linus Torvalds @ 2017-09-01 19:45 UTC (permalink / raw)
  To: L. A. Walsh
  Cc: Thorsten Leemhuis, Steve French, Linux Kernel Mailing List,
	linux-cifs, Pavel Shilovsky

On Fri, Sep 1, 2017 at 11:23 AM, L. A. Walsh <linux-cifs@tlinx.org> wrote:
>    Why be incompatible with the majority of Windows installations?
> I.e.  If you really want to up security from 1.0 (not adverse to that),
> then why not go to 2.1 as used by Win7?  Win7 is still in support
> from MS -- and they haven't indicated a need to upgrade to 3.x for
> security reasons.  3.x may have new security features, no argument, but
> that doesn't mean 2.1, is insecure.

I'm certainly ok with changing the default to 2.1 if that helps people.

Is that actually likely to help the people who now see problems with
the existing 3.0 default?

I don't know the exact security issue details with cifs, but I _think_
it was explicitly _only_ smb-1.0, right?

                   Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-01 19:45             ` Linus Torvalds
  (?)
@ 2017-09-02  2:16             ` Steve French
  2017-09-02  3:56               ` Linus Torvalds
  -1 siblings, 1 reply; 24+ messages in thread
From: Steve French @ 2017-09-02  2:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: L. A. Walsh, Thorsten Leemhuis, Linux Kernel Mailing List,
	linux-cifs, Pavel Shilovsky

On Fri, Sep 1, 2017 at 2:45 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Sep 1, 2017 at 11:23 AM, L. A. Walsh <linux-cifs@tlinx.org> wrote:
>>    Why be incompatible with the majority of Windows installations?
>> I.e.  If you really want to up security from 1.0 (not adverse to that),
>> then why not go to 2.1 as used by Win7?  Win7 is still in support
>> from MS -- and they haven't indicated a need to upgrade to 3.x for
>> security reasons.  3.x may have new security features, no argument, but
>> that doesn't mean 2.1, is insecure.
>
> I'm certainly ok with changing the default to 2.1 if that helps people.
>
> Is that actually likely to help the people who now see problems with
> the existing 3.0 default?
>
> I don't know the exact security issue details with cifs, but I _think_
> it was explicitly _only_ smb-1.0, right?

The default was SMB1 (CIFS) and was recently changed to SMB3.
The dialect still can be overridden by specifying "vers=1.0" or "vers=2.1"
etc. on mount.

We just put together a patch to better explain the default changes
(with additional warning messages) as suggested.

SMB3 is significantly better than SMB2.1 (supporting encrypted shares
and sessions for example, and requiring support for "secure negotiate")
and some servers require SMB3 minimum as a result, but it was agreed
at the last test event to eventually support multi-dialect negotiation (for
which SMB2.1 e.g. Windows 7, would be the oldest and least secure
dialect we would support) but in this interim stage we had to pick one,
and the improvements in SMB3 (over SMB2.1) tipped the balance.

In 4.14 we will likely have the ability to more securely do multi-dialect
negotiation, and this issue of SMB2.1 vs. SMB3 will be moot as the
server will choose its most recent dialect.


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-02  2:16             ` Steve French
@ 2017-09-02  3:56               ` Linus Torvalds
       [not found]                 ` <CA+55aFwUHLxBhOh7DxtjSSnKX6KBj+k+p=_CzE8i_xgq-LNj0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2017-09-02  3:56 UTC (permalink / raw)
  To: Steve French
  Cc: L. A. Walsh, Thorsten Leemhuis, Linux Kernel Mailing List,
	linux-cifs, Pavel Shilovsky

On Fri, Sep 1, 2017 at 7:16 PM, Steve French <smfrench@gmail.com> wrote:
>
> The default was SMB1 (CIFS) and was recently changed to SMB3.
> The dialect still can be overridden by specifying "vers=1.0" or "vers=2.1"
> etc. on mount.
>
> We just put together a patch to better explain the default changes
> (with additional warning messages) as suggested.
>
> SMB3 is significantly better than SMB2.1 (supporting encrypted shares
> and sessions for example, and requiring support for "secure negotiate")
> and some servers require SMB3 minimum as a result,

The default shouldn't be about "best and most secure", but "most
convenient, while still not actively *IN*secure"

So "some servers require 3.0" may be true, but if it's also the case
that "most servers still don't do 3.0 at all", then it's a "some" vs
"most".

Which is the most common one? That should be the default.

I realize that eventually we'll have auto-negotiation, but that's
clearly not for 4.13. So in the meantime the only issue is what the
right default should be without auto-negotiation.

So it should be about what the failure rate is. If trying for smb3 has
a high failure rate because people simply don't have that yet, then
making that the default was clearly the wrong choice.

Because being "better" is immaterial if it doesn't work.

              Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-02  3:56               ` Linus Torvalds
@ 2017-09-02  5:22                     ` Andrew Bartlett
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Bartlett @ 2017-09-02  5:22 UTC (permalink / raw)
  To: Linus Torvalds, Steve French
  Cc: L. A. Walsh, Thorsten Leemhuis, Linux Kernel Mailing List,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, Pavel Shilovsky

On Fri, 2017-09-01 at 20:56 -0700, Linus Torvalds wrote:
> On Fri, Sep 1, 2017 at 7:16 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > 
> > The default was SMB1 (CIFS) and was recently changed to SMB3.
> > The dialect still can be overridden by specifying "vers=1.0" or "vers=2.1"
> > etc. on mount.
> > 
> > We just put together a patch to better explain the default changes
> > (with additional warning messages) as suggested.
> > 
> > SMB3 is significantly better than SMB2.1 (supporting encrypted shares
> > and sessions for example, and requiring support for "secure negotiate")
> > and some servers require SMB3 minimum as a result,
> 
> The default shouldn't be about "best and most secure", but "most
> convenient, while still not actively *IN*secure"
> 
> So "some servers require 3.0" may be true, but if it's also the case
> that "most servers still don't do 3.0 at all", then it's a "some" vs
> "most".
> 
> Which is the most common one? That should be the default.
> 
> I realize that eventually we'll have auto-negotiation, but that's
> clearly not for 4.13. So in the meantime the only issue is what the
> right default should be without auto-negotiation.
> 
> So it should be about what the failure rate is. If trying for smb3 has
> a high failure rate because people simply don't have that yet, then
> making that the default was clearly the wrong choice.
> 
> Because being "better" is immaterial if it doesn't work.

My quick research shows:

SMB 2.1 but not SMB3 is on:
 Windows 7
 Windows 8
 Windows 2008
 Windows 2012
 Samba 3.6 and earlier (SMB1 only by default)

SMB3 is on:
 Windows 8.1
 Windows 2012 R2
 Windows 10
 Windows 2016 
 Samba 4.0 and above (released 2012)

I'm not sure exactly which Mac versions.  

Some breakage will be old (and quite recent) NAS and routers that use
SMB1 and often very old Samba, but most of those only do SMB1. 

In terms of 'actively *IN*secure', it really depends what you mean by
that.  

That is, like all big changes, the movement against SMB1 has had
multiple motivations:
 - a desire no longer to expose really old code in Windows (recently
exploited)
 - a desire to retire an old and complex protocol
 - SMB3 having proper secure negotiation (I'll leave it to Steve to
explain the difference between SMB2 and 3 in that respect, I'm not so
current on the details). 

I hope this help,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
@ 2017-09-02  5:22                     ` Andrew Bartlett
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Bartlett @ 2017-09-02  5:22 UTC (permalink / raw)
  To: Linus Torvalds, Steve French
  Cc: L. A. Walsh, Thorsten Leemhuis, Linux Kernel Mailing List,
	linux-cifs, Pavel Shilovsky

On Fri, 2017-09-01 at 20:56 -0700, Linus Torvalds wrote:
> On Fri, Sep 1, 2017 at 7:16 PM, Steve French <smfrench@gmail.com> wrote:
> > 
> > The default was SMB1 (CIFS) and was recently changed to SMB3.
> > The dialect still can be overridden by specifying "vers=1.0" or "vers=2.1"
> > etc. on mount.
> > 
> > We just put together a patch to better explain the default changes
> > (with additional warning messages) as suggested.
> > 
> > SMB3 is significantly better than SMB2.1 (supporting encrypted shares
> > and sessions for example, and requiring support for "secure negotiate")
> > and some servers require SMB3 minimum as a result,
> 
> The default shouldn't be about "best and most secure", but "most
> convenient, while still not actively *IN*secure"
> 
> So "some servers require 3.0" may be true, but if it's also the case
> that "most servers still don't do 3.0 at all", then it's a "some" vs
> "most".
> 
> Which is the most common one? That should be the default.
> 
> I realize that eventually we'll have auto-negotiation, but that's
> clearly not for 4.13. So in the meantime the only issue is what the
> right default should be without auto-negotiation.
> 
> So it should be about what the failure rate is. If trying for smb3 has
> a high failure rate because people simply don't have that yet, then
> making that the default was clearly the wrong choice.
> 
> Because being "better" is immaterial if it doesn't work.

My quick research shows:

SMB 2.1 but not SMB3 is on:
 Windows 7
 Windows 8
 Windows 2008
 Windows 2012
 Samba 3.6 and earlier (SMB1 only by default)

SMB3 is on:
 Windows 8.1
 Windows 2012 R2
 Windows 10
 Windows 2016 
 Samba 4.0 and above (released 2012)

I'm not sure exactly which Mac versions.  

Some breakage will be old (and quite recent) NAS and routers that use
SMB1 and often very old Samba, but most of those only do SMB1. 

In terms of 'actively *IN*secure', it really depends what you mean by
that.  

That is, like all big changes, the movement against SMB1 has had
multiple motivations:
 - a desire no longer to expose really old code in Windows (recently
exploited)
 - a desire to retire an old and complex protocol
 - SMB3 having proper secure negotiation (I'll leave it to Steve to
explain the difference between SMB2 and 3 in that respect, I'm not so
current on the details). 

I hope this help,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-01  2:42           ` Steve French
@ 2017-09-02 14:25                 ` Thorsten Leemhuis
  0 siblings, 0 replies; 24+ messages in thread
From: Thorsten Leemhuis @ 2017-09-02 14:25 UTC (permalink / raw)
  To: Steve French, Pavel Shilovskiy, ronnie sahlberg
  Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, Jeff Layton

Hi! Just a quick feedback from my side.

After reading Andrew explanation in this thread about the "movement
against SMB1" I kind of think "maybe the proposed revert for 4.13 and
doing it properly in 4.14 would really have been a good fit". But
whatever, doesn't bother me much any more:

Steve French wrote on 01.09.2017 04:42:
> Any thoughts on this patch to add additional warnings for the user -
> logging when using default dialects (or when server returns dialect
> not supported), and noting the default dialect change?
> See https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=bb86f22eeddbb5879675b55168b8fa8990d74a21

I noticed Linus committed a sightly updated variant earlier today
(https://git.kernel.org/torvalds/c/7e682f766f28 ). I just gave it a
quick try and it worked well. I can still mount smb3 shares. For
cifs/smb1 shares mount.cifs obviously still fails with the confusing
error message. But at least one gets a better explanation in dmesg now.

Many thx for this! Ciao, Thorsten.

P.S.: For the curious reader (and search engines!), this is the
confusing mount message you'll get when trying to access a CIFS/SMB1
only share with Linux master currently:

mount error(112): Host is down
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)"

And this is the warning you see in dmesg now when not specifying
"vers=1.0" as option (-o) to mount/mount.cifs:

No dialect specified on mount. Default has changed to a
more secure dialect, SMB3 (vers=3.0), from CIFS (SMB1). To use the less
secure SMB1 dialect to access old servers which do not support SMB3
specify vers=1.0 on mount. For somewhat newer servers such as Windows 7
try vers=2.1.
CIFS VFS: cifs_mount failed w/return code = -112


> On Thu, Aug 31, 2017 at 7:29 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Yes - updating the parsing slightly and printks as suggested makes sense
>>
>> Some additional warning messages in the userspace helper (adding Jeff
>> Layton), mount.cifs can also help.
>>
>> I also have an experimental set of patches to allow multi-dialect
>> negotiation with at least three of the acceptable dialects
>> (smb2.1/smb3/smb3.02) which will help, but complicate secure dialect
>> validation ("validate negotiate") but that will have to wait till next
>> release.
>>
>> On Thu, Aug 31, 2017 at 7:12 PM, Linus Torvalds
>> <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>>> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org> wrote:
>>>> Lo! To give a bit more background to this (the mail I reply to was the
>>>> first I sent with git send-email and I missed some details): Maybe I'm
>>>> over stretching my abilities/position as regression tracker with this
>>>> RFC for a revert, but I hope it at least triggers a discussion if such a
>>>> revert should be done or not.
>>>
>>> I don't think that a revert is appropriate.
>>>
>>> But perhaps just a single printk() or something if the user does *not*
>>> specify the version explicitly? Just saying something like
>>>
>>>   We used to default to 1.0, we now default to 3.0, if you want old
>>> defaults, use "vers=1.0"
>>>
>>> Oh, looking at that version parsing code, I think we also need to fix
>>> that legacy "ver=1" thing (ver without the 's') which now silently
>>> ignores "ver=1" as being the "default", even though it's not.
>>>
>>> I do *not* believe that "default to version 1" is acceptable.
>>>
>>>                 Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
@ 2017-09-02 14:25                 ` Thorsten Leemhuis
  0 siblings, 0 replies; 24+ messages in thread
From: Thorsten Leemhuis @ 2017-09-02 14:25 UTC (permalink / raw)
  To: Steve French, Pavel Shilovskiy, ronnie sahlberg; +Cc: linux-cifs, Jeff Layton

Hi! Just a quick feedback from my side.

After reading Andrew explanation in this thread about the "movement
against SMB1" I kind of think "maybe the proposed revert for 4.13 and
doing it properly in 4.14 would really have been a good fit". But
whatever, doesn't bother me much any more:

Steve French wrote on 01.09.2017 04:42:
> Any thoughts on this patch to add additional warnings for the user -
> logging when using default dialects (or when server returns dialect
> not supported), and noting the default dialect change?
> See https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=bb86f22eeddbb5879675b55168b8fa8990d74a21

I noticed Linus committed a sightly updated variant earlier today
(https://git.kernel.org/torvalds/c/7e682f766f28 ). I just gave it a
quick try and it worked well. I can still mount smb3 shares. For
cifs/smb1 shares mount.cifs obviously still fails with the confusing
error message. But at least one gets a better explanation in dmesg now.

Many thx for this! Ciao, Thorsten.

P.S.: For the curious reader (and search engines!), this is the
confusing mount message you'll get when trying to access a CIFS/SMB1
only share with Linux master currently:

mount error(112): Host is down
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)"

And this is the warning you see in dmesg now when not specifying
"vers=1.0" as option (-o) to mount/mount.cifs:

No dialect specified on mount. Default has changed to a
more secure dialect, SMB3 (vers=3.0), from CIFS (SMB1). To use the less
secure SMB1 dialect to access old servers which do not support SMB3
specify vers=1.0 on mount. For somewhat newer servers such as Windows 7
try vers=2.1.
CIFS VFS: cifs_mount failed w/return code = -112


> On Thu, Aug 31, 2017 at 7:29 PM, Steve French <smfrench@gmail.com> wrote:
>> Yes - updating the parsing slightly and printks as suggested makes sense
>>
>> Some additional warning messages in the userspace helper (adding Jeff
>> Layton), mount.cifs can also help.
>>
>> I also have an experimental set of patches to allow multi-dialect
>> negotiation with at least three of the acceptable dialects
>> (smb2.1/smb3/smb3.02) which will help, but complicate secure dialect
>> validation ("validate negotiate") but that will have to wait till next
>> release.
>>
>> On Thu, Aug 31, 2017 at 7:12 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>>> Lo! To give a bit more background to this (the mail I reply to was the
>>>> first I sent with git send-email and I missed some details): Maybe I'm
>>>> over stretching my abilities/position as regression tracker with this
>>>> RFC for a revert, but I hope it at least triggers a discussion if such a
>>>> revert should be done or not.
>>>
>>> I don't think that a revert is appropriate.
>>>
>>> But perhaps just a single printk() or something if the user does *not*
>>> specify the version explicitly? Just saying something like
>>>
>>>   We used to default to 1.0, we now default to 3.0, if you want old
>>> defaults, use "vers=1.0"
>>>
>>> Oh, looking at that version parsing code, I think we also need to fix
>>> that legacy "ver=1" thing (ver without the 's') which now silently
>>> ignores "ver=1" as being the "default", even though it's not.
>>>
>>> I do *not* believe that "default to version 1" is acceptable.
>>>
>>>                 Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
  2017-09-02  5:22                     ` Andrew Bartlett
@ 2017-09-02 17:09                         ` Linus Torvalds
  -1 siblings, 0 replies; 24+ messages in thread
From: Linus Torvalds @ 2017-09-02 17:09 UTC (permalink / raw)
  To: Andrew Bartlett
  Cc: Steve French, L. A. Walsh, Thorsten Leemhuis,
	Linux Kernel Mailing List, linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	Pavel Shilovsky

On Fri, Sep 1, 2017 at 10:22 PM, Andrew Bartlett <abartlet-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>
> My quick research shows:
>
> SMB 2.1 but not SMB3 is on:
>  Windows 7
>  Windows 8
>  Windows 2008
>  Windows 2012
>  Samba 3.6 and earlier (SMB1 only by default)
>
> SMB3 is on:
>  Windows 8.1
>  Windows 2012 R2
>  Windows 10
>  Windows 2016
>  Samba 4.0 and above (released 2012)

But most, if not all, of those SMB3 cases _also_ support SMB2.1,
right?  So the "3.0 _only_" case ends up being a fairly rare case
where things have been explicitly limited, and any previous Linux use
must have had that explicit "vers=3.0" flag anyway?

No?

Anyway, we can't avoid *some* breakage (ie the places that literally
only support 1.0 will have to add the explicit "vers=1.0" to get the
mount).

And I merged the code to add better error reporting yesterday, so
hopefully regardless of the default we choose the breakage is not
nearly as confusing to people any more.

                      Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: RFC: Revert move default dialect from CIFS to to SMB3
@ 2017-09-02 17:09                         ` Linus Torvalds
  0 siblings, 0 replies; 24+ messages in thread
From: Linus Torvalds @ 2017-09-02 17:09 UTC (permalink / raw)
  To: Andrew Bartlett
  Cc: Steve French, L. A. Walsh, Thorsten Leemhuis,
	Linux Kernel Mailing List, linux-cifs, Pavel Shilovsky

On Fri, Sep 1, 2017 at 10:22 PM, Andrew Bartlett <abartlet@samba.org> wrote:
>
> My quick research shows:
>
> SMB 2.1 but not SMB3 is on:
>  Windows 7
>  Windows 8
>  Windows 2008
>  Windows 2012
>  Samba 3.6 and earlier (SMB1 only by default)
>
> SMB3 is on:
>  Windows 8.1
>  Windows 2012 R2
>  Windows 10
>  Windows 2016
>  Samba 4.0 and above (released 2012)

But most, if not all, of those SMB3 cases _also_ support SMB2.1,
right?  So the "3.0 _only_" case ends up being a fairly rare case
where things have been explicitly limited, and any previous Linux use
must have had that explicit "vers=3.0" flag anyway?

No?

Anyway, we can't avoid *some* breakage (ie the places that literally
only support 1.0 will have to add the explicit "vers=1.0" to get the
mount).

And I merged the code to add better error reporting yesterday, so
hopefully regardless of the default we choose the breakage is not
nearly as confusing to people any more.

                      Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2017-09-03 15:28 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-31 21:01 RFC: Revert move default dialect from CIFS to to SMB3" Thorsten Leemhuis
2017-08-31 21:01 ` Thorsten Leemhuis
     [not found] ` <1504213298-27431-1-git-send-email-linux-rCxcAJFjeRkk+I/owrrOrA@public.gmane.org>
2017-08-31 21:36   ` RFC: Revert move default dialect from CIFS to to SMB3 Thorsten Leemhuis
2017-08-31 21:36     ` Thorsten Leemhuis
2017-09-01  0:12     ` Linus Torvalds
2017-09-01  0:29       ` Steve French
     [not found]         ` <CAH2r5msWDXzwbFPtUHCKbqHrEBTsvw5eaTayj5RkdgYCLM5nAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-01  2:42           ` Steve French
     [not found]             ` <CAH2r5mv9roEvMX+C-csU=GZFM_HMbqxnHfF11NUp+2yonDVPgA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-01  3:06               ` ronnie sahlberg
2017-09-01 11:07               ` Jeff Layton
2017-09-02 14:25               ` Thorsten Leemhuis
2017-09-02 14:25                 ` Thorsten Leemhuis
2017-09-01 18:23       ` L. A. Walsh
     [not found]         ` <59A9A59E.6040205-gT3AUAsYRbTYtjvyW6yDsg@public.gmane.org>
2017-09-01 19:45           ` Linus Torvalds
2017-09-01 19:45             ` Linus Torvalds
2017-09-02  2:16             ` Steve French
2017-09-02  3:56               ` Linus Torvalds
     [not found]                 ` <CA+55aFwUHLxBhOh7DxtjSSnKX6KBj+k+p=_CzE8i_xgq-LNj0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-02  5:22                   ` Andrew Bartlett
2017-09-02  5:22                     ` Andrew Bartlett
     [not found]                     ` <1504329770.3249.61.camel-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
2017-09-02 17:09                       ` Linus Torvalds
2017-09-02 17:09                         ` Linus Torvalds
2017-09-01  0:03   ` RFC: Revert move default dialect from CIFS to to SMB3" L. A. Walsh
     [not found]     ` <59A8A3E2.40804-gT3AUAsYRbTYtjvyW6yDsg@public.gmane.org>
2017-09-01  3:11       ` Andrew Bartlett
2017-09-01  0:04 ` L. A. Walsh
2017-09-01  0:41   ` Steve French

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.