python / cpython

The Python programming language
https://www.python.org
Other
62.17k stars 29.88k forks source link

adds SSL server socket support to socketmodule.c #33192

Closed e3c7e305-6bfc-4e76-9113-b3f0526bd848 closed 23 years ago

e3c7e305-6bfc-4e76-9113-b3f0526bd848 commented 23 years ago
BPO 401647
Nosy @gvanrossum, @akuchling
Files
  • None: None
  • 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: ```python assignee = 'https://github.com/gvanrossum' closed_at = created_at = labels = ['extension-modules'] title = 'adds SSL server socket support to socketmodule.c' updated_at = user = 'https://bugs.python.org/drewcsillag' ``` bugs.python.org fields: ```python activity = actor = 'gvanrossum' assignee = 'gvanrossum' closed = True closed_date = None closer = None components = ['Extension Modules'] creation = creator = 'drew_csillag' dependencies = [] files = ['2834'] hgrepos = [] issue_num = 401647 keywords = ['patch'] message_count = 16.0 messages = ['34427', '34428', '34429', '34430', '34431', '34432', '34433', '34434', '34435', '34436', '34437', '34438', '34439', '34440', '34441', '34442'] nosy_count = 6.0 nosy_names = ['gvanrossum', 'jhylton', 'akuchling', 'naris', 'drew_csillag', 'elie'] pr_nums = [] priority = 'normal' resolution = 'rejected' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue401647' versions = [] ```

    e3c7e305-6bfc-4e76-9113-b3f0526bd848 commented 23 years ago
    gvanrossum commented 23 years ago

    Drew, could you provide an example of how this is used? If I can't test it I can't add it. It doesn't have to be a test module (although a test module for all the SSL support is sorely needed) but I would like to see a little motivation for why this is useful. Also note that the SSL support in the socket module is controversial; there are some who believe that a different approach is needed, e.g. based on M2crypto.

    gvanrossum commented 23 years ago

    Still no reply. I'll reject the patch now, for lack of sufficient motivation.

    gvanrossum commented 23 years ago

    OK, Andrew, it's all yours.

    (But I disagree with your last point: client-only SSL support is still a lot better than no SSL support! :-)

    gvanrossum commented 23 years ago

    Rejected. Drew agrees after re-educating himself about M2Crypto.

    akuchling commented 23 years ago

    Guido, want me to re-open this patch and take it over? IMHO, if the SSL support is left in, then this patch should be added; no point in having only half-working support.

    akuchling commented 23 years ago

    newServerSSLObject() is a near-duplicate of newSSLobject(). Rather than just cut-and-paste the code into a new function, newSSLobject() should take a fourth argument, and perform either the client or server initialization.

    I've already modified the patch to do this. Before uploading the modified version, I'd like to test it, but can't figure out what it wants for the key and cert arguments. Drew, do you have a test program you used to test the code? Can you please e-mail it to me, or add it as a comment to this patch?

    akuchling commented 23 years ago

    Drew provided instructions for creating a test key and certificate, but I couldn't make them work. Given that:

    I'd suggest dropping SSL support from 2.1.

    Reassigning to GvR; I can't test this code if I can't even make a client connection work!

    03bde425-37ce-4291-88bd-d6cecc46a30e commented 23 years ago

    too late for 2.0

    e3c7e305-6bfc-4e76-9113-b3f0526bd848 commented 23 years ago

    Sorry for the no-reply... Anyhoo, the ssl stuff currently in the socket module only allows ssl on client connections (i.e. where you connect to somebody else) as opposed to server connections (i.e. where somebody connects to you).

    For example, you have a cheesy SSL socket client:

    from socket import *
    s = socket(AF_INET, SOCK_STREAM)
    s.connect(('',9999))
    ss = ssl(s,None,None)
    ss.write("foo!\n")

    The patch is required in order to be able to write the corresponding server, as such:

    from socket import *
    s = socket(AF_INET, SOCK_STREAM)
    s.bind(('',9999))
    s.listen(5)
    f,a = s.accept()
    ss = sslserv(f, "keyfilename", "certfilename")
    print ss.read(5)

    If you try to just use the ssl function on both sides and it doesn't work.

    e5636dcb-4eb5-4548-ac1e-779da027ecaf commented 23 years ago

    too late ? this patch solves world hunger and brings world peace!

    such a valuable patch, but i guess deadlines are deadlines :-(

    20cd5ea8-841c-4b0c-9e4b-0132832450c3 commented 23 years ago

    Logged In: YES user_id=106050

    So, what is the fate of the socket.ssl stuff? Is it safe to rely on SSL support of socket module or it will be dropped?

    Thanks, ilya

    gvanrossum commented 23 years ago

    Logged In: YES user_id=6380

    The existing socket.ssl() support will remain in existence, or a backwards compatibility solution will be provided.

    20cd5ea8-841c-4b0c-9e4b-0132832450c3 commented 23 years ago

    Logged In: YES user_id=106050

    That is great!

    Are you planning to commit the ssl-server-socket patch into the socket module to make the latter suitable for building SSL servers?

    Thanks, ilya

    akuchling commented 23 years ago

    Logged In: YES user_id=11375

    No, which is why this patch is marked as "Rejected". Use M2Crypto if you want to write SSL servers.

    gvanrossum commented 23 years ago

    Logged In: YES user_id=6380

    No, no, no! We won't be adding new functionality.