rust-cryptoki icon indicating copy to clipboard operation
rust-cryptoki copied to clipboard

Vendored mechanisms

Open Zorvalt opened this issue 2 years ago • 2 comments

Hello,

I am working with a PKCS11 interface requiring the use of vendored mechanisms. I did not find a way to do so with this crate and was wondering if this could be added. Beside this issue (#54) which focuses on errors, it seems to me the subject was not discussed, at least publicly.

Do you plan implementing this feature or would you accept a PR for it?

In case you are open to the idea, I already looked at the code and it seems to me this boils down to provide a mean to extend the Mechanism enum. Because the rust-cryptoki library cannot know in advance, I could not think of a simple way to ensure the library user constructs correct PKCS11 mechanisms.

I think a simple solution would be to offer an additional VendoredMechanism in the Mechanism enum which would expose the inners of what makes a CK_MECHANISM : a MechanismType and some arbitrary bytes as parameters.

In a working PoC, I implemented this:

// In src/mechanism/vendored.rs
pub struct VendoredMechanism {
    pub mech_type: MechanismType,
    pub params: Vec<CK_BYTE>,
}

// In src/mechanism/mod.rs
pub enum Mechanism {
    ...
    VendoredMechanism(vendored::VendoredMechanism),
}

impl Mechanism {
    /// Get the type of a mechanism
    pub fn mechanism_type(&self) -> MechanismType {
        ...
        Mechanism::VendoredMechanism(mech) => mech.mech_type,
    }
}

impl From<&Mechanism> for CK_MECHANISM {
    fn from(mech: &Mechanism) -> Self {
        let mechanism = mech.mechanism_type().into();
        match mech {
            Mechanism::VendoredMechanism(mech) => CK_MECHANISM {
                mechanism,
                pParameter: mech.params.as_slice() as *const _ as *mut c_void,
                ulParameterLen: mech.params.len() as u64,
            },
        }
    }
}

Which can be used as so:

pub const SOME_VENDORED_MECH: MechanismType = MechanismType {
    val: SOME_VENDORED_MECH_VALUE,
};

pub struct SomeVendoredMech {
    pub field1: u8,
    pub field2: u8,
}

impl From<&SomeVendoredMech> for VendoredMechanism {
    fn from(mech: &SomeVendoredMech) -> Self {
        VendoredMechanism {
            mech_type: SOME_VENDORED_MECH,
            params: vec![
                mech.field1,
                mech.field2,
            ],
        }
    }
}

I know this code is not that great, it's just to start the discussion and suggest a simple way to go. I look forward for your input!

Zorvalt avatar Oct 25 '22 11:10 Zorvalt

Hey! Thanks for raising this - yes, it's a hole in the API that we've not (yet) addressed. We are, however, happy to take PRs for it, so feel free to create one with your code, it looks pretty sensible to me.

One thing to note is that currently your vendored mechanism implementation only allows you to override our defined mechanism types without being able to specify other vendor types. The spec (section 3.5) says:

Mechanism types CKM_VENDOR_DEFINED and above are permanently reserved for token vendors.

I'd suggest making the fields of VendoredMechanism private, and adding a constructor that checks that mechanism type value.

ionut-arm avatar Oct 26 '22 13:10 ionut-arm

I've been working on an implementation for this and I think it would be worth discussing some design thoughts here before moving forward.

  • vendor defined mechanisms should be behind an optional feature
    • we won't be able to protect every possible use of the custom mechanisms, so this would serve as a way of "the user accepting responsibility" for the vendor defined portions
    • edit: I missed the discussion of putting vendor support behind a feature in #54, but agree with doing so)
  • Ideally there should be a way to bind a type of params to a mechanism type but I don't know of a good way to do this while still being able to include the vendor defined mechanism in the Mechanism enum. I was thinking about something like:
pub trait VendorMechanism {
  const TYPE: MechanismType;

  fn param_bytes(&self) -> &[u8];
}

...

enum Mechanism {
   ...
   VendorDefined(Box<dyn VendorMechanism>)
}

but the VendorMechanism wouldn't be object safe that wouldn't work. As an (admittedly worse) alternative we could change the mechanism type like so: fn mechanism_type(&self) -> MechanismType; but it's not ideal

  • Should we protect the user from defining different mechanism types with the same value
    • if we add vendor defined mechanisms as an optional feature, I'm fine with not doing the protection as it adds otherwise unnecessary overhead

juniperparsnips avatar Feb 02 '24 16:02 juniperparsnips