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

Review the pkcs11 crate dependency #731

Closed
6 tasks
ximon18 opened this issue Nov 22, 2021 · 6 comments
Closed
6 tasks

Review the pkcs11 crate dependency #731

ximon18 opened this issue Nov 22, 2021 · 6 comments
Labels
hsm Relates to adding HSM support to Krill

Comments

@ximon18
Copy link
Member

ximon18 commented Nov 22, 2021

User @ionut-arm commented on PR #727 that the cryptoki Rust crate was created in part to address "the (security) problems in the [pkcs11] crate".

We should review the decision to use the pkcs11 crate for interfacing with PKCS#11 libraries in Krill. For example:

  • Are there known security and/or memory safety issues with the pkcs11 crate?
  • Does Krill use the crate in such a way that it is affected by any such issues?
  • Is the crate still actively maintained?
  • Should we switch to away from the pkcs11 crate?
  • Should we switch to the cryptoki crate?
  • Are there other viable alternatives?
@ximon18 ximon18 added the hsm Relates to adding HSM support to Krill label Nov 22, 2021
@ximon18 ximon18 added this to Probably Required in HSM support for keys Nov 22, 2021
@ximon18
Copy link
Member Author

ximon18 commented Jan 4, 2022

See also: #752

@ximon18
Copy link
Member Author

ximon18 commented Jan 4, 2022

Does Krill use the crate in such a way that it is affected by any such issues?

Unknown, but we have since encountered issue #752 with the pkcs11 crate on ARMv7.

Is the crate still actively maintained?

Comments here and here seem to indicate that the crate is NOT actively maintained.

Should we switch to the cryptoki crate?

Possibly, it seems to not suffer from the #752 issue at least, but it is poorly documented, suffers from some crashes (see here and here (to be fair so does the pkcs11 crate, e.g. see here)), seems to be in need of more testing (see here) and may not have the features Krill needs as the Parsec projects "initial target was to abstract everything that we need in Parsec", and the interface is quite different to that of the pkcs11 crate (modifying the simple keyls tool to switch crates was quite a substantial change).

We would need to create a proof-of-concept modification of Krill to see if the Krill test suite passes with the cryptoki crate.

If that looks good we could address the lacking docs and tests through contributions to the crate if resources and time permit.

@hug-dev
Copy link

hug-dev commented Jan 4, 2022

If that looks good we could address the lacking docs and tests through contributions to the crate if resources and time permit.

Hello, happy new year and thank you for considering the cryptoki crate! We would definitely welcome any contributions to our crate 🆗

I would like to add a few biased comments to what you posted above 😛:

  • we need to confirm but pkcs11.open_session_no_callback against Luna Network HSM crashed with SIGSEGV parallaxsecond/rust-cryptoki#72 seems to be fixed on main
  • we had and merged a lot of contributions since 0.2.0, some improving the way the documentation is represented
  • we don't have too many tests in the crate (still a good base of integration tests) but note that we are using the crate in parsec and by testing Parsec using PKCS11 we are also testing the crate (for what we use in Parsec). Parsec has a huge amount of tests checking various crypto algorithms and key management operations.

There are definitely still things to do and @vkkoskie really nicely listed the areas of improvement, see here.

@ximon18
Copy link
Member Author

ximon18 commented Jan 4, 2022

Thanks @hug-dev for the useful input!

@ximon18
Copy link
Member Author

ximon18 commented Jan 5, 2022

A quick PoC seems to show we can use the cryptoki crate, see PR #754.

@ximon18
Copy link
Member Author

ximon18 commented Feb 14, 2022

Closing as we moved to the cryptoki crate in the dev branch to resolve #752.

@ximon18 ximon18 closed this as completed Feb 14, 2022
@ximon18 ximon18 moved this from Probably Required to Done in HSM support for keys Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hsm Relates to adding HSM support to Krill
Projects
Development

No branches or pull requests

2 participants