Treat CK*_VENDOR_DEFINED correctly about rust-cryptoki HOT 5 OPEN

Hi 👋 !

Thanks for the issue! You are perfectly right, we do not support those _VENDOR_DEFINED values correctly at all at the moment. I think it was because we overlooked the part where it says that the vendor defined values are a range of value, as opposed as a single one.

For your proposed changes:

• All enums documented to support vendor extensions should at least recognize them properly, even if full support isn't provided.

Agree!

• Where defined, all values x that satisfy VENDOR_DEFINED <= x <= ULONG::MAX should be propagated back up to the user to respond to. For Rv, this would likely be a sibling type to Ok and Error(RvError). For all others it would be an additional discriminant containing the non-standard value (i.e., VendorDefined(Ulong))

I think that even for CKR_VENDOR_DEFINED this could be put in a RvError::VendorDefined(Ulong)? Unless the CKR_VENDOR_DEFINED value could represent another success state than CKR_OK?

This is all fine when converting from C types to Rust, but when doing the reverse for some of the types when the variants are defined as const, we might have to add a new method vendor_defined(val: Ulong) to allow people to create their vendor defined values. We should also check that it belongs to the allowed range.
I think using Ulong is fine because the specification typedef the C types to CK_ULONG.

• All stringification sites should include the value associated with vendor-defined values (e,g. "CKM_VENDOR_DEFINED(0x{%08x})"

Agree!

from rust-cryptoki.

I think that even for CKR_VENDOR_DEFINED this could be put in a RvError::VendorDefined(Ulong)? Unless the CKR_VENDOR_DEFINED value could represent another success state than CKR_OK?

I was actually proposing that it be a third member of Rv. The interpretation of the contained value is neither a success nor a failure from the perspective of the standard. Its interpretation is entirely for the vendor to decide. This has mild ergonomics impacts by adding a third case to a typically binary concept. One possible solution to this is to feature gate vendor extension values and make them non-default. This has at least three advantages off the top of my head:

• It doesn't breaking Rv pattern matching in existing user code.
• It would permit treating vendor-range values as errors (correctly) when client code is assuming only definitions from the standard.
• It would allow the Rv enum to avoid being marked non-exhaustive (which may just be my personal taste)

This is all fine when converting from C types to Rust, but when doing the reverse for some of the types when the variants are defined as const, we might have to add a new method vendor_defined(val: Ulong) to allow people to create their vendor defined values. We should also check that it belongs to the allowed range.

I did actually make an attempt at this and found this to be very difficult in practice. Rust doesn't currently provide an error response mechanism for const and compile-time contexts. There is a static_assertions crate that provides some interesting ways around this, but all involve exporting macros as part of the API, which found a bit clunky. I'd recommend taking a look at it, though, to see if you think it's worth it. And again, these can be behind the same opt-in feature gate.

I think using Ulong is fine because the specification typedef the C types to CK_ULONG.

Correct. In all cases where a *_VENDOR_DEFINED variant is mentioned in the standard, it's a CK_ULONG type. All other vendor-defined entities are constructed by the vendor. E.g., a vendor-defined object would be composed out of a combination of pre-defined attributes and those in the CKA_VENDOR_DEFINED range.

from rust-cryptoki.

The interpretation of the contained value is neither a success nor a failure from the perspective of the standard. Its interpretation is entirely for the vendor to decide.

That makes sense then with adding a new variant to Rv catering for this. The only problem then would be for the into_result function which transforms a Rv into a Rust Result. Might be easier to put aside the use case of multiple successful return values and add the VendorDefined in RvError. There are even some cases with existing error values which do not represent real errors but just indications. For example:

Note that the error codes CKR_ATTRIBUTE_SENSITIVE, CKR_ATTRIBUTE_TYPE_INVALID, and CKR_BUFFER_TOO_SMALL do not denote true errors for C_GeAttributeValue.

We should also check that it belongs to the allowed range.

I was thinking of doing the checks dynamically (and then having a real function and not a const)

from rust-cryptoki.

As an aside, a possible future item that should be discussed is whether or not this crate should be updated to support PKCS#11 v3.0 (or 3.1, whose release is supposedly imminent). Many vendors had to implement quite a few common crypto algorithms as VENDOR_DEFINED because they were missing in v2.4 but were required (e.g. supporting FIPS 140).

Not sure if it should be done with this crate or if another crate (rust-crypto-3 or something) should be forked from here.

from rust-cryptoki.

Re: const. I was imagining the case in which a vendor/user of the crate wanted to define their own enum values extending standard ones (i.e., types defined here). In order to do that the range check would have to happen at declaration/compile time, which requires const-friendly error support that's not in the language today except through unattractive hacks. That's all a long way of saying I agree. :)

Re: v3.0. I think feature gates are the ideal way to do that, especially if 2.4 and 3.0 (which I haven't looked at closely) overlap a great deal.

from rust-cryptoki.