Final cleanup for revoker/display

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.

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()
-

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):

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.
 

letsencrypt/client/interfaces.py

@@ -65,6 +65,7 @@ class IAuthenticator(zope.interface.Interface):
 
         """
 
+
 class IConfig(zope.interface.Interface):
     """Let's Encrypt user-supplied configuration.
 

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.
 

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:

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):

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

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",

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()

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 or args.rev_cert or args.rev_key:
-    if args.revoke:
         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", standalone.StandaloneAuthenticator),
-        standalone_authenticator.StandaloneAuthenticator),
     ]
     auth = client.determine_authenticator(all_auths)
     if auth is None:

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)