linux-kernel - Re: [PATCH] crypto: expose needs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 1 Mar 2021 14:09:47 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Christoph Böhmwalder 
        <christoph.boehmwalder@...bit.com>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: expose needs_key in procfs

On Mon, Mar 01, 2021 at 09:51:56PM +0100, Christoph Böhmwalder wrote:
> > Do you have a specific use case in mind for this information?  Normally, users
> > should already know which algorithm they want to use (or set of algorithms they
> > might want to use).
> 
> I have a pretty specific use case in mind, yes. For DRBD, we use crypto
> algorithms for peer authentication and for the online-verify mechanism (to
> verify data integrity). The peer authentication algos require a shared
> secret (HMAC), while the verify algorithms are just hash functions without
> keys (we don't configure a shared secret here, so these must explicitly be
> "keyless").
> 
> Now, we also have a solution which sits on top of DRBD (LINSTOR), which
> resides purely in userspace. We recently implemented a feature where LINSTOR
> automatically chooses the "best" verify algorithm for all nodes in a
> cluster. It does this by parsing /proc/crypto and prioritizing accordingly.
> The problem is that /proc/crypto currently doesn't contain information about
> whether or not an algorithm requires a key – i.e. whether or not it is
> suitable for DRBD's online-verify mechanism.
> 
> See this commit for some context:
> https://github.com/LINBIT/drbd/commit/34ee32e6922994c8e9390859e1790ca

Shouldn't you know ahead of time which algorithm you are using (or set of
algorithms which you might use), and not be parsing /proc/crypto and choosing
some random one (which might be a non-cryptographic algorithm like CRC-32, or
something known to be insecure like MD5)?

Using the algorithm attributes in /proc/crypto only really makes sense if the
decision of which algorithm to use is punted to a higher level and the program
just needs to be able to pass through *any* algorithm available in Linux -- like
how 'cryptsetup' works.  But it's preferable to avoid that sort of design, as it
invites users to start depending on weird or insecure things.

> > 
> > Also, what about algorithms such as blake2b-256 which optionally take a key (as
> > indicated by CRYPTO_ALG_OPTIONAL_KEY being set)?  So it's not really "yes" or
> > "no"; there is a third state as well.
> 
> Correct me if I'm missing something, but crypto_shash_alg_needs_key reads:
> 
> static inline bool crypto_shash_alg_needs_key(struct shash_alg *alg)
> {
> 	return crypto_shash_alg_has_setkey(alg) &&
> 		!(alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY);
> }
> 
> So this already accounts for optional keys. It just returns "no" for an
> optional key, which seems like reasonable behavior to me (it doesn't *need*
> a key after all).
> 
> Another option would be to make it "yes/no/optional". I'm not sure if that's
> more desirable for most people.
> 

BLAKE2 does need a key if it is being used as a keyed hash algorithm.  So it
depends on the user, not the algorithm per se.

- Eric