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

SSL support for asyncore #54293

Closed
pitrou opened this issue Oct 13, 2010 · 6 comments
Closed

SSL support for asyncore #54293

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

Comments

@pitrou
Copy link
Member

pitrou commented Oct 13, 2010

BPO 10084
Nosy @pitrou, @giampaolo, @djc
Files
  • asyncore_ssl_v1.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 2014-06-14.15:07:54.309>
    created_at = <Date 2010-10-13.13:09:08.921>
    labels = ['type-feature', 'library']
    title = 'SSL support for asyncore'
    updated_at = <Date 2014-06-14.15:07:54.308>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2014-06-14.15:07:54.308>
    actor = 'giampaolo.rodola'
    assignee = 'giampaolo.rodola'
    closed = True
    closed_date = <Date 2014-06-14.15:07:54.309>
    closer = 'giampaolo.rodola'
    components = ['Library (Lib)']
    creation = <Date 2010-10-13.13:09:08.921>
    creator = 'pitrou'
    dependencies = []
    files = ['20752']
    hgrepos = []
    issue_num = 10084
    keywords = ['patch']
    message_count = 6.0
    messages = ['118519', '118520', '118552', '128459', '128473', '220561']
    nosy_count = 3.0
    nosy_names = ['pitrou', 'giampaolo.rodola', 'djc']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue10084'
    versions = ['Python 3.3']

    @pitrou
    Copy link
    Member Author

    pitrou commented Oct 13, 2010

    It might be useful to make public the SSL support for asyncore which is currently implemented in various tests.

    @pitrou pitrou added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 13, 2010
    @pitrou
    Copy link
    Member Author

    pitrou commented Oct 13, 2010

    (I'm posting this issue after having read this message:
    http://mail.python.org/pipermail/python-list/2010-October/1257689.html
    where the poster is clearly confused about SSL support for asyncore)

    @giampaolo
    Copy link
    Contributor

    Problem with SSL dispatcher subclasses used in tests is that they are all similar to pyftpdlib's SSLConnection class ( http://code.google.com/p/pyftpdlib/source/browse/trunk/pyftpdlib/contrib/handlers.py?spec=svn743&r=729#73 ) and I'm not sure it's API is suitable for a general use case.
    It fits well for pyftpdlib, servers in general and stdlib tests but I'm not sure about other uses cases.
    In details I'm thinking about clients, secure connections reverted back to clear-text (e.g FTP might need this) and recent issues about certificates validation.
    Before writing anything we should agree on an API and make sure it is able to cover all use cases.

    @giampaolo
    Copy link
    Contributor

    Initial draft of a patch including tests and a new ssl_dispatcher subclass.
    asynchat needs to be changed as well, probably by using a mixin class.

    @pitrou
    Copy link
    Member Author

    pitrou commented Feb 12, 2011

    First comments:

    • secure_connection() should be named ssl_something() like other
      methods. ssl_start() perhaps?

    • in ssl_shutdown():
      + elif err.args[0] == ssl.SSL_ERROR_SSL:
      + pass

    SSL_ERROR_SSL doesn't exist. Perhaps you mean ssl.SSL_ERROR_EOF?

    - in send(), you should handle SSL_ERROR_WANT_READ and
    SSL_ERROR_WANT_WRITE as in recv(). Also:
    +                if err.args[0] in (ssl.SSL_ERROR_EOF, ssl.SSL_ERROR_ZERO_RETURN):
    +                    return 0

    lacks a self.handle_close()?

    • in recv(), you have "return ''" where it should be "return b''"

    • in test_ssl_established(), I think it would be nice if you used e.g.
      getpeercert() to check that we really are in SSL mode. Also, you could
      make certificate checking mandatory using e.g.:

        ssl_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
        ssl_context.verify_mode = ssl.CERT_REQUIRED
        cert_path = os.path.join(os.path.dirname(__file__), "keycert.pem")
        ssl_context.load_cert_chain(cert_path)
        ssl_context.load_verify_locations(cert_path)
    • in addition to test_handle_read() and test_handle_write(), there
      should be a test where a server and a client really send data to each
      other, and receive at all

    (also, I'm not sure why these tests can't be shared with non-SSL test
    classes)

    • test_create_socket() and test_bind() don't seem to test anything
      SSL-related

    @giampaolo
    Copy link
    Contributor

    asyncore module has been deprecated as per https://docs.python.org/3/library/asyncore.html:

    <<This module exists for backwards compatibility only. For new code we recommend using asyncio.>>

    Closing this out as won't fix.

    @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