[issue29870] ssl socket leak

2017-03-22 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29870>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29781] SSLObject.version returns incorrect value before handshake.

2017-03-10 Thread Cory Benfield

Cory Benfield added the comment:

I updated the test script to try with a file-descriptor set and OpenSSL returns 
TLSv1.2 for that one as well. This strongly suggests that OpenSSL's 
SSL_get_version documentation is somewhat misleading, and that an SSL object 
will return a version even when it's not connected.

If Python wants to consider this a bug, it will need to track connections state 
for the SSLObject like it does for the SSLSocket. Otherwise, Python can 
redocument version for SSLObject to say that it will always return a value.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29781>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29781] SSLObject.version returns incorrect value before handshake.

2017-03-10 Thread Cory Benfield

Cory Benfield added the comment:

This actually appears to be an outcome of OpenSSL's logic. I've attached a 
smallish C file that, when run against OpenSSL 1.0.2 on my machine, prints 
"TLSv1.2".

This seems like a behaviour we'll have to work around in Python to get the 
outcome we want here.

--
Added file: http://bugs.python.org/file46715/test.c

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29781>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29781] SSLObject.version returns incorrect value before handshake.

2017-03-10 Thread Cory Benfield

Cory Benfield added the comment:

A quick test reveals that Python 3.5 is also affected.

--
versions: +Python 3.5

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29781>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29781] SSLObject.version returns incorrect value before handshake.

2017-03-10 Thread Cory Benfield

New submission from Cory Benfield:

The SSLObject object from the ssl module has a version() method that is 
undocumented. A reasonable assumption for the behaviour of that method is that 
it would follow the behaviour of the same method on SSLSocket(), which has the 
following documentation:

> Return the actual SSL protocol version negotiated by the connection as
> a string, or None is no secure connection is established. As of this
> writing, possible return values include "SSLv2", "SSLv3", "TLSv1",
> "TLSv1.1" and "TLSv1.2". Recent OpenSSL versions may define more return
> values.

However, SSLObject does not follow that behaviour:

Python 3.6.0 (default, Jan 18 2017, 18:08:34) 
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ssl
>>> ctx = ssl.create_default_context()
>>> in_bio = ssl.MemoryBIO()
>>> out_bio = ssl.MemoryBIO()
>>> buffers = ctx.wrap_bio(in_bio, out_bio)
>>> buffers.version()
'TLSv1.2'

That is, a SSLObject that does not have a TLS session established will 
incorrectly report that it is using a TLS version. This method should return 
None in this case.

--
assignee: christian.heimes
components: SSL
messages: 289346
nosy: Lukasa, christian.heimes
priority: normal
severity: normal
status: open
title: SSLObject.version returns incorrect value before handshake.
versions: Python 3.6

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29781>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue29136] Add OP_NO_TLSv1_3

2017-01-25 Thread Cory Benfield

Cory Benfield added the comment:

For those who want to keep track, the relevant OpenSSL ticket for configuring 
TLSv1.3 cipher suites is https://github.com/openssl/openssl/issues/2276.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29136>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28938] match_hostname treats SAN IP address as DNS name and fails to check CN then

2016-12-11 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa
status: pending -> open

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue28938>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28570] httplib mishandles unknown 1XX status codes

2016-10-31 Thread Cory Benfield

Cory Benfield added the comment:

101 should probably be special-cased, because in that case the underlying 
protocol is being totally changed.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue28570>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28570] httplib mishandles unknown 1XX status codes

2016-10-31 Thread Cory Benfield

New submission from Cory Benfield:

Long story short


http.client does not tolerate non-100 or 101 status codes from the 1XX block in 
a sensible manner: it treats them as final responses, rather than as 
provisional ones.


Expected behaviour
~~

When http.client receives a 1XX status code that it does not recognise, it 
should either surface these to a user that expects them by means of a callback, 
or it should ignore them and only show the user the eventual final status code. 
This is required by RFC 7231 Section 6.2 (Informational 1xx), which reads:

> A client MUST be able to parse one or more 1xx responses received prior to a 
> final response, even if the client does not expect one. A user agent MAY 
> ignore unexpected 1xx responses.


Actual behaviour


http.client treats the 1XX status code as final. It parses the header block as 
though it were a response in the 2XX or higher range. http.client will assume 
that there is no content in the 1XX response, and so will leave the following 
final response unconsumed in the socket buffer. This means that if the 
HTTPConnection is re-used, the user will see the final response following the 
1XX as the response to the next request, even though it is not.


Steps to reproduce
~~

The following "server" can demonstrate the issue:

import socket
import time

document = b'''

  

title


  
  
Hello, world!
  

'''

s = socket.socket()
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(('localhost', 8080))
s.listen(5)

while True:
new_socket, _ = s.accept()
data = b''

while not data.endswith(b'\r\n\r\n'):
data += new_socket.recv(8192)

new_socket.sendall(
b'HTTP/1.1 103 Early Hints\r\n'
b'Server: socketserver/1.0.0\r\n'
b'Link: ; rel=preload; as=style\r\n'
b'Link: ; rel=preload; as=script\r\n'
b'\r\n'
)
time.sleep(1)
new_socket.sendall(
b'HTTP/1.1 200 OK\r\n'
b'Server: socketserver/1.0.0\r\n'
b'Content-Type: text/html\r\n'
b'Content-Length: %s\r\n'
b'Link: ; rel=preload; as=style\r\n'
b'Link: ; rel=preload; as=script\r\n'
b'Connection: close\r\n'
b'\r\n' % len(document)
)
new_socket.sendall(document)
new_socket.close()


If this server is run directly, the following client can be used to test it:

from http.client import HTTPConnection

c = HTTPConnection('localhost', 8080)
c.request('GET', '/')
r = c.getresponse()

print("Status: {} {}".format(r.status, r.reason))
print("Headers: {}".format(r.getheaders()))
print("Body: {}".format(r.read()))


This client will print

Status: 103 Early Hints
Headers: [('Content-Length', '0'), ('Server', 'socketserver/1.0.0'), ('Link', 
'; rel=preload; as=style'), ('Link', '; 
rel=preload; as=script')]
Body: b''


This response is wrong: the 200 header block is hidden from the client. 
Unfortunately, http.client doesn't allow us to ask for the next response: 
another call to "getresponse()" raises a "ResponseNotReady: Idle" exception.

--
components: Library (Lib)
messages: 279823
nosy: Lukasa
priority: normal
severity: normal
status: open
title: httplib mishandles unknown 1XX status codes
type: behavior
versions: Python 3.7

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue28570>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28453] SSLObject.selected_alpn_protocol() not documented

2016-10-16 Thread Cory Benfield

Cory Benfield added the comment:

Yeah, probably! There aren't many protocols with defined ALPN identifiers, but 
we should still probably explain how this works. Do you want me to write them?

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue28453>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17305] IDNA2008 encoding missing

2016-10-12 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue17305>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue22450] urllib doesn't put Accept: */* in the headers

2016-09-09 Thread Cory Benfield

Cory Benfield added the comment:

So, leaping in on the Requests side of things for a moment, two notes. Firstly: 
copying curl is rarely a bad thing to do, especially for a behaviour curl has 
had for a long time.

However, in this case the stronger argument is that just because the RFCs say 
that Accept: */* is implied doesn't mean it can safely be omitted. In practice, 
origin servers behave unexpectedly when the header is omitted, and in general 
behave more predictable when it is emitted. For that reason, it should be added 
by Python's standard library. 

HTTP/1.1 is a protocol where "as deployed" means much more than "as specified", 
sadly.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue22450>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28022] SSL releated deprecation for 3.6

2016-09-08 Thread Cory Benfield

Cory Benfield added the comment:

10/10, yes. I will happily provide code review for this.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue28022>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27850] Remove 3DES from cipher list (sweet32 CVE-2016-2183)

2016-09-08 Thread Cory Benfield

Cory Benfield added the comment:

Thanks for your response Larry. I think it cleared up my understanding a bit, 
and I'm (extremely!) sympathetic to your desire to not get any closer to this 
problem than you have to.

I think it may be worth, in future, defining what effort will be made to 
achieve compatibility with libraries that Python relies on. I can see several 
questions here that, AFAIK, have no concrete answer:

- Can a Python minor version increase (e.g. 3.6 -> 3.7) add support for a new 
ABI in a library dependency? (This one has an answer, which is certainly yes, 
but we could still stand to write it down because you'd be amazed how often it 
helps to write down the basic starting point of the argument.)
- Can a Python patch version increase *before* security release mode (e.g. 
3.6.1 -> 3.6.2) add support for a new ABI in a library dependency?
- What about a new API that maintains ABI compatibility?
- Can a Python security version increase (e.g. 3.4.5 -> 3.4.6) add support for 
a new ABI in a library dependency?
- What about a new API that maintains ABI compatibility?
- How do the answers to the above questions vary if the change is 
security-focused (e.g. AES is broken tomorrow so ChaCha20 is the only safe 
cipher left in OpenSSL)?

I'm not qualified or authoritative enough to answer those questions, but having 
an answer to them would help modulate expectations from people like myself.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27850>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27850] Remove 3DES from cipher list (sweet32 CVE-2016-2183)

2016-09-07 Thread Cory Benfield

Cory Benfield added the comment:

> Future OpenSSLs don't affect Python 3.4, as Python 3.4 won't be upgraded to 
> them.

Can I get a clarification on this, please, Larry? I just want to confirm I 
understand what your meaning is here.

My reading of this is that for OpenSSL Python defines a range of compatible 
sonames at the time of the first release in a series (e.g. 3.4.0), and then 
will never extend that in either direction for that release series. Put another 
way: patches to extend the supported OpenSSL versions are not acceptable in 
patch releases of Python.

Is that reading accurate?

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27850>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27850] Remove 3DES from cipher list (sweet32 CVE-2016-2183)

2016-08-24 Thread Cory Benfield

Cory Benfield added the comment:

As another data point, I just pushed a PR to remove HIGH from urllib3/requests 
for exactly this reason, and Twisted already doesn't use it.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27850>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27850] Remove 3DES from cipher list (sweet32 CVE-2016-2183)

2016-08-24 Thread Cory Benfield

Cory Benfield added the comment:

+1 from me, Requests, urllib3, and Twisted are all removing 3DES cipher suites 
from our default list.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27850>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27766] Add ChaCha20 Poly1305 to SSL ciphers

2016-08-15 Thread Cory Benfield

Cory Benfield added the comment:

Christian: Certainly I'd like to be able to use that API from within urllib3 
and Twisted. Having something public would be really convenient. Of course, 
it'd be good if OpenSSL exposed something useful here, but in the absence of 
that Python would be convenient.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27766>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27766] Add ChaCha20 Poly1305 to SSL ciphers

2016-08-15 Thread Cory Benfield

Cory Benfield added the comment:

Update for Requests+urllib3 is here: https://github.com/shazow/urllib3/pull/947

Update for Twisted is here: https://twistedmatrix.com/trac/ticket/8760

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27766>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27766] Add ChaCha20 Poly1305 to SSL ciphers

2016-08-15 Thread Cory Benfield

Cory Benfield added the comment:

Yup. So for Requests at least, the fix is easy: because OpenSSL kindly just 
quietly ignores cipher suites it doesn't know about we can unconditionally add 
it to the requests/urllib3 cipher string. In the first instance we'll just do 
it statically, and then we can consider down the road whether 
Python/cryptography could give us a way to ask whether we should prefer 
ChaCha20 over AES-GCM.

In the short term, my expectation is that we'd still want to prioritise AES-GCM 
over ChaCha20 in Requests: is there any reason to think that I'm wrong there?

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27766>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27766] Add ChaCha20 Poly1305 to SSL ciphers

2016-08-15 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27766>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27744] Add AF_ALG (Linux Kernel crypto) to socket module

2016-08-12 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27744>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27716] http.client truncates UTF-8 encoded headers

2016-08-09 Thread Cory Benfield

Cory Benfield added the comment:

Honestly, David, everything's a mess on this front. The authoritative document 
here is RFC 7230 Section 3.2.4 
(https://tools.ietf.org/html/rfc7230#section-3.2.4). The last paragraph of that 
section reads:

   Historically, HTTP has allowed field content with text in the
   ISO-8859-1 charset [ISO-8859-1], supporting other charsets only
   through use of [RFC2047] encoding.  In practice, most HTTP header
   field values use only a subset of the US-ASCII charset [USASCII].
   Newly defined header fields SHOULD limit their field values to
   US-ASCII octets.  A recipient SHOULD treat other octets in field
   content (obs-text) as opaque data.

In the case of http.client, actually maps pretty closely to Python 3's bytes 
object: header field values are basically ASCII + arbitrary opaque bytes. While 
UTF-8 is not strictly called out as allowed, neither is it called out as 
forbidden.

In this case, I'd say that there's no need to be too pedantic about Latin 1 at 
this stage in the pipeline. Python 3 is welcome to decode using Latin 1 *after* 
the header block has been split, because at least then it can be fixed up due 
to the round-tripping nature of Latin 1. But doing it here seems to just 
confuse the email parser.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27716>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27716] http.client truncates UTF-8 encoded headers

2016-08-09 Thread Cory Benfield

Cory Benfield added the comment:

Simple repro case:

import http.client
conn = http.client.HTTPConnection('pl.bab.la')
conn.request("GET", '/slownik/angielski-polski/')
resp = conn.getresponse()
resp.read()  # Hangs here

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27716>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27716] http.client truncates UTF-8 encoded headers

2016-08-09 Thread Cory Benfield

New submission from Cory Benfield:

Originally reported as Requests issue #3485: 
https://github.com/kennethreitz/requests/issues/3485

On Python 3, http.client uses the email module to parse its HTTP headers. The 
email module, for better or worse, requires that it parse headers as *text*: 
that is, that they be decoded from bytes first and then parsed.

This doesn't work for UTF-8 encoded headers. For example, the URL 
`'http://pl.bab.la/slownik/angielski-polski/'` returns the following Link 
header, encoded as UTF-8: `Link: <http://www.babla.cn/英语-波兰语/>; 
rel="alternate"; hreflang="zh-Hans", 
<http://cs.bab.la/slovnik/anglicky-polsky/>; rel="alternate"; hreflang="cs", 
<http://da.bab.la/ordbog/engelsk-polsk/>; rel="alternate"; hreflang="da", 
<http://de.bab.la/woerterbuch/englisch-polnisch/>; rel="alternate"; 
hreflang="de", <http://www.babla.gr/αγγλικα-πολωνικα/>; rel="alternate"; 
hreflang="el", <http://en.bab.la/dictionary/english-polish/>; rel="alternate"; 
hreflang="en", <http://eo.bab.la/vortaro/angla-pola/>; rel="alternate"; 
hreflang="eo", <http://es.bab.la/diccionario/ingles-polaco/>; rel="alternate"; 
hreflang="es", <http://fi.bab.la/sanakirja/englanti-puola/>; rel="alternate"; 
hreflang="fi", <http://fr.bab.la/dictionnaire/anglais-polonais/>; 
rel="alternate"; hreflang="fr", <http://www.babla.in/अंग्रेज़ी-पोलिश/>; 
rel="alternate"; hreflang="hi", <http://hu.bab.la/szótár/angol-lengyel/>; 
rel="alternate"; hreflang="hu", 
<http://www.babla.co.id/bahasa-inggris-bahasa-polandia/>; rel="alternate"; 
hreflang="id", <http://it.bab.la/dizionario/inglese-polacco/>; rel="alternate"; 
hreflang="it", <http://ja.bab.la/辞書/英語-ポーランド語/>; rel="alternate"; 
hreflang="ja", <http://www.babla.kr/영어-폴란드어/>; rel="alternate"; hreflang="ko", 
<http://nl.bab.la/woordenboek/engels-pools/>; rel="alternate"; hreflang="nl", 
<http://www.babla.no/engelsk-polsk/>; rel="alternate"; hreflang="no", 
<http://pl.bab.la/slownik/angielski-polski/>; rel="alternate"; hreflang="pl", 
<http://pt.bab.la/dicionario/ingles-polones/>; rel="alternate"; hreflang="pt", 
<http://ro.bab.la/dictionar/engleza-poloneza/>; rel="alternate"; hreflang="ro", 
<http://www.babla.ru/английский-польский/>; rel="alternate"; hreflang="ru", 
<http://sv.bab.la/lexikon/engelsk-polsk/>; rel="alternate"; hreflang="sv", 
<http://sw.bab.la/kamusi/kiingereza-kipolishi/>; rel="alternate"; 
hreflang="sw", <http://www.babla.co.th/english-polish/>; rel="alternate"; 
hreflang="th", <http://tr.bab.la/sozluk/ingilizce-lehce/>; rel="alternate"; 
hreflang="tr", <http://www.babla.vn/tieng-anh-tieng-ba-lan/>; rel="alternate"; 
hreflang="vi"`.

When decoded using ISO-8859-1, this header gets truncated and this also causes 
the header block parsing to stop. This means that we don't see the 
Content-Length header, causing the HTTP client to wait for connection closure 
to consider the body terminated.

Really the only correct fix for this is for http.client to stop insisting that 
the headers be decoded before they are parsed, and instead to decode *after*. 
That way, at least, user code can recover the headers and handle them more 
sensibly.

--
components: Library (Lib)
messages: 272236
nosy: Lukasa
priority: normal
severity: normal
status: open
title: http.client truncates UTF-8 encoded headers
versions: Python 3.5

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27716>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27706] Random.seed, whose purpose is purportedly determinism, behaves non-deterministically with strings due to hash randomization

2016-08-08 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27706>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27568] "HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts

2016-07-19 Thread Cory Benfield

Cory Benfield added the comment:

Ok, so I've taken a preliminary look at this patch. It looks good to me! I have 
one question: right now the patch as written will blow away not just 
HTTP_PROXY, but also any other mixed-case spelling of that name (e.g. 
HtTp_PrOxY) in a CGI environment.

That's *probably* not a concern: I think in practice such a spelling is almost 
never used. However, I wanted to draw it out explicitly: we should probably 
include a code comment that indicates that we know that it's a side effect of 
the code, and that we don't care.

For what it's worth, we should also consider commenting with a note regarding 
the CVE number assigned to Python. We may want to consider getting a CVE number 
for this specific fix as well, though I'd need to chat to someone in the PSRT 
at this point to get an idea of what they think.

Good work!

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27568>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27568] "HTTPoxy", use of HTTP_PROXY flag supplied by attacker in CGI scripts

2016-07-19 Thread Cory Benfield

Cory Benfield added the comment:

I like this patch a great deal, I'll happily review it with docs and tests.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27568>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23794] http package should support HTTP/2

2016-06-30 Thread Cory Benfield

Cory Benfield added the comment:

It occurs to me that I should update this issue to mention that the HTTP/2 
parser I spoke about above now exists, and has existed for some time, as 
hyper-h2. It's available on PyPI: https://pypi.python.org/pypi/h2

It's fully spec-compliant, and already used as the basis for HTTP/2 
implementations in mitmproxy, Twisted, and hyper itself.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue23794>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27288] secrets should use getrandom() on Linux

2016-06-11 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27288>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27292] Warn users that os.urandom() can return insecure values

2016-06-11 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27292>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27266] Always use getrandom() in os.random() on Linux and add block=False parameter to os.urandom()

2016-06-09 Thread Cory Benfield

Cory Benfield added the comment:

> It looks like people don't read what I'm writing :-( 

I'm reading you, Victor, but you and I aren't disagreeing, so I'm not trying to 
convince you. =) I'm trying to convince Larry.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27266>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27266] Always use getrandom() in os.random() on Linux and add block=False parameter to os.urandom()

2016-06-09 Thread Cory Benfield

Cory Benfield added the comment:

> Those say that if you call getrandom(GRND_NONBLOCK) before the entropy
> pool has been initialized, it will return EAGAIN, but any time you read
> from /dev/urandom you will always get random data.


Yeah, so this is why the crypto folks were all up in arms about falling back to 
the /dev/urandom behaviour on Linux: Linux's /dev/urandom behaviour is really 
pretty dangerous.

In essence, on Linux, /dev/urandom will *always* return you some bytes, and 
their actual quality is entirely uncertain. This means that fundamentally 
without interrogating the kernel state before you read, you can't really be 
sure that /dev/urandom is safe for what you want it to be safe for.

But /dev/random isn't a suitable replacement in most cases, because /dev/random 
goes too far the other way: it has this hyper-paranoid notion about "entropy" 
that isn't really well-founded, and so sometimes it'll go on strike for a while.

This is why we've been pushing to keep hold of the os.urandom() implementation 
that CPython 3.5 now has: it irons out one of the most annoying warts in Linux 
RNGs. As Ted Tso has said elsewhere in this thread, /dev/urandom only exhibits 
its current behaviour for backward compatibility reasons: getrandom(flags=0) is 
really the ideal behaviour for almost any case *except* what Python is doing at 
startup.

Not that it needs saying again, but I'm still in favour of doing something like 
what Christian is suggesting, or like I suggested earlier (have os.urandom 
remain good, have Python internally fall back to weaker seeds for random and 
SipHash if it's run so early that the system hasn't started up fully yet).

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27266>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27266] Always use getrandom() in os.random() on Linux and add block=False parameter to os.urandom()

2016-06-09 Thread Cory Benfield

Cory Benfield added the comment:

> But Theodore Ts'o said on the tracker: if you call getrandom() and don't pass 
> in GRND_RANDOM, it's equivalent to /dev/urandom.  So, if getrandom is 
> available, getrandom(flags=0) will always work and never block.

Can we please try to be clear about what kind of blocking we mean? 
getrandom(flags=0) absolutely *can* block: that's what the original issue was 
all about. To ensure it *never* blocks you need to call 
getrandom(GRND_NONBLOCK): that's why the flag exists.

Put another way:

- getrandom(flags=GRND_RANDOM) == /dev/random
- getrandom(flags=GRND_NONBLOCK) == /dev/urandom on Linux
- getrandom(flags=0) == /dev/urandom everywhere but Linux

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27266>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27266] Always use getrandom() in os.random() on Linux and add block=False parameter to os.urandom()

2016-06-09 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27266>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-08 Thread Cory Benfield

Cory Benfield added the comment:

> You're right, it's remotely possible that on platforms where /dev/urandom
> could block, Python startup could therefore also block.  And I'm not
> proposing we fix that, as so far nobody has reported it as a problem.
> 
> This suggests to me that yes I'm talking specifically about the regression
> on Linux in the 3.5 series.

Ok, so with that clarification I personally would prefer Victor's patch from 
#27266, but can also understand wanting to leave the codebase as-is. Either way 
would be consistent with your goals, Larry. Victor's patch is more secure, but 
does cause os.urandom to diverge from the semantics of /dev/urandom in extreme 
conditions (specifically, early boot) on Linux.

That's your tradeoff to make, Larry. =) I think both sides have been 
well-argued here. Thanks for clarifying.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-08 Thread Cory Benfield

Cory Benfield added the comment:

> What I'm trying to avoid here is the surprising situation where someone is 
> using Python on a system where /dev/urandom will never block, and 
> os.urandom() blocks.

At this point I literally do not understand what issue we're trying to solve 
then.

If the problem is that os.urandom() must behave exactly like /dev/urandom, then 
sure, that got regressed.

However, I've been talking based on your two previous pronouncements:

> First, my previous statements stand: Python startup must not block.  "import 
> random" must not block.

and

> I am officially making a pronouncement as Release Manager: Python 3.5 *must
> not* take 90 seconds to start up under *any* circumstances.  I view this as
> a performance regression, and it is and will remain a release blocker for
> 3.5.2.
> 
> Python *must not* require special command-line flags to avoid a 90 second
> startup time.  Python *must not* require a special environment-variable to
> avoid a 90 second startup time.  This is no longer open to debate, and I
> will only be overruled by Guido.

Now, if that's the case, then this patch does not fix that problem. It fixes 
that problem *on Linux*, but not on BSDs.

Perhaps you meant to say that those pronouncements only apply to Linux. That's 
fine, it's your prerogative. But as written, they don't: they're unconditional. 
And if they are unconditional, then again I feel like we have to say that 
/dev/urandom should get *out* of the call path on interpreter startup, because 
it absolutely can block. And based on Colm's original problem around gathering 
entropy, which is almost certainly not a Linux-specific concern, I see no 
reason to believe that this is a hypothetical concern on the BSDs.

So, let me ask a very direct question: does the position about 90s startup 
apply only to Linux?

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-08 Thread Cory Benfield

Cory Benfield added the comment:

I have never seen it block in person, but I also wouldn't expect to. OS X's 
blocking guarantees are the same as FreeBSD's: that is, both /dev/random and 
/dev/urandom block until sufficient entropy has been gathered at startup and 
then never block again.

This means that, on OS X, in practice, /dev/urandom does never block, because 
you basically can't run user code early enough to encounter this problem.

FreeBSD again has the exact same behaviour: /dev/urandom is a symlink to 
/dev/random, and both will block at startup until sufficient entropy is 
gathered and never again. This is the bigger risk for Python, because if Linux 
people want to use Python in their init system it's not unreasonable for 
FreeBSD folks to want to do it too.

This is why I'm concerned about this "solution": while there's no question that 
adding getrandom() made the situation worse on Linux, it has drawn our 
attention to the fact that Python is relying on Linux-only semantics of 
/dev/urandom on all Unices. That's probably not a good plan.

(The above is all facts. Everything in these parentheticals is opinion. Please 
disregard as appropriate.

I agree with the OS X devs in that I believe their implementation *is* better 
than Linux's: sorry Ted! There is no reason to be concerned about using a good 
kernel CSPRNG once sufficient entropy has been gathered *once*. The CSPRNG 
essentially "stretches" the entropy out into a long sequence of numbers, much 
like a cipher like AES "stretches" the entropy in the key across the entire 
cipherstream. Talking about "running out" of entropy in one of these devices is 
weird to me: as a blog post linked earlier mentions, it's like talking about 
"running out of key" in an encryption algorithm.

It seems to me, then, that Linux's /dev/random is wrong in most situations 
(because it sometimes blocks), and /dev/urandom is wrong in some situations 
(because it'll run before it has enough entropy to properly seed the CSPRNG and 
it won't tell you that that is what has happened). On OS X, the best of both 
worlds occurs: you get no random numbers until sufficient entropy has been 
gathered to seed the CSPRNG, and then you get good random numbers from that 
point on.

Please note: I am not a trained cryptographer. However, trained cryptographers 
have agreed with this set of sentiments, so I think I'm on pretty good ground 
here.)

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-08 Thread Cory Benfield

Cory Benfield added the comment:

> If you read #25003, it's clear that /dev/urandom is a well-known UNIX 
> facility with well-known, predictable behavior.  One behavior that I'll draw 
> particular attention to now: it will never block.  If the system is low on 
> entropy, /dev/urandom will produce lower-quality random bits.

That's not accurate.

/dev/urandom is a well-known UNIX facility, yes, but it does not have 
consistent behaviour across Unices. The behaviour you've described here Larry 
is a well-known *Linux* behaviour.

However, on other Unices the behaviour is different. Here, for example, is the 
relevant man page from Mac OS X ("man 4 random"):

 /dev/urandom is a compatibility nod to Linux. On Linux, /dev/urandom will
 produce lower quality output if the entropy pool drains, while
 /dev/random will prefer to block and wait for additional entropy to be
 collected.  With Yarrow, this choice and distinction is not necessary,
 and the two devices behave identically. You may use either.

Note the specific wording here: "the two devices behave identically". That is 
to say, on OS X both /dev/random and /dev/urandom are identical devices, and 
that includes the fact that both will in principle block if used without 
sufficient entropy.

OS X's implementation is a direct descendent of FreeBSD's, so the same caveats 
apply there, and in fact all the BSDs have this exact same behaviour.

So, again, I repeat my objection from above: if the concern is that starting 
Python must never block, then Python must *never* read from /dev/urandom on 
startup. Otherwise, Python *can* block on BSDs (OS X included in principle, 
though in practice I doubt Apple will use Python that early in boot).

At this point I literally no longer care whether os.urandom() is just a wrapper 
around /dev/urandom: we can look back on this in 10 years and see how we feel 
about the choices made by core dev at that time. But if we're arguing that this 
issue is about "Python must never block at startup", then we really have to 
acknowledge that /dev/urandom *can block* on some Unices, and so is entirely 
unacceptable for reading at startup.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

> Python hash randomization only happens once.  So it's not a matter of how 
> early we try the attack, it's a matter of how early we seed Python hash 
> randomization.

Sorry Larry, I was insufficiently clear (relying on context from earlier). I 
totally agree that Python startup should not block. I'm saying that having 
getrandom() called in "blocking mode" for os.urandom, random.SystemRandom, and 
secrets is not a DoS vector.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

> So you are intentionally accepting a new vector for DoS attacks, and calling
this non-reduced security?

This is only a DoS vector if you can hit the server so early in the boot 
process that it doesn't have enough entropy. The *second* enough entropy has 
been gathered getrandom() will never block again.

In essence, then, the situation where it becomes possible to DoS a server is 
entirely outside an attackers control and extremely unlikely to ever actually 
occur in real life: you can only DoS the server if you can demand entropy 
before the system has gathered enough, and if the server has managed to *boot* 
by then then the alternative is that it is incapable of generating secure 
random numbers and shouldn't be running exposed against the web anyway.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27250] Add os.urandom_block()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

Uh, sorry, I meant #26839.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27250>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27250] Add os.urandom_block()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

Marc-Andre: No. See the discussion in the related issue #27249 for more.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27250>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue27250] Add os.urandom_block()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

Let me make the security person argument even though you've dismissed it in 
your original post:

Security should be on by default and opted out of, not the other way around. If 
the obvious choice is insecure then users who aren't careful enough won't 
notice, because even bad RNGs produce numbers that *look* random if not 
carefully evaluated. This means they won't fix it and it will stay there, 
broken, until they are attacked (which they may never notice).

On the other hand, if the obvious choice is secure then users who aren't 
careful enough (that is, ones that don't care about security but do care about 
blocking) *will* notice, because their apps will hang on startup. They'll 
investigate the hang, see the docs, and fix their code. This is a *much better* 
failure mode. There is a reason that modern cars warn you about any upcoming 
problem with the car (e.g. worn brake pads, low oil etc.): users whose 
expectations are violated in a way they can easily detect are much more likely 
to act than users whose expectations are subtly violated.

This is doubly true here, because the only system that is complaining about the 
random numbers right now is CPython, which, being the same system providing 
these new functions, can always ensure that it's doing the right thing. With 
this patch as proposed here, external library developers need to see this 
discussion and realise that, for Python 3.5 or lower, they want os.urandom, and 
for Python 3.6 and higher, they want os.urandom_block(). And, again, because 
os.urandom() still appears to produce high quality random numbers (and most of 
the time *actually does so*), I guarantee at least one application will not 
notice the change and will become open to attack.

Those are the two arguments for making non-blocking be explicit, rather than 
making blocking be specific.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27250>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

Victor Stinner: I found that comment to be pretty patronising, but I'm assuming 
that wasn't the intent. However, your characterisation of my comment was not as 
I intended it: when I said "because it can block", I meant because on almost 
every system urandom will block if there is insufficient randomness to seed the 
kernel CSPRNG.

On FreeBSD, /dev/urandom blocks on startup until sufficiently seeded. On OS X, 
/dev/urandom behaves exactly the same as /dev/random (from the man page: "the 
two devices behave identically"), which is to say it blocks until the CSPRNG is 
sufficiently seeded. On Windows, CryptGenRandom (used by this code) specifies 
no blocking guarantees and the opinion of the wider community is that it too 
will block until sufficient entropy is gathered from startup.

So, let me say this: if the purpose of this patch was to prevent long startup 
delays, *it failed*. On all the systems above os.urandom may continue to block 
system startup. If the purpose of this patch is to prevent the system blocking 
at startup then you *must not use urandom at Python interpreter startup*.

This is why I object to this patch: it weakens the Linux interpreter while not 
fixing the actual problem. If Python does not need a CSPRNG at startup, then it 
should not block waiting for one *on any system*. If it does need a CSPRNG, 
then it should block until seeded. I don't see why some weird in-between 
solution is good enough here.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26839] Python 3.5 running on Linux kernel 3.17+ can block at startup or on importing the random module on getrandom()

2016-06-07 Thread Cory Benfield

Cory Benfield added the comment:

This patch explicitly violates several of the documented constraints of the 
Python standard library.

For example, random.SystemRandom uses os.urandom to generate its random 
numbers. SystemRandom is then used by the secrets module to generate *its* 
random numbers. This means that os.urandom *is* explicitly used by the Python 
standard library to generate cryptographically secure random numbers. It was 
done so in part expressly because the call to random() could block.

If Python needs a non-blocking RNG for internal purposes, that's totally fine, 
a new function should be written that does exactly that. But any code that is 
calling secrets or random.SystemRandom is expecting the documented guarantees 
of that module: that is, that the security profile of the random numbers 
generated by those objects are cryptographically secure. This patch ensures 
that that guarantee is *violated* on Linux systems run on cloud servers, which 
is more than a little alarming to me.

--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26839>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue26970] Replace OpenSSL's CPRNG with system entropy source

2016-05-06 Thread Cory Benfield

Changes by Cory Benfield <c...@lukasa.co.uk>:


--
nosy: +Lukasa

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26970>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue25672] set SSL_MODE_RELEASE_BUFFERS

2015-11-20 Thread Cory Benfield

Cory Benfield added the comment:

Thanks for the updated info Marc-Andre.

Yeah, while generally speaking OpenSSL doesn't ship betas, it does provide them 
as tarballs. I have a beta of 1.0.2 floating around somewhere on my machine 
that I was using for ALPN testing back in 2014, and so I can speak from 
personal experience and say that people do actually work with betas sometimes. 
On this issue (defending ourselves from a CVE) my instinct is to be 
conservative. However, we should allow later patch releases of OpenSSL 1.0.0 to 
have this optimisation if they're safe.

Therefore, I've uploaded a new patch that does allow for 1.0.0m and later to 
use this optimisation too. It makes the conditional a little more complex, but 
c'est la vie.

--
Added file: http://bugs.python.org/file41094/ssl3.patch

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25672>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue25672] set SSL_MODE_RELEASE_BUFFERS

2015-11-20 Thread Cory Benfield

Cory Benfield added the comment:

Good idea Benjamin. I've uploaded a second patch that adjusts the check to be a 
runtime one, rather than a compiled one.

--
Added file: http://bugs.python.org/file41091/ssl2.patch

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25672>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue25672] set SSL_MODE_RELEASE_BUFFERS

2015-11-19 Thread Cory Benfield

Cory Benfield added the comment:

Ok, I've just uploaded an initial draft of the patch for review.

--
keywords: +patch
Added file: http://bugs.python.org/file41083/ssl.patch

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25672>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue25672] Unconditionally set SSL_MODE_RELEASE_BUFFERS

2015-11-19 Thread Cory Benfield

New submission from Cory Benfield:

Originally raised by Ben Bangert on the python-dev mailing list.

It turns out that OpenSSL has a mode setting, SSL_MODE_RELEASE_BUFFERS, that 
can be set by a call to SSK_CTX_set_mode. This mode can potentially reduce 
connection overhead by nearly 18kB *per connection*, a reduction of something 
like 60%. Further, this does not change the behaviour of OpenSSL in any 
meaningful way.

For this reason, we should unconditionally set this mode on all SSL Context 
objects we create.

I'm happy to submit a patch to the standard library that will do this.

--
components: Library (Lib)
messages: 254918
nosy: Lukasa
priority: normal
severity: normal
status: open
title: Unconditionally set SSL_MODE_RELEASE_BUFFERS
versions: Python 3.6

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25672>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue25672] Unconditionally set SSL_MODE_RELEASE_BUFFERS

2015-11-19 Thread Cory Benfield

Cory Benfield added the comment:

Oh, one further requirement: we should *not* set this mode for OpenSSL releases 
1.x through 1.0.1g, which have a NULL pointer dereference vulnerability (CVE 
2014-0198). Thanks to Marc-Andre Lemburg for spotting this.

See also: https://www.rapid7.com/db/vulnerabilities/http-openssl-cve-2014-0198

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25672>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue25529] Provide access to the validated certificate chain in ssl module

2015-11-01 Thread Cory Benfield

New submission from Cory Benfield:

I’m currently working on adding support for HPKP to the Requests and urllib3 
modules. HPKP (HTTP Public Key Pinning), specified in RFC 7469, is an extension 
to HTTP that allows a web server to specify a whitelist of public keys that are 
valid for TLS certificates on that domain. This prevents a rogue certificate 
authority from issuing a certificate that would be trusted by a browser and 
would allow a man-in-the-middle attack on a domain (as happened to Google in 
2013[0]).

Right now, the draft version of the support I have will only work when you use 
PyOpenSSL for your TLS needs, not the standard library. This is because to get 
HPKP to work I need access to the validated certificate chain: that is, the 
certificate chain that OpenSSL has built and validated for the TLS connection. 
I also need to be able to work with those certificates in order to extract 
their public keys. The standard library’s ssl module does not expose any of 
this functionality.

To get this to work with the standard library, I would require the following 
things from the standard library:

1. The ability to access the validated certificate chain. This requires saving 
off the certificate each time the OpenSSL verify callback is called. This is an 
easy enough change to make.
2. The ability to extract the public key from the saved certificates. This 
could be done by extending the logic used for getpeercert() to provide a 
DER-encoded ASN.1 representation of the public key in the dictionary, and then 
using that representation for each cert in the peer cert chain.

The motivation for making this available in the standard library would be pip. 
Right now python.org and all its subdomains (including pypi.python.org) are 
HPKP-enabled. Making this support available in the standard library would 
ensure that all pip installations are safe from man-in-the-middle attacks on 
its packaging infrastructure. Without it, a number of third-party packages 
would be required to add this security. In particular, pip could distribute a 
HPKP preload value for pypi.python.org, which would ensure that pip is truly 
invulnerable to MITM TLS attacks via malicious attackers coercing a CA to 
provide TLS certificates for *.python.org.

I’m happy to do the work required to provide this functionality, but I’d only 
like to start work if people believe there’s a likelihood of getting it merged.

[0]: 
https://nakedsecurity.sophos.com/2013/01/08/the-turktrust-ssl-certificate-fiasco-what-happened-and-what-happens-next/

--
components: Library (Lib)
messages: 253864
nosy: Lukasa
priority: normal
severity: normal
status: open
title: Provide access to the validated certificate chain in ssl module
type: enhancement
versions: Python 3.6

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25529>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17849] Missing size argument in readline() method for httplib's class LineAndFileWrapper

2015-09-03 Thread Cory Benfield

Cory Benfield added the comment:

Martin, the idea of simply rejecting the LifeAndFileWrapper HTTP/0.9 fallback 
for _tunnel feels reasonable to me, that can work. Let me whip up an 
alternative patch that does that.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue17849>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17849] Missing size argument in readline() method for httplib's class LineAndFileWrapper

2015-09-03 Thread Cory Benfield

Cory Benfield added the comment:

Ok, version three of the patch is now available.

--
Added file: http://bugs.python.org/file40345/readline_3.patch

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue17849>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17849] Missing size argument in readline() method for httplib's class LineAndFileWrapper

2015-09-02 Thread Cory Benfield

Cory Benfield added the comment:

Martin: as noted in the comments on this issue, I believed the specific test 
case did not match any of the problems people were encountering: it was just 
the least bad way to trigger the specific flow that was being encountered.

In practice for these issues it's likely some other error condition is 
involved. Regardless, Python 2.7 claims to support HTTP/0.9, which certainly 
allows for proxies to not send a status line, so the test case is a reasonable 
example of what should be supported by httplib.

I'll take another run at this patch to see if I can clean it up.

--

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue17849>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17849] Missing size argument in readline() method for httplib's class LineAndFileWrapper

2015-09-02 Thread Cory Benfield

Cory Benfield added the comment:

Ok, new patch now attached.

--
Added file: http://bugs.python.org/file40328/readline_2.patch

___
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue17849>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23670] Modifications to support iOS as a cross-compilation target

2015-07-27 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23670
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue24667] OrderedDict.popitem() raises KeyError

2015-07-19 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24667
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue24363] httplib fails to handle semivalid HTTP headers

2015-06-03 Thread Cory Benfield

Cory Benfield added the comment:

 It is obvious that this case could be treated as a folded (continuation) 
 line. But in general I think it would be better to ignore the erroneous line, 
 or to record it as a defect so that the server module or other user can check 
 it.

Just to clarify, in an instance very similar to this one this would be 
*terrible* advice. The token that would be lost here is the 'Secure' field on 
the cookie, which is an extremely important token to have: if we don't 
correctly parse it, we run the risk of sending the cookie on plaintext 
connections.

Discarding data is the problem, and while discarding *less* data is an 
improvement, it would be good if we could resolve this problem in such a way 
that we'd have correctly parsed this header.

Generally speaking, if we treat these as continuation lines I think we have the 
best change of making a useful header out of this mess.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24363
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue24363] httplib fails to handle semivalid HTTP headers

2015-06-03 Thread Cory Benfield

Cory Benfield added the comment:

While we're here and I'm recommending to drop as little data as possible: we 
need to be really careful about not exposing ourselves to any kind of data 
smuggling attack here.

It's really important that we don't let attackers construct bodies of requests 
or responses that will cause us to misinterpret header blocks. It's therefore 
going to be really tricky to balance those concerns.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24363
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue24363] httplib fails to handle semivalid HTTP headers

2015-06-02 Thread Cory Benfield

Cory Benfield added the comment:

This is one of those bugs that's actually super tricky to correctly fix.

The correct path is to have the goal of conservatively accepting as many 
headers as possible. Probably this means looking ahead to the next few lines 
and seeing if they appear to roughly keep a header structure (\r\n line breaks 
and colon separated values). However, I'm not entirely sure how to structure 
that sentiment in code at this time.

--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24363
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue24107] Add support for retrieving the certificate chain

2015-05-01 Thread Cory Benfield

New submission from Cory Benfield:

In order to perform HTTP Public Key Pinning (HPKP), it's necessary to have 
access to every certificate in the certificate trust chain. This is because the 
pinned key may actually be an intermediate or root certificate, rather than the 
leaf certificate.

PyOpenSSL offers this functionality, and it ought to be a relatively simple 
enhancement to expose the equivalent function in the stdlib.

For more background, see the urllib3 issue tracking the HPKP feature: 
https://github.com/shazow/urllib3/pull/607

--
components: Library (Lib)
messages: 242341
nosy: Lukasa
priority: normal
severity: normal
status: open
title: Add support for retrieving the certificate chain
type: enhancement
versions: Python 2.7, Python 3.5, Python 3.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24107
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23989] Add recommendation to use requests to the documentation, per summit

2015-04-17 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23989
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23794] http package should support HTTP/2

2015-04-13 Thread Cory Benfield

Cory Benfield added the comment:

I spoke to some people after my PyCon talk about this, and agreed that it would 
be a good idea to split out the framing and HPACK stuff from hyper to make it 
easier for people like aiohttp to prototype.

The framing layer is already available from PyPI[0] and on GitHub[1], and I'm 
working on pulling the HPACK layer out right now[2], which should be available 
later today.

[0]: https://pypi.python.org/pypi/hyperframe
[1]: https://github.com/Lukasa/hyperframe
[2]: https://github.com/Lukasa/hpack

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23794
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23794] http package should support HTTP/2

2015-04-03 Thread Cory Benfield

Cory Benfield added the comment:

 figure out some kind of adapter interface in order to facilitate swapping 
 between 1.1 and 2 (This can start with a clean HTTP/1.1 interface)

I've been thinking about this a lot with hyper, and I'm about to start work on 
it (having just finished an alpha implementation of HTTP/1.1). My current plan 
is to try out a proxy object pattern, but you should feel free to use the hyper 
code as a leaping-off point for some experiments into how to do the swapping.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23794
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23794] http package should support HTTP/2

2015-03-28 Thread Cory Benfield

Cory Benfield added the comment:

I'm happy to talk about bringing hyper's HTTP/2 layer into http.client. It's 
worth noting that at this point I have no current plans to build a server into 
hyper, though if there was interest in using hyper as a baseline then I could 
take a swing at it.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23794
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23576] HTTPS reads can block when content length not available and timeout set.

2015-03-03 Thread Cory Benfield

New submission from Cory Benfield:

Initially reported on the requests bug list at 
https://github.com/kennethreitz/requests/issues/2467

In cases when a remote web server sends a non-chunked response that does not 
have a content length, it is possible to get http.client to hang on a read. To 
hit it requires a specific set of circumstances:

- Python 3 (this bug is not seen on Python 2 in my testing)
- HTTPS (using plaintext HTTP resolves the problem)
- A socket timeout must be set (leaving it unset, i.e. leaving the socket in 
blocking mode, resolves this problem)
- The server must not send a content-length or set Transfer-Coding: chunked.
- The reads must not be an even divisor of the content's length.
- The timeout must be longer than the time the server is willing to keep the 
connection open (otherwise an exception is thrown).

The following code can be used as a sample to demonstrate the bug:

import http.client
import time

def test(connection_class=http.client.HTTPSConnection, timeout=10, read_size=7):
start = time.time()
c = connection_class('sleepy-reaches-6892.herokuapp.com')
c.connect()
if timeout is not None:
c.sock.settimeout(timeout)
c.request('GET', '/test')
r = c.getresponse()
while True:
data = r.read(read_size)
if not data:
break
print('Finished in {}'.format(time.time() - start))


Below are the results from several different runs:

test(): Finished in 4.8294830322265625
test(connection_class=http.client.HTTPConnection): Finished in 
0.3060309886932373
test(timeout=None): Finished in 0.6070599555969238
test(read_size=2): Finished in 0.6600658893585205

As you can see, only this particular set of features causes the bug. Note that 
if you change the URL to one that does return a Content-Length (e.g. 
http2bin.org/get), this bug also goes away.

This is a really weird edge case bug. There's some extensive investigation over 
at the requests bug report, but I've not been able to convincingly point at the 
problem yet.

--
components: Library (Lib)
messages: 237151
nosy: Lukasa
priority: normal
severity: normal
status: open
title: HTTPS reads can block when content length not available and timeout set.
versions: Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23476] SSL cert verify fail for www.verisign.com

2015-03-01 Thread Cory Benfield

Cory Benfield added the comment:

My reading of the OpenSSL issue is that there are no negative side effects from 
turning this on.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23476
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23476] SSL cert verify fail for www.verisign.com

2015-02-24 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23476
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue23476] SSL cert verify fail for www.verisign.com

2015-02-24 Thread Cory Benfield

Cory Benfield added the comment:

The problem specifically is that OpenSSL only uses a *root* in the trust store 
as an anchor. That means any certificate that is signed by another certificate 
will not terminate the chain of trust. Browsers do better here, by trusting the 
entirety of the trust store, regardless of whether or not it's a root 
certificate.

Donald is correct: this is not really Python's fault, it's OpenSSL's.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23476
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue20188] ALPN support for TLS

2015-01-22 Thread Cory Benfield

Cory Benfield added the comment:

Updating to note that OpenSSL 1.0.2 has been released[0], which makes this 
feature supportable.

[0]: https://mta.openssl.org/pipermail/openssl-announce/2015-January/19.html

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20188
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue22638] ssl module: the SSLv3 protocol is vulnerable (POODLE attack)

2014-10-15 Thread Cory Benfield

Cory Benfield added the comment:

 I don't believe it's in OpenSSL though.

There's an outstanding OpenSSL patch: 
http://marc.info/?l=openssl-devm=141333049205629w=2

However, as Donald has pointed out, this is really only meaningful for servers 
and co-operating clients. It's not a useful panacea.

--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22638
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue20188] ALPN support for TLS

2014-09-06 Thread Cory Benfield

Cory Benfield added the comment:

Updating to mention a concern with ALPN implementation. HTTP/2 requires that a 
cipher with AEAD be negotiated. However, it also allows for offering a wider 
range of cipher suites: if an AEAD cipher is not present, this will allow 
fallback to HTTP/1.1.

There's some interplay between ALPN and cipher selection here. We'll want to 
ensure that either ALPN negotiation can inform cipher selection or vice-versa, 
or writing a Python HTTP/2 server will get tricky fast.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20188
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue10721] Remove HTTP 0.9 server support

2014-06-26 Thread Cory Benfield

Cory Benfield added the comment:

To answer your question, Mark, RFC 7230 has removed the expectation that 
HTTP/1.1 servers will be able to support HTTP/0.9 requests.

--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue10721
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue20188] ALPN support for TLS

2014-04-28 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20188
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue21308] PEP 466: backport ssl changes

2014-04-19 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21308
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue16037] httplib: header parsing is unlimited

2014-03-12 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16037
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue16037] httplib: header parsing is unlimited

2014-03-12 Thread Cory Benfield

Cory Benfield added the comment:

I presume Barry's disinclination to merge this to 2.6 with a new exception 
applies equally to 2.7, which is why this hasn't been merged to 2.7 yet?

I'm happy to review an updated 2.7 patch that raises an HTTPException if that's 
what we need to keep this moving.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16037
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17849] Missing size argument in readline() method for httplib's class LineAndFileWrapper

2014-01-17 Thread Cory Benfield

Cory Benfield added the comment:

I've been messing around trying to write a test for this, and it looks like 
genuinely the only place this can be hit is in _tunnel(). None of the other 
readline() calls can be hit with the LineAndFileWrapper() wrapping the file 
descriptor.

--
nosy: +Lukasa

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17849
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue17849] Missing size argument in readline() method for httplib's class LineAndFileWrapper

2014-01-17 Thread Cory Benfield

Cory Benfield added the comment:

Alright, here's a patch.

I'm not ecstatic about this test: it's utterly contrived, but it also seems to 
be the least bad way to reproduce this bug. Let me know if you strongly object 
and I'll take another swing at it (it'll probably involve extending the 
FakeSocket quite extensively).

--
keywords: +patch
Added file: http://bugs.python.org/file33512/readline.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17849
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2014-01-11 Thread Cory Benfield

Cory Benfield added the comment:

Is there anything I can do to help move this forward? I appreciate you're all 
busy so I'm happy for this to take as long as it takes, I just wanted to make 
sure it's not blocked behind me.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-18 Thread Cory Benfield

Cory Benfield added the comment:

Ok, here's a patch for 2.7 as well.

I decided to allow the empty header names in rfc822.py as well, if only because 
I wanted the changed parsing code to match. If anyone thinks that's an 
excessive change, I'll happily remove it.

--
Added file: http://bugs.python.org/file33198/hdrs_27.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

Changes by Cory Benfield c...@lukasa.co.uk:


--
components: Library (Lib)
nosy: Lukasa
priority: normal
severity: normal
status: open
title: httplib infinite read on invalid header
type: behavior
versions: Python 2.7, Python 3.1, Python 3.2, Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

New submission from Cory Benfield:

Initially spotted on Requests GitHub bugtracker: 
https://github.com/kennethreitz/requests/issues/1804

On receiving an HTTP response with an invalid header, httplib stops parsing the 
headers and attempts to receive the rest of the message as body content. 
Normally that would be fine, but problems occur if later on in the headers 
Transfer-Encoding: chunked is declared. This leads to a hang while reading 
the body content until the remote end forcibly closes the connection.

This bug certainly affects versions 2.7 through 3.3.

To reproduce (note that we need to request gzip to get the server to send the 
bad header):

import http.client
h = http.client.HTTPConnection('www.sainsburysbank.co.uk')
h.request('GET', '/', headers={'Accept-Encoding': 'gzip'})
r = h.getresponse()
hdrs = r.getheaders()
body = r.read()  # Hang here.

cURL configured equivalently doesn't exhibit this problem, that is the 
following works fine:

curl --compressed http://www.sainsburysbank.co.uk/


It's not clear to me that this behaviour is wrong. The server is definitely 
violating RFC 2616 which expressly forbids empty header names. I'm open to 
consultation about what the correct fix should be here (which may be nothing at 
all).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

Cory Benfield added the comment:

The easiest way to 'fix' the DoS problem is to throw an exception if an invalid 
header is parsed. That's a backwards-compatibility problem though: things that 
previously 'worked' now won't. That presumably limits the ability to back-apply 
this fix to 2.7.7.

An alternative option is to speculatively attempt to parse the next line for 
headers or the end of the header block. I'm not sure this is a great idea: at 
this stage all we know is that the header block is malformed, so it's not clear 
that 'doing our best' is a good idea either, especially since that attitude got 
us here to begin with.

The best 'middle of the road' option is to abort message parsing at this stage 
without throwing an exception. This leads to truncated headers and no body, 
where previously we'd have got truncated headers and a body that potentially 
included the missing headers. We could also potentially add a warning about the 
problem.

Are there any preferences for a fix here, or a better solution than the above 
(none of which I'm wild about)?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

Cory Benfield added the comment:

Maybe. If we do it we have to apply that timeout to all the socket actions on 
that HTTP connection. This would have the effect of changing the default value 
of the timeout parameter on the HTTPConnection object from 
socket._GLOBAL_DEFAULT_TIMEOUT to whatever value was chosen. We could do this 
for reads only, and avoid applying the timeout to connect() calls, but that's 
kind of weird.

We hit the same problem though: by default, HTTPConnections block indefinitely 
on all socket calls: we'd be changing that default to some finite timeout 
instead. Does that sound like a good way to go?

As for curl's error recovery strategy, I'm pretty sure it just keeps parsing 
the header block. That can definitely be done here. We do have an error 
reporting mechanism as well (sort of): we set the HTTPMessage.status field to 
some error string. We could do that, and continue to parse the header block: 
that's probably the least destructive way to fix this.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

Cory Benfield added the comment:

An update: in Python 2.7 through 3.3, fixing this should only affect 
http.client/httplib, because they do most of their header parsing themselves. 
Fixing this in later versions of Python is more interesting, as http.client got 
rewritten to use email.parser (which uses email.feedparser). This means any 
change to fix this problem in HTTP will also affect anything else that uses 
this module. Not sure how problematic that is yet.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

Cory Benfield added the comment:

Actually, that might be OK. I don't know the email package at all, but I 
suspect being able to handle empty header keys (by ignoring them) is a 
reasonable thing to do in the email case as well. Thoughts?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19996] httplib infinite read on invalid header

2013-12-16 Thread Cory Benfield

Cory Benfield added the comment:

Alright, here's a patch for the current tip. I'll need to prepare a different 
patch for earlier versions of Python, which will take me a little while longer 
to do (maybe not today). I've also signed a contributor agreement, but it 
doesn't look like that's propagated here yet.

--
keywords: +patch
Added file: http://bugs.python.org/file33169/hdrs.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19996
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com