From 0e2d72b0d88517eb64981440e18dd1be285ead6d Mon Sep 17 00:00:00 2001 From: Saoirse Aronson Date: Thu, 9 Apr 2026 13:34:56 +0200 Subject: [PATCH] Remove dead code in crypto and fix algo bug The (ultimately not public) ed25519 and p256 crypto types expose a lot of methods that are never called; dead code left over from when the abstraction over multiple signing algorithms was added. The P256 code in several places had copy pasted code from ed25519 which incorrectly specified the algorithm as as ed25519; most of these were dead code, but one was in the Hash implementation of P256's public key, which incorrectly hashed the ed25519 identifier. --- biscuit-auth/src/crypto/ed25519.rs | 64 +----------------------------- biscuit-auth/src/crypto/p256.rs | 50 +++-------------------- 2 files changed, 7 insertions(+), 107 deletions(-) diff --git a/biscuit-auth/src/crypto/ed25519.rs b/biscuit-auth/src/crypto/ed25519.rs index 9782fc22..eb97128b 100644 --- a/biscuit-auth/src/crypto/ed25519.rs +++ b/biscuit-auth/src/crypto/ed25519.rs @@ -11,7 +11,7 @@ //! //! The implementation is based on [ed25519_dalek](https://github.com/dalek-cryptography/ed25519-dalek). #![allow(non_snake_case)] -use crate::{error::Format, format::schema}; +use crate::error::Format; use super::error; use super::Signature; @@ -30,10 +30,6 @@ pub struct KeyPair { } impl KeyPair { - pub fn new() -> Self { - Self::new_with_rng(&mut rand::rngs::OsRng) - } - pub fn new_with_rng(rng: &mut T) -> Self { let kp = ed25519_dalek::SigningKey::generate(rng); KeyPair { kp } @@ -76,10 +72,6 @@ impl KeyPair { PublicKey(self.kp.verifying_key()) } - pub fn algorithm(&self) -> crate::format::schema::public_key::Algorithm { - crate::format::schema::public_key::Algorithm::Ed25519 - } - #[cfg(feature = "pem")] pub fn from_private_key_der(bytes: &[u8]) -> Result { let kp = SigningKey::from_pkcs8_der(bytes) @@ -116,12 +108,6 @@ impl KeyPair { } } -impl std::default::Default for KeyPair { - fn default() -> Self { - Self::new() - } -} - /// the private part of a [KeyPair] #[derive(Debug, PartialEq)] pub struct PrivateKey(pub(crate) ed25519_dalek::SecretKey); @@ -132,11 +118,6 @@ impl PrivateKey { self.0.to_vec() } - /// serializes to an hex-encoded string - pub fn to_bytes_hex(&self) -> String { - hex::encode(self.0) - } - /// deserializes from a byte array pub fn from_bytes(bytes: &[u8]) -> Result { let bytes: [u8; 32] = bytes @@ -145,12 +126,6 @@ impl PrivateKey { Ok(PrivateKey(bytes)) } - /// deserializes from an hex-encoded string - pub fn from_bytes_hex(str: &str) -> Result { - let bytes = hex::decode(str).map_err(|e| error::Format::InvalidKey(e.to_string()))?; - Self::from_bytes(&bytes) - } - #[cfg(feature = "pem")] pub fn from_der(bytes: &[u8]) -> Result { let kp = SigningKey::from_pkcs8_der(bytes) @@ -188,10 +163,6 @@ impl PrivateKey { pub fn public(&self) -> PublicKey { PublicKey(SigningKey::from_bytes(&self.0).verifying_key()) } - - pub fn algorithm(&self) -> crate::format::schema::public_key::Algorithm { - crate::format::schema::public_key::Algorithm::Ed25519 - } } impl std::clone::Clone for PrivateKey { @@ -216,11 +187,6 @@ impl PublicKey { self.0.to_bytes() } - /// serializes to an hex-encoded string - pub fn to_bytes_hex(&self) -> String { - hex::encode(self.to_bytes()) - } - /// deserializes from a byte array pub fn from_bytes(bytes: &[u8]) -> Result { let bytes: [u8; 32] = bytes @@ -233,30 +199,6 @@ impl PublicKey { .map_err(Format::InvalidKey) } - /// deserializes from an hex-encoded string - pub fn from_bytes_hex(str: &str) -> Result { - let bytes = hex::decode(str).map_err(|e| error::Format::InvalidKey(e.to_string()))?; - Self::from_bytes(&bytes) - } - - pub fn from_proto(key: &schema::PublicKey) -> Result { - if key.algorithm != schema::public_key::Algorithm::Ed25519 as i32 { - return Err(error::Format::DeserializationError(format!( - "deserialization error: unexpected key algorithm {}", - key.algorithm - ))); - } - - PublicKey::from_bytes(&key.key) - } - - pub fn to_proto(&self) -> schema::PublicKey { - schema::PublicKey { - algorithm: schema::public_key::Algorithm::Ed25519 as i32, - key: self.to_bytes().to_vec(), - } - } - pub fn verify_signature( &self, data: &[u8], @@ -277,10 +219,6 @@ impl PublicKey { .map_err(error::Format::Signature) } - pub fn algorithm(&self) -> crate::format::schema::public_key::Algorithm { - crate::format::schema::public_key::Algorithm::Ed25519 - } - #[cfg(feature = "pem")] pub fn from_der(bytes: &[u8]) -> Result { use ed25519_dalek::pkcs8::DecodePublicKey; diff --git a/biscuit-auth/src/crypto/p256.rs b/biscuit-auth/src/crypto/p256.rs index a76e4ce4..4545d4d7 100644 --- a/biscuit-auth/src/crypto/p256.rs +++ b/biscuit-auth/src/crypto/p256.rs @@ -3,13 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ #![allow(non_snake_case)] -use crate::{error::Format, format::schema}; +use crate::error::Format; use super::error; use super::Signature; use p256::ecdsa::{signature::Signer, signature::Verifier, SigningKey, VerifyingKey}; -use p256::elliptic_curve::rand_core::{CryptoRng, OsRng, RngCore}; +use p256::elliptic_curve::rand_core::{CryptoRng, RngCore}; use p256::NistP256; use std::hash::Hash; @@ -20,10 +20,6 @@ pub struct KeyPair { } impl KeyPair { - pub fn new() -> Self { - Self::new_with_rng(&mut OsRng) - } - pub fn new_with_rng(rng: &mut T) -> Self { let kp = SigningKey::random(rng); @@ -66,10 +62,6 @@ impl KeyPair { PublicKey(*self.kp.verifying_key()) } - pub fn algorithm(&self) -> crate::format::schema::public_key::Algorithm { - crate::format::schema::public_key::Algorithm::Secp256r1 - } - #[cfg(feature = "pem")] pub fn from_private_key_der(bytes: &[u8]) -> Result { use p256::pkcs8::DecodePrivateKey; @@ -110,12 +102,6 @@ impl KeyPair { } } -impl std::default::Default for KeyPair { - fn default() -> Self { - Self::new() - } -} - /// the private part of a [KeyPair] #[derive(Debug, PartialEq)] pub struct PrivateKey(SigningKey); @@ -194,10 +180,6 @@ impl PrivateKey { pub fn public(&self) -> PublicKey { PublicKey(*(&self.0).verifying_key()) } - - pub fn algorithm(&self) -> crate::format::schema::public_key::Algorithm { - crate::format::schema::public_key::Algorithm::Ed25519 - } } impl std::clone::Clone for PrivateKey { @@ -275,24 +257,6 @@ impl PublicKey { Ok(kp) } - pub fn from_proto(key: &schema::PublicKey) -> Result { - if key.algorithm != schema::public_key::Algorithm::Ed25519 as i32 { - return Err(error::Format::DeserializationError(format!( - "deserialization error: unexpected key algorithm {}", - key.algorithm - ))); - } - - PublicKey::from_bytes(&key.key) - } - - pub fn to_proto(&self) -> schema::PublicKey { - schema::PublicKey { - algorithm: schema::public_key::Algorithm::Ed25519 as i32, - key: self.to_bytes().to_vec(), - } - } - pub fn verify_signature( &self, data: &[u8], @@ -312,10 +276,6 @@ impl PublicKey { .map_err(error::Format::Signature) } - pub fn algorithm(&self) -> crate::format::schema::public_key::Algorithm { - crate::format::schema::public_key::Algorithm::Ed25519 - } - pub(crate) fn write(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "secp256r1/{}", hex::encode(&self.to_bytes())) } @@ -326,18 +286,20 @@ impl PublicKey { impl Hash for PublicKey { fn hash(&self, state: &mut H) { - (crate::format::schema::public_key::Algorithm::Ed25519 as i32).hash(state); + (crate::format::schema::public_key::Algorithm::Secp256r1 as i32).hash(state); self.to_bytes().hash(state); } } #[cfg(test)] mod tests { + use p256::elliptic_curve::rand_core::OsRng; + use super::*; #[test] fn serialization() { - let kp = KeyPair::new(); + let kp = KeyPair::new_with_rng(&mut OsRng); let private = kp.private(); let public = kp.public(); let private_hex = private.to_bytes_hex();