comparison roundup/cgi/client.py @ 5549:901d7ba146ad

Avoid errors from invalid Authorization headers (issue2550992). When a client sends an invalid HTTP Authorization header, this can result in an email being sent to the admin with a backtrace and the "An error has occurred" page being returned to the client, rather than it just being treated as a normal authentication failure. I've observed this happening with an "Authorization: Bearer" header (missing the argument that should come after Bearer); I don't know what client was sending that invalid header, but since it just sent HEAD requests for three random pages with the invalid header, I'm reasonably sure it was a malfunctioning bot rather than any normal browser used by a human. The problem is in the code: # try handling Basic Auth ourselves auth = self.env['HTTP_AUTHORIZATION'] scheme, challenge = auth.split(' ', 1) which results in "ValueError: need more than 1 value to unpack" in the case where the split() call returns only one value. As a malfunctioning client (quite likely probing lots of sites for some sort of vulnerability in some unrelated web service; any site accessible from the public Internet must expect to receive many such probes all the time), it both isn't useful to report to the Roundup admin (because there are such clients attempting dubious things all the time for any site, so reporting them to the admin is just noise) and in the form of that exception gives no user-friendly information to the admin either. Now subsequent code in the Authorization handling *does* catch errors from invalid arguments, so the code certainly isn't consistently trying to pass such exceptions through to the admin either: try: decoded = b2s(base64.b64decode(challenge)) except TypeError: # invalid challenge decoded = '' This fix arranges for ValueError exceptions to be similarly caught around the tuple unpacking of both the header contents and the decoded challenge for Basic auth, so that those exceptions also do not result in exception emails.
author Joseph Myers <jsm@polyomino.org.uk>
date Thu, 27 Sep 2018 11:38:05 +0000
parents 674ad58667b4
children a06a88ed38ae
comparison
equal deleted inserted replaced
5548:fea11d05110e 5549:901d7ba146ad
835 # we have external auth (e.g. by Apache) 835 # we have external auth (e.g. by Apache)
836 user = self.env['REMOTE_USER'] 836 user = self.env['REMOTE_USER']
837 elif self.env.get('HTTP_AUTHORIZATION', ''): 837 elif self.env.get('HTTP_AUTHORIZATION', ''):
838 # try handling Basic Auth ourselves 838 # try handling Basic Auth ourselves
839 auth = self.env['HTTP_AUTHORIZATION'] 839 auth = self.env['HTTP_AUTHORIZATION']
840 scheme, challenge = auth.split(' ', 1) 840 try:
841 scheme, challenge = auth.split(' ', 1)
842 except ValueError:
843 # Invalid header.
844 scheme = ''
845 challenge = ''
841 if scheme.lower() == 'basic': 846 if scheme.lower() == 'basic':
842 try: 847 try:
843 decoded = b2s(base64.b64decode(challenge)) 848 decoded = b2s(base64.b64decode(challenge))
844 except TypeError: 849 except TypeError:
845 # invalid challenge 850 # invalid challenge
846 decoded = '' 851 decoded = ''
847 username, password = decoded.split(':', 1) 852 try:
853 username, password = decoded.split(':', 1)
854 except ValueError:
855 # Invalid challenge.
856 username = ''
857 password = ''
848 try: 858 try:
849 # Current user may not be None, otherwise 859 # Current user may not be None, otherwise
850 # instatiation of the login action will fail. 860 # instatiation of the login action will fail.
851 # So we set the user to anonymous first. 861 # So we set the user to anonymous first.
852 self.make_user_anonymous() 862 self.make_user_anonymous()

Roundup Issue Tracker: http://roundup-tracker.org/