New Auditor: Flag Identity Providers with disabled signature verifica…

…tion (#37)

Add auditor for OIDC and SAML IDPs without signature verification

docs/auditors/idp.md

@@ -9,6 +9,16 @@ Keycloak can be configured to delegate user authentication to an upstream Identi
 
     These auditors are currently fairly bare-bones, as we haven't yet had time to read up on what specific problems may lurk in the different possible setups. If you have expertise in this area, please reach out or contribute your own auditors.
 
+## IdentityProviderWithSignatureVerificationDisabled
+
+This auditor warns about Identity Providers configured not to check the signatures of the upstream IDP.
+Not checking the signatures of the tokens the IDP provides is dangerous, as the tokens are no longer cryptographically protected against tampering.
+This may lead to an account takeover or other attacks.
+We strongly recommend to set up signature checks.
+
+The auditor supports OIDC, Keycloak OIDC, and SAML IDPs.
+Provider-specific IDPs (like GitHub, GitLab, etc.) do not have an option to disable signature verification and should thus be safe by default.
+
 ## OIDCIdentityProviderWithoutPKCE
 
 This auditor warns about OIDC Identity Providers configured within a realm that do not have the Proof Key for Code

kcwarden/auditors/idp/identity_provider_with_signature_verification_disabled.py

@@ -0,0 +1,24 @@
+from kcwarden.api import Auditor
+from kcwarden.custom_types.result import Severity
+
+
+class IdentityProviderWithSignatureVerificationDisabled(Auditor):
+    DEFAULT_SEVERITY = Severity.Critical
+    SHORT_DESCRIPTION = "Identity Provider does not verify upstream IDPs signatures"
+    LONG_DESCRIPTION = "Keycloak allows you to configure external identity providers. When using OpenID Connect, it is important to verify the cryptographic signature that secures the identity and access tokens generated by the identity provider. This IDP configuration disables this signature check, which makes it vulnerable to accepting forged access tokens, leading to account takeover and other security issues."
+    REFERENCE = ""
+
+    def should_consider_idp(self, idp) -> bool:
+        return self.is_not_ignored(idp) and idp.get_provider_id() in ["oidc", "keycloak-oidc", "saml"]
+
+    def idp_does_not_verify_signatures(self, config):
+        return config.get("validateSignature") == "false"
+
+    def audit(self):
+        for idp in self._DB.get_all_identity_providers():
+            # Skip IDPs that were explicitly ignored, or that aren't OIDC IDPs
+            if not self.should_consider_idp(idp):
+                continue
+            # Check if signature verification is disabled
+            if self.idp_does_not_verify_signatures(idp.get_config()):
+                yield self.generate_finding(idp)

tests/auditors/idp/test_oidc_identity_provider_with_signature_verification_disabled.py

@@ -0,0 +1,94 @@
+import pytest
+from unittest.mock import Mock
+
+from kcwarden.auditors.idp.identity_provider_with_signature_verification_disabled import (
+    IdentityProviderWithSignatureVerificationDisabled,
+)
+from kcwarden.custom_types import config_keys
+
+
+class TestOIDCIdentityProviderWithSignatureVerificationDisabled:
+    @pytest.fixture
+    def auditor(self, database, default_config):
+        auditor_instance = IdentityProviderWithSignatureVerificationDisabled(database, default_config)
+        auditor_instance._DB = Mock()
+        return auditor_instance
+
+    @pytest.mark.parametrize(
+        "provider_id, expected",
+        [
+            ("oidc", True),  # OIDC provider should be considered
+            ("keycloak-oidc", True),  # Keycloak OIDC provider should be considered
+            ("saml", True),  # SAML provider should also be considered
+            ("github", False),  # SAML provider should also be considered
+        ],
+    )
+    def test_should_consider_idp(self, auditor, provider_id, expected):
+        mock_idp = Mock()
+        mock_idp.get_provider_id.return_value = provider_id
+        assert auditor.should_consider_idp(mock_idp) == expected
+
+    @pytest.mark.parametrize(
+        "config, expected",
+        [
+            ({"validateSignature": "true"}, False),  # Signature verification enabled
+            ({"validateSignature": "false"}, True),  # Signature verification disabled
+        ],
+    )
+    def test_idp_does_not_enforce_pkce(self, auditor, config, expected):
+        assert auditor.idp_does_not_verify_signatures(config) == expected
+
+    def test_audit_function_no_findings(self, auditor, mock_idp):
+        # Setup IDP with correct PKCE configuration
+        mock_idp.get_provider_id.return_value = "oidc"
+        mock_idp.get_config.return_value = {"validateSignature": "true"}
+        auditor._DB.get_all_identity_providers.return_value = [mock_idp]
+
+        results = list(auditor.audit())
+        assert len(results) == 0
+
+    def test_audit_function_with_findings(self, auditor, mock_idp):
+        # Setup IDP without correct PKCE configuration
+        mock_idp.get_provider_id.return_value = "oidc"
+        mock_idp.get_config.return_value = {"validateSignature": "false"}
+        auditor._DB.get_all_identity_providers.return_value = [mock_idp]
+
+        results = list(auditor.audit())
+        assert len(results) == 1
+
+    def test_audit_function_multiple_idps(self, auditor):
+        # Create separate mock IDPs with distinct settings
+        idp1 = Mock()
+        idp1.get_provider_id.return_value = "oidc"
+        idp1.get_config.return_value = {"validateSignature": "false"}
+
+        idp2 = Mock()
+        idp2.get_provider_id.return_value = "oidc"
+        idp2.get_config.return_value = {"validateSignature": "true"}
+
+        idp3 = Mock()
+        idp3.get_provider_id.return_value = "keycloak-oidc"
+        idp3.get_config.return_value = {"validateSignature": "false"}
+
+        idp4 = Mock()
+        idp4.get_provider_id.return_value = "saml"
+        idp4.get_config.return_value = {"validateSignature": "false"}
+
+        auditor._DB.get_all_identity_providers.return_value = [idp1, idp2, idp3, idp4]
+        results = list(auditor.audit())
+        assert len(results) == 3  # Expect findings from idp1, idp3 and idp4
+
+    def test_ignore_list_functionality(self, auditor, mock_idp):
+        # Setup IDP without force sync mode and with mappers
+        # Setup IDP without correct PKCE configuration
+        mock_idp.get_provider_id.return_value = "oidc"
+        mock_idp.get_config.return_value = {"validateSignature": "false"}
+        mock_idp.get_alias.return_value = "ignored_idp"
+        mock_idp.get_name.return_value = mock_idp.get_alias.return_value
+        auditor._DB.get_all_identity_providers.return_value = [mock_idp]
+
+        # Add the IDP to the ignore list
+        auditor._CONFIG = {config_keys.AUDITOR_CONFIG: {auditor.get_classname(): ["ignored_idp"]}}
+
+        results = list(auditor.audit())
+        assert len(results) == 0  # No findings due to ignore list