Skip to content

Commit eb7793a

Browse files
authored
Rework OpenID service cookie handling (#521)
* Rework openid service cookie handling to address a couple of potential issues hightligthed by code scans. Namely, encode and decode cookie values to base64. * Avoid manual cookie mangling and rely on SimpleCookie instead. Add extra protection against cookie or header splitting issues even though input validation should already prevent it. * Allow @ in retry_url as we may end up there with `/openid/id/EMAIL` .
1 parent e2ba5b9 commit eb7793a

1 file changed

Lines changed: 36 additions & 4 deletions

File tree

mig/server/grid_openid.py

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,11 +397,19 @@ def __retry_url_from_cookie(self):
397397
cookies = self.headers.get('Cookie')
398398
cookie = http.cookies.SimpleCookie(cookies)
399399
cookie_dict = dict((k, v.value) for k, v in cookie.items())
400-
retry_url = cookie_dict.get('retry_url', '')
400+
retry_url_enc = cookie_dict.get('retry_url_enc', '')
401+
logger.debug("found retry_url_enc: %s" % retry_url_enc)
402+
if retry_url_enc:
403+
# NOTE: b64decode takes str and returns bytes
404+
retry_url = force_native_str(base64.b64decode(retry_url_enc))
405+
else:
406+
retry_url = ''
407+
logger.debug("decoded retry_url: %s" % retry_url)
401408
if retry_url and retry_url.startswith("http"):
402409
raise InputException("invalid retry_url: %s" % retry_url)
403410
elif retry_url:
404-
valid_url(retry_url)
411+
# NOTE: we get here with /openid/id/EMAIL as retry_url
412+
valid_url(retry_url, extra_chars='@')
405413
except http.cookies.CookieError as err:
406414
retry_url = None
407415
logger.error("found invalid cookie: %s" % err)
@@ -411,6 +419,31 @@ def __retry_url_from_cookie(self):
411419

412420
return retry_url
413421

422+
def __format_retry_url_for_cookie(self):
423+
"""Format retry_url to fit in cookie"""
424+
if self.retry_url is None:
425+
retry_url = ''
426+
else:
427+
retry_url = self.retry_url
428+
# NOTE: prevent header injection from cookie or header splitting
429+
retry_url = retry_url.replace('\r', '').replace('\n', '')
430+
logger.debug("encoding retry_url: %s" % retry_url)
431+
try:
432+
# NOTE: we get here with /openid/id/EMAIL as retry_url
433+
valid_url(retry_url, extra_chars='@')
434+
except InputException as exc:
435+
retry_url = ''
436+
logger.error("found invalid retry_url: %s" % exc)
437+
438+
# NOTE: b64encode takes bytes and returns bytes
439+
enc = force_native_str(base64.b64encode(force_utf8(retry_url)))
440+
logger.debug("encoded retry_url: %s" % enc)
441+
cookie = http.cookies.SimpleCookie()
442+
cookie['retry_url_enc'] = enc
443+
cookie['retry_url_enc']['secure'] = True
444+
cookie['retry_url_enc']['httponly'] = True
445+
return cookie.output(header='').strip()
446+
414447
def clearUser(self):
415448
"""Reset all saved user variables"""
416449
self.user = None
@@ -1578,8 +1611,7 @@ def showPage(self, response_code, title,
15781611

15791612
self.send_response(response_code)
15801613
self.writeUserHeader()
1581-
self.send_header('Set-Cookie', 'retry_url=%s;secure;httponly'
1582-
% self.retry_url)
1614+
self.send_header('Set-Cookie', self.__format_retry_url_for_cookie())
15831615
self.send_header('Content-type', 'text/html')
15841616
self.end_headers()
15851617
page_template = openid_page_template(configuration, head_extras)

0 commit comments

Comments
 (0)