Skip to content
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

Vendored mechanisms #105

Open
Zorvalt opened this issue Oct 25, 2022 · 2 comments
Open

Vendored mechanisms #105

Zorvalt opened this issue Oct 25, 2022 · 2 comments

Comments

@Zorvalt
Copy link

Zorvalt commented Oct 25, 2022

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!

@ionut-arm
Copy link
Member

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.

@juniperparsnips
Copy link

juniperparsnips commented Feb 2, 2024

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 Treat CK*_VENDOR_DEFINED correctly #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants