Re: [pkcs11] Issues with SP 800 KDF in PKCS #11.

[Robert Relyea <rrelyea@REDHAT.COM>]

Based on these comments I'll update the header for 1, 2, and 3. Alex has accepted that he just misread the spec for comment 4 based on Darren's reply, so I don't think we need to do anything there.

I've included Alex's comments below (which includes another question for Darren).

bob


On 11/12/2019 01:35 PM, JOHNSON Darren wrote:

Hi,

Iâve tried to provide some feedback for 2 and 4.

Â

I plan on being on the call this week if we need to discuss it further.

Â

Thanks.

Darren

Â

From: pkcs11@lists.oasis-open.org [mailto:pkcs11@lists.oasis-open.org] On Behalf Of Robert Relyea
Sent: Tuesday, November 12, 2019 4:07 PM
To: pkcs11@lists.oasis-open.org
Subject: [pkcs11] Issues with SP 800 KDF in PKCS #11.

Â

Â

Â

The guys working in our CS team has been trying to implement SP 800 KDF that we added to 3.0 and have found the following issues (with his suggested fixes). (Inline are my comments for them).

1. PKCS#11 v3.0 Cryptographic Token Interface Current Mechanisms specification specifies a CK_SP800_108_COUNTER, while the pkcs11t.h from PKCS#11 v3.0 Cryptographic Token Interface Base Specification specifies CK_SP800_108_OPTIONAL_COUNTER.

• I've provided both, making CK_SP800_108_COUNTER define to CK_SP800_108_OPTIONAL_COUNTER

I think we can handle this as a non-material change as recommended above.

2. In both CK_SP800_108_KDF_PARAMS and CK_SP800_108_FEEDBACK_KDF_PARAMS, pAdditionalDerivedKeys is a single CK_DERIVED_KEY, when it should be a CK_DERIVED_KEY_PTR, since it is interpreted as an array of CK_DERIVED_KEYs. This appears in both the CTICM and pkcs11t.h.

• I've fixed it in both places.

This is an issue. The text says it's an array, but the header and definition has the wrong type? Is this material or a typo?

[DJ] The intent was to make it an array as many protocols required 3-keys to be derived. Only allowing one additional key to be derived would be restrictive.Â

3. PKCS#11 v3.0 Cryptographic Token Interface Current Mechanisms specification specifies CK_SP800_108_DKM_LENGTH_SUM_OF_KEYS and CK_SP800_108_DKM_LENGTH_SUM_OF_SEGMENTS methods (and defines what they do), but the pkcs11t.h from PKCS#11 v3.0 Cryptographic Token Interface Base Specification doesn't assign them constant values.

• I've fixed it by assigning 1 to the former, 2 to the latter:

oÂÂÂ /* ERRATA: PKCS#11 v3.0 Cryptographic Token Interface Current Mechanisms

oÂÂÂ Â* defines that these constants exist, but doesn't specify values. pkcs11t.h

oÂÂÂ Â* from PKCS#11 v3.0 Cryptographic Token Interface Base Specification doesn't

oÂÂÂ Â* define these constants either. */

oÂÂÂ #define CK_SP800_108_DKM_LENGTH_SUM_OF_KEYSÂÂÂÂ 0x00000001UL

#define CK_SP800_108_DKM_LENGTH_SUM_OF_SEGMENTS 0x00000002UL

I think I can handle this as non-material like the other missing identifiers.

4. For all three sections, PKCS#11 v3.0 says: about CK_ SP800_108_DKM_LENGTH, "This data field type is optional.", however no default method is chosen in the later section on deriving additional keys. So either PKCS#11 v3.0 should define a default, or make the field required when pAdditionalDerivedKeys is non-empty (and make it explicitly ignored otherwise).

• Additionally, without this field, there's no way to indicate the endianness of L parameter, so perhaps it would be better to make this a required field.

I'm not sure how to handle this one. Basically the field is optional, but if it's not specified, the behavior is undefined. Maybe leave it and say as such in v3.1.

[DJ] I would say it is clear, but I wrote it so I guess my vote doesnât count there J.

Those fields are defined as optional. Which means they do not need to be included in the CK_PRF_DATA_PARAM structure. Section â2.42.8 Constructing the PRF Input Dataâ explains how each item CK_PRF_DATA_PARAM structure is concatenated. So if the DKM_LENGTH is not in the structure, it wonât be part of the concatenated/serialized data that is passed in to the PRF.

There is no default value. If DKM value is desired, then it must be specified in the mechanism parameters. ÂThat was the problem with SP800-108 in general, it gave very few specifics.

Iâm a bit confused by their reference to âderiving additional keysâ. DKM is not required if additional keys is required. That âderiving additional keysâ section just provides clear examples so that âifâ you are deriving multiple keys, and âifâ specify one of the DKM options, then it shows what the DKM value will be. But again, if you donât specify one of the DKM options, then there wonât be a DKM value in the PRF input.

Yeah sorry about the noise on this one. The DKM section was vague at
first to me since I apparently skipped a required sentence. I think
it is fine as-is.

Mostly this stemmed from my original confusion around how the
additional derived keys worked. I thought -- on first reading of
the PKCS#11 spec -- that keys could be at arbitrary locations in
the key stream (and that one needed to parse the template to figure
out where they were actually located and what their size was, to
ensure they didn't overlap).

Instead they're always sequential and aligned to the start of a PRF
boundary, so the spec makes more sense, reading that sentence. (Calling
the struct DKM conflated it with the additional derived keys in my
mind so I didn't figure out what I thought this errata was about until
I had implemented derived key materials. I've updated the PR errata
list).


Its fine, but a touch weird. What's the use case for those requirements
that DJ mentions in #3? I'm curious if they offer any more test vectors.
NIST doesn't have any public test vectors fully exercising the extent of
the KDF (and none with the length field appended to the PRF input), so
any additional ones would be useful.


bob