Skip to content

Commit

Permalink
feat: Use ed25519-consensus instead of ring (#606)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamspofford-dfinity authored Oct 7, 2024
1 parent 98164b2 commit fba7c4c
Show file tree
Hide file tree
Showing 19 changed files with 118 additions and 116 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

* Changed `BasicIdentity`'s implmentation from `ring` to `ed25519-consensus`.
* Added `AgentBuilder::with_max_polling_time` to config the maximum time to wait for a response from the replica.
* `DelegatedIdentity::new` now checks the delegation chain. The old behavior is available under `new_unchecked`.

Expand Down
27 changes: 15 additions & 12 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ ic-certification = "2.2"
candid = "0.10.1"
candid_parser = "0.1.1"
clap = "4.4.3"
ed25519-consensus = "2.1.0"
futures-util = "0.3.21"
hex = "0.4.3"
k256 = "0.13.4"
leb128 = "0.2.5"
p256 = "0.13.2"
rand = "0.8.5"
reqwest = { version = "0.12", default-features = false }
ring = "0.17.7"
serde = "1.0.162"
serde_bytes = "0.11.13"
serde_cbor = "0.11.2"
Expand Down
9 changes: 5 additions & 4 deletions ic-agent/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ async-watch = { version = "0.3", optional = true }
backoff = "0.4.0"
cached = { version = "0.52", features = ["ahash"], default-features = false }
candid = { workspace = true }
der = "0.7"
ecdsa = "0.16"
ed25519-consensus = { version = "2" }
ed25519-consensus = { workspace = true }
elliptic-curve = "0.13"
futures-util = { workspace = true }
hex = { workspace = true }
Expand All @@ -38,9 +39,9 @@ p256 = { workspace = true, features = ["pem"] }
leb128 = { workspace = true }
pkcs8 = { version = "0.10.2", features = ["std"] }
sec1 = { version = "0.7.2", features = ["pem"] }
rand = "0.8.5"
rand = { workspace = true }
rangemap = "1.4"
ring = { workspace = true, features = ["std"] }
ring = { version = "0.17", optional = true }
serde = { workspace = true, features = ["derive"] }
serde_bytes = { workspace = true }
serde_cbor = { workspace = true }
Expand All @@ -65,7 +66,6 @@ optional = true

[target.'cfg(not(target_family = "wasm"))'.dependencies]
tokio = { version = "1.24.2", features = ["time"] }
rustls-webpki = "0.102"

[target.'cfg(target_family = "wasm")'.dependencies]
getrandom = { version = "0.2", features = ["js"], optional = true }
Expand Down Expand Up @@ -95,6 +95,7 @@ web-sys = { version = "0.3", features = [
[features]
default = ["pem"]
pem = ["dep:pem"]
ring = ["dep:ring"]
ic_ref_tests = [
"default",
] # Used to separate integration tests for ic-ref which need a server running.
Expand Down
8 changes: 2 additions & 6 deletions ic-agent/src/agent/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,9 @@ type AgentFuture<'a, V> = Pin<Box<dyn Future<Output = Result<V, AgentError>> + '
/// }
///
/// # fn create_identity() -> impl ic_agent::Identity {
/// # let rng = ring::rand::SystemRandom::new();
/// # let key_pair = ring::signature::Ed25519KeyPair::generate_pkcs8(&rng)
/// # .expect("Could not generate a key pair.");
/// #
/// # ic_agent::identity::BasicIdentity::from_key_pair(
/// # ring::signature::Ed25519KeyPair::from_pkcs8(key_pair.as_ref())
/// # .expect("Could not read the key pair."),
/// # ic_agent::identity::BasicIdentity::from_signing_key(
/// # ed25519_consensus::SigningKey::new(rand::thread_rng())
/// # )
/// # }
/// #
Expand Down
60 changes: 46 additions & 14 deletions ic-agent/src/identity/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{agent::EnvelopeContent, export::Principal, Identity, Signature};
#[cfg(feature = "pem")]
use crate::identity::error::PemError;

use ring::signature::{Ed25519KeyPair, KeyPair};
use ed25519_consensus::SigningKey;
use simple_asn1::{
oid, to_der,
ASN1Block::{BitString, ObjectIdentifier, Sequence},
Expand All @@ -13,9 +13,11 @@ use std::fmt;

use super::Delegation;

/// A Basic Identity which sign using an ED25519 key pair.
/// A cryptographic identity which signs using an Ed25519 key pair.
///
/// The caller will be represented via [`Principal::self_authenticating`], which contains the SHA-224 hash of the public key.
pub struct BasicIdentity {
key_pair: Ed25519KeyPair,
private_key: KeyCompat,
der_encoded_public_key: Vec<u8>,
}

Expand All @@ -28,35 +30,65 @@ impl fmt::Debug for BasicIdentity {
}

impl BasicIdentity {
/// Create a BasicIdentity from reading a PEM file at the path.
/// Create a `BasicIdentity` from reading a PEM file at the path.
#[cfg(feature = "pem")]
pub fn from_pem_file<P: AsRef<std::path::Path>>(file_path: P) -> Result<Self, PemError> {
Self::from_pem(std::fs::File::open(file_path)?)
}

/// Create a BasicIdentity from reading a PEM File from a Reader.
/// Create a `BasicIdentity` from reading a PEM File from a Reader.
#[cfg(feature = "pem")]
pub fn from_pem<R: std::io::Read>(pem_reader: R) -> Result<Self, PemError> {
use der::{Decode, PemReader};
use pkcs8::PrivateKeyInfo;

let bytes: Vec<u8> = pem_reader
.bytes()
.collect::<Result<Vec<u8>, std::io::Error>>()?;
let pki = PrivateKeyInfo::decode(&mut PemReader::new(&bytes)?)?;
let private_key = SigningKey::try_from(pki.private_key)?;
Ok(BasicIdentity::from_signing_key(private_key))
}

Ok(BasicIdentity::from_key_pair(Ed25519KeyPair::from_pkcs8(
pem::parse(bytes)?.contents(),
)?))
/// Create a `BasicIdentity` from a `SigningKey` from `ed25519-consensus`.
pub fn from_signing_key(key: SigningKey) -> Self {
let public_key = key.verification_key();
let der_encoded_public_key = der_encode_public_key(public_key.as_bytes().to_vec());

Self {
private_key: KeyCompat::Standard(key),
der_encoded_public_key,
}
}

/// Create a BasicIdentity from a KeyPair from the ring crate.
pub fn from_key_pair(key_pair: Ed25519KeyPair) -> Self {
/// Create a `BasicIdentity` from an `Ed25519KeyPair` from `ring`.
#[cfg(feature = "ring")]
pub fn from_key_pair(key_pair: ring::signature::Ed25519KeyPair) -> Self {
use ring::signature::KeyPair;
let der_encoded_public_key = der_encode_public_key(key_pair.public_key().as_ref().to_vec());

Self {
key_pair,
private_key: KeyCompat::Ring(key_pair),
der_encoded_public_key,
}
}
}

enum KeyCompat {
Standard(SigningKey),
#[cfg(feature = "ring")]
Ring(ring::signature::Ed25519KeyPair),
}

impl KeyCompat {
fn sign(&self, payload: &[u8]) -> Vec<u8> {
match self {
Self::Standard(k) => k.sign(payload).to_bytes().to_vec(),
#[cfg(feature = "ring")]
Self::Ring(k) => k.sign(payload).as_ref().to_vec(),
}
}
}

impl Identity for BasicIdentity {
fn sender(&self) -> Result<Principal, String> {
Ok(Principal::self_authenticating(&self.der_encoded_public_key))
Expand All @@ -75,9 +107,9 @@ impl Identity for BasicIdentity {
}

fn sign_arbitrary(&self, content: &[u8]) -> Result<Signature, String> {
let signature = self.key_pair.sign(content);
let signature = self.private_key.sign(content);
Ok(Signature {
signature: Some(signature.as_ref().to_vec()),
signature: Some(signature),
public_key: self.public_key(),
delegations: None,
})
Expand Down
6 changes: 2 additions & 4 deletions ic-agent/src/identity/delegated.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use candid::Principal;
use der::{Decode, SliceReader};
use ecdsa::signature::Verifier;
use k256::Secp256k1;
use p256::NistP256;
use pkcs8::{spki::SubjectPublicKeyInfoRef, AssociatedOid, ObjectIdentifier};
use sec1::{
der::{Decode, SliceReader},
EcParameters, EncodedPoint,
};
use sec1::{EcParameters, EncodedPoint};

use crate::{agent::EnvelopeContent, Signature};

Expand Down
11 changes: 8 additions & 3 deletions ic-agent/src/identity/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@ pub enum PemError {
#[error("An error occurred while reading the file: {0}")]
PemError(#[from] pem::PemError),

/// The key was rejected by Ring.
#[error("A key was rejected by Ring: {0}")]
KeyRejected(#[from] ring::error::KeyRejected),
/// An error occurred while reading the file in DER format.
#[cfg(feature = "pem")]
#[error("An error occurred while reading the file: {0}")]
DerError(#[from] der::Error),

/// The key was rejected by ed25519-consensus.
#[error("A key was rejected by ed25519-consensus: {0}")]
KeyRejected(#[from] ed25519_consensus::Error),

/// The key was rejected by k256.
#[error("A key was rejected by k256: {0}")]
Expand Down
9 changes: 2 additions & 7 deletions ic-agent/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@
//! }
//!
//! # fn create_identity() -> impl ic_agent::Identity {
//! # let rng = ring::rand::SystemRandom::new();
//! # let key_pair = ring::signature::Ed25519KeyPair::generate_pkcs8(&rng)
//! # .expect("Could not generate a key pair.");
//! #
//! # ic_agent::identity::BasicIdentity::from_key_pair(
//! # ring::signature::Ed25519KeyPair::from_pkcs8(key_pair.as_ref())
//! # .expect("Could not read the key pair."),
//! # ic_agent::identity::BasicIdentity::from_signing_key(
//! # ed25519_consensus::SigningKey::new(rand::thread_rng()),
//! # )
//! # }
//! #
Expand Down
2 changes: 1 addition & 1 deletion ic-transport-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ mod request_id;
pub mod signed;

/// The authentication envelope, containing the contents and their signature. This struct can be passed to `Agent`'s
/// `*_signed` methods via [`to_bytes`](Envelope::to_bytes).
/// `*_signed` methods via [`encode_bytes`](Envelope::encode_bytes).
#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(rename_all = "snake_case")]
pub struct Envelope<'a> {
Expand Down
5 changes: 2 additions & 3 deletions ic-utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ categories = ["api-bindings", "data-structures", "no-std"]
keywords = ["internet-computer", "agent", "utility", "icp", "dfinity"]
include = ["src", "Cargo.toml", "../LICENSE", "README.md"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
async-trait = "0.1.68"
candid = { workspace = true, features = ["value"] }
Expand All @@ -33,8 +31,9 @@ semver = "1.0.7"
once_cell = "1.10.0"

[dev-dependencies]
ed25519-consensus = { workspace = true }
ic-agent = { workspace = true, default-features = true }
ring = { workspace = true }
rand = { workspace = true }
tokio = { workspace = true, features = ["full"] }

[features]
Expand Down
Loading

0 comments on commit fba7c4c

Please sign in to comment.