[pkcs11] derive secret key with hardware private key

[ncoder-1]

Hi,

I'm trying to derive a secret key from a private key stored on a pkcs11 token and a generated ecdh public key. Unfortunately, I'm doing something wrong as I'm getting the following error:

terminate called after throwing an instance of 'Botan::PKCS11::PKCS11_ReturnError'
  what():  PKCS11 error 113

I'm able to login onto the token and find the private key ojbect. I'm also able to do the opposite and derive a secret key from a generated private key and a token-stored public key. Is my PKCS11_ECDH_PrivateKey object improperly referenced or missing the attributes required to derive?

This works:

Botan::PKCS11::AttributeContainer pubkey_search_template;
pubkey_search_template.add_string(Botan::PKCS11::AttributeType::Label, "Public key for Key Management");
auto found_objs = Botan::PKCS11::Object::search<Botan::PKCS11::Object>(session, pubkey_search_template.attributes());
if (found_objs.size() != 1) return 1;

Botan::PKCS11::PKCS11_ECDH_PublicKey hsm_pub_key(session, found_objs.at(0).handle());

Botan::AutoSeeded_RNG rng;

Botan::ECDH_PrivateKey privkey_ecdh(rng, Botan::EC_Group("secp384r1"));

Botan::PK_Key_Agreement ecdhKA(privkey_ecdh, rng, "Raw");

Botan::SymmetricKey secret_key_A = ecdhKA.derive_key(48, hsm_pub_key.public_point().encode(Botan::PointGFp::UNCOMPRESSED));

std::cout << "Secret Key A: " << '\n' << secret_key_A.to_string() << '\n';

This doesn't:

Botan::PKCS11::AttributeContainer privkey_search_template;
privkey_search_template.add_string(Botan::PKCS11::AttributeType::Label, "Private key for Key Management");
auto found_objs_priv = Botan::PKCS11::Object::search<Botan::PKCS11::Object>(session, privkey_search_template.attributes());
if (found_objs_priv.size() != 1) return 1;
  
Botan::PKCS11::PKCS11_ECDH_PrivateKey hsm_priv_key(session, found_objs_priv.at(0).handle());

Botan::AutoSeeded_RNG rng;

Botan::ECDH_PrivateKey privkey_ecdh(rng, Botan::EC_Group("secp384r1"));

Botan::PK_Key_Agreement ecdhKA(hsm_priv_key, rng, "Raw", "pkcs11");

Botan::SymmetricKey secret_key_A = ecdhKA.derive_key(48, privkey_ecdh.public_point().encode(Botan::PointGFp::UNCOMPRESSED)); // crashes here

std::cout << "Secret Key A: " << '\n' << secret_key_A.to_string() << '\n';

Any help appreciated!

[ncoder-1]

After further digging by setting the debug environment flag (YKCS11_DBG=1) for the pkcs11 module I'm using (libykcs11.so), it seems PKCS11 error 113 is actually "Key derivation parameters invalid".

The function that produces that error is in ykcs11.c, and copied below:

if (params->kdf != CKD_NULL || params->ulSharedDataLen != 0 || params->pPublicData == NULL || params->ulPublicDataLen != size) {
    DBG("Key derivation parameters invalid");
    return CKR_MECHANISM_PARAM_INVALID;
  }

At the debug breakpoint of the derive_key call, CK_ECDH1_DERIVE_PARAMS is as follows: kdf=1 (which is CKD_NULL, good) ulSharedDataLen=0 (good) pSharedData=NULL (reasonable as not filled) ulPublicDataLen=99 (looks like this should be 97 for YKPIV_ALGO_ECCP384) pPublicData=filled with the pub key data (good)

I've checked and privkey_ecdh.public_value().size() is indeed 97. For PK_Key_Agreement::derive_key, in_len is also 97.

EDIT:

I've narrowed it down to the agree function of PKCS11_ECDH_KA_Operation here.

Is there any way to set my privkey_ecdh variable to have a PublicPointEncoding::Raw point_encoding so I can try?

Again, any help to get this working is appreciated.

[reneme]

In the code location you narrowed down, the public key is encoded as DER (while will likely cause the problematic 2-byte discrepancy). m_key.point_encoding() uses the private key's encoding as a reference which is set to ::Der by default. Off the top of my head, I frankly don't know why that is done.

Anyway, calling privkey_ecdn.set_public_point(), providing the HSM's public point and defining the point encoding as Raw might help. Could you try that?

To me that feels like workaround for a somewhat surprising interface. I don't see why one would need to provide the public point for this PKCS#11-hosted private key to change the point encoding. Though, I'm also not an expert on the PKCS #11 specs and their recent developments. Especially, as the method you pointed to is annotated with this comment:

/// The encoding in V2.20 was not specified and resulted in different implementations
/// choosing different encodings. Applications relying only on a V2.20 encoding (e.g. the
/// DER variant) other than the one specified now (raw) may not work with all V2.30
/// compliant tokens.

Please feel free to suggest improvements or open a pull request.

[ncoder-1]

Maybe I'm doing something wrong, but set_public_point() doesn't exist in a Botan::ECDH_PrivateKey object like privkey_ecdh above. It looks only implemented in PKCS11_EC_PrivateKey. I'll dig deeper on that one, maybe I can find a workaround.

I've also noticed that der_encoded_other_key is really just other_key (original pub key) with these extra two bytes prepended to the begining of the std::vector<uchar>: 4,97 while other_key actually starts with 4,356. Everything else is the same after, so essentially der_encoded_other_key is: (4,97 + other_key). Is it possible there's a double encoding happening? I'm not familiar with DER encoding.

That said, I'm going to compile botan with a bypass of the agree function of PKCS11_ECDH_KA_Operation and see what happens. I'll post some results when that's done.

[ncoder-1]

Good news, when bypassing the DER_Encoder in the agree function of PKCS11_ECDH_KA_Operation here, everything works and a symmetric key is generated.

I've also validated the symmetric key generated independently from botan by doing the following:

Generate ECDSA key from the ECDH key (needed for openssl): Botan::ECDSA_PrivateKey privkey_ecdsa_from_ecdh(rng, Botan::EC_Group("secp384r1"), privkey_ecdh.private_value());

Output the public key (take the output and save into temp.pub): std::cout << "Pubkey fake ECDSA X509: " << '\n' << Botan::X509::PEM_encode(privkey_ecdsa_from_ecdh) << '\n';

Convert temp.pub into a DER binary file: openssl ec -pubin -in temp.pub -pubout -outform DER -out ecpubkey.der

Perform the the derive action with pkcs11-tool: pkcs11-tool --module /usr/lib/libykcs11.so -l --derive --input-file ecpubkey.der --output-file derived-key

Confirm output matches botan's output: xxd -c 48 -p derived-key

And doing that produced the same symmetric key as botan did with the bypass.

So now what? I'm not sure how to implement a permanent fix that would not break other things. I'm suggesting maybe we put a flag to bypass the DER encoding? Or maybe somehow allow setting the private key's PublicPointEncoding?

[reneme]

Maybe I'm doing something wrong, but set_public_point() doesn't exist in a Botan::ECDH_PrivateKey object like privkey_ecdh above. It looks only implemented in PKCS11_EC_PrivateKey.

Oh sorry, I think I mixed something up in my previous reply. What I meant to say was that you should call hsm_priv_key.set_public_point(). This should -- in theory -- have the same effect as your mentioned encoding bypass.

Is it possible there's a double encoding happening? I'm not familiar with DER encoding.

No, that's exactly what DER is supposed to do. It basically just adds some extra type/length-encoding flags to your key.

Could you please try again with set_public_point() on your PKCS11 key?

[ncoder-1]

Well, good news again, I was able to use this:

hsm_priv_key.set_public_point(hsm_pub_key.public_point(), Botan::PKCS11::PublicPointEncoding::Raw);

and it worked, but I had to find the public key object prior and pass it (hsm_pub_key) otherwise I'd understandably have a Public point not set. Inferring the public key from a PKCS#11 ec private key is not possible. error.

I'd just like to confirm that this is the expected behaviour and if so, I'll close this issue.

Edit: The reason I ask is because that would mean that in order to do a derive_key() on a PKCS11 private key, you need to arbitrarily have had found its public key, set the private key pub points to "raw" using that found public key and then you can do it. But if it was "raw" by default, you wouldn't have to do all those extra steps (and lines of codes). Not that I mind but that seems the opposite of what it should be.

Thanks again!

[reneme]

Yay!

That is the behaviour I would have hoped for last week already. Sorry again for mixing up the variable names. I agree though, that this is a pretty "surprising" interface with unnecessary extra steps. I'll try to open a draft PR in the coming days. Maybe that allows for easier hands-on discussions.

[ncoder-1]

Thanks!

[lieser]

I think this is resolved now? If not please reopen.