diff options
-rw-r--r-- | Doc/library/http.client.rst | 2 | ||||
-rw-r--r-- | Doc/library/ssl.rst | 15 | ||||
-rw-r--r-- | Lib/http/client.py | 4 | ||||
-rw-r--r-- | Lib/imaplib.py | 14 | ||||
-rw-r--r-- | Lib/nntplib.py | 11 | ||||
-rw-r--r-- | Lib/poplib.py | 11 | ||||
-rw-r--r-- | Lib/ssl.py | 72 | ||||
-rw-r--r-- | Lib/test/test_httplib.py | 9 | ||||
-rw-r--r-- | Lib/test/test_imaplib.py | 11 | ||||
-rw-r--r-- | Lib/test/test_nntplib.py | 10 | ||||
-rw-r--r-- | Lib/test/test_poplib.py | 14 | ||||
-rw-r--r-- | Lib/test/test_ssl.py | 38 | ||||
-rw-r--r-- | Misc/NEWS | 18 |
13 files changed, 191 insertions, 38 deletions
diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index f96ecc2..892e62f 100644 --- a/Doc/library/http.client.rst +++ b/Doc/library/http.client.rst @@ -169,8 +169,8 @@ The following exceptions are raised as appropriate: A subclass of :exc:`HTTPException`. Raised if a server responds with a HTTP status code that we don't understand. -The constants defined in this module are: +The constants defined in this module are: .. data:: HTTP_PORT diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst index 59ebcd4..e86da5f 100644 --- a/Doc/library/ssl.rst +++ b/Doc/library/ssl.rst @@ -286,10 +286,10 @@ Certificate handling Verify that *cert* (in decoded format as returned by :meth:`SSLSocket.getpeercert`) matches the given *hostname*. The rules applied are those for checking the identity of HTTPS servers as outlined - in :rfc:`2818`, except that IP addresses are not currently supported. - In addition to HTTPS, this function should be suitable for checking the - identity of servers in various SSL-based protocols such as FTPS, IMAPS, - POPS and others. + in :rfc:`2818` and :rfc:`6125`, except that IP addresses are not currently + supported. In addition to HTTPS, this function should be suitable for + checking the identity of servers in various SSL-based protocols such as + FTPS, IMAPS, POPS and others. :exc:`CertificateError` is raised on failure. On success, the function returns nothing:: @@ -304,6 +304,13 @@ Certificate handling .. versionadded:: 3.2 + .. versionchanged:: 3.3.3 + The function now follows :rfc:`6125`, section 6.4.3 and does neither + match multiple wildcards (e.g. ``*.*.com`` or ``*a*.example.org``) nor + a wildcard inside an internationalized domain names (IDN) fragment. + IDN A-labels such as ``www*.xn--pthon-kva.org`` are still supported, + but ``x*.python.org`` no longer matches ``xn--tda.python.org``. + .. function:: cert_time_to_seconds(timestring) Returns a floating-point value containing a normal seconds-after-the-epoch diff --git a/Lib/http/client.py b/Lib/http/client.py index 939615b..6e32c7b 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -214,6 +214,8 @@ MAXAMOUNT = 1048576 # maximal line length when calling readline(). _MAXLINE = 65536 +_MAXHEADERS = 100 + class HTTPMessage(email.message.Message): # XXX The only usage of this method is in @@ -261,6 +263,8 @@ def parse_headers(fp, _class=HTTPMessage): if len(line) > _MAXLINE: raise LineTooLong("header line") headers.append(line) + if len(headers) > _MAXHEADERS: + raise HTTPException("got more than %d headers" % _MAXHEADERS) if line in (b'\r\n', b'\n', b''): break hstring = b''.join(headers).decode('iso-8859-1') diff --git a/Lib/imaplib.py b/Lib/imaplib.py index 42353bb..0994f2b 100644 --- a/Lib/imaplib.py +++ b/Lib/imaplib.py @@ -43,6 +43,15 @@ IMAP4_PORT = 143 IMAP4_SSL_PORT = 993 AllowedVersions = ('IMAP4REV1', 'IMAP4') # Most recent first +# Maximal line length when calling readline(). This is to prevent +# reading arbitrary length lines. RFC 3501 and 2060 (IMAP 4rev1) +# don't specify a line length. RFC 2683 however suggests limiting client +# command lines to 1000 octets and server command lines to 8000 octets. +# We have selected 10000 for some extra margin and since that is supposedly +# also what UW and Panda IMAP does. +_MAXLINE = 10000 + + # Commands Commands = { @@ -256,7 +265,10 @@ class IMAP4: def readline(self): """Read line from remote.""" - return self.file.readline() + line = self.file.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise self.error("got more than %d bytes" % _MAXLINE) + return line def send(self, data): diff --git a/Lib/nntplib.py b/Lib/nntplib.py index 01d4303..9766be3 100644 --- a/Lib/nntplib.py +++ b/Lib/nntplib.py @@ -85,6 +85,13 @@ __all__ = ["NNTP", "decode_header", ] +# maximal line length when calling readline(). This is to prevent +# reading arbitrary lenght lines. RFC 3977 limits NNTP line length to +# 512 characters, including CRLF. We have selected 2048 just to be on +# the safe side. +_MAXLINE = 2048 + + # Exceptions raised when an error or invalid response is received class NNTPError(Exception): """Base class for all nntplib exceptions""" @@ -424,7 +431,9 @@ class _NNTPBase: """Internal: return one line from the server, stripping _CRLF. Raise EOFError if the connection is closed. Returns a bytes object.""" - line = self.file.readline() + line = self.file.readline(_MAXLINE +1) + if len(line) > _MAXLINE: + raise NNTPDataError('line too long') if self.debugging > 1: print('*get*', repr(line)) if not line: raise EOFError diff --git a/Lib/poplib.py b/Lib/poplib.py index be98a7d..d68f169 100644 --- a/Lib/poplib.py +++ b/Lib/poplib.py @@ -40,6 +40,12 @@ CR = b'\r' LF = b'\n' CRLF = CR+LF +# maximal line length when calling readline(). This is to prevent +# reading arbitrary lenght lines. RFC 1939 limits POP3 line length to +# 512 characters, including CRLF. We have selected 2048 just to be on +# the safe side. +_MAXLINE = 2048 + class POP3: @@ -118,7 +124,10 @@ class POP3: # Raise error_proto('-ERR EOF') if the connection is closed. def _getline(self): - line = self.file.readline() + line = self.file.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise error_proto('line too long') + if self._debugging > 1: print('*get*', repr(line)) if not line: raise error_proto('-ERR EOF') octets = len(line) @@ -166,31 +166,59 @@ class CertificateError(ValueError): pass -def _dnsname_to_pat(dn, max_wildcards=1): +def _dnsname_match(dn, hostname, max_wildcards=1): + """Matching according to RFC 6125, section 6.4.3 + + http://tools.ietf.org/html/rfc6125#section-6.4.3 + """ pats = [] - for frag in dn.split(r'.'): - if frag.count('*') > max_wildcards: - # Issue #17980: avoid denials of service by refusing more - # than one wildcard per fragment. A survey of established - # policy among SSL implementations showed it to be a - # reasonable choice. - raise CertificateError( - "too many wildcards in certificate DNS name: " + repr(dn)) - if frag == '*': - # When '*' is a fragment by itself, it matches a non-empty dotless - # fragment. - pats.append('[^.]+') - else: - # Otherwise, '*' matches any dotless fragment. - frag = re.escape(frag) - pats.append(frag.replace(r'\*', '[^.]*')) - return re.compile(r'\A' + r'\.'.join(pats) + r'\Z', re.IGNORECASE) + if not dn: + return False + + leftmost, *remainder = dn.split(r'.') + + wildcards = leftmost.count('*') + if wildcards > max_wildcards: + # Issue #17980: avoid denials of service by refusing more + # than one wildcard per fragment. A survery of established + # policy among SSL implementations showed it to be a + # reasonable choice. + raise CertificateError( + "too many wildcards in certificate DNS name: " + repr(dn)) + + # speed up common case w/o wildcards + if not wildcards: + return dn.lower() == hostname.lower() + + # RFC 6125, section 6.4.3, subitem 1. + # The client SHOULD NOT attempt to match a presented identifier in which + # the wildcard character comprises a label other than the left-most label. + if leftmost == '*': + # When '*' is a fragment by itself, it matches a non-empty dotless + # fragment. + pats.append('[^.]+') + elif leftmost.startswith('xn--') or hostname.startswith('xn--'): + # RFC 6125, section 6.4.3, subitem 3. + # The client SHOULD NOT attempt to match a presented identifier + # where the wildcard character is embedded within an A-label or + # U-label of an internationalized domain name. + pats.append(re.escape(leftmost)) + else: + # Otherwise, '*' matches any dotless string, e.g. www* + pats.append(re.escape(leftmost).replace(r'\*', '[^.]*')) + + # add the remaining fragments, ignore any wildcards + for frag in remainder: + pats.append(re.escape(frag)) + + pat = re.compile(r'\A' + r'\.'.join(pats) + r'\Z', re.IGNORECASE) + return pat.match(hostname) def match_hostname(cert, hostname): """Verify that *cert* (in decoded format as returned by - SSLSocket.getpeercert()) matches the *hostname*. RFC 2818 rules - are mostly followed, but IP addresses are not accepted for *hostname*. + SSLSocket.getpeercert()) matches the *hostname*. RFC 2818 and RFC 6125 + rules are followed, but IP addresses are not accepted for *hostname*. CertificateError is raised on failure. On success, the function returns nothing. @@ -201,7 +229,7 @@ def match_hostname(cert, hostname): san = cert.get('subjectAltName', ()) for key, value in san: if key == 'DNS': - if _dnsname_to_pat(value).match(hostname): + if _dnsname_match(value, hostname): return dnsnames.append(value) if not dnsnames: @@ -212,7 +240,7 @@ def match_hostname(cert, hostname): # XXX according to RFC 2818, the most specific Common Name # must be used. if key == 'commonName': - if _dnsname_to_pat(value).match(hostname): + if _dnsname_match(value, hostname): return dnsnames.append(value) if len(dnsnames) > 1: diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 778e919..31c0b6a 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -347,6 +347,15 @@ class BasicTest(TestCase): self.fail("Did not expect response from HEAD request") self.assertEqual(bytes(b), b'\x00'*5) + def test_too_many_headers(self): + headers = '\r\n'.join('Header%d: foo' % i + for i in range(client._MAXHEADERS + 1)) + '\r\n' + text = ('HTTP/1.1 200 OK\r\n' + headers) + s = FakeSocket(text) + r = client.HTTPResponse(s) + self.assertRaisesRegex(client.HTTPException, + r"got more than \d+ headers", r.begin) + def test_send_file(self): expected = (b'GET /foo HTTP/1.1\r\nHost: example.com\r\n' b'Accept-Encoding: identity\r\nContent-Length:') diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py index c37ea1d..81bfd1f 100644 --- a/Lib/test/test_imaplib.py +++ b/Lib/test/test_imaplib.py @@ -325,6 +325,17 @@ class BaseThreadedNetworkedTests(unittest.TestCase): self.assertEqual(ret, "OK") + def test_linetoolong(self): + class TooLongHandler(SimpleIMAPHandler): + def handle(self): + # Send a very long response line + self.wfile.write(b'* OK ' + imaplib._MAXLINE*b'x' + b'\r\n') + + with self.reaped_server(TooLongHandler) as server: + self.assertRaises(imaplib.IMAP4.error, + self.imap_class, *server.server_address) + + class ThreadedNetworkedTests(BaseThreadedNetworkedTests): server_class = socketserver.TCPServer diff --git a/Lib/test/test_nntplib.py b/Lib/test/test_nntplib.py index 7cf497a..d00c9db 100644 --- a/Lib/test/test_nntplib.py +++ b/Lib/test/test_nntplib.py @@ -584,6 +584,11 @@ class NNTPv1Handler: <a4929a40-6328-491a-aaaf-cb79ed7309a2@q2g2000vbk.googlegroups.com> <f30c0419-f549-4218-848f-d7d0131da931@y3g2000vbm.googlegroups.com> .""") + elif (group == 'comp.lang.python' and + date_str in ('20100101', '100101') and + time_str == '090000'): + self.push_lit('too long line' * 3000 + + '\n.') else: self.push_lit("""\ 230 An empty list of newsarticles follows @@ -1179,6 +1184,11 @@ class NNTPv1v2TestsMixin: self.assertEqual(cm.exception.response, "435 Article not wanted") + def test_too_long_lines(self): + dt = datetime.datetime(2010, 1, 1, 9, 0, 0) + self.assertRaises(nntplib.NNTPDataError, + self.server.newnews, "comp.lang.python", dt) + class NNTPv1Tests(NNTPv1v2TestsMixin, MockedNNTPTestsMixin, unittest.TestCase): """Tests an NNTP v1 server (no capabilities).""" diff --git a/Lib/test/test_poplib.py b/Lib/test/test_poplib.py index 935848b..dd51ac9 100644 --- a/Lib/test/test_poplib.py +++ b/Lib/test/test_poplib.py @@ -94,7 +94,7 @@ class DummyPOP3Handler(asynchat.async_chat): def cmd_list(self, arg): if arg: - self.push('+OK %s %s' %(arg, arg)) + self.push('+OK %s %s' % (arg, arg)) else: self.push('+OK') asynchat.async_chat.push(self, LIST_RESP) @@ -278,6 +278,10 @@ class TestPOP3Class(TestCase): foo = self.client.retr('foo') self.assertEqual(foo, expected) + def test_too_long_lines(self): + self.assertRaises(poplib.error_proto, self.client._shortcmd, + 'echo +%s' % ((poplib._MAXLINE + 10) * 'a')) + def test_dele(self): self.assertOK(self.client.dele('foo')) @@ -400,7 +404,13 @@ if SUPPORTS_SSL: def tearDown(self): if self.client.file is not None and self.client.sock is not None: - self.client.quit() + try: + self.client.quit() + except poplib.error_proto: + # happens in the test_too_long_lines case; the overlong + # response will be treated as response to QUIT and raise + # this exception + pass self.server.stop() def test_stls(self): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 2605e68..b1cb8c5 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -358,11 +358,7 @@ class BasicSocketTests(unittest.TestCase): fail(cert, 'Xa.com') fail(cert, '.a.com') - cert = {'subject': ((('commonName', 'a.*.com'),),)} - ok(cert, 'a.foo.com') - fail(cert, 'a..com') - fail(cert, 'a.com') - + # only match one left-most wildcard cert = {'subject': ((('commonName', 'f*.com'),),)} ok(cert, 'foo.com') ok(cert, 'f.com') @@ -377,6 +373,36 @@ class BasicSocketTests(unittest.TestCase): fail(cert, 'example.org') fail(cert, 'null.python.org') + # error cases with wildcards + cert = {'subject': ((('commonName', '*.*.a.com'),),)} + fail(cert, 'bar.foo.a.com') + fail(cert, 'a.com') + fail(cert, 'Xa.com') + fail(cert, '.a.com') + + cert = {'subject': ((('commonName', 'a.*.com'),),)} + fail(cert, 'a.foo.com') + fail(cert, 'a..com') + fail(cert, 'a.com') + + # wildcard doesn't match IDNA prefix 'xn--' + idna = 'püthon.python.org'.encode("idna").decode("ascii") + cert = {'subject': ((('commonName', idna),),)} + ok(cert, idna) + cert = {'subject': ((('commonName', 'x*.python.org'),),)} + fail(cert, idna) + cert = {'subject': ((('commonName', 'xn--p*.python.org'),),)} + fail(cert, idna) + + # wildcard in first fragment and IDNA A-labels in sequent fragments + # are supported. + idna = 'www*.pythön.org'.encode("idna").decode("ascii") + cert = {'subject': ((('commonName', idna),),)} + ok(cert, 'www.pythön.org'.encode("idna").decode("ascii")) + ok(cert, 'www1.pythön.org'.encode("idna").decode("ascii")) + fail(cert, 'ftp.pythön.org'.encode("idna").decode("ascii")) + fail(cert, 'pythön.org'.encode("idna").decode("ascii")) + # Slightly fake real-world example cert = {'notAfter': 'Jun 26 21:41:46 2011 GMT', 'subject': ((('commonName', 'linuxfrz.org'),),), @@ -437,7 +463,7 @@ class BasicSocketTests(unittest.TestCase): cert = {'subject': ((('commonName', 'a*b.com'),),)} ok(cert, 'axxb.com') cert = {'subject': ((('commonName', 'a*b.co*'),),)} - ok(cert, 'axxb.com') + fail(cert, 'axxb.com') cert = {'subject': ((('commonName', 'a*b*.com'),),)} with self.assertRaises(ssl.CertificateError) as cm: ssl.match_hostname(cert, 'axxbxxc.com') @@ -23,6 +23,24 @@ Library - Issue #19329: Optimized compiling charsets in regular expressions. +- Issue #16037: HTTPMessage.readheaders() raises an HTTPException when more than + 100 headers are read. Adapted from patch by Jyrki Pulliainen. + +- Issue #16040: CVE-2013-1752: nntplib: Limit maximum line lengths to 2048 to + prevent readline() calls from consuming too much memory. Patch by Jyrki + Pulliainen. + +- Issue #16041: CVE-2013-1752: poplib: Limit maximum line lengths to 2048 to + prevent readline() calls from consuming too much memory. Patch by Jyrki + Pulliainen. + +- Issue #17997: Change behavior of ``ssl.match_hostname()`` to follow RFC 6125, + for security reasons. It now doesn't match multiple wildcards nor wildcards + inside IDN fragments. + +- Issue #16039: CVE-2013-1752: Change use of readline in imaplib module to limit + line length. Patch by Emil Lind. + - Issue #19330: the unnecessary wrapper functions have been removed from the implementations of the new contextlib.redirect_stdout and contextlib.suppress context managers, which also ensures they provide |