Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: upgrade ed25519-dalek dependency to v2 #64

Merged
merged 2 commits into from
Feb 16, 2024
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
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ tokio = { version = "1.22", features = ["full"] }
url = "2.3"
warp = { version = "0.3", default-features = false }
serde_json = "1.0"
rand = "0.8.5"

[[example]]
name = "websocket_client"
Expand Down
8 changes: 4 additions & 4 deletions examples/http_client.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use {
relay_client::{http::Client, ConnectionOptions},
relay_rpc::{
auth::{ed25519_dalek::Keypair, rand, AuthToken},
auth::{ed25519_dalek::SigningKey, AuthToken},
domain::Topic,
},
std::{sync::Arc, time::Duration},
Expand All @@ -20,7 +20,7 @@ struct Args {
project_id: String,
}

fn create_conn_opts(key: &Keypair, address: &str, project_id: &str) -> ConnectionOptions {
fn create_conn_opts(key: &SigningKey, address: &str, project_id: &str) -> ConnectionOptions {
let aud = Url::parse(address)
.unwrap()
.origin()
Expand All @@ -39,10 +39,10 @@ fn create_conn_opts(key: &Keypair, address: &str, project_id: &str) -> Connectio
async fn main() -> anyhow::Result<()> {
let args = Args::from_args();

let key1 = Keypair::generate(&mut rand::thread_rng());
let key1 = SigningKey::generate(&mut rand::thread_rng());
let client1 = Client::new(&create_conn_opts(&key1, &args.address, &args.project_id))?;

let key2 = Keypair::generate(&mut rand::thread_rng());
let key2 = SigningKey::generate(&mut rand::thread_rng());
let client2 = Client::new(&create_conn_opts(&key2, &args.address, &args.project_id))?;

let topic = Topic::generate();
Expand Down
12 changes: 6 additions & 6 deletions examples/webhook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use {
ConnectionOptions,
},
relay_rpc::{
auth::{ed25519_dalek::Keypair, rand, AuthToken},
auth::{ed25519_dalek::SigningKey, AuthToken},
domain::{DecodedClientId, Topic},
jwt::VerifyableClaims,
rpc,
Expand Down Expand Up @@ -35,7 +35,7 @@ struct Args {
webhook_server_port: u16,
}

fn create_conn_opts(key: &Keypair, address: &str, project_id: &str) -> ConnectionOptions {
fn create_conn_opts(key: &SigningKey, address: &str, project_id: &str) -> ConnectionOptions {
let aud = Url::parse(address)
.unwrap()
.origin()
Expand Down Expand Up @@ -122,26 +122,26 @@ async fn main() -> anyhow::Result<()> {
// Give time for the server to start up.
tokio::time::sleep(Duration::from_millis(500)).await;

let publisher_key = Keypair::generate(&mut rand::thread_rng());
let publisher_key = SigningKey::generate(&mut rand::thread_rng());
let publisher = Client::new(&create_conn_opts(
&publisher_key,
&args.address,
&args.project_id,
))?;
println!(
"[publisher] client id: {}",
DecodedClientId::from(publisher_key.public_key()).to_did_key()
DecodedClientId::from(publisher_key.verifying_key()).to_did_key()
);

let subscriber_key = Keypair::generate(&mut rand::thread_rng());
let subscriber_key = SigningKey::generate(&mut rand::thread_rng());
let subscriber = Client::new(&create_conn_opts(
&subscriber_key,
&args.address,
&args.project_id,
))?;
println!(
"[subscriber] client id: {}",
DecodedClientId::from(subscriber_key.public_key()).to_did_key()
DecodedClientId::from(subscriber_key.verifying_key()).to_did_key()
);

let topic = Topic::generate();
Expand Down
4 changes: 2 additions & 2 deletions examples/websocket_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use {
ConnectionOptions,
},
relay_rpc::{
auth::{ed25519_dalek::Keypair, rand, AuthToken},
auth::{ed25519_dalek::SigningKey, AuthToken},
domain::Topic,
},
std::{sync::Arc, time::Duration},
Expand Down Expand Up @@ -59,7 +59,7 @@ impl ConnectionHandler for Handler {
}

fn create_conn_opts(address: &str, project_id: &str) -> ConnectionOptions {
let key = Keypair::generate(&mut rand::thread_rng());
let key = SigningKey::generate(&mut rand::thread_rng());

let auth = AuthToken::new("http://example.com")
.aud(address)
Expand Down
10 changes: 5 additions & 5 deletions relay_client/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use {
},
http::{HeaderMap, StatusCode},
relay_rpc::{
auth::ed25519_dalek::Keypair,
auth::ed25519_dalek::SigningKey,
domain::{DecodedClientId, SubscriptionId, Topic},
jwt::{self, JwtError, VerifyableClaims},
rpc::{self, Receipt, RequestPayload},
Expand Down Expand Up @@ -168,7 +168,7 @@ impl Client {
pub async fn watch_register(
&self,
request: WatchRegisterRequest,
keypair: &Keypair,
keypair: &SigningKey,
) -> Response<rpc::WatchRegister> {
let iat = chrono::Utc::now().timestamp();
let ttl_sec: i64 = request
Expand All @@ -180,7 +180,7 @@ impl Client {

let claims = rpc::WatchRegisterClaims {
basic: jwt::JwtBasicClaims {
iss: DecodedClientId::from_key(&keypair.public_key()).into(),
iss: DecodedClientId::from_key(&keypair.verifying_key()).into(),
aud: self.origin.clone(),
iat,
sub: request.service_url,
Expand Down Expand Up @@ -212,13 +212,13 @@ impl Client {
pub async fn watch_unregister(
&self,
request: WatchUnregisterRequest,
keypair: &Keypair,
keypair: &SigningKey,
) -> Response<rpc::WatchUnregister> {
let iat = chrono::Utc::now().timestamp();

let claims = rpc::WatchUnregisterClaims {
basic: jwt::JwtBasicClaims {
iss: DecodedClientId::from_key(&keypair.public_key()).into(),
iss: DecodedClientId::from_key(&keypair.verifying_key()).into(),
aud: self.origin.clone(),
iat,
sub: request.service_url,
Expand Down
4 changes: 2 additions & 2 deletions relay_rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ serde = { version = "1.0", features = ["derive", "rc"] }
serde-aux = { version = "4.1", default-features = false }
serde_json = "1.0"
thiserror = "1.0"
ed25519-dalek = { git = "https://github.com/dalek-cryptography/ed25519-dalek.git", rev = "7529d65" }
rand = "0.7"
ed25519-dalek = { version = "2.1.1", features = ["rand_core"] }
rand = "0.8"
chrono = { version = "0.4", default-features = false, features = [
"std",
"clock",
Expand Down
12 changes: 6 additions & 6 deletions relay_rpc/src/auth.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
pub use ed25519_dalek;
use {
crate::{
domain::DecodedClientId,
jwt::{JwtBasicClaims, JwtHeader},
},
chrono::{DateTime, Utc},
ed25519_dalek::{ed25519::signature::Signature, Keypair, Signer},
ed25519_dalek::{Signer, SigningKey},
serde::{Deserialize, Serialize},
std::{fmt::Display, time::Duration},
};
pub use {chrono, ed25519_dalek, rand};

#[cfg(feature = "cacao")]
pub mod cacao;
Expand Down Expand Up @@ -80,7 +80,7 @@ impl AuthToken {
self
}

pub fn as_jwt(&self, key: &Keypair) -> Result<SerializedAuthToken, Error> {
pub fn as_jwt(&self, key: &SigningKey) -> Result<SerializedAuthToken, Error> {
let iat = self.iat.unwrap_or_else(Utc::now);
let aud = self.aud.as_deref().unwrap_or(DEFAULT_TOKEN_AUD);

Expand All @@ -89,7 +89,7 @@ impl AuthToken {
}

pub fn encode_auth_token(
key: &Keypair,
key: &SigningKey,
sub: impl Into<String>,
aud: impl Into<String>,
iat: DateTime<Utc>,
Expand All @@ -105,7 +105,7 @@ pub fn encode_auth_token(

let claims = {
let data = JwtBasicClaims {
iss: DecodedClientId::from_key(&key.public_key()).into(),
iss: DecodedClientId::from_key(&key.verifying_key()).into(),
sub: sub.into(),
aud: aud.into(),
iat: iat.timestamp(),
Expand All @@ -121,7 +121,7 @@ pub fn encode_auth_token(
let signature = {
let data = key.sign(message.as_bytes());

encoder.encode(data.as_bytes())
encoder.encode(&data.to_bytes())
};

Ok(SerializedAuthToken(format!("{message}.{signature}")))
Expand Down
16 changes: 8 additions & 8 deletions relay_rpc/src/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use {
new_type,
},
derive_more::{AsMut, AsRef},
ed25519_dalek::PublicKey,
ed25519_dalek::VerifyingKey,
serde::{Deserialize, Serialize},
serde_aux::prelude::deserialize_number_from_string,
std::{str::FromStr, sync::Arc},
Expand Down Expand Up @@ -82,7 +82,7 @@ pub struct DidKey(
#[serde(with = "crate::serde_helpers::client_id_as_did_key")] pub DecodedClientId,
);

impl From<DidKey> for PublicKey {
impl From<DidKey> for VerifyingKey {
fn from(val: DidKey) -> Self {
val.0.as_public_key()
}
Expand Down Expand Up @@ -119,24 +119,24 @@ impl DecodedClientId {
}

#[inline]
pub fn from_key(key: &PublicKey) -> Self {
pub fn from_key(key: &VerifyingKey) -> Self {
Self(*key.as_bytes())
}

#[inline]
pub fn as_public_key(&self) -> PublicKey {
pub fn as_public_key(&self) -> VerifyingKey {
// We know that the length is correct, so we can just unwrap.
PublicKey::from_bytes(&self.0).unwrap()
VerifyingKey::from_bytes(&self.0).unwrap()
Copy link
Member Author

@chris13524 chris13524 Feb 14, 2024

Choose a reason for hiding this comment

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

This function only accepts correct-length arrays, so this comment is wrong. This function may panic for other reasons, we should probably investigate if using unwrap() is dangerous here

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right. It can panic if the bytes are invalid. I think the easiest and non-breaking solution here is to add try_as_public_key() method which wouldn't unwrap. And also adding a doc for as_public_key() that it will panic on invalid key.

Copy link
Member Author

Choose a reason for hiding this comment

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

Leaving it as-is since this bug already existed, but made an issue to track it

}
}

impl From<PublicKey> for DecodedClientId {
fn from(key: PublicKey) -> Self {
impl From<VerifyingKey> for DecodedClientId {
fn from(key: VerifyingKey) -> Self {
Self::from_key(&key)
}
}

impl From<DecodedClientId> for PublicKey {
impl From<DecodedClientId> for VerifyingKey {
fn from(val: DecodedClientId) -> Self {
val.as_public_key()
}
Expand Down
16 changes: 8 additions & 8 deletions relay_rpc/src/jwt.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use {
crate::domain::DidKey,
chrono::Utc,
ed25519_dalek::{ed25519::signature::Signature, Keypair, PublicKey, Signer},
ed25519_dalek::{Signer, SigningKey},
serde::{de::DeserializeOwned, Deserialize, Serialize},
std::collections::HashSet,
};
Expand Down Expand Up @@ -103,20 +103,19 @@ pub trait VerifyableClaims: Serialize + DeserializeOwned {
/// Encodes the claims into a JWT string, signing it with the provided key.
/// Returns an error if the provided key does not match the public key in
/// the claims (`iss`), or if serialization fails.
fn encode(&self, key: &Keypair) -> Result<String, JwtError> {
let public_key = PublicKey::from_bytes(self.basic().iss.as_ref())
.map_err(|_| JwtError::InvalidKeypair)?;
fn encode(&self, key: &SigningKey) -> Result<String, JwtError> {
let public_key = self.basic().iss.0.as_public_key();

// Make sure the keypair matches the public key in the claims.
if public_key != key.public_key() {
if public_key != key.verifying_key() {
return Err(JwtError::InvalidKeypair);
}

let encoder = &data_encoding::BASE64URL_NOPAD;
let header = encoder.encode(serde_json::to_string(&JwtHeader::default())?.as_bytes());
let claims = encoder.encode(serde_json::to_string(self)?.as_bytes());
let message = format!("{header}.{claims}");
let signature = encoder.encode(key.sign(message.as_bytes()).as_bytes());
let signature = encoder.encode(&key.sign(message.as_bytes()).to_bytes());

Ok(format!("{message}.{signature}"))
}
Expand Down Expand Up @@ -240,7 +239,8 @@ mod test {
domain::ClientId,
jwt::{JwtBasicClaims, JwtError, VerifyableClaims, JWT_VALIDATION_TIME_LEEWAY_SECS},
},
ed25519_dalek::Keypair,
ed25519_dalek::SigningKey,
rand::rngs::OsRng,
std::{collections::HashSet, time::Duration},
};

Expand Down Expand Up @@ -283,7 +283,7 @@ mod test {
let jwt = Jwt("eyJhbGciOiJFZERTQSIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJkaWQ6a2V5Ono2TWtvZEhad25lVlJTaHRhTGY4SktZa3hwREdwMXZHWm5wR21kQnBYOE0yZXh4SGwiLCJzdWIiOiJjNDc5ZmU1ZGM0NjRlNzcxZTc4YjE5M2QyMzlhNjViNThkMjc4Y2FkMWMzNGJmYjBiNTcxNmU1YmI1MTQ5MjhlIiwiYXVkIjoid3NzOi8vcmVsYXkud2FsbGV0Y29ubmVjdC5jb20iLCJpYXQiOjE2NTY5MTAwOTcsImV4cCI6NDgxMjY3MDA5N30.nLdxz4f6yJ8HsWZJUvpSHjFjoat4PfJav-kyqdHj6JXcX5SyDvp3QNB9doyzRWb9jpbA36Av0qn4kqLl-pGuBg".to_owned());
assert!(matches!(jwt.decode(&aud), Err(JwtError::Serialization(..))));

let keypair = Keypair::generate(&mut rand::thread_rng());
let keypair = SigningKey::generate(&mut OsRng);
let sub: String = "test".to_owned();

// IAT in future.
Expand Down
20 changes: 9 additions & 11 deletions relay_rpc/src/rpc/watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,25 +125,23 @@ mod test {
super::*,
crate::{auth::RELAY_WEBSOCKET_ADDRESS, domain::DecodedClientId},
chrono::DateTime,
ed25519_dalek::Keypair,
ed25519_dalek::SigningKey,
};

const KEYPAIR: [u8; 64] = [
const KEYPAIR: [u8; 32] = [
215, 142, 127, 216, 153, 183, 205, 110, 103, 118, 181, 195, 60, 71, 5, 221, 100, 196, 207,
81, 229, 11, 116, 121, 235, 104, 1, 121, 25, 18, 218, 83, 216, 230, 100, 248, 132, 110, 55,
65, 221, 87, 66, 160, 36, 95, 116, 86, 169, 49, 107, 17, 13, 50, 22, 147, 199, 109, 125,
155, 89, 190, 186, 171,
81, 229, 11, 116, 121, 235, 104, 1, 121, 25, 18, 218, 83,
];

#[test]
fn watch_register_jwt() {
let key = Keypair::from_bytes(&KEYPAIR).unwrap();
let key = SigningKey::from_bytes(&KEYPAIR);
let iat = DateTime::parse_from_rfc3339("2000-01-01T00:00:00Z").unwrap();
let exp = DateTime::parse_from_rfc3339("3000-01-01T00:00:00Z").unwrap();

let claims = WatchRegisterClaims {
basic: JwtBasicClaims {
iss: DecodedClientId::from_key(&key.public_key()).into(),
iss: DecodedClientId::from_key(&key.verifying_key()).into(),
aud: RELAY_WEBSOCKET_ADDRESS.to_owned(),
sub: "https://example.com".to_owned(),
iat: iat.timestamp(),
Expand Down Expand Up @@ -172,13 +170,13 @@ mod test {

#[test]
fn watch_unregister_jwt() {
let key = Keypair::from_bytes(&KEYPAIR).unwrap();
let key = SigningKey::from_bytes(&KEYPAIR);
let iat = DateTime::parse_from_rfc3339("2000-01-01T00:00:00Z").unwrap();
let exp = DateTime::parse_from_rfc3339("3000-01-01T00:00:00Z").unwrap();

let claims = WatchUnregisterClaims {
basic: JwtBasicClaims {
iss: DecodedClientId::from_key(&key.public_key()).into(),
iss: DecodedClientId::from_key(&key.verifying_key()).into(),
aud: RELAY_WEBSOCKET_ADDRESS.to_owned(),
sub: "https://example.com".to_owned(),
iat: iat.timestamp(),
Expand All @@ -205,14 +203,14 @@ mod test {

#[test]
fn watch_event_jwt() {
let key = Keypair::from_bytes(&KEYPAIR).unwrap();
let key = SigningKey::from_bytes(&KEYPAIR);
let iat = DateTime::parse_from_rfc3339("2000-01-01T00:00:00Z").unwrap();
let exp = DateTime::parse_from_rfc3339("3000-01-01T00:00:00Z").unwrap();
let topic = Topic::from("474e88153f4db893de42c35e1891dc0e37a02e11961385de0475460fb48b8639");

let claims = WatchEventClaims {
basic: JwtBasicClaims {
iss: DecodedClientId::from_key(&key.public_key()).into(),
iss: DecodedClientId::from_key(&key.verifying_key()).into(),
aud: RELAY_WEBSOCKET_ADDRESS.to_owned(),
sub: "https://example.com".to_owned(),
iat: iat.timestamp(),
Expand Down
Loading