latchset / kryoptic

a pkcs#11 software token written in Rust
GNU General Public License v3.0
10 stars 4 forks source link

Implement support for CKA_ALLOWED_MECHANISMS #73

Closed simo5 closed 2 months ago

simo5 commented 2 months ago

Filters at operation init time and returns an error if the key indicated that it has a list of allowed mechanisms and the mechanism presented is not in the list.

A missing attribute is handled as if any mechanism is allowed.

Fixes #71

Jakuje commented 2 months ago

Should also the WrapKey, UnwrapKey, DeriveKey operations be checked for allowed mechanisms?

For the Message* functions, this will be needed too once they will be implemented.

simo5 commented 2 months ago

Should also the WrapKey, UnwrapKey, DeriveKey operations be checked for allowed mechanisms?

For the Message* functions, this will be needed too once they will be implemented.

No the wrapping and derive are controlled already by CKA_WRAP, CKA_UNWRAP, etc... The spec mentions only operations done with the key, not to the key.

And yes I expect once we implement message functions we'll basically just write some common wrapper ...

Jakuje commented 2 months ago

Should also the WrapKey, UnwrapKey, DeriveKey operations be checked for allowed mechanisms? For the Message* functions, this will be needed too once they will be implemented.

No the wrapping and derive are controlled already by CKA_WRAP, CKA_UNWRAP, etc... The spec mentions only operations done with the key, not to the key.

But you do the unwrapping of the key with another private key in case of RSA and specific RSA mechanism that I think should be checked (wrapping is done with public key so it is not relevant).

The Derive operation is also an operation on a private key with specific mechanism (regardless of the output key).

simo5 commented 2 months ago

Should also the WrapKey, UnwrapKey, DeriveKey operations be checked for allowed mechanisms? For the Message* functions, this will be needed too once they will be implemented.

No the wrapping and derive are controlled already by CKA_WRAP, CKA_UNWRAP, etc... The spec mentions only operations done with the key, not to the key.

But you do the unwrapping of the key with another private key in case of RSA and specific RSA mechanism that I think should be checked (wrapping is done with public key so it is not relevant).

The Derive operation is also an operation on a private key with specific mechanism (regardless of the output key).

You are right, added checks to (un)wrap and derive functions too

simo5 commented 2 months ago

lgtm. Would be great to extend the existing tests of other operations, but we can live without that for now.

From functionality pov given the check is in a function that is called identically from all callers I think current "unit" testing is sufficient. Of course functional/integration testing would be nice to be extended to a lot more of these, but perhaps later, once we know what this is commonly used for, doing full matrix testing would be too much.