diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index cc8e5eb19a0..a16809c5e5b 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -828,6 +828,59 @@ impl super::Nexus { } }; + // Once the IDP metadata document is available, parse it into an + // EntityDescriptor + use samael::metadata::EntityDescriptor; + let idp_metadata: EntityDescriptor = + idp_metadata_document_string.parse().map_err(|e| { + Error::invalid_request(&format!( + "idp_metadata_document_string could not be parsed as an EntityDescriptor! {}", + e + )) + })?; + + // Check for at least one signing key - do not accept IDPs that have + // none! + let mut found_signing_key = false; + + if let Some(idp_sso_descriptors) = idp_metadata.idp_sso_descriptors { + for idp_sso_descriptor in &idp_sso_descriptors { + for key_descriptor in &idp_sso_descriptor.key_descriptors { + // Key use is an optional attribute. If it's present, check + // if it's "signing". If it's not present, the contained key + // information could be used for either signing or + // encryption. + let is_signing_key = + if let Some(key_use) = &key_descriptor.key_use { + key_use == "signing" + } else { + true + }; + + if is_signing_key { + if let Some(x509_data) = + &key_descriptor.key_info.x509_data + { + if !x509_data.certificates.is_empty() { + found_signing_key = true; + break; + } + } + } + } + } + } else { + return Err(Error::invalid_request( + "no md:IDPSSODescriptor section", + )); + } + + if !found_signing_key { + return Err(Error::invalid_request( + "no signing key found in IDP metadata", + )); + } + let provider = db::model::SamlIdentityProvider { identity: db::model::SamlIdentityProviderIdentity::new( Uuid::new_v4(), diff --git a/nexus/tests/integration_tests/data/saml_idp_descriptor.xml b/nexus/tests/integration_tests/data/saml_idp_descriptor.xml index 2d850195cbd..b510723141d 100644 --- a/nexus/tests/integration_tests/data/saml_idp_descriptor.xml +++ b/nexus/tests/integration_tests/data/saml_idp_descriptor.xml @@ -22,6 +22,13 @@ https://idp.example.org/myicon.png + + + + MIIB0DCCAXagAwIBAgIUfvG7FgnAf1y/b0t1bJxqTJxrsA0wCgYIKoZIzj0EAwIwRjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAkNBMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2NvMRIwEAYDVQQDEwlzYW1sLnRlc3QwHhcNMjMwNjA2MTgwMzAwWhcNMjgwNjA0MTgwMzAwWjBGMQswCQYDVQQGEwJVUzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDVNhbiBGcmFuY2lzY28xEjAQBgNVBAMTCXNhbWwudGVzdDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABKd/VM8rcnguEezFNLH2XoCF46tc/9qacSrwPT17BACE6Qi2ptPRi7EJbJ2Ba5rCPKoRvVkW4Ra6N0NjbrmM6yqjQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTnBe8+9wrmtOst71d7sjOSQYDDPDAKBggqhkjOPQQDAgNIADBFAiB9u01tz7C8p2W/9P39h5uf8efnYwTWmv+2m1/mvkLsygIhANTcN0dUHioQzz5C0smWnm1PhTXmxICpQzKjAxVhVavn + + + diff --git a/nexus/tests/integration_tests/data/saml_idp_descriptor_encryption_key_only.xml b/nexus/tests/integration_tests/data/saml_idp_descriptor_encryption_key_only.xml new file mode 100644 index 00000000000..cc050b9c495 --- /dev/null +++ b/nexus/tests/integration_tests/data/saml_idp_descriptor_encryption_key_only.xml @@ -0,0 +1,44 @@ + + + + + + + https://registrar.example.net/category/self-certified + + + + + + + Example.org + The identity provider at Example.org + https://idp.example.org/myicon.png + + + + + + MIIB0DCCAXagAwIBAgIUfvG7FgnAf1y/b0t1bJxqTJxrsA0wCgYIKoZIzj0EAwIwRjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAkNBMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2NvMRIwEAYDVQQDEwlzYW1sLnRlc3QwHhcNMjMwNjA2MTgwMzAwWhcNMjgwNjA0MTgwMzAwWjBGMQswCQYDVQQGEwJVUzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDVNhbiBGcmFuY2lzY28xEjAQBgNVBAMTCXNhbWwudGVzdDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABKd/VM8rcnguEezFNLH2XoCF46tc/9qacSrwPT17BACE6Qi2ptPRi7EJbJ2Ba5rCPKoRvVkW4Ra6N0NjbrmM6yqjQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTnBe8+9wrmtOst71d7sjOSQYDDPDAKBggqhkjOPQQDAgNIADBFAiB9u01tz7C8p2W/9P39h5uf8efnYwTWmv+2m1/mvkLsygIhANTcN0dUHioQzz5C0smWnm1PhTXmxICpQzKjAxVhVavn + + + + + + + + Example.org Non-Profit Org + Example.org + https://www.example.org/ + + + SAML Technical Support + mailto:technical-support@example.org + + diff --git a/nexus/tests/integration_tests/data/saml_idp_descriptor_no_keys.xml b/nexus/tests/integration_tests/data/saml_idp_descriptor_no_keys.xml new file mode 100644 index 00000000000..2d850195cbd --- /dev/null +++ b/nexus/tests/integration_tests/data/saml_idp_descriptor_no_keys.xml @@ -0,0 +1,37 @@ + + + + + + + https://registrar.example.net/category/self-certified + + + + + + + Example.org + The identity provider at Example.org + https://idp.example.org/myicon.png + + + + + + + Example.org Non-Profit Org + Example.org + https://www.example.org/ + + + SAML Technical Support + mailto:technical-support@example.org + + diff --git a/nexus/tests/integration_tests/saml.rs b/nexus/tests/integration_tests/saml.rs index a46f7f35ab1..a4fdbd2e9c2 100644 --- a/nexus/tests/integration_tests/saml.rs +++ b/nexus/tests/integration_tests/saml.rs @@ -29,6 +29,10 @@ type ControlPlaneTestContext = // note: no signing keys pub const SAML_IDP_DESCRIPTOR: &str = include_str!("data/saml_idp_descriptor.xml"); +pub const SAML_IDP_DESCRIPTOR_ENCRYPTION_KEY_ONLY: &str = + include_str!("data/saml_idp_descriptor_encryption_key_only.xml"); +pub const SAML_IDP_DESCRIPTOR_NO_KEYS: &str = + include_str!("data/saml_idp_descriptor_no_keys.xml"); // Create a SAML IdP #[nexus_test] @@ -279,6 +283,120 @@ async fn test_create_a_saml_idp_invalid_descriptor_no_redirect_binding( .expect("unexpected success"); } +// Fail to create a SAML IdP from a metadata document that only has encryption +// keys +#[nexus_test] +async fn test_create_a_saml_idp_metadata_only_encryption_keys( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + const SILO_NAME: &str = "saml-silo"; + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) + .await; + + let saml_idp_descriptor = + SAML_IDP_DESCRIPTOR_ENCRYPTION_KEY_ONLY.to_string(); + + let server = Server::run(); + server.expect( + Expectation::matching(request::method_path("GET", "/descriptor")) + .respond_with(status_code(200).body(saml_idp_descriptor)), + ); + + NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &format!("/v1/system/identity-providers/saml?silo={}", SILO_NAME), + ) + .body(Some(¶ms::SamlIdentityProviderCreate { + identity: IdentityMetadataCreateParams { + name: "some-totally-real-saml-provider" + .to_string() + .parse() + .unwrap(), + description: "a demo provider".to_string(), + }, + + idp_metadata_source: params::IdpMetadataSource::Url { + url: server.url("/descriptor").to_string(), + }, + + idp_entity_id: "entity_id".to_string(), + sp_client_id: "client_id".to_string(), + acs_url: "http://acs".to_string(), + slo_url: "http://slo".to_string(), + technical_contact_email: "technical@fake".to_string(), + + signing_keypair: None, + + group_attribute_name: None, + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("unexpected success"); +} + +// Fail to create a SAML IdP from a metadata document that has no keys +#[nexus_test] +async fn test_create_a_saml_idp_metadata_no_keys( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + const SILO_NAME: &str = "saml-silo"; + create_silo(&client, SILO_NAME, true, shared::SiloIdentityMode::SamlJit) + .await; + + let saml_idp_descriptor = SAML_IDP_DESCRIPTOR_NO_KEYS.to_string(); + + let server = Server::run(); + server.expect( + Expectation::matching(request::method_path("GET", "/descriptor")) + .respond_with(status_code(200).body(saml_idp_descriptor)), + ); + + NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &format!("/v1/system/identity-providers/saml?silo={}", SILO_NAME), + ) + .body(Some(¶ms::SamlIdentityProviderCreate { + identity: IdentityMetadataCreateParams { + name: "some-totally-real-saml-provider" + .to_string() + .parse() + .unwrap(), + description: "a demo provider".to_string(), + }, + + idp_metadata_source: params::IdpMetadataSource::Url { + url: server.url("/descriptor").to_string(), + }, + + idp_entity_id: "entity_id".to_string(), + sp_client_id: "client_id".to_string(), + acs_url: "http://acs".to_string(), + slo_url: "http://slo".to_string(), + technical_contact_email: "technical@fake".to_string(), + + signing_keypair: None, + + group_attribute_name: None, + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("unexpected success"); +} + // Create a hidden Silo with a SAML IdP #[nexus_test] async fn test_create_a_hidden_silo_saml_idp( @@ -722,6 +840,47 @@ fn test_correct_saml_response_ecdsa_sha256() { assert_eq!(relay_state, None); } +// Test rejecting a SAML response signed with a key that doesn't match the silo +// identity provider's key +#[test] +fn test_reject_saml_response_signed_with_other_key() { + let silo_saml_identity_provider = SamlIdentityProvider { + idp_metadata_document_string: SAML_RESPONSE_IDP_DESCRIPTOR.to_string(), + + idp_entity_id: "https://some.idp.test/oxide_rack/".to_string(), + sp_client_id: "https://customer.site/oxide_rack/saml".to_string(), + acs_url: "https://customer.site/oxide_rack/saml".to_string(), + slo_url: "http://slo".to_string(), + technical_contact_email: "technical@fake".to_string(), + + public_cert: None, + private_key: None, + + group_attribute_name: None, + }; + + let body_bytes = serde_urlencoded::to_string(SamlLoginPost { + saml_response: base64::engine::general_purpose::STANDARD + .encode(&SAML_RESPONSE_SIGNED_WITH_ECDSA_SHA256), + relay_state: None, + }) + .unwrap(); + + let result = silo_saml_identity_provider.authenticated_subject( + &body_bytes, + // Set max_issue_delay so that SAMLResponse is valid + Some( + chrono::Utc::now() + - "2022-05-04T15:36:12.631Z" + .parse::>() + .unwrap() + + chrono::Duration::seconds(60), + ), + ); + + assert!(result.is_err()); +} + // Test a SAML response with only the assertion signed #[test] fn test_accept_saml_response_only_assertion_signed() {