Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 31 additions & 38 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ clap = { version = "4.5", features = [
"unstable-styles",
] }
clap_complete = { version = "4.5", features = ["unstable-dynamic"] }
cryptoki = "0.10.0"
cryptoki = "0.12.0"
csv = "1.1"
darling = "0.21"
doku = "0.21"
Expand Down Expand Up @@ -241,7 +241,7 @@ zeroize = "1.5"
# dlopen is directly under libc, so we need to modify the extern block to not link libdl
# TODO: remove once fix is upstreamed to libloading
[patch.crates-io]
libloading = { git = "https://github.com/Bravo555/rust_libloading.git", branch = "0.8.8" }
libloading = { git = "https://github.com/Bravo555/rust_libloading.git", branch = "0.8.9" }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. This is to remove dl linker flag under musl: nagisa/rust_libloading@master...Bravo555:rust_libloading:0.8.9

What's the plan to get rid of this patch?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once I manage to reproduce the error message that the change is fixing, so I can include it in the PR, I will create a PR to the libloading repository.

I was neglecting to create the PR because I don't really have a good idea of how our build process for musl works such that this error appears, so I can't reproduce the error without actually using our CI pipelline. For this reason, I'm not able to produce a minimal example of this problem showing that this change "fixes" it, or even really describe the problem in detail at all. All i know is that it works for us, but I'm not completely sure that this change is "correct" for the upstream repository, so I was worrying that I won't be able to provide enough context for the maintainer describing why the change is correct.

Nevertheless, I've been putting it off for too long so I will create PR including a best effort description and the error message. If maintainer asks for more details about building for musl, I'll ask @reubenmiller for help.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a bit more research it turns out that perhaps patching libloading was not necessary and we should make the changes in our build process instead: nagisa/rust_libloading#195 (comment)


[profile.release]
codegen-units = 1
Expand Down
19 changes: 5 additions & 14 deletions crates/extensions/tedge-p11-server/src/pkcs11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use asn1_rs::ToDer;
use camino::Utf8Path;
use camino::Utf8PathBuf;
use cryptoki::context::CInitializeArgs;
use cryptoki::context::CInitializeFlags;
use cryptoki::context::Pkcs11;
use cryptoki::error::Error;
use cryptoki::mechanism::Mechanism;
Expand Down Expand Up @@ -226,7 +227,7 @@ impl TedgeP11Service for Cryptoki {
impl Cryptoki {
pub fn new(config: CryptokiConfigDirect) -> anyhow::Result<Self> {
let pkcs11client = Self::load(&config.module_path)?;
pkcs11client.initialize(CInitializeArgs::OsThreads)?;
pkcs11client.initialize(CInitializeArgs::new(CInitializeFlags::OS_LOCKING_OK))?;

Ok(Self {
context: Arc::new(Mutex::new(pkcs11client)),
Expand All @@ -252,10 +253,11 @@ impl Cryptoki {
// the spec says "(C_Finalize) should be the last Cryptoki call made by an application", so call it on the old
// client before initializing new client
// https://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/errata01/os/pkcs11-base-v2.40-errata01-os-complete.html#_Toc441755803
old_client.finalize();
let _ = old_client.finalize();

// can return Error::AlreadyInitialized, but it shouldn't, only warn if it does anyway
if let Err(err) = context.initialize(CInitializeArgs::OsThreads) {
if let Err(err) = context.initialize(CInitializeArgs::new(CInitializeFlags::OS_LOCKING_OK))
{
warn!(?err, "Initializing cryptoki library failed");
}

Expand Down Expand Up @@ -513,17 +515,6 @@ impl CryptokiSession<'_> {
// ideally we'd simply get a cryptoki mechanism that corresponds to this sigscheme but it's not possible;
// instead we have to manually parse additional attributes to select a proper sigscheme; currently don't do it
// and just select the most common sigscheme for both types of keys

// NOTE: cryptoki has AttributeType::AllowedMechanisms, but when i use it in get_attributes() with opensc-pkcs11
// module it gets ignored (not present or supported) and with softhsm2 module it panics(seems to be an issue
// with cryptoki, but regardless):

// thread 'main' panicked at library/core/src/panicking.rs:218:5:
// unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
// note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
// thread caused non-unwinding panic. aborting.
// Aborted (core dumped)

let key = match keytype {
KeyType::EC => {
let sigscheme =
Expand Down
2 changes: 1 addition & 1 deletion crates/extensions/tedge-p11-server/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl SecretString {

impl From<SecretString> for AuthPin {
fn from(value: SecretString) -> Self {
AuthPin::new(value.0)
AuthPin::new(value.0.into())
}
}

Expand Down
1 change: 0 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ feature-depth = 1
ignore = [
{ id = "RUSTSEC-2024-0384", reason = "crate: instant. Used by backoff and requires refactoring to remove this dependency" },
{ id = "RUSTSEC-2025-0012", reason = "crate: backoff. Needs refactoring to remove dependency" },
{ id = "RUSTSEC-2024-0436", reason = "crate: paste. Used by cryptoki dependency" },
{ id = "RUSTSEC-2023-0071", reason = "crate: rsa. No patch available, not using affected API, added rules to clippy to forbid using these APIs" },
{ id = "RUSTSEC-2026-0009", reason = "crate: time. Patching requires updating MSRV, applied workaround in one usage, added rules to clippy to forbid using these APIs elsewhere" },

Expand Down