From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CE7B6346782; Tue, 20 Jan 2026 22:41:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768948871; cv=none; b=jrRbFzFmJ7P8OAXaZGXng+RDJdtNP2ymr0tnIVSxmj8YlfcQwTG2qQA3ratHv4SXTlr9nig90gIJm8VL996SYp9pg2/UGxt15Yrcxsh0995bLmEMRiZoBC7676GgGWNqzopfi2H+dkmxKtyzuPr3H+TJ4r4NpeUHclpnACACeq0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768948871; c=relaxed/simple; bh=lYDtHpMC2PRSP0n3ZWhyho0wNOnkm4bjkT4lWx/9tWQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TphdABeZ4X8LBOtdYlpUjXHDXCRisWilTUUqJ/c1VfImo8iDFmn/owjCIeUeKGyJs29z0dvaqLKvwPNrMgrLAKrNmI8sf020G6WXUhHDv1+lphUe97yMGm8uv6297wDVwD5vBqSHOxpvukRLlJ81gcEHfq/FULG5UPRRM5GuM1Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=drAFt7nh; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="drAFt7nh" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 788FEC16AAE; Tue, 20 Jan 2026 22:41:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1768948871; bh=lYDtHpMC2PRSP0n3ZWhyho0wNOnkm4bjkT4lWx/9tWQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=drAFt7nh9TEGlVtgDNL5IoFO+BVPtTRY/8D58XglJEkey5tIi7jeozQahuj2QepJv GuSxqJbJ6Tm5+dBLCIJYB7JGs+WIgu6x7I/2x4ANQ2jkoewQtuy5hgczrCHCybrLC8 xQuwiHaHmR3ige2BqszpmfKcVOA4EQQksj4e1vCIqXBHmwaEsxKZ4Sz9fCkldQ/Aqm MuIwg3d4KAysLVLXS1XrtHyNd2eAvY4H7JoHvHQ22nKTPgGxp/eABhpnBAC6gwKdP7 UqLkLQUiMTn+iJv3B6jaaTZiSpiSM2op+3+cV0x4IHrU7c9wwFBVknnAHDx9HKERop Up9p9S0tTTLKw== Date: Tue, 20 Jan 2026 14:41:08 -0800 From: Eric Biggers To: David Howells Cc: Lukas Wunner , Ignat Korchagin , Jarkko Sakkinen , Herbert Xu , Luis Chamberlain , Petr Pavlu , Daniel Gomez , Sami Tolvanen , "Jason A . Donenfeld" , Ard Biesheuvel , Stephan Mueller , linux-crypto@vger.kernel.org, keyrings@vger.kernel.org, linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, Tadeusz Struk , "David S. Miller" Subject: Re: [PATCH v13 07/12] crypto: Add RSASSA-PSS support Message-ID: <20260120224108.GC6191@quark> References: <20260120145103.1176337-1-dhowells@redhat.com> <20260120145103.1176337-8-dhowells@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260120145103.1176337-8-dhowells@redhat.com> 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= > > The hash algorithm to be used to digest the data. > > (2) pss_mask=,... > > The mask generation function (MGF) and its parameters. > > (3) pss_salt= > > 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