From 1897cc3c8f1a3440bc35108c0f849cdfcdabfe77 Mon Sep 17 00:00:00 2001 From: Jinna Kiisuo Date: Mon, 9 Sep 2024 17:50:49 +0300 Subject: [PATCH 1/3] Update OIDC id_token_signing_alg_values_supported for wider algo support Previously the message verification required RS256 with no other checks on algo. While technically RS256 MUST be supported, some implementations have abandoned it's use as insecure and instead require for example ES256 as a minimum baseline. This change slightly relaxes the check in a future compatible way while still making sure an actual alg is specified instead of `none`. ```python >>> bad = ["none"] >>> good = ["ES256"] >>> dodgy = ["none", "RS256"] >>> empty = [] >>> any(i.lower() != "none" for i in dodgy) True >>> any(i.lower() != "none" for i in empty) False >>> any(i.lower() != "none" for i in good) True >>> any(i.lower() != "none" for i in bad) False ``` --- src/idpyoidc/message/oidc/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/idpyoidc/message/oidc/__init__.py b/src/idpyoidc/message/oidc/__init__.py index a1c9949f..fe7f6c25 100644 --- a/src/idpyoidc/message/oidc/__init__.py +++ b/src/idpyoidc/message/oidc/__init__.py @@ -942,8 +942,14 @@ def verify(self, **kwargs): "token_endpoint_auth_signing_alg_values_supported" ) - if "RS256" not in self["id_token_signing_alg_values_supported"]: - raise ValueError("RS256 missing from id_token_signing_alg_values_supported") + # Check that any alg that is not "none" is supported. + # While OpenID Connect Core 1.0 says RS256 MUST be supported, + # reality has moved on and more modern alg values may be required. + if not any(i.lower() != "none" for i in self["id_token_signing_alg_values_supported"]): + raise ValueError( + "Secure signing algorithm (for example RS256 or ES256) missing from id_token_signing_alg_values_supported: %s" + % self["id_token_signing_alg_values_supported"] + ) if not parts.query and not parts.fragment: pass From 01c22cf21e4ecfc94538ee2d722a087726a5cd6a Mon Sep 17 00:00:00 2001 From: Jinna Kiisuo Date: Mon, 9 Sep 2024 23:51:17 +0300 Subject: [PATCH 2/3] Add tests for non-RS256 sign-alg & failing a none-only sign-alg --- tests/test_06_oidc.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_06_oidc.py b/tests/test_06_oidc.py index 09b3b256..3b589e4e 100644 --- a/tests/test_06_oidc.py +++ b/tests/test_06_oidc.py @@ -516,6 +516,32 @@ def test_token_endpoint_is_required_for_other_than_implicit_flow_only(self): with pytest.raises(MissingRequiredAttribute): ProviderConfigurationResponse(**provider_config).verify() + def test_required_parameters_without_rs256(self): + provider_config = { + "issuer": "https://server.example.com", + "authorization_endpoint": "https://server.example.com/connect/authorize", + "jwks_uri": "https://server.example.com/jwks.json", + "response_types_supported": ["code", "code id_token", "id_token", "token id_token"], + "subject_types_supported": ["public", "pairwise"], + "id_token_signing_alg_values_supported": ["none", "ES256", "HS256"], + } + + with pytest.raises(MissingRequiredAttribute): + ProviderConfigurationResponse(**provider_config).verify() + + def test_required_parameters_only_none_signing_alg(self): + provider_config = { + "issuer": "https://server.example.com", + "authorization_endpoint": "https://server.example.com/connect/authorize", + "jwks_uri": "https://server.example.com/jwks.json", + "response_types_supported": ["code", "code id_token", "id_token", "token id_token"], + "subject_types_supported": ["public", "pairwise"], + "id_token_signing_alg_values_supported": ["none"], + } + + with pytest.raises(ValueError): + ProviderConfigurationResponse(**provider_config).verify() + class TestRegistrationRequest(object): def test_deserialize(self): From c06902e79101a277f353e60593eb652dc4f26e08 Mon Sep 17 00:00:00 2001 From: Jinna Kiisuo Date: Tue, 10 Sep 2024 00:17:36 +0300 Subject: [PATCH 3/3] Fix test logic for correct assert and tangential miss from existing test --- tests/test_06_oidc.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_06_oidc.py b/tests/test_06_oidc.py index 3b589e4e..b6747af3 100644 --- a/tests/test_06_oidc.py +++ b/tests/test_06_oidc.py @@ -470,6 +470,7 @@ def test_example_response(self): [ "issuer", "authorization_endpoint", + "token_endpoint", "jwks_uri", "response_types_supported", "subject_types_supported", @@ -480,6 +481,7 @@ def test_required_parameters(self, required_param): provider_config = { "issuer": "https://server.example.com", "authorization_endpoint": "https://server.example.com/connect/authorize", + "token_endpoint": "https://server.example.com/connect/token", "jwks_uri": "https://server.example.com/jwks.json", "response_types_supported": ["code", "code id_token", "id_token", "token id_token"], "subject_types_supported": ["public", "pairwise"], @@ -520,19 +522,20 @@ def test_required_parameters_without_rs256(self): provider_config = { "issuer": "https://server.example.com", "authorization_endpoint": "https://server.example.com/connect/authorize", + "token_endpoint": "https://server.example.com/connect/token", "jwks_uri": "https://server.example.com/jwks.json", "response_types_supported": ["code", "code id_token", "id_token", "token id_token"], "subject_types_supported": ["public", "pairwise"], "id_token_signing_alg_values_supported": ["none", "ES256", "HS256"], } - with pytest.raises(MissingRequiredAttribute): - ProviderConfigurationResponse(**provider_config).verify() + assert ProviderConfigurationResponse(**provider_config).verify() def test_required_parameters_only_none_signing_alg(self): provider_config = { "issuer": "https://server.example.com", "authorization_endpoint": "https://server.example.com/connect/authorize", + "token_endpoint": "https://server.example.com/connect/token", "jwks_uri": "https://server.example.com/jwks.json", "response_types_supported": ["code", "code id_token", "id_token", "token id_token"], "subject_types_supported": ["public", "pairwise"],