diff --git a/lib/ex_saml/core/saml.ex b/lib/ex_saml/core/saml.ex index f483741..3ccc01f 100644 --- a/lib/ex_saml/core/saml.ex +++ b/lib/ex_saml/core/saml.ex @@ -812,15 +812,16 @@ defmodule ExSaml.Core.Saml do - Audience matches (if present in conditions) - Assertion is not stale """ - @spec validate_assertion(tuple(), String.t(), String.t()) :: + @spec validate_assertion(tuple(), String.t(), String.t(), String.t() | charlist() | nil) :: {:ok, Assertion.t()} | {:error, term()} - def validate_assertion(assertion_xml, recipient, audience) do + def validate_assertion(assertion_xml, recipient, audience, expected_issuer \\ nil) do case decode_assertion(assertion_xml) do {:error, reason} -> {:error, reason} {:ok, assertion} -> with :ok <- validate_version(assertion), + :ok <- validate_issuer(assertion, expected_issuer), :ok <- validate_recipient(assertion, recipient), :ok <- validate_audience(assertion, audience), :ok <- check_not_before(assertion), @@ -833,6 +834,20 @@ defmodule ExSaml.Core.Saml do defp validate_version(%Assertion{version: "2.0"}), do: :ok defp validate_version(_), do: {:error, :bad_version} + # Validate the assertion Issuer against the IdP's configured entity_id. When + # no expected issuer is provided (nil / empty), the check is skipped for + # backward compatibility. This is defense in depth (identity-provider + # confusion) that becomes essential whenever assertion signature checking is + # relaxed. See shark-up/cryptr-gateway#165 finding #13. + defp validate_issuer(_assertion, nil), do: :ok + + defp validate_issuer(%Assertion{issuer: issuer}, expected) do + case to_string(expected) do + "" -> :ok + expected_str -> if to_string(issuer) == expected_str, do: :ok, else: {:error, :bad_issuer} + end + end + defp validate_recipient(%Assertion{recipient: r}, recipient) do if to_string(r) == to_string(recipient) do :ok diff --git a/lib/ex_saml/core/sp.ex b/lib/ex_saml/core/sp.ex index f382448..8677ed9 100644 --- a/lib/ex_saml/core/sp.ex +++ b/lib/ex_saml/core/sp.ex @@ -252,7 +252,12 @@ defmodule ExSaml.Core.Sp do :ok <- verify_envelope_signature(xml, sp), :ok <- verify_assertion_signature(assertion_xml, sp), {:ok, assertion} <- - Saml.validate_assertion(assertion_xml, sp.consume_uri, get_entity_id(sp)), + Saml.validate_assertion( + assertion_xml, + sp.consume_uri, + get_entity_id(sp), + sp.idp_entity_id + ), :ok <- check_duplicate(assertion, xml, duplicate_fun) do {:ok, assertion} end diff --git a/lib/ex_saml/core/sp_config.ex b/lib/ex_saml/core/sp_config.ex index 78cf99c..833eb72 100644 --- a/lib/ex_saml/core/sp_config.ex +++ b/lib/ex_saml/core/sp_config.ex @@ -23,7 +23,8 @@ defmodule ExSaml.Core.SpConfig do consume_uri: "", logout_uri: nil, encrypt_mandatory: false, - entity_id: nil + entity_id: nil, + idp_entity_id: nil @type t :: %__MODULE__{ org: Org.t(), @@ -41,6 +42,7 @@ defmodule ExSaml.Core.SpConfig do consume_uri: String.t(), logout_uri: String.t() | nil, encrypt_mandatory: boolean(), - entity_id: String.t() | nil + entity_id: String.t() | nil, + idp_entity_id: String.t() | nil } end diff --git a/lib/ex_saml/idp_data.ex b/lib/ex_saml/idp_data.ex index 1dcafdc..c52ce0c 100644 --- a/lib/ex_saml/idp_data.ex +++ b/lib/ex_saml/idp_data.ex @@ -395,7 +395,8 @@ defmodule ExSaml.IdpData do metadata_uri: Helper.get_metadata_uri(idp_data.base_url, path_segment_idp_id), consume_uri: Helper.get_consume_uri(idp_data.base_url, path_segment_idp_id), logout_uri: Helper.get_logout_uri(idp_data.base_url, path_segment_idp_id), - entity_id: sp_entity_id + entity_id: sp_entity_id, + idp_entity_id: idp_data.entity_id } end diff --git a/mix.lock b/mix.lock index e2eb94c..fa65c9f 100644 --- a/mix.lock +++ b/mix.lock @@ -18,7 +18,7 @@ "mix_audit": {:hex, :mix_audit, "2.1.5", "c0f77cee6b4ef9d97e37772359a187a166c7a1e0e08b50edf5bf6959dfe5a016", [:make, :mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:yaml_elixir, "~> 2.11", [hex: :yaml_elixir, repo: "hexpm", optional: false]}], "hexpm", "87f9298e21da32f697af535475860dc1d3617a010e0b418d2ec6142bc8b42d69"}, "nebulex": {:hex, :nebulex, "2.6.6", "677e27fcfa89eaa085d9509d5e066f305f98c1b2264ce6676eaca6fb08d4939e", [:mix], [{:decorator, "~> 1.4", [hex: :decorator, repo: "hexpm", optional: true]}, {:shards, "~> 1.1", [hex: :shards, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: true]}], "hexpm", "8cbf531af6fe407383b6ba410a43a19319af47804929d8a8d1975a780b9952df"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.2", "8efba0122db06df95bfaa78f791344a89352ba04baedd3849593bfce4d0dc1c6", [:mix], [], "hexpm", "4b21398942dda052b403bbe1da991ccd03a053668d147d53fb8c4e0efe09c973"}, - "plug": {:hex, :plug, "1.19.2", "e4950525b22c6789dfb38a3f95d47171ba159da3fc5a33be9643b43d5e8adb98", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b6fce20a56af5e60fa5dfecf3f907bb98ec981be43c79a3809a499bc3d133de0"}, + "plug": {:hex, :plug, "1.20.2", "adbee2441232412e37fbb357fd5e4cd533fdd253b29f2e1992262b0f1fb01462", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b16baf55877d60891002ffc1ce0b3ff7d6f30a38a23e02e4d4293c4ac266f136"}, "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"}, "sobelow": {:hex, :sobelow, "0.14.1", "2f81e8632f15574cba2402bcddff5497b413c01e6f094bc0ab94e83c2f74db81", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8fac9a2bd90fdc4b15d6fca6e1608efb7f7c600fa75800813b794ee9364c87f2"}, "sweet_xml": {:hex, :sweet_xml, "0.7.5", "803a563113981aaac202a1dbd39771562d0ad31004ddbfc9b5090bdcd5605277", [:mix], [], "hexpm", "193b28a9b12891cae351d81a0cead165ffe67df1b73fe5866d10629f4faefb12"},