Skip to content

Commit c3675b9

Browse files
author
bplost
committed
Clean up client-certfile support
Cleaned up the support for client certificate checking Added test file for specific client authentication tests - Good cert (allows connection) - Bad cert (does not allow connection) - No cert (does not allow connection)
1 parent 128b61f commit c3675b9

File tree

4 files changed

+176
-21
lines changed

4 files changed

+176
-21
lines changed

‎docs/api.rst‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,12 +557,7 @@ Extended handlers
557557
means the user will have to issue PROT before PASV or PORT (default
558558
``False``).
559559

560-
.. data:: tls_mutual_authentication
561-
562-
When True requires the client to send a valid certificate to connect to the server
563-
(default ``False``).
564-
565-
.. data:: tls_mutual_auth_certfile
560+
.. data:: client_certfile
566561

567562
The path of the certificate to check the client certificate against. Must be provided
568563
when tls_mutual_authentication is set to ``True`` (default ``None``).

‎pyftpdlib/handlers.py‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@
2222

2323
try:
2424
from OpenSSL import SSL # requires "pip install pyopenssl"
25-
from OpenSSL.SSL import VERIFY_PEER, VERIFY_FAIL_IF_NO_PEER_CERT, VERIFY_CLIENT_ONCE, SESS_CACHE_OFF, OP_NO_TICKET
25+
from OpenSSL.SSL import OP_NO_TICKET
26+
from OpenSSL.SSL import SESS_CACHE_OFF
27+
from OpenSSL.SSL import VERIFY_CLIENT_ONCE
28+
from OpenSSL.SSL import VERIFY_FAIL_IF_NO_PEER_CERT
29+
from OpenSSL.SSL import VERIFY_PEER
2630
except ImportError:
2731
SSL = None
2832

@@ -3416,8 +3420,7 @@ class TLS_FTPHandler(SSLConnection, FTPHandler):
34163420
keyfile = None
34173421
ssl_protocol = SSL.SSLv23_METHOD
34183422
# client certificate configurable attributes
3419-
tls_mutual_authentication = False
3420-
tls_mutual_auth_certfile = None
3423+
client_certfile = None
34213424
# - SSLv2 is easily broken and is considered harmful and dangerous
34223425
# - SSLv3 has several problems and is now dangerous
34233426
# - Disable compression to prevent CRIME attacks for OpenSSL 1.0+
@@ -3450,17 +3453,21 @@ def __init__(self, conn, server, ioloop=None):
34503453
self._pbsz = False
34513454
self._prot = False
34523455
self.ssl_context = self.get_ssl_context()
3456+
if self.client_certfile is not None:
3457+
self.ssl_context.set_verify(VERIFY_PEER |
3458+
VERIFY_FAIL_IF_NO_PEER_CERT |
3459+
VERIFY_CLIENT_ONCE,
3460+
self.verify_certs_callback)
34533461

34543462
def __repr__(self):
34553463
return FTPHandler.__repr__(self)
3456-
3457-
#commented out prints - for different Python version support
3458-
@staticmethod
3459-
def verify_certs_callback(connection, x509, errnum, errdepth, ok):
3460-
#if not ok:
3461-
#print "Bad client certificate detected."
3462-
#else:
3463-
#print "Client certificate ok."
3464+
3465+
# Cannot be @classmethod, need instance to log
3466+
def verify_certs_callback(self, connection, x509, errnum, errdepth, ok):
3467+
if not ok:
3468+
self.log("Bad client certificate detected.")
3469+
else:
3470+
self.log("Client certificate is valid.")
34643471
return ok
34653472

34663473
@classmethod
@@ -3477,9 +3484,12 @@ def get_ssl_context(cls):
34773484
if not cls.keyfile:
34783485
cls.keyfile = cls.certfile
34793486
cls.ssl_context.use_privatekey_file(cls.keyfile)
3480-
if cls.tls_mutual_authentication:
3481-
cls.ssl_context.set_verify(VERIFY_PEER | VERIFY_FAIL_IF_NO_PEER_CERT | VERIFY_CLIENT_ONCE, cls.verify_certs_callback)
3482-
cls.ssl_context.load_verify_locations(cls.tls_mutual_auth_certfile)
3487+
if cls.client_certfile is not None:
3488+
cls.ssl_context.set_verify(VERIFY_PEER |
3489+
VERIFY_FAIL_IF_NO_PEER_CERT |
3490+
VERIFY_CLIENT_ONCE,
3491+
cls.verify_certs_callback)
3492+
cls.ssl_context.load_verify_locations(cls.client_certfile)
34833493
cls.ssl_context.set_session_cache_mode(SESS_CACHE_OFF)
34843494
cls.ssl_options = cls.ssl_options | OP_NO_TICKET
34853495
if cls.ssl_options:

‎pyftpdlib/test/test_functional_ssl.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ class TestCornerCasesTLSMixin(TLSTestMixin, TestCornerCases):
210210

211211
@unittest.skipUnless(FTPS_SUPPORT, FTPS_UNSUPPORT_REASON)
212212
class TestFTPS(unittest.TestCase):
213-
"""Specific tests fot TSL_FTPHandler class."""
213+
"""Specific tests for TLS_FTPHandler class."""
214214

215215
def setUp(self):
216216
self.server = FTPSServer()

‎pyftpdlib/test/test_functional_ssl_client_certfile.py‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
#!/usr/bin/env python
2+
3+
# Copyright (C) 2007-2016 Giampaolo Rodola' <g.rodola@gmail.com>.
4+
# Use of this source code is governed by MIT license that can be
5+
# found in the LICENSE file.
6+
7+
import contextlib
8+
import ftplib
9+
import os
10+
import socket
11+
import sys
12+
import ssl
13+
14+
import OpenSSL # requires "pip install pyopenssl"
15+
16+
from pyftpdlib.handlers import TLS_FTPHandler
17+
from pyftpdlib.test import configure_logging
18+
from pyftpdlib.test import PASSWD
19+
from pyftpdlib.test import remove_test_files
20+
from pyftpdlib.test import ThreadedTestFTPd
21+
from pyftpdlib.test import TIMEOUT
22+
from pyftpdlib.test import TRAVIS
23+
from pyftpdlib.test import unittest
24+
from pyftpdlib.test import USER
25+
from pyftpdlib.test import VERBOSITY
26+
from pyftpdlib.test.test_functional import TestCallbacks
27+
from pyftpdlib.test.test_functional import TestConfigurableOptions
28+
from pyftpdlib.test.test_functional import TestCornerCases
29+
from pyftpdlib.test.test_functional import TestFtpAbort
30+
from pyftpdlib.test.test_functional import TestFtpAuthentication
31+
from pyftpdlib.test.test_functional import TestFtpCmdsSemantic
32+
from pyftpdlib.test.test_functional import TestFtpDummyCmds
33+
from pyftpdlib.test.test_functional import TestFtpFsOperations
34+
from pyftpdlib.test.test_functional import TestFtpListingCmds
35+
from pyftpdlib.test.test_functional import TestFtpRetrieveData
36+
from pyftpdlib.test.test_functional import TestFtpStoreData
37+
from pyftpdlib.test.test_functional import TestIPv4Environment
38+
from pyftpdlib.test.test_functional import TestIPv6Environment
39+
from pyftpdlib.test.test_functional import TestSendfile
40+
from pyftpdlib.test.test_functional import TestTimeouts
41+
from _ssl import SSLError
42+
43+
44+
FTPS_SUPPORT = hasattr(ftplib, 'FTP_TLS')
45+
if sys.version_info < (2, 7):
46+
FTPS_UNSUPPORT_REASON = "requires python 2.7+"
47+
else:
48+
FTPS_UNSUPPORT_REASON = "FTPS test skipped"
49+
50+
CERTFILE = os.path.abspath(os.path.join(os.path.dirname(__file__),
51+
'keycert.pem'))
52+
CLIENT_CERTFILE = os.path.abspath(os.path.join(os.path.dirname(__file__),
53+
'clientcert.pem'))
54+
55+
del OpenSSL
56+
57+
58+
if FTPS_SUPPORT:
59+
class FTPSClient(ftplib.FTP_TLS):
60+
"""A modified version of ftplib.FTP_TLS class which implicitly
61+
secure the data connection after login().
62+
"""
63+
64+
def login(self, *args, **kwargs):
65+
ftplib.FTP_TLS.login(self, *args, **kwargs)
66+
self.prot_p()
67+
68+
class FTPSServerAuth(ThreadedTestFTPd):
69+
"""A threaded FTPS server that forces client certificate
70+
authentication used for functional testing.
71+
"""
72+
handler = TLS_FTPHandler
73+
handler.certfile = CERTFILE
74+
handler.client_certfile = CLIENT_CERTFILE
75+
76+
77+
# =====================================================================
78+
# dedicated FTPS tests with client authentication
79+
# =====================================================================
80+
81+
82+
@unittest.skipUnless(FTPS_SUPPORT, FTPS_UNSUPPORT_REASON)
83+
class TestFTPS(unittest.TestCase):
84+
"""Specific tests for TLS_FTPHandler class."""
85+
86+
def setUp(self):
87+
self.server = FTPSServerAuth()
88+
self.server.start()
89+
90+
def tearDown(self):
91+
self.client.close()
92+
self.server.stop()
93+
94+
def assertRaisesWithMsg(self, excClass, msg, callableObj, *args, **kwargs):
95+
try:
96+
callableObj(*args, **kwargs)
97+
except excClass as err:
98+
if str(err) == msg:
99+
return
100+
raise self.failureException("%s != %s" % (str(err), msg))
101+
else:
102+
if hasattr(excClass, '__name__'):
103+
excName = excClass.__name__
104+
else:
105+
excName = str(excClass)
106+
raise self.failureException("%s not raised" % excName)
107+
108+
def test_auth_client_cert(self):
109+
self.client = ftplib.FTP_TLS(timeout=TIMEOUT, certfile=CLIENT_CERTFILE)
110+
self.client.connect(self.server.host, self.server.port)
111+
# secured
112+
try:
113+
self.client.login()
114+
except Exception as e:
115+
self.fail("login with certificate should work")
116+
117+
def test_auth_client_nocert(self):
118+
self.client = ftplib.FTP_TLS(timeout=TIMEOUT)
119+
self.client.connect(self.server.host, self.server.port)
120+
try:
121+
self.client.login()
122+
except SSLError as e:
123+
# client should not be able to log in
124+
if "SSLV3_ALERT_HANDSHAKE_FAILURE" in e.reason:
125+
pass
126+
else:
127+
self.fail("Incorrect SSL error with missing client certificate")
128+
else:
129+
self.fail("Client able to log in with no certificate")
130+
131+
def test_auth_client_badcert(self):
132+
self.client = ftplib.FTP_TLS(timeout=TIMEOUT, certfile=CERTFILE)
133+
self.client.connect(self.server.host, self.server.port)
134+
try:
135+
self.client.login()
136+
except Exception as e:
137+
# client should not be able to log in
138+
if "TLSV1_ALERT_UNKNOWN_CA" in e.reason:
139+
pass
140+
else:
141+
self.fail("Incorrect SSL error with bad client certificate")
142+
else:
143+
self.fail("Client able to log in with bad certificate")
144+
145+
configure_logging()
146+
remove_test_files()
147+
148+
149+
if __name__ == '__main__':
150+
unittest.main(verbosity=VERBOSITY)

0 commit comments

Comments
 (0)