Unmaintained Dependency paste (RUSTSEC-2024-0436)

[Nicoretti]

Hi Cryptoki maintainers,

Thanks for providing this library!

I noticed that cryptoki uses the paste crate (https://github.com/dtolnay/paste), and I wanted to bring to your attention RUSTSEC-2024-0436, which notes that paste is currently unmaintained.

I've been thinking about possible solutions, and given how small the paste crate is, vendoring it directly into cryptoki seems like a reasonable and straightforward option. This would remove the external dependency and give you more control over its maintenance and security going forward. Plus, it wouldn't negatively impact the current security posture.

To that end, I was wondering if you'd be open to a PR where I vendor the paste code directly into cryptoki?

Thanks for your time and all your work on cryptoki!

best
Nico

[wiktor-k]

Others had the same problems and it seems there are quite a few forks already, including pastey. I don't have any suggestions here, on one hand inlining is annoying, on another, if that's just a small piece of code... 🤷‍♂

[Nicoretti]

We are talking 4 files, overall ~800-900 Loc (including doc comments).
Files could be merged into a single one though from my pov.

[hug-dev]

Thanks for bringing this up! If there are already forks available I would also prefer using them directly instead of having to maintain the crate ourselves 😅 Do you see any advantages of having the crate in our repo?

Would definitely be open for a PR trying out pastey or another suitable replacement!

[Nicoretti]

@wiktor-k , @hug-dev
Thanks for the timely replies. I understand the ups and downs from a maintainer's perspective (In the end it is your decision). I was also taken aback that the vendoring is more significant than expected due to the large test suite. I have created a PR (#280) which can serve as a basis for discussion. If you decide against vendoring, feel free to close/reject it. If you use a maintained alternative, that would also be a good solution from a user's perspective 👍.

[Nicoretti]

@hug-dev

Thanks for bringing this up! If there are already forks available I would also prefer using them directly instead of having to maintain the crate ourselves 😅

I can fully understand that.

Do you see any advantages of having the crate in our repo?

Compared to having another well maintained dependency? -> No

If there are no alternatives available, it does not make the status quo worse as you likely don't touch it,
but if you need to fix or adjust something, you can.

Should I close PR #280 ?

[hug-dev]

Then yes I think we can close the PR and plan to switch to an alternative! Hope it was not too much work and thank you for testing it anyway!

[Jakuje]

Then yes I think we can close the PR and plan to switch to an alternative! Hope it was not too much work and thank you for testing it anyway!

Changing to pastey should be really just a change in the Cargo.toml:

[dependencies]
- paste = "1"
+ paste = { package = "pastey", version = "*" }

as described in https://crates.io/crates/pastey. But I am not convinced the new fork is less of a security issue than old "unmaintained" crate.

In kryoptic we dropped this dependency as we used it only in one place. Here I can see it is only used in one place too, but I can not see if there is a way to go without this on the first sight.

[TroyKomodo]

Then yes I think we can close the PR and plan to switch to an alternative! Hope it was not too much work and thank you for testing it anyway!

Changing to pastey should be really just a change in the Cargo.toml:

[dependencies]

• paste = "1"

• paste = { package = "pastey", version = "*" }
  as described in https://crates.io/crates/pastey. But I am not convinced the new fork is less of a security issue than old "unmaintained" crate.

In kryoptic we dropped this dependency as we used it only in one place. Here I can see it is only used in one place too, but I can not see if there is a way to go without this on the first sight.

I think there is a serious problem in the rust ecosystem that people fork unmaintained crates and then add some new features to it and dont actually maintain it. This is a prime setup for a supply chain attack.

1. find a crate which hasnt been updated in a long time with significant ecosystem usage.
2. fork it, push a few patches
3. file a CVE saying its unmaintained
4. watch as the ecosystem switches over to a new crate
5. push an exploit to the fork boom

I think this is generally an issue within the rust ecosystem since people are so eager to switch to a new crate to avoid the CVE notices in CI and it enviably creates security holes since these crates we switch to have not been vetted.

Here is a recent example from serde_yml (fork of serde_yaml) https://x.com/davidtolnay/status/1883906113428676938

pasty is much in the same boat as serde_yml being that it was once a davidtolnay crate that gets forked and the new dev adds a few minor lack luster changes with some AI and then publishes a new crate.

I would advise anyone to add this to your deny.toml

[bans]
deny = [
  # AI Fork of serde_yaml
  "serde_yml",
  # AI Fork of paste
  "pastey",
]

[wiktor-k]

I think there is a serious problem in the rust ecosystem that people fork unmaintained crates and then add some new features to it and dont actually maintain it. This is a prime setup for a supply chain attack.

I agree that this is a highly suboptimal situation but the root cause, at least from my point of view, was marking paste as unmaintained by the author who considered it "finished". There were others who volunteered to do maintenance but that offer has not been considered. (Tangentially but this makes my Go-loving friends snicker and whisper "extended stdlib" 😶‍🌫️)

file a CVE saying its unmaintained

The author said the crate is unmaintained. Of course there are crates that have a maintainer missing-in-action and your attack scenario is plausible.

I think this is generally an issue within the rust ecosystem since people are so eager to switch to a new crate to avoid the CVE notices in CI and it enviably creates security holes since these crates we switch to have not been vetted.

It's an "issue" within the rust ecosystem because it's so easy to check for this in Rust. Cargo-deny is incredibly straightforward to setup and the centralised advisory db is easy to update.

pasty is much in the same boat as serde_yml being that it was once a davidtolnay crate that gets forked and the new dev adds a few minor lack luster changes with some AI and then publishes a new crate.

Personally I don't have anything against "lack luster changes". This is something different than being malicious.

I view this as a systemic problem that should be taken care of by a Rust governing entity. Apache Foundation was a safe haven for old projects in other ecosystems back in the day, maybe Rust needs something like this too?

I guess the way forward here is to look at the macro invocation at hand and see if we can simplify that. Sadly, since this is (at least for me) a side project, this may take some time :-/

[hug-dev]

We can track this which would allow us to remove paste and change the code as following when stable:

diff --git a/cryptoki/src/context/general_purpose.rs b/cryptoki/src/context/general_purpose.rs
index be7ead9..8c8b3bd 100644
--- a/cryptoki/src/context/general_purpose.rs
+++ b/cryptoki/src/context/general_purpose.rs
@@ -35,11 +35,10 @@ pub(super) fn get_library_info(ctx: &Pkcs11) -> Result<Info> {

 macro_rules! check_fn {
     ($pkcs11:expr, $func_name:ident) => {{
-        let func = paste! { $pkcs11
+        let func = $pkcs11
             .impl_
                 .get_function_list()
-                .[<C_ $func_name>]
-        };
+                .${concat(C_, $func_name)};
         func.is_some()
     }};
 }

got the idea from xiph/rav1e#3430

[Jakuje]

Can you open a PR with the changes?