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

WIP: supports mutable IV in GcmParams, close #225

Open zkonge opened this issue 1 year ago • 1 comments

so that some PKCS11 implementation (like AWS CloudHSM) could write random IV into it.

Fixes https://github.com/parallaxsecond/rust-cryptoki/issues/225

zkonge avatar Sep 20 '24 17:09 zkonge

Although the trait constraint for Mechanism is enough for the alias rule:

Mechanism: !Clone+!Send

I open another branch https://github.com/parallaxsecond/rust-cryptoki/commit/43c8863750065224f56b29a3ab2a16b51272bdbe that make all operations use the owned Mechanism.

It's not necessary, but I think it can give us (and the compiler) more restrictions on when we can get things wrong.

Any ideas about that? And is there anyone prefer passing the mutable reference instead of owned data?

zkonge avatar Sep 23 '24 14:09 zkonge

Any ideas about that? And is there anyone prefer passing the mutable reference instead of owned data?

Wait, I thought mutable ref would allow the PKCS11 implementation to set these fields that are to be read by the caller, which would be impossible when using owned data? (unless the owned object is being returned from the function too).

Btw, I think we can resume work on this given recent changes in main.

Sorry for the delays @zkonge :bow:

wiktor-k avatar Oct 23 '24 07:10 wiktor-k

Hello, I'm curious if there's any update on this one? I know this PR is out of date with the current release, 0.9.0, but I was able to make the same changes to an updated branch and I have success with AES GCM on AWS CloudHSM. I've verified that the mutable reference allows the IV to be provided back to the caller. This is with the V5 cloudhsm pkcs11 client.

The changes don't seem to affect anything with SoftHSM tests, and my Luna SA works with it as well.

fiercelybearded avatar Feb 25 '25 22:02 fiercelybearded

Hmm... good question. We regularly have "contributor missing-in-action" situations here but it looks like all checks passed and the PR is merge-able so we could take it as is.

@hug-dev what do you think about merging it as is?

wiktor-k avatar Feb 26 '25 08:02 wiktor-k

I think it's fine since we agreed in https://github.com/parallaxsecond/rust-cryptoki/issues/225 that the approach of passing &mut Mechanism should be done everywhere.

Although I would have liked a bit more comments on the reason of the mutability I think this is fine! We should complete this at some point moving all operations to &mut Mechanism as described in the issue.

Better to merge this as is if more people can benefit from it :)

hug-dev avatar Feb 26 '25 08:02 hug-dev

Yep, totally agreed on all points. Thanks for your time, Hugues :bow:

wiktor-k avatar Feb 26 '25 09:02 wiktor-k