Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEP 476: verify HTTPS certificates by default #66607

Closed
ncoghlan opened this issue Sep 15, 2014 · 18 comments
Closed

PEP 476: verify HTTPS certificates by default #66607

ncoghlan opened this issue Sep 15, 2014 · 18 comments
Assignees
Labels
deferred-blocker type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

BPO 22417
Nosy @jcea, @ncoghlan, @orsenthil, @larryhastings, @tiran, @benjaminp, @jwilk, @alex, @koobs, @dstufft, @raulcd, @cl0p3z
Dependencies
  • bpo-22366: urllib.request.urlopen should take a "context" (SSLContext) argument
  • Files
  • pep476_minimal_implementation.diff: Minimal changes to verify HTTPS by default
  • issue22417.diff
  • issue22417.diff
  • issue22417.diff
  • issue22417.diff
  • issue22417.diff
  • issue22417.diff
  • issue22417.diff
  • koobs-freebsd9.python3x-build2357.log
  • issue22417.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/alex'
    closed_at = <Date 2014-11-24.04:58:27.902>
    created_at = <Date 2014-09-15.12:34:29.853>
    labels = ['deferred-blocker', 'type-feature']
    title = 'PEP 476: verify HTTPS certificates by default'
    updated_at = <Date 2017-01-04.09:42:57.931>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2017-01-04.09:42:57.931>
    actor = 'christian.heimes'
    assignee = 'alex'
    closed = True
    closed_date = <Date 2014-11-24.04:58:27.902>
    closer = 'benjamin.peterson'
    components = []
    creation = <Date 2014-09-15.12:34:29.853>
    creator = 'ncoghlan'
    dependencies = ['22366']
    files = ['36624', '36901', '37077', '37081', '37094', '37113', '37122', '37123', '37192', '37261']
    hgrepos = []
    issue_num = 22417
    keywords = ['patch', 'needs review']
    message_count = 18.0
    messages = ['226912', '227093', '229255', '230288', '230317', '230428', '230512', '230541', '230542', '230548', '230554', '230556', '230568', '231140', '231583', '231592', '284604', '284623']
    nosy_count = 14.0
    nosy_names = ['jcea', 'ncoghlan', 'orsenthil', 'larry', 'christian.heimes', 'benjamin.peterson', 'jwilk', 'Arfrever', 'alex', 'python-dev', 'koobs', 'dstufft', 'raulcd', 'clopez']
    pr_nums = []
    priority = 'deferred blocker'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22417'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @ncoghlan
    Copy link
    Contributor Author

    Attached minimal patch updates http.client.HTTPSConnection to validate certs by default and adjusts test.test_httplib accordingly.

    It doesn't currently include any docs changes, or changes to urllib.

    The process wide "revert to the old behaviour" hook is to monkeypatch the ssl module:

        ssl._create_default_https_context = ssl._create_unverified_context

    To monkeypatch the stdlib to validate *everything* (this one isn't new, just noting it for the record):

        ssl._create_stdlib_context = ssl.create_default_context

    @ncoghlan ncoghlan added the type-feature A feature request or enhancement label Sep 15, 2014
    @ncoghlan
    Copy link
    Contributor Author

    Currently marking as a deferred blocker, as Alex wasn't sure he'd be able to get PEP-476 fully updated in time for 3.4.2rc1, and was willing to accept waiting for 2.7.9 and 3.4.3 rather than delaying 3.4.2 any further.

    However, that was before Senthil accepted the patch in 22366 for 3.5, which means we're at "feature complete" for the proposed changes.

    There's still the bpo-22366 backport patch, PEP update, docs updates and What's New updates to go, so assigning to Alex to decide if he wants to work with Larry to get this ready to go for 3.4.2 (noting that the PEP still needs the final tick of approval from Guido after being updated to reflect the proposed implementation).

    Otherwise we can get it ready for 2.7.9 with the other SSL changes, and it will appear in the 3.4.3 maintenance release, rather than 3.4.2.

    (Note that I'm busy most of this weekend, so +1 from me in advance if you decide to go ahead with getting it into 3.4.2)

    @alex
    Copy link
    Member

    alex commented Oct 13, 2014

    Patch with the implementation, and initial work on documentation. Needs review please, I suspect we need more docs in more places. Feedback please!

    @alex
    Copy link
    Member

    alex commented Oct 30, 2014

    Patch now makes more precise assertions about the type of error that's occurring.

    @alex
    Copy link
    Member

    alex commented Oct 31, 2014

    Updates to teh docs based on teh feedback from Antoine.

    @alex
    Copy link
    Member

    alex commented Nov 1, 2014

    New version of the patch based on feedback from benjamin, should make it easier to do the 3.4 branch stuff.

    @alex
    Copy link
    Member

    alex commented Nov 2, 2014

    New patch uses self-signed.pythontest.net, instead of svn.python.org. svn.python.org is signed by CACert, which is in the root on some machines.

    @benjaminp
    Copy link
    Contributor

    % ./python Lib/test/regrtest.py -v test_urllib2_localnet
    == CPython 3.4.2+ (3.4:7be6ef737aaf+, Nov 3 2014, 10:03:11) [GCC 4.8.3]
    == Linux-3.16.5-gentoo-x86_64-Intel-R-_Core-TM-i7-2860QM_CPU@_2.50GHz-with-gentoo-2.2 little-endian
    == hash algorithm: siphash24 64bit
    == /home/benjamin/dev/python/3.4/build/test_python_28724
    Testing with flags: sys.flags(debug=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=0, no_site=0, ignore_environment=0, verbose=0, bytes_warning=0, quiet=0, hash_randomization=1, isolated=0)
    [1/1] test_urllib2_localnet
    test_basic_auth_httperror (test.test_urllib2_localnet.BasicAuthTests) ... ok
    test_basic_auth_success (test.test_urllib2_localnet.BasicAuthTests) ... ok
    test_proxy_qop_auth_int_works_or_throws_urlerror (test.test_urllib2_localnet.ProxyAuthTests) ... ok
    test_proxy_qop_auth_works (test.test_urllib2_localnet.ProxyAuthTests) ... ok
    test_proxy_with_bad_password_raises_httperror (test.test_urllib2_localnet.ProxyAuthTests) ... ok
    test_proxy_with_no_password_raises_httperror (test.test_urllib2_localnet.ProxyAuthTests) ... ok
    test_200 (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_200_with_parameters (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_404 (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_bad_address (test.test_urllib2_localnet.TestUrlopen) ... skipped "Use of the 'network' resource not enabled"
    test_basic (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_chunked (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_geturl (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_https (test.test_urllib2_localnet.TestUrlopen) ... Got an error:
    [SSL: TLSV1_ALERT_UNKNOWN_CA] tlsv1 alert unknown ca (_ssl.c:600)
    stopping HTTPS server
    joining HTTPS thread
    ERROR
    test_https_sni (test.test_urllib2_localnet.TestUrlopen) ... Got an error:
    [SSL: TLSV1_ALERT_UNKNOWN_CA] tlsv1 alert unknown ca (_ssl.c:600)
    stopping HTTPS server
    joining HTTPS thread
    ERROR
    test_https_with_cadefault (test.test_urllib2_localnet.TestUrlopen) ... stopping HTTPS server
    Got an error:
    [SSL: TLSV1_ALERT_UNKNOWN_CA] tlsv1 alert unknown ca (_ssl.c:600)
    joining HTTPS thread
    ok
    test_https_with_cafile (test.test_urllib2_localnet.TestUrlopen) ... Got an error:
    [SSL: TLSV1_ALERT_UNKNOWN_CA] tlsv1 alert unknown ca (_ssl.c:600)
    stopping HTTPS server
    joining HTTPS thread
    stopping HTTPS server
    joining HTTPS thread
    ok
    test_info (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_iteration (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_line_iteration (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_redirection (test.test_urllib2_localnet.TestUrlopen) ... ok
    test_sending_headers (test.test_urllib2_localnet.TestUrlopen) ... ok

    ======================================================================
    ERROR: test_https (test.test_urllib2_localnet.TestUrlopen)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 1182, in do_open
        h.request(req.get_method(), req.selector, req.data, headers)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1090, in request
        self._send_request(method, url, body, headers)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1128, in _send_request
        self.endheaders(body)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1086, in endheaders
        self._send_output(message_body)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 924, in _send_output
        self.send(msg)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 859, in send
        self.connect()
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1230, in connect
        server_hostname=sni_hostname)
      File "/home/benjamin/dev/python/3.4/Lib/ssl.py", line 364, in wrap_socket
        _context=self)
      File "/home/benjamin/dev/python/3.4/Lib/ssl.py", line 584, in __init__
        self.do_handshake()
      File "/home/benjamin/dev/python/3.4/Lib/ssl.py", line 811, in do_handshake
        self._sslobj.do_handshake()
    ssl.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:600)
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/home/benjamin/dev/python/3.4/Lib/test/test_urllib2_localnet.py", line 548, in test_https
        data = self.urlopen("https://localhost:%s/bizarre" % handler.port)
      File "/home/benjamin/dev/python/3.4/Lib/test/test_urllib2_localnet.py", line 455, in urlopen
        f = urllib.request.urlopen(url, data, **kwargs)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 161, in urlopen
        return opener.open(url, data, timeout)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 463, in open
        response = self._open(req, data)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 481, in _open
        '_open', req)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 441, in _call_chain
        result = func(*args)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 1225, in https_open
        context=self._context, check_hostname=self._check_hostname)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 1184, in do_open
        raise URLError(err)
    urllib.error.URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:600)>

    ======================================================================
    ERROR: test_https_sni (test.test_urllib2_localnet.TestUrlopen)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 1182, in do_open
        h.request(req.get_method(), req.selector, req.data, headers)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1090, in request
        self._send_request(method, url, body, headers)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1128, in _send_request
        self.endheaders(body)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1086, in endheaders
        self._send_output(message_body)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 924, in _send_output
        self.send(msg)
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 859, in send
        self.connect()
      File "/home/benjamin/dev/python/3.4/Lib/http/client.py", line 1230, in connect
        server_hostname=sni_hostname)
      File "/home/benjamin/dev/python/3.4/Lib/ssl.py", line 364, in wrap_socket
        _context=self)
      File "/home/benjamin/dev/python/3.4/Lib/ssl.py", line 584, in __init__
        self.do_handshake()
      File "/home/benjamin/dev/python/3.4/Lib/ssl.py", line 811, in do_handshake
        self._sslobj.do_handshake()
    ssl.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:600)
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/home/benjamin/dev/python/3.4/Lib/test/test_urllib2_localnet.py", line 587, in test_https_sni
        self.urlopen("https://localhost:%s" % handler.port)
      File "/home/benjamin/dev/python/3.4/Lib/test/test_urllib2_localnet.py", line 455, in urlopen
        f = urllib.request.urlopen(url, data, **kwargs)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 161, in urlopen
        return opener.open(url, data, timeout)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 463, in open
        response = self._open(req, data)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 481, in _open
        '_open', req)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 441, in _call_chain
        result = func(*args)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 1225, in https_open
        context=self._context, check_hostname=self._check_hostname)
      File "/home/benjamin/dev/python/3.4/Lib/urllib/request.py", line 1184, in do_open
        raise URLError(err)
    urllib.error.URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:600)>

    Ran 22 tests in 3.087s

    @alex
    Copy link
    Member

    alex commented Nov 3, 2014

    Latest patch fixes the urllib2_localnet tests.

    @alex
    Copy link
    Member

    alex commented Nov 3, 2014

    Fix for the failing test_ssl testes.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 3, 2014

    New changeset 2afe5413d7af by Benjamin Peterson in branch '3.4':
    PEP-476: enable HTTPS certificate verification by default (bpo-22417)
    https://hg.python.org/cpython/rev/2afe5413d7af

    New changeset 731375f83406 by Benjamin Peterson in branch 'default':
    merge 3.4 (bpo-22417)
    https://hg.python.org/cpython/rev/731375f83406

    @benjaminp
    Copy link
    Contributor

    Okay, 3.4/3.5 have been dealt with. I had to hack up test_logging a bit. (bpo-22788 would make that better). 2.7 now needs a backport.

    @benjaminp
    Copy link
    Contributor

    Somehow the Windows bots are failing to verify python.org http://buildbot.python.org/all/builders/x86%20XP-4%203.x/builds/11179/steps/test/logs/stdio

    @koobs
    Copy link

    koobs commented Nov 13, 2014

    Builds failing on koobs-freebsd9 buildbot for:

    3.x: since revision b2c17681404f80edae2ee4846db701104d942cc4
    3.4: since revision 246c9570a75798a4757001620cf92cc8d2eba684

    Attaching both initial build failure test logs.

    @alex
    Copy link
    Member

    alex commented Nov 23, 2014

    Attached patch backports this to 2.7.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 24, 2014

    New changeset fb83916c3ea1 by Benjamin Peterson in branch '2.7':
    PEP-476: verify certificates by default (bpo-22417)
    https://hg.python.org/cpython/rev/fb83916c3ea1

    @cl0p3z
    Copy link
    Mannequin

    cl0p3z mannequin commented Jan 4, 2017

    The python 2.7 documentation for urrlib still has a big warning notice at the top saying:

    """
    Warning

    When opening HTTPS URLs, it does not attempt to validate the server certificate. Use at your own risk!
    """
    ^^ https://docs.python.org/2/library/urllib.html

    I believe this is incorrect since this patch was backported to the 2.7 branch. I checked it, and it verifies SSL certs by default.

    I guess the documentation for urllib should be updated to remove that warning?

    @tiran
    Copy link
    Member

    tiran commented Jan 4, 2017

    Carlos, you are correct. Please create a new issue and make it a documentation issue for 2.7. Thanks!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    deferred-blocker type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants