Does anyone know what this means? The page views fine for me and this is the first error of this type I've seen since we launched the archives. I wonder what WebWhacker is doing to cause this? cheers, Chris Automatic Zope Response wrote:
Zope reported an error for: http://zope.nipltd.com/public/lists.html Error Type: Error Error Value: Incorrect padding
Traceback: Traceback (innermost last): File /mnt/data/2/zope/external/2-1-6/lib/python/ZPublisher/Publish.py, line 151, in publish File /mnt/data/2/zope/external/2-1-6/lib/python/Products/SiteAccess/ChangeBehaviors.py, line 228, in traverse File /mnt/data/2/zope/external/2-1-6/lib/python/AccessControl/User.py, line 433, in validate (Object: RoleManager) File /var/tmp/python/python-root/usr/lib/python1.5/base64.py, line 46, in decodestring File /var/tmp/python/python-root/usr/lib/python1.5/base64.py, line 32, in decode Error: Incorrect padding
HTTP_USER_AGENT => Mozilla/4.0 (compatible; WebWhacker 4.0; Windows) PARENTS => [<Folder instance at 88d4e10>, <Application instance at 869e048>] PATH_INFO => /public/lists.html PATH_TRANSLATED => /public/lists.html REMOTE_ADDR => 127.0.0.1 REQUEST_METHOD => GET RESPONSE => HTTP/1.0 204 No Content Server: Zope/Zope 2.1.6 (source release, python 1.5.2, linux2) ZServer/1.1b1 Date: Sun, 23 Jul 2000 05:12:21 GMT Connection: close Content-Length: 0 SERVER_SOFTWARE => Zope/Zope 2.1.6 (source release, python 1.5.2, linux2) ZServer/1.1b1
Chris Withers wrote:
Does anyone know what this means?
The page views fine for me and this is the first error of this type I've seen since we launched the archives. I wonder what WebWhacker is doing to cause this?
Well... you can get an incorect padding error like this:
import binascii binascii.a2b_base64('a ')
Traceback (innermost last): File "<stdin>", line 1, in ? binascii.Error: Incorrect padding You can also get by adding arbitrary non-space characters to the end of a basic authentication string. Here's the line that'll be causing the problem. (Zope 2.2final, lib/python/AccessControl/User.py line 441) name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1)) My guess is that the argument "auth" passed to validate() has some trailing characters. Either that, or WebWhacker passed just "Basic " as an auth string. Maybe log the auth argument and re-whack your site. (Warning... this is certainly a nasty security hole. Be sure to get rid of the logging afterwards.) -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote:
My guess is that the argument "auth" passed to validate() has some trailing characters. Either that, or WebWhacker passed just "Basic " as an auth string.
Yuk, that sounds like a Zope bug. Collector time with patch? A judicious string.strip should solve the problem, surely?
Maybe log the auth argument and re-whack your site. (Warning... this is certainly a nasty security hole. Be sure to get rid of the logging afterwards.)
I didn't whack my site, I just got mailed the error... Okay, own up, who tried to WebWhack the list archives?! ;-) cheers, Chris
Chris Withers wrote:
Steve Alexander wrote:
My guess is that the argument "auth" passed to validate() has some trailing characters. Either that, or WebWhacker passed just "Basic " as an auth string.
Yuk, that sounds like a Zope bug. Collector time with patch? A judicious string.strip should solve the problem, surely?
PS: A string.upper wouldn't go amiss either, then earlier versions of Mozilla that send an incorrectly capitalised 'Basic' might also be allowed to authenticate with Zope :-) cheers, Chris
Chris Withers wrote:
Chris Withers wrote:
Steve Alexander wrote:
My guess is that the argument "auth" passed to validate() has some trailing characters. Either that, or WebWhacker passed just "Basic " as an auth string.
Yuk, that sounds like a Zope bug. Collector time with patch? A judicious string.strip should solve the problem, surely?
PS:
A string.upper wouldn't go amiss either, then earlier versions of Mozilla that send an incorrectly capitalised 'Basic' might also be allowed to authenticate with Zope :-)
It is already there in 2.2final: if lower(auth[:6])!='basic ': ^^^^^ RFC 1945 has it as "Basic". http://www.freesoft.org/CIE/RFC/1945/67.htm I also checked, and this version of the patch *should* work: # Only do basic authentication if lower(auth[:6])!='basic ': return None name,password=tuple(split(decodestring(strip(auth[6:])), ':', 1)) The "strip" is in there just in case a client responds with "basic base64blah" instead of "basic base64blah". However, it still doesn't work if the client sends something bogus -- the tuple will only be one item long, rather than two. If you want to be protected against bogosity in basic authentication, you can stick with the original line, and put it inside a try-except block: # Only do basic authentication if lower(auth[:6])!='basic ': return None try: name,password=\ tuple(split(decodestring(split(auth)[-1]), ':', 1)) except: # Bogus basic authentication. Perhaps log something? return None -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote: <snip excellent patches> Well, I think Brian Lloyd'd have to make the call... Nice work though, 2 for 2 on the day, that's pretty good going, are DC paying you yet? ;-) cheers, Chris
On Mon, Jul 24, 2000 at 05:22:25PM +0100, Steve Alexander wrote:
Chris Withers wrote:
A string.upper wouldn't go amiss either, then earlier versions of Mozilla that send an incorrectly capitalised 'Basic' might also be allowed to authenticate with Zope :-)
Heh, and allow Mozilla to gain the bug again? Zope wan't the only server Moz broke on though..
It is already there in 2.2final: if lower(auth[:6])!='basic ': ^^^^^
RFC 1945 has it as "Basic".
RFC 1945 says one paragraph before that that the header should be matched case-insensitively. That was what the Moz bug was all about.
I also checked, and this version of the patch *should* work:
# Only do basic authentication if lower(auth[:6])!='basic ': return None name,password=tuple(split(decodestring(strip(auth[6:])), ':', 1))
The "strip" is in there just in case a client responds with
"basic base64blah" instead of "basic base64blah".
The split already takes out the whitespace. No need to strip.
However, it still doesn't work if the client sends something bogus -- the tuple will only be one item long, rather than two.
That is a bug in the client then.
If you want to be protected against bogosity in basic authentication, you can stick with the original line, and put it inside a try-except block:
# Only do basic authentication if lower(auth[:6])!='basic ': return None try: name,password=\ tuple(split(decodestring(split(auth)[-1]), ':', 1)) except: # Bogus basic authentication. Perhaps log something? return None
This would mask bugs in clients. Not a good idea. -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
Martijn Pieters wrote:
RFC 1945 says one paragraph before that that the header should be matched case-insensitively. That was what the Moz bug was all about.
Thanks for the clarification.
# Only do basic authentication if lower(auth[:6])!='basic ': return None name,password=tuple(split(decodestring(strip(auth[6:])), ':', 1))
The "strip" is in there just in case a client responds with
"basic base64blah" instead of "basic base64blah".
The split already takes out the whitespace. No need to strip.
There's a base64.decodestring() between the strip and the split. Leading and/or trailing spaces make base64 strings invalid. -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
On Mon, Jul 24, 2000 at 08:36:26PM +0200, Martijn Pieters wrote:
I also checked, and this version of the patch *should* work:
# Only do basic authentication if lower(auth[:6])!='basic ': return None name,password=tuple(split(decodestring(strip(auth[6:])), ':', 1))
The "strip" is in there just in case a client responds with
"basic base64blah" instead of "basic base64blah".
Oops. You took out the strip. But IIRC, base64 does a strip as well. -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
Martijn Pieters wrote:
Oops. You took out the strip. But IIRC, base64 does a strip as well.
So it does!
from base64 import * s = encodestring('foo') decodestring(s) 'foo' decodestring(s+' ') 'foo' decodestring(' '+s) 'foo'
-- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote:
Martijn Pieters wrote:
Oops. You took out the strip. But IIRC, base64 does a strip as well.
So it does!
from base64 import * s = encodestring('foo') decodestring(s) 'foo' decodestring(s+' ') 'foo' decodestring(' '+s) 'foo'
So what was causing the original error then? Chris
On Mon, Jul 24, 2000 at 07:49:34PM +0100, Chris Withers wrote:
Steve Alexander wrote:
Martijn Pieters wrote:
Oops. You took out the strip. But IIRC, base64 does a strip as well.
So it does!
from base64 import * s = encodestring('foo') decodestring(s) 'foo' decodestring(s+' ') 'foo' decodestring(' '+s) 'foo'
So what was causing the original error then?
Buggy client? -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
Martijn Pieters wrote:
So what was causing the original error then?
Buggy client?
If so, surely Zope should just return an Unauthorized error rather than exposing its internals?! If you're a server and the client is buggy, tell it so, but don't look like you just screwed up really badly ;-) cheers, Chris
On Mon, Jul 24, 2000 at 07:57:00PM +0100, Chris Withers wrote:
Martijn Pieters wrote:
So what was causing the original error then?
Buggy client?
If so, surely Zope should just return an Unauthorized error rather than exposing its internals?!
If you're a server and the client is buggy, tell it so, but don't look like you just screwed up really badly ;-)
I disagree. The client used is bad, this kind of error doesn't show often and serves a purpose here; fix the client! -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
On Mon, Jul 24, 2000 at 07:57:00PM +0100, Chris Withers wrote:
Martijn Pieters wrote:
So what was causing the original error then?
Buggy client?
If so, surely Zope should just return an Unauthorized error rather than exposing its internals?!
If you're a server and the client is buggy, tell it so, but don't look like you just screwed up really badly ;-)
Oops. Speed read fumble. We partly agree, and this is tricky. Unauthorised is wrong, it should return a Bad Request (or whatever the correct HTTP error is in this case). File a patch! =) -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
Chris Withers wrote:
So what was causing the original error then?
string.split('basic')[-1] was returning 'basic'. base64.decodestring('basic') causes an "Incorrect padding" error. base64.decodestring('basic ') causes an "Incorrect padding" error too. Martijn Pieters wrote:
We partly agree, and this is tricky. Unauthorised is wrong, it should return a Bad Request (or whatever the correct HTTP error is in this case). File a patch! =)
I've attached a patch to lib/python/AccessControl/User.py. If there are no suggestions of improvements, or complaints :-) I'll stick it into the Collector. I looked over the RFC, and Bad Request seems to be the best response code. -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net *** lib/python/AccessControl/User.py.original Mon Jul 24 20:31:40 2000 --- lib/python/AccessControl/User.py Mon Jul 24 20:51:33 2000 *************** *** 438,444 **** # Only do basic authentication if lower(auth[:6])!='basic ': return None ! name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1)) # Check for superuser super=self._super --- 438,451 ---- # Only do basic authentication if lower(auth[:6])!='basic ': return None ! try: ! name,password=\ ! tuple(split(decodestring(split(auth)[-1]), ':', 1)) ! except: # not a proper basic auth string ! request.response.setStatus(400) ! raise 'InternalError', request.response._error_html( ! "Internal Error", ! "Zope could not understand the Basic Authentication supplied.") # Check for superuser super=self._super
Steve Alexander wrote:
Chris Withers wrote:
So what was causing the original error then?
string.split('basic')[-1] was returning 'basic'.
base64.decodestring('basic') causes an "Incorrect padding" error.
base64.decodestring('basic ') causes an "Incorrect padding" error too.
Martijn Pieters wrote:
We partly agree, and this is tricky. Unauthorised is wrong, it should return a Bad Request (or whatever the correct HTTP error is in this case). File a patch! =)
I've attached a patch to lib/python/AccessControl/User.py. If there are no suggestions of improvements, or complaints :-) I'll stick it into the Collector.
Mention in the collector that the padding that's incorrect doesn't refer to spaces, but to equal signs. The ASCII representation of base64 is in sets of 4 characters, which translate to 3 bytes. If the ASCII data is of a length that doesn't evenly divide by 4, the string is supposed to end with equal signs as "padding". If it doesn't, you'll get the "incorrect padding" error. Incidentally, try the following: base64.decodestring('basic===') This products gibberish, but it does work. :-) Shane
On Mon, Jul 24, 2000 at 08:56:54PM +0100, Steve Alexander wrote:
I've attached a patch to lib/python/AccessControl/User.py. If there are no suggestions of improvements, or complaints :-) I'll stick it into the Collector.
I looked over the RFC, and Bad Request seems to be the best response code.
Agreed.
*** lib/python/AccessControl/User.py.original Mon Jul 24 20:31:40 2000 --- lib/python/AccessControl/User.py Mon Jul 24 20:51:33 2000 *************** *** 438,444 **** # Only do basic authentication if lower(auth[:6])!='basic ': return None ! name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1))
# Check for superuser super=self._super --- 438,451 ---- # Only do basic authentication if lower(auth[:6])!='basic ': return None ! try: ! name,password=\ ! tuple(split(decodestring(split(auth)[-1]), ':', 1)) ! except: # not a proper basic auth string ! request.response.setStatus(400) ! raise 'InternalError', request.response._error_html( ! "Internal Error", ! "Zope could not understand the Basic Authentication supplied.")
# Check for superuser super=self._super
Would it be a good idea to add the header? And let's make that a less generic except clause, we don't want to mask Zope bugs =) -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
Martijn Pieters wrote:
On Mon, Jul 24, 2000 at 08:56:54PM +0100, Steve Alexander wrote:
I've attached a patch to lib/python/AccessControl/User.py. If there are no suggestions of improvements, or complaints :-) I'll stick it into the Collector.
I looked over the RFC, and Bad Request seems to be the best response code.
Agreed.
*** lib/python/AccessControl/User.py.original Mon Jul 24 20:31:40 2000 --- lib/python/AccessControl/User.py Mon Jul 24 20:51:33 2000 *************** *** 438,444 **** # Only do basic authentication if lower(auth[:6])!='basic ': return None ! name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1))
# Check for superuser super=self._super --- 438,451 ---- # Only do basic authentication if lower(auth[:6])!='basic ': return None ! try: ! name,password=\ ! tuple(split(decodestring(split(auth)[-1]), ':', 1)) ! except: # not a proper basic auth string ! request.response.setStatus(400) ! raise 'InternalError', request.response._error_html( ! "Internal Error", ! "Zope could not understand the Basic Authentication supplied.")
# Check for superuser super=self._super
Would it be a good idea to add the header?
I don't quite follow.
And let's make that a less generic except clause, we don't want to mask Zope bugs =)
Ok. I was thinking of just using HTTPResponse.badRequestError(). However, it has a "name" argument, and I don't quite understand how it is meant to be used. def badRequestError(self,name): self.setStatus(400) if regex.match('^[A-Z_0-9]+$',name) >= 0: raise 'InternalError', self._error_html( "Internal Error", "Sorry, an internal error occurred in this Zope resource.") raise 'BadRequest',self._error_html( "Invalid request", "The parameter, <em>%s</em>, " % name + "was omitted from the request.<p>" + "Make sure to specify all required parameters, " + "and try the request again." ) So, any of name="FOO", name="123", name="FOO123", name="" produces an internal error; name="foo", name="Foo", name="foo123" produces a bad request. What's the rationale? Should I just use this, then? request.response.badRequestError(name="0") # internal error -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote:
! raise 'InternalError', request.response._error_html(
Can someone enlighten me as to what this does? Does it reset the HTTP response code? Is _error_html something that gets the acquired standard_error_message? If not, it should do ;-) Has anyone made any progress on getting Unauthorized errors to use standard_error_message? Here's hoping... Chris
On Mon, Jul 24, 2000 at 09:36:53PM +0100, Steve Alexander wrote:
Martijn Pieters wrote:
Would it be a good idea to add the header?
I don't quite follow.
Add the non-compliant Basic Auth header to the error message, to aid in debugging the client.
And let's make that a less generic except clause, we don't want to mask Zope bugs =)
Ok.
I was thinking of just using HTTPResponse.badRequestError(). However, it has a "name" argument, and I don't quite understand how it is meant to be used.
def badRequestError(self,name): self.setStatus(400) if regex.match('^[A-Z_0-9]+$',name) >= 0: raise 'InternalError', self._error_html( "Internal Error", "Sorry, an internal error occurred in this Zope resource.")
raise 'BadRequest',self._error_html( "Invalid request", "The parameter, <em>%s</em>, " % name + "was omitted from the request.<p>" + "Make sure to specify all required parameters, " + "and try the request again." )
So, any of name="FOO", name="123", name="FOO123", name="" produces an internal error; name="foo", name="Foo", name="foo123" produces a bad request.
What's the rationale?
Should I just use this, then?
request.response.badRequestError(name="0") # internal error
I have no idea. I liked your first solution better, as the message was far more informative. -- Martijn Pieters | Software Engineer mailto:mj@digicool.com | Digital Creations http://www.digicool.com/ | Creators of Zope http://www.zope.org/ | ZopeStudio: http://www.zope.org/Products/ZopeStudio -----------------------------------------------------
Chris Withers wrote:
Steve Alexander wrote:
My guess is that the argument "auth" passed to validate() has some trailing characters. Either that, or WebWhacker passed just "Basic " as an auth string.
Yuk, that sounds like a Zope bug. Collector time with patch? A judicious string.strip should solve the problem, surely?
from base64 import decodestring from string import * auth="Basic" tuple(split(decodestring(split(auth)[-1]), ':', 1)) Traceback (innermost last): File "<stdin>", line 1, in ? File "/usr//lib/python1.5/base64.py", line 46, in decodestring decode(f, g) File "/usr//lib/python1.5/base64.py", line 32, in decode s = binascii.a2b_base64(line) binascii.Error: Incorrect padding
The problem is in the expression "split(auth)[-1]". If the auth string contains no space but at least one character, then split(auth)[-1] == split(auth)[0]. Perhaps what we should do is change this (lib/python/AccessControl/User.py, line 438) # Only do basic authentication if lower(auth[:6])!='basic ': return None name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1)) to this: # Only do basic authentication if lower(auth[:6])!='basic ' and len(auth)>6: return None name,password=tuple(split(decodestring(split(auth)[-1]), ':', 1)) or even: # Only do basic authentication if lower(auth[:6])!='basic ': return None name,password=tuple(split(decodestring(auth[6:]), ':', 1)) (Need to check the last one with the RFC -- are you allowed anything else other than "basic blarglebase64usernamepassword" ? )
Maybe log the auth argument and re-whack your site. (Warning... this is certainly a nasty security hole. Be sure to get rid of the logging afterwards.)
I didn't whack my site, I just got mailed the error...
Well... try whacking yourself and see what the whacker actually sends -- we're just guessing until then. -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
participants (4)
-
Chris Withers -
Martijn Pieters -
Shane Hathaway -
Steve Alexander