2017-08-31 21:22:11

by Thorsten Leemhuis

[permalink] [raw]
Subject: RFC: Revert move default dialect from CIFS to to SMB3"

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 <[email protected]>
CC: Steve French <[email protected]>
CC: Pavel Shilovsky <[email protected]>
---
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


2017-08-31 21:36:05

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

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 <[email protected]>
> CC: Steve French <[email protected]>
> CC: Pavel Shilovsky <[email protected]>
> ---
> 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;
>
>

2017-09-01 00:12:12

by Linus Torvalds

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <[email protected]> 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

2017-09-01 00:30:00

by L. A. Walsh

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3"

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.


>
>

2017-09-01 00:30:07

by Steve French

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

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
<[email protected]> wrote:
> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <[email protected]> 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

2017-09-01 00:41:59

by Steve French

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3"

On Thu, Aug 31, 2017 at 7:04 PM, L. A. Walsh <[email protected]> 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

2017-09-01 18:23:42

by L. A. Walsh

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

Linus Torvalds wrote:
> On Thu, Aug 31, 2017 at 2:36 PM, Thorsten Leemhuis <[email protected]> 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

2017-09-01 19:45:46

by Linus Torvalds

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

On Fri, Sep 1, 2017 at 11:23 AM, L. A. Walsh <[email protected]> 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

2017-09-02 02:17:02

by Steve French

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

On Fri, Sep 1, 2017 at 2:45 PM, Linus Torvalds
<[email protected]> wrote:
> On Fri, Sep 1, 2017 at 11:23 AM, L. A. Walsh <[email protected]> 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

2017-09-02 03:56:53

by Linus Torvalds

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

On Fri, Sep 1, 2017 at 7:16 PM, Steve French <[email protected]> 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

2017-09-02 05:58:31

by Andrew Bartlett

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

On Fri, 2017-09-01 at 20:56 -0700, Linus Torvalds wrote:
> On Fri, Sep 1, 2017 at 7:16 PM, Steve French <[email protected]> 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

2017-09-02 17:09:45

by Linus Torvalds

[permalink] [raw]
Subject: Re: RFC: Revert move default dialect from CIFS to to SMB3

On Fri, Sep 1, 2017 at 10:22 PM, Andrew Bartlett <[email protected]> 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