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.

Sign up for GitHub

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

Jump to bottom

ftplib.FTP_TLS's default constructor does not work with TLSv1.1 or TLSv1.2 #67300

Closed
varde mannequin opened this issue Dec 24, 2014 · 9 comments
Closed

ftplib.FTP_TLS's default constructor does not work with TLSv1.1 or TLSv1.2 #67300

varde mannequin opened this issue Dec 24, 2014 · 9 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@varde
Copy link
Mannequin

varde mannequin commented Dec 24, 2014

BPO 23111
Nosy @pitrou, @giampaolo, @benjaminp

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 2015-01-04.16:20:32.664>
created_at = <Date 2014-12-24.21:45:07.593>
labels = ['type-bug', 'library']
title = "ftplib.FTP_TLS's default constructor does not work with TLSv1.1 or TLSv1.2"
updated_at = <Date 2015-01-04.16:20:32.662>
user = 'https://bugs.python.org/varde'

bugs.python.org fields:

activity = <Date 2015-01-04.16:20:32.662>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2015-01-04.16:20:32.664>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2014-12-24.21:45:07.593>
creator = 'varde'
dependencies = []
files = []
hgrepos = []
issue_num = 23111
keywords = []
message_count = 9.0
messages = ['233087', '233118', '233155', '233214', '233217', '233221', '233222', '233421', '233423']
nosy_count = 6.0
nosy_names = ['pitrou', 'giampaolo.rodola', 'benjamin.peterson', 'Arfrever', 'python-dev', 'varde']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23111'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']
@varde
Copy link
Mannequin Author

varde mannequin commented Dec 24, 2014

When trying to connect to a server which only supports TLS version 1.1 or 1.2, the following error is raised:
ssl.SSLError: [SSL: WRONG_VERSION_NUMBER] wrong version number (ssl.c:598)
For some reason, the SSL version is set to ssl.PROTOCOL_TLSv1 before initialisation and an SSL context is created in __init_, making any subsequent change to ssl_version useless.
The only way to establish a successful connection is to pass a custom SSL context to the constructor.
I think ssl_version should be settable at construction time before the context is created.
I'm not sure exposing ssl_version is useful either, the documentation mentions it but it has no use after initialisation.

The following lines should also be changed:
if self.ssl_version == ssl.PROTOCOL_TLSv1:
resp = self.voidcmd('AUTH TLS')

@varde varde mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Dec 24, 2014
@pitrou
Copy link
Member

pitrou commented Dec 26, 2014

The only way to establish a successful connection is to pass a custom SSL context to the constructor.

Why don't you do just that?

@varde
Copy link
Mannequin Author

varde mannequin commented Dec 28, 2014

Well, because the ssl_version parameter should have a purpose. If it doesn't, the least we could do is remove it from the docs.

@giampaolo
Copy link
Contributor

ssl_version is a class attribute so you can simply set that before instantiating FTP_TLS class:

>> import ftplib
>> ftplib.FTP_TLS.ssl_version = ...
>> client = ftplib.FTP_TLS(...)
>> ...

@varde
Copy link
Mannequin Author

varde mannequin commented Dec 30, 2014

I know that, but it seems pretty unusual. And I would never had guessed from the documentation, I had to read the source.
My point is that it should be easier to just connect to a TLSv1.2 server: the documentation should mention the fact that ssl_version is a class attribute or it should be set to something more compatible like ssl.PROTOCOL_SSLv23.
I'm not sure about the implications of the latter.
I'm not saying that this is a serious bug, but I'm used to Python providing us with something that works (more or less) out of the box.

@python-dev
Copy link
Mannequin

python-dev mannequin commented Dec 30, 2014

New changeset 414c450e8406 by Benjamin Peterson in branch '3.4':
make PROTOCOL_SSLv23 the default protocol version for ftplib (closes bpo-23111)
https://hg.python.org/cpython/rev/414c450e8406

New changeset 33603f7949c5 by Benjamin Peterson in branch 'default':
merge 3.4 (bpo-23111)
https://hg.python.org/cpython/rev/33603f7949c5

@python-dev python-dev mannequin closed this as completed Dec 30, 2014
@python-dev
Copy link
Mannequin

python-dev mannequin commented Dec 30, 2014

New changeset 29689050ec78 by Benjamin Peterson in branch '3.4':
update docs for bpo-23111
https://hg.python.org/cpython/rev/29689050ec78

@Arfrever
Copy link
Mannequin

Arfrever mannequin commented Jan 4, 2015

I think that this fix should be applied also in 2.7 branch.

@Arfrever Arfrever mannequin reopened this Jan 4, 2015
@python-dev
Copy link
Mannequin

python-dev mannequin commented Jan 4, 2015

New changeset 98ee845a139a by Benjamin Peterson in branch '2.7':
make SSLv23 the default version in ftplib (closes bpo-23111)
https://hg.python.org/cpython/rev/98ee845a139a

@python-dev python-dev mannequin closed this as completed Jan 4, 2015
@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
Assignees
No one assigned
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@giampaolo @pitrou