From e423f10513aefe49fa7697f443e0f01ecba11324 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 4 Dec 2023 18:06:07 +0100 Subject: [PATCH] :loud_sound: Replace fatal error with logging There appear to be ~1 incident/day caused by this IP address check, and from the looks of it they all seems like genuine, non-malicious usage. In office spaces where VPNs and network changes (which result in different IP addresses) are common, this check leads to false positives and we've assessed the likelihood of actual attacks to be low. * Session cookies are typically flagged as HttpOnly, so XSS to steal credentials/cookies is not a viable exploit path * If an attacker can intercept the session cookie via MITM over plain HTTP, the DigiD conditions are not met and you really, really should not run anything over plain HTTP. * If an attacker can MITM an HTTPS connection, you have far bigger issues, since then interecepting/modifying the actual HTTP traffic is much more interesting than hijacking a DigiD session. * Initial HTTP connections (first-ever) are possible, even when using HSTS (which you should deploy), but can be mitigated by making sure your URLs are added to the preload lists vendored in browsers. --- digid_eherkenning/saml2/base.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/digid_eherkenning/saml2/base.py b/digid_eherkenning/saml2/base.py index 806bba2..8e79ae5 100644 --- a/digid_eherkenning/saml2/base.py +++ b/digid_eherkenning/saml2/base.py @@ -170,11 +170,24 @@ def verify_saml2_response(self, response, client_ip_address): # authn_ip_address = authn_request["client_ip_address"] if authn_ip_address != client_ip_address: - raise OneLogin_Saml2_ValidationError( - f"A different IP address ({authn_ip_address})" - f"was used when retrieving the AuthNRequest then for retrieving" - f" the request to the ACS ({client_ip_address}).", - OneLogin_Saml2_ValidationError.WRONG_INRESPONSETO, + # Relaxed validation - IP address changes between mobile towers are not + # necessarily a common occurrence (typically the mobile provider manages + # your mobile IP address), but what does happen is switching between wifi/VPN + # or mobile network in the user's office (or even on the go, with a Dual SIM + # set up for example). So instead of complicating the error messages for + # these edge cases and given the very low likelyhook an attacker is able to + # steal the session cookie/data, we opt for detection and logging instead. + logger.warning( + "A different IP address (%s) was used when retrieving the AuthNRequest " + "than for resolving the SAML artifact in the ACS (%s).", + authn_ip_address, + client_ip_address, + # record meta information for potential audit trails + extra={ + "authn_ip_address": authn_ip_address, + "client_ip_address": client_ip_address, + "security": True, + }, ) # I remember reading somewhere that the assurance level on the response