Work with CSR and private key file contents in the client.

letsencrypt/client/client.py

@@ -44,13 +44,13 @@ class Client(object):
             CONFIG.SERVER_ROOT)
 
         self.server = ca_server
-        
+
         if cert_signing_request:
-            self.csr_file = cert_signing_request.name
+            self.csr_file = cert_signing_request
         else:
             self.csr_file = None
         if private_key:
-            self.key_file = private_key.name
+            self.key_file = private_key
         else:
             self.key_file = None
 
@@ -65,6 +65,16 @@ class Client(object):
         self.server_url = "https://%s/acme/" % self.server
 
     def authenticate(self, domains=None, redirect=None, eula=False):
+        """
+
+        :param domains: List of domains
+        :type domains: list
+        :param redirect:
+        :type redirect: bool|None
+        :param eula: EULA accepted
+        :type eula: bool
+        :raise Exception: CSR does not contain one of the specified names.
+        """
         domains = [] if domains is None else domains
 
         # Check configuration
@@ -248,7 +258,7 @@ class Client(object):
         """Send ACME message to server and return expected message.
 
         :param msg: ACME message (JSON serializable).
-        :type acem_msg: dict
+        :type msg: dict
 
         :param expected: Name of the expected response ACME message type.
         :type expected: str
@@ -552,7 +562,7 @@ class Client(object):
         :param name: TODO
         :type name: TODO
 
-        :param challanges: A list of challenges from ACME "challenge"
+        :param challenges: A list of challenges from ACME "challenge"
                            server message to be fulfilled by the client
                            in order to prove possession of the identifier.
         :type challenges: list
@@ -673,33 +683,19 @@ class Client(object):
         # The client can eventually do things like prompt the user
         # and allow the user to take more appropriate actions
 
-        # 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 "
-                          "generating the provided CSR"))
-            sys.exit(1)
         # If CSR is provided, it must be readable and valid.
-        try:
-            if self.csr_file and not crypto_util.valid_csr(self.csr_file):
-                raise Exception("The provided CSR is not a valid CSR")
-        except IOError:
-            raise Exception("The provided CSR could not be read")
+        if self.csr_file and not crypto_util.valid_csr(self.csr_file):
+            raise Exception("The provided CSR is not a valid CSR")
+
         # If key is provided, it must be readable and valid.
-        try:
-            if self.key_file and not crypto_util.valid_privkey(self.key_file):
-                raise Exception("The provided key is not a valid key")
-        except IOError:
-            raise Exception("The provided key could not be read")
+        if self.key_file and not crypto_util.valid_privkey(self.key_file):
+            raise Exception("The provided key is not a valid key")
 
         # 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):
-                    raise Exception("The key and CSR do not match")
-            except IOError:
-                raise Exception("The key or CSR files could not be read")
+            if not crypto_util.csr_matches_pubkey(self.csr_file, self.key_file):
+                raise Exception("The key and CSR do not match")
 
     def get_all_names(self):
         """Return all valid names in the configuration."""
@@ -753,6 +749,12 @@ def remove_cert_key(cert):
 
 
 def sanity_check_names(names):
+    """Make sure host names are valid.
+
+    :param names: List of host names
+    :type names: list
+
+    """
     for name in names:
         if not is_hostname_sane(name):
             logger.fatal(repr(name) + " is an impossible hostname")
@@ -760,9 +762,17 @@ def sanity_check_names(names):
 
 
 def is_hostname_sane(hostname):
-    """
+    """Make sure the given host name is sane.
+
     Do enough to avoid shellcode from the environment.  There's
     no need to do more.
+
+    :param hostname: Host name to validate
+    :type hostname: str
+
+    :returns: True if hostname is valid, otherwise false.
+    :rtype: bool
+
     """
     # hostnames & IPv4
     allowed = string.ascii_letters + string.digits + "-."

letsencrypt/client/crypto_util.py

@@ -206,39 +206,33 @@ def get_cert_info(filename):
 # A. Do more checks to verify that the CSR is trusted/valid
 # B. Audit the parsing code for vulnerabilities
 
-def valid_csr(csr_filename):
+def valid_csr(csr_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
+    :param csr_file: CSR file contents
+    :type csr_file: str
 
     :returns: Validity of CSR.
     :rtype: bool
 
     """
     try:
-        csr = M2Crypto.X509.load_request(csr_filename)
+        csr = M2Crypto.X509.load_request_string(csr_file)
         return bool(csr.verify(csr.get_pubkey()))
     except M2Crypto.X509.X509Error:
         return False
 
 
-def csr_matches_names(csr_filename, domains):
+def csr_matches_names(csr_file, 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
 
-    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 csr_file: CSR file contents
+    :type csr_file: str
 
     :param domains: Domains the CSR should contain.
     :type domains: list
@@ -248,49 +242,41 @@ def csr_matches_names(csr_filename, domains):
 
     """
     try:
-        csr = M2Crypto.X509.load_request(csr_filename)
+        csr = M2Crypto.X509.load_request_string(csr_file)
         return csr.get_subject().CN in domains
     except M2Crypto.X509.X509Error:
         return False
 
 
-def valid_privkey(privkey_filename):
+def valid_privkey(privkey_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
+    :param privkey_file: Private key file contents
+    :type privkey_file: str
 
     :returns: Validity of private key.
     :rtype: bool
 
     """
     try:
-        return bool(M2Crypto.RSA.load_key(privkey_filename).check_key())
+        return bool(M2Crypto.RSA.load_key_string(privkey_file).check_key())
     except M2Crypto.RSA.RSAError:
         return False
 
 
-def csr_matches_pubkey(csr_filename, privkey_filename):
+def csr_matches_pubkey(csr_file, privkey_file):
     """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.
+    :param csr_file: CSR file contents
+    :type csr_file: str
 
-    TODO: Seems that this doesn not handle X509 eceptions either.
-
-    :param csr_filename: Path to the purported CSR file.
-    :type csr_filename: str
-
-    :param privkey_filename: Path to the purported private key file.
-    :type privkey_filename: str
+    :param privkey_file: Private key file contents
+    :type privkey_file: str
 
     :returns: Correspondence of private key to CSR subject public key.
     :rtype: bool
 
     """
-    csr = M2Crypto.X509.load_request(csr_filename)
-    privkey = M2Crypto.RSA.load_key(privkey_filename)
+    csr = M2Crypto.X509.load_request_string(csr_file)
+    privkey = M2Crypto.RSA.load_key_string(privkey_file)
     return csr.get_pubkey().get_rsa().pub() == privkey.pub()

letsencrypt/scripts/main.py

@@ -28,10 +28,10 @@ def main():
                         nargs="+")
     parser.add_argument("-s", "--server", dest="server",
                         help="The ACME CA server address.")
-    parser.add_argument("-p", "--privkey", dest="privkey", type=open_file,
+    parser.add_argument("-p", "--privkey", dest="privkey", type=read_file,
                         help="Path to the private key file for certificate "
                              "generation.")
-    parser.add_argument("-c", "--csr", dest="csr", type=open_file,
+    parser.add_argument("-c", "--csr", dest="csr", type=read_file,
                         help="Path to the certificate signing request file "
                              "corresponding to the private key file. The "
                              "private key file argument is required if this "
@@ -63,8 +63,8 @@ def main():
 
     try:
         args = parser.parse_args()
-    except IOError as e:
-        parser.error(e)
+    except IOError as exc:
+        parser.error(exc)
 
     # Enforce '--privkey' is set along with '--csr'.
     if args.csr and not args.privkey:
@@ -94,14 +94,17 @@ def main():
         acme.authenticate(args.domains, args.redirect, args.eula)
 
 
-def open_file(filename):
-    """Returns a file object for the given filename.
+def read_file(filename):
+    """Returns the given file's contents with universal new line support.
 
     :param filename: Filename
     :type filename: str
 
-    :return: file object
-    :raise IOError: File does not exist or is not readable.
+    :returns: File contents
+    :rtype: str
+    :raise IOError: File does not exist or is not readable. file() will
+    potentially throw its own IOError exceptions in addition to the two
+    explicitely thrown.
 
     """
 
@@ -111,7 +114,7 @@ def open_file(filename):
     if not os.access(filename, os.R_OK):
         raise IOError("the file '{0}' is not readable".format(filename))
 
-    return file(filename, 'rU')
+    return file(filename, 'rU').read()
 
 
 def rollback(config, checkpoints):