Thread (11 messages) 11 messages, 6 authors, 2021-08-24

Re: Building cifs.ko without any support for insecure crypto?

From: ronnie sahlberg <ronniesahlberg@gmail.com>
Date: 2021-08-19 03:43:31
Also in: linux-cifs

On Wed, Aug 18, 2021 at 9:44 PM Ard Biesheuvel [off-list ref] wrote:
On Tue, 17 Aug 2021 at 00:19, Eric Biggers [off-list ref] wrote:
quoted
On Sun, Aug 15, 2021 at 08:38:23PM +1000, ronnie sahlberg wrote:
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help