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

build: update ed25519-dalek and base64 #368

Merged
merged 2 commits into from
Aug 29, 2023
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
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 <matt@example.com>".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
Loading