From 5ca95a55ca7e3bf8f4ccac1aa6eaee2e365e005f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 25 Nov 2014 09:48:35 +0100 Subject: [PATCH] Fix recent commits style --- letsencrypt/client/client.py | 21 ++++++----- letsencrypt/client/crypto_util.py | 60 +++++++++++++++++-------------- 2 files changed, 46 insertions(+), 35 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index d4029e891..aeaaf8762 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -47,7 +47,7 @@ class Client(object): self.csr_file = cert_signing_request self.key_file = private_key - self.__validate_csr_key_cli() + self._validate_csr_key_cli() self.server_url = "https://%s/acme/" % self.server @@ -643,10 +643,13 @@ class Client(object): else: return key_pem, csr_pem - def __validate_csr_key_cli(self): + def _validate_csr_key_cli(self): + """Validate CSR and key files. + + Verifies that the client key and csr arguments are valid and + correspond to one another. + """ - Verifies that the client key and csr arguments are valid and correspond - to one another""" # If CSR is provided, the private key should also be provided. if self.csr_file and not self.key_file: logger.fatal(("Please provide the private key file used in " @@ -657,24 +660,24 @@ class Client(object): if self.csr_file and not crypto_util.valid_csr(self.csr_file): logger.fatal("The provided CSR is not a valid CSR") sys.exit(1) - except IOError, e: + except IOError: raise Exception("The provided CSR could not be read") # If key is provided, it must be readable and valid. try: if self.key_file and not crypto_util.valid_privkey(self.key_file): logger.fatal("The provided key is not a valid key") sys.exit(1) - except IOError, e: + except IOError: raise Exception("The provided key could not be read") # If CSR and key are provided, the key must be the same key used # in the CSR. if self.csr_file and self.key_file: try: - if not crypto_util.csr_matches_pubkey(self.csr_file, - self.key_file): + if not crypto_util.csr_matches_pubkey( + self.csr_file, self.key_file): raise Exception("The key and CSR do not match") - except IOError, e: + except IOError: raise Exception("The key or CSR files could not be read") # def choice_of_ca(self): diff --git a/letsencrypt/client/crypto_util.py b/letsencrypt/client/crypto_util.py index 39b363362..b874ff691 100644 --- a/letsencrypt/client/crypto_util.py +++ b/letsencrypt/client/crypto_util.py @@ -205,16 +205,20 @@ def get_cert_info(filename): # B. Audit the parsing code for vulnerabilities def valid_csr(csr_filename): - """Check if csr_filename is a valid CSR for the given domains. - (Currently, could raise non-X.509-related errors such as IOError - associated with problems reading the file.) + """Validate CSR. + + Check if `csr_filename` is a valid CSR for the given domains. + + TODO: Currently, could raise non-X.509-related errors such as IOError + associated with problems reading the file. Comment or handle. :param csr_filename: Path to the purported CSR file. :type csr_filename: str :returns: Validity of CSR. - :rtype: bool""" + :rtype: bool + """ try: csr = M2Crypto.X509.load_request(csr_filename) return bool(csr.verify(csr.get_pubkey())) @@ -223,52 +227,57 @@ def valid_csr(csr_filename): def csr_matches_names(csr_filename, domains): - """Check if csr_filename contains the subject of one of the domains + """Check if CSR contains the subject of one of the domains. + M2Crypto currently does not expose the OpenSSL interface to also check the SAN extension. This is insufficient for full testing - (Currently, could raise non-X.509-related errors such as IOError - associated with problems reading the file.) + + TODO: Currently, could raise non-X.509-related errors such as IOError + associated with problems reading the file. Comment or handle. :param csr_filename: Path to the purported CSR file. :type csr_filename: str - :param domains: domains the csr should contain + :param domains: Domains the CSR should contain. :type domains: list - :returns: If the csr subject contains one of the domains - :rtype: bool""" + :returns: If the CSR subject contains one of the domains + :rtype: bool + """ try: csr = M2Crypto.X509.load_request(csr_filename) - subject = csr.get_subject() - - return subject.CN in domains - + return csr.get_subject().CN in domains except M2Crypto.X509.X509Error: return False def valid_privkey(privkey_filename): - """Check if privkey_filename is a valid RSA private key. (Currently, - could raise non-RSA-related errors such as IOError associated with - problems reading the file.) + """Is valid RSA private key? + + TODO: Currently, could raise non-X.509-related errors such as IOError + associated with problems reading the file. Comment or handle. :param privkey_filename: Path to the purported private key file. :type privkey_filename: str :returns: Validity of private key. - :rtype: bool""" + :rtype: bool + """ try: - privkey = M2Crypto.RSA.load_key(privkey_filename) - return bool(privkey.check_key()) + return bool(M2Crypto.RSA.load_key(privkey_filename).check_key()) except M2Crypto.RSA.RSAError: return False def csr_matches_pubkey(csr_filename, privkey_filename): - """Check if the private key in the file corresponds to the subject - public key in the CSR. + """Does private key correspond to the subject public key in the CSR? + + TODO: Currently, could raise non-X.509-related errors such as IOError + associated with problems reading the file. Comment or handle. + + TODO: Seems that this doesn not handle X509 eceptions either. :param csr_filename: Path to the purported CSR file. :type csr_filename: str @@ -277,10 +286,9 @@ def csr_matches_pubkey(csr_filename, privkey_filename): :type privkey_filename: str :returns: Correspondence of private key to CSR subject public key. - :rtype: bool""" + :rtype: bool + """ csr = M2Crypto.X509.load_request(csr_filename) privkey = M2Crypto.RSA.load_key(privkey_filename) - csr_pub = csr.get_pubkey().get_rsa().pub() - privkey_pub = privkey.pub() - return csr_pub == privkey_pub + return csr.get_pubkey().get_rsa().pub() == privkey.pub()