All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: David Howells <dhowells@redhat.com>
Cc: Lukas Wunner <lukas@wunner.de>,
	Ignat Korchagin <ignat@cloudflare.com>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Petr Pavlu <petr.pavlu@suse.com>,
	Daniel Gomez <da.gomez@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	"Jason A . Donenfeld" <Jason@zx2c4.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	Stephan Mueller <smueller@chronox.de>,
	linux-crypto@vger.kernel.org, keyrings@vger.kernel.org,
	linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tadeusz Struk <tadeusz.struk@intel.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v13 07/12] crypto: Add RSASSA-PSS support
Date: Wed, 21 Jan 2026 02:14:13 +0000	[thread overview]
Message-ID: <20260121021413.GA998999@google.com> (raw)
In-Reply-To: <20260120224108.GC6191@quark>

On Tue, Jan 20, 2026 at 02:41:11PM -0800, Eric Biggers wrote:
> 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.

Looking into this a bit more, I'm increasingly skeptical that RSASSA-PSS
would be a worthwhile addition, especially when integrated into CMS and
X.509.  It seems that while in theory it's an improvement over PKCS#1
v1.5 padding, the specifications were messed up and it has way too many
unnecessary and error-prone parameters.  Here are some references that
describe some of the issues in RSASSA-PSS:

    * https://boringssl-review.googlesource.com/c/boringssl/+/81656
    * https://www.metzdowd.com/pipermail/cryptography/2019-November/035449.html

It seems it might not be very widely used either.

I think the fact that this patchset implements RSASSA-PSS verification
incorrectly (by not verifying that the leading bit is zero) further
validates these concerns.

With RSA also being two generations behind the current generation of
signature algorithms (RSA => elliptic curves => lattices), I'm wondering
what the motivation for this feature is.

- Eric

  parent reply	other threads:[~2026-01-21  2:14 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20 14:50 [PATCH v13 00/12] x509, pkcs7, crypto: Add ML-DSA and RSASSA-PSS signing David Howells
2026-01-20 14:50 ` [PATCH v13 01/12] crypto: Add ML-DSA crypto_sig support David Howells
2026-01-20 17:37   ` Jarkko Sakkinen
2026-01-20 20:52   ` Eric Biggers
2026-01-20 14:50 ` [PATCH v13 02/12] pkcs7: Allow the signing algo to calculate the digest itself David Howells
2026-01-20 17:53   ` Jarkko Sakkinen
2026-01-21 12:31     ` David Howells
2026-01-24 11:46       ` Jarkko Sakkinen
2026-01-20 21:12   ` Eric Biggers
2026-01-23 11:37     ` David Howells
2026-01-20 14:50 ` [PATCH v13 03/12] pkcs7: Allow direct signing of data with ML-DSA David Howells
2026-01-20 14:50 ` [PATCH v13 04/12] pkcs7, x509: Add ML-DSA support David Howells
2026-01-20 21:17   ` Eric Biggers
2026-01-20 14:50 ` [PATCH v13 05/12] modsign: Enable ML-DSA module signing David Howells
2026-01-20 21:38   ` Eric Biggers
2026-01-21 14:21     ` David Howells
2026-01-20 14:50 ` [PATCH v13 06/12] crypto: Add supplementary info param to asymmetric key signature verification David Howells
2026-01-20 14:50 ` [PATCH v13 07/12] crypto: Add RSASSA-PSS support David Howells
2026-01-20 22:41   ` Eric Biggers
2026-01-20 23:15     ` David Howells
2026-01-20 23:36       ` Eric Biggers
2026-01-21  8:11         ` Ignat Korchagin
2026-01-21  2:14     ` Eric Biggers [this message]
2026-01-21  8:15       ` Ignat Korchagin
2026-01-20 14:50 ` [PATCH v13 08/12] pkcs7, x509: " David Howells
2026-01-20 14:50 ` [PATCH v13 09/12] modsign: Enable RSASSA-PSS module signing David Howells
2026-01-20 14:50 ` [PATCH v13 10/12] pkcs7: Add FIPS selftest for RSASSA-PSS David Howells
2026-01-20 14:50 ` [PATCH v13 11/12] x509, pkcs7: Limit crypto combinations that may be used for module signing David Howells
2026-01-20 18:31   ` Ignat Korchagin
2026-01-20 18:54     ` David Howells
2026-01-20 21:51   ` Vitaly Chikunov
2026-01-20 23:18     ` David Howells
2026-01-20 22:14   ` Eric Biggers
2026-01-20 14:50 ` [PATCH v13 12/12] pkcs7: Add ML-DSA FIPS selftest David Howells
2026-01-20 17:54   ` Jarkko Sakkinen
2026-01-20 17:55     ` Jarkko Sakkinen
2026-01-20 21:43   ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260121021413.GA998999@google.com \
    --to=ebiggers@kernel.org \
    --cc=Jason@zx2c4.com \
    --cc=ardb@kernel.org \
    --cc=da.gomez@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=ignat@cloudflare.com \
    --cc=jarkko@kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mcgrof@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=samitolvanen@google.com \
    --cc=smueller@chronox.de \
    --cc=tadeusz.struk@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.