[rom] Add "pure" and "pre-hash" domain separators for SPHINCS+. #23765

jadephilipoom · 2024-06-21T11:02:59Z

Includes commits from #23762; draft until that PR is merged.

Adds an OTP parameter to toggle between "pure" and "pre-hashed" SPHINCS+ mode, and code in the ROM to toggle between the two.

I did not add tests for the pre-hashed mode because, to my knowledge, test data is not available. @moidx @cfrantz are you aware of any existing pre-generated tests or have any thoughts on how to test this other than to just make some more test data a la spx_verify_functest to check that the opentitantool and device code agree?

Part of #23144

cfrantz · 2024-06-21T15:54:45Z

hw/ip/otp_ctrl/data/BUILD

+                # Use pre-hashed SPX+ (if SPX+ enabled). See the definition of
+                # `hardened_bool_t` in sw/device/lib/base/hardened.h.
+                "CREATOR_SW_CFG_SIGVERIFY_SPX_PREHASH": otp_hex(CONST.HARDENED_FALSE),


I think we should consult with @moidx on this: should the SPX+ variant be determined by OTP, or should we put something in the manifest to identify the signature variant?

The specification recommends not using the same key in both pre-hashed and pure forms, so using this new OTP setting is ok.

I prefer to use the existing ROT_CREATOR_AUTH_CODESIGN_SPX_KEY_CONFIG0 so that we can make this decision at key provisioning time.

opentitan/hw/ip/otp_ctrl/data/otp_ctrl_mmap.hjson

Lines 430 to 433 in 6e698b4

{

name: "ROT_CREATOR_AUTH_CODESIGN_SPX_KEY_CONFIG0",

size: "4"

},

The key types can be augmented here:

opentitan/sw/device/silicon_creator/lib/sigverify/spx_key.h

Lines 56 to 76 in 6e698b4

/**

* SPX configuration ID.

*

* Used to identify the SPX parameter confuration used to sign/verify a message.

*

* Encoding generated with:

* ./util/design/sparse-fsm-encode.py -d 6 -m 2 -n 32 -s 359186736 --language=c

*/

typedef enum sigverify_spx_config_id {

/**SPHINCS+-SHAKE-128s*/

kSigverifySpxConfigIdShake128s = 0x0142410e,

/**

* SPHINCS+-SHAKE-128s-q20

*

* As specified in https://eprint.iacr.org/2022/1725.pdf.

*

* n | h | d | b | k | w | bitsec | sigsize

* 16 | 18 | 1 | 24 | 6 | 16 | 128 | 3264

*/

kSigverifySpxConfigIdShake128sQ20 = 0x9b28d8da,

} sigverify_spx_config_id_t;

We can consider removing the config fields if we are not planning to use them, but take a look first. It is possible that the config field is part of the key struct used in spx sigverify.

As I understand it, the guidance is that one should not use the same key for creating pure vs. pre-hashed signatures. If we follow that guidance, then like the algorithm variant, then pure vs. pre-hashed tells you how you can use the key.

With that in mind, recall the OTP structure that encodes the key:

SPX_KEY_TYPEn: a word associating a key with the TEST, DEV or PROD lifecycle states. SPX_KEYn: 32 bytes of key material SPX_KEY_CONFIGn: a word identifying the key algorithm (SHAKE-128s, SHA2-128s, etc).

I think we can dual-purpose the CONFIGn word to encode both the algorithm and key-usage constraint. That is to say:

enum key_config_t { kShake_128s_pure, kShake_128s_prehashed, kSha2_128s_pure, kSha2_128s_prehashed, ... }

Then plumb the config_id field through and use it to call the verify function appropriately.

It is almost there...

opentitan/sw/device/silicon_creator/rom/sigverify_key_types.h

Lines 223 to 236 in 6e698b4

typedef struct sigverify_rom_spx_key_entry {

/**

* Type of the key.

*/

sigverify_key_type_t key_type;

/**

* An SPX public key.

*/

sigverify_spx_key_t key;

/**

* Parameter configuration ID for the SPX key.

*/

sigverify_spx_config_id_t config_id;

} sigverify_rom_spx_key_entry_t;

cfrantz · 2024-06-24T17:48:31Z

LGTM, pending the resolution of how we decide between Pure and Pre-Hashed.

moidx · 2024-06-24T21:39:32Z

hw/ip/otp_ctrl/data/BUILD

+                # Use pre-hashed SPX+ (if SPX+ enabled). See the definition of
+                # `hardened_bool_t` in sw/device/lib/base/hardened.h.
+                "CREATOR_SW_CFG_SIGVERIFY_SPX_PREHASH": otp_hex(CONST.HARDENED_FALSE),


It is almost there...

opentitan/sw/device/silicon_creator/rom/sigverify_key_types.h

Lines 223 to 236 in 6e698b4

typedef struct sigverify_rom_spx_key_entry {

/**

* Type of the key.

*/

sigverify_key_type_t key_type;

/**

* An SPX public key.

*/

sigverify_spx_key_t key;

/**

* Parameter configuration ID for the SPX key.

*/

sigverify_spx_config_id_t config_id;

} sigverify_rom_spx_key_entry_t;

moidx · 2024-06-24T21:41:05Z

sw/device/silicon_creator/lib/sigverify/spx_verify.c

+ * In our case, `ctx` is always the empty string and PH (the pre-hashing
+ * function) is always SHA256.
+ */
+static const uint8_t kSpxVerifyPrehashDomainSep[] = {


I think this is the right byte ordering. Also the right OID identifier for SHA-2-256.

OID ← toByte(0x0609608648016503040201, 11) ▷ 2.16.840.1.101.3.4.2.1

jadephilipoom · 2024-06-25T12:48:28Z

I've now re-worked the PR to use the existing configuration field as discussed above.

jadephilipoom · 2024-06-25T14:10:52Z

Moving out of draft since #23762 is now merged

cfrantz · 2024-06-25T15:34:59Z

sw/device/silicon_creator/lib/sigverify/spx_verify.c

-    lifecycle_state_t lc_state, const void *msg_prefix_1,
-    size_t msg_prefix_1_len, const void *msg_prefix_2, size_t msg_prefix_2_len,
-    const void *msg, size_t msg_len, uint32_t *flash_exec) {
+    const sigverify_spx_config_id_t *config, lifecycle_state_t lc_state,


The config_id_t is an integer; I don't think we need to pass a pointer to it here.

cfrantz · 2024-06-25T15:37:37Z

sw/device/silicon_creator/rom/rom.c

@@ -330,14 +330,15 @@ static rom_error_t rom_verify(const manifest_t *manifest,
      &ecdsa_key));
  // SPX+ key.
  const sigverify_spx_key_t *spx_key = NULL;
+  const sigverify_spx_config_id_t *spx_config = NULL;


Since this value is just an integer, we probably don't need a pointer to it here -- we can just get a copy of it from spx_key_get and pass-by-value to spx_verify.

If you're concerned with having an initial invalid value, zero or all-ones would be perfectly good as invalid values.

cfrantz · 2024-06-25T15:43:49Z

sw/host/opentitanlib/src/crypto/spx.rs

@@ -86,6 +94,13 @@ impl SpxKeypair {
        SpxSignature(sphincsplus::spx_sign(&self.sk, full_message).unwrap())
    }

+    /// Sign `message` using the secret key in "prehash" mode with an empty context.
+    pub fn sign_prehash(&self, oid: &[u8], message: &[u8]) -> SpxSignature {


How do you want to connect this up to the CLI?

Obviously, we could have a command-line switch that selects among raw, pure and pre-hash. We could also attach some meta-data to the key to indicate its usage constraint.

Yes, I think a mode option with values raw, pure, and pre-hash, defaulting to pure, would make sense.

jadephilipoom · 2024-06-25T15:47:41Z

Resolves #23144

In the TC we agreed to switch to SHA2 parameters and then implement an option that allowed us to select either SHA2 or SHAKE if there was enough time before ROM freeze and enough ROM space. However, I think at this point there is not enough time before ROM freeze -- I could maybe implement the switching in time, but we'd be merging it at the very last second and I think we're better off focusing on testing this version instead. So once this is merged, the SPHINCS+ updates for EarlGrey prod should be complete.

sw/host/opentitanlib/src/crypto/spx.rs

Change to reflect the switch from SHAKE to SHA2 parameters. Signed-off-by: Jade Philipoom <[email protected]>

Signed-off-by: Jade Philipoom <[email protected]>

I observed this test time out on CI (when it was almost done, likely a few seconds off-target). This is likely due to some more tests being added, but because it's so close it hasn't failed every time in CI. Increasing the timeout should fix the issue. Signed-off-by: Jade Philipoom <[email protected]>

jadephilipoom · 2024-06-26T09:16:59Z

I've added a new commit that just increases the timeout to "long" for the HMAC functest in silicon_creator; since I added a few new tests in #23761, it seems like it's very close to the timeout limit and I observed it time out on CI for this PR (halfway through printing the result of the last test).

moidx

🎉🎉🎉🎉!

jadephilipoom requested review from cfrantz and moidx June 21, 2024 11:02

cfrantz reviewed Jun 21, 2024

View reviewed changes

moidx approved these changes Jun 24, 2024

View reviewed changes

jadephilipoom force-pushed the spx-sha2-pure-prehash branch from bd03ac4 to 4c2272a Compare June 25, 2024 12:48

jadephilipoom force-pushed the spx-sha2-pure-prehash branch from 4c2272a to e1de2a3 Compare June 25, 2024 14:09

jadephilipoom marked this pull request as ready for review June 25, 2024 14:10

jadephilipoom requested review from a team as code owners June 25, 2024 14:10

jadephilipoom requested review from cfrantz and engdoreis and removed request for a team June 25, 2024 14:10

cfrantz reviewed Jun 25, 2024

View reviewed changes

jadephilipoom force-pushed the spx-sha2-pure-prehash branch from e1de2a3 to 924b288 Compare June 25, 2024 15:54

cfrantz reviewed Jun 25, 2024

View reviewed changes

sw/host/opentitanlib/src/crypto/spx.rs Show resolved Hide resolved

jadephilipoom added 2 commits June 26, 2024 10:03

[rom] Change names of SPHINCS+ configuration options.

8d73bae

Change to reflect the switch from SHAKE to SHA2 parameters. Signed-off-by: Jade Philipoom <[email protected]>

[rom] Add "pure" and "pre-hashed" domain separators for SPHINCS+.

9c79331

Signed-off-by: Jade Philipoom <[email protected]>

jadephilipoom force-pushed the spx-sha2-pure-prehash branch from 924b288 to 9c79331 Compare June 26, 2024 08:14

cfrantz approved these changes Jun 26, 2024

View reviewed changes

moidx self-requested a review June 26, 2024 14:52

moidx approved these changes Jun 28, 2024

View reviewed changes

moidx merged commit baae174 into lowRISC:master Jun 28, 2024
32 checks passed

jadephilipoom deleted the spx-sha2-pure-prehash branch June 28, 2024 08:14

jadephilipoom mentioned this pull request Jul 1, 2024

[RFC] SLH-DSA Updates for Earl Grey #23144

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rom] Add "pure" and "pre-hash" domain separators for SPHINCS+. #23765

[rom] Add "pure" and "pre-hash" domain separators for SPHINCS+. #23765

jadephilipoom commented Jun 21, 2024 •

edited

Loading

cfrantz Jun 21, 2024

moidx Jun 24, 2024

cfrantz Jun 24, 2024

moidx Jun 24, 2024

cfrantz commented Jun 24, 2024

moidx Jun 24, 2024

moidx Jun 24, 2024

jadephilipoom commented Jun 25, 2024

jadephilipoom commented Jun 25, 2024

cfrantz Jun 25, 2024

cfrantz Jun 25, 2024

cfrantz Jun 25, 2024

jadephilipoom Jun 25, 2024

jadephilipoom commented Jun 25, 2024

jadephilipoom commented Jun 26, 2024 •

edited

Loading

moidx left a comment

	{
	name: "ROT_CREATOR_AUTH_CODESIGN_SPX_KEY_CONFIG0",
	size: "4"
	},

	/**
	* SPX configuration ID.
	*
	* Used to identify the SPX parameter confuration used to sign/verify a message.
	*
	* Encoding generated with:
	* ./util/design/sparse-fsm-encode.py -d 6 -m 2 -n 32 -s 359186736 --language=c
	*/
	typedef enum sigverify_spx_config_id {
	/*SPHINCS+-SHAKE-128s/
	kSigverifySpxConfigIdShake128s = 0x0142410e,
	/**
	* SPHINCS+-SHAKE-128s-q20
	*
	* As specified in https://eprint.iacr.org/2022/1725.pdf.
	*
	* n \| h \| d \| b \| k \| w \| bitsec \| sigsize
	* 16 \| 18 \| 1 \| 24 \| 6 \| 16 \| 128 \| 3264
	*/
	kSigverifySpxConfigIdShake128sQ20 = 0x9b28d8da,
	} sigverify_spx_config_id_t;

	typedef struct sigverify_rom_spx_key_entry {
	/**
	* Type of the key.
	*/
	sigverify_key_type_t key_type;
	/**
	* An SPX public key.
	*/
	sigverify_spx_key_t key;
	/**
	* Parameter configuration ID for the SPX key.
	*/
	sigverify_spx_config_id_t config_id;
	} sigverify_rom_spx_key_entry_t;

[rom] Add "pure" and "pre-hash" domain separators for SPHINCS+. #23765

[rom] Add "pure" and "pre-hash" domain separators for SPHINCS+. #23765

Conversation

jadephilipoom commented Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfrantz commented Jun 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jadephilipoom commented Jun 25, 2024

jadephilipoom commented Jun 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jadephilipoom commented Jun 25, 2024

jadephilipoom commented Jun 26, 2024 • edited Loading

moidx left a comment

Choose a reason for hiding this comment

jadephilipoom commented Jun 21, 2024 •

edited

Loading

jadephilipoom commented Jun 26, 2024 •

edited

Loading