diff --git a/.gitignore b/.gitignore index 7f1d8f3..23455a6 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /.direnv /result **/.DS_Store +.idea diff --git a/Cargo.lock b/Cargo.lock index dcd8941..33f4b2f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -748,7 +748,7 @@ checksum = "eded382c5f5f786b989652c49544c4877d9f015cc22e145a5ea8ea66c2921cd2" [[package]] name = "samael" -version = "0.0.19" +version = "0.0.20" dependencies = [ "base64", "bindgen", diff --git a/Cargo.toml b/Cargo.toml index 672d8d5..3707699 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "samael" -version = "0.0.19" +version = "0.0.20" authors = ["Nathan Jaremko "] edition = "2021" license = "MIT" diff --git a/README.md b/README.md index 5c987aa..c766c36 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ You'll need these dependencies for this example ```toml [dependencies] tokio = { version = "1.28.1", features = ["full"] } -samael = { version = "0.0.12", features = ["xmlsec"] } +samael = { version = "0.0.20", features = ["xmlsec"] } warp = "0.3.5" reqwest = "0.11.18" openssl = "0.10.52" diff --git a/bindings.h b/bindings.h index 9a88429..0c25042 100644 --- a/bindings.h +++ b/bindings.h @@ -7,5 +7,6 @@ #include #include #include +#include #include #include diff --git a/bindings.rs b/bindings.rs index a57a1e5..a54e573 100644 --- a/bindings.rs +++ b/bindings.rs @@ -15,6 +15,7 @@ fn main() { // Tell the compiler about our custom cfg flags println!("cargo:rustc-check-cfg=cfg(xmlsec_dynamic)"); println!("cargo:rustc-check-cfg=cfg(xmlsec_static)"); + println!("cargo:rerun-if-changed=bindings.h"); if env::var_os("CARGO_FEATURE_XMLSEC").is_some() { let path_out = PathBuf::from(env::var("OUT_DIR").unwrap()); @@ -41,24 +42,22 @@ fn main() { println!("cargo:rustc-link-lib=ssl"); // -lssl println!("cargo:rustc-link-lib=crypto"); // -lcrypto - if !path_bindings.exists() { - PkgConfig::new() - .probe("xmlsec1") - .expect("Could not find xmlsec1 using pkg-config"); + PkgConfig::new() + .probe("xmlsec1") + .expect("Could not find xmlsec1 using pkg-config"); - let bindbuild = BindgenBuilder::default() - .header("bindings.h") - .clang_args(cflags) - .clang_args(fetch_xmlsec_config_libs()) - .layout_tests(true) - .generate_comments(true); + let bindbuild = BindgenBuilder::default() + .header("bindings.h") + .clang_args(cflags) + .clang_args(fetch_xmlsec_config_libs()) + .layout_tests(true) + .generate_comments(true); - let bindings = bindbuild.generate().expect("Unable to generate bindings"); + let bindings = bindbuild.generate().expect("Unable to generate bindings"); - bindings - .write_to_file(path_bindings) - .expect("Couldn't write bindings!"); - } + bindings + .write_to_file(path_bindings) + .expect("Couldn't write bindings!"); } } diff --git a/flake.lock b/flake.lock index 93d4215..1785152 100644 --- a/flake.lock +++ b/flake.lock @@ -3,11 +3,11 @@ "advisory-db": { "flake": false, "locked": { - "lastModified": 1735928634, - "narHash": "sha256-Qg1vJOuEohAbdRmTTOLrbbGsyK9KRB54r3+aBuOMctM=", + "lastModified": 1773786698, + "narHash": "sha256-o/J7ZculgwSs1L4H4UFlFZENOXTJzq1X0n71x6oNNvY=", "owner": "rustsec", "repo": "advisory-db", - "rev": "63a2f39924f66ca89cf5761f299a8a244fe02543", + "rev": "99e9de91bb8b61f06ef234ff84e11f758ecd5384", "type": "github" }, "original": { @@ -18,11 +18,11 @@ }, "crane": { "locked": { - "lastModified": 1736898272, - "narHash": "sha256-D10wlrU/HCpSRcb3a7yk+bU3ggpMD1kGbseKtO+7teo=", + "lastModified": 1773189535, + "narHash": "sha256-E1G/Or6MWeP+L6mpQ0iTFLpzSzlpGrITfU2220Gq47g=", "owner": "ipetkov", "repo": "crane", - "rev": "6a589f034202a7c6e10bce6c5d1d392d7bc0f340", + "rev": "6fa2fb4cf4a89ba49fc9dd5a3eb6cde99d388269", "type": "github" }, "original": { @@ -51,11 +51,11 @@ }, "nix-filter": { "locked": { - "lastModified": 1731533336, - "narHash": "sha256-oRam5PS1vcrr5UPgALW0eo1m/5/pls27Z/pabHNy2Ms=", + "lastModified": 1757882181, + "narHash": "sha256-+cCxYIh2UNalTz364p+QYmWHs0P+6wDhiWR4jDIKQIU=", "owner": "numtide", "repo": "nix-filter", - "rev": "f7653272fd234696ae94229839a99b73c9ab7de0", + "rev": "59c44d1909c72441144b93cf0f054be7fe764de5", "type": "github" }, "original": { @@ -66,16 +66,16 @@ }, "nixpkgs": { "locked": { - "lastModified": 1736867362, - "narHash": "sha256-i/UJ5I7HoqmFMwZEH6vAvBxOrjjOJNU739lnZnhUln8=", + "lastModified": 1773705440, + "narHash": "sha256-xB30bbAp0e7ogSEYyc126mAJMt4FRFh8wtm6ADE1xuM=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "9c6b49aeac36e2ed73a8c472f1546f6d9cf1addc", + "rev": "48652e9d5aea46e555b3df87354280d4f29cd3a3", "type": "github" }, "original": { "owner": "NixOS", - "ref": "nixos-24.11", + "ref": "nixos-25.11", "repo": "nixpkgs", "type": "github" } @@ -97,11 +97,11 @@ ] }, "locked": { - "lastModified": 1736907983, - "narHash": "sha256-fw55wVwpJW36Md2HZBKuxX3YHGeqsGsspPLtCMVr1Y8=", + "lastModified": 1773803479, + "narHash": "sha256-GD6i1F2vrSxbsmbS92+8+x3DbHOJ+yrS78Pm4xigW4M=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "eaa365c911441e07e387ff6acc596619fc50b156", + "rev": "f17186f52e82ec5cf40920b58eac63b78692ac7c", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index 3fa0f48..b07b971 100644 --- a/flake.nix +++ b/flake.nix @@ -1,6 +1,6 @@ { inputs = { - nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.11"; + nixpkgs.url = "github:NixOS/nixpkgs/nixos-25.11"; flake-utils.url = "github:numtide/flake-utils"; nix-filter.url = "github:numtide/nix-filter"; rust-overlay = { diff --git a/src/crypto/cert_encoding.rs b/src/crypto/cert_encoding.rs index ce7d061..9d0bae4 100644 --- a/src/crypto/cert_encoding.rs +++ b/src/crypto/cert_encoding.rs @@ -1,5 +1,5 @@ -use base64::{engine::general_purpose, Engine as _}; use crate::crypto::CertificateDer; +use base64::{engine::general_purpose, Engine as _}; // strip out 76-width format and decode base64 pub fn decode_x509_cert(x509_cert: &str) -> Result { @@ -10,7 +10,9 @@ pub fn decode_x509_cert(x509_cert: &str) -> Result>(); - general_purpose::STANDARD.decode(stripped).map(|data| data.into()) + general_purpose::STANDARD + .decode(stripped) + .map(|data| data.into()) } // 76-width base64 encoding (MIME) diff --git a/src/crypto/crypto_disabled.rs b/src/crypto/crypto_disabled.rs index 55c7457..63010dc 100644 --- a/src/crypto/crypto_disabled.rs +++ b/src/crypto/crypto_disabled.rs @@ -1,6 +1,6 @@ //! This module provides the behaviour if no crypto is available. -use crate::crypto::{CryptoError, CryptoProvider, CertificateDer}; +use crate::crypto::{CertificateDer, CryptoError, CryptoProvider, ReduceMode}; use crate::schema::CipherValue; pub struct NoCrypto; @@ -16,9 +16,12 @@ impl CryptoProvider for NoCrypto { Ok(()) } - fn reduce_xml_to_signed(_xml_str: &str, _certs: &[CertificateDer]) -> Result { - // Since we cannot verify anything. Return empty. - Ok(String::new()) + fn reduce_xml_to_signed( + _xml_str: &str, + _certs: &[CertificateDer], + _reduce_mode: ReduceMode, + ) -> Result { + Err(CryptoError::CryptoDisabled) } fn decrypt_assertion_key_info( @@ -44,3 +47,16 @@ impl CryptoProvider for NoCrypto { Err(CryptoError::CryptoDisabled) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reduce_xml_to_signed_fails_closed_when_crypto_is_disabled() { + let error = NoCrypto::reduce_xml_to_signed("", &[], ReduceMode::ValidateAndMark) + .expect_err("signature reduction should fail when crypto support is disabled"); + + assert!(matches!(error, CryptoError::CryptoDisabled)); + } +} diff --git a/src/crypto/mod.rs b/src/crypto/mod.rs index fb54d4a..a4ff692 100644 --- a/src/crypto/mod.rs +++ b/src/crypto/mod.rs @@ -9,7 +9,7 @@ use crate::schema::CipherValue; pub use cert_encoding::*; pub use ids::*; use thiserror::Error; -pub use url_verification::{UrlVerifier, UrlVerifierError, sign_url}; +pub use url_verification::{sign_url, UrlVerifier, UrlVerifierError}; #[cfg(feature = "xmlsec")] pub use xmlsec::*; @@ -61,9 +61,34 @@ impl From> for CertificateDer { } } +/// Defines which algorithm is used to reduce signed XML. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReduceMode { + /// Returns xmlsec's pre-digest content for exactly one verified reference across the document. + PreDigest, + /// Legacy mode that preserves the verified content and every element ancestor up to the + /// document root. + /// + /// This is kept for compatibility with older callers. It is not the default because unsigned + /// ancestors can survive reduction in this mode. + ValidateAndMark, + /// Returns a rooted XML document containing only xmlsec-verified content. + /// + /// If the verified reference is a full element, that element becomes the output root. If the + /// verified reference reduces to a child sequence, the referenced element is retained only as a + /// stripped shell so the verified descendants remain rooted. + ValidateAndMarkNoAncestors, +} + +impl Default for ReduceMode { + fn default() -> Self { + Self::ValidateAndMarkNoAncestors + } +} + pub trait CryptoProvider { type PrivateKey; - + fn verify_signed_xml>( xml: Bytes, x509_cert_der: &CertificateDer, @@ -71,11 +96,15 @@ pub trait CryptoProvider { ) -> Result<(), CryptoError>; /// Takes an XML document, parses it, verifies all XML digital signatures against the given - /// certificates, and returns a derived version of the document where all elements that are not - /// covered by a digital signature have been removed. + /// certificates, and returns output according to `reduce_mode`. + /// + /// `ReduceMode::PreDigest` returns xmlsec's verified pre-digest payload for exactly one + /// reference. The validate modes return a rooted XML document derived from the original input + /// with all unsigned content removed. fn reduce_xml_to_signed( xml_str: &str, certs_der: &[CertificateDer], + reduce_mode: ReduceMode, ) -> Result; fn decrypt_assertion_key_info( diff --git a/src/crypto/url_verification.rs b/src/crypto/url_verification.rs index 6539b7c..ed839e2 100644 --- a/src/crypto/url_verification.rs +++ b/src/crypto/url_verification.rs @@ -1,12 +1,12 @@ +use crate::crypto::CertificateDer; use crate::signature::SignatureAlgorithm; use base64::{engine::general_purpose, Engine as _}; -use std::collections::HashMap; -use std::str::FromStr; use openssl::error::ErrorStack; use openssl::pkey::{PKey, Private}; +use std::collections::HashMap; +use std::str::FromStr; use thiserror::Error; use url::Url; -use crate::crypto::CertificateDer; #[derive(Debug, Error, Clone)] pub enum UrlVerifierError { @@ -17,7 +17,7 @@ pub enum UrlVerifierError { #[error("No query to sign")] NoQueryToSign, #[error("Signing error")] - SigningError(#[from] ErrorStack) + SigningError(#[from] ErrorStack), } pub struct UrlVerifier { @@ -55,9 +55,7 @@ impl UrlVerifier { Ok(Self { public_key }) } - pub fn from_x509( - public_cert: &CertificateDer, - ) -> Result> { + pub fn from_x509(public_cert: &CertificateDer) -> Result> { let public_cert = openssl::x509::X509::from_der(public_cert.der_data())?; let public_key = public_cert.public_key()?; Ok(Self { public_key }) @@ -195,8 +193,10 @@ impl UrlVerifier { } } - -pub fn sign_url(mut unsigned_url: Url, private_key: &PKey) -> Result { +pub fn sign_url( + mut unsigned_url: Url, + private_key: &PKey, +) -> Result { // Refer to section 3.4.4.1 (page 17) of // // https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf @@ -230,7 +230,6 @@ pub fn sign_url(mut unsigned_url: Url, private_key: &PKey) -> Result, }, - #[error("failed to define namespace: {}", error)] - XmlNamespaceDefinitionError { - #[source] - error: Box, - }, + #[error("encountered malformed XML ID attribute value: {id}")] + XmlInvalidIdAttribute { id: String }, + + #[error("encountered duplicate XML ID attribute value: {id}")] + XmlDuplicateIdAttribute { id: String }, + + #[error("failed to register XML ID attribute value: {id}")] + XmlIdRegistrationError { id: String }, + + #[error("failed to resolve verified reference URI: {uri}")] + XmlReferenceResolutionError { uri: String }, + + #[error("verified pre-digest fragment could not be matched back to the source document")] + XmlPredigestFragmentInvalid, #[error("OpenSSL error stack: {}", error)] OpenSSLError { @@ -87,6 +95,32 @@ impl From for CryptoError { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum VerifiedSelectionKind { + WholeNode, + ChildSequence, +} + +#[derive(Debug, Clone)] +struct VerifiedSelection { + anchor: libxml::tree::Node, + kind: VerifiedSelectionKind, + uri: String, + predigest_xml: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +struct AttributeName { + local_name: String, + namespace: Option, +} + +struct ParsedSelection { + _document: libxml::tree::Document, + shell_source: Option, + sequence_nodes: Vec, +} + pub struct XmlSec; impl super::CryptoProvider for XmlSec { @@ -118,63 +152,86 @@ impl super::CryptoProvider for XmlSec { fn reduce_xml_to_signed( xml_str: &str, certs_der: &[CertificateDer], + reduce_mode: ReduceMode, ) -> Result { let mut xml = XmlParser::default().parse_string(xml_str)?; - let mut root_elem = xml - .get_root_element() - .ok_or(XmlSecProviderError::XmlMissingRootElement)?; // collect ID attribute values and tell libxml about them collect_id_attributes(&mut xml)?; - // verify each signature - { - let mut signature_nodes = find_signature_nodes(&root_elem); - for sig_node in signature_nodes.drain(..) { - let mut verified = false; - for key_data in certs_der { - let mut sig_ctx = XmlSecSignatureContext::new()?; - let key = XmlSecKey::from_memory(key_data.der_data(), XmlSecKeyFormat::CertDer)?; - sig_ctx.insert_key(key); - verified = sig_ctx.verify_node(&sig_node)?; - if verified { - break; - } + let signature_nodes = { + let root_elem = xml + .get_root_element() + .ok_or(XmlSecProviderError::XmlMissingRootElement)?; + find_signature_nodes(&root_elem) + }; + if signature_nodes.is_empty() { + return Err(CryptoError::InvalidSignature); + } + + let mut predigest_results = Vec::new(); + let mut selections = Vec::new(); + + for sig_node in &signature_nodes { + let mut verified = false; + let mut verified_references = Vec::new(); + + for key_data in certs_der { + let mut sig_ctx = XmlSecSignatureContext::new_with_flags( + wrapper::XMLSEC_DSIG_FLAGS_STORE_SIGNEDINFO_REFERENCES, + )?; + let key = XmlSecKey::from_memory(key_data.der_data(), XmlSecKeyFormat::CertDer)?; + sig_ctx.insert_key(key); + verified = sig_ctx.verify_node(sig_node)?; + if verified { + verified_references = sig_ctx.get_verified_references()?; + break; } + } + + if !verified { + return Err(CryptoError::InvalidSignature); + } - if !verified { - return Err(CryptoError::InvalidSignature); + for verified_reference in verified_references { + if reduce_mode == ReduceMode::PreDigest { + predigest_results.push(verified_reference.predigest_xml); + continue; } + + selections.push(build_verified_selection( + &xml, + sig_node, + verified_reference, + )?); } } - // define the "signature verified" namespace - let sig_ver_ns = libxml::tree::Namespace::new("sv", XMLNS_SIGVER, &mut root_elem) - .map_err(|err| XmlSecProviderError::XmlNamespaceDefinitionError { error: err })?; + if reduce_mode == ReduceMode::PreDigest { + if predigest_results.len() != 1 { + return Err(CryptoError::InvalidSignature); + } + return Ok(predigest_results.remove(0)); + } + + if selections.is_empty() { + return Err(CryptoError::InvalidSignature); + } + + let (nodes_to_keep, allowed_attributes, output_root_ptr) = + build_keep_state(&xml, &selections, reduce_mode)?; - // remove all existing "signature verified" attributes - // (we can't do this before verifying the signatures: - // they might be contained in the XML document proper and signed) - remove_signature_verified_attributes(&mut root_elem)?; + drop(selections); + drop(signature_nodes); - // place the "signature verified" attributes on all elements that are: - // * signed - // * a descendant of a signed element - // * an ancestor of a signed element - place_signature_verified_attributes(root_elem, &xml, &sig_ver_ns); + move_output_root_to_document_root(&mut xml, output_root_ptr)?; - // delete all elements that don't have a "signature verified" attribute let mut root_elem = xml .get_root_element() .ok_or(XmlSecProviderError::XmlMissingRootElement)?; - remove_unverified_elements(&mut root_elem); + prune_unverified_nodes(&mut root_elem, &nodes_to_keep, &allowed_attributes)?; - // remove all "signature verified" attributes again - remove_signature_verified_attributes(&mut root_elem)?; - - // serialize XML again - let reduced_xml_str = xml.to_string(); - Ok(reduced_xml_str) + Ok(xml.to_string()) } fn decrypt_assertion_key_info( @@ -273,6 +330,7 @@ impl super::CryptoProvider for XmlSec { fn collect_id_attributes(doc: &mut libxml::tree::Document) -> Result<(), XmlSecProviderError> { const ID_STR: &str = "ID"; let id_attr_name = CString::new(ID_STR).unwrap(); + let mut seen_ids = HashSet::new(); let mut nodes_to_visit = Vec::new(); if let Some(root_elem) = doc.get_root_element() { @@ -280,18 +338,33 @@ fn collect_id_attributes(doc: &mut libxml::tree::Document) -> Result<(), XmlSecP } while let Some(node) = nodes_to_visit.pop() { if let Some(id_value) = node.get_attribute(ID_STR) { - let id_value_cstr = CString::new(id_value).unwrap(); + let id_value_cstr = CString::new(id_value.clone()).map_err(|_| { + XmlSecProviderError::XmlInvalidIdAttribute { + id: id_value.clone(), + } + })?; + if !xml_id_value_is_valid(&id_value_cstr) { + return Err(XmlSecProviderError::XmlInvalidIdAttribute { id: id_value }); + } + if !seen_ids.insert(id_value.clone()) { + return Err(XmlSecProviderError::XmlDuplicateIdAttribute { id: id_value }); + } + let node_ptr = node.node_ptr(); unsafe { let attr = libxml::bindings::xmlHasProp(node_ptr, id_attr_name.as_ptr() as *const u8); - assert!(!attr.is_null()); - libxml::bindings::xmlAddID( - std::ptr::null_mut(), - doc.doc_ptr(), - id_value_cstr.as_ptr() as *const u8, - attr, - ); + if attr.is_null() + || libxml::bindings::xmlAddID( + std::ptr::null_mut(), + doc.doc_ptr(), + id_value_cstr.as_ptr() as *const u8, + attr, + ) + .is_null() + { + return Err(XmlSecProviderError::XmlIdRegistrationError { id: id_value }); + } } } @@ -321,218 +394,818 @@ fn find_signature_nodes(node: &libxml::tree::Node) -> Vec { ret } -/// Removes all signature-verified attributes ([`ATTRIB_SIGVER`] in the namespace [`XMLNS_SIGVER`]) -/// from all elements in the subtree rooted at the given node. -fn remove_signature_verified_attributes( - node: &mut libxml::tree::Node, -) -> Result<(), XmlSecProviderError> { - node.remove_attribute_ns(ATTRIB_SIGVER, XMLNS_SIGVER) - .map_err(|err| XmlSecProviderError::XmlAttributeRemovalError { error: err })?; - for mut child_elem in node.get_child_elements() { - remove_signature_verified_attributes(&mut child_elem)?; +struct XPathContext { + pointer: NonNull, +} + +impl XPathContext { + fn new(doc: &libxml::tree::Document, signature_node: &libxml::tree::Node) -> Option { + let pointer = unsafe { + libxml::bindings::xmlXPtrNewContext( + doc.doc_ptr(), + signature_node.node_ptr(), + std::ptr::null_mut(), + ) + }; + NonNull::new(pointer).map(|pointer| Self { pointer }) + } +} + +impl Drop for XPathContext { + fn drop(&mut self) { + unsafe { libxml::bindings::xmlXPathFreeContext(self.pointer.as_ptr()) } + } +} + +struct XPathObject { + pointer: NonNull, +} + +impl XPathObject { + fn evaluate(context: &XPathContext, xpointer: &CString) -> Option { + let pointer = unsafe { + libxml::bindings::xmlXPtrEval( + xpointer.as_ptr() as *const libxml::bindings::xmlChar, + context.pointer.as_ptr(), + ) + }; + NonNull::new(pointer).map(|pointer| Self { pointer }) } +} + +impl Drop for XPathObject { + fn drop(&mut self) { + unsafe { libxml::bindings::xmlXPathFreeObject(self.pointer.as_ptr()) } + } +} + +fn xml_id_value_is_valid(id_value: &CStr) -> bool { + unsafe { libxml::bindings::xmlValidateNCName(id_value.as_ptr() as *const u8, 0) == 0 } +} + +fn build_verified_selection( + doc: &libxml::tree::Document, + signature_node: &libxml::tree::Node, + verified_reference: VerifiedReference, +) -> Result { + let reference_uri = verified_reference.uri.clone().unwrap_or_default(); + let (anchor, _kind) = + resolve_reference_anchor(verified_reference.uri.as_deref(), doc, signature_node).ok_or( + XmlSecProviderError::XmlReferenceResolutionError { + uri: reference_uri.clone(), + }, + )?; + let parsed_selection = parse_selection(&anchor, &verified_reference.predigest_xml)?; + + Ok(VerifiedSelection { + anchor, + kind: if parsed_selection.shell_source.is_some() { + VerifiedSelectionKind::WholeNode + } else { + VerifiedSelectionKind::ChildSequence + }, + uri: reference_uri, + predigest_xml: verified_reference.predigest_xml, + }) +} + +fn parse_selection( + anchor: &libxml::tree::Node, + predigest_xml: &str, +) -> Result { + let wrapped_fragment = format!("{predigest_xml}"); + let fragment_doc = XmlParser::default().parse_string(&wrapped_fragment)?; + let fragment_root = fragment_doc + .get_root_element() + .ok_or(XmlSecProviderError::XmlMissingRootElement)?; + let fragment_children = supported_fragment_children(&fragment_root); + + if fragment_children.is_empty() { + return Err(XmlSecProviderError::XmlPredigestFragmentInvalid.into()); + } + + let shell_source = if fragment_children.len() == 1 + && fragment_children[0].is_element_node() + && anchor.is_element_node() + && element_identity_matches(anchor, &fragment_children[0]) + { + Some(fragment_children[0].clone()) + } else { + None + }; + let sequence_nodes = if let Some(shell_source) = &shell_source { + supported_fragment_children(shell_source) + } else { + fragment_children + }; + + Ok(ParsedSelection { + _document: fragment_doc, + shell_source, + sequence_nodes, + }) +} + +fn build_keep_state( + doc: &libxml::tree::Document, + selections: &[VerifiedSelection], + reduce_mode: ReduceMode, +) -> Result< + ( + HashSet, + HashMap>, + usize, + ), + CryptoError, +> { + if selections.is_empty() { + return Err(CryptoError::InvalidSignature); + } + + let mut nodes_to_keep = HashSet::new(); + let mut allowed_attributes = HashMap::new(); + + for selection in selections { + let parsed_selection = parse_selection(&selection.anchor, &selection.predigest_xml)?; + let matched = match selection.kind { + VerifiedSelectionKind::WholeNode => { + let shell_source = parsed_selection + .shell_source + .as_ref() + .ok_or(XmlSecProviderError::XmlPredigestFragmentInvalid)?; + collect_matching_node( + &selection.anchor, + shell_source, + &mut nodes_to_keep, + &mut allowed_attributes, + ) + } + VerifiedSelectionKind::ChildSequence => collect_matching_sequence( + &supported_fragment_children(&selection.anchor), + &parsed_selection.sequence_nodes, + &mut nodes_to_keep, + &mut allowed_attributes, + ), + }; + + if !matched { + return Err(XmlSecProviderError::XmlReferenceResolutionError { + uri: selection.uri.clone(), + } + .into()); + } + + if selection.kind == VerifiedSelectionKind::ChildSequence { + let anchor_ptr = selection.anchor.node_ptr() as usize; + nodes_to_keep.insert(anchor_ptr); + // The anchor shell is retained only to keep the verified child sequence rooted. + allowed_attributes.entry(anchor_ptr).or_default(); + } + } + + if nodes_to_keep.is_empty() { + return Err(XmlSecProviderError::XmlPredigestFragmentInvalid.into()); + } + + let root = doc + .get_root_element() + .ok_or(XmlSecProviderError::XmlMissingRootElement)?; + if reduce_mode == ReduceMode::ValidateAndMark { + let matched_nodes = nodes_to_keep.clone(); + add_required_ancestors( + &root, + &matched_nodes, + &mut nodes_to_keep, + &mut allowed_attributes, + )?; + } + let output_root_ptr = determine_output_root_ptr(&root, &nodes_to_keep)?; + Ok((nodes_to_keep, allowed_attributes, output_root_ptr)) +} + +fn supported_fragment_children(node: &libxml::tree::Node) -> Vec { + node.get_child_nodes() + .into_iter() + .filter(is_supported_fragment_node) + .collect() +} + +fn is_supported_fragment_node(node: &libxml::tree::Node) -> bool { + matches!( + node.get_type(), + Some( + NodeType::ElementNode + | NodeType::TextNode + | NodeType::CDataSectionNode + | NodeType::CommentNode + | NodeType::PiNode + ) + ) && !is_ignorable_text_node(node) +} + +fn is_ignorable_text_node(node: &libxml::tree::Node) -> bool { + node.get_type() == Some(NodeType::TextNode) && node.get_content().trim().is_empty() +} + +fn collect_matching_sequence( + original_children: &[libxml::tree::Node], + fragment_children: &[libxml::tree::Node], + nodes_to_keep: &mut HashSet, + allowed_attributes: &mut HashMap>, +) -> bool { + collect_matching_sequence_from( + original_children, + fragment_children, + 0, + 0, + nodes_to_keep, + allowed_attributes, + ) +} + +fn collect_matching_sequence_from( + original_children: &[libxml::tree::Node], + fragment_children: &[libxml::tree::Node], + original_index: usize, + fragment_index: usize, + nodes_to_keep: &mut HashSet, + allowed_attributes: &mut HashMap>, +) -> bool { + if fragment_index == fragment_children.len() { + return true; + } + + for index in original_index..original_children.len() { + let mut candidate_nodes_to_keep = nodes_to_keep.clone(); + let mut candidate_allowed_attributes = allowed_attributes.clone(); + + if collect_matching_node_inner( + &original_children[index], + &fragment_children[fragment_index], + &mut candidate_nodes_to_keep, + &mut candidate_allowed_attributes, + ) && collect_matching_sequence_from( + original_children, + fragment_children, + index + 1, + fragment_index + 1, + &mut candidate_nodes_to_keep, + &mut candidate_allowed_attributes, + ) { + *nodes_to_keep = candidate_nodes_to_keep; + *allowed_attributes = candidate_allowed_attributes; + return true; + } + } + + false +} + +fn collect_matching_node( + original_node: &libxml::tree::Node, + fragment_node: &libxml::tree::Node, + nodes_to_keep: &mut HashSet, + allowed_attributes: &mut HashMap>, +) -> bool { + let mut candidate_nodes_to_keep = nodes_to_keep.clone(); + let mut candidate_allowed_attributes = allowed_attributes.clone(); + + if collect_matching_node_inner( + original_node, + fragment_node, + &mut candidate_nodes_to_keep, + &mut candidate_allowed_attributes, + ) { + *nodes_to_keep = candidate_nodes_to_keep; + *allowed_attributes = candidate_allowed_attributes; + true + } else { + false + } +} + +fn collect_matching_node_inner( + original_node: &libxml::tree::Node, + fragment_node: &libxml::tree::Node, + nodes_to_keep: &mut HashSet, + allowed_attributes: &mut HashMap>, +) -> bool { + match fragment_node.get_type() { + Some(NodeType::ElementNode) => { + if !original_node.is_element_node() + || !fragment_node_matches_original(fragment_node, original_node) + { + return false; + } + + allowed_attributes + .entry(original_node.node_ptr() as usize) + .or_default() + .extend(node_attribute_names(fragment_node)); + nodes_to_keep.insert(original_node.node_ptr() as usize); + + let fragment_children = supported_fragment_children(fragment_node); + collect_matching_sequence( + &supported_fragment_children(original_node), + &fragment_children, + nodes_to_keep, + allowed_attributes, + ) + } + Some(NodeType::TextNode | NodeType::CDataSectionNode) => { + if !matches!( + original_node.get_type(), + Some(NodeType::TextNode | NodeType::CDataSectionNode) + ) || fragment_node.get_content() != original_node.get_content() + { + return false; + } + + nodes_to_keep.insert(original_node.node_ptr() as usize); + true + } + Some(NodeType::CommentNode) => { + if original_node.get_type() != Some(NodeType::CommentNode) + || fragment_node.get_content() != original_node.get_content() + { + return false; + } + + nodes_to_keep.insert(original_node.node_ptr() as usize); + true + } + Some(NodeType::PiNode) => { + if original_node.get_type() != Some(NodeType::PiNode) + || fragment_node.get_name() != original_node.get_name() + || fragment_node.get_content() != original_node.get_content() + { + return false; + } + + nodes_to_keep.insert(original_node.node_ptr() as usize); + true + } + _ => false, + } +} + +fn add_required_ancestors( + root: &libxml::tree::Node, + matched_nodes: &HashSet, + nodes_to_keep: &mut HashSet, + allowed_attributes: &mut HashMap>, +) -> Result<(), CryptoError> { + for node_ptr in matched_nodes.iter().copied().collect::>() { + let node = find_node_by_ptr_in_tree(root, node_ptr as *const _) + .ok_or(XmlSecProviderError::XmlPredigestFragmentInvalid)?; + mark_all_element_ancestors(&node, nodes_to_keep, allowed_attributes); + } + Ok(()) } -/// Obtains the first child element of the given node that has the given name and namespace. -fn get_first_child_name_ns( +fn mark_all_element_ancestors( node: &libxml::tree::Node, - name: &str, - ns: &str, -) -> Option { - let mut found_node = None; - for child in node.get_child_elements() { - if let Some(child_ns) = child.get_namespace() { - if child_ns.get_href() != ns { - continue; - } - } else { - continue; + nodes_to_keep: &mut HashSet, + allowed_attributes: &mut HashMap>, +) { + let mut current = node.get_parent(); + while let Some(parent) = current { + if parent.is_element_node() { + let parent_ptr = parent.node_ptr() as usize; + nodes_to_keep.insert(parent_ptr); + allowed_attributes.entry(parent_ptr).or_default(); } + current = parent.get_parent(); + } +} - if child.get_name() == name { - found_node = Some(child); - break; +fn determine_output_root_ptr( + root: &libxml::tree::Node, + nodes_to_keep: &HashSet, +) -> Result { + let mut candidates = Vec::new(); + collect_top_level_kept_elements(root, nodes_to_keep, false, &mut candidates); + + if candidates.len() == 1 { + Ok(candidates[0]) + } else { + Err(XmlSecProviderError::XmlPredigestFragmentInvalid.into()) + } +} + +fn collect_top_level_kept_elements( + node: &libxml::tree::Node, + nodes_to_keep: &HashSet, + has_kept_element_ancestor: bool, + candidates: &mut Vec, +) { + if !node.is_element_node() { + return; + } + + let is_kept = nodes_to_keep.contains(&(node.node_ptr() as usize)); + if is_kept && !has_kept_element_ancestor { + candidates.push(node.node_ptr() as usize); + return; + } + + for child in node.get_child_elements() { + collect_top_level_kept_elements( + &child, + nodes_to_keep, + has_kept_element_ancestor || is_kept, + candidates, + ); + } +} + +fn move_output_root_to_document_root( + doc: &mut libxml::tree::Document, + output_root_ptr: usize, +) -> Result<(), CryptoError> { + let current_root = doc + .get_root_element() + .ok_or(XmlSecProviderError::XmlMissingRootElement)?; + if current_root.node_ptr() as usize == output_root_ptr { + return Ok(()); + } + + let mut new_root = find_node_by_ptr_in_tree(¤t_root, output_root_ptr as *const _) + .ok_or(XmlSecProviderError::XmlPredigestFragmentInvalid)?; + new_root.unlink(); + doc.set_root_element(&new_root); + Ok(()) +} + +fn element_identity_matches(left: &libxml::tree::Node, right: &libxml::tree::Node) -> bool { + element_local_name(left) == element_local_name(right) + && left.get_namespace().map(|namespace| namespace.get_href()) + == right.get_namespace().map(|namespace| namespace.get_href()) +} + +fn element_local_name(node: &libxml::tree::Node) -> String { + node.get_name() + .rsplit(':') + .next() + .unwrap_or_default() + .to_string() +} + +fn fragment_node_matches_original( + fragment_node: &libxml::tree::Node, + original_node: &libxml::tree::Node, +) -> bool { + match ( + fragment_node.is_element_node(), + original_node.is_element_node(), + ) { + (true, true) => { + element_identity_matches(fragment_node, original_node) + && node_attribute_names(fragment_node) + .into_iter() + .all(|attribute_name| { + get_attribute_value(fragment_node, &attribute_name) + == get_attribute_value(original_node, &attribute_name) + }) + } + (false, false) if fragment_node.is_text_node() && original_node.is_text_node() => { + fragment_node.get_content() == original_node.get_content() } + _ => false, } - found_node } -/// Searches the subtree rooted at the given node and returns the elements which match the given -/// predicate. -fn get_elements_by_predicate bool>( - elem: &libxml::tree::Node, - mut pred: F, -) -> Vec { - let mut nodes_to_visit = Vec::new(); - let mut nodes = Vec::new(); - nodes_to_visit.push(elem.clone()); - while let Some(node) = nodes_to_visit.pop() { - if pred(&node) { - nodes.push(node.clone()); +fn node_attribute_names(node: &libxml::tree::Node) -> Vec { + let mut attributes = Vec::new(); + + unsafe { + let mut current_attr = first_attribute_ptr(node.node_ptr()); + while !current_attr.is_null() { + attributes.push(attribute_name_from_ptr(current_attr)); + current_attr = (*current_attr).next; } - let mut children = node.get_child_elements(); - nodes_to_visit.append(&mut children); } - nodes + + attributes } -/// Searches for and returns the element with the given pointer value from the subtree rooted at the -/// given node. -fn get_node_by_ptr( - elem: &libxml::tree::Node, - ptr: *const libxml::bindings::xmlNode, -) -> Option { - let mut elems = get_elements_by_predicate(elem, |node| { - let node_ptr = node.node_ptr() as *const _; - node_ptr == ptr - }); - let elem = elems.drain(..).next(); - elem +fn attribute_name_from_ptr(attr_ptr: *mut libxml::bindings::xmlAttr) -> AttributeName { + unsafe { + let local_name = CStr::from_ptr((*attr_ptr).name as *const i8) + .to_string_lossy() + .into_owned(); + let namespace = if (*attr_ptr).ns.is_null() { + None + } else { + let href = (*(*attr_ptr).ns).href; + if href.is_null() { + None + } else { + Some( + CStr::from_ptr(href as *const i8) + .to_string_lossy() + .into_owned(), + ) + } + }; + + AttributeName { + local_name, + namespace, + } + } } -struct XPathContext { - pub pointer: libxml::bindings::xmlXPathContextPtr, +fn find_attribute_ptr( + node_ptr: *mut libxml::bindings::xmlNode, + attribute_name: &AttributeName, +) -> *mut libxml::bindings::xmlAttr { + unsafe { + let mut current_attr = first_attribute_ptr(node_ptr); + while !current_attr.is_null() { + if attribute_name_from_ptr(current_attr) == *attribute_name { + return current_attr; + } + current_attr = (*current_attr).next; + } + } + + std::ptr::null_mut() } -impl Drop for XPathContext { - fn drop(&mut self) { - unsafe { libxml::bindings::xmlXPathFreeContext(self.pointer) } + +fn first_attribute_ptr(node_ptr: *mut libxml::bindings::xmlNode) -> *mut libxml::bindings::xmlAttr { + if node_ptr.is_null() { + return std::ptr::null_mut(); } + + unsafe { (*node_ptr).properties } } -struct XPathObject { - pub pointer: libxml::bindings::xmlXPathObjectPtr, +fn get_attribute_value( + node: &libxml::tree::Node, + attribute_name: &AttributeName, +) -> Option { + let attr_ptr = find_attribute_ptr(node.node_ptr(), attribute_name); + if attr_ptr.is_null() { + return None; + } + + unsafe { + let value_ptr = + libxml::bindings::xmlNodeGetContent(attr_ptr as *mut libxml::bindings::xmlNode); + if value_ptr.is_null() { + return Some(String::new()); + } + + let value = CStr::from_ptr(value_ptr as *const i8) + .to_string_lossy() + .into_owned(); + libc::free(value_ptr as *mut libc::c_void); + Some(value) + } } -impl Drop for XPathObject { - fn drop(&mut self) { - unsafe { libxml::bindings::xmlXPathFreeObject(self.pointer) } + +fn remove_attribute( + node: &mut libxml::tree::Node, + attribute_name: &AttributeName, +) -> Result<(), Box> { + let node_ptr = node.node_ptr_mut().map_err(|error| { + Box::new(std::io::Error::new(std::io::ErrorKind::Other, error)) + as Box + })?; + let attr_ptr = find_attribute_ptr(node_ptr, attribute_name); + if attr_ptr.is_null() { + return Ok(()); + } + + unsafe { + let remove_prop_status = libxml::bindings::xmlRemoveProp(attr_ptr); + if remove_prop_status == 0 { + Ok(()) + } else { + Err(From::from(format!( + "libxml2 failed to remove property with status: {:?}", + remove_prop_status + ))) + } } } -/// Searches for and returns the element at the root of the subtree signed by the given signature -/// node. -fn get_signed_node( +fn resolve_reference_anchor( + uri: Option<&str>, + doc: &libxml::tree::Document, signature_node: &libxml::tree::Node, +) -> Option<(libxml::tree::Node, VerifiedSelectionKind)> { + match uri { + None | Some("") => doc + .get_root_element() + .map(|root| (root, VerifiedSelectionKind::WholeNode)), + Some(uri) => { + let stripped = uri.strip_prefix('#')?; + if stripped.starts_with("xpointer(") { + resolve_xpointer_anchor(stripped, doc, signature_node) + } else { + resolve_id_to_node(stripped, doc) + .map(|node| (node, VerifiedSelectionKind::WholeNode)) + } + } + } +} + +fn resolve_id_to_node(id: &str, doc: &libxml::tree::Document) -> Option { + let id_cstring = CString::new(id).ok()?; + + unsafe { + let target_attr_ptr = + libxml::bindings::xmlGetID(doc.doc_ptr(), id_cstring.as_ptr() as *const u8); + if target_attr_ptr.is_null() { + return None; + } + + let target_node_ptr = (*target_attr_ptr).parent; + if target_node_ptr.is_null() { + return None; + } + find_node_by_ptr_in_tree(&doc.get_root_element()?, target_node_ptr) + } +} + +fn resolve_xpointer_anchor( + xpointer: &str, doc: &libxml::tree::Document, -) -> Option { - let object_elem_opt = get_first_child_name_ns(signature_node, "Object", XMLNS_XML_DSIG); - if let Some(object_elem) = object_elem_opt { - return Some(object_elem); - } - - let sig_info_elem_opt = get_first_child_name_ns(signature_node, "SignedInfo", XMLNS_XML_DSIG); - if let Some(sig_info_elem) = sig_info_elem_opt { - let ref_elem_opt = get_first_child_name_ns(&sig_info_elem, "Reference", XMLNS_XML_DSIG); - if let Some(ref_elem) = ref_elem_opt { - if let Some(uri) = ref_elem.get_attribute("URI") { - if let Some(stripped) = uri.strip_prefix('#') { - // prepare a XPointer context - let c_uri = CString::new(stripped).unwrap(); - let ctx_ptr = unsafe { - libxml::bindings::xmlXPtrNewContext( - doc.doc_ptr(), - signature_node.node_ptr(), - std::ptr::null_mut(), - ) - }; - if ctx_ptr.is_null() { - return None; - } - let ctx = XPathContext { pointer: ctx_ptr }; - - // evaluate the XPointer expression - let obj_ptr = unsafe { - libxml::bindings::xmlXPtrEval( - c_uri.as_ptr() as *const libxml::bindings::xmlChar, - ctx.pointer, - ) - }; - if obj_ptr.is_null() { - return None; - } - let obj = XPathObject { pointer: obj_ptr }; - - // extract the nodeset from the result - let obj_type = unsafe { (*obj.pointer).type_ }; - if obj_type != libxml::bindings::xmlXPathObjectType_XPATH_NODESET { - return None; - } - let obj_nodeset = unsafe { (*obj.pointer).nodesetval }; - let nodeset_count = unsafe { (*obj_nodeset).nodeNr }; - - // go through the nodes and find them in the document - for i in 0..nodeset_count { - let node_ptr_ptr = - unsafe { (*obj_nodeset).nodeTab.offset(i.try_into().unwrap()) }; - let node_ptr = unsafe { *node_ptr_ptr }; - if let Some(node) = - get_node_by_ptr(&doc.get_root_element().unwrap(), node_ptr) - { - return Some(node); - } - } - } + signature_node: &libxml::tree::Node, +) -> Option<(libxml::tree::Node, VerifiedSelectionKind)> { + let xpointer = CString::new(xpointer).ok()?; + let document_root = doc.get_root_element()?; + let context = XPathContext::new(doc, signature_node)?; + let object = XPathObject::evaluate(&context, &xpointer)?; + + let mut matched_nodes = resolve_xpointer_matched_nodes(&object, &document_root)?; + if matched_nodes.len() == 1 && matched_nodes[0].is_element_node() { + return Some((matched_nodes.remove(0), VerifiedSelectionKind::WholeNode)); + } + + lowest_common_ancestor(&matched_nodes) + .map(|ancestor| (ancestor, VerifiedSelectionKind::ChildSequence)) +} + +fn resolve_xpointer_matched_nodes( + object: &XPathObject, + document_root: &libxml::tree::Node, +) -> Option> { + let node_ptrs = unsafe { + let object_ptr = object.pointer.as_ptr(); + if (*object_ptr).type_ != libxml::bindings::xmlXPathObjectType_XPATH_NODESET { + return None; + } + + let node_set = (*object_ptr).nodesetval; + if node_set.is_null() { + return None; + } + + let node_count = (*node_set).nodeNr; + if node_count <= 0 { + return Some(Vec::new()); + } + + let node_tab = (*node_set).nodeTab; + if node_tab.is_null() { + return None; + } + + std::slice::from_raw_parts(node_tab, usize::try_from(node_count).ok()?).to_vec() + }; + + let mut matched_nodes = Vec::new(); + for node_ptr in node_ptrs { + if node_ptr.is_null() { + continue; + } + + if let Some(node) = find_node_by_ptr_in_tree(document_root, node_ptr) { + if let Some(element) = nearest_element_ancestor(&node) { + matched_nodes.push(element); } } } + Some(matched_nodes) +} + +fn find_node_by_ptr_in_tree( + root: &libxml::tree::Node, + target_ptr: *const libxml::bindings::xmlNode, +) -> Option { + if root.node_ptr() == target_ptr as *mut _ { + return Some(root.clone()); + } + + for child in root.get_child_nodes() { + if let Some(found) = find_node_by_ptr_in_tree(&child, target_ptr) { + return Some(found); + } + } + None } -/// Place the signature-verified attributes ([`ATTRIB_SIGVER`] in the given namespace) on the given -/// element, all its descendants and its whole chain of ancestors (but not necessarily all their -/// descendants). -fn place_signature_verified_attributes( - root_elem: libxml::tree::Node, - doc: &libxml::tree::Document, - ns: &libxml::tree::Namespace, -) { - let mut ptr_to_required_node: HashMap = HashMap::new(); - let mut signature_nodes = find_signature_nodes(&root_elem); - for sig_node in signature_nodes.drain(..) { - if let Some(sig_root_node) = get_signed_node(&sig_node, doc) { - let mut nodes = Vec::new(); - let mut parent = sig_root_node.get_parent(); - nodes.push(sig_root_node); - - // mark all children - while let Some(node) = nodes.pop() { - let node_ptr = node.node_ptr() as usize; - for child in node.get_child_elements() { - nodes.push(child); +fn nearest_element_ancestor(node: &libxml::tree::Node) -> Option { + if node.is_element_node() { + return Some(node.clone()); + } + + let mut current = node.get_parent(); + while let Some(parent) = current { + if parent.is_element_node() { + return Some(parent); + } + current = parent.get_parent(); + } + + None +} + +fn lowest_common_ancestor(nodes: &[libxml::tree::Node]) -> Option { + let ancestor_paths = nodes + .iter() + .map(|node| { + let mut path = Vec::new(); + let mut current = Some(node.clone()); + + while let Some(current_node) = current { + if current_node.is_element_node() { + path.push(current_node.clone()); } - ptr_to_required_node.entry(node_ptr).or_insert(node); + current = current_node.get_parent(); } - // mark the ancestor chain - while let Some(p) = parent { - let p_ptr = p.node_ptr() as usize; - parent = p.get_parent(); - ptr_to_required_node.entry(p_ptr).or_insert(p); - } + path.reverse(); + path + }) + .collect::>(); + + let shortest_path_len = ancestor_paths + .iter() + .map(Vec::len) + .min() + .unwrap_or_default(); + let mut common_ancestor = None; + + for index in 0..shortest_path_len { + let candidate = &ancestor_paths[0][index]; + if ancestor_paths + .iter() + .all(|path| path[index].node_ptr() == candidate.node_ptr()) + { + common_ancestor = Some(candidate.clone()); + } else { + break; } } - drop(root_elem); - for node in ptr_to_required_node.values_mut() { - node.set_attribute_ns(ATTRIB_SIGVER, VALUE_SIGVER, ns) - .unwrap(); - } + + common_ancestor } -/// Remove all elements that do not contain a signature-verified attribute ([`ATTRIB_SIGVER`] in -/// the namespace [`XMLNS_SIGVER`]). -fn remove_unverified_elements(node: &mut libxml::tree::Node) { - // depth-first - for mut child in node.get_child_elements() { - remove_unverified_elements(&mut child); +fn prune_unverified_nodes( + node: &mut libxml::tree::Node, + nodes_to_keep: &HashSet, + allowed_attributes: &HashMap>, +) -> Result<(), CryptoError> { + for mut child in node.get_child_nodes() { + prune_unverified_nodes(&mut child, nodes_to_keep, allowed_attributes)?; } - if node.get_attribute_ns(ATTRIB_SIGVER, XMLNS_SIGVER) != Some(String::from(VALUE_SIGVER)) { - // element is unverified; remove it + if !nodes_to_keep.contains(&(node.node_ptr() as usize)) { node.unlink_node(); + return Ok(()); } + + if node.is_element_node() { + prune_unsigned_attributes(node, allowed_attributes)?; + } + + Ok(()) } +fn prune_unsigned_attributes( + node: &mut libxml::tree::Node, + allowed_attributes: &HashMap>, +) -> Result<(), CryptoError> { + let Some(allowed_attributes) = allowed_attributes.get(&(node.node_ptr() as usize)) else { + return Ok(()); + }; + + for attribute_name in node_attribute_names(node) { + if !allowed_attributes.contains(&attribute_name) { + remove_attribute(node, &attribute_name) + .map_err(|error| XmlSecProviderError::XmlAttributeRemovalError { error })?; + } + } + + Ok(()) +} fn decrypt( t: Cipher, @@ -571,3 +1244,268 @@ fn decrypt_aead( out.truncate(count + rest); Ok(out) } + +#[cfg(test)] +mod tests { + use super::*; + + fn reduce_with_selection(xml: &str, reduce_mode: ReduceMode, make_selection: F) -> String + where + F: FnOnce(&libxml::tree::Document) -> VerifiedSelection, + { + let mut doc = XmlParser::default().parse_string(xml).unwrap(); + let selections = [make_selection(&doc)]; + let (nodes_to_keep, allowed_attributes, output_root_ptr) = + build_keep_state(&doc, &selections, reduce_mode).unwrap(); + drop(selections); + move_output_root_to_document_root(&mut doc, output_root_ptr).unwrap(); + let mut root = doc.get_root_element().unwrap(); + prune_unverified_nodes(&mut root, &nodes_to_keep, &allowed_attributes).unwrap(); + doc.to_string() + } + + #[test] + fn validate_and_mark_keeps_ancestors_for_signed_assertions() { + let xml = r#" + + + + + https://idp.example.com + + + remove-me + + +"#; + + let reduced = reduce_with_selection(xml, ReduceMode::ValidateAndMark, |doc| { + let assertion = doc.get_root_element().unwrap().get_child_elements()[0] + .get_child_elements()[0] + .get_child_elements()[0] + .clone(); + + VerifiedSelection { + anchor: assertion, + kind: VerifiedSelectionKind::WholeNode, + uri: "#assertion".to_string(), + predigest_xml: r#" + https://idp.example.com + "# + .to_string(), + } + }); + + assert!(reduced.contains("")); + } + + #[test] + fn validate_and_mark_no_ancestors_roots_signed_assertions_at_the_assertion() { + let xml = r#" + + + + + https://idp.example.com + + + remove-me + + +"#; + + let reduced = reduce_with_selection(xml, ReduceMode::ValidateAndMarkNoAncestors, |doc| { + let assertion = doc.get_root_element().unwrap().get_child_elements()[0] + .get_child_elements()[0] + .get_child_elements()[0] + .clone(); + + VerifiedSelection { + anchor: assertion, + kind: VerifiedSelectionKind::WholeNode, + uri: "#assertion".to_string(), + predigest_xml: r#" + https://idp.example.com + "# + .to_string(), + } + }); + + assert!(reduced.contains("")); + } + + #[test] + fn child_sequence_roots_on_the_anchor_shell_and_strips_unsigned_attributes() { + let xml = r#" + + value + remove-me + +"#; + + let reduced = reduce_with_selection(xml, ReduceMode::ValidateAndMarkNoAncestors, |doc| { + VerifiedSelection { + anchor: doc.get_root_element().unwrap(), + kind: VerifiedSelectionKind::ChildSequence, + uri: "".to_string(), + predigest_xml: r#"value"#.to_string(), + } + }); + + assert!(reduced.contains("value"#)); + assert!(!reduced.contains("unsigned=\"drop\"")); + assert!(!reduced.contains("")); + } + + #[test] + fn child_sequence_supports_comments_pi_and_cdata() { + let xml = r#"valuetailremove-meremove-me"#; + + let doc = XmlParser::default().parse_string(xml).unwrap(); + let root = doc.get_root_element().unwrap(); + let parent = root.get_child_elements()[0].clone(); + let parsed = parse_selection( + &parent, + "cdata-valuevaluetail", + ) + .unwrap(); + + let original_children = supported_fragment_children(&parent); + let mut nodes_to_keep = HashSet::new(); + let mut allowed_attributes = HashMap::new(); + + assert!(collect_matching_sequence( + &original_children, + &parsed.sequence_nodes, + &mut nodes_to_keep, + &mut allowed_attributes, + )); + + assert_eq!(parsed.sequence_nodes.len(), 5); + assert!(nodes_to_keep.len() >= 5); + let child = parent + .get_child_elements() + .into_iter() + .find(|node| node.get_name() == "Child") + .unwrap(); + let child_attributes = allowed_attributes + .get(&(child.node_ptr() as usize)) + .expect("matched child should record allowed attributes"); + + assert!(child_attributes.contains(&AttributeName { + local_name: "attr".to_string(), + namespace: None, + })); + assert!(!child_attributes.contains(&AttributeName { + local_name: "unsigned".to_string(), + namespace: None, + })); + } + + #[test] + fn attribute_matching_is_namespace_aware() { + let xml = r#" + + value + +"#; + + let doc = XmlParser::default().parse_string(xml).unwrap(); + let mut parent = doc.get_root_element().unwrap().get_child_elements()[0].clone(); + let fragment_doc = XmlParser::default() + .parse_string(r#"value"#) + .unwrap(); + let fragment_parent = fragment_doc + .get_root_element() + .unwrap() + .get_child_elements()[0] + .clone(); + + assert!(fragment_node_matches_original(&fragment_parent, &parent)); + + let allowed_attributes = HashMap::from([( + parent.node_ptr() as usize, + node_attribute_names(&fragment_parent).into_iter().collect(), + )]); + prune_unsigned_attributes(&mut parent, &allowed_attributes).unwrap(); + let reduced = doc.to_string(); + + assert!(reduced.contains(r#"a:id="keep""#)); + assert!(reduced.contains(r#"plain="keep""#)); + assert!(!reduced.contains(r#"b:id="drop""#)); + } + + #[test] + fn collect_id_attributes_rejects_non_ncname_values() { + let mut doc = XmlParser::default() + .parse_string(r#""#) + .unwrap(); + + let error = collect_id_attributes(&mut doc).expect_err( + "non-NCName ID values should be rejected before entering libxml's ID table", + ); + + assert!(matches!( + error, + XmlSecProviderError::XmlInvalidIdAttribute { .. } + )); + } + + #[test] + fn collect_id_attributes_rejects_duplicate_values() { + let mut doc = XmlParser::default() + .parse_string(r#""#) + .unwrap(); + + let error = collect_id_attributes(&mut doc).expect_err("duplicate IDs should fail closed"); + + assert!(matches!( + error, + XmlSecProviderError::XmlDuplicateIdAttribute { .. } + )); + } + + #[test] + fn resolve_reference_anchor_supports_explicit_xpointer_fragments() { + let mut doc = XmlParser::default() + .parse_string( + r#""#, + ) + .unwrap(); + collect_id_attributes(&mut doc).unwrap(); + + let root = doc.get_root_element().unwrap(); + let signature_node = root.get_child_elements()[0].clone(); + let (target, kind) = + resolve_reference_anchor(Some("#xpointer(id('b'))"), &doc, &signature_node).expect( + "explicit XPointer fragments should resolve through the XPointer evaluator", + ); + + assert_eq!(target.get_name(), "B"); + assert_eq!(kind, VerifiedSelectionKind::WholeNode); + } + + #[test] + fn resolve_reference_anchor_rejects_barename_xpointer_lookalikes() { + let mut doc = XmlParser::default() + .parse_string( + r#""#, + ) + .unwrap(); + collect_id_attributes(&mut doc).unwrap(); + + let root = doc.get_root_element().unwrap(); + let signature_node = root.get_child_elements()[0].clone(); + let target = resolve_reference_anchor(Some("#element(/1/3)"), &doc, &signature_node); + assert!( + target.is_none(), + "barename URI fragments must not be interpreted as XPointer expressions" + ); + } +} diff --git a/src/crypto/xmlsec/wrapper/mod.rs b/src/crypto/xmlsec/wrapper/mod.rs index 8da05c6..0f98b04 100644 --- a/src/crypto/xmlsec/wrapper/mod.rs +++ b/src/crypto/xmlsec/wrapper/mod.rs @@ -16,15 +16,17 @@ pub use libxml::tree::document::Document as XmlDocument; pub use libxml::tree::node::Node as XmlNode; mod backend; +mod bindings; mod error; mod keys; mod xmldsig; mod xmlsec_internal; -mod bindings; // exports +pub use self::bindings::XMLSEC_DSIG_FLAGS_STORE_SIGNEDINFO_REFERENCES; pub use self::error::XmlSecError; pub use self::error::XmlSecResult; pub use self::keys::XmlSecKey; pub use self::keys::XmlSecKeyFormat; +pub use self::xmldsig::VerifiedReference; pub use self::xmldsig::XmlSecSignatureContext; diff --git a/src/crypto/xmlsec/wrapper/xmldsig.rs b/src/crypto/xmlsec/wrapper/xmldsig.rs index 154f7ce..4611850 100644 --- a/src/crypto/xmlsec/wrapper/xmldsig.rs +++ b/src/crypto/xmlsec/wrapper/xmldsig.rs @@ -8,11 +8,19 @@ use super::XmlSecResult; use super::{XmlDocument, XmlSecError}; use std::os::raw::c_uchar; -use std::ptr::{null, null_mut}; +use std::ptr::{null, null_mut, NonNull}; + +/// The verified data for a single ``. +pub struct VerifiedReference { + /// The reference URI attribute, if present. + pub uri: Option, + /// The canonicalized XML that xmlsec verified for this reference. + pub predigest_xml: String, +} -/// Signature signing/veryfying context +/// Signature signing/verifying context pub struct XmlSecSignatureContext { - ctx: *mut bindings::xmlSecDSigCtx, + ctx: NonNull, } impl XmlSecSignatureContext { @@ -21,12 +29,87 @@ impl XmlSecSignatureContext { super::xmlsec_internal::guarantee_xmlsec_init()?; let ctx = unsafe { bindings::xmlSecDSigCtxCreate(null_mut()) }; + let ctx = NonNull::new(ctx).ok_or(XmlSecError::ContextInitError)?; + + Ok(Self { ctx }) + } - if ctx.is_null() { - return Err(XmlSecError::ContextInitError); + /// Builds a context with specific flags. + pub fn new_with_flags(flags: u32) -> XmlSecResult { + let mut ctx = Self::new()?; + unsafe { + ctx.ctx.as_mut().flags |= flags; } + Ok(ctx) + } - Ok(Self { ctx }) + /// Retrieves the verified references from ``. + pub fn get_verified_references(&self) -> XmlSecResult> { + let mut result = Vec::new(); + + unsafe { + let list_ptr = &self.ctx.as_ref().signedInfoReferences as *const bindings::xmlSecPtrList + as *mut bindings::xmlSecPtrList; + let count = bindings::xmlSecPtrListGetSize(list_ptr); + + for i in 0..count { + let ref_ctx_ptr = bindings::xmlSecPtrListGetItem(list_ptr, i); + if ref_ctx_ptr.is_null() { + return Err(XmlSecError::SigningError); + } + + let ref_ctx_ptr = ref_ctx_ptr as bindings::xmlSecDSigReferenceCtxPtr; + let ref_ctx = &*ref_ctx_ptr; + + if ref_ctx.status != bindings::xmlSecDSigStatus_xmlSecDSigStatusSucceeded { + return Err(XmlSecError::VerifyError); + } + + let uri = if ref_ctx.uri.is_null() { + None + } else { + let uri_cstr = std::ffi::CStr::from_ptr(ref_ctx.uri as *const i8); + Some( + uri_cstr + .to_str() + .map_err(|_| XmlSecError::InvalidInputString)? + .to_string(), + ) + }; + + let predigest_buf = bindings::xmlSecDSigReferenceCtxGetPreDigestBuffer(ref_ctx_ptr); + if predigest_buf.is_null() { + return Err(XmlSecError::VerifyError); + } + + let data_ptr = bindings::xmlSecBufferGetData(predigest_buf); + let data_size = bindings::xmlSecBufferGetSize(predigest_buf); + let predigest_xml = predigest_xml_from_raw_buffer(data_ptr, data_size)?; + + result.push(VerifiedReference { uri, predigest_xml }); + } + } + + Ok(result) + } + + /// Retrieves the URI strings from the verified reference contexts. + pub fn get_verified_reference_uris(&self) -> XmlSecResult> { + Ok(self + .get_verified_references()? + .into_iter() + .filter_map(|reference| reference.uri) + .collect()) + } + + /// Retrieves the pre-digest data from the first and only verified reference. + pub fn get_predigest_data(&self) -> XmlSecResult { + let mut references = self.get_verified_references()?; + if references.len() != 1 { + return Err(XmlSecError::SigningError); + } + + Ok(references.remove(0).predigest_xml) } /// Sets the key to use for signature or verification. In case a key had @@ -35,11 +118,12 @@ impl XmlSecSignatureContext { let mut old = None; unsafe { - if !(*self.ctx).signKey.is_null() { - old = Some(XmlSecKey::from_ptr((*self.ctx).signKey)); + let ctx = self.ctx.as_mut(); + if !ctx.signKey.is_null() { + old = Some(XmlSecKey::from_ptr(ctx.signKey)); } - (*self.ctx).signKey = XmlSecKey::leak(key); + ctx.signKey = XmlSecKey::leak(key); } old @@ -49,12 +133,13 @@ impl XmlSecSignatureContext { #[allow(unused)] pub fn release_key(&mut self) -> Option { unsafe { - if (*self.ctx).signKey.is_null() { + let ctx = self.ctx.as_mut(); + if ctx.signKey.is_null() { None } else { - let key = XmlSecKey::from_ptr((*self.ctx).signKey); + let key = XmlSecKey::from_ptr(ctx.signKey); - (*self.ctx).signKey = null_mut(); + ctx.signKey = null_mut(); Some(key) } @@ -166,10 +251,51 @@ impl XmlSecSignatureContext { } } +fn predigest_xml_from_raw_buffer( + data_ptr: *const c_uchar, + data_size: usize, +) -> XmlSecResult { + if data_size == 0 { + return Ok(String::new()); + } + + if data_ptr.is_null() { + return Err(XmlSecError::VerifyError); + } + + let data_slice = unsafe { std::slice::from_raw_parts(data_ptr, data_size) }; + let predigest_xml = std::str::from_utf8(data_slice) + .map_err(|_| XmlSecError::InvalidInputString)? + .to_string(); + Ok(predigest_xml) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn predigest_xml_from_raw_buffer_accepts_empty_buffers() { + let predigest_xml = predigest_xml_from_raw_buffer(std::ptr::null(), 0) + .expect("zero-length pre-digest buffers should be accepted"); + + assert!(predigest_xml.is_empty()); + } + + #[test] + fn predigest_xml_from_raw_buffer_rejects_invalid_utf8() { + let bytes = [0xff]; + let error = predigest_xml_from_raw_buffer(bytes.as_ptr(), bytes.len()) + .expect_err("non-utf8 pre-digest buffers should be rejected"); + + assert!(matches!(error, XmlSecError::InvalidInputString)); + } +} + impl XmlSecSignatureContext { fn key_is_set(&self) -> XmlSecResult<()> { unsafe { - if !(*self.ctx).signKey.is_null() { + if !self.ctx.as_ref().signKey.is_null() { Ok(()) } else { Err(XmlSecError::KeyNotLoaded) @@ -178,7 +304,7 @@ impl XmlSecSignatureContext { } fn sign_node_raw(&self, node: *mut bindings::xmlNode) -> XmlSecResult<()> { - let rc = unsafe { bindings::xmlSecDSigCtxSign(self.ctx, node) }; + let rc = unsafe { bindings::xmlSecDSigCtxSign(self.ctx.as_ptr(), node) }; if rc < 0 { Err(XmlSecError::SigningError) @@ -188,13 +314,13 @@ impl XmlSecSignatureContext { } fn verify_node_raw(&self, node: *mut bindings::xmlNode) -> XmlSecResult { - let rc = unsafe { bindings::xmlSecDSigCtxVerify(self.ctx, node) }; + let rc = unsafe { bindings::xmlSecDSigCtxVerify(self.ctx.as_ptr(), node) }; if rc < 0 { return Err(XmlSecError::VerifyError); } - match unsafe { (*self.ctx).status } { + match unsafe { self.ctx.as_ref().status } { bindings::xmlSecDSigStatus_xmlSecDSigStatusSucceeded => Ok(true), _ => Ok(false), } @@ -203,7 +329,7 @@ impl XmlSecSignatureContext { impl Drop for XmlSecSignatureContext { fn drop(&mut self) { - unsafe { bindings::xmlSecDSigCtxDestroy(self.ctx) }; + unsafe { bindings::xmlSecDSigCtxDestroy(self.ctx.as_ptr()) }; } } diff --git a/src/idp/tests.rs b/src/idp/tests.rs index d1e4d33..8e40f4f 100644 --- a/src/idp/tests.rs +++ b/src/idp/tests.rs @@ -1,11 +1,22 @@ +#![cfg(feature = "xmlsec")] + use super::*; use chrono::prelude::*; -use crate::crypto::{Crypto, CryptoProvider}; +use crate::crypto::{CertificateDer, Crypto, CryptoProvider, ReduceMode, XmlSec}; use crate::idp::sp_extractor::{RequiredAttribute, SPMetadataExtractor}; use crate::idp::verified_request::UnverifiedAuthnRequest; use crate::service_provider::ServiceProvider; +fn cert_der_from_pem(pem: &[u8]) -> CertificateDer { + CertificateDer::from( + openssl::x509::X509::from_pem(pem) + .expect("failed to parse test certificate") + .to_der() + .expect("failed to encode test certificate"), + ) +} + #[test] fn test_self_signed_authn_request() { let authn_request_xml = include_str!("../../test_vectors/authn_request.xml"); @@ -112,7 +123,9 @@ fn test_signed_response() { fn test_signed_response_threads() { let verify = move || { let authn_request_xml = include_str!("../../test_vectors/authn_request.xml"); - let cert_der = include_bytes!("../../test_vectors/sp_cert.der").to_vec().into(); + let cert_der = include_bytes!("../../test_vectors/sp_cert.der") + .to_vec() + .into(); let unverified = UnverifiedAuthnRequest::from_xml(authn_request_xml).expect("failed to parse"); let _ = unverified @@ -219,10 +232,9 @@ fn test_do_not_accept_unsigned_response() { assert!(resp.is_err()); let err = resp.err().unwrap(); - //todo: is this the correct error, doesn't really seem to fit the test description. assert!(matches!( err, - crate::service_provider::Error::FailedToParseSamlResponse(_) + crate::service_provider::Error::FailedToValidateSignature )) } @@ -309,12 +321,12 @@ fn test_accept_signed_with_correct_key_idp_2() { "/test_vectors/response_signed_by_idp_2.xml", )); - let resp = sp.parse_xml_response( - correct_cert_signed_response_xml, - Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), - ); - - assert!(resp.is_ok()); + let _resp = sp + .parse_xml_response( + correct_cert_signed_response_xml, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect("failed to parse response"); } #[test] @@ -340,10 +352,114 @@ fn test_accept_signed_with_correct_key_idp_3() { "/test_vectors/response_signed_by_idp_ecdsa.xml", )); - let resp = sp.parse_xml_response( - correct_cert_signed_response_xml, - Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + let _resp = sp + .parse_xml_response( + correct_cert_signed_response_xml, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect("failed to parse response"); +} + +#[test] +fn test_malicious_ancestors_not_included() { + let signed_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/ancestor_attack_signed.xml" + )); + let cert = cert_der_from_pem(include_bytes!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/idp_2_metadata_public.pem" + ))); + + for (reduce_mode, result) in [ + (ReduceMode::PreDigest, true), + (ReduceMode::ValidateAndMark, true), + (ReduceMode::ValidateAndMarkNoAncestors, true), + ] { + let reduced = XmlSec::reduce_xml_to_signed(signed_xml, &[cert.clone()], reduce_mode) + .expect("reduce_xml_to_signed should succeed"); + + assert_eq!( + !reduced.contains("https://attacker.evil.com"), + result, + "ancestor should not be contained in {reduce_mode:?}" + ); + } +} + +#[test] +fn test_object_reference_removed() { + let signed_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/object_attack_response.xml" + )); + let cert = cert_der_from_pem(include_bytes!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/idp_2_metadata_public.pem" + ))); + + for (reduce_mode, result) in [ + (ReduceMode::PreDigest, true), + (ReduceMode::ValidateAndMark, true), + (ReduceMode::ValidateAndMarkNoAncestors, true), + ] { + let reduced = XmlSec::reduce_xml_to_signed(signed_xml, &[cert.clone()], reduce_mode) + .expect("reduce_xml_to_signed should succeed"); + + assert_eq!( + !reduced.contains("ds:Object"), + result, + "object should not be present in {reduce_mode:?}" + ); + } +} + +#[test] +fn test_xpointer_attack_fixture_does_not_verify() { + let signed_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/xpointer_attack_signed.xml" + )); + let cert = CertificateDer::from( + include_bytes!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/public.der" + )) + .to_vec(), ); - assert!(resp.is_ok()); + let error = XmlSec::verify_signed_xml(signed_xml, &cert, Some("ID")) + .expect_err("xpointer attack fixture should fail signature verification"); + + assert!(matches!( + error, + crate::crypto::CryptoError::InvalidSignature + )); +} + +#[test] +fn test_xpath_transforms_validated() { + let signed_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/xpath_transform.xml" + )); + let cert = cert_der_from_pem(include_bytes!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/idp_2_metadata_public.pem" + ))); + + for (reduce_mode, result) in [ + (ReduceMode::PreDigest, true), + (ReduceMode::ValidateAndMark, true), + (ReduceMode::ValidateAndMarkNoAncestors, true), + ] { + let reduced = XmlSec::reduce_xml_to_signed(signed_xml, &[cert.clone()], reduce_mode) + .expect("reduce_xml_to_signed should succeed"); + + assert_eq!( + !reduced.contains("malicious"), + result, + "malicious content should not be present in {reduce_mode:?}" + ); + } } diff --git a/src/idp/verified_request.rs b/src/idp/verified_request.rs index 912499b..10ad670 100644 --- a/src/idp/verified_request.rs +++ b/src/idp/verified_request.rs @@ -1,6 +1,6 @@ use quick_xml::events::Event; -use crate::crypto::{decode_x509_cert, Crypto, CryptoProvider, CertificateDer}; +use crate::crypto::{decode_x509_cert, CertificateDer, Crypto, CryptoProvider}; use crate::schema::AuthnRequest; use super::error::Error; @@ -51,7 +51,10 @@ impl<'a> UnverifiedAuthnRequest<'a> { .map(|()| VerifiedAuthnRequest(self.request)) } - pub fn try_verify_with_cert(self, der_cert: &CertificateDer) -> Result { + pub fn try_verify_with_cert( + self, + der_cert: &CertificateDer, + ) -> Result { Crypto::verify_signed_xml(self.xml.as_bytes(), der_cert, Some("ID"))?; Ok(VerifiedAuthnRequest(self.request)) } diff --git a/src/schema/authn_request.rs b/src/schema/authn_request.rs index c96fa1f..b0bdc38 100644 --- a/src/schema/authn_request.rs +++ b/src/schema/authn_request.rs @@ -1,4 +1,4 @@ -use crate::crypto::{Crypto, CryptoProvider, CertificateDer}; +use crate::crypto::{CertificateDer, Crypto, CryptoProvider}; use crate::schema::{Conditions, Issuer, NameIdPolicy, RequestedAuthnContext, Subject}; use crate::signature::Signature; use chrono::prelude::*; @@ -249,7 +249,9 @@ mod test { let public_cert = include_bytes!(concat!( env!("CARGO_MANIFEST_DIR"), "/test_vectors/public.der" - )).to_vec().into(); + )) + .to_vec() + .into(); let authn_request_sign_template = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), @@ -261,9 +263,7 @@ mod test { .add_key_info(&public_cert) .to_signed_xml(private_key)?; - assert!( - Crypto::verify_signed_xml(signed_authn_request, &public_cert, Some("ID"),).is_ok() - ); + assert!(Crypto::verify_signed_xml(signed_authn_request, &public_cert, Some("ID"),).is_ok()); Ok(()) } diff --git a/src/schema/encrypted_assertion.rs b/src/schema/encrypted_assertion.rs index 2a24b69..41646fa 100644 --- a/src/schema/encrypted_assertion.rs +++ b/src/schema/encrypted_assertion.rs @@ -3,10 +3,10 @@ use quick_xml::Writer; use serde::Deserialize; use std::io::Cursor; -use crate::schema::Assertion; -use crate::service_provider::Error; use crate::crypto::{Crypto, CryptoProvider}; use crate::key_info::{EncryptedKeyInfo, KeyInfo}; +use crate::schema::Assertion; +use crate::service_provider::Error; use crate::signature::DigestMethod; const NAME: &str = "saml2:EncryptedAssertion"; @@ -34,7 +34,10 @@ impl EncryptedAssertion { self.data.as_ref().and_then(|ed| ed.value_info()) } - pub fn decrypt(&self, decryption_key: &::PrivateKey) -> Result { + pub fn decrypt( + &self, + decryption_key: &::PrivateKey, + ) -> Result { let (ekey, method) = self .encrypted_key_info() .ok_or(Error::MissingEncryptedKeyInfo)?; diff --git a/src/service_provider/mod.rs b/src/service_provider/mod.rs index 45b6b49..b057976 100644 --- a/src/service_provider/mod.rs +++ b/src/service_provider/mod.rs @@ -1,5 +1,7 @@ use crate::crypto; -use crate::crypto::{CertificateDer, Crypto, CryptoError, CryptoProvider, sign_url}; +#[cfg(feature = "xmlsec")] +use crate::crypto::sign_url; +use crate::crypto::{CertificateDer, Crypto, CryptoError, CryptoProvider, ReduceMode}; use crate::metadata::{Endpoint, IndexedEndpoint, KeyDescriptor, NameIdFormat, SpSsoDescriptor}; use crate::schema::{Assertion, Response}; use crate::traits::ToXml; @@ -12,14 +14,18 @@ use base64::{engine::general_purpose, Engine as _}; use chrono::prelude::*; use chrono::Duration; use flate2::{write::DeflateEncoder, Compression}; +use quick_xml::{de::DeError, events::Event as XmlEvent, Reader}; use std::fmt::Debug; use std::io::Write; use thiserror::Error; use url::Url; +const SUBJECT_CONFIRMATION_METHOD_BEARER: &str = "urn:oasis:names:tc:SAML:2.0:cm:bearer"; + #[cfg(test)] mod tests; +#[cfg(feature = "xmlsec")] use crate::schema::EncryptedAssertion; #[derive(Debug, Error)] @@ -35,6 +41,14 @@ pub enum Error { }, #[error("SAML Assertion expired at: {}", time)] AssertionExpired { time: String }, + #[error("SAML Assertion is missing a bearer SubjectConfirmation")] + AssertionBearerSubjectConfirmationMissing, + #[error("SAML Assertion is missing SubjectConfirmationData")] + AssertionSubjectConfirmationMissing, + #[error("SAML Assertion SubjectConfirmation expired at: {}", time)] + AssertionSubjectConfirmationExpired { time: String }, + #[error("SAML Assertion SubjectConfirmation is not valid until: {}", time)] + AssertionSubjectConfirmationExpiredBefore { time: String }, #[error( "SAML Assertion Issuer does not match IDP entity ID: {:?} != {:?}", issuer, @@ -53,6 +67,20 @@ pub enum Error { requirement )] AssertionConditionAudienceRestrictionFailed { requirement: String }, + #[error( + "SAML Assertion Recipient does not match SP ACS URL. {:?} != {:?}", + assertion_recipient, + sp_acs_url + )] + AssertionRecipientMismatch { + assertion_recipient: Option, + sp_acs_url: Option, + }, + #[error( + "SAML Assertion 'InResponseTo' does not match any of the possible request IDs: {:?}", + possible_ids + )] + AssertionInResponseToInvalid { possible_ids: Vec }, #[error( "SAML Response 'InResponseTo' does not match any of the possible request IDs: {:?}", possible_ids @@ -103,6 +131,9 @@ pub enum Error { #[error("Failed to parse SAMLResponse")] FailedToParseSamlResponse(#[source] quick_xml::DeError), + #[error("Failed to parse reduced signed SAML assertion")] + FailedToParseSamlAssertion(#[source] Box), + #[error("Error parsing the XML in the crypto provider")] CryptoXmlError(#[source] CryptoError), @@ -259,14 +290,13 @@ impl ServiceProvider { } fn name_id_format(&self) -> Option { - self.authn_name_id_format.as_ref() - .map(|v| -> String { - if v.is_empty() { - NameIdFormat::TransientNameIDFormat.value().to_string() - } else { - v.to_string() - } - }) + self.authn_name_id_format.as_ref().map(|v| -> String { + if v.is_empty() { + NameIdFormat::TransientNameIDFormat.value().to_string() + } else { + v.to_string() + } + }) } pub fn sso_binding_location(&self, binding: &str) -> Option { @@ -346,15 +376,63 @@ impl ServiceProvider { response_xml: &str, possible_request_ids: Option<&[&str]>, ) -> Result { - let reduced_xml = if let Some(sign_certs) = self.idp_signing_certs()? { - Crypto::reduce_xml_to_signed(response_xml, &sign_certs) - .map_err(|_e| Error::FailedToValidateSignature)? - } else { - String::from(response_xml) - }; - let response: Response = reduced_xml - .parse() - .map_err(Error::FailedToParseSamlResponse)?; + self.parse_xml_response_with_mode( + response_xml, + possible_request_ids, + ReduceMode::ValidateAndMarkNoAncestors, + ) + } + + pub fn parse_xml_response_with_mode( + &self, + response_xml: &str, + possible_request_ids: Option<&[&str]>, + reduce_mode: ReduceMode, + ) -> Result { + let (reduced_xml, reduced_from_verified_signature) = + if let Some(sign_certs) = self.idp_signing_certs()? { + ( + Crypto::reduce_xml_to_signed(response_xml, &sign_certs, reduce_mode) + .map_err(|_e| Error::FailedToValidateSignature)?, + true, + ) + } else { + (String::from(response_xml), false) + }; + + match root_element_local_name(&reduced_xml).as_deref() { + Some("Response") => { + let response: Response = reduced_xml + .parse() + .map_err(Error::FailedToParseSamlResponse)?; + return self.validate_parsed_response(response, possible_request_ids); + } + Some("Assertion") if reduced_from_verified_signature => { + let assertion: Assertion = reduced_xml + .parse() + .map_err(Error::FailedToParseSamlAssertion)?; + self.validate_assertion(&assertion, possible_request_ids)?; + return Ok(assertion); + } + Some(root_name) => { + return Err(Error::FailedToParseSamlResponse(DeError::Custom(format!( + "unexpected SAML root element `{root_name}`" + )))); + } + None => {} + } + + let response_parse_error = reduced_xml + .parse::() + .expect_err("non-empty XML without a root element must fail to parse as Response"); + Err(Error::FailedToParseSamlResponse(response_parse_error)) + } + + fn validate_parsed_response( + &self, + response: Response, + possible_request_ids: Option<&[&str]>, + ) -> Result { self.validate_destination(&response)?; let mut request_id_valid = false; if self.allow_idp_initiated { @@ -401,13 +479,13 @@ impl ServiceProvider { } } - if let Some(encrypted_assertion) = &response.encrypted_assertion { + if let Some(_encrypted_assertion) = &response.encrypted_assertion { #[cfg(feature = "xmlsec")] return self - .decrypt_assertion(encrypted_assertion) + .decrypt_assertion(_encrypted_assertion) .and_then(|assertion| { self.validate_assertion(&assertion, possible_request_ids) - .and(Ok(assertion)) + .map(|()| assertion) }); #[cfg(not(feature = "xmlsec"))] @@ -420,6 +498,7 @@ impl ServiceProvider { } } + #[cfg(feature = "xmlsec")] fn decrypt_assertion( &self, encrypted_assertion: &EncryptedAssertion, @@ -432,7 +511,7 @@ impl ServiceProvider { fn validate_assertion( &self, assertion: &Assertion, - _possible_request_ids: Option<&[&str]>, + possible_request_ids: Option<&[&str]>, ) -> Result<(), Error> { if assertion.issue_instant + self.max_issue_delay < Utc::now() { return Err(Error::AssertionExpired { @@ -481,9 +560,102 @@ impl ServiceProvider { } } + self.validate_assertion_subject_confirmation(assertion, possible_request_ids)?; + Ok(()) } + fn validate_assertion_subject_confirmation( + &self, + assertion: &Assertion, + possible_request_ids: Option<&[&str]>, + ) -> Result<(), Error> { + let confirmations = assertion + .subject + .as_ref() + .and_then(|subject| subject.subject_confirmations.as_ref()) + .into_iter() + .flatten() + .collect::>(); + let bearer_confirmations = confirmations + .iter() + .filter(|confirmation| { + confirmation.method.as_deref() == Some(SUBJECT_CONFIRMATION_METHOD_BEARER) + }) + .collect::>(); + + if bearer_confirmations.is_empty() { + return Err(Error::AssertionBearerSubjectConfirmationMissing); + } + + let mut first_error = None; + for confirmation in bearer_confirmations { + let Some(data) = confirmation.subject_confirmation_data.as_ref() else { + first_error.get_or_insert(Error::AssertionSubjectConfirmationMissing); + continue; + }; + + if let Some(not_before) = data.not_before { + if Utc::now() < not_before - self.max_clock_skew { + first_error.get_or_insert(Error::AssertionSubjectConfirmationExpiredBefore { + time: (not_before - self.max_clock_skew) + .to_rfc3339_opts(SecondsFormat::Secs, true), + }); + continue; + } + } + + if let Some(not_on_or_after) = data.not_on_or_after { + if not_on_or_after + self.max_clock_skew < Utc::now() { + first_error.get_or_insert(Error::AssertionSubjectConfirmationExpired { + time: (not_on_or_after + self.max_clock_skew) + .to_rfc3339_opts(SecondsFormat::Secs, true), + }); + continue; + } + } else { + first_error.get_or_insert(Error::AssertionSubjectConfirmationMissing); + continue; + } + + if data.recipient.as_deref() != self.acs_url.as_deref() { + first_error.get_or_insert(Error::AssertionRecipientMismatch { + assertion_recipient: data.recipient.clone(), + sp_acs_url: self.acs_url.clone(), + }); + continue; + } + + if self.allow_idp_initiated { + return Ok(()); + } + + if let Some(possible_request_ids) = possible_request_ids { + let request_id_valid = data + .in_response_to + .as_deref() + .is_some_and(|id| possible_request_ids.iter().any(|possible| possible == &id)); + if request_id_valid { + return Ok(()); + } + + first_error.get_or_insert(Error::AssertionInResponseToInvalid { + possible_ids: possible_request_ids + .iter() + .map(|id| id.to_string()) + .collect(), + }); + continue; + } + + first_error.get_or_insert(Error::AssertionInResponseToInvalid { + possible_ids: Vec::new(), + }); + } + + Err(first_error.unwrap_or(Error::AssertionSubjectConfirmationMissing)) + } + fn validate_destination(&self, response: &Response) -> Result<(), Error> { if (response.signature.is_some() || response.destination.is_some()) && response.destination.as_deref() != self.acs_url.as_deref() @@ -529,6 +701,21 @@ impl ServiceProvider { } } +fn root_element_local_name(xml: &str) -> Option { + let mut reader = Reader::from_str(xml); + + loop { + match reader.read_event() { + Ok(XmlEvent::Start(start)) | Ok(XmlEvent::Empty(start)) => { + return Some(String::from_utf8_lossy(start.local_name().as_ref()).into_owned()); + } + Ok(XmlEvent::Eof) => return None, + Ok(_) => {} + Err(_) => return None, + } + } +} + fn parse_certificates(key_descriptor: &KeyDescriptor) -> Result, Error> { key_descriptor .key_info @@ -593,6 +780,7 @@ impl AuthnRequest { } // todo: how does this fit to the seperate crypto? + #[cfg(feature = "xmlsec")] pub fn signed_redirect( &self, relay_state: &str, @@ -608,4 +796,12 @@ impl AuthnRequest { } } + #[cfg(not(feature = "xmlsec"))] + pub fn signed_redirect( + &self, + _relay_state: &str, + _private_key: &::PrivateKey, + ) -> Result, Box> { + Err(Box::new(CryptoError::CryptoDisabled)) + } } diff --git a/src/service_provider/tests.rs b/src/service_provider/tests.rs index 5f77d7e..a15211a 100644 --- a/src/service_provider/tests.rs +++ b/src/service_provider/tests.rs @@ -1,24 +1,135 @@ +#![cfg(feature = "xmlsec")] + #[cfg(test)] mod encrypted_assertion_tests { + use crate::crypto::ReduceMode; use crate::metadata::EntityDescriptor; + use crate::schema::{Assertion, Response}; use crate::service_provider::{Error, ServiceProvider, ServiceProviderBuilder}; - use chrono::{Duration, Utc}; + use crate::traits::ToXml; + use chrono::{DateTime, Duration, Utc}; use openssl::pkey::PKey; + fn encrypted_response_max_issue_delay(response_xml: &str) -> Duration { + let response: Response = response_xml.parse().unwrap(); + Utc::now() - response.issue_instant + Duration::seconds(60) + } + + fn required_clock_skew_for_not_before(not_before: DateTime) -> Duration { + std::cmp::max( + not_before - Utc::now() + Duration::seconds(60), + Duration::zero(), + ) + } + + fn required_clock_skew_for_not_on_or_after(not_on_or_after: DateTime) -> Duration { + std::cmp::max( + Utc::now() - not_on_or_after + Duration::seconds(60), + Duration::zero(), + ) + } + + fn required_validation_clock_skew(assertion: &Assertion) -> Duration { + let mut required = Duration::zero(); + + if let Some(conditions) = assertion.conditions.as_ref() { + if let Some(not_before) = conditions.not_before { + required = std::cmp::max(required, required_clock_skew_for_not_before(not_before)); + } + if let Some(not_on_or_after) = conditions.not_on_or_after { + required = std::cmp::max( + required, + required_clock_skew_for_not_on_or_after(not_on_or_after), + ); + } + } + + if let Some(confirmations) = assertion + .subject + .as_ref() + .and_then(|subject| subject.subject_confirmations.as_ref()) + { + for confirmation in confirmations { + if let Some(data) = confirmation.subject_confirmation_data.as_ref() { + if let Some(not_before) = data.not_before { + required = + std::cmp::max(required, required_clock_skew_for_not_before(not_before)); + } + if let Some(not_on_or_after) = data.not_on_or_after { + required = std::cmp::max( + required, + required_clock_skew_for_not_on_or_after(not_on_or_after), + ); + } + } + } + } + + required + } + + fn decrypted_encrypted_response_assertion( + response_xml: &str, + key: &PKey, + ) -> Assertion { + let response: Response = response_xml.parse().unwrap(); + response + .encrypted_assertion + .as_ref() + .unwrap() + .decrypt(key) + .unwrap() + } + + fn encrypted_response_validation_clock_skew( + response_xml: &str, + key: &PKey, + ) -> Duration { + let assertion = decrypted_encrypted_response_assertion(response_xml, key); + required_validation_clock_skew(&assertion) + } + + fn encrypted_response_entity_id( + response_xml: &str, + key: &PKey, + ) -> String { + decrypted_encrypted_response_assertion(response_xml, key) + .conditions + .as_ref() + .and_then(|conditions| conditions.audience_restrictions.as_ref()) + .and_then(|restrictions| restrictions.iter().flat_map(|r| r.audience.iter()).next()) + .cloned() + .unwrap_or_else(|| "example".to_string()) + } + // Helper function to create a service provider with a private key - fn create_sp_with_private_key(key: PKey) -> ServiceProvider { + fn create_sp_with_private_key_for_response( + response_xml: &str, + key: PKey, + ) -> ServiceProvider { + let max_clock_skew = encrypted_response_validation_clock_skew(response_xml, &key); + let entity_id = encrypted_response_entity_id(response_xml, &key); + // Create a service provider with the private key ServiceProviderBuilder::default() .idp_metadata(create_mock_idp()) .allow_idp_initiated(true) .key(key) - .entity_id(Some("example".to_string())) // Set entity_id to match the audience requirement - .max_clock_skew(Duration::days(365)) - .max_issue_delay(Duration::days(365)) + .entity_id(Some(entity_id)) + .max_clock_skew(max_clock_skew) + .max_issue_delay(encrypted_response_max_issue_delay(response_xml)) .build() .unwrap() } + fn create_sp_with_private_key(key: PKey) -> ServiceProvider { + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_encrypted.xml" + )); + create_sp_with_private_key_for_response(response_xml, key) + } + fn create_mock_idp() -> EntityDescriptor { EntityDescriptor { entity_id: Some("saml-mock".to_string()), @@ -29,20 +140,20 @@ mod encrypted_assertion_tests { #[test] fn test_missing_private_key() { + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_encrypted.xml" + )); + // Create a service provider without a private key let sp = ServiceProviderBuilder::default() .idp_metadata(create_mock_idp()) .max_clock_skew(Duration::days(365)) - .max_issue_delay(Duration::days(365)) + .max_issue_delay(encrypted_response_max_issue_delay(response_xml)) .build() .unwrap(); // Sample response with an encrypted assertion - let response_xml = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/test_vectors/response_encrypted.xml" - )); - // Attempt to parse the response let result = sp.parse_xml_response(response_xml, Some(&["example"])); @@ -96,17 +207,380 @@ mod encrypted_assertion_tests { )); let key = PKey::private_key_from_pem(pkey).unwrap(); - let sp = create_sp_with_private_key(key); - let response_xml = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), - "/test_vectors/response_encrypted.xml" + "/test_vectors/response_encrypted_valid.xml" )); + let sp = create_sp_with_private_key_for_response(response_xml, key); let result = sp.parse_xml_response(response_xml, Some(&["example"])); - println!("Result: {:?}", result); - assert!(result.is_ok()); } + + fn extract_first_certificate(xml: &str) -> String { + let start = xml + .find("") + .expect("response should contain a signing certificate"); + let start = start + "".len(); + let end = xml[start..] + .find("") + .expect("response should contain a closing certificate tag"); + xml[start..start + end].to_string() + } + + fn create_predigest_assertion_sp(acs_url: &str) -> ServiceProvider { + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let cert = extract_first_certificate(response_xml); + let metadata_xml = format!( + r#" + + + + + {cert} + + + + +"# + ); + create_predigest_assertion_sp_with_metadata(acs_url, metadata_xml.parse().unwrap()) + } + + fn create_predigest_assertion_sp_without_signing_certs(acs_url: &str) -> ServiceProvider { + create_predigest_assertion_sp_with_metadata( + acs_url, + EntityDescriptor { + entity_id: Some("http://idp.example.com/metadata.php".to_string()), + ..Default::default() + }, + ) + } + + fn create_predigest_assertion_sp_with_metadata( + acs_url: &str, + idp_metadata: EntityDescriptor, + ) -> ServiceProvider { + let response_instant = "2014-07-17T01:01:48Z".parse::>().unwrap(); + let max_issue_delay = Utc::now() - response_instant + Duration::seconds(60); + + ServiceProviderBuilder::default() + .idp_metadata(idp_metadata) + .entity_id(Some("http://sp.example.com/demo1/metadata.php".to_string())) + .acs_url(Some(acs_url.to_string())) + .max_clock_skew(Duration::days(5000)) + .max_issue_delay(max_issue_delay) + .build() + .unwrap() + } + + fn refresh_assertion_validation_windows(assertion: &mut Assertion) { + if let Some(conditions) = assertion.conditions.as_mut() { + conditions.not_before = Some(Utc::now() - Duration::minutes(1)); + conditions.not_on_or_after = Some(Utc::now() + Duration::days(1)); + } + + if let Some(confirmations) = assertion + .subject + .as_mut() + .and_then(|subject| subject.subject_confirmations.as_mut()) + { + for confirmation in confirmations { + if let Some(data) = confirmation.subject_confirmation_data.as_mut() { + data.not_before = Some(Utc::now() - Duration::minutes(1)); + data.not_on_or_after = Some(Utc::now() + Duration::days(1)); + } + } + } + } + + #[test] + fn test_predigest_accepts_signed_assertion_response() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + + let assertion = sp + .parse_xml_response_with_mode( + response_xml, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ReduceMode::PreDigest, + ) + .expect("signed assertion should parse in pre-digest mode"); + + assert_eq!( + assertion.issuer.value.as_deref(), + Some("http://idp.example.com/metadata.php") + ); + } + + #[test] + fn test_predigest_rejects_mismatched_request_id() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + + let error = sp + .parse_xml_response_with_mode( + response_xml, + Some(&["WRONG_REQUEST_ID"]), + ReduceMode::PreDigest, + ) + .expect_err("mismatched request id should fail in pre-digest mode"); + + assert!(matches!(error, Error::AssertionInResponseToInvalid { .. })); + } + + #[test] + fn test_predigest_rejects_mismatched_recipient() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?wrong"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + + let error = sp + .parse_xml_response_with_mode( + response_xml, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ReduceMode::PreDigest, + ) + .expect_err("mismatched recipient should fail in pre-digest mode"); + + assert!(matches!(error, Error::AssertionRecipientMismatch { .. })); + } + + #[test] + fn test_predigest_response_api_rejects_unsigned_bare_assertion_without_signing_certs() { + let sp = create_predigest_assertion_sp_without_signing_certs( + "http://sp.example.com/demo1/index.php?acs", + ); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let response: Response = response_xml.parse().unwrap(); + let assertion_xml = response.assertion.as_ref().unwrap().to_string().unwrap(); + + let error = sp + .parse_xml_response_with_mode( + &assertion_xml, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ReduceMode::PreDigest, + ) + .expect_err("response parser should not accept a bare unsigned assertion"); + + assert!(matches!(error, Error::FailedToParseSamlResponse(_))); + } + + #[test] + fn test_default_response_api_ignores_unsigned_response_wrapper_for_signed_assertions() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let mutated_response_xml = response_xml + .replace( + r#"Destination="http://sp.example.com/demo1/index.php?acs""#, + r#"Destination="http://sp.example.com/demo1/index.php?wrong""#, + ) + .replacen( + r#"InResponseTo="ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685""#, + r#"InResponseTo="WRONG_REQUEST_ID""#, + 1, + ); + + let assertion = sp + .parse_xml_response( + &mutated_response_xml, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect("default response parsing should trust the signed assertion, not the unsigned response wrapper"); + + assert_eq!( + assertion.issuer.value.as_deref(), + Some("http://idp.example.com/metadata.php") + ); + } + + #[test] + fn test_response_validation_requires_assertion_recipient_binding() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let mut response: Response = response_xml.parse().unwrap(); + response.destination = None; + response + .assertion + .as_mut() + .unwrap() + .subject + .as_mut() + .unwrap() + .subject_confirmations + .as_mut() + .unwrap()[0] + .subject_confirmation_data + .as_mut() + .unwrap() + .recipient = None; + + let error = sp + .validate_parsed_response( + response, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect_err("missing bearer recipient should fail response validation"); + + assert!(matches!(error, Error::AssertionRecipientMismatch { .. })); + } + + #[test] + fn test_validate_assertion_requires_subject_confirmation_expiry() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let response: Response = response_xml.parse().unwrap(); + let mut assertion = response.assertion.unwrap(); + refresh_assertion_validation_windows(&mut assertion); + assertion + .subject + .as_mut() + .unwrap() + .subject_confirmations + .as_mut() + .unwrap()[0] + .subject_confirmation_data + .as_mut() + .unwrap() + .not_on_or_after = None; + + let error = sp + .validate_assertion( + &assertion, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect_err("missing subject confirmation expiry should be rejected"); + + assert!(matches!(error, Error::AssertionSubjectConfirmationMissing)); + } + + #[test] + fn test_validate_assertion_requires_bearer_subject_confirmation() { + let sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let response: Response = response_xml.parse().unwrap(); + let mut assertion = response.assertion.unwrap(); + refresh_assertion_validation_windows(&mut assertion); + assertion + .subject + .as_mut() + .unwrap() + .subject_confirmations + .as_mut() + .unwrap()[0] + .method = Some("urn:oasis:names:tc:SAML:2.0:cm:holder-of-key".to_string()); + + let error = sp + .validate_assertion( + &assertion, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect_err("non-bearer confirmations should be rejected"); + + assert!(matches!( + error, + Error::AssertionBearerSubjectConfirmationMissing + )); + } + + #[test] + fn test_validate_assertion_rejects_expired_subject_confirmation() { + let mut sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + sp.max_clock_skew = Duration::zero(); + + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let response: Response = response_xml.parse().unwrap(); + let mut assertion = response.assertion.unwrap(); + refresh_assertion_validation_windows(&mut assertion); + assertion + .subject + .as_mut() + .unwrap() + .subject_confirmations + .as_mut() + .unwrap()[0] + .subject_confirmation_data + .as_mut() + .unwrap() + .not_on_or_after = Some(Utc::now() - Duration::minutes(1)); + + let error = sp + .validate_assertion( + &assertion, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect_err("expired subject confirmation should be rejected"); + + assert!(matches!( + error, + Error::AssertionSubjectConfirmationExpired { .. } + )); + } + + #[test] + fn test_validate_assertion_rejects_not_yet_valid_subject_confirmation() { + let mut sp = create_predigest_assertion_sp("http://sp.example.com/demo1/index.php?acs"); + sp.max_clock_skew = Duration::zero(); + + let response_xml = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_vectors/response_signed_assertion.xml" + )); + let response: Response = response_xml.parse().unwrap(); + let mut assertion = response.assertion.unwrap(); + refresh_assertion_validation_windows(&mut assertion); + assertion + .subject + .as_mut() + .unwrap() + .subject_confirmations + .as_mut() + .unwrap()[0] + .subject_confirmation_data + .as_mut() + .unwrap() + .not_before = Some(Utc::now() + Duration::minutes(1)); + + let error = sp + .validate_assertion( + &assertion, + Some(&["ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685"]), + ) + .expect_err("future subject confirmation should be rejected"); + + assert!(matches!( + error, + Error::AssertionSubjectConfirmationExpiredBefore { .. } + )); + } } diff --git a/src/signature.rs b/src/signature.rs index dcefde3..ae52bbb 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -1,3 +1,4 @@ +use crate::crypto::CertificateDer; use crate::key_info::{KeyInfo, X509Data}; use base64::{engine::general_purpose, Engine as _}; use quick_xml::events::{BytesEnd, BytesStart, BytesText, Event}; @@ -5,7 +6,6 @@ use quick_xml::Writer; use serde::Deserialize; use std::io::Cursor; use std::str::FromStr; -use crate::crypto::CertificateDer; const NAME: &str = "ds:Signature"; const SCHEMA: (&str, &str) = ("xmlns:ds", "http://www.w3.org/2000/09/xmldsig#"); diff --git a/test_vectors/ancestor_attack_signed.xml b/test_vectors/ancestor_attack_signed.xml new file mode 100644 index 0000000..deb41b3 --- /dev/null +++ b/test_vectors/ancestor_attack_signed.xml @@ -0,0 +1,16 @@ + + + https://attacker.evil.com + + https://capriza.github.io/samling/samling.html + + + + https://capriza.github.io/samling/samling.htmlM/k75WWiSQAYuyO4pDdKrgIffDvHufJaz0sp9SrX0hc=gv1T8Z3vm/b2+3+pirOATtCPs3IT+KjPOp/ws3wISgl8zf78s+yLtmioLTRn48oPKvV3EQXn05Jr2xSpvj4hw3N/PvXq93sZmiv6hW9VPmN/ujZIJFr3WgtxzmRESn+VA1iJ/rrTr2HtOpQtaMFBA1O2xQRdozLb4Ukxt4jMJlo=MIICpzCCAhACCQDuFX0Db5iljDANBgkqhkiG9w0BAQsFADCBlzELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEjAQBgNVBAcMCVBhbG8gQWx0bzEQMA4GA1UECgwHU2FtbGluZzEPMA0GA1UECwwGU2FsaW5nMRQwEgYDVQQDDAtjYXByaXphLmNvbTEmMCQGCSqGSIb3DQEJARYXZW5naW5lZXJpbmdAY2Fwcml6YS5jb20wHhcNMTgwNTE1MTgxMTEwWhcNMjgwNTEyMTgxMTEwWjCBlzELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEjAQBgNVBAcMCVBhbG8gQWx0bzEQMA4GA1UECgwHU2FtbGluZzEPMA0GA1UECwwGU2FsaW5nMRQwEgYDVQQDDAtjYXByaXphLmNvbTEmMCQGCSqGSIb3DQEJARYXZW5naW5lZXJpbmdAY2Fwcml6YS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAJEBNDJKH5nXr0hZKcSNIY1l4HeYLPBEKJLXyAnoFTdgGrvi40YyIx9lHh0LbDVWCgxJp21BmKll0CkgmeKidvGlr3FUwtETro44L+SgmjiJNbftvFxhNkgA26O2GDQuBoQwgSiagVadWXwJKkodH8tx4ojBPYK1pBO8fHf3wOnxAgMBAAEwDQYJKoZIhvcNAQELBQADgYEACIylhvh6T758hcZjAQJiV7rMRg+Omb68iJI4L9f0cyBcJENR+1LQNgUGyFDMm9Wm9o81CuIKBnfpEE2Jfcs76YVWRJy5xJ11GFKJJ5T0NEB7txbUQPoJOeNoE736lF5vYw6YKp8fJqPW0L2PLWe9qTn8hxpdnjo3k6r5gXyl8tk=ancestorurn:oasis:names:tc:SAML:2.0:ac:classes:unspecified + + + diff --git a/test_vectors/object_attack_response.xml b/test_vectors/object_attack_response.xml new file mode 100644 index 0000000..497eb48 --- /dev/null +++ b/test_vectors/object_attack_response.xml @@ -0,0 +1,53 @@ + + + https://capriza.github.io/samling/samling.html + + + + + https://capriza.github.io/samling/samling.html + + + + + + + + + + + fOtw9VUYwLgYZkkqnhoSUUtwbZJ55JLhnJ6oaHhr18I= + + + a7Q0Z4ir9KGLcycVebOCJGre7K9uUDpELNbZbtAkSr22rVC2TITvdtAQfN7M/dFG +5+blo/wgl4zxkXZ/9KUE4vYep1cHHmr+KhkKD4/h8wWf5p0PRagy0umHmgUQrSpB +ruK+WWOTfOQWpwmWSEOGUcJ16qshg+AKCvCh/plNJis= + + signing + + + + + malicious_name + + + + + + + + + ancestor + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified + + + + + diff --git a/test_vectors/object_attack_response_template.xml b/test_vectors/object_attack_response_template.xml new file mode 100644 index 0000000..c2595a1 --- /dev/null +++ b/test_vectors/object_attack_response_template.xml @@ -0,0 +1,45 @@ + + https://capriza.github.io/samling/samling.html + + + + + https://capriza.github.io/samling/samling.html + + + + + + + + + + + + + + + + signing + + + + ancestor + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified + + + + + diff --git a/test_vectors/response_encrypted_valid.xml b/test_vectors/response_encrypted_valid.xml new file mode 100644 index 0000000..f3655f2 --- /dev/null +++ b/test_vectors/response_encrypted_valid.xml @@ -0,0 +1,10 @@ + + saml-mock + + + + KIn/D5GeA6sSBEC4t5rpulHZ+dEJl/UwrhMqaKnsrAylI3UBHfA6qriHxSeNW+qnacIUfgSUycGK3SNkb2s7GNbguRRubYGBMH4Z3XkxDc9kG3oQq6MLpVqNlOul/Fbzm0r3LGAgroWrCGwdJQ/beT99zKSTWn8yWY3YHr9n+n8= + + wfPzmmuQGDC4tp4/hwVWp6QUauN64bTt5j7pb6q//KN5w1aTsTCQgXtkxC0nHF0MCIqoYTQoHoAWnfpjZDRE4pSMfVM76PQFgIaNnzCBwEZc1KeEvLsKuyv3y3dvZsD82Y5XsJG1JZzaDhtF/iUsDFaRt6i+EtGsX/1RLMQim28RUTer6/NIO+rc02qd106gVqIwk/yi6JLxmM7hpf5/erLpw2C4LJ9m8e05pX36n80sUgl+rdhahDbYzwqfVwOQ6/7ptE+dpD5DmzbmGwKd7oSYvs5t6dwfWwuIZ/ylsZiNU9DZgYu7oC8vFIGHIHP6GGgTmAl8mQPjYNvDcvoZRtYW8yk4sMHImRQHp+vIDOcBtYqqzHjnzTSH8aHghUf56zk7JuBkqR5EObRlUyJMMpjoVKL8xWbgO6VXLjskQbED/nqaya0hLj4M/uoK+kqr7g3YU6ZEe8sTzgC1vnLGG3thI8VdME6cRmq/eXkrNGMqDh7RJ8niBOs9F3I2PpCAxab7ScfYzZG0tiL/yntv4rlY8hcitloLeOo+QSl++zHCak3Sn8437HPmEv9txz503nKHLqnxu0ryEHFROJpz6LIVqPBIiC7V6aYjNHx12e1na6MHAKn1XIeeRHxpZZMAueA/wTkZoYJ1Oirc+QRcHvNY/UjfTqfyiWRKFN5dEBT+sTcIrNdKxdOMkt7VLS7RuJ3Cq+GPlFiD/H0etg9AHuLNDSg4ZLJPgOFRQGWzysOBaAFu/bbKqCiNOuZl+c3CgmaovYjLH/EByQ6r8X9VXnjKNPe8Ndj8KKPGim83uCv5MbVpZog+S5aEVV397LL21+lEYeVSVOYXkR9or2WDolmXrZSPZTMrVyf+VHL0vt2rBx/I+k2wpHzZWV2xSwPrPlsBbQh9h7pkQ6Mz4CQgKmv1wirYW9wFddsaVijtMrC49mGd8sMLyH+q1X9dqOTVD3TqG43dGuriGTwr0GgQufPztst54p7FWo2jcveUvC8uqxkq5SSUuy3NHcV134oZviMRTX6h3V5yqt4Og+ioTAIanwQyZ33MORuOWljTte2ULp37mGMlcVt3z80f5S/TNzeX5AJ7lrmx5r/YwI09MVlR5tyjyFxUvJ0yRsNrOEgxlwWKqhZAn2CX/punBfOMKZnnkVUbLfJed7c03d89kI4szUXHP82+aYN3bAMjcLWZZWvRWIKHpeadgHxEqpNTiZ94ZLglNA645t6dycBG/W1rgVg5yNKkEtuvh9oWPeBoOoYxmgIGPv9K415ZS4Cmrl7PppQNa0Cjg3HNOJ/gPuvDoUcF6ElfEhnmdU742jorXyQExr6zHoYIf8xByC98UWfpG/llMCLnPwU5gQ9XVOPN7wUikSJ7cgPzz2S2Dpl7ACdrG2bT0INmYw99MlsYf93yAzwTwdo6GGUCKRT9LQZMI7ZaghjMhVnxEZQrgMTL9PYjC+pu8DwCTUOCRrfDvq3gOmcSz+VNX6wgxqCtOfRMVBQ1QwPrrnfRdJdbejERcBGyByCdVtenvzlLYWwFUCz2n5Na2ClSf9aHqHMia9cyGsNrl9Iwt4htT0xNA12RTfbx8ajLY5Xg2HOkJn/L + + diff --git a/test_vectors/xpath_transform.xml b/test_vectors/xpath_transform.xml new file mode 100644 index 0000000..db39871 --- /dev/null +++ b/test_vectors/xpath_transform.xml @@ -0,0 +1,46 @@ + + + https://capriza.github.io/samling/samling.html + + + + + https://capriza.github.io/samling/samling.html + + + + + + + + + not(ancestor-or-self::saml:Assertion[@ID='malicious_assertion']) + + + + + fOtw9VUYwLgYZkkqnhoSUUtwbZJ55JLhnJ6oaHhr18I= + + + RIvr0btopD3/SPmdPzV3RphHoI6Qa2ilMm2cvEUFCBdB6tKIP+tDMNKwBVaHxAu8 +dL8c2ZneoXiwOMoLnTHzFMcMu14j1HMmr0qRoPrGNR2XQVKs9dS3r6FE9S7hYB2y +hdJxZVIW5VBYBsrGiBM/0ADQyfTrmBD3mxoq8q5FC8o= + + signing + + + + ancestor + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified + + + + https://capriza.github.io/samling/samling.htmlancestor + diff --git a/test_vectors/xpath_transform_template.xml b/test_vectors/xpath_transform_template.xml new file mode 100644 index 0000000..c3a13de --- /dev/null +++ b/test_vectors/xpath_transform_template.xml @@ -0,0 +1,48 @@ + + https://capriza.github.io/samling/samling.html + + + + + https://capriza.github.io/samling/samling.html + + + + + + + + + not(ancestor-or-self::saml:Assertion[@ID='malicious_assertion']) + + + + + + + + + + signing + + + + ancestor + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified + + + + + diff --git a/test_vectors/xpointer_attack_signed.xml b/test_vectors/xpointer_attack_signed.xml new file mode 100644 index 0000000..dffb489 --- /dev/null +++ b/test_vectors/xpointer_attack_signed.xml @@ -0,0 +1,32 @@ + + + https://idp.example.com + + + + + + + + + + + VI1E515fVc0UjEE3xn95znrlbR8GtYEygN9TTcE2e7E= + + + IxjoDHQY0B9u4O8GQbtJXcXQo3OQWhQJtc+t01tQqDEARAa8B4o9udWaqVnLvtGs +UPGZZoT6lsO8jPqqQ7oecnWBf4fY5MYl77REEOvBuRW9LoD51bPGFpICnX9nIg5g +mpF7prXj9Y7khRywC/xgDLgcnv9s+hQj+iICDaboHE4= + + signing + + + + attacker@evil.com + admin + + + legitimate@example.com + user + + diff --git a/test_vectors/xpointer_attack_template.xml b/test_vectors/xpointer_attack_template.xml new file mode 100644 index 0000000..0fcefc5 --- /dev/null +++ b/test_vectors/xpointer_attack_template.xml @@ -0,0 +1,31 @@ + + + https://idp.example.com + + + + + + + + + + + + + + + + signing + + + + legitimate@example.com + user + +