Skip to content

Commit

Permalink
Merge pull request #50 from maykinmedia/bug/remove-ip-address-pinning
Browse files Browse the repository at this point in the history
🔊 Replace fatal error with logging
  • Loading branch information
alextreme authored Dec 4, 2023
2 parents c78718b + e423f10 commit 2c27449
Showing 1 changed file with 18 additions and 5 deletions.
23 changes: 18 additions & 5 deletions digid_eherkenning/saml2/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2c27449

Please sign in to comment.