Opened 14 years ago

Closed 9 years ago

Last modified 9 years ago

#2980 defect closed fixed (fixed)

t.w.xmlrpc.QueryProtocol erroneous newlines in base64 encoding of long credentials

Reported by: stockst Owned by: therve
Priority: normal Milestone:
Component: web Keywords:
Cc: Thijs Triemstra Branch:
Author:

Description

In twisted.web.xmlrpc.QueryProtocol.connectionMade(), long Basic Auth credentials (username and password) that result in a base64 encoding > 76 characters have embedded newlines. This results in rejected XML-RPC requests as the Basic Auth RFC (RFC2617) does not impose a 76 character limit and the remote server interprets the newlines as part of the credential.

This is Python's fault as the string method .encode("base64") is embedding the newlines to break the lines at 76-character boundaries.

>>> "this-is-a-really-long-username:this-is-a-really-long-password".encode('base64')
'dGhpcy1pcy1hLXJlYWxseS1sb25nLXVzZXJuYW1lOnRoaXMtaXMtYS1yZWFsbHktbG9uZy1wYXNz\nd29yZA==\n'

The fix is easy, use python's base64 module to do the encoding:

import base64 

def connectionMade(self):
    self.sendCommand('POST', self.factory.path)
    self.sendHeader('User-Agent', 'Twisted/XMLRPClib')
    self.sendHeader('Host', self.factory.host)
    self.sendHeader('Content-type', 'text/xml')
    self.sendHeader('Content-length', str(len(self.factory.payload)))
    if self.factory.user:
        auth = '%s:%s' % (self.factory.user, self.factory.password)
        #auth = auth.encode('base64').strip()
        auth = base64.b64encode(auth.encode)
        self.sendHeader('Authorization', 'Basic %s' % (auth,))
    self.endHeaders()
    self.transport.write(self.factory.payload)

Attachments (2)

2980.diff (1.5 KB) - added by Michael Hudson-Doyle 9 years ago.
2980-2.diff (1.8 KB) - added by Michael Hudson-Doyle 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 in reply to:  description Changed 14 years ago by stockst

Replying to stockst:

Grr, typo, the fix should be:

import base64
def connectionMade(self):
    self.sendCommand('POST', self.factory.path)
    self.sendHeader('User-Agent', 'Twisted/XMLRPClib')
    self.sendHeader('Host', self.factory.host)
    self.sendHeader('Content-type', 'text/xml')
    self.sendHeader('Content-length', str(len(self.factory.payload)))
    if self.factory.user:
        auth = '%s:%s' % (self.factory.user, self.factory.password)
        #auth = auth.encode('base64').strip()
        auth = base64.b64encode(auth)
        self.sendHeader('Authorization', 'Basic %s' % (auth,))
    self.endHeaders()
    self.transport.write(self.factory.payload)

Sorry about that.

comment:2 in reply to:  description Changed 14 years ago by stockst

Not fixed in Twisted 8.x, bump.

Replying to stockst:

In twisted.web.xmlrpc.QueryProtocol.connectionMade(), long Basic Auth credentials (username and password) that result in a base64 encoding > 76 characters have embedded newlines. This results in rejected XML-RPC requests as the Basic Auth RFC (RFC2617) does not impose a 76 character limit and the remote server interprets the newlines as part of the credential.

This is Python's fault as the string method .encode("base64") is embedding the newlines to break the lines at 76-character boundaries.

...

comment:3 Changed 14 years ago by Jean-Paul Calderone

Could you write a unit test that fails without the fix?

comment:4 Changed 11 years ago by <automation>

Owner: jknight deleted

Changed 9 years ago by Michael Hudson-Doyle

Attachment: 2980.diff added

Changed 9 years ago by Michael Hudson-Doyle

Attachment: 2980-2.diff added

comment:5 Changed 9 years ago by Michael Hudson-Doyle

Keywords: review added; xmlrpc QueryProtocol basic auth removed

Writing a unit test that fails is easy, see attachment:2980-2.diff. The way it fails without the code change isn't especially clear due to Twisted being stricter at interpreting newlines in headers than RFC 2616 suggests and more tolerant than I would expect of bogus authorization headers. But it's probably OK?

comment:6 Changed 9 years ago by therve

Owner: set to therve

comment:7 Changed 9 years ago by therve

Keywords: review removed

Looks good, applying.

comment:8 Changed 9 years ago by therve

Resolution: fixed
Status: newclosed

(In [38003]) Use the base64 module in xmlrpc QueryProtocol to generate valid authorization header for long user names and passwords.

Author: mwh Reviewer: therve Fixes: #2980

comment:9 Changed 9 years ago by Thijs Triemstra

Cc: Thijs Triemstra added
Resolution: fixed
Status: closedreopened

The topfile was placed in the words project where it should be in web.

comment:10 Changed 9 years ago by Jean-Paul Calderone

Resolution: fixed
Status: reopenedclosed

The correct way to re-open a ticket is by reverting the revision that closed it.

comment:11 Changed 9 years ago by therve

Resolution: fixed
Status: closedreopened

(In [38004]) Revert r38003: NEWS fragment in the wrong place

Reopens: #2980

comment:12 Changed 9 years ago by therve

Resolution: fixed
Status: reopenedclosed

(In [38005]) Use the base64 module in xmlrpc QueryProtocol to generate valid authorization header for long user names and passwords. This is a re-merge of r38003 with NEWS fragment in the right place.

Author: mwh Reviewer: therve Fixes: #2980

comment:13 Changed 9 years ago by Thijs Triemstra

Thanks!

Note: See TracTickets for help on using tickets.