Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <YRXlwDBfQql36wJx@sol.localdomain> <CAN05THSm5fEcnLKxcsidKPRUC6PVLCkWMBZUW05KNm4uMJNHWw@mail.gmail.com>
 <YRbT7IbSCXo4Dl0u@sol.localdomain> <CAN05THScNOVh5biQnqM8YDOvNid4Dh=wZS=ObczzmSEpv1LpRw@mail.gmail.com>
 <YRrkhzOARiT6TqQA@gmail.com> <CAMj1kXH93HU5SNUDLpn+c0ryJUYWpRKVXeoPK8jPOSwiS3_79A@mail.gmail.com>
In-Reply-To: <CAMj1kXH93HU5SNUDLpn+c0ryJUYWpRKVXeoPK8jPOSwiS3_79A@mail.gmail.com>
From:   ronnie sahlberg <ronniesahlberg@gmail.com>
Date:   Thu, 19 Aug 2021 13:43:17 +1000
Message-ID: <CAN05THS27h9QFpNuVVQmqz8k8_SKD8V8TbzZVYxco7S86i0zWA@mail.gmail.com>
Subject: Re: Building cifs.ko without any support for insecure crypto?
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     Eric Biggers <ebiggers@kernel.org>,
        linux-cifs <linux-cifs@vger.kernel.org>,
        Steve French <sfrench@samba.org>,
        "samba-technical@lists.samba.org" <samba-technical@lists.samba.org>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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