From 5d76c0feb1cc4690149be589a96e61217b9488ad Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 19 Feb 2015 22:30:11 -0800 Subject: [PATCH] Final cleanup for revoker/display --- letsencrypt/client/client.py | 2 + letsencrypt/client/crypto_util.py | 1 - letsencrypt/client/display/ops.py | 4 +- letsencrypt/client/display/util.py | 3 +- letsencrypt/client/interfaces.py | 1 + letsencrypt/client/le_util.py | 1 + letsencrypt/client/revoker.py | 20 +++++++-- letsencrypt/client/tests/client_test.py | 3 +- .../client/tests/display/revocation_test.py | 1 + letsencrypt/client/tests/display/util_test.py | 2 - letsencrypt/client/tests/revoker_test.py | 41 +++++++++++++++++++ letsencrypt/scripts/main.py | 8 ++-- tox.ini | 2 +- 13 files changed, 74 insertions(+), 15 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index e1ffd9551..4084df2dc 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -320,6 +320,7 @@ def init_key(key_size, key_dir): return le_util.Key(key_filename, key_pem) + def init_csr(privkey, names, cert_dir): """Initialize a CSR with the given private key. @@ -344,6 +345,7 @@ def init_csr(privkey, names, cert_dir): return le_util.CSR(csr_filename, csr_der, "der") + # This should be controlled by commandline parameters def determine_authenticator(all_auths): """Returns a valid IAuthenticator. diff --git a/letsencrypt/client/crypto_util.py b/letsencrypt/client/crypto_util.py index c9bae885e..4c053aeae 100644 --- a/letsencrypt/client/crypto_util.py +++ b/letsencrypt/client/crypto_util.py @@ -161,4 +161,3 @@ def make_ss_cert(key_str, domains, not_before=None, assert m2_cert.verify() # print check_purpose(,0 return m2_cert.as_pem() - diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py index 9cd8e16e5..29ce6929d 100644 --- a/letsencrypt/client/display/ops.py +++ b/letsencrypt/client/display/ops.py @@ -109,12 +109,14 @@ def _choose_names_manually(): def success_installation(domains): """Display a box confirming the installation of HTTPS. + .. todo:: This should be centered on the screen + :param list domains: domain names which were enabled """ util(interfaces.IDisplay).notification( "Congratulations! You have successfully enabled " - "%s!" % _gen_https_names(domains), pause=True) + "%s!" % _gen_https_names(domains), pause=False) def _gen_https_names(domains): diff --git a/letsencrypt/client/display/util.py b/letsencrypt/client/display/util.py index ad324b1b8..111e46500 100644 --- a/letsencrypt/client/display/util.py +++ b/letsencrypt/client/display/util.py @@ -143,6 +143,7 @@ class NcursesDisplay(object): return self.dialog.checklist( message, width=self.width, height=self.height, choices=choices) + class FileDisplay(object): """File-based display.""" @@ -305,7 +306,6 @@ class FileDisplay(object): # Transform indices to appropriate tags return [tags[index-1] for index in indices] - def _print_menu(self, message, choices): """Print a menu on the screen. @@ -400,6 +400,7 @@ def separate_list_input(input_): # Each string is naturally unicode, this causes problems with M2Crypto SANs return [str(string) for string in no_commas.split()] + def _parens_around_char(label): """Place parens around first character of label. diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py index 1dbe930d8..f53354390 100644 --- a/letsencrypt/client/interfaces.py +++ b/letsencrypt/client/interfaces.py @@ -65,6 +65,7 @@ class IAuthenticator(zope.interface.Interface): """ + class IConfig(zope.interface.Interface): """Let's Encrypt user-supplied configuration. diff --git a/letsencrypt/client/le_util.py b/letsencrypt/client/le_util.py index 7c3ef0762..1615fc29d 100644 --- a/letsencrypt/client/le_util.py +++ b/letsencrypt/client/le_util.py @@ -11,6 +11,7 @@ Key = collections.namedtuple("Key", "file pem") # Note: form is the type of data, "pem" or "der" CSR = collections.namedtuple("CSR", "file data form") + def make_or_verify_dir(directory, mode=0o755, uid=0): """Make sure directory exists with proper permissions. diff --git a/letsencrypt/client/revoker.py b/letsencrypt/client/revoker.py index 73ac779c9..58b64b6b2 100644 --- a/letsencrypt/client/revoker.py +++ b/letsencrypt/client/revoker.py @@ -40,6 +40,8 @@ class Revoker(object): :ivar config: Configuration. :type config: :class:`~letsencrypt.client.interfaces.IConfig` + :ivar bool no_confirm: Whether or not to ask for confirmation for revocation + """ def __init__(self, installer, config, no_confirm=False): self.network = network.Network(config.server) @@ -62,25 +64,31 @@ class Revoker(object): """ certs = [] + clean_pem = Crypto.PublicKey.RSA.importKey(authkey.pem).exportKey("PEM") with open(self.list_path, "rb") as csvfile: csvreader = csv.reader(csvfile) for row in csvreader: # idx, cert, key # Add all keys that match to marked list - # TODO: This doesn't account for padding in the file that might - # differ. This should only consider the key material. # Note: The key can be different than the pub key found in the # certificate. _, b_k = self._row_to_backup(row) - if authkey.pem == open(b_k).read(): + if clean_pem == Crypto.PublicKey.RSA.importKey( + open(b_k).read()).exportKey("PEM"): certs.append(Cert.fromrow(row, self.config.cert_key_backup)) if certs: self._safe_revoke(certs) + else: + logging.info("No certificates using the authorized key were found.") def revoke_from_cert(self, cert_path): """Revoke a certificate by specifying a file path. + .. todo:: Add the ability to revoke the certificate even if the cert + is not stored locally. A path to the auth key will need to be + attained from the user. + :param str cert_path: path to ACME certificate in pem form """ @@ -94,6 +102,9 @@ class Revoker(object): if cert == cert_to_revoke: self._safe_revoke([cert]) + return + + logging.info("Associated ACME certificate was not found.") def revoke_from_menu(self): """List trusted Let's Encrypt certificates.""" @@ -128,6 +139,9 @@ class Revoker(object): Namely, additional certs/keys may exist. There should never be any certs/keys in the LIST that don't exist in the directory however. + :param dict csha1_vhlist: map from cert sha1 fingerprints to a list + of it's installed location paths. + """ certs = [] with open(self.list_path, "rb") as csvfile: diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index 41aaf38a4..79bf799c9 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -49,7 +49,7 @@ class DetermineAuthenticatorTest(unittest.TestCase): @mock.patch("letsencrypt.client.client.logging") @mock.patch("letsencrypt.client.client.ops.choose_authenticator") - def test_misconfigured(self, mock_choose, mock_log): + def test_misconfigured(self, mock_choose, mock_log): # pylint: disable=unused-argument self.mock_apache.side_effect = errors.LetsEncryptMisconfigurationError mock_choose.return_value = self.mock_apache @@ -61,6 +61,7 @@ class DetermineAuthenticatorTest(unittest.TestCase): self._call, [("desc", self.mock_apache, "1", "2", "3", "4", "5")]) + class RollbackTest(unittest.TestCase): """Test the rollback function.""" def setUp(self): diff --git a/letsencrypt/client/tests/display/revocation_test.py b/letsencrypt/client/tests/display/revocation_test.py index ecf247757..db37effb2 100644 --- a/letsencrypt/client/tests/display/revocation_test.py +++ b/letsencrypt/client/tests/display/revocation_test.py @@ -54,6 +54,7 @@ class ChooseCertsTest(unittest.TestCase): self.assertTrue(self.certs[choice] == self.cert1) self.assertEqual(mock_util().notification.call_count, 1) + class SuccessRevocationTest(unittest.TestCase): def setUp(self): from letsencrypt.client.revoker import Cert diff --git a/letsencrypt/client/tests/display/util_test.py b/letsencrypt/client/tests/display/util_test.py index 89dc3cfe3..5a0bb4c1f 100644 --- a/letsencrypt/client/tests/display/util_test.py +++ b/letsencrypt/client/tests/display/util_test.py @@ -67,7 +67,6 @@ class NcursesDisplayTest(DisplayT): ret = self.displayer.menu("Message", self.choices) - mock_menu.assert_called_with( "Message", choices=self.choices, ok_label="OK", cancel_label="Cancel", @@ -82,7 +81,6 @@ class NcursesDisplayTest(DisplayT): ret = self.displayer.menu("Message", self.tags, help_label="More Info") - mock_menu.assert_called_with( "Message", choices=self.tags_choices, ok_label="OK", cancel_label="Cancel", diff --git a/letsencrypt/client/tests/revoker_test.py b/letsencrypt/client/tests/revoker_test.py index f5c9feace..b415e2154 100644 --- a/letsencrypt/client/tests/revoker_test.py +++ b/letsencrypt/client/tests/revoker_test.py @@ -77,6 +77,26 @@ class RevokerTest(RevokerBase): self.assertEqual(mock_net.call_count, 2) + @mock.patch("letsencrypt.client.revoker.network." + "Network.send_and_receive_expected") + @mock.patch("letsencrypt.client.revoker.revocation") + def test_revoke_by_wrong_key(self, mock_display, mock_net): + mock_display().confirm_revocation.return_value = True + + key_path = pkg_resources.resource_filename( + "letsencrypt.client.tests", os.path.join( + "testdata", "rsa256_key.pem")) + + wrong_key = le_util.Key(key_path, open(key_path).read()) + self.revoker.revoke_from_key(wrong_key) + + # Nothing was removed + self.assertEqual(len(self._get_rows()), 2) + # No revocation went through + self.assertEqual(mock_net.call_count, 0) + + + @mock.patch("letsencrypt.client.revoker.network." "Network.send_and_receive_expected") @mock.patch("letsencrypt.client.revoker.revocation") @@ -95,6 +115,26 @@ class RevokerTest(RevokerBase): self.assertEqual(mock_net.call_count, 1) + @mock.patch("letsencrypt.client.revoker.network." + "Network.send_and_receive_expected") + @mock.patch("letsencrypt.client.revoker.revocation") + def test_revoke_by_cert_not_found(self, mock_display, mock_net): + mock_display().confirm_revocation.return_value = True + + self.revoker.revoke_from_cert(self.paths[0]) + self.revoker.revoke_from_cert(self.paths[0]) + + row0 = self.certs[0].get_row() + row1 = self.certs[1].get_row() + + # Same check as last time... just reversed. + self.assertEqual(self._get_rows(), [row1]) + + self.assertTrue(self._backups_exist(row1)) + self.assertFalse(self._backups_exist(row0)) + + self.assertEqual(mock_net.call_count, 1) + @mock.patch("letsencrypt.client.revoker.network." "Network.send_and_receive_expected") @mock.patch("letsencrypt.client.revoker.revocation") @@ -228,6 +268,7 @@ class RevokerInstallerTest(RevokerBase): # pylint: disable=protected-access self.assertEqual(revoker._get_installed_locations(), {}) + class RevokerClassMethodsTest(RevokerBase): def setUp(self): super(RevokerClassMethodsTest, self).setUp() diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 2e102ffa6..06add9add 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -19,7 +19,7 @@ from letsencrypt.client import client from letsencrypt.client import interfaces from letsencrypt.client import le_util from letsencrypt.client import log -from letsencrypt.client import standalone_authenticator +from letsencrypt.client import standalone_authenticator as standalone from letsencrypt.client.apache import configurator from letsencrypt.client.display import util as display_util from letsencrypt.client.display import ops @@ -124,8 +124,7 @@ def main(): # pylint: disable=too-many-branches client.view_config_changes(config) sys.exit() - # TODO: if revoke, rev_cert... - if args.revoke: + if args.revoke or args.rev_cert or args.rev_key: client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key) sys.exit() @@ -139,8 +138,7 @@ def main(): # pylint: disable=too-many-branches # list of (Description, Known Authenticator classes, init arguments) all_auths = [ ("Apache Web Server", configurator.ApacheConfigurator, config), - ("Standalone Authenticator", - standalone_authenticator.StandaloneAuthenticator), + ("Standalone Authenticator", standalone.StandaloneAuthenticator), ] auth = client.determine_authenticator(all_auths) if auth is None: diff --git a/tox.ini b/tox.ini index d4af50fa5..59d943a3e 100644 --- a/tox.ini +++ b/tox.ini @@ -17,7 +17,7 @@ setenv = basepython = python2.7 commands = pip install -e .[testing] - python setup.py nosetests --with-coverage --cover-min-percentage=83 + python setup.py nosetests --with-coverage --cover-min-percentage=84 [testenv:lint] # recent versions of pylint do not support Python 2.6 (#97, #187)