[cryptolib] Move SHA-2/HMAC computations to HMAC HWIP

[ballifatih]

Description

I want to highlight how our current hybrid SHA-2/HMAC cryptolib+HW stack looks, so we can discuss how to move from SW implementation to HW implementation. Here is the current stack:

The arrows indicate the use of functions, for example HMAC-256 (streaming) is implemented with calls to SHA-256 (streaming). This means, for SHA-2/HMAC calls made to cryptolib:

• Usually one-shot calls use {init, update, final} calls from its streaming equivalent in sequence.
• We are not using HMAC functionality of HMAC HW at the moment, but only hashing.
• Except one-shot SHA-256, all use cases lead to the use of OTBN, which might create bottleneck with other OTBN use cases. AFAIU, it is required to load run_sha256/512.s here, so the user might lose the context in OTBN. For instance, that might be problematic if HMAC-based SW DRBG runs in the middle of asymmetric operation.
• I am not sure about performance, but I would expect HMAC to more efficiently handle SHA-2 and HMAC loads. Plus, we already have the new context switching functionality.

This implies multiple changes with a goal to flatten this stack and move SHA-2/HMAC computations from OTBN to HMAC HWIP. It would imply:

• Add streaming support for SHA-256 to HMAC driver (hmac.c), so that we can use the context switching supported natively by HMAC HWIP.
• Extend HMAC driver (hmac.c) to support SHA-384 and SHA-512 (with streaming functionality) and connect these directly to crypto-API-level SHA-2 calls. [cryptolib] Integrate hw SHA2 384/512 to cryptolib #22547
• Extend HMAC driver (hmac.c) to support HMAC-384 and HMAC-512 (with streaming functionality) and connect these to crypto-API-level HMAC calls.
• Redirect one-shot SHA-256/384/512 and HMAC-256/384/512 calls to HMAC driver (hmac.c).

HMAC HWIP also imposes certain restrictions on the key length that we also need to be aware of:

opentitan/hw/ip/hmac/README.md

Line 28 in ec71152

It supports SHA-2 256/384/512 and 256/384/512/1024-bit keys in HMAC mode, so long as the key length does not exceed the block size of the configured SHA-2 mode, i.e., 1024-bit keys are not supported for SHA-2 256 where the block size is 512-bit.

FYI @vsukhoml, regarding the choice of parameters available for HMAC. If you have particular parameters in mind, it might help up prioritize the tasks accordingly.

cc: @jadephilipoom @moidx @johannheyszl @gdessouky

[vsukhoml]

that might be problematic if HMAC-based SW DRBG runs in the middle of asymmetric operation

Because OTBN is used, right? We don't usually run multiple crypto operations as our current implementation design choices prohibits it due to other kind of shared context.

The plan looks good, however we probably need 2 modes - streaming (when context is explicitly stored in memory), and "atomic" when all operations are done one after another and then only result is used. The latter is SW DRBG use case - no intermediate context load/store is needed and even more it is explicitly not needed for efficiency.
Snapshot of the code:

/* HMAC_DRBG flow in NIST SP 800-90Ar1, 10.2, RFC 6979
 */
/* V = HMAC(K, V) */
static void update_v(const uint32_t *k, uint32_t *v)
{
	struct hmac_sha256_ctx ctx;

	HMAC_SHA256_hw_init(&ctx, k, SHA256_DIGEST_SIZE);
	HMAC_SHA256_update(&ctx, v, SHA256_DIGEST_SIZE);
	memcpy(v, HMAC_SHA256_final(&ctx), SHA256_DIGEST_SIZE);
}

/* K = HMAC(K, V || tag || p0 || p1 || p2) */
/* V = HMAC(K, V) */
static void update_kv(uint32_t *k, uint32_t *v, uint8_t tag, const void *p0,
		      size_t p0_len, const void *p1, size_t p1_len,
		      const void *p2, size_t p2_len)
{
	struct hmac_sha256_ctx ctx;

	HMAC_SHA256_hw_init(&ctx, k, SHA256_DIGEST_SIZE);
	HMAC_SHA256_update(&ctx, v, SHA256_DIGEST_SIZE);
	HMAC_SHA256_update(&ctx, &tag, 1);
	HMAC_SHA256_update(&ctx, p0, p0_len);
	HMAC_SHA256_update(&ctx, p1, p1_len);
	HMAC_SHA256_update(&ctx, p2, p2_len);
	memcpy(k, HMAC_SHA256_final(&ctx), SHA256_DIGEST_SIZE);
	update_v(k, v);
}

the choice of parameters available for HMAC
If you support in HW HMAC keys of block size, then you only need to support keys of larger than block size, which are trivially SHA operation on the key. Why not to add this simple wrapper to avoid kicking can down the road?

[jadephilipoom]

I agree, now that we have HW support there's no need to use the OTBN implementations. This looks like a good plan to me.

* I am not sure about performance, but I would expect HMAC to more efficiently handle SHA-2 and HMAC loads. Plus, we already have the new context switching functionality.

Yep, last I checked the hardware implementation was (unsurprisingly) many times faster -- the OTBN one was a temporary workaround because we didn't have context switching.

* [ ]  Extend HMAC driver (hmac.c) to support SHA-384 and SHA-512 (with streaming functionality). [[cryptolib] Integrate hw SHA2 384/512 to cryptolib #22547](https://github.com/lowRISC/opentitan/issues/22547)

* [ ]  Extend HMAC driver (hmac.c) to support HMAC-384 and HMAC-512 (with streaming functionality).

Just to clarify: I think one of these duplicate entries is probably supposed to be something like "Redirect streaming SHA-256/384/512 and HMAC-256/384/512 calls to HMAC driver", no?

[ballifatih]

Because OTBN is used, right? We don't usually run multiple crypto operations as our current implementation design choices prohibits it due to other kind of shared context.

If I understand correctly, this might become an issue only if you care about the OTBN context at the time you make the HMAC/SHA-2 (except SHA-256) call. OTBN-based HMAC/SHA-2 calls preempt the current app on OTBN, which means any context stored in its DMEM (such as keys) will be removed, so that computation needs to start from scratch. Does anyone think there is a conflict here? This may be important for prioritization of the tasks here accordingly (otherwise I think most items here are P2-P3).

then you only need to support keys of larger than block size, which are trivially SHA operation on the key.

Sure, we can support larger key sizes by additional hash invocation from SW, I just wanted to point out the limitations of HW (compared to OTBN-based solution). Probably not that relevant.

Based on your code snippet, I think the highest priority item here is to provide both streaming and one-shot HMAC-256 through HMAC HWIP.

Just to clarify: I think one of these duplicate entries is probably supposed to be something like "Redirect streaming SHA-256/384/512 and HMAC-256/384/512 calls to HMAC driver", no?

I did not quite understand the clarification, but I assumed my wording was not precise. I also added “connect these directly to crypto-API-level SHA-2/HMAC calls.” to both items. My partitioning of the tasks might not be optimal, but I think what is really important here is that both SHA-2 and HMAC use native functions from HMAC HW instead of HMAC algorithm invoking SHA-2 algorithm.

[jadephilipoom]

If I understand correctly, this might become an issue only if you care about the OTBN context at the time you make the HMAC/SHA-2 (except SHA-256) call. OTBN-based HMAC/SHA-2 calls preempt the current app on OTBN, which means any context stored in its DMEM (such as keys) will be removed, so that computation needs to start from scratch. Does anyone think there is a conflict here?

Yep, this describes the current setup -- the cryptolib doesn't rely on any hardware state between operations in general, but the *_async_start and *_async_finalize calls are exceptions. They run long-running operations on OTBN and it's safe to run non-OTBN operations while they're going. However, running another OTBN operation will result in either an error (if OTBN is still busy) or it will actually erase the state you should have gotten from finalize. This issue discusses how to make the second case an error as well, which might be nicer behavior: #19173

However, either way, switching SHA-2 and HMAC to the hardware will eliminate conflict with async OTBN operations.

[gdessouky]

There is an open PR correcting the documentation; key sizes supported are 128/256/384/512/1024-bit so long as the key size is smaller than or equal to the block size of the digest size, assuming a preliminary hash invocation would hash down a larger key to the block size before feeding it with the intended data to hash as @vsukhoml says.

Atomic and streaming modes can both be supported with the HW depending on how the HW is triggered with the commands. Atomic would be triggered by hash_start then hash_process until hashing is completed (hmac_done is received) vs. streaming which would additionally use hash_stop and then hash_continue repeatedly until a message completes and then hash_process is fed.

[ballifatih]

Closing this issue as all variants are tied to HMAC HWIP through recent driver changes.