linux-kernel - Re: [PATCH v13 07/12] crypto: Add RSASSA-PSS support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260120224108.GC6191@quark>
Date: Tue, 20 Jan 2026 14:41:08 -0800
From: Eric Biggers <ebiggers@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: Lukas Wunner <lukas@...ner.de>, Ignat Korchagin <ignat@...udflare.com>,
	Jarkko Sakkinen <jarkko@...nel.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Petr Pavlu <petr.pavlu@...e.com>,
	Daniel Gomez <da.gomez@...nel.org>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	"Jason A . Donenfeld" <Jason@...c4.com>,
	Ard Biesheuvel <ardb@...nel.org>,
	Stephan Mueller <smueller@...onox.de>, linux-crypto@...r.kernel.org,
	keyrings@...r.kernel.org, linux-modules@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Tadeusz Struk <tadeusz.struk@...el.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v13 07/12] crypto: Add RSASSA-PSS support

On Tue, Jan 20, 2026 at 02:50:53PM +0000, David Howells wrote:
> Add support for RSASSA-PSS [RFC8017 sec 8.1] signature verification support
> to the RSA driver in crypto/.

This additional feature significantly increases the scope of your
patchset, especially considering that the kernel previously didn't
implement RSASSA-PSS at all.  This patchset also doesn't include any
explanation for why this additional feature is needed.  It might make
sense to add this feature, but it needs to be properly explained, and it
would be preferable for it to be its own patchset.

> The verification function requires an info string formatted as a
> space-separated list of key=value pairs.  The following parameters need to
> be provided:
> 
>  (1) sighash=<algo>
> 
>      The hash algorithm to be used to digest the data.
> 
>  (2) pss_mask=<type>,...
> 
>      The mask generation function (MGF) and its parameters.
> 
>  (3) pss_salt=<len>
> 
>      The length of the salt used.
> 
> The only MGF currently supported is "mgf1".  This takes an additional
> parameter indicating the mask-generating hash (which need not be the same
> as the data hash).  E.g.:
> 
>      "sighash=sha256 pss_mask=mgf1,sha256 pss_salt=32"

One of the issues with RSASSA-PSS is the excessive flexibility in the
parameters, which often end up being attacker controlled.  Therefore
many implementations of RSASSA-PSS restrict the allowed parameters to
something reasonable, e.g. restricting the allowed hash algorithms,
requiring the two hash algorithms to be the same, and requiring the salt
size to match the digest size.  We should do likewise if possible.

> +		case rsassa_pss_verify_pss_mask:
> +			if (memcmp(args[0].from, "mgf1", 4) != 0)
> +				return -ENOPKG;

Out-of-bounds read.

As I mentioned in another reply, error-prone string parsing isn't a
great choice.  C has native support for function parameters.

> +static int emsa_pss_verify(struct rsassa_pss_ctx *ctx,
> +			   const u8 *M, unsigned int M_len,
> +			   const u8 *EM, unsigned int emLen)
> +{
> +	unsigned int emBits, hLen, sLen, DB_len;
> +	const u8 *maskedDB, *H;
> +	u8 *mHash, *dbMask, *DB, *salt, *Mprime, *Hprime;
> +	int err, i;
> +
> +	emBits = 8 - fls(EM[0]);
> +	emBits = emLen * 8 - emBits;

This does not implement EMSA-PSS-VERIFY correctly.  Please check the
specification.  emBits is supposed to be determined solely from the
public key.

Please also add KUnit tests for this code, including edge cases.

FYI: it will take a couple more passes to properly review this:
unfortunately this encoding scheme is a bit complicated, and some of
your implementation choices like using strings instead of
straightforward function parameters don't particularly help.  These are
just a couple things I noticed in a first pass.

- Eric