All of lore.kernel.org
 help / color / mirror / Atom feed
* Building cifs.ko without any support for insecure crypto?
@ 2021-08-13  3:23 Eric Biggers
  2021-08-13  4:46 ` ronnie sahlberg
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Biggers @ 2021-08-13  3:23 UTC (permalink / raw)
  To: linux-cifs, Steve French; +Cc: samba-technical, linux-crypto

Hi!

We should be working to eliminate any uses of insecure crypto algorithms (e.g.
DES, ARC4, MD4, MD5) from the kernel.  In particular, it should be possible to
build a kernel for a modern system without including any such algorithms.

Currently, CONFIG_CIFS is problematic because it selects all these algorithms
(kconfig options: CONFIG_CRYPTO_LIB_DES, CONFIG_CRYPTO_LIB_ARC4,
CONFIG_CRYPTO_MD4, CONFIG_CRYPTO_MD5).

It looks like these algorithms might only be used by SMB2.0 and earlier, and the
more modern SMB versions don't use them.  Is that the case?  It mostly looks
like that, but there's one case I'm not sure about -- there's a call chain which
appears to use ARC4 and HMAC-MD5 even with the most recent SMB version:

    smb311_operations.sess_setup()
      SMB2_sess_setup()
        SMB2_sess_auth_rawntlmssp_authenticate()
          build_ntlmssp_auth_blob()
            setup_ntlmv2_rsp()

Also, there's already an option CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n which
disables support for SMB2.0 and earlier.  However, it doesn't actually compile
out the code but rather just prevents it from being used.  That means that the
DES and ARC4 library interfaces are still depended on at link time, so they
can't be omitted.  Have there been any considerations towards making
CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n compile out the code for SMB2.0 and earlier?

- Eric

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-13  3:23 Building cifs.ko without any support for insecure crypto? Eric Biggers
@ 2021-08-13  4:46 ` ronnie sahlberg
  2021-08-13 20:19   ` Eric Biggers
  0 siblings, 1 reply; 11+ messages in thread
From: ronnie sahlberg @ 2021-08-13  4:46 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-cifs, Steve French, samba-technical, linux-crypto

On Fri, Aug 13, 2021 at 1:34 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> Hi!
>
> We should be working to eliminate any uses of insecure crypto algorithms (e.g.
> DES, ARC4, MD4, MD5) from the kernel.  In particular, it should be possible to
> build a kernel for a modern system without including any such algorithms.
>
> Currently, CONFIG_CIFS is problematic because it selects all these algorithms
> (kconfig options: CONFIG_CRYPTO_LIB_DES, CONFIG_CRYPTO_LIB_ARC4,
> CONFIG_CRYPTO_MD4, CONFIG_CRYPTO_MD5).
>
> It looks like these algorithms might only be used by SMB2.0 and earlier, and the
> more modern SMB versions don't use them.  Is that the case?  It mostly looks
> like that, but there's one case I'm not sure about -- there's a call chain which
> appears to use ARC4 and HMAC-MD5 even with the most recent SMB version:
>
>     smb311_operations.sess_setup()
>       SMB2_sess_setup()
>         SMB2_sess_auth_rawntlmssp_authenticate()
>           build_ntlmssp_auth_blob()
>             setup_ntlmv2_rsp()

md4 and md5 are used with the NTLMSSP authentication for all dialects,
including the latest 3.1.1.
The only other authentication mechanism for SMB is krb5.

This means that if we build a kernel without md4/md5 then we can no
longer use NTLMSSP user/password
style authentication, only kerberos.
I guess that the use cases where a kernel without these algorithms are
present are ok with kerberos as the
only authentication mech.

Afaik arc4 is only used for signing in the smb1 case.

>
> Also, there's already an option CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n which
> disables support for SMB2.0 and earlier.  However, it doesn't actually compile
> out the code but rather just prevents it from being used.  That means that the
> DES and ARC4 library interfaces are still depended on at link time, so they
> can't be omitted.  Have there been any considerations towards making
> CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n compile out the code for SMB2.0 and earlier?

I think initially we just wanted to disable its use. If we want to
compile a kernel completely without arc4/md4/md5 I think we would need
to:

1, Change CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n to compile out the code
as you suggests.
This should remove the dependency for arc4. I think this would be a
good thing to do.

2, Have a different CONFIG_... to compile out the use of NTLMSSP
authentication. This must be a different define
since md4/md5 are also used for non-legacy dialects.
And this should remove the dependency of md4/5.

For the latter, I guess we would need a global, i.e. not
cifs-specific, config option for this. I assume other users of
rc4/md4/md5
would also want this.
A new CONFIG_INSECURE_CRYPTO=n ?

Ronnie Sahlberg

>
> - Eric

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-13  4:46 ` ronnie sahlberg
@ 2021-08-13 20:19   ` Eric Biggers
  2021-08-15 10:38     ` ronnie sahlberg
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Biggers @ 2021-08-13 20:19 UTC (permalink / raw)
  To: ronnie sahlberg; +Cc: linux-cifs, Steve French, samba-technical, linux-crypto

On Fri, Aug 13, 2021 at 02:46:21PM +1000, ronnie sahlberg wrote:
> On Fri, Aug 13, 2021 at 1:34 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Hi!
> >
> > We should be working to eliminate any uses of insecure crypto algorithms (e.g.
> > DES, ARC4, MD4, MD5) from the kernel.  In particular, it should be possible to
> > build a kernel for a modern system without including any such algorithms.
> >
> > Currently, CONFIG_CIFS is problematic because it selects all these algorithms
> > (kconfig options: CONFIG_CRYPTO_LIB_DES, CONFIG_CRYPTO_LIB_ARC4,
> > CONFIG_CRYPTO_MD4, CONFIG_CRYPTO_MD5).
> >
> > It looks like these algorithms might only be used by SMB2.0 and earlier, and the
> > more modern SMB versions don't use them.  Is that the case?  It mostly looks
> > like that, but there's one case I'm not sure about -- there's a call chain which
> > appears to use ARC4 and HMAC-MD5 even with the most recent SMB version:
> >
> >     smb311_operations.sess_setup()
> >       SMB2_sess_setup()
> >         SMB2_sess_auth_rawntlmssp_authenticate()
> >           build_ntlmssp_auth_blob()
> >             setup_ntlmv2_rsp()
> 
> md4 and md5 are used with the NTLMSSP authentication for all dialects,
> including the latest 3.1.1.

That's unfortunate.  Surely Microsoft knows that md4 has been severely
compromised for over 25 years?  And md5 for 15 years.

> The only other authentication mechanism for SMB is krb5.

Is the long-term plan to have everyone migrate to kerberos?  Currently kerberos
doesn't appear to be the default, so not many people actually use it -- right?

> This means that if we build a kernel without md4/md5 then we can no
> longer use NTLMSSP user/password
> style authentication, only kerberos.
>
> I guess that the use cases where a kernel without these algorithms are
> present are ok with kerberos as the
> only authentication mech.

Well, maybe.  Even without kerberos, would it still be possible to use SMB with
a "guest" user only?

> 
> Afaik arc4 is only used for signing in the smb1 case.
> 
> >
> > Also, there's already an option CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n which
> > disables support for SMB2.0 and earlier.  However, it doesn't actually compile
> > out the code but rather just prevents it from being used.  That means that the
> > DES and ARC4 library interfaces are still depended on at link time, so they
> > can't be omitted.  Have there been any considerations towards making
> > CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n compile out the code for SMB2.0 and earlier?
> 
> I think initially we just wanted to disable its use. If we want to
> compile a kernel completely without arc4/md4/md5 I think we would need
> to:
> 
> 1, Change CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n to compile out the code
> as you suggests.
> This should remove the dependency for arc4. I think this would be a
> good thing to do.
> 
> 2, Have a different CONFIG_... to compile out the use of NTLMSSP
> authentication. This must be a different define
> since md4/md5 are also used for non-legacy dialects.
> And this should remove the dependency of md4/5.
> 
> For the latter, I guess we would need a global, i.e. not
> cifs-specific, config option for this. I assume other users of
> rc4/md4/md5
> would also want this.
> A new CONFIG_INSECURE_CRYPTO=n ?

There is already an option CRYPTO_USER_API_ENABLE_OBSOLETE that could be
renamed and reused if we wanted to expand its scope to all insecure crypto.

Although a one-size-fits all kernel-wide option controlling "insecure" crypto
could be controversial, as there is no consensus whether some crypto algorithms
are secure or not, and different subsystems have different constraints.

- Eric

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-13 20:19   ` Eric Biggers
@ 2021-08-15 10:38     ` ronnie sahlberg
  2021-08-16 22:19       ` Eric Biggers
  0 siblings, 1 reply; 11+ messages in thread
From: ronnie sahlberg @ 2021-08-15 10:38 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-cifs, Steve French, samba-technical, linux-crypto

On Sat, Aug 14, 2021 at 6:19 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Fri, Aug 13, 2021 at 02:46:21PM +1000, ronnie sahlberg wrote:
> > On Fri, Aug 13, 2021 at 1:34 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > Hi!
> > >
> > > We should be working to eliminate any uses of insecure crypto algorithms (e.g.
> > > DES, ARC4, MD4, MD5) from the kernel.  In particular, it should be possible to
> > > build a kernel for a modern system without including any such algorithms.
> > >
> > > Currently, CONFIG_CIFS is problematic because it selects all these algorithms
> > > (kconfig options: CONFIG_CRYPTO_LIB_DES, CONFIG_CRYPTO_LIB_ARC4,
> > > CONFIG_CRYPTO_MD4, CONFIG_CRYPTO_MD5).
> > >
> > > It looks like these algorithms might only be used by SMB2.0 and earlier, and the
> > > more modern SMB versions don't use them.  Is that the case?  It mostly looks
> > > like that, but there's one case I'm not sure about -- there's a call chain which
> > > appears to use ARC4 and HMAC-MD5 even with the most recent SMB version:
> > >
> > >     smb311_operations.sess_setup()
> > >       SMB2_sess_setup()
> > >         SMB2_sess_auth_rawntlmssp_authenticate()
> > >           build_ntlmssp_auth_blob()
> > >             setup_ntlmv2_rsp()
> >
> > md4 and md5 are used with the NTLMSSP authentication for all dialects,
> > including the latest 3.1.1.
>
> That's unfortunate.  Surely Microsoft knows that md4 has been severely
> compromised for over 25 years?  And md5 for 15 years.

Absolutely, but it is the reality we have to live in.
It is actually an md4 hash wrapper inside a md5 hash so "better" but still...

>
> > The only other authentication mechanism for SMB is krb5.
>
> Is the long-term plan to have everyone migrate to kerberos?  Currently kerberos
> doesn't appear to be the default, so not many people actually use it -- right?
>

I have no idea on the plans here.

Kerberos do require a bit of infrastructure to set up and manage, so a
keb5 only solution
would be a no-go for most/all home-nas environments and also all small
business "that is not
big enough to justify running a full ActiveDirectory environment."

For use, I would say that most larger domains and enterprises do use
AcriveDirectory and thus also Krb5 because of single signon and a
single account database.
But ActiveDirectory does require a fair amount of setup and management
so I think most smaller businesses and all home-NAS
users just use user/password NTLMSSP authentication.

What are the plans here? To just offer the possibility to disable all
these old crypto and hashes on a local kernel compile?
Or is the plan to just outright remove it from the kernel sources?

If the first, I think that could possible be done for cifs. I think a
lot of the security minded larger enterprises already may be disabling
both SMB1 and also NTLM on serverside, so they would be fine.

For the latter, I think it would be a no-go since aside from krb5
there are just no other viable authentication mechs for smb.



> > This means that if we build a kernel without md4/md5 then we can no
> > longer use NTLMSSP user/password
> > style authentication, only kerberos.
> >
> > I guess that the use cases where a kernel without these algorithms are
> > present are ok with kerberos as the
> > only authentication mech.
>
> Well, maybe.  Even without kerberos, would it still be possible to use SMB with
> a "guest" user only?

Yes and no.
In smb we actually have two different concepts of guest.
We have GUEST, where the SERVER decides you are a guest user, based on
some who-knows-what configuration
but you still authenticate properly (and have a session key).
and we have ANONYMOUS/NULL, where the client does not even have any
credentials at all (and you have no session key).

For the latest and most secure dialect of SMB 3.1.1 the command to
even map a share : SMB2 TREE_CONNECT
MUST be signed, and thus you have to have a session key, to sign it.
So, anonymous/null users can not even map a share without credentials.
Because they do not have a session key and thus can not
sign these command.

So in the case of no NTLMSSP, a user would technically be an ANONYMOUS
user, not a GUEST user.
The difference is subtle but important.   Technically a GUEST users
did authenticate properly, and got a SESSION KEY,
and logged in, but the server said "nah, you are GUEST".  But still,
the user got a session key.

Another huge drawback with anonymous/null/guest is that without a
properly authenticated session tied to a user account
you no longer have any meaningful ACL controls.   Which may be fine
for a home-NAS but not elsewhere.

There are no good solutions here.




TL;DR
If NTLMSSP authentication is disabled, there are no other options to
map a share than using KRB5
and setting up the krb5 infrastructure. And thus smaller sites will
not be able to use CIFS :-(
So while I think it is feasible to add support to cifs.ko to
conditionally disable features depending in a kernel compile (no SMB1
if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I don't
think it is feasible to disable these by default.
I will work on making it possible to build cifs.ko with limied
functionality when these algorithms are disabled though.



>
> >
> > Afaik arc4 is only used for signing in the smb1 case.
> >
> > >
> > > Also, there's already an option CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n which
> > > disables support for SMB2.0 and earlier.  However, it doesn't actually compile
> > > out the code but rather just prevents it from being used.  That means that the
> > > DES and ARC4 library interfaces are still depended on at link time, so they
> > > can't be omitted.  Have there been any considerations towards making
> > > CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n compile out the code for SMB2.0 and earlier?
> >
> > I think initially we just wanted to disable its use. If we want to
> > compile a kernel completely without arc4/md4/md5 I think we would need
> > to:
> >
> > 1, Change CONFIG_CIFS_ALLOW_INSECURE_LEGACY=n to compile out the code
> > as you suggests.
> > This should remove the dependency for arc4. I think this would be a
> > good thing to do.
> >
> > 2, Have a different CONFIG_... to compile out the use of NTLMSSP
> > authentication. This must be a different define
> > since md4/md5 are also used for non-legacy dialects.
> > And this should remove the dependency of md4/5.
> >
> > For the latter, I guess we would need a global, i.e. not
> > cifs-specific, config option for this. I assume other users of
> > rc4/md4/md5
> > would also want this.
> > A new CONFIG_INSECURE_CRYPTO=n ?
>
> There is already an option CRYPTO_USER_API_ENABLE_OBSOLETE that could be
> renamed and reused if we wanted to expand its scope to all insecure crypto.
>
> Although a one-size-fits all kernel-wide option controlling "insecure" crypto
> could be controversial, as there is no consensus whether some crypto algorithms
> are secure or not, and different subsystems have different constraints.
>
> - Eric

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-15 10:38     ` ronnie sahlberg
@ 2021-08-16 22:19       ` Eric Biggers
  2021-08-17  5:35         ` Steve French
  2021-08-18 11:44         ` Ard Biesheuvel
  0 siblings, 2 replies; 11+ messages in thread
From: Eric Biggers @ 2021-08-16 22:19 UTC (permalink / raw)
  To: ronnie sahlberg; +Cc: linux-cifs, Steve French, samba-technical, linux-crypto

On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
> 
> What are the plans here? To just offer the possibility to disable all
> these old crypto and hashes on a local kernel compile?
> Or is the plan to just outright remove it from the kernel sources?
> 
> If the first, I think that could possible be done for cifs. I think a
> lot of the security minded larger enterprises already may be disabling
> both SMB1 and also NTLM on serverside, so they would be fine.
> 
> For the latter, I think it would be a no-go since aside from krb5
> there are just no other viable authentication mechs for smb.

Removing the code would be best, but allowing it to be compiled out would be the
next best thing.

> TL;DR
> If NTLMSSP authentication is disabled, there are no other options to
> map a share than using KRB5
> and setting up the krb5 infrastructure. And thus smaller sites will
> not be able to use CIFS :-(
> So while I think it is feasible to add support to cifs.ko to
> conditionally disable features depending in a kernel compile (no SMB1
> if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I don't
> think it is feasible to disable these by default.
> I will work on making it possible to build cifs.ko with limied
> functionality when these algorithms are disabled though.

FWIW, the way this came up is that the Compatibility Test Suite for Android 11
verifies that CONFIG_CRYPTO_MD4 isn't set.  The reason that test got added is
because for a short time, CONFIG_CRYPTO_MD4 had accidentally been enabled in the
recommended kernel config for Android.  Since "obviously" no one would be using
a completely broken crypto algorithm from 31 years ago, when fixing that bug we
decided to go a bit further and just forbid it from the kernel config.

I guess we'll have to remove that test for now (assuming that CONFIG_CIFS is to
be allowed at all on an Android device, and that the people who want to use it
don't want to use kerberos which is probably the case).

It is beyond ridiculous that this is even an issue though, given that MD4 has
been severely compromised for over 25 years.

One thing which we should seriously consider doing is removing md4 from the
crypto API and moving it into fs/cifs/.  It isn't a valid crypto algorithm, so
anyone who wants to use it should have to maintain it themselves.

- Eric

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-16 22:19       ` Eric Biggers
@ 2021-08-17  5:35         ` Steve French
  2021-08-18 11:44         ` Ard Biesheuvel
  1 sibling, 0 replies; 11+ messages in thread
From: Steve French @ 2021-08-17  5:35 UTC (permalink / raw)
  To: Eric Biggers
  Cc: ronnie sahlberg, linux-cifs, Steve French, samba-technical,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE

Clearly NTLM (NTLMv1) should be disabled by default, despite the fact
that some old appliances use it - but it already is disabled by
default (and disabling its dependencies by default are fine too as
long as it doesn't break NTLMSSP with current dialects)

Rearranging the code similar to what Ronnie's patches do so we can
configure without SMB1 - and don't enable SMB1 and any algorithms
needed by SMB1 is fine, but SMB1 is still surprisingly broadly
deployed.   But ... we may want to consider building the default with
the name "smb3.ko" (with SMB1 disabled), since mount can now do "mount
-t smb3 ..." to make it more clear that what you are mounting is safe
- but I worry less about this since a user has to make an explicit
choice on mount "vers=1.0" to mount with SMB1 ("CIFS" dialect) - the
defaults give you SMB2.1 or later (usually will negotiate SMB3.1.1).
SMB2 and SMB1 have to be explicitly requested on mount to work so the
risk is low in using them.

But, the question about removing things needed by NTLMSSP/NTLMv2 is
harder as probably the majority of machines would default to this
(huge number of Macs, iPhones, iPads, Windows systems, Samba Linux,
smb tools etc) - these are hundreds of millions of machines that can
support Kerberos but are usually setup to negotiate NTLMSSP/NTLMv2.
If we want to move forward with disabling things needed by NTLMSSP for
current systems (SMB3, SMB3.1.1 etc) then we STRONGLY need to
implement a "peer to peer" authentication choice ASAP on Linux.
Currently Windows systems will use PKU2U (see e.g.
https://datatracker.ietf.org/doc/draft-zhu-pku2u/) and Macs have a
peer to peer Kerberos that they support IIRC.

Defaulting to Kerberos would be fine ... if ... we also had a peer to
peer mechanism (e.g. PKU2U) implemented to add to the defaults that
are negotiated in SPNEGO by the Linux client.

On Mon, Aug 16, 2021 at 5:20 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
> >
> > What are the plans here? To just offer the possibility to disable all
> > these old crypto and hashes on a local kernel compile?
> > Or is the plan to just outright remove it from the kernel sources?
> >
> > If the first, I think that could possible be done for cifs. I think a
> > lot of the security minded larger enterprises already may be disabling
> > both SMB1 and also NTLM on serverside, so they would be fine.
> >
> > For the latter, I think it would be a no-go since aside from krb5
> > there are just no other viable authentication mechs for smb.
>
> Removing the code would be best, but allowing it to be compiled out would be the
> next best thing.
>
> > TL;DR
> > If NTLMSSP authentication is disabled, there are no other options to
> > map a share than using KRB5
> > and setting up the krb5 infrastructure. And thus smaller sites will
> > not be able to use CIFS :-(
> > So while I think it is feasible to add support to cifs.ko to
> > conditionally disable features depending in a kernel compile (no SMB1
> > if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I don't
> > think it is feasible to disable these by default.
> > I will work on making it possible to build cifs.ko with limied
> > functionality when these algorithms are disabled though.
>
> FWIW, the way this came up is that the Compatibility Test Suite for Android 11
> verifies that CONFIG_CRYPTO_MD4 isn't set.  The reason that test got added is
> because for a short time, CONFIG_CRYPTO_MD4 had accidentally been enabled in the
> recommended kernel config for Android.  Since "obviously" no one would be using
> a completely broken crypto algorithm from 31 years ago, when fixing that bug we
> decided to go a bit further and just forbid it from the kernel config.
>
> I guess we'll have to remove that test for now (assuming that CONFIG_CIFS is to
> be allowed at all on an Android device, and that the people who want to use it
> don't want to use kerberos which is probably the case).
>
> It is beyond ridiculous that this is even an issue though, given that MD4 has
> been severely compromised for over 25 years.
>
> One thing which we should seriously consider doing is removing md4 from the
> crypto API and moving it into fs/cifs/.  It isn't a valid crypto algorithm, so
> anyone who wants to use it should have to maintain it themselves.
>
> - Eric



-- 
Thanks,

Steve

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-16 22:19       ` Eric Biggers
  2021-08-17  5:35         ` Steve French
@ 2021-08-18 11:44         ` Ard Biesheuvel
  2021-08-19  3:43           ` ronnie sahlberg
  1 sibling, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2021-08-18 11:44 UTC (permalink / raw)
  To: Eric Biggers
  Cc: ronnie sahlberg, linux-cifs, Steve French, samba-technical,
	Linux Crypto Mailing List

On Tue, 17 Aug 2021 at 00:19, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
> >
> > What are the plans here? To just offer the possibility to disable all
> > these old crypto and hashes on a local kernel compile?
> > Or is the plan to just outright remove it from the kernel sources?
> >
> > If the first, I think that could possible be done for cifs. I think a
> > lot of the security minded larger enterprises already may be disabling
> > both SMB1 and also NTLM on serverside, so they would be fine.
> >
> > For the latter, I think it would be a no-go since aside from krb5
> > there are just no other viable authentication mechs for smb.
>
> Removing the code would be best, but allowing it to be compiled out would be the
> next best thing.
>
> > TL;DR
> > If NTLMSSP authentication is disabled, there are no other options to
> > map a share than using KRB5
> > and setting up the krb5 infrastructure. And thus smaller sites will
> > not be able to use CIFS :-(
> > So while I think it is feasible to add support to cifs.ko to
> > conditionally disable features depending in a kernel compile (no SMB1
> > if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I don't
> > think it is feasible to disable these by default.
> > I will work on making it possible to build cifs.ko with limied
> > functionality when these algorithms are disabled though.
>
> FWIW, the way this came up is that the Compatibility Test Suite for Android 11
> verifies that CONFIG_CRYPTO_MD4 isn't set.  The reason that test got added is
> because for a short time, CONFIG_CRYPTO_MD4 had accidentally been enabled in the
> recommended kernel config for Android.  Since "obviously" no one would be using
> a completely broken crypto algorithm from 31 years ago, when fixing that bug we
> decided to go a bit further and just forbid it from the kernel config.
>
> I guess we'll have to remove that test for now (assuming that CONFIG_CIFS is to
> be allowed at all on an Android device, and that the people who want to use it
> don't want to use kerberos which is probably the case).
>
> It is beyond ridiculous that this is even an issue though, given that MD4 has
> been severely compromised for over 25 years.
>
> One thing which we should seriously consider doing is removing md4 from the
> crypto API and moving it into fs/cifs/.  It isn't a valid crypto algorithm, so
> anyone who wants to use it should have to maintain it themselves.
>

+1 to moving the md4 code into fs/cifs, so that the CIFS maintainers
can own it and phase it out on their own schedule, and prevent its
inadvertent use in other places.

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-18 11:44         ` Ard Biesheuvel
@ 2021-08-19  3:43           ` ronnie sahlberg
  2021-08-19  3:53             ` Andrew Bartlett
  2021-08-23 10:04             ` Simo Sorce
  0 siblings, 2 replies; 11+ messages in thread
From: ronnie sahlberg @ 2021-08-19  3:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Eric Biggers, linux-cifs, Steve French, samba-technical,
	Linux Crypto Mailing List

On Wed, Aug 18, 2021 at 9:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 17 Aug 2021 at 00:19, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
> > >
> > > What are the plans here? To just offer the possibility to disable all
> > > these old crypto and hashes on a local kernel compile?
> > > Or is the plan to just outright remove it from the kernel sources?
> > >
> > > If the first, I think that could possible be done for cifs. I think a
> > > lot of the security minded larger enterprises already may be disabling
> > > both SMB1 and also NTLM on serverside, so they would be fine.
> > >
> > > For the latter, I think it would be a no-go since aside from krb5
> > > there are just no other viable authentication mechs for smb.
> >
> > Removing the code would be best, but allowing it to be compiled out would be the
> > next best thing.
> >
> > > TL;DR
> > > If NTLMSSP authentication is disabled, there are no other options to
> > > map a share than using KRB5
> > > and setting up the krb5 infrastructure. And thus smaller sites will
> > > not be able to use CIFS :-(
> > > So while I think it is feasible to add support to cifs.ko to
> > > conditionally disable features depending in a kernel compile (no SMB1
> > > if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I don't
> > > think it is feasible to disable these by default.
> > > I will work on making it possible to build cifs.ko with limied
> > > functionality when these algorithms are disabled though.
> >
> > FWIW, the way this came up is that the Compatibility Test Suite for Android 11
> > verifies that CONFIG_CRYPTO_MD4 isn't set.  The reason that test got added is
> > because for a short time, CONFIG_CRYPTO_MD4 had accidentally been enabled in the
> > recommended kernel config for Android.  Since "obviously" no one would be using
> > a completely broken crypto algorithm from 31 years ago, when fixing that bug we
> > decided to go a bit further and just forbid it from the kernel config.
> >
> > I guess we'll have to remove that test for now (assuming that CONFIG_CIFS is to
> > be allowed at all on an Android device, and that the people who want to use it
> > don't want to use kerberos which is probably the case).
> >
> > It is beyond ridiculous that this is even an issue though, given that MD4 has
> > been severely compromised for over 25 years.
> >
> > One thing which we should seriously consider doing is removing md4 from the
> > crypto API and moving it into fs/cifs/.  It isn't a valid crypto algorithm, so
> > anyone who wants to use it should have to maintain it themselves.
> >
>
> +1 to moving the md4 code into fs/cifs, so that the CIFS maintainers
> can own it and phase it out on their own schedule, and prevent its
> inadvertent use in other places.

Ok, let me summarize the status and what I think we will need to do in cifs.

DES
---
Removal of DES is not controversial since this only affects SMB1.
SMB2 has been around since 2006 and it is starting to become viable to at least
disable the SMB1 protocol by default today.
There are still servers that only support SMB1 but they are becoming rare.
I think also Microsoft Windows default to disable (but not remove)
SMB1 by default
on some configurations today.

I am proposing that we remove the hard dependency to DES and instead
make it a soft dependency to "do not build SMB1 if DES is missing".

MD4/MD5/ARC4
----------------------
These are all used together in NTLMSSP authentication today, including
in the very latest
versions of the protocol. This is the only authentication mechanism
available in cifs
aside from the "extended" kerberos 5 protocol that ActiveDirectory implements.
As such the vast majority of clients rely on this when accessing SMB servers.

ARC4 is technically possible to remove since it is only used in the
final stage for KEY_EXCHANGE
when negotiating the session key. It could be removed but it would
make NTLMSSP weaker.
But if we have to move other crypto into fs/cifs anyway we can just as
well copy ARC4 into fs/cifs.

MD4 is used to create a hash, which is then one of the inputs into
MD5-HMAC for the core part of the
NTLMSSP authentication so we would need private versions of at least
md4 to be copied to fs/cifs as well.

I am proposing that we fork both ARC4 and MD4 and host private
versions of these in fs/cifs.


I have patches for both DES removal and forking ARC4 prepared for linux-cifs.
MD4 will require more work since we use it via the crypto_alloc_hash()
api but we will do that too.

What about MD5? Is it also scheduled for removal? if so we will need
to fork it too.


-- ronnie

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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-19  3:43           ` ronnie sahlberg
@ 2021-08-19  3:53             ` Andrew Bartlett
  2021-08-23 10:04             ` Simo Sorce
  1 sibling, 0 replies; 11+ messages in thread
From: Andrew Bartlett @ 2021-08-19  3:53 UTC (permalink / raw)
  To: ronnie sahlberg, Ard Biesheuvel
  Cc: Eric Biggers, linux-cifs, Steve French, samba-technical,
	Linux Crypto Mailing List

On Thu, 2021-08-19 at 13:43 +1000, ronnie sahlberg via samba-technical
wrote:
> On Wed, Aug 18, 2021 at 9:44 PM Ard Biesheuvel <ardb@kernel.org>
> wrote:
> > On Tue, 17 Aug 2021 at 00:19, Eric Biggers <ebiggers@kernel.org>
> > wrote:
> > > On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
> > > > What are the plans here? To just offer the possibility to
> > > > disable all
> > > > these old crypto and hashes on a local kernel compile?
> > > > Or is the plan to just outright remove it from the kernel
> > > > sources?
> > > > 
> > > > If the first, I think that could possible be done for cifs. I
> > > > think a
> > > > lot of the security minded larger enterprises already may be
> > > > disabling
> > > > both SMB1 and also NTLM on serverside, so they would be fine.
> > > > 
> > > > For the latter, I think it would be a no-go since aside from
> > > > krb5
> > > > there are just no other viable authentication mechs for smb.
> > > 
> > > Removing the code would be best, but allowing it to be compiled
> > > out would be the
> > > next best thing.
> > > 
> > > > TL;DR
> > > > If NTLMSSP authentication is disabled, there are no other
> > > > options to
> > > > map a share than using KRB5
> > > > and setting up the krb5 infrastructure. And thus smaller sites
> > > > will
> > > > not be able to use CIFS :-(
> > > > So while I think it is feasible to add support to cifs.ko to
> > > > conditionally disable features depending in a kernel compile
> > > > (no SMB1
> > > > if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I
> > > > don't
> > > > think it is feasible to disable these by default.
> > > > I will work on making it possible to build cifs.ko with limied
> > > > functionality when these algorithms are disabled though.
> > > 
> > > FWIW, the way this came up is that the Compatibility Test Suite
> > > for Android 11
> > > verifies that CONFIG_CRYPTO_MD4 isn't set.  The reason that test
> > > got added is
> > > because for a short time, CONFIG_CRYPTO_MD4 had accidentally been
> > > enabled in the
> > > recommended kernel config for Android.  Since "obviously" no one
> > > would be using
> > > a completely broken crypto algorithm from 31 years ago, when
> > > fixing that bug we
> > > decided to go a bit further and just forbid it from the kernel
> > > config.
> > > 
> > > I guess we'll have to remove that test for now (assuming that
> > > CONFIG_CIFS is to
> > > be allowed at all on an Android device, and that the people who
> > > want to use it
> > > don't want to use kerberos which is probably the case).
> > > 
> > > It is beyond ridiculous that this is even an issue though, given
> > > that MD4 has
> > > been severely compromised for over 25 years.
> > > 
> > > One thing which we should seriously consider doing is removing
> > > md4 from the
> > > crypto API and moving it into fs/cifs/.  It isn't a valid crypto
> > > algorithm, so
> > > anyone who wants to use it should have to maintain it themselves.
> > > 
> > 
> > +1 to moving the md4 code into fs/cifs, so that the CIFS
> > maintainers
> > can own it and phase it out on their own schedule, and prevent its
> > inadvertent use in other places.
> 
> Ok, let me summarize the status and what I think we will need to do
> in cifs.
> 
> DES
> ---
> Removal of DES is not controversial since this only affects SMB1.
> SMB2 has been around since 2006 and it is starting to become viable
> to at least
> disable the SMB1 protocol by default today.
> There are still servers that only support SMB1 but they are becoming
> rare.
> I think also Microsoft Windows default to disable (but not remove)
> SMB1 by default
> on some configurations today.
> 
> I am proposing that we remove the hard dependency to DES and instead
> make it a soft dependency to "do not build SMB1 if DES is missing".

NTLMSSP is also used over SMB1 (and presumably in cifs.ko), allowing
NTLMv2.  This means you would only need DES with 'bare' NTLM, which
means you are talking to a very, very old server, eg NT4 or Win9X, or
are running old crypto for giggles. 

Andrew Bartlett

-- 
Andrew Bartlett (he/him)       https://samba.org/~abartlet/
Samba Team Member (since 2001) https://samba.org
Samba Team Lead, Catalyst IT   https://catalyst.net.nz/services/samba

Samba Development and Support, Catalyst IT - Expert Open Source
Solutions


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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-19  3:43           ` ronnie sahlberg
  2021-08-19  3:53             ` Andrew Bartlett
@ 2021-08-23 10:04             ` Simo Sorce
  2021-08-24 16:41               ` Steve French
  1 sibling, 1 reply; 11+ messages in thread
From: Simo Sorce @ 2021-08-23 10:04 UTC (permalink / raw)
  To: ronnie sahlberg, Ard Biesheuvel
  Cc: Eric Biggers, linux-cifs, Steve French, samba-technical,
	Linux Crypto Mailing List

On Thu, 2021-08-19 at 13:43 +1000, ronnie sahlberg wrote:
> On Wed, Aug 18, 2021 at 9:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > 
> > On Tue, 17 Aug 2021 at 00:19, Eric Biggers <ebiggers@kernel.org> wrote:
> > > 
> > > On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
> > > > 
> > > > What are the plans here? To just offer the possibility to disable all
> > > > these old crypto and hashes on a local kernel compile?
> > > > Or is the plan to just outright remove it from the kernel sources?
> > > > 
> > > > If the first, I think that could possible be done for cifs. I think a
> > > > lot of the security minded larger enterprises already may be disabling
> > > > both SMB1 and also NTLM on serverside, so they would be fine.
> > > > 
> > > > For the latter, I think it would be a no-go since aside from krb5
> > > > there are just no other viable authentication mechs for smb.
> > > 
> > > Removing the code would be best, but allowing it to be compiled out would be the
> > > next best thing.
> > > 
> > > > TL;DR
> > > > If NTLMSSP authentication is disabled, there are no other options to
> > > > map a share than using KRB5
> > > > and setting up the krb5 infrastructure. And thus smaller sites will
> > > > not be able to use CIFS :-(
> > > > So while I think it is feasible to add support to cifs.ko to
> > > > conditionally disable features depending in a kernel compile (no SMB1
> > > > if des/rc4 is missing, no NTLM if rc4/md4/md5 is missing)  I don't
> > > > think it is feasible to disable these by default.
> > > > I will work on making it possible to build cifs.ko with limied
> > > > functionality when these algorithms are disabled though.
> > > 
> > > FWIW, the way this came up is that the Compatibility Test Suite for Android 11
> > > verifies that CONFIG_CRYPTO_MD4 isn't set.  The reason that test got added is
> > > because for a short time, CONFIG_CRYPTO_MD4 had accidentally been enabled in the
> > > recommended kernel config for Android.  Since "obviously" no one would be using
> > > a completely broken crypto algorithm from 31 years ago, when fixing that bug we
> > > decided to go a bit further and just forbid it from the kernel config.
> > > 
> > > I guess we'll have to remove that test for now (assuming that CONFIG_CIFS is to
> > > be allowed at all on an Android device, and that the people who want to use it
> > > don't want to use kerberos which is probably the case).
> > > 
> > > It is beyond ridiculous that this is even an issue though, given that MD4 has
> > > been severely compromised for over 25 years.
> > > 
> > > One thing which we should seriously consider doing is removing md4 from the
> > > crypto API and moving it into fs/cifs/.  It isn't a valid crypto algorithm, so
> > > anyone who wants to use it should have to maintain it themselves.
> > > 
> > 
> > +1 to moving the md4 code into fs/cifs, so that the CIFS maintainers
> > can own it and phase it out on their own schedule, and prevent its
> > inadvertent use in other places.
> 
> Ok, let me summarize the status and what I think we will need to do in cifs.
> 
> DES
> ---
> Removal of DES is not controversial since this only affects SMB1.
> SMB2 has been around since 2006 and it is starting to become viable to at least
> disable the SMB1 protocol by default today.
> There are still servers that only support SMB1 but they are becoming rare.
> I think also Microsoft Windows default to disable (but not remove)
> SMB1 by default
> on some configurations today.
> 
> I am proposing that we remove the hard dependency to DES and instead
> make it a soft dependency to "do not build SMB1 if DES is missing".
> 
> MD4/MD5/ARC4
> ----------------------
> These are all used together in NTLMSSP authentication today, including
> in the very latest
> versions of the protocol. This is the only authentication mechanism
> available in cifs
> aside from the "extended" kerberos 5 protocol that ActiveDirectory implements.
> As such the vast majority of clients rely on this when accessing SMB servers.
> 
> ARC4 is technically possible to remove since it is only used in the
> final stage for KEY_EXCHANGE
> when negotiating the session key. It could be removed but it would
> make NTLMSSP weaker.
> But if we have to move other crypto into fs/cifs anyway we can just as
> well copy ARC4 into fs/cifs.
> 
> MD4 is used to create a hash, which is then one of the inputs into
> MD5-HMAC for the core part of the
> NTLMSSP authentication so we would need private versions of at least
> md4 to be copied to fs/cifs as well.
> 
> I am proposing that we fork both ARC4 and MD4 and host private
> versions of these in fs/cifs.

Another way to handle this part is to calculate the hash in userspace
and handle the kernel just the hashes. This would allow you to remove
MD4 from the kernel. I guess it would break putting a password on the
kernel command line, but is that really a thing to do? Kernels do not
boot from cifs shares so you can always use userspace tools (or pass
hexed hashes directly on the command line in a pinch).

> I have patches for both DES removal and forking ARC4 prepared for linux-cifs.
> MD4 will require more work since we use it via the crypto_alloc_hash()
> api but we will do that too.
> 
> What about MD5? Is it also scheduled for removal? if so we will need
> to fork it too.

MD5 is still used for a ton of stuff, however it may make sense to
consider moving it in /lib and our of /lib/crypto as it is not usable
in cryptographic settings anymore anyway.

HTH,
Simo.

> -- ronnie
> 

-- 
Simo Sorce
RHEL Crypto Team
Red Hat, Inc





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

* Re: Building cifs.ko without any support for insecure crypto?
  2021-08-23 10:04             ` Simo Sorce
@ 2021-08-24 16:41               ` Steve French
  0 siblings, 0 replies; 11+ messages in thread
From: Steve French @ 2021-08-24 16:41 UTC (permalink / raw)
  To: Simo Sorce
  Cc: ronnie sahlberg, Ard Biesheuvel, Eric Biggers, linux-cifs,
	Steve French, samba-technical, Linux Crypto Mailing List

On Mon, Aug 23, 2021 at 5:05 AM Simo Sorce <simo@redhat.com> wrote:
<snip>
> Another way to handle this part is to calculate the hash in userspace
> and handle the kernel just the hashes. This would allow you to remove
> MD4 from the kernel. I guess it would break putting a password on the
> kernel command line, but is that really a thing to do? Kernels do not
> boot from cifs shares so you can always use userspace tools (or pass
> hexed hashes directly on the command line in a pinch).

We can boot from cifs (and given the security features of SMB3.1.1 it probably
makes more sense than some of the alternatives) albeit with some POSIX
restrictions unless booting from ksmbd with POSIX extensions enabled.
Paulo added the support for booting from cifs.ko in the 5.5 kernel.


> > I have patches for both DES removal and forking ARC4 prepared for linux-cifs.
> > MD4 will require more work since we use it via the crypto_alloc_hash()
> > api but we will do that too.
> >
> > What about MD5? Is it also scheduled for removal? if so we will need
> > to fork it too.
>
> MD5 is still used for a ton of stuff, however it may make sense to
> consider moving it in /lib and our of /lib/crypto as it is not usable
> in cryptographic settings anymore anyway.

Seems reasonable

-- 
Thanks,

Steve

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

end of thread, other threads:[~2021-08-24 16:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13  3:23 Building cifs.ko without any support for insecure crypto? Eric Biggers
2021-08-13  4:46 ` ronnie sahlberg
2021-08-13 20:19   ` Eric Biggers
2021-08-15 10:38     ` ronnie sahlberg
2021-08-16 22:19       ` Eric Biggers
2021-08-17  5:35         ` Steve French
2021-08-18 11:44         ` Ard Biesheuvel
2021-08-19  3:43           ` ronnie sahlberg
2021-08-19  3:53             ` Andrew Bartlett
2021-08-23 10:04             ` Simo Sorce
2021-08-24 16: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.