Roundup Tracker - Issues

Issue 2550992

classification
Errors from invalid Authorization headers
Type: behavior Severity: normal
Components: Web interface Versions: devel
process
Status: fixed fixed
:
: : joseph_myers, schlatterbeck
Priority: normal : patch, patch

Created on 2018-08-12 23:13 by joseph_myers, last changed 2018-09-27 11:40 by joseph_myers.

Files
File name Uploaded Description Edit Remove
auth.patch joseph_myers, 2018-08-12 23:13 Patch to catch exceptions
Messages
msg6191 Author: [hidden] (joseph_myers) Date: 2018-08-12 23:13
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 = ''

The attached patch 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.
msg6247 Author: [hidden] (joseph_myers) Date: 2018-09-16 13:46
Any comments on this patch?
msg6262 Author: [hidden] (joseph_myers) Date: 2018-09-27 11:40
Fix committed.
History
Date User Action Args
2018-09-27 11:40:31joseph_myerssetkeywords: patch, patch
status: new -> fixed
resolution: fixed
messages: + msg6262
2018-09-16 13:46:27joseph_myerssetkeywords: patch, patch
messages: + msg6247
2018-08-13 07:19:52schlatterbecksetkeywords: patch, patch
nosy: + schlatterbeck
2018-08-12 23:13:15joseph_myerscreate