Navigation Menu

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

ftplib should support SSL contexts #53052

Closed
pitrou opened this issue May 24, 2010 · 8 comments
Closed

ftplib should support SSL contexts #53052

pitrou opened this issue May 24, 2010 · 8 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented May 24, 2010

BPO 8806
Nosy @pitrou, @giampaolo
Files
  • ftplib.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/giampaolo'
    closed_at = <Date 2010-05-26.18:07:22.369>
    created_at = <Date 2010-05-24.16:07:41.272>
    labels = ['type-feature', 'library']
    title = 'ftplib should support SSL contexts'
    updated_at = <Date 2010-05-26.18:07:22.344>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2010-05-26.18:07:22.344>
    actor = 'giampaolo.rodola'
    assignee = 'giampaolo.rodola'
    closed = True
    closed_date = <Date 2010-05-26.18:07:22.369>
    closer = 'giampaolo.rodola'
    components = ['Library (Lib)']
    creation = <Date 2010-05-24.16:07:41.272>
    creator = 'pitrou'
    dependencies = []
    files = ['17452']
    hgrepos = []
    issue_num = 8806
    keywords = ['patch']
    message_count = 8.0
    messages = ['106366', '106379', '106380', '106384', '106390', '106541', '106542', '106562']
    nosy_count = 3.0
    nosy_names = ['janssen', 'pitrou', 'giampaolo.rodola']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue8806'
    versions = ['Python 3.2']

    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2010

    3.2 introduces SSL contexts, which allow bundling SSL configuration options, certificates and private keys into a single (potentially long-lived) structure.
    http://docs.python.org/dev/py3k/library/ssl.html#ssl.SSLContext

    The FTP_TLS constructor should allow passing an SSL context object instead of a key/cert pair.

    @pitrou pitrou added the type-feature A feature request or enhancement label May 24, 2010
    @giampaolo
    Copy link
    Contributor

    I'm assigning this one to me.
    Btw, before writing anything I think it would be better to decide what to do first, possibly by adopting the same approach everywhere (smtplib, imaplib, poplib, urllib and http.client).

    My proposal:

    • the user should still be able to use keyfile and certfile if desired, they're quicker to use and backward compatibility must be maintained.

    • SSL context can be provided by passing a new "context" argument to the constructor.

    • if context arg is passed FTP_TLS.ssl_version should be ignored and SSLContext.protocol used instead

    • keyfile/certfile and context arguments should be mutually exclusive (ValueError)

    Makes sense?

    @giampaolo giampaolo self-assigned this May 24, 2010
    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2010

    My proposal:

    • the user should still be able to use keyfile and certfile if desired, they're quicker to use and backward compatibility must be maintained.

    • SSL context can be provided by passing a new "context" argument to the constructor.

    • if context arg is passed FTP_TLS.ssl_version should be ignored and SSLContext.protocol used instead

    • keyfile/certfile and context arguments should be mutually exclusive (ValueError)

    Makes sense?

    Entirely sense to me :)

    @giampaolo
    Copy link
    Contributor

    Patch in attachment.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2010

    Patch in attachment.

    You could add checks for:

        self.assertIs(self.client.sock.context, ctx)
    [...]
        self.assertIs(sock.context, ctx)

    (I know, the "context" attribute isn't documented, I'm going to fix
    this)

    @pitrou
    Copy link
    Member Author

    pitrou commented May 26, 2010

    (I know, the "context" attribute isn't documented, I'm going to fix
    this)

    Now documented at:
    http://docs.python.org/dev/py3k/library/ssl.html#ssl.SSLSocket.context

    @giampaolo
    Copy link
    Contributor

    If you're fine with the current patch I can go on and commit it (including the context attribute test).

    @giampaolo
    Copy link
    Contributor

    Committed in r81548.

    @giampaolo giampaolo added the stdlib Python modules in the Lib dir label May 26, 2010
    @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