diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index 1ef81dfb6..c45fa62d5 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/crates/matrix-sdk-crypto/src/error.rs @@ -287,6 +287,10 @@ pub enum SignatureError { #[error(transparent)] InvalidKey(#[from] vodozemac::KeyError), + /// The message could not be signed with X.509 + #[error(transparent)] + X509SigningError(#[from] rustls::Error), + /// The signature could not be decoded. #[error("the given signature is not valid and can't be decoded")] InvalidSignature, diff --git a/crates/matrix-sdk-crypto/src/x509/rust_x509_sign.rs b/crates/matrix-sdk-crypto/src/x509/rust_x509_sign.rs index 9326065f0..a5bf7a1f0 100644 --- a/crates/matrix-sdk-crypto/src/x509/rust_x509_sign.rs +++ b/crates/matrix-sdk-crypto/src/x509/rust_x509_sign.rs @@ -21,6 +21,7 @@ use rustls::{ pki_types::{CertificateDer, PrivateKeyDer, pem::PemObject}, sign::SigningKey, }; +use thiserror::Error; use vodozemac::base64_encode; use crate::{SignatureError, types::X509Signature, x509::x509_signer::X509Sign}; @@ -40,29 +41,51 @@ pub struct RustX509Sign { signing_key: Arc, } +#[derive(Error, Debug)] +pub enum RustX509SignError { + /// There was an error parsing the certificate chain. + #[error("failed to parse certificate chain {0}")] + CertificateParseError(rustls::pki_types::pem::Error), + + /// No certificates were found. + #[error("no certificates found in chain")] + CertificateNotFoundError, + + /// There was an error parsing the private key. + #[error("failed to parse private key {0}")] + PrivateKeyParseError(rustls::pki_types::pem::Error), + + /// There was an error loading the private key. + #[error("failed to load private key {0}")] + PrivateKeyLoadError(rustls::Error), +} + impl RustX509Sign { /// Create a new `RustX509Sign` from the supplied PEM data. - pub fn new_from_pem_data(certificate_chain_pem: &str, private_key_pem: &str) -> Self { + pub fn new_from_pem_data( + certificate_chain_pem: &str, + private_key_pem: &str, + ) -> Result { let provider = aws_lc_rs::default_provider(); let cert_iter = CertificateDer::pem_slice_iter(certificate_chain_pem.as_bytes()); let last_cert = cert_iter .last() - .expect("unable to parse certificate chain") - .expect("no certificates found in chain"); + .ok_or(RustX509SignError::CertificateNotFoundError)? + .map_err(|e| RustX509SignError::CertificateParseError(e))?; let last_cert_aki = get_authority_key_identifier(&last_cert).expect("no AKI found in last cert"); let device_id = DeviceKeyId::from_parts("io.element.x509".into(), last_cert_aki.as_str().into()); let private_key = PrivateKeyDer::from_pem_slice(private_key_pem.as_bytes()) - .expect("unable to parse private key"); + .map_err(|e| RustX509SignError::PrivateKeyParseError(e))?; let signing_key: Arc = provider .key_provider .load_private_key(private_key) - .expect("unable to load private key"); + .map_err(|e| RustX509SignError::PrivateKeyLoadError(e))?; - Self { certificate_chain: certificate_chain_pem.to_owned(), device_id, signing_key } + Ok(Self { certificate_chain: certificate_chain_pem.to_owned(), device_id, signing_key }) } } @@ -71,15 +94,13 @@ impl X509Sign for RustX509Sign { /// /// Returns (key ID, signature) fn sign(&self, message: &[u8]) -> Result<(OwnedDeviceKeyId, X509Signature), SignatureError> { - // TODO RAV: error handling - let signature_scheme = SignatureScheme::RSA_PSS_SHA512; let signer = self .signing_key .choose_scheme(&[signature_scheme]) - .expect("unable to choose signature scheme"); + .ok_or(SignatureError::UnsupportedAlgorithm)?; - let signature = signer.sign(message).expect("unable to sign"); + let signature = signer.sign(message).map_err(|e| SignatureError::X509SigningError(e))?; Ok(( self.device_id.clone(), X509Signature { @@ -146,7 +167,7 @@ mod tests { #[test] fn can_sign() { - let x509_sign = RustX509Sign::new_from_pem_data(TEST_CERT_CHAIN, TEST_CERT_KEY); + let x509_sign = RustX509Sign::new_from_pem_data(TEST_CERT_CHAIN, TEST_CERT_KEY).unwrap(); let (key_id, sig) = x509_sign.sign(b"hello world").unwrap(); diff --git a/crates/matrix-sdk-crypto/src/x509/rust_x509_verify.rs b/crates/matrix-sdk-crypto/src/x509/rust_x509_verify.rs index f376ff9ca..97be13fd4 100644 --- a/crates/matrix-sdk-crypto/src/x509/rust_x509_verify.rs +++ b/crates/matrix-sdk-crypto/src/x509/rust_x509_verify.rs @@ -18,7 +18,7 @@ use rustls::{ RootCertStore, crypto::CryptoProvider, pki_types::{CertificateDer, UnixTime, pem::PemObject}, - server::{WebPkiClientVerifier, danger::ClientCertVerifier}, + server::{VerifierBuilderError, WebPkiClientVerifier, danger::ClientCertVerifier}, }; use vodozemac::base64_decode; use webpki::EndEntityCert; @@ -34,7 +34,7 @@ pub struct RustX509Verify { /// (using `rustls`) rather than delegating the work to some external system. impl RustX509Verify { /// Create a new `RustX509Verify` from the supplied CA certificates PEM. - pub fn new_from_pem_data(ca_certs_pem: &str) -> Self { + pub fn new_from_pem_data(ca_certs_pem: &str) -> Result { let mut root_store = RootCertStore::empty(); for result in CertificateDer::pem_slice_iter(ca_certs_pem.as_bytes()) { root_store @@ -43,10 +43,9 @@ impl RustX509Verify { } let verifier = WebPkiClientVerifier::builder(Arc::new(root_store)) //.with_crls(...) - .build() - .unwrap(); + .build()?; - Self { verifier } + Ok(Self { verifier }) } } @@ -96,8 +95,10 @@ impl X509Verify for RustX509Verify { }; // TODO: AJB: make it harder to forget to base64 encode/decode this? - let signature_bin = - base64_decode(&sig.signature).expect("Failed to decode base64 signature"); + let Ok(signature_bin) = base64_decode(&sig.signature) else { + tracing::warn!("Failed to base64-decode the signaturea"); + return false; + }; let result = cert.verify_signature(alg, message, &signature_bin); if let Err(e) = result { diff --git a/crates/matrix-sdk-crypto/src/x509/x509_data.rs b/crates/matrix-sdk-crypto/src/x509/x509_data.rs index b1835678f..1035436ce 100644 --- a/crates/matrix-sdk-crypto/src/x509/x509_data.rs +++ b/crates/matrix-sdk-crypto/src/x509/x509_data.rs @@ -36,14 +36,14 @@ impl X509Data { // TODO: it would be sensible to validate that the private key matches the // certificate chain here, to catch configuration errors early. - X509Data { - x509_signer: Some(X509Signer::new(Arc::new(RustX509Sign::new_from_pem_data( - cert_chain_pem, - private_key_pem, - )))), - x509_verifier: Some(X509Verifier::new(Arc::new(RustX509Verify::new_from_pem_data( - ca_certs_pem, - )))), - } + let x509_signer = RustX509Sign::new_from_pem_data(cert_chain_pem, private_key_pem) + .map(|signer| X509Signer::new(Arc::new(signer))) + .ok(); + + let x509_verifier = RustX509Verify::new_from_pem_data(ca_certs_pem) + .map(|verifier| X509Verifier::new(Arc::new(verifier))) + .ok(); + + X509Data { x509_signer, x509_verifier } } }