Skip to content

Commit a782282

Browse files
crobinsocrobinso
committed
CVE-2013-2191: Switch to pycurl to get SSL host and cert validation
Right now python-bugzilla will happily allow connecting to a host with a self signed SSL certificate, or hostname that doesn't match the cert. This isn't a safe default. Standard python libs don't handle these cases, but pycurl does. So we switch to pycurl for the transport layer. Add a --nosslverify CLI switch to turn off this functionality if the user chooses. Thanks to Tomas Hoger for much of the sample code.
1 parent f7498b0 commit a782282

5 files changed

Lines changed: 120 additions & 55 deletions

File tree

‎bin/bugzilla‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ def setup_parser():
9090
p.add_option('--bztype', default='auto',
9191
help="Bugzilla type. Autodetected if not set. "
9292
"Available types: %s" % " ".join(bugzilla.classlist))
93+
p.add_option("--nosslverify", dest="sslverify",
94+
action="store_false", default=True,
95+
help="Don't error on invalid bugzilla SSL certificate")
9396
p.add_option('--user',
9497
help="username")
9598
p.add_option('--password',
@@ -1024,7 +1027,8 @@ def main(bzinstance=None):
10241027
bz = bzinstance
10251028
else:
10261029
bz = bzclass(url=global_opt.bugzilla,
1027-
cookiefile=global_opt.cookiefile or -1)
1030+
cookiefile=global_opt.cookiefile or -1,
1031+
sslverify=global_opt.sslverify)
10281032

10291033

10301034
# Handle 'login' action
@@ -1126,7 +1130,7 @@ if __name__ == '__main__':
11261130
main()
11271131
except KeyboardInterrupt:
11281132
log.debug("", exc_info=True)
1129-
print "\ninterrupted."
1133+
print "\nExited at user request."
11301134
sys.exit(1)
11311135
except socket.error, e:
11321136
log.debug("", exc_info=True)
@@ -1139,8 +1143,19 @@ if __name__ == '__main__':
11391143
except xmlrpclib.ProtocolError, e:
11401144
log.debug("", exc_info=True)
11411145
print "\nInvalid server response: %d %s" % (e.errcode, e.errmsg)
1142-
redir = e.headers.getheader("location", 0)
1146+
1147+
# Give SSL recommendations
1148+
import pycurl
1149+
sslerrcodes = [getattr(pycurl, ename) for ename in dir(pycurl) if
1150+
ename.startswith("E_SSL")]
1151+
if e.errcode in sslerrcodes:
1152+
print ("\nIf you trust the remote server, you can work "
1153+
"around this error with:\n"
1154+
" bugzilla --nosslverify ...")
1155+
1156+
# Detect redirect
1157+
redir = (e.headers and e.headers.getheader("location", 0) or None)
11431158
if redir:
1144-
print "Server was attempting a redirect."
1145-
print 'Try "bugzilla --bugzilla %s ..."' % redir
1159+
print ("\nServer was attempting a redirect. Try: "
1160+
" bugzilla --bugzilla %s ..." % redir)
11461161
sys.exit(4)

‎bugzilla/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class Bugzilla(object):
9797
def __init__(self, **kwargs):
9898
log.info("Bugzilla v%s initializing" % __version__)
9999
if 'url' not in kwargs:
100-
raise TypeError("You must pass a valid bugzilla xmlrpc.cgi URL")
100+
raise TypeError("You must pass a valid bugzilla URL")
101101

102102
# pylint: disable=W0233
103103
# Use of __init__ of non parent class

‎bugzilla/base.py‎

Lines changed: 94 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@
1111

1212
import cookielib
1313
import os
14+
import StringIO
1415
import urllib2
16+
import urlparse
1517
import xmlrpclib
1618

19+
import pycurl
20+
1721
from bugzilla import __version__, log
1822
from bugzilla.bug import _Bug, _User
1923

@@ -95,43 +99,73 @@ def _build_cookiejar(cookiefile):
9599
return retcj
96100

97101

98-
# CookieTransport code mostly borrowed from pybugz
99-
class _CookieTransport(xmlrpclib.Transport):
100-
def __init__(self, uri, cookiejar, use_datetime=0):
101-
self.verbose = 0
102-
103-
# python 2.4 compat
102+
class _CURLTransport(xmlrpclib.Transport):
103+
def __init__(self, url, cookiejar,
104+
sslverify=True, sslcafile=None, debug=0):
104105
if hasattr(xmlrpclib.Transport, "__init__"):
105-
xmlrpclib.Transport.__init__(self, use_datetime=use_datetime)
106+
xmlrpclib.Transport.__init__(self, use_datetime=False)
107+
108+
self.verbose = debug
109+
110+
# transport constructor needs full url too, as xmlrpc does not pass
111+
# scheme to request
112+
self.scheme = urlparse.urlparse(url)[0]
113+
if self.scheme not in ["http", "https"]:
114+
raise Exception("Invalid URL scheme: %s (%s)" % (self.scheme, url))
115+
116+
self.c = pycurl.Curl()
117+
self.c.setopt(pycurl.POST, 1)
118+
self.c.setopt(pycurl.CONNECTTIMEOUT, 30)
119+
self.c.setopt(pycurl.HTTPHEADER, [
120+
"Content-Type: text/xml",
121+
])
122+
self.c.setopt(pycurl.VERBOSE, debug)
123+
124+
self.set_cookiejar(cookiejar)
125+
126+
# ssl settings
127+
if self.scheme == "https":
128+
# override curl built-in ca file setting
129+
if sslcafile is not None:
130+
self.c.setopt(pycurl.CAINFO, sslcafile)
131+
132+
# disable ssl verification
133+
if not sslverify:
134+
self.c.setopt(pycurl.SSL_VERIFYPEER, 0)
135+
self.c.setopt(pycurl.SSL_VERIFYHOST, 0)
136+
137+
def set_cookiejar(self, cj):
138+
self.c.setopt(pycurl.COOKIEFILE, cj.filename or "")
139+
self.c.setopt(pycurl.COOKIEJAR, cj.filename or "")
140+
141+
def get_cookies(self):
142+
return self.c.getinfo(pycurl.INFO_COOKIELIST)
143+
144+
def open_helper(self, url, request_body):
145+
self.c.setopt(pycurl.URL, url)
146+
self.c.setopt(pycurl.POSTFIELDS, request_body)
147+
148+
b = StringIO.StringIO()
149+
self.c.setopt(pycurl.WRITEFUNCTION, b.write)
150+
try:
151+
self.c.perform()
152+
except pycurl.error, e:
153+
raise xmlrpclib.ProtocolError(url, e[0], e[1], None)
106154

107-
self.uri = uri
108-
self.opener = urllib2.build_opener()
109-
self.opener.add_handler(urllib2.HTTPCookieProcessor(cookiejar))
155+
b.seek(0)
156+
return b
110157

111158
def request(self, host, handler, request_body, verbose=0):
112-
req = urllib2.Request(self.uri)
113-
req.add_header('User-Agent', self.user_agent)
114-
req.add_header('Content-Type', 'text/xml')
159+
self.verbose = verbose
160+
url = "%s://%s%s" % (self.scheme, host, handler)
115161

116-
if hasattr(self, 'accept_gzip_encoding') and self.accept_gzip_encoding:
117-
req.add_header('Accept-Encoding', 'gzip')
162+
# xmlrpclib fails to escape \r
163+
request_body = request_body.replace('\r', '
')
118164

119-
req.add_data(request_body)
165+
stringio = self.open_helper(url, request_body)
166+
return self.parse_response(stringio)
120167

121-
resp = self.opener.open(req)
122168

123-
# In Python 2, resp is a urllib.addinfourl instance, which does not
124-
# have the getheader method that parse_response expects.
125-
if not hasattr(resp, 'getheader'):
126-
resp.getheader = resp.headers.getheader
127-
128-
if resp.code == 200:
129-
self.verbose = verbose
130-
return self.parse_response(resp)
131-
132-
resp.close()
133-
raise xmlrpclib.ProtocolError(self.uri, resp.status,
134-
resp.reason, resp.msg)
135169

136170

137171
class BugzillaError(Exception):
@@ -186,8 +220,6 @@ def url_to_query(url):
186220
Given a big huge bugzilla query URL, returns a query dict that can
187221
be passed along to the Bugzilla.query() method.
188222
'''
189-
import urlparse
190-
191223
q = {}
192224
(ignore, ignore, path,
193225
ignore, query, ignore) = urlparse.urlparse(url)
@@ -219,13 +251,16 @@ def fix_url(url):
219251
url = url + '/xmlrpc.cgi'
220252
return url
221253

222-
def __init__(self, url=None, user=None, password=None, cookiefile=-1):
254+
def __init__(self, url=None, user=None, password=None, cookiefile=-1,
255+
sslverify=True):
223256
# Settings the user might want to tweak
224257
self.user = user or ''
225258
self.password = password or ''
226259
self.url = ''
227260

261+
self._transport = None
228262
self._cookiejar = None
263+
self._sslverify = bool(sslverify)
229264

230265
self.logged_in = False
231266

@@ -371,9 +406,11 @@ def connect(self, url=None):
371406
url = self.url
372407
url = self.fix_url(url)
373408

374-
transport = _CookieTransport(url, self._cookiejar)
375-
transport.user_agent = self.user_agent
376-
self._proxy = xmlrpclib.ServerProxy(url, transport)
409+
self._transport = _CURLTransport(url, self._cookiejar,
410+
sslverify=self._sslverify)
411+
self._transport.user_agent = self.user_agent
412+
self._proxy = xmlrpclib.ServerProxy(url, self._transport)
413+
377414

378415
self.url = url
379416
# we've changed URLs - reload config
@@ -431,8 +468,6 @@ def login(self, user=None, password=None):
431468
except xmlrpclib.Fault:
432469
r = False
433470

434-
if r and self._cookiejar.filename is not None:
435-
self._cookiejar.save()
436471
return r
437472

438473
def logout(self):
@@ -1178,18 +1213,32 @@ def openattachment(self, attachid):
11781213
'''Get the contents of the attachment with the given attachment ID.
11791214
Returns a file-like object.'''
11801215
att_uri = self._attachment_uri(attachid)
1181-
opener = urllib2.build_opener(
1182-
urllib2.HTTPCookieProcessor(self._cookiejar))
1183-
att = opener.open(att_uri)
11841216

1185-
# RFC 2183 defines the content-disposition header, if you're curious
1186-
disp = att.headers['content-disposition'].split(';')
1217+
headers = {}
1218+
ret = StringIO.StringIO()
1219+
1220+
def headers_cb(buf):
1221+
if not ":" in buf:
1222+
return
1223+
name, val = buf.split(":", 1)
1224+
headers[name.lower()] = val
1225+
1226+
c = pycurl.Curl()
1227+
c.setopt(pycurl.URL, att_uri)
1228+
c.setopt(pycurl.WRITEFUNCTION, ret.write)
1229+
c.setopt(pycurl.HEADERFUNCTION, headers_cb)
1230+
c.setopt(pycurl.COOKIEFILE, self._cookiejar.filename or "")
1231+
c.perform()
1232+
c.close()
1233+
1234+
disp = headers['content-disposition'].split(';')
11871235
disp.pop(0)
11881236
parms = dict([p.strip().split("=", 1) for p in disp])
1189-
# Parameter values can be quoted/encoded as per RFC 2231
1190-
att.name = _decode_rfc2231_value(parms['filename'])
1237+
ret.name = _decode_rfc2231_value(parms['filename'])
1238+
11911239
# Hooray, now we have a file-like object with .read() and .name
1192-
return att
1240+
ret.seek(0)
1241+
return ret
11931242

11941243
def updateattachmentflags(self, bugid, attachid, flagname, **kwargs):
11951244
'''

‎python-bugzilla.spec‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ BuildRequires: python-setuptools
1919
BuildRequires: python-setuptools-devel
2020
%endif
2121

22+
Requires: python-pycurl
2223
Requires: python-magic
2324

2425
%description

‎tests/ro_functional.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ def _testQuery(self, args, mincount, expectbug):
7474
return
7575

7676
self.assertTrue(len(out.splitlines()) >= mincount)
77-
self.assertTrue(any([l.startswith("#" + expectbug)
78-
for l in out.splitlines()]))
77+
self.assertTrue(bool([l for l in out.splitlines() if
78+
l.startswith("#" + expectbug)]))
7979

8080
# Check --ids output option
8181
out2 = self.clicomm(cli + " --ids")
8282
self.assertTrue(len(out.splitlines()) == len(out2.splitlines()))
83-
self.assertTrue(any([l == expectbug for l in out2.splitlines()]))
83+
self.assertTrue(bool([l for l in out2.splitlines() if l == expectbug]))
8484

8585

8686
def _testQueryFull(self, bugid, mincount, expectstr):
@@ -145,7 +145,7 @@ class BZ34(BaseTest):
145145

146146

147147
class BZ42(BaseTest):
148-
url = "https://bugzilla.freedesktop.org/xmlrpc.cgi"
148+
url = "https://bugs.freedesktop.org/xmlrpc.cgi"
149149
bzclass = bugzilla.Bugzilla4
150150
closestatus = "CLOSED,RESOLVED"
151151

0 commit comments

Comments
 (0)