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

urllib.request and http.client should allow certificate checking #53249

Closed
debatem1 mannequin opened this issue Jun 16, 2010 · 13 comments
Closed

urllib.request and http.client should allow certificate checking #53249

debatem1 mannequin opened this issue Jun 16, 2010 · 13 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@debatem1
Copy link
Mannequin

debatem1 mannequin commented Jun 16, 2010

BPO 9003
Nosy @orsenthil, @pitrou, @giampaolo
Dependencies
  • bpo-1589: New SSL module doesn't seem to verify hostname against commonName in certificate
  • Files
  • httpcli.patch
  • httpcli2.patch
  • httpcli+urllib.patch
  • httpcli+urllib2.patch
  • httpcli+urllib3.patch
  • 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/orsenthil'
    closed_at = <Date 2010-10-13.10:40:17.083>
    created_at = <Date 2010-06-16.01:04:36.433>
    labels = ['type-feature', 'library']
    title = 'urllib.request and http.client should allow certificate checking'
    updated_at = <Date 2014-11-23.17:46:30.105>
    user = 'https://bugs.python.org/debatem1'

    bugs.python.org fields:

    activity = <Date 2014-11-23.17:46:30.105>
    actor = 'python-dev'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2010-10-13.10:40:17.083>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2010-06-16.01:04:36.433>
    creator = 'debatem1'
    dependencies = ['1589']
    files = ['19162', '19178', '19185', '19186', '19187']
    hgrepos = []
    issue_num = 9003
    keywords = ['patch']
    message_count = 13.0
    messages = ['107900', '118081', '118178', '118206', '118276', '118291', '118292', '118379', '118381', '118383', '118391', '118510', '231573']
    nosy_count = 15.0
    nosy_names = ['zooko', 'janssen', 'orsenthil', 'pitrou', 'giampaolo.rodola', 'vila', 'heikki', 'ahasenack', 'kiilerix', 'debatem1', 'jsamuel', 'devin', 'asdfasdfasdfasdfasdfasdfasdf', 'Ryan.Tucker', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue9003'
    versions = ['Python 3.2']

    @debatem1
    Copy link
    Mannequin Author

    debatem1 mannequin commented Jun 16, 2010

    urllib currently blindly accepts bad certificates when passed an https address. This behavior, clearly not desirable for many users, is also not documented. I propose one of two changes:

    1. add mechanisms for enforcing correct behavior to urllib, or
    2. change the documentation for that module to include something akin to the following warning:

    "Warning: urllib does not perform certificate checks if passed an HTTPS url! This permits remote machines to masquerade as your intended destination."

    @debatem1 debatem1 mannequin added the stdlib Python modules in the Lib dir label Jun 16, 2010
    @orsenthil orsenthil self-assigned this Jun 16, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Oct 6, 2010

    A big warning is now present (*) in the urllib and httplib documentation pages. Also, once bpo-1589 is fixed, we can go forward and make {http.client,urllib.request} check hostname and cert if the user gives the location of a bunch of CA certs.

    (*) see e.g. http://docs.python.org/dev/library/urllib.request.html

    @pitrou pitrou changed the title urllib about https behavior urllib.request and http.client should allow certificate checking Oct 6, 2010
    @pitrou pitrou added the type-feature A feature request or enhancement label Oct 6, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Oct 8, 2010

    Here is the API addition I would suggest for the http.client module:

    Add two new keyword arguments context and check_hostname to HTTPSConnection; context would allow to pass a SSLContext instance for certificate checking and other options (default None, meaning no checking); check_hostname would specify whether to check the hostname against the URL (default to check only if context is present and context.verify_mode != CERT_NONE).

    Here is the API addition I would suggest for the urllib.request module:

    • Add constructor arguments context and check_hostname to HTTPSHandler. They will be passed to the underlying HTTPSConnection.

    • Add ssl_ca_file and ssl_ca_path arguments to the high-level function urlopen(); if at least one of them is present, a custom opener with a custom HTTPSHandler will be created, mandating the checking of server certificates

    @pitrou
    Copy link
    Member

    pitrou commented Oct 8, 2010

    Here is a preliminary patch for http.client. I think it would be good to have local tests using a custom HTTPS server, too.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 9, 2010

    Here is another patch for http.client containing more tests, including with a mismatching cert. Comments welcome.

    @debatem1
    Copy link
    Mannequin Author

    debatem1 mannequin commented Oct 9, 2010

    Any chance on folding the HTTPSServer class into http.server?

    Geremy Condra

    @pitrou
    Copy link
    Member

    pitrou commented Oct 9, 2010

    Any chance on folding the HTTPSServer class into http.server?

    Its API and implementation would first have to be cleaned up.
    I'd prefer if it were the subject of a separate issue.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 11, 2010

    Here is a patch which also adds 'cafile' and 'capath' keyword arguments to urlopen().

    @pitrou
    Copy link
    Member

    pitrou commented Oct 11, 2010

    Here is a new patch with doc updates for urllib.request.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 11, 2010

    This patch should fix the test hanging issues witnessed on some machines.

    @orsenthil
    Copy link
    Member

    orsenthil commented Oct 11, 2010

    Yes, it does solve the problem of httplib and urllib2_localnet tests which
    were hanging with the earlier patch on certain machines..

    @pitrou
    Copy link
    Member

    pitrou commented Oct 13, 2010

    Patch committed in r85408. I believe this fixes, at last, the whole issue people were complaining about.

    @pitrou pitrou closed this as completed Oct 13, 2010
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2014

    New changeset 1882157b298a by Benjamin Peterson in branch '2.7':
    allow passing cert/ssl information to urllib2.urlopen and httplib.HTTPSConnection
    https://hg.python.org/cpython/rev/1882157b298a

    @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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants