Adding algorithm agility to the crypto library functions

Adding algorithm agility to the crypto library functions

[James Bottomley]

The TPM session calculation functions recently got converted to use the
rawkey functions instead of open coding it 64a7cfbcf548 ("tpm: Use
HMAC-SHA256 library instead of open-coded HMAC").  This works today
because the user has no input on the hmac hash algorithm so, although
the TPM specifies it to be agile, we can simply choose sha256. 
However, we have plans to use what are called policy sessions, which
have require the same hash as the user supplied object used for its
name (essentially a hash chosen by the user).  In a TPM these hashes
can be any of the family sha1 sha256, sha384 sha512 plus a few esoteric
ones like sm3.  So the question becomes: to avoid going back to open
coding the hmac and using the shash API, is there a way of adding hash
agility to the library algorithms?  I suppose I could also do this
inside our hmac code using a large set of switch statements, but that
would be a bit gross.

If no-one's planning to do this I can take a stab ... it would probably
still be a bunch of switch statements, but not in my code ...

Regards,

James

Re: Adding algorithm agility to the crypto library functions

[Eric Biggers]

On Tue, Oct 14, 2025 at 12:01:34PM -0400, James Bottomley wrote:
> The TPM session calculation functions recently got converted to use the
> rawkey functions instead of open coding it 64a7cfbcf548 ("tpm: Use
> HMAC-SHA256 library instead of open-coded HMAC").

To be clear, drivers/char/tpm/tpm2-sessions.c has always been hard-coded
to use SHA-256 and HMAC-SHA256, and it's always used the SHA-256
library.  It just open-coded the HMAC construction.

> because the user has no input on the hmac hash algorithm so, although
> the TPM specifies it to be agile, we can simply choose sha256. 
> However, we have plans to use what are called policy sessions, which
> have require the same hash as the user supplied object used for its
> name (essentially a hash chosen by the user).  In a TPM these hashes
> can be any of the family sha1 sha256, sha384 sha512 plus a few esoteric
> ones like sm3.  So the question becomes: to avoid going back to open
> coding the hmac and using the shash API, is there a way of adding hash
> agility to the library algorithms?  I suppose I could also do this
> inside our hmac code using a large set of switch statements, but that
> would be a bit gross.
> 
> If no-one's planning to do this I can take a stab ... it would probably
> still be a bunch of switch statements, but not in my code ...

This isn't the job of lib/crypto/.

If a caller would like to support a certain set of algorithms, it should
just write the 'if' or 'switch' statement itself.

The nice thing about that is that it results in the minimum number of
branches and the minimum stack usage for the possible set of algorithms
at that particular call site.  (Compare to crypto_shash which always
uses indirect calls and always uses almost 400 bytes for each
SHASH_DESC_ON_STACK().  SHASH_DESC_ON_STACK() has to be sized for the
worst possible case among every hash algorithm in existence, regardless
of whether the caller is actually using it or not.)

That approach is already used successfully in fs/verity/ and net/sctp/.
I'm planning to make fs/btrfs/ adopt it too.  It works fine, and it's
the most efficient solution.

If a particular caller has a super long list of algorithms or is dealing
with a legacy arbitrary user-specified string, then it's of course still
free to use crypto_shash.

But I have to wonder, do you really need to add support for all these
hash algorithms?  Adding SHA-1 and SM3 support, really?

What stops you from just saying that the kernel supports SHA-256 for
these user supplied objects, and that's it?

Getting kernel developers to think carefully about what set of crypto
algorithms they'd like to support in their features, rather than punting
the problem to a generic crypto layer that supports all sorts of
insecure and esoteric options, isn't necessarily a bad thing...

- Eric

Re: Adding algorithm agility to the crypto library functions

[Ard Biesheuvel]

On Tue, 14 Oct 2025 at 18:57, Eric Biggers <ebiggers@kernel.org> wrote:
>
...
> > because the user has no input on the hmac hash algorithm so, although
> > the TPM specifies it to be agile, we can simply choose sha256.
> > However, we have plans to use what are called policy sessions, which
> > have require the same hash as the user supplied object used for its
> > name (essentially a hash chosen by the user).  In a TPM these hashes
> > can be any of the family sha1 sha256, sha384 sha512 plus a few esoteric
> > ones like sm3.  So the question becomes: to avoid going back to open
> > coding the hmac and using the shash API, is there a way of adding hash
> > agility to the library algorithms?  I suppose I could also do this
> > inside our hmac code using a large set of switch statements, but that
> > would be a bit gross.
> >
> > If no-one's planning to do this I can take a stab ... it would probably
> > still be a bunch of switch statements, but not in my code ...
>
> This isn't the job of lib/crypto/.
>
> If a caller would like to support a certain set of algorithms, it should
> just write the 'if' or 'switch' statement itself.
>
> The nice thing about that is that it results in the minimum number of
> branches and the minimum stack usage for the possible set of algorithms
> at that particular call site.  (Compare to crypto_shash which always
> uses indirect calls and always uses almost 400 bytes for each
> SHASH_DESC_ON_STACK().  SHASH_DESC_ON_STACK() has to be sized for the
> worst possible case among every hash algorithm in existence, regardless
> of whether the caller is actually using it or not.)
>

I agree with this in principle, but couldn't we provide /some/ level
of support in the library so that users don't have to do it, and end
up breaking things, either functionally or in terms of security? The
fact that you yourself have already implemented this in 3+ places
suggests that there may be many other occurrences of this pattern in
the future.

I agree that adding a generic hash API that takes a char[] algo_name
and supports every flavor of hash that we happen to implement is a bad
idea. But perhaps there is some middle ground here, with a build-time
constant mask representing the algorithms that are actually supported
at the call site, so that the compiler can prune the bits we don't
need? I'm asking for the sake of discussion here, though - I don't
have a use case myself where this is needed.

> But I have to wonder, do you really need to add support for all these
> hash algorithms?  Adding SHA-1 and SM3 support, really?
>
> What stops you from just saying that the kernel supports SHA-256 for
> these user supplied objects, and that's it?
>
> Getting kernel developers to think carefully about what set of crypto
> algorithms they'd like to support in their features, rather than punting
> the problem to a generic crypto layer that supports all sorts of
> insecure and esoteric options, isn't necessarily a bad thing...
>

Yeah, that's a good point too.

Re: Adding algorithm agility to the crypto library functions

[Eric Biggers]

On Tue, Oct 14, 2025 at 07:08:39PM +0200, Ard Biesheuvel wrote:
> On Tue, 14 Oct 2025 at 18:57, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> ...
> > > because the user has no input on the hmac hash algorithm so, although
> > > the TPM specifies it to be agile, we can simply choose sha256.
> > > However, we have plans to use what are called policy sessions, which
> > > have require the same hash as the user supplied object used for its
> > > name (essentially a hash chosen by the user).  In a TPM these hashes
> > > can be any of the family sha1 sha256, sha384 sha512 plus a few esoteric
> > > ones like sm3.  So the question becomes: to avoid going back to open
> > > coding the hmac and using the shash API, is there a way of adding hash
> > > agility to the library algorithms?  I suppose I could also do this
> > > inside our hmac code using a large set of switch statements, but that
> > > would be a bit gross.
> > >
> > > If no-one's planning to do this I can take a stab ... it would probably
> > > still be a bunch of switch statements, but not in my code ...
> >
> > This isn't the job of lib/crypto/.
> >
> > If a caller would like to support a certain set of algorithms, it should
> > just write the 'if' or 'switch' statement itself.
> >
> > The nice thing about that is that it results in the minimum number of
> > branches and the minimum stack usage for the possible set of algorithms
> > at that particular call site.  (Compare to crypto_shash which always
> > uses indirect calls and always uses almost 400 bytes for each
> > SHASH_DESC_ON_STACK().  SHASH_DESC_ON_STACK() has to be sized for the
> > worst possible case among every hash algorithm in existence, regardless
> > of whether the caller is actually using it or not.)
> >
> 
> I agree with this in principle, but couldn't we provide /some/ level
> of support in the library so that users don't have to do it, and end
> up breaking things, either functionally or in terms of security? The
> fact that you yourself have already implemented this in 3+ places
> suggests that there may be many other occurrences of this pattern in
> the future.
> 
> I agree that adding a generic hash API that takes a char[] algo_name
> and supports every flavor of hash that we happen to implement is a bad
> idea. But perhaps there is some middle ground here, with a build-time
> constant mask representing the algorithms that are actually supported
> at the call site, so that the compiler can prune the bits we don't
> need? I'm asking for the sake of discussion here, though - I don't
> have a use case myself where this is needed.

Right, it would be theoretically possible to have something like:

    static inline void hash(u32 supported_algos_bitmask, u32 algo,
                            const u8 *data, size_t len, u8 *out)
    {
            if ((supported_algos_bitmask & (1 << HASH_ALGO_SHA256)) &&
                algo == HASH_ALGO_SHA256)
                    sha256(data, len, out);
            else if ((supported_algos_bitmask & (1 << HASH_ALGO_SHA512)) &&
                    algo == HASH_ALGO_SHA512)
                    sha512(data, len, out);
            ... and so on
    }

And then something similar for init/update/final.  They'd have to take
in some sort of generic context type, which the caller would ensure is
sized to the maximum size needed by any supported algorithm.

I'm not sure I like this, though.  Just having the callers do it seems a
lot more straightforward.  (And again, crypto_shash remains an option
too.)

Of course, we should also keep in mind that usually in-kernel users only
want a single algorithm in the first place.  So this entire discussion
is about the less common case.

> > But I have to wonder, do you really need to add support for all these
> > hash algorithms?  Adding SHA-1 and SM3 support, really?
> >
> > What stops you from just saying that the kernel supports SHA-256 for
> > these user supplied objects, and that's it?
> >
> > Getting kernel developers to think carefully about what set of crypto
> > algorithms they'd like to support in their features, rather than punting
> > the problem to a generic crypto layer that supports all sorts of
> > insecure and esoteric options, isn't necessarily a bad thing...
> >
> 
> Yeah, that's a good point too.

- Eric

Re: Adding algorithm agility to the crypto library functions

[James Bottomley]

On Tue, 2025-10-14 at 19:08 +0200, Ard Biesheuvel wrote:
[...]
> On Tue, 14 Oct 2025 at 18:57, Eric Biggers <ebiggers@kernel.org>
> wrote:
> 
> > But I have to wonder, do you really need to add support for all
> > these hash algorithms?  Adding SHA-1 and SM3 support, really?

I'm OK with not supporting sha1 because it's deprecated (although there
is no known preimage attack that would work for TPM policies).  I'm
also happy saying we'll reject sm3 objects unless someone wants to
patch it into lib/crypto.  But ...

> > What stops you from just saying that the kernel supports SHA-256
> > for these user supplied objects, and that's it?

NIST is deprecating sha-256 for Post Quantum so it's a time limited
choice before we have to do agile anyway.

> > Getting kernel developers to think carefully about what set of
> > crypto algorithms they'd like to support in their features, rather
> > than punting the problem to a generic crypto layer that supports
> > all sorts of insecure and esoteric options, isn't necessarily a bad
> > thing...
> 
> Yeah, that's a good point too.

Well it's a tradeoff between supporting objects created outside the
kernel by a process we have no control over and trying to be good about
algorithm hygiene.  The question we'll get asked in the trouble tickets
is why won't the kernel process an object my TPM created.  I'm happy
saying because sha1 is deprecated or if you want sm3 support you need
to write it but I think we have to support at least the viable sha2
variants.

Regards,

James

Re: Adding algorithm agility to the crypto library functions

[Eric Biggers]

On Tue, Oct 14, 2025 at 01:32:01PM -0400, James Bottomley wrote:
> NIST is deprecating sha-256 for Post Quantum so it's a time limited
> choice before we have to do agile anyway.

So support SHA-256 and SHA-512.

Let's not do "we have to support multiple algorithms anyway, so let's
add a bunch of other random algorithms too".

- Eric

Re: Adding algorithm agility to the crypto library functions

[James Bottomley]

On Tue, 2025-10-14 at 17:39 +0000, Eric Biggers wrote:
> On Tue, Oct 14, 2025 at 01:32:01PM -0400, James Bottomley wrote:
> > NIST is deprecating sha-256 for Post Quantum so it's a time limited
> > choice before we have to do agile anyway.
> 
> So support SHA-256 and SHA-512.

NIST seems to be leaning towards SHA-384 but some people have been
arguing for SHA-512 instead, so it looks like it has to be all three of
them.

> Let's not do "we have to support multiple algorithms anyway, so let's
> add a bunch of other random algorithms too".

I'm not saying we have to match the entire agile set of the TPM (as I
said, I think there are good reasons for saying no to sha1 and sm3),
but I do think we have to do the three sha-2 variants.

Regards,

James

Re: Adding algorithm agility to the crypto library functions

[Simon Richter]

Hi,

On 10/15/25 2:32 AM, James Bottomley wrote:

> The question we'll get asked in the trouble tickets
> is why won't the kernel process an object my TPM created.  I'm happy
> saying because sha1 is deprecated [...]
This is largely a policy decision, though, so it should be left to the 
user, perhaps with a default of "following NIST policy."

    Simon