BIND 10 trac2595, updated. e273dedf8a3875ddb241b796c19144d14819f5d8 [2595] Additional tests for ssl socket wrapping code
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jan 21 13:28:35 UTC 2013
The branch, trac2595 has been updated
via e273dedf8a3875ddb241b796c19144d14819f5d8 (commit)
via fae9e0052b6f71b38e003bda562cc49ab4d4b9c7 (commit)
via aa76df71dcd756e7b233af47a6c936263813d90b (commit)
via 55b05119c73ef4858995efd2377c16e17f8a0f06 (commit)
via 0f4eb55137c6c0d2da2f14270a3c843fae14d187 (commit)
from d4d5c0ee659e6ecfb3e3384f2462a58559464f80 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit e273dedf8a3875ddb241b796c19144d14819f5d8
Author: Jelte Jansen <jelte at isc.org>
Date: Mon Jan 21 11:58:26 2013 +0100
[2595] Additional tests for ssl socket wrapping code
commit fae9e0052b6f71b38e003bda562cc49ab4d4b9c7
Author: Jelte Jansen <jelte at isc.org>
Date: Mon Jan 21 11:47:32 2013 +0100
[2595] Use build patch for pem files in cmdctl test
commit aa76df71dcd756e7b233af47a6c936263813d90b
Author: Jelte Jansen <jelte at isc.org>
Date: Mon Jan 21 10:12:42 2013 +0100
[2595] Minor update to internal comment
commit 55b05119c73ef4858995efd2377c16e17f8a0f06
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Jan 18 15:09:37 2013 +0100
[2595] Update __try_login in bindcmd and add test
commit 0f4eb55137c6c0d2da2f14270a3c843fae14d187
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Jan 18 12:39:31 2013 +0100
[2595] Make certificate error non-fatal
-----------------------------------------------------------------------
Summary of changes:
src/bin/bindctl/bindcmd.py | 19 +++++++++++-----
src/bin/bindctl/tests/bindctl_test.py | 28 +++++++++++++++++++++++
src/bin/cmdctl/cmdctl.py.in | 19 +++++-----------
src/bin/cmdctl/cmdctl_messages.mes | 4 ----
src/bin/cmdctl/tests/Makefile.am | 2 +-
src/bin/cmdctl/tests/cmdctl_test.py | 39 +++++++++++++++++++++------------
6 files changed, 74 insertions(+), 37 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/bindctl/bindcmd.py b/src/bin/bindctl/bindcmd.py
index 2bf8f47..f6727d7 100644
--- a/src/bin/bindctl/bindcmd.py
+++ b/src/bin/bindctl/bindcmd.py
@@ -209,9 +209,19 @@ WARNING: Python readline module isn't available, so the command line editor
return True
def __try_login(self, username, password):
+ '''
+ Attempts to log in to bindctl by sending a POST with
+ the given username and password.
+ On success of the POST (mind, not the login, only the network
+ operation), returns a tuple (response, data).
+ On failure, raises a FailToLogin exception, and prints some
+ information on the failure.
+ '''
param = {'username': username, 'password' : password}
try:
- return self.send_POST('/login', param)
+ response = self.send_POST('/login', param)
+ data = response.read().decode()
+ return (response, data)
except socket.error as err:
print("Socket error while sending login information: ", err)
if err.errno == errno.ECONNRESET:
@@ -230,8 +240,8 @@ WARNING: Python readline module isn't available, so the command line editor
# Look at existing username/password combinations and try to log in
users = self._get_saved_user_info(self.csv_file_dir, CSV_FILE_NAME)
for row in users:
- response = self.__try_login(row[0], row[1])
- data = response.read().decode()
+ response, data = self.__try_login(row[0], row[1])
+
if response.status == http.client.OK:
# Is interactive?
if sys.stdin.isatty():
@@ -252,8 +262,7 @@ WARNING: Python readline module isn't available, so the command line editor
username = input("Username: ")
passwd = getpass.getpass()
- response = self.__try_login(username, passwd)
- data = response.read().decode()
+ response, data = self.__try_login(username, passwd)
print(data)
if response.status == http.client.OK:
diff --git a/src/bin/bindctl/tests/bindctl_test.py b/src/bin/bindctl/tests/bindctl_test.py
index 5c6aeb2..a7e9205 100644
--- a/src/bin/bindctl/tests/bindctl_test.py
+++ b/src/bin/bindctl/tests/bindctl_test.py
@@ -347,6 +347,34 @@ class TestConfigCommands(unittest.TestCase):
precmd('EOF')
self.assertRaises(socket.error, precmd, 'continue')
+ def test_try_login(self):
+ # Make sure __try_login raises the correct exception
+ # upon failure of either send_POST or the read() on the
+ # response
+
+ orig_send_POST = self.tool.send_POST
+ try:
+ def send_POST_raiseImmediately(self, params):
+ raise socket.error("test error")
+
+ self.tool.send_POST = send_POST_raiseImmediately
+ self.assertRaises(FailToLogin,
+ self.tool._BindCmdInterpreter__try_login,
+ "foo", "bar")
+
+ def send_POST_raiseOnRead(self, params):
+ class MyResponse:
+ def read(self):
+ raise socket.error("read error")
+ return MyResponse()
+
+ self.tool.send_POST = send_POST_raiseOnRead
+ self.assertRaises(FailToLogin,
+ self.tool._BindCmdInterpreter__try_login,
+ "foo", "bar")
+ finally:
+ self.tool.send_POST = orig_send_POST
+
def test_run(self):
def login_to_cmdctl():
return True
diff --git a/src/bin/cmdctl/cmdctl.py.in b/src/bin/cmdctl/cmdctl.py.in
index cf10399..7a84a08 100755
--- a/src/bin/cmdctl/cmdctl.py.in
+++ b/src/bin/cmdctl/cmdctl.py.in
@@ -82,13 +82,6 @@ SPECFILE_LOCATION = SPECFILE_PATH + os.sep + "cmdctl.spec"
class CmdctlException(Exception):
pass
-class CmdctlFatalException(Exception):
- """
- Exception for fatal errors, which should not be caught anywhere but on
- the highest level.
- """
- pass
-
def check_file(file_name):
# TODO: Check contents of certificate file
if not os.path.exists(file_name):
@@ -296,7 +289,8 @@ class CommandControl():
if key == 'version':
continue
elif key in ['key_file', 'cert_file']:
- #TODO, only check whether the file exist,
+ # TODO, only check whether the file exist, is a
+ # file, and is readable
# further check need to be done: eg. whether
# the private/certificate is valid.
path = new_config[key]
@@ -561,9 +555,10 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
self.close_request(sock)
# raise socket error to finish the request
raise socket.error
- except CmdctlException as cce:
- logger.fatal(CMDCTL_SSL_SETUP_FAILURE_READING_CERT, cce)
- raise CmdctlFatalException("Unable to accept SSL connections")
+ except (CmdctlException, IOError) as cce:
+ logger.error(CMDCTL_SSL_SETUP_FAILURE_READING_CERT, cce)
+ # raise socket error to finish the request
+ raise socket.error
def get_request(self):
'''Get client request socket and wrap it in SSL context. '''
@@ -652,8 +647,6 @@ if __name__ == '__main__':
logger.info(CMDCTL_STOPPED_BY_KEYBOARD)
except CmdctlException as err:
logger.fatal(CMDCTL_UNCAUGHT_EXCEPTION, err);
- except CmdctlFatalException as fe:
- logger.fatal(CMDCTL_FATAL_EXCEPTION, fe)
if httpd:
httpd.shutdown()
diff --git a/src/bin/cmdctl/cmdctl_messages.mes b/src/bin/cmdctl/cmdctl_messages.mes
index aca8dc3..5fbb430 100644
--- a/src/bin/cmdctl/cmdctl_messages.mes
+++ b/src/bin/cmdctl/cmdctl_messages.mes
@@ -43,9 +43,6 @@ specific error is printed in the message.
This debug message indicates that the given command has been sent to
the given module.
-% CMDCTL_FATAL_EXCEPTION A fatal error occured: %1
-While running, b10-cmdctl encountered a fatal exception and it will shut down
-
% CMDCTL_NO_SUCH_USER username not found in user database: %1
A login attempt was made to b10-cmdctl, but the username was not known.
Users can be added with the tool b10-cmdctl-usermgr.
@@ -64,7 +61,6 @@ the given module.
% CMDCTL_SSL_SETUP_FAILURE_READING_CERT failed to read certificate or key: %1
The b10-cmdctl daemon is unable to read either the certificate file or
the private key file, and is therefore unable to accept any SSL connections.
-This is a fatal error, as b10-cmdctl cannot be of any use in this state.
The specific error is printed in the message.
The administrator should solve the issue with the files, or recreate them
with the b10-certgen tool.
diff --git a/src/bin/cmdctl/tests/Makefile.am b/src/bin/cmdctl/tests/Makefile.am
index 6d8f282..74b801a 100644
--- a/src/bin/cmdctl/tests/Makefile.am
+++ b/src/bin/cmdctl/tests/Makefile.am
@@ -25,7 +25,7 @@ endif
echo Running test: $$pytest ; \
$(LIBRARY_PATH_PLACEHOLDER) \
PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/cmdctl \
- CMDCTL_SPEC_PATH=$(abs_top_builddir)/src/bin/cmdctl \
+ CMDCTL_BUILD_PATH=$(abs_top_builddir)/src/bin/cmdctl \
CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
diff --git a/src/bin/cmdctl/tests/cmdctl_test.py b/src/bin/cmdctl/tests/cmdctl_test.py
index 958ea47..f660bf1 100644
--- a/src/bin/cmdctl/tests/cmdctl_test.py
+++ b/src/bin/cmdctl/tests/cmdctl_test.py
@@ -22,14 +22,14 @@ import sys
from cmdctl import *
import isc.log
-SPEC_FILE_PATH = '..' + os.sep
-if 'CMDCTL_SPEC_PATH' in os.environ:
- SPEC_FILE_PATH = os.environ['CMDCTL_SPEC_PATH'] + os.sep
-
SRC_FILE_PATH = '..' + os.sep
if 'CMDCTL_SRC_PATH' in os.environ:
SRC_FILE_PATH = os.environ['CMDCTL_SRC_PATH'] + os.sep
+BUILD_FILE_PATH = '..' + os.sep
+if 'CMDCTL_BUILD_PATH' in os.environ:
+ BUILD_FILE_PATH = os.environ['CMDCTL_BUILD_PATH'] + os.sep
+
# Rewrite the class for unittest.
class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
def __init__(self):
@@ -302,7 +302,7 @@ class MyCommandControl(CommandControl):
return {}
def _setup_session(self):
- spec_file = SPEC_FILE_PATH + 'cmdctl.spec'
+ spec_file = BUILD_FILE_PATH + 'cmdctl.spec'
module_spec = isc.config.module_spec_from_file(spec_file)
config = isc.config.config_data.ConfigData(module_spec)
self._module_name = 'Cmdctl'
@@ -472,7 +472,7 @@ class TestSecureHTTPServer(unittest.TestCase):
def test_check_file(self):
# Just some file that we know exists
- file_name = SRC_FILE_PATH + 'cmdctl-keyfile.pem'
+ file_name = BUILD_FILE_PATH + 'cmdctl-keyfile.pem'
check_file(file_name)
with UnreadableFile(file_name):
self.assertRaises(CmdctlException, check_file, file_name)
@@ -481,8 +481,8 @@ class TestSecureHTTPServer(unittest.TestCase):
def test_check_key_and_cert(self):
- keyfile = SRC_FILE_PATH + 'cmdctl-keyfile.pem'
- certfile = SRC_FILE_PATH + 'cmdctl-certfile.pem'
+ keyfile = BUILD_FILE_PATH + 'cmdctl-keyfile.pem'
+ certfile = BUILD_FILE_PATH + 'cmdctl-certfile.pem'
# no exists
self.assertRaises(CmdctlException, self.server._check_key_and_cert,
@@ -513,17 +513,28 @@ class TestSecureHTTPServer(unittest.TestCase):
def test_wrap_sock_in_ssl_context(self):
sock = socket.socket()
- # Test exception is Fatal here (all specific cases are tested
- # in test_check_key_and_cert())
- self.assertRaises(CmdctlFatalException,
+ # Bad files should result in a socket.error raised by our own
+ # code in the basic file checks
+ self.assertRaises(socket.error,
self.server._wrap_socket_in_ssl_context,
sock,
'no_such_file', 'no_such_file')
+ # Using a non-certificate file would cause an SSLError, which
+ # is caught by our code which then raises a basic socket.error
+ self.assertRaises(socket.error,
+ self.server._wrap_socket_in_ssl_context,
+ sock,
+ BUILD_FILE_PATH + 'cmdctl.py',
+ BUILD_FILE_PATH + 'cmdctl-certfile.pem')
+
+ # Should succeed
sock1 = socket.socket()
- self.server._wrap_socket_in_ssl_context(sock1,
- SRC_FILE_PATH + 'cmdctl-keyfile.pem',
- SRC_FILE_PATH + 'cmdctl-certfile.pem')
+ ssl_sock = self.server._wrap_socket_in_ssl_context(sock1,
+ BUILD_FILE_PATH + 'cmdctl-keyfile.pem',
+ BUILD_FILE_PATH + 'cmdctl-certfile.pem')
+ self.assertIsInstance(ssl_sock, ssl.SSLSocket)
+
class TestFuncNotInClass(unittest.TestCase):
def test_check_port(self):
More information about the bind10-changes
mailing list