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

Documentation for ssl.wrap_socket's ssl_version parameter is odd #68560

Closed
ericvsmith opened this issue Jun 3, 2015 · 6 comments
Closed

Documentation for ssl.wrap_socket's ssl_version parameter is odd #68560

ericvsmith opened this issue Jun 3, 2015 · 6 comments
Labels
docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ericvsmith
Copy link
Member

BPO 24372
Nosy @ericvsmith, @tiran

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 = None
closed_at = <Date 2021-12-11.21:20:15.538>
created_at = <Date 2015-06-03.13:10:46.514>
labels = ['type-feature', 'library', 'docs']
title = "Documentation for ssl.wrap_socket's ssl_version parameter is odd"
updated_at = <Date 2021-12-11.21:20:15.537>
user = 'https://github.com/ericvsmith'

bugs.python.org fields:

activity = <Date 2021-12-11.21:20:15.537>
actor = 'eric.smith'
assignee = 'docs@python'
closed = True
closed_date = <Date 2021-12-11.21:20:15.538>
closer = 'eric.smith'
components = ['Documentation', 'Library (Lib)']
creation = <Date 2015-06-03.13:10:46.514>
creator = 'eric.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 24372
keywords = []
message_count = 6.0
messages = ['244742', '244745', '244747', '244748', '244750', '275046']
nosy_count = 3.0
nosy_names = ['eric.smith', 'christian.heimes', 'docs@python']
pr_nums = []
priority = 'low'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue24372'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

@ericvsmith
Copy link
Member Author

The documentation for ssl.wrap_socket()'s ssl_version parameter says "ssl_version={see docs}". But as far as I can tell, there's no reason not to document it as PROTOCOL_SSLv23, since that's what it actually is in the code.

My use case is that I'm producing a utility function for an RFC-5804 Manage Sieve client, which implements starttls(). I want to expose most of wrap_sockets()'s parameters, and pass them in to ssl.wrap_socket() itself. In order to do so, I need to specify the default values to match wrap_socket(). But ssl_version's default isn't documented.

Is the reason to allow us to change the default in a bug fix release? If that's the case, I suggest creating a sentinel value, like ssl.DEFAULT_PROTOCOL, and have that be the documented default. Then I could declare my parameter as defaulting to ssl.DEFAULT_PROTOCOL, pass it in to ssl.wrap_socket(), and have it use the "current" default.

If that's a desirable solution, I'll happily write a patch. If so, it would just apply to 3.6, but if we just want to document the actual value of ssl_version, then it would apply to the versions I've selected.

@ericvsmith ericvsmith added docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jun 3, 2015
@ericvsmith
Copy link
Member Author

It occurs to me that None would also be a fine default, and probably the smarter choice.

Then wrap_socket could say:
if ssl_version is None:
ssl_version = PROTOCOL_SSLv23

And we could change the default as needed, without being held to the actual value. And wrapper code, like mine, would declare a default of None and things would continue to work across versions.

@tiran
Copy link
Member

tiran commented Jun 3, 2015

I'd like to deprecate ssl.wrap_socket() in favor of SSLContext.wrap_socket(). Libraries should rather accept a context than expose the awkward interface of ssl.wrap_socket(). A context object is far more powerful and easier to use.

@tiran
Copy link
Member

tiran commented Jun 3, 2015

DISCLAIMER:

I'm currently planning and researching two SSL related PEPs. One PEP is about SSLContext.

I like to standardize a minimal interface for SSL context object, so libraries can deal with any abitrary SSL implementation. With just one small addition and an SSLContext ABC, every library could then support stdlib ssl, PyOpenSSL, GnuTLS, NSS, PyQT/PySide QSslSocket and even non-standard sockets like NSPR PRFileDesc and QTcpSocket.

My idea only works with a SSL context object. So I'm interested that people use the new feature rather then the old ssl.wrap_socket() function. :)

@ericvsmith
Copy link
Member Author

I have a requirement to support 2.7.5, so SSLContext is currently a problem for me.

I realize that 2.7 could at best get a documentation change.

@tiran
Copy link
Member

tiran commented Sep 8, 2016

OpenSSL 1.1.0 has deprecated all versions except TLS_METHOD and DTLS_METHOD. TLS_METHOD is the new name for auto-nego SSLv23_METHOD. Python 3.6 defaults to TLS_METHOD already. bpo-27876 will add a better way to configure accepted versions.

@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
docs Documentation in the Doc dir 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