Skip to content

Commit

Permalink
Merge pull request #368 from rvolosatovs/update/dalek
Browse files Browse the repository at this point in the history
build: update `ed25519-dalek` and `base64`
  • Loading branch information
thomastaylor312 authored Aug 29, 2023
2 parents 175db03 + 90373bd commit 7672acf
Show file tree
Hide file tree
Showing 10 changed files with 220 additions and 231 deletions.
297 changes: 125 additions & 172 deletions Cargo.lock

Large diffs are not rendered by default.

13 changes: 8 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ anyhow = "1.0.44"
async-compression = { version = "0.3", default-features = false, features = ["tokio", "gzip"], optional = true }
async-trait = "0.1.51"
atty = { version = "0.2", optional = true }
base64 = "0.13.0"
base64 = "0.21"
bcrypt = "0.13"
bytes = "1.1.0"
clap = { version = "3", features = ["derive", "env", "cargo"], optional = true }
clap = { workspace = true, features = ["derive", "env", "cargo"], optional = true }
dirs = { version = "4.0.0", optional = true }
ed25519-dalek = "1.0.1"
ed25519-dalek = { version = "2", features = ["pkcs8", "rand_core"] }
either = { version = "1.6.1", optional = true }
futures = "0.3.17"
hyper = { version = "0.14.12", optional = true }
Expand All @@ -62,8 +62,7 @@ mime = { version = "0.3.16", optional = true }
mime_guess = { version = "2.0.3", optional = true }
oauth2 = { version = "4.1.0", features = ["reqwest"], optional = true }
openid = { version = "0.10", default-features = false, optional = true }
# We need the older version of rand for dalek
rand = "0.7"
rand = "0.8"
reqwest = { version = "0.11.4", features = ["stream"], default-features = false, optional = true }
semver = { version = "1.0.4", features = ["serde"] }
serde = { version = "1.0.130", features = ["derive"] }
Expand All @@ -89,8 +88,12 @@ warp = { version = "0.3", features = ["tls"], optional = true }
remove_dir_all = "0.8"

[dev-dependencies]
clap = { workspace = true, features = ["cargo"] }
rstest = "0.15.0"

[workspace.dependencies]
clap = "3"

[[bin]]
name = "bindle-server"
path = "bin/server.rs"
Expand Down
6 changes: 4 additions & 2 deletions bin/client/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use bindle::{
provider::Provider,
};

use base64::Engine;
use clap::Parser;
use sha2::Digest;
use tokio::io::AsyncWriteExt;
Expand Down Expand Up @@ -423,9 +424,10 @@ async fn run() -> std::result::Result<(), ClientError> {
.await
.unwrap_or_else(|_| KeyRing::default());
// First, check that the key is actually valid
let raw = base64::decode(&opts.key)
let key = base64::engine::general_purpose::STANDARD
.decode(&opts.key)
.map_err(|_| SignatureError::CorruptKey(opts.key.clone()))?;
ed25519_dalek::PublicKey::from_bytes(&raw)
ed25519_dalek::VerifyingKey::try_from(key.as_slice())
.map_err(|_| SignatureError::CorruptKey(opts.key.clone()))?;
keyring.add_entry(KeyEntry {
label: opts.label,
Expand Down
5 changes: 4 additions & 1 deletion src/authn/http_basic.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::{collections::HashMap, path::Path};

use base64::Engine;

use super::Authenticator;
use crate::authz::Authorizable;

Expand Down Expand Up @@ -99,7 +101,8 @@ fn parse_basic(auth_data: &str) -> anyhow::Result<(String, String)> {
None => anyhow::bail!("Wrong auth type. Only Basic auth is supported"),
Some(suffix) => {
// suffix should be base64 string
let decoded = String::from_utf8(base64::decode(suffix)?)?;
let decoded =
String::from_utf8(base64::engine::general_purpose::STANDARD.decode(suffix)?)?;
let pair: Vec<&str> = decoded.splitn(2, ':').collect();
if pair.len() != 2 {
anyhow::bail!("Malformed Basic header")
Expand Down
13 changes: 4 additions & 9 deletions src/authn/oidc.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! An authenticator that validates OIDC issued JWTs
use base64::Engine;
use jsonwebtoken::{Algorithm, DecodingKey, Validation};
use openid::biscuit::jwk::{AlgorithmParameters, KeyType, PublicKeyUse};
use serde::Deserialize;
Expand Down Expand Up @@ -151,22 +152,16 @@ impl OidcAuthenticator {
// that key is malformed (which should be a rare instance because it would mean
// misconfiguration on the OIDC provider's part), just skip it
let raw =
base64::decode(k.common.x509_chain.unwrap_or_default().pop()?).ok()?;
base64::engine::general_purpose::STANDARD.decode(k.common.x509_chain.unwrap_or_default().pop()?).ok()?;
DecodingKey::from_ec_der(&raw)
}
KeyType::RSA => {
if let AlgorithmParameters::RSA(rsa) = k.algorithm {
// NOTE: jsonwebtoken expects a base64 encoded component (big endian) so
// we are reencoding it here
DecodingKey::from_rsa_components(
&base64::encode_config(
rsa.n.to_bytes_be(),
base64::URL_SAFE_NO_PAD,
),
&base64::encode_config(
rsa.e.to_bytes_be(),
base64::URL_SAFE_NO_PAD,
),
&base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(rsa.n.to_bytes_be()),
&base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(rsa.e.to_bytes_be()),
).map_or_else(|e| {
tracing::error!(error = %e, "Unable to parse decoding key from discovery client, skipping");
None
Expand Down
4 changes: 3 additions & 1 deletion src/client/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
sync::Arc,
};

use base64::Engine;
use oauth2::reqwest::async_http_client;
use oauth2::{
basic::*, devicecode::DeviceAuthorizationResponse, AuthUrl, Client as Oauth2Client, ClientId,
Expand Down Expand Up @@ -110,7 +111,8 @@ impl HttpBasic {
#[async_trait::async_trait]
impl TokenManager for HttpBasic {
async fn apply_auth_header(&self, builder: RequestBuilder) -> Result<RequestBuilder> {
let data = base64::encode(format!("{}:{}", self.username, self.password));
let data = base64::engine::general_purpose::STANDARD
.encode(format!("{}:{}", self.username, self.password));
let mut header_val = HeaderValue::from_str(&format!("Basic {}", data))
.map_err(|e| ClientError::Other(e.to_string()))?;
header_val.set_sensitive(true);
Expand Down
11 changes: 7 additions & 4 deletions src/invoice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub use api::{
ErrorResponse, HealthResponse, InvoiceCreateResponse, KeyOptions, MissingParcelsResponse,
QueryOptions,
};
use base64::Engine;
#[doc(inline)]
pub use bindle_spec::BindleSpec;
#[doc(inline)]
Expand Down Expand Up @@ -203,7 +204,8 @@ impl Invoice {
let key = keyfile.key()?;
// The spec says it is illegal for the a single key to sign the same invoice
// more than once.
let encoded_key = base64::encode(key.public.to_bytes());
let encoded_key =
base64::engine::general_purpose::STANDARD.encode(key.verifying_key().as_bytes());
if let Some(sigs) = self.signature.as_ref() {
for s in sigs {
if s.key == encoded_key {
Expand All @@ -223,7 +225,7 @@ impl Invoice {
let signature_entry = Signature {
by: signer_name,
key: encoded_key,
signature: base64::encode(signature.to_bytes()),
signature: base64::engine::general_purpose::STANDARD.encode(signature.to_bytes()),
role: signer_role,
at: ts.as_secs(),
};
Expand Down Expand Up @@ -270,7 +272,8 @@ fn sign_one(
let key = keyfile.key()?;
// The spec says it is illegal for the a single key to sign the same invoice
// more than once.
let encoded_key = base64::encode(key.public.to_bytes());
let encoded_key =
base64::engine::general_purpose::STANDARD.encode(key.verifying_key().as_bytes());
if let Some(sigs) = inv.signature.as_ref() {
for s in sigs {
if s.key == encoded_key {
Expand All @@ -290,7 +293,7 @@ fn sign_one(
let signature_entry = Signature {
by: signer_name,
key: encoded_key,
signature: base64::encode(signature.to_bytes()),
signature: base64::engine::general_purpose::STANDARD.encode(signature.to_bytes()),
role: signer_role,
at: ts.as_secs(),
};
Expand Down
73 changes: 45 additions & 28 deletions src/invoice/signature.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
//! Contains the Signature type along with associated types and Roles

pub use ed25519_dalek::{Keypair, PublicKey, Signature as EdSignature, Signer};
use base64::Engine;
pub use ed25519_dalek::{
Signature as EdSignature,
Signer,
SigningKey as Keypair, // re-export under old name for backwards compatibility
SigningKey,
VerifyingKey as PublicKey, // re-export under old name for backwards compatibility
VerifyingKey,
};
use serde::{Deserialize, Serialize};
use thiserror::Error;
#[cfg(not(target_arch = "wasm32"))]
Expand Down Expand Up @@ -197,7 +205,7 @@ impl KeyRing {
self.key.push(entry)
}

pub fn contains(&self, key: &PublicKey) -> bool {
pub fn contains(&self, key: &VerifyingKey) -> bool {
// This could definitely be optimized.
for k in self.key.iter() {
// Note that we are skipping malformed keys because they don't matter
Expand Down Expand Up @@ -246,40 +254,43 @@ impl KeyEntry {
/// In most cases, it is fine to construct a KeyEntry struct manually. This
/// constructor merely encapsulates the logic to store the public key in its
/// canonical encoded format (as a String).
pub fn new(label: &str, roles: Vec<SignatureRole>, public_key: PublicKey) -> Self {
let key = base64::encode(public_key.to_bytes());
pub fn new(label: &str, roles: Vec<SignatureRole>, public_key: VerifyingKey) -> Self {
let key = base64::engine::general_purpose::STANDARD.encode(public_key.to_bytes());
KeyEntry {
label: label.to_owned(),
roles,
key,
label_signature: None,
}
}
pub fn sign_label(&mut self, key: Keypair) {
pub fn sign_label(&mut self, key: SigningKey) {
let sig = key.sign(self.label.as_bytes());
self.label_signature = Some(base64::encode(sig.to_bytes()));
self.label_signature =
Some(base64::engine::general_purpose::STANDARD.encode(sig.to_bytes()));
}
pub fn verify_label(self, key: PublicKey) -> anyhow::Result<()> {
pub fn verify_label(self, key: VerifyingKey) -> anyhow::Result<()> {
match self.label_signature {
None => {
tracing::log::info!("Label was not signed. Skipping.");
Ok(())
}
Some(txt) => {
let decoded_txt = base64::decode(txt)?;
let decoded_txt = base64::engine::general_purpose::STANDARD.decode(txt)?;
let sig = EdSignature::try_from(decoded_txt.as_slice())?;
key.verify_strict(self.label.as_bytes(), &sig)?;
Ok(())
}
}
}
pub(crate) fn public_key(&self) -> Result<PublicKey, SignatureError> {
let rawbytes = base64::decode(&self.key).map_err(|_e| {
// We swallow the source error because it could disclose information about
// the secret key.
SignatureError::CorruptKey("Base64 decoding of the public key failed".to_owned())
})?;
let pk = PublicKey::from_bytes(rawbytes.as_slice()).map_err(|e| {
pub(crate) fn public_key(&self) -> Result<VerifyingKey, SignatureError> {
let rawbytes = base64::engine::general_purpose::STANDARD
.decode(&self.key)
.map_err(|_e| {
// We swallow the source error because it could disclose information about
// the secret key.
SignatureError::CorruptKey("Base64 decoding of the public key failed".to_owned())
})?;
let pk = VerifyingKey::try_from(rawbytes.as_slice()).map_err(|e| {
error!(%e, "Error loading public key");
// Don't leak information about the key, because this could be sent to
// a remote. A generic error is all the user should see.
Expand All @@ -297,7 +308,7 @@ impl TryFrom<SecretKeyEntry> for KeyEntry {
let mut s = Self {
label: secret.label,
roles: secret.roles,
key: base64::encode(skey.public.to_bytes()),
key: base64::engine::general_purpose::STANDARD.encode(skey.verifying_key().as_bytes()),
label_signature: None,
};
s.sign_label(skey);
Expand All @@ -312,7 +323,7 @@ impl TryFrom<&SecretKeyEntry> for KeyEntry {
let mut s = Self {
label: secret.label.clone(),
roles: secret.roles.clone(),
key: base64::encode(skey.public.to_bytes()),
key: base64::engine::general_purpose::STANDARD.encode(skey.verifying_key().as_bytes()),
label_signature: None,
};
s.sign_label(skey);
Expand All @@ -337,28 +348,34 @@ pub struct SecretKeyEntry {
impl SecretKeyEntry {
pub fn new(label: &str, roles: Vec<SignatureRole>) -> Self {
let mut rng = rand::rngs::OsRng {};
let rawkey = Keypair::generate(&mut rng);
let keypair = base64::encode(rawkey.to_bytes());
let rawkey = SigningKey::generate(&mut rng);
let keypair = base64::engine::general_purpose::STANDARD.encode(rawkey.to_keypair_bytes());
Self {
label: label.to_owned(),
keypair,
roles,
}
}

pub(crate) fn key(&self) -> Result<Keypair, SignatureError> {
let rawbytes = base64::decode(&self.keypair).map_err(|_e| {
pub(crate) fn key(&self) -> Result<SigningKey, SignatureError> {
let rawbytes = base64::engine::general_purpose::STANDARD
.decode(&self.keypair)
.map_err(|_e| {
// We swallow the source error because it could disclose information about
// the secret key.
SignatureError::CorruptKey("Base64 decoding of the keypair failed".to_owned())
})?;
let rawbytes = rawbytes.try_into().map_err(|_e| {
// We swallow the source error because it could disclose information about
// the secret key.
SignatureError::CorruptKey("Base64 decoding of the keypair failed".to_owned())
SignatureError::CorruptKey("Invalid keypair length".to_owned())
})?;
let keypair = Keypair::from_bytes(&rawbytes).map_err(|e| {
SigningKey::from_keypair_bytes(&rawbytes).map_err(|e| {
tracing::log::error!("Error loading key: {}", e);
// Don't leak information about the key, because this could be sent to
// a remote. A generic error is all the user should see.
SignatureError::CorruptKey("Could not load keypair".to_owned())
})?;
Ok(keypair)
})
}
}

Expand Down Expand Up @@ -481,7 +498,7 @@ impl SecretKeyStorage for SecretKeyFile {
#[cfg(test)]
mod test {
use super::*;
use ed25519_dalek::Keypair;
use ed25519_dalek::SigningKey;

#[test]
fn test_parse_role() {
Expand All @@ -508,7 +525,7 @@ mod test {
#[test]
fn test_sign_label() {
let mut rng = rand::rngs::OsRng {};
let keypair = Keypair::generate(&mut rng);
let keypair = SigningKey::generate(&mut rng);

let mut ke = KeyEntry {
label: "Matt Butcher <[email protected]>".to_owned(),
Expand All @@ -517,7 +534,7 @@ mod test {
label_signature: None,
};

let pubkey = keypair.public;
let pubkey = keypair.verifying_key();
ke.sign_label(keypair);

assert!(ke.label_signature.is_some());
Expand Down
18 changes: 11 additions & 7 deletions src/invoice/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use crate::invoice::Signed;

use super::signature::KeyRing;
use super::{Invoice, Signature, SignatureError, SignatureRole};
use ed25519_dalek::{PublicKey, Signature as EdSignature};
use base64::Engine;
use ed25519_dalek::{Signature as EdSignature, VerifyingKey};
use tracing::debug;

use std::borrow::{Borrow, BorrowMut};
Expand Down Expand Up @@ -136,13 +137,15 @@ fn parse_roles(r: Option<&&str>) -> Result<Vec<SignatureRole>, &'static str> {
/// A strategy for verifying an invoice.
impl VerificationStrategy {
fn verify_signature(&self, sig: &Signature, cleartext: &[u8]) -> Result<(), SignatureError> {
let pk = base64::decode(sig.key.as_bytes())
let pk = base64::engine::general_purpose::STANDARD
.decode(sig.key.as_bytes())
.map_err(|_| SignatureError::CorruptKey(sig.key.clone()))?;
let sig_block = base64::decode(sig.signature.as_bytes())
let sig_block = base64::engine::general_purpose::STANDARD
.decode(sig.signature.as_bytes())
.map_err(|_| SignatureError::CorruptSignature(sig.key.clone()))?;

let pubkey =
PublicKey::from_bytes(&pk).map_err(|_| SignatureError::CorruptKey(sig.key.clone()))?;
let pubkey = VerifyingKey::try_from(pk.as_slice())
.map_err(|_| SignatureError::CorruptKey(sig.key.clone()))?;
let ed_sig = EdSignature::try_from(sig_block.as_slice())
.map_err(|_| SignatureError::CorruptSignature(sig.key.clone()))?;
pubkey
Expand Down Expand Up @@ -231,9 +234,10 @@ impl VerificationStrategy {
filled_roles.push(role);
}
// See if the public key is known to us
let pubkey = base64::decode(&s.key)
let pubkey = base64::engine::general_purpose::STANDARD
.decode(&s.key)
.map_err(|_| SignatureError::CorruptKey(s.key.to_string()))?;
let pko = PublicKey::from_bytes(pubkey.as_slice())
let pko = VerifyingKey::try_from(pubkey.as_slice())
.map_err(|_| SignatureError::CorruptKey(s.key.to_string()))?;

debug!("Looking for key");
Expand Down
Loading

0 comments on commit 7672acf

Please sign in to comment.