Question: How to correctly handle a Session

[EliseChouleur]

Hi !
It's more a best practice question than an issue.
I have historical code which I'm updating from rust-pkcs11 to rust-cryptoki.
The usage of cryptoki is to extract certificates and sign some data but also implements the rustls sign capacity :

impl Signer for PKCS11RSASigner {
    fn sign(&self, message: &[u8]) -> Result<Vec<u8>, TLSError> {
        let mut h = Sha256::default();
        let to_sign2 = h.digest(message).to_bytes();

        let mut final_to_sign = vec![0x30, 0x31, 0x30, 0x0D, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20];
        final_to_sign.extend_from_slice(to_sign2.as_slice());
        
        sign_with_key(final_to_sign.clone(), AUTHENTICATION_KEY_ID).map_err(|err| {
            error!("rustls sign_with_key error : {:?}", err);
            TLSError::NoCertificatesPresented
        })
    }
}

What is be the best practice to handle a session here ?
Create a new one just inside this implementation while there is already one active in the function calling rustls ?
Thanks a lot for your feedback 🙏

[wiktor-k]

Create a new one just inside this implementation while there is already one active in the function calling rustls ?

If there's one active this may be problematic 🤔 can you have a reference to the Session in your struct?

CCing @baloo since they've also designed a RustCrypto wrapper around this.

[baloo]

My unfinished prototype can be found here: #192

but essentially, my approach was for the signer to carry a SessionLike object:

#[derive(signature::Signer)]
pub struct Signer<C: SignAlgorithm, S: SessionLike> {
    session: S,
    private_key: ObjectHandle,
    verifying_key: VerifyingKey<C>,
}

SessionLike defining the "entrypoints" for the signer to tap into cryptoki:

pub trait SessionLike {
    fn create_object(&self, template: &[Attribute]) -> Result<ObjectHandle>;
    fn find_objects(&self, template: &[Attribute]) -> Result<Vec<ObjectHandle>>;
    fn get_attributes(
        &self,
        object: ObjectHandle,
        attributes: &[AttributeType],
    ) -> Result<Vec<Attribute>>;
    fn update_attributes(&self, object: ObjectHandle, template: &[Attribute]) -> Result<()>;
    fn sign(&self, mechanism: &Mechanism, key: ObjectHandle, data: &[u8]) -> Result<Vec<u8>>;
    fn generate_random_slice(&self, random_data: &mut [u8]) -> Result<()>;
    fn wrap_key(
        &self,
        mechanism: &Mechanism,
        wrapping_key: ObjectHandle,
        key: ObjectHandle,
    ) -> Result<Vec<u8>>;
    fn login(&self, user_type: UserType, pin: Option<&AuthPin>) -> Result<()>;
    fn logout(&self) -> Result<()>;
}

SessionLike is then defined for &'s Session and Session. If you then wanted to define the trait for an Arc<Session> or an Arc<Mutex<Session>> I think that's possible.

I don't think the thread-safety garantees or uniqueness of the session are defined at the spec level. I know the two implementation I worked with in the past had varying behavior. One of them you had to opt-in to have thread-safety.

[EliseChouleur]

Create a new one just inside this implementation while there is already one active in the function calling rustls ?

If there's one active this may be problematic 🤔 can you have a reference to the Session in your struct?

CCing @baloo since they've also designed a RustCrypto wrapper around this.

On my case, the Signer trait in defined insite rustls crate: https://github.com/rustls/rustls/blob/20023ee91fbd0c27503b0693825bf7d23cabdbe2/rustls/src/crypto/signer.rs#L75
So I can't change it.

[wiktor-k]

On my case, the Signer trait in defined insite rustls crate: https://github.com/rustls/rustls/blob/20023ee91fbd0c27503b0693825bf7d23cabdbe2/rustls/src/crypto/signer.rs#L75 So I can't change it.

Ack. But PKCS11RSASigner is in your crate? If so then the reference to the Session can live there... 🤔

[EliseChouleur]

I get this error, but I'll check further

error[E0277]: `S` cannot be shared between threads safely
    |
231 | impl<S> Signer for PKCS11RSASigner<S>
    |                    ^^^^^^^^^^^^^^^^^^ `S` cannot be shared between threads safely
    |
note: required because it appears within the type `PKCS11RSASigner<S>`
    |
220 | struct PKCS11RSASigner<S: SessionLike> {
    |        ^^^^^^^^^^^^^^^
note: required by a bound in `Signer`
   --> .cargo/git/checkouts/rustls-4595b772ffea9fe4/f0c30ed/rustls/src/sign.rs:24:27
    |
24  | pub trait Signer : Send + Sync {
    |                           ^^^^ required by this bound in `Signer`

[wiktor-k]

Hmm... sounds like it may need wrapping in an Arc<Mutex<Session>>... your code is not open-source so we can test directly there? 🤔

[EliseChouleur]

Unfortunately it's not, I could try to make a small example but it will take me some time.

I tried to put Session in an Arc Mutex some time ago but I couldn't manage as Session isn't clonable

[wiktor-k]

Unfortunately it's not, I could try to make a small example but it will take me some time.

Completely understandable 👍

I tried to put Session in an Arc Mutex some time ago but I couldn't manage as Session isn't clonable

Hmm... As far as I know neither Arc nor Mutex requires the thing to be cloneable? 🤔 (Arc has a clone function but it clones the Arc object not the inner one).

[hug-dev]

I checked in Parsec and we open and close sessions just to perform one single operation. I think it's fine to open sessions in parallel but I think that the Login and Logout status of all sessions are affected at the same time