Session IP Restrictions Problematic on LTE Networks

[ThomasBrierley]

Currently Tsugi disallows LTI sessions from traversing arbitrary IP addresses; but allows sessions to move within the host space of a classic "Class C" network i.e ignoring the last 8 bits:

tsugi-php/src/Core/LTIX.php

Lines 2107 to 2119 in 8b1eeda

	// We only check the first three octets as some systems wander through the addresses on
	// class C - Perhaps it is even NAT - who knows - but we forgive those on the same Class C
	$session_addr = self::wrapped_session_get($session_object, 'REMOTE_ADDR', null);
	$ipaddr = Net::getIP();
	if ( (!$trusted) && $session_addr && $ipaddr &&
	Net::isRoutable($session_addr) && Net::isRoutable($ipaddr) ) {
	$sess_pieces = explode('.',$session_addr);
	$serv_pieces = explode('.',$ipaddr);
	if ( count($sess_pieces) == 4 && count($serv_pieces) == 4 ) {
	if ( $sess_pieces[0] != $serv_pieces[0] || $sess_pieces[1] != $serv_pieces[1] ||
	$sess_pieces[2] != $serv_pieces[2] ) {
	if ( strpos($iphistory, $session_addr) !== false ) {
	error_log("IP Address changed, session_addr=". $session_addr.' current='.$ipaddr." but trusting iphistory=".$iphistory);

Presumably this is intended as an additional layer of protection against possible session hijacking vulnerabilities? However this is proving too restrictive for users on LTE networks where UE will move through many different public IP addresses at relatively high frequency. The exact behaviour varies between carriers, but changes generally occur whenever the modem switches cells, towers, or reconnects to the same cell. Any of which is possible without the UE necessarily physically moving.

I'm far from a network expert, but the problem seems to be that these networks use smaller prefixes than 24 bits. In my testing between IP reassigns, some carriers randomly change all the host bits, easily revealing the small prefix; on others they appear to use at least a 24bit prefix but on closer inspection the IPs are merely more adjacent, suggesting a difference in IP pool management i.e there being no guarantee the next IP wont cross the 24bit threshold, only being less likely to.

Anecdotally I'm seeing around 1.86% of all sessions being expired due to IP changes (based on usage on the order of thousands/day). From my random interrogations the majority of these appear to be due to IP changes within a network prefix smaller than 24bits rather than between different networks.

Originally I was thinking this restriction could be made configurable. But then again this issue is fairly general... students will use LTE networks to do work, on their phone, as a hotspot, or an LTE modem on a laptop/tablet etc... In which case, perhaps this restriction should be considered for removal?

[csev]

Thomas, Thanks for this. Your analysis is spot on. Out current strategy is a compromise. Without this protection, folks who bookmark or send URLs to each other would confer login provilidges through nothing by the URL.’ Tsugi greatly benefits from a cookiless approach to sessions. We work flawlessly in iframes - you can use a tool from multiple courses and multiple roles in different tabs and they don’t all switch to the “latest log in”. This “know the address of the browser” in the session and compare them - has got us a long way. I have studied and tweaked it often over the years. At one point, CloudFlare set a browser-marking cookie and I used that as well - but then it was seen as a tracker approach and browsers zapped it and that approach to reduce the probability went out the window. It did not eliminate the probability - it just made the window smaller. Your message comes at a good time - working with SakaiPlus - where I was forced to use a cookie based session, I made it work with sameSite=none - but we don’t want to switch to cookie based sessions. But we might use the cookie to mark the browser and then use that marker as an alternate way to safely say this session belongs to this browser. I would love to hear if you have any thoughts on the right solution. Thanks. /Chuck

…

On Oct 24, 2022, at 8:10 PM, Thomas Brierley ***@***.***> wrote: Currently Tsugi disallows LTI sessions from traversing arbitrary IP addresses; but allows sessions to move within the host space of a classic "Class C" network i.e ignoring the last 8 bits: https://github.com/tsugiproject/tsugi-php/blob/8b1eeda28f71457a5199c0fce2ba34e911553fb2/src/Core/LTIX.php#L2107-L2119 <https://github.com/tsugiproject/tsugi-php/blob/8b1eeda28f71457a5199c0fce2ba34e911553fb2/src/Core/LTIX.php#L2107-L2119> Presumably this is intended as an additional layer of protection against possible session hijacking vulnerabilities? However this is proving too restrictive for users on LTE networks where UE will move through many different public IP addresses at relatively high frequency. The exact behaviour varies between carriers, but changes generally occur whenever the modem switches cells, towers, or reconnects to the same cell. Any of which is possible without the UE necessarily physically moving. I'm far from a network expert, but the problem seems to be that these networks use smaller prefixes than 24 bits. In my testing between IP reassigns, some carriers randomly change all the host bits, easily revealing the small prefix; on others they appear to use at least a 24bit prefix but on closer inspection the IPs are merely more adjacent, suggesting a difference in IP pool management i.e there being no guarantee the next IP wont cross the 24bit threshold, only being less likely to. Anecdotally I'm seeing around 1.86% of all sessions being expired due to IP changes (based on usage on the order of thousands/day). From my random interrogations the majority of these appear to be due to IP changes within a network prefix smaller than 24bits rather than between different networks. Originally I was thinking this restriction could be made configurable. But then again this issue is fairly general... students will use LTE networks to do work, on their phone, as a hotspot, or an LTE modem on a laptop/tablet etc... In which case, perhaps this restriction should be considered for removal? — Reply to this email directly, view it on GitHub <#34>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJEJJSL34SXWBUN7VZHW2DWE4QPJANCNFSM6AAAAAARNO3XNI>. You are receiving this because you are subscribed to this thread.

[ThomasBrierley]

Thanks Chuck

This restriction makes a lot of sense now. I agree maintaining cookie-less sessions for iframes is the way forward, and the independent launch sessions are invaluable. So in a nut shell: session ids may be inadvertently leaked through user accessible URLs, and also through old fashioned photons I suppose. I have a couple ideas - but please note I have little experience with session security:

Conditional Top-Frame Cookie Secret

The risk appears to be limited to top frames, since embedded iframes do not have visible URLs and are less easily user accessible. Also the incoming 3rd party cookie restrictions only apply to iframes, but not top frames (and likely never will). So in theory it would be safe to switch between query string and cookie based sessions dependent on being embedded, without visibly exposing the session id to the user, their bookmarks or onlookers.

However this feels inconsistent, and would break independence of simultaneous top-frame sessions. So instead, all sessions could continue to use the session id query string, but top-frames could have an additional cookie-based secret requirement. i.e upon launch, top-frame sessions are marked, and cookie set if not exist, then each request auth on top-frame marked session ids must contain the cookie secret (shared between all top-frames for that browser).

Effectively separating top-frame sessions into query-strings for session separation and cookie secrets for authn. To distinguish top-frame from embedded launches, there is a suitable LTI parameter launch_presentation_document_target, but reliability is dependent on the LMS setting this correctly. It could also be determined in the front end but that point seems too late.

Functional difference: none

HTTP Body Sessions

This one is a bit radical and may not be a complete solution on it's own due to breaking browser history: kind of like a CSRF token, the session id could be sent in the body of the HTTP launch response, e.g as an embedded JavaScript variable or even a meta tag or some other visibly hidden element - I think this has the equivalent XSS potential as a query string.

However this has compatibility problems with existing applications built with Tsugi, since with query strings and cookies you get a nice implicit session id on any html based navigation and JS fetch requests. These would no longer work, instead requiring the session id variable to be explicitly included. It also breaks browser history, since the session id is entirely ephemeral.

This has the advantage of entirely eliminating accidental sharing of session ids, (you'd have to dig it out of either the JS scope with a debugger or subsequent network requests). I'm uncertain how much of an issue the history is, since when navigating backwards in an LMS it will just cause a fresh LTI launch anyway.

Functional differences: Breaks direct history navigation and refresh.

[ThomasBrierley]

but we don’t want to switch to cookie based sessions. But we might use the cookie to mark the browser and then use that marker as an alternate way to safely say this session belongs to this browser.

I just realised my rather long winded "Conditional Top-Frame Cookie Secret" proposal was essentially what you were already suggesting, hah.

[csev]

Thomas,

I implemented browser marking and checking using SameSite=None,Secure - this means the cookie gets set in almost all cases (the notable exception is Safari).

I(f you check your cookies after this upgrade, you see a non-session cookie called “TSUGI-BROWSER-MARK” with about a 30 day expiration. This cookie is not the session, it is just a random number we assign to the browser - the session is still passed around on the URL or in form data.

The browser mark check is added to the “don’t migrate the session” check. If the IP address does not change - life is fine. If the IP address changes beyond Class C (formerly this would log you out) it falls back to check the browser mark - if the browser mark in the cookie matches the one in the session - we assume the IP address change is OK.

If the browser mark saved the session it puts this in the log:

PHP message: IP address change, trusting browser mark 635fb01ee5bd2

This cookie gets set in a frame, or as top frame - and once set it sticks around for 30 days.

This is now in master - and you should get it on next upgrade. My servers are upgraded and seem to be happy. I will watch closely.

tsugiproject/tsugi@8b8e5e4

This was surprisingly simple - partially because I learned a lot about SameSite=None testing as I built the new LTI Advantage provider in Sakai called SakaiPlus.

I am curious as to if this reduces the disconnect likelyhood that Thomas was seeing.

Comments welcome.

[ThomasBrierley]

Thanks Chuck

This is great, I wasn't expecting such a fast change!

I will have an opportunity to upgrade in December where I can A-B test this. I'm expecting it to improve by roughly half, since iOS users are stuck with Safari.

If I understand correctly, the session requirement has essentially changed from is-same-class-C-IP to is-same-class-C-IP OR has-cookie-mark. I think the remaining problem is the short future support of 3rd party cookies for embedded frames? The current status is Safari and Firefox block these now, and Chrome is on track to block them in 2023.

My thinking was this issue could be side stepped, since it only affects embedded frames, and embedded frame URLs are not visible or bookmarkable by default, so they could be exempt from any restrictions. i.e it could be argued the risk of the user accidentally leaking session info from embedded URLs is very low - easier perhaps than digging it out of a cookie, but still unlikely by accident.

Interested to know what you think, if this is a bad idea.

This could be done by capturing the LTI launch_presentation_document_target field for a third condition. I'll try my hand at making a small patch and get back to you.