p11_key: do not assume a key handle relate to a previously loaded key

[etienne-lms]

Do not assume the PKCS#11 handle value stored in libp11 key cache still relates to the very same object previously loaded. Maybe the client application has modified the PKCS#11 token content using an interface that does not go through the OpenSSL pkcs11 engine. This change addresses the issue by reloading the object attributes from the token when the PKCS#11 handle value is present in the engine cache.

[mtrojnar]

The purpose of caching is precisely to reuse stored values. If I understand this PR correctly, it effectively disables caching because keys may have been modified by other means. The correct solution for #541 appears to be reloading the engine after adding, removing, or modifying keys. Keep in mind that closing and reopening the engine does not invalidate any EVP_PKEY objects acquired from the previous instance, as they retain a reference to that earlier engine.

[etienne-lms]

Thanks @mtrojnar for your feedback. Indeed, after a sequence like eng = ENGINE_by_id("pkcs11"); ENGINE_free(eng); ENGINE_remove(eng); the engine loaded with ENGINE_by_id("pkcs11") no more reproduces the issue related to pkcs11 engine caching object handle.

I'll close this P-R.

[sbrandt-aixtrusion]

When I call ENGINE_remove, I can't reload the engine with ENGINE_by_id.

When I call ENGINE_finish and ENGINE_free, and then ENGINE_by_id, I get the same public key, so, engine was not unloaded.

With finish, remove and free:

pkcs11-problem 
destroyPkcs11Object: trying to delete 1 objects of label testkey, class 2
destroyPkcs11Object: trying to delete 1 objects of label testkey, class 3
Private and public key 'testkey' have been deleted by name.
Private and public key 'testkey' have been generated.
Trying to check key validity by signing and verifying.
Public key:
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEC6M6psEuLIdpxSZ4uR63WPndeqbzwBFx
RnvsGGYrXRGGvVLftxdpz5Bl03BcTXJ1oOXhwxuTmuuMzsii+mbMp0mOAyLnsFR9
gcw4/smW9Sk7VKeiXwn6wtTnFBUx/Bgq
-----END PUBLIC KEY-----

Signature created
Signature verified
First PKCS11 key is valid, and has been freed.
Reloading engine.
calling ENGINE_finish(0xaaab1675c710)
calling ENGINE_remove(0xaaab1675c710)
calling ENGINE_free(0xaaab1675c710)
Info: freed 'pkcs11' engine at 0xaaab1675c710
Caught exception: EVP_ENGINE_by_id(pkcs11) failed: - SSL error:1300006D:engine routines::init failed
- SSL error:13000074:engine routines::no such engine

Just free and remove (order changed):

...
Reloading engine.
calling ENGINE_free(0xaaaaf37c7710)
calling ENGINE_remove(0xaaaaf37c7710)
Info: freed 'pkcs11' engine at 0xaaaaf37c7710
Caught exception: EVP_ENGINE_by_id(pkcs11) failed: - SSL error:1300006D:engine routines::init failed
- SSL error:13000074:engine routines::no such engine

With finish and free, without remove:

Reloading engine.
calling ENGINE_finish(0xaaaafef42710)
calling ENGINE_free(0xaaaafef42710)
Info: freed 'pkcs11' engine at 0xaaaafef42710
destroyPkcs11Object: trying to delete 1 objects by keyid, class 2
destroyPkcs11Object: trying to delete 1 objects by keyid, class 3
Private and public key 'testkey' have been deleted by id (again).
Private and public key 'testkey' have been generated (again).
Trying to check key validity by signing and verifying (again).
Public key:
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEzVdaE85BmSK0TsQyjNociB+VbfiV9zA6
uo2eVd8CF15GR8Ce/uaVuqNpsIXmhbKUYlpp1CJV8iX17ZPnjmNhs90Oy3jWN3YM
5Ds5H/olnu9CdeuoV+OowKLavWZ9NmoR
-----END PUBLIC KEY-----

Signature created
Verification failed

What am I doing differently?

[sbrandt-aixtrusion]

Simplification:

    ENGINE_load_builtin_engines();
    int ret;
    {
        auto eng = ENGINE_by_id("pkcs11");
        printf("ENGINE_by_id('pkcs11') -> %p\n", (void*)eng);
        ret = ENGINE_init(eng);
        printf("ENGINE_init(%p) -> %d\n", (void*)eng, ret);

        ret = ENGINE_free(eng);
        printf("ENGINE_free(%p) -> %d\n", (void*)eng, ret);
        ret = ENGINE_remove(eng);
        printf("ENGINE_remove(%p) -> %d\n", (void*)eng, ret);
    }
    {
        auto eng = ENGINE_by_id("pkcs11");
        printf("ENGINE_by_id('pkcs11') -> %p\n", (void*)eng);
    }

results in

ENGINE_by_id('pkcs11') -> 0xaaab0d029aa0
ENGINE_init(0xaaab0d029aa0) -> 1
ENGINE_free(0xaaab0d029aa0) -> 1
ENGINE_remove(0xaaab0d029aa0) -> 1
ENGINE_by_id('pkcs11') -> (nil)

which means the engine cannot be loaded again.

If I remove the call to ENGINE_init, I can reload it after remove:

ENGINE_by_id('pkcs11') -> 0xaaab0c0c2aa0
ENGINE_free(0xaaab0c0c2aa0) -> 1
ENGINE_remove(0xaaab0c0c2aa0) -> 1
ENGINE_by_id('pkcs11') -> 0xaaab0c0c2aa0

but that does not really help me or anyone.

[sbrandt-aixtrusion]

Okay, it does work if ENGINE_finish() is also called .. but just in the simple version for now.

[sbrandt-aixtrusion]

But as soon as I actually try to get a key, the engine can't be reloaded. So, again, the order

• ENGINE_by_id("pkcs11")
• ENGINE_init, ctrl_cmd
• ENGINE_load_private_key
• ENGINE_finish
• ENGINE_free
• ENGINE_remove

results in a system where the next call for ENGINE_by_id("pkcs11") no longer works.

ENGINE_by_id('pkcs11') -> 0x55c386269b20
ENGINE_init(0x55c386269b20) -> 1
ENGINE_ctrl_cmd(0x55c386269b20, MODULE_PATH) -> 1
ENGINE_ctrl_cmd_string(0x55c386269b20, PIN) -> 1
Found uninitialized token
Found uninitialized token
ENGINE_load_private_key() -> 0x55c38627d730
EVP_PKEY_free(0x55c38627d730)
ENGINE_finish(0x55c386269b20) -> 1
ENGINE_free(0x55c386269b20) -> 1
ENGINE_remove(0x55c386269b20) -> 1
ENGINE_by_id('pkcs11') -> (nil)

// calling twice.
Crypto::simpleEngineTest();
Crypto::simpleEngineTest();

void Crypto::simpleEngineTest()
{
    int ret;
    ENGINE_load_builtin_engines();
    auto eng = ENGINE_by_id("pkcs11");
    printf("ENGINE_by_id('pkcs11') -> %p\n", (void*)eng);
    ret = ENGINE_init(eng);
    printf("ENGINE_init(%p) -> %d\n", (void*)eng, ret);
    #if defined(__x86_64__)
    ret = ENGINE_ctrl_cmd(eng, "MODULE_PATH", 0, (void *)"/usr/lib/softhsm/libsofthsm2.so", NULL, 1);
    #else
    ret = ENGINE_ctrl_cmd(eng, "MODULE_PATH", 0, (void *)"/usr/lib/libckteec.so.0", NULL, 1);
    #endif
    printf("ENGINE_ctrl_cmd(%p, MODULE_PATH) -> %d\n", (void*)eng, ret);
    ret = ENGINE_ctrl_cmd_string(eng, "PIN", "12345", 0);
    printf("ENGINE_ctrl_cmd_string(%p, PIN) -> %d\n", (void*)eng, ret);

    auto* private_key = ENGINE_load_private_key(eng, "pkcs11:object=testkey", NULL, NULL);
    printf("ENGINE_load_private_key() -> %p\n", (void*)private_key);
    EVP_PKEY_free(private_key);
    printf("EVP_PKEY_free(%p)\n", (void*)private_key);

    ret = ENGINE_finish(eng);
    printf("ENGINE_finish(%p) -> %d\n", (void*)eng, ret);
    ret = ENGINE_free(eng);
    printf("ENGINE_free(%p) -> %d\n", (void*)eng, ret);
    ret = ENGINE_remove(eng);
    printf("ENGINE_remove(%p) -> %d\n", (void*)eng, ret);
}

[mtrojnar]

Can you please open an issue for the second ENGINE_by_id('pkcs11') failing? This is not something that could possibly be addressed by this PR.

[sbrandt-aixtrusion]

See #556