Hmac support #27

jschmidb · 2025-04-23T08:35:47Z

This pull request adds support for MSA 11 protected key HMAC support to libzpc.

Support for protected HMAC keys comes with MSA 11. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

ifranzki · 2025-04-25T07:31:26Z

src/hmac_key.c

+		rc = ZPC_ERROR_PVSECRET_ID_NOT_FOUND_IN_UV;
+		goto ret;
+	}
+


Is it checked anywhere that the retrievable secret type is the expected HMAC type? There is ZPC_HMAC_SECRET_HMAC_SHA_256 or ZPC_HMAC_SECRET_HMAC_SHA_512, and the to be imported key must match the keysize set in the key object.

Also the hash type set via zpc_hmac_key_set_hash_function() must match to the secret type. Inconsistencies should be rejected (i.e. you can't use hash function SHA-512 with a retrievable secret of type ZPC_HMAC_SECRET_HMAC_SHA_256).

Yes, the logic is like:

setting the hash function for a key obj causes the key size to be set accordingly: 512 or 1024. There is no separate "set_size" function.

importing a secret ID only checks if the pvsecret is available in the system.

But when using the key later in the hmac context, the CPACF function code is set depending on the hash function of the key (see local routine __hmac_init in hmac.c). I.e. the hash function is not a property of the context, but only of the key.

Right, but what if one set SHA-256 as a hash function, but the secret ID points to a secret of type HMAC_SHA_512. This won't work together. I don't see any checks for that.

Per our offline discussions on this, I added commit "Make rc more informative".

ifranzki · 2025-04-25T07:32:08Z

src/hmac_key.c

+	case ZPC_HMAC_HASHFUNC_SHA_256:
+	case ZPC_HMAC_HASHFUNC_SHA_384:
+	case ZPC_HMAC_HASHFUNC_SHA_512:
+		break;


Is it checked anywhere that the hash function set here matches the retrievable secret type (ZPC_HMAC_SECRET_HMAC_SHA_xxx) ?

As commented above, the retrievable secret type is derived from the hash function. The import function checks if the expected secret is available in the system. For this check, the expected secret type is set in the ioctl, see routine hmac_key_make_uvrsecrettoken() in hmac_key.c.

Ok, so if the secret ID as such is valid and exists, but the type does not match the expended type, then it fails.
Does it produce different error codes if
1.) the secret Id does not exist at all
2.) the secret Id exists, but is a wrong type ?
This would be important to differentiate to give the user a hit about whats wrong.
(did not look into the code, sorry)

Hmm, good point. At the moment the PKEY_VERIFYKEY2 ioctl just fails if the "expected" secret is not available. But I'm not checking if the given secret ID probably points to an existing secret with another type. Maybe I could change the check so that both types are tried. Then we would see the exact error.

As discussed offline I added another commit to make the rc ZPC_ERROR_PVSECRET_ID_NOT_FOUND_IN_UV more informative.

holger-dengler

A mentioned in the comments, I would prefer to get rid of the assert() calls.
Please also fix the version number.
The rest looks good so far.

holger-dengler · 2025-05-14T09:38:52Z

libzpc.map

@@ -95,4 +95,15 @@ global:
 local: *;
 } ZPC_1.1.0;

+ZPC_1.4.0 {


Why are we skip 1.3.0? If this related to other features, I would recommend to do the PRs in the right sequence.

1.3.0 (and even 1.3.1) is already upstream. This was the fips 140-3 update in 2024 and caused the API behavior to change, but there were no new API functions. Therefore we go to 1.4.0 now.

holger-dengler · 2025-05-14T12:55:11Z

src/hmac_key.c

+	}
+	attr_init = 1;
+	rv = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
+	assert(rv == 0);


I know, that the existing code uses assert() calls a lot. IMO this is not the right way of error checking in libraries. I would prefer error return values instead of assert() for any new library-code.

Just to be mentioned: using assert() in the test-cases is absolutely fine!

(this applies to all asserts in ./src/*.c)

jschmidb · 2025-05-15T08:52:09Z

As mentioned in the comments, I would prefer to get rid of the assert() calls.

Per our offline discussion on this I'm leaving the assert calls for now. This will be a major restructuring and should be done in a separate item.

The following variants of protected HMAC keys are supported: - created from Ultravisor (UV) retrievable secrets (pvsecrets). - created from given clear key data (via PKEY_KBLOB2PROTK3 ioctl). - generated via sysfs attributes protkey_hmac_512 and protkey_hmac_1024 in /sys/devices/virtual/misc/pkey/protkey. Each new read on one of these sysfs attributes provides a new key of the related type. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

In the same way as for the libzpc CMAC API, if the mac parm is NULL, then an intermediate signing op is performed. If the mac parm is not NULL and the maclen is a valid MAC length, then the final MAC is computed. For verify, if the mac parm is NULL, then an intermediate verify op is performed. If the mac parm is not NULL and the maclen is a valid MAC length, then the given MAC is checked for correctness. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

Provide additional test functionality for protected-key HMAC testing. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

Mainly remove duplicate code and change some int's to size_t. While at it, also make parsing of pvsecret list file a bit more flexible. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

Tests for protected-key HMAC cover protected keys retrieved from Ultravisor retrievable secrets, random HMAC keys, and HMAC keys created from given clear key data. Tests with random HMAC keys and HMAC keys created from clear key data can be performed on a z17 or later. Ultravisor retrievable secrets are only supported on z17 or later in a secure execution guest under KVM. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

Searching for a given secret ID always means searching for an ID of a given secret type (AES-128-KEY, AES-192-KEY, HMAC-SHA-256-KEY, etc.). If this fails, there are two possible reasons: there is no secret with the given ID in the system, or the given ID belongs to a secret with another type. Therefore make the related rc and its description more informative. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

holger-dengler · 2025-05-15T10:19:48Z

ok with me.

jschmidb added 2 commits April 23, 2025 10:23

HMAC: detect MSA 11

661d319

Support for protected HMAC keys comes with MSA 11. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

HMAC: introduce new error constants

a8170b4

Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

jschmidb self-assigned this Apr 23, 2025

jschmidb requested review from ifranzki and holger-dengler April 23, 2025 08:36

jschmidb force-pushed the hmac_support branch from cbbb097 to 6c8aaa9 Compare April 23, 2025 10:23

ifranzki reviewed Apr 25, 2025

View reviewed changes

jschmidb force-pushed the hmac_support branch from b6af796 to 9a356d5 Compare April 28, 2025 09:04

jschmidb force-pushed the hmac_support branch 2 times, most recently from c73dff0 to fad91b0 Compare May 14, 2025 11:02

holger-dengler requested changes May 14, 2025

View reviewed changes

jschmidb added 7 commits May 15, 2025 11:24

HMAC: update testlib routines

e30465b

Provide additional test functionality for protected-key HMAC testing. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

HMAC: consolidate some test routines

ab64f1d

Mainly remove duplicate code and change some int's to size_t. While at it, also make parsing of pvsecret list file a bit more flexible. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

HMAC: update README.md

589ab6e

Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>

jschmidb force-pushed the hmac_support branch from fad91b0 to b171959 Compare May 15, 2025 09:24

ifranzki approved these changes May 15, 2025

View reviewed changes

holger-dengler closed this May 15, 2025

holger-dengler reopened this May 15, 2025

holger-dengler self-requested a review May 15, 2025 10:20

holger-dengler approved these changes May 15, 2025

View reviewed changes

jschmidb merged commit 546b8c1 into opencryptoki:main May 15, 2025
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hmac support #27

Hmac support #27

jschmidb commented Apr 23, 2025

ifranzki Apr 25, 2025 •

edited

Loading

jschmidb Apr 25, 2025

ifranzki Apr 25, 2025

jschmidb May 13, 2025

ifranzki Apr 25, 2025

jschmidb Apr 25, 2025

ifranzki Apr 25, 2025 •

edited

Loading

jschmidb Apr 25, 2025

jschmidb Apr 28, 2025

holger-dengler left a comment

holger-dengler May 14, 2025

jschmidb May 15, 2025

holger-dengler May 14, 2025

jschmidb commented May 15, 2025

holger-dengler commented May 15, 2025

Hmac support #27

Hmac support #27

Conversation

jschmidb commented Apr 23, 2025

ifranzki Apr 25, 2025 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ifranzki Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holger-dengler left a comment

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

jschmidb commented May 15, 2025

holger-dengler commented May 15, 2025

ifranzki Apr 25, 2025 •

edited

Loading

ifranzki Apr 25, 2025 •

edited

Loading