-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Treat CK*_VENDOR_DEFINED correctly #54
Comments
Hi 👋 ! Thanks for the issue! You are perfectly right, we do not support those For your proposed changes:
Agree!
I think that even for 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
Agree! |
I was actually proposing that it be a third member of
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.
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. |
That makes sense then with adding a new variant to
I was thinking of doing the checks dynamically (and then having a real function and not a |
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 Not sure if it should be done with this crate or if another crate (rust-crypto-3 or something) should be forked from here. |
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. |
All enum types with an associated VENDOR_DEFINED constant describe that value in roughly the same way in the standard. Using key types as an example: "Key types CKK_VENDOR_DEFINED and above are permanently reserved for token vendors. For interoperability, vendors should register their key types through the PKCS process."
The intent is that vendors who aren't concerned with interoperability or standardization are permitted to safely use that entire range of values. This crate largely omits these values, but not consistently. The two places where it does use them, they aren't treated properly:
CKM_VENDOR_DEFINED
is recognized for the Display trait, but only as a single value. All other in-range values are treated as errors.CKR_VENDOR_DEFINED
is reported as an error return value. All other in-range values are converted toGeneralError
.In both cases, the underlying value is made visible in printed strings but lost within the type system and to the user.
Proposed changes:
VENDOR_DEFINED <= x <= ULONG::MAX
should be propagated back up to the user to respond to. ForRv
, this would likely be a sibling type toOk
andError(RvError)
. For all others it would be an additional discriminant containing the non-standard value (i.e.,VendorDefined(Ulong)
)"CKM_VENDOR_DEFINED(0x{%08x})"
The text was updated successfully, but these errors were encountered: