Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,14 @@ impl From<Vec<u8>> for CertificateDer {
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ReduceMode {
/// Returns xmlsec's pre-digest content for exactly one verified reference across the document.
///
/// This is the strictest mode. It only works if there is one Signature element
/// in the XML.
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
/// This is kept for compatibility with older callers. It is not recommended because unsigned
/// ancestors can survive reduction in this mode.
ValidateAndMark,
/// Returns a rooted XML document containing only xmlsec-verified content.
Expand Down
16 changes: 13 additions & 3 deletions src/crypto/xmlsec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,20 @@ impl super::CryptoProvider for XmlSec {
}

if reduce_mode == ReduceMode::PreDigest {
if predigest_results.len() != 1 {
return Err(CryptoError::InvalidSignature);
if predigest_results.len() == 1 {
return Ok(predigest_results.remove(0));
} else {
for predigest in predigest_results {
return match crate::service_provider::root_element_local_name(&predigest)
.as_deref()
{
// We want to trust the full response as that includes the relevant data.
// Everything is signed so even if we found a signed Assertion in here we could trust it, for now lets keep it minimal.
Some("Response") => Ok(predigest),
_ => Err(CryptoError::InvalidSignature),
};
}
}
return Ok(predigest_results.remove(0));
}

if selections.is_empty() {
Expand Down
6 changes: 2 additions & 4 deletions src/crypto/xmlsec/wrapper/xmldsig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ impl XmlSecSignatureContext {
let uri = if ref_ctx.uri.is_null() {
None
} else {
let uri_cstr =
std::ffi::CStr::from_ptr(ref_ctx.uri as *const std::ffi::c_char);
let uri_cstr = std::ffi::CStr::from_ptr(ref_ctx.uri as *const std::ffi::c_char);
Some(
uri_cstr
.to_str()
Expand All @@ -85,8 +84,7 @@ impl XmlSecSignatureContext {

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 as usize)?;
let predigest_xml = predigest_xml_from_raw_buffer(data_ptr, data_size as usize)?;

result.push(VerifiedReference { uri, predigest_xml });
}
Expand Down
42 changes: 21 additions & 21 deletions src/idp/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,18 +371,18 @@ fn test_malicious_ancestors_not_included() {
"/test_vectors/idp_2_metadata_public.pem"
)));

for (reduce_mode, result) in [
(ReduceMode::PreDigest, true),
(ReduceMode::ValidateAndMark, true),
(ReduceMode::ValidateAndMarkNoAncestors, true),
for (reduce_mode, should_contain_attacker_url) in [
(ReduceMode::PreDigest, false),
(ReduceMode::ValidateAndMark, false),
(ReduceMode::ValidateAndMarkNoAncestors, false),
] {
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:?}"
reduced.contains("https://attacker.evil.com"),
should_contain_attacker_url,
"Attacker URL containment mismatch for {reduce_mode:?}: expected {should_contain_attacker_url}"
);
}
}
Expand All @@ -398,18 +398,18 @@ fn test_object_reference_removed() {
"/test_vectors/idp_2_metadata_public.pem"
)));

for (reduce_mode, result) in [
(ReduceMode::PreDigest, true),
(ReduceMode::ValidateAndMark, true),
(ReduceMode::ValidateAndMarkNoAncestors, true),
for (reduce_mode, should_contain_object) in [
(ReduceMode::PreDigest, false),
(ReduceMode::ValidateAndMark, false),
(ReduceMode::ValidateAndMarkNoAncestors, false),
] {
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:?}"
reduced.contains("ds:Object"),
should_contain_object,
"Object containment mismatch for {reduce_mode:?}: expected {should_contain_object}"
);
}
}
Expand Down Expand Up @@ -448,18 +448,18 @@ fn test_xpath_transforms_validated() {
"/test_vectors/idp_2_metadata_public.pem"
)));

for (reduce_mode, result) in [
(ReduceMode::PreDigest, true),
(ReduceMode::ValidateAndMark, true),
(ReduceMode::ValidateAndMarkNoAncestors, true),
for (reduce_mode, should_contain_malicious) in [
(ReduceMode::PreDigest, false),
(ReduceMode::ValidateAndMark, false),
(ReduceMode::ValidateAndMarkNoAncestors, false),
] {
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:?}"
reduced.contains("malicious"),
should_contain_malicious,
"Malicious content containment mismatch for {reduce_mode:?}: expected {should_contain_malicious}"
);
}
}
24 changes: 17 additions & 7 deletions src/service_provider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,24 @@ impl ServiceProvider {
&self,
encoded_resp: &str,
possible_request_ids: Option<&[&str]>,
) -> Result<Assertion, Box<dyn std::error::Error>> {
self.parse_base64_response_with_mode(
encoded_resp,
possible_request_ids,
ReduceMode::default(),
)
}

pub fn parse_base64_response_with_mode(
&self,
encoded_resp: &str,
possible_request_ids: Option<&[&str]>,
reduce_mode: ReduceMode,
) -> Result<Assertion, Box<dyn std::error::Error>> {
let bytes = general_purpose::STANDARD.decode(encoded_resp)?;
let decoded = std::str::from_utf8(&bytes)?;
let assertion = self.parse_xml_response(decoded, possible_request_ids)?;
let assertion =
self.parse_xml_response_with_mode(decoded, possible_request_ids, reduce_mode)?;
Ok(assertion)
}

Expand All @@ -376,11 +390,7 @@ impl ServiceProvider {
response_xml: &str,
possible_request_ids: Option<&[&str]>,
) -> Result<Assertion, Error> {
self.parse_xml_response_with_mode(
response_xml,
possible_request_ids,
ReduceMode::ValidateAndMarkNoAncestors,
)
self.parse_xml_response_with_mode(response_xml, possible_request_ids, ReduceMode::default())
}

pub fn parse_xml_response_with_mode(
Expand Down Expand Up @@ -701,7 +711,7 @@ impl ServiceProvider {
}
}

fn root_element_local_name(xml: &str) -> Option<String> {
pub(crate) fn root_element_local_name(xml: &str) -> Option<String> {
let mut reader = Reader::from_str(xml);

loop {
Expand Down
52 changes: 52 additions & 0 deletions src/service_provider/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,29 @@ mod encrypted_assertion_tests {
);
}

// TODO: this should work, but it does not with ValidateAndMark
#[test]
fn test_validate_and_mark_only_assertion_signed() {
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::ValidateAndMark
)
.unwrap();

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");
Expand Down Expand Up @@ -583,4 +606,33 @@ mod encrypted_assertion_tests {
Error::AssertionSubjectConfirmationExpiredBefore { .. }
));
}

#[test]
fn test_parse_xml_response_with_empty_saml_response() {
let mut sp = create_predigest_assertion_sp_with_metadata("https://api.dev.zoo.dev/auth/saml/00000000-00000000-00000000-00000000/login", r#"<?xml version="1.0"?>
<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://some.idp.test/blah/">
<md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
<md:KeyDescriptor use="signing">
<ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<ds:X509Data>
<ds:X509Certificate>MIIB+jCCAWOgAwIBAgIUdcXUPTE+mOSWxRCJh8ldmDMPzREwDQYJKoZIhvcNAQELBQAwDzENMAsGA1UEAwwEVGVzdDAeFw0yNjA0MDIxMjQzMTNaFw0yNzA0MDIxMjQzMTNaMA8xDTALBgNVBAMMBFRlc3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAJEBNDJKH5nXr0hZKcSNIY1l4HeYLPBEKJLXyAnoFTdgGrvi40YyIx9lHh0LbDVWCgxJp21BmKll0CkgmeKidvGlr3FUwtETro44L+SgmjiJNbftvFxhNkgA26O2GDQuBoQwgSiagVadWXwJKkodH8tx4ojBPYK1pBO8fHf3wOnxAgMBAAGjUzBRMB0GA1UdDgQWBBSLoT4AEwcK1+0IMwgo6JYfA4e8ZTAfBgNVHSMEGDAWgBSLoT4AEwcK1+0IMwgo6JYfA4e8ZTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4GBAAtV1hclbZBD17LMbBwyrTj7szmmeUVISPeFEPaAKqiTXrHwRZ+akajboB2JjT3YYMXX2/eDaSvq9f20vJQUvkEAaYu8eNNDKWgm4btJFAeJT8uGxizmTspdJ0cxFSwxqaosV3qIqJgpwLbzUXEcu6mKfyqDM6AeFZdZevkxmKlE</ds:X509Certificate>
</ds:X509Data>
</ds:KeyInfo>
</md:KeyDescriptor>
<md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://some.idp.test/blah/"/>
<md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>
<md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://some.idp.test/blah/"/>
</md:IDPSSODescriptor>
</md:EntityDescriptor>
"#.parse().unwrap());
sp.allow_idp_initiated = true;
let response_xml = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/test_vectors/multi_saml_response_signed_2.xml"
));

let assertion = sp
.parse_xml_response_with_mode(response_xml, None, ReduceMode::PreDigest)
.expect("signed assertion should parse in pre-digest mode");
}
}
13 changes: 13 additions & 0 deletions test_vectors/cert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-----BEGIN CERTIFICATE-----
MIIB+jCCAWOgAwIBAgIUdcXUPTE+mOSWxRCJh8ldmDMPzREwDQYJKoZIhvcNAQEL
BQAwDzENMAsGA1UEAwwEVGVzdDAeFw0yNjA0MDIxMjQzMTNaFw0yNzA0MDIxMjQz
MTNaMA8xDTALBgNVBAMMBFRlc3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGB
AJEBNDJKH5nXr0hZKcSNIY1l4HeYLPBEKJLXyAnoFTdgGrvi40YyIx9lHh0LbDVW
CgxJp21BmKll0CkgmeKidvGlr3FUwtETro44L+SgmjiJNbftvFxhNkgA26O2GDQu
BoQwgSiagVadWXwJKkodH8tx4ojBPYK1pBO8fHf3wOnxAgMBAAGjUzBRMB0GA1Ud
DgQWBBSLoT4AEwcK1+0IMwgo6JYfA4e8ZTAfBgNVHSMEGDAWgBSLoT4AEwcK1+0I
Mwgo6JYfA4e8ZTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4GBAAtV
1hclbZBD17LMbBwyrTj7szmmeUVISPeFEPaAKqiTXrHwRZ+akajboB2JjT3YYMXX
2/eDaSvq9f20vJQUvkEAaYu8eNNDKWgm4btJFAeJT8uGxizmTspdJ0cxFSwxqaos
V3qIqJgpwLbzUXEcu6mKfyqDM6AeFZdZevkxmKlE
-----END CERTIFICATE-----
6 changes: 6 additions & 0 deletions test_vectors/multi_saml_response.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/env bash

openssl req -new -x509 -key test_vectors/private.der -inform DER -out test_vectors/cert.pem -days 365 -subj "/CN=Test"
openssl pkey -in test_vectors/private.der -inform DER -out test_vectors/private.pem
xmlsec1 --sign --privkey-pem:signing test_vectors/private.pem,test_vectors/cert.pem --id-attr:ID urn:oasis:names:tc:SAML:2.0:assertion:Assertion --id-attr:ID urn:oasis:names:tc:SAML:2.0:protocol:Response --node-xpath "//*[local-name()='Assertion' and namespace-uri()='urn:oasis:names:tc:SAML:2.0:assertion']/*[local-name()='Signature' and namespace-uri()='http://www.w3.org/2000/09/xmldsig#']" --output test_vectors/multi_saml_response_signed.xml --verbose test_vectors/multi_saml_response_template.xml
xmlsec1 --sign --privkey-pem:signing test_vectors/private.pem,test_vectors/cert.pem --id-attr:ID urn:oasis:names:tc:SAML:2.0:assertion:Assertion --id-attr:ID urn:oasis:names:tc:SAML:2.0:protocol:Response --node-xpath "/*[local-name()='Response' and namespace-uri()='urn:oasis:names:tc:SAML:2.0:protocol']/*[local-name()='Signature' and namespace-uri()='http://www.w3.org/2000/09/xmldsig#']" --output test_vectors/multi_saml_response_signed_2.xml --verbose test_vectors/multi_saml_response_signed.xml
92 changes: 92 additions & 0 deletions test_vectors/multi_saml_response_signed.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?xml version="1.0"?>
<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Version="2.0" ID="_f2f02ca12d7292f8a4e03b7b99d71a45f8896c2677" IssueInstant="2022-05-04T15:36:12.631Z" Destination="https://api.dev.zoo.dev/auth/saml/00000000-00000000-00000000-00000000/login">
<saml:Issuer>https://some.idp.test/blah/</saml:Issuer>
<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<ds:SignedInfo>
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
<ds:Reference URI="#_f2f02ca12d7292f8a4e03b7b99d71a45f8896c2677">
<ds:Transforms>
<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
</ds:Transforms>
<ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
<ds:DigestValue/>
</ds:Reference>
</ds:SignedInfo>
<ds:SignatureValue/>
<ds:KeyInfo>
<ds:KeyName>signing</ds:KeyName>
<ds:X509Data>
<ds:X509Certificate/>
</ds:X509Data>
</ds:KeyInfo>
</ds:Signature>
<samlp:Status>
<samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
</samlp:Status>
<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Version="2.0" ID="_a8d644a6c83fc56337b0330d8b0ff684978a5fe56c" IssueInstant="2022-05-04T15:36:12.556Z">
<saml:Issuer>https://some.idp.test/blah/</saml:Issuer>
<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<ds:SignedInfo>
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
<ds:Reference URI="#_a8d644a6c83fc56337b0330d8b0ff684978a5fe56c">
<ds:Transforms>
<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
</ds:Transforms>
<ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
<ds:DigestValue>ptR4sQtcSeB51h3ggeXGv6dlvp3V3NEcvF33GhgdTlE=</ds:DigestValue>
</ds:Reference>
</ds:SignedInfo>
<ds:SignatureValue>SoTLlmi1vhoGBgwjlwFY4+CLkczw81PFDXOeK1vSlrC0eOhe89wc2Ve7fFCGe5N/
L5luYt+QcWyoi7M3E48EuuxyYnJtznG6nLjezTUe2Rd1HvQwtDptmjyes3SD1o1/
FM9UpYbnPAQgvdmfr/oM5R2xIxwhw7ZBDyPhk5HpwL8=</ds:SignatureValue>
<ds:KeyInfo>
<ds:KeyName>signing</ds:KeyName>
<ds:X509Data>
<ds:X509Certificate>MIIB+jCCAWOgAwIBAgIUdcXUPTE+mOSWxRCJh8ldmDMPzREwDQYJKoZIhvcNAQEL
BQAwDzENMAsGA1UEAwwEVGVzdDAeFw0yNjA0MDIxMjQzMTNaFw0yNzA0MDIxMjQz
MTNaMA8xDTALBgNVBAMMBFRlc3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGB
AJEBNDJKH5nXr0hZKcSNIY1l4HeYLPBEKJLXyAnoFTdgGrvi40YyIx9lHh0LbDVW
CgxJp21BmKll0CkgmeKidvGlr3FUwtETro44L+SgmjiJNbftvFxhNkgA26O2GDQu
BoQwgSiagVadWXwJKkodH8tx4ojBPYK1pBO8fHf3wOnxAgMBAAGjUzBRMB0GA1Ud
DgQWBBSLoT4AEwcK1+0IMwgo6JYfA4e8ZTAfBgNVHSMEGDAWgBSLoT4AEwcK1+0I
Mwgo6JYfA4e8ZTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4GBAAtV
1hclbZBD17LMbBwyrTj7szmmeUVISPeFEPaAKqiTXrHwRZ+akajboB2JjT3YYMXX
2/eDaSvq9f20vJQUvkEAaYu8eNNDKWgm4btJFAeJT8uGxizmTspdJ0cxFSwxqaos
V3qIqJgpwLbzUXEcu6mKfyqDM6AeFZdZevkxmKlE
</ds:X509Certificate>
</ds:X509Data>
</ds:KeyInfo>
</ds:Signature>
<saml:Subject>
<saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">some@customer.com</saml:NameID>
<saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
<saml:SubjectConfirmationData NotOnOrAfter="3923-09-01T02:16:12.556Z" Recipient="https://api.dev.zoo.dev/auth/saml/00000000-00000000-00000000-00000000/login"/>
</saml:SubjectConfirmation>
</saml:Subject>
<saml:Conditions NotBefore="2022-05-04T15:36:12.556Z" NotOnOrAfter="3923-09-01T02:16:12.556Z"/>
<saml:AuthnStatement AuthnInstant="2022-05-04T15:36:12.556Z" SessionNotOnOrAfter="3923-09-01T02:16:12.556Z" SessionIndex="_samling_4356273_859641652">
<saml:AuthnContext>
<saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml:AuthnContextClassRef>
</saml:AuthnContext>
</saml:AuthnStatement>
<saml:AttributeStatement>
<saml:Attribute Name="email" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">
<saml:AttributeValue>thing@thing.com</saml:AttributeValue>
</saml:Attribute>
</saml:AttributeStatement>
<saml:AttributeStatement>
<saml:Attribute Name="first_name" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">
<saml:AttributeValue>Test</saml:AttributeValue>
</saml:Attribute>
</saml:AttributeStatement>
<saml:AttributeStatement>
<saml:Attribute Name="last_name" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">
<saml:AttributeValue>User</saml:AttributeValue>
</saml:Attribute>
</saml:AttributeStatement>
</saml:Assertion>
</samlp:Response>
Loading