`RUSTSEC-2022-0093` security advisory

[divarvel]

RUSTSEC-2022-0093 has been issued on 2023-08-14, targeting a dependency of biscuit-rust.

tl;dr:

Upgrade biscuit-rust to v4.0.0 as soon as possible, and audit your use of biscuit creation functions (biscuit sealing and attenuation are not vulnerable). biscuit-rust itself does not present unsafe API use, but it does not prevent downstream code from providing bogus biscuit_auth::crypto::KeyPair values. Such values can only be created through safe APIs, but they can still be mutated into unsafe values. Idiomatic use of the biscuit library is safe, but it is still theorically possible to trigger unsafe use by manually mutating a safe KeyPair into an unsafe one.

Explanation

The advisory targets ed25519-dalek v1.x. The vulnerability is due to an unsafe API exposed by the library: signing operations took as input a public key and a private key that could be separately assembled, even though the public key is derived from the secret key. Providing inconsistent public and private keys can leak data about the private key.

ed25519-dalek v2.0.0 has been released on 2023-08-11 and fixes the issue by providing a safe API. Biscuit-rust has a PR migrating to ed25519-dalek v2: https://github.com/biscuit-auth/biscuit-rust/pull/136 this PR has been opened several months ago, targetting release candidates of ed25519-dalek v2. Now that v2 has been released, we'll soon merge this PR and issue a major biscuit-rust release (the major bump is required because of a coupled bump for rand, required by ed25519-dalek v2).

Now, even though the advisory was released only a few days ago, there was general awareness of the unsafe API of ed25519 libraries in several languages: https://github.com/MystenLabs/ed25519-unsafe-libs As soon as awareness of the API misuse risk spread, an informal audit of the biscuit-rust codebase was conducted, concluding that there was no API misuse in biscuit-rust (use with a keypair constructed from user-supplied public and private keys). However, several functions from the public API directly take a KeyPair argument which is ultimately used to sign content, so unsafe use is still possible by code depending on biscuit-rust. That's why we advise upgrading biscuit-rust as soon as possible.

Code audit

ed25519::KeyPair::try_sign is the dangerous function: called on a KeyPair value with a private key and an attacker-crafted public key, it can leak information about the private key. It is called in three places:

• biscuit-auth/src/format/mod.rs line 463 in pub fn seal. It is called on a KeyPair generated by TokenNext.keypair(), generated solely from a PrivateKey. This call is safe.
• biscuit-auth/src/token/third_party.rs line 143 in pub fn create_block. It is called on a KeyPair directly generated from a PrivateKey. This call is safe.
• biscuit-auth/src/crypto/mod.rs line 239 in pub fn sign. This call takes a crypto::KeyPair as an argument. crypto::KeyPair is a type from biscuit_rust that wraps a KeyPair from ed25519-dalek. So it is only safe if called with a safe KeyPair value. Note that even if this is a pub fn, it is in a module that is not exported by the library. crypto::sign is called in several places.
  • biscuit-auth/src/format/mod.rs line 342. It is called with a KeyPair generated by TokenNext.keypair() which is safe
  • biscuit-auth/src/format/mod.rs line 375. It is called with a KeyPair generated by TokenNext.keypair() which is safe
  • biscuit-auth/src/format/mod.rs line 308. It is called with a KeyPair provided as an argument of SerializedBiscuit::new which is a public API function. It is called in biscuit-auth/src/token/mod.rs line 245, which is not part of a public API function, but is called in biscuit-auth/src/token/builder.rs line 405, which itself is called in biscuit-auth/src/token/builder.rs line 395 and indirectly line 387, both being part of public API functions.

To summarize:

• biscuit-rust itself does not use the API unsafely;
• but biscuit-rust still allows indirect unsafe API use by downstream libraries, specifically in:
  • BiscuitBuilder::new()
  • BiscuitBuilder::new_with_symbols()
  • BiscuitBuilder::new_with_rng()
  • SerializedBiscuit::new()

All those functions take crypto::KeyPair values that can only be built through safe APIs. Such safely-built values can still be mutated by swapping the wrapped ed25519::KeyPair value with an arbitrary one with no safeness guaranteed. Doing so allows unsafe API use.

Mitigation in biscuit-rust v4.0.0

crypto::KeyPair now wraps a ed25519::SigningKey value which is always safe to use for signing operations. This is technically a breaking change on a public API, so a major update is needed, even though typical use cases won't be affected.