From f046db1d174073b7c775b6c3e6249ce69845246e Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 25 Nov 2014 03:05:50 -0800 Subject: [PATCH] Added TODOs and a warning --- letsencrypt/client/acme.py | 2 +- letsencrypt/client/apache_configurator.py | 5 ++ letsencrypt/client/client.py | 68 ++++++----------------- letsencrypt/client/crypto_util.py | 14 +++-- letsencrypt/scripts/main.py | 14 ++--- 5 files changed, 38 insertions(+), 65 deletions(-) diff --git a/letsencrypt/client/acme.py b/letsencrypt/client/acme.py index 392546b29..9384abadb 100644 --- a/letsencrypt/client/acme.py +++ b/letsencrypt/client/acme.py @@ -21,7 +21,7 @@ SCHEMATA = dict([ "error", "revocation", "revocationRequest", - "statusRequest", + "statusRequest" ] ]) diff --git a/letsencrypt/client/apache_configurator.py b/letsencrypt/client/apache_configurator.py index 54b16af13..f497dd763 100644 --- a/letsencrypt/client/apache_configurator.py +++ b/letsencrypt/client/apache_configurator.py @@ -1240,6 +1240,11 @@ DocumentRoot " + CONFIG.CONFIG_DIR + "challenge_page/ \n \ result: Apache config includes virtual servers for issued challs """ + # WARNING: THIS IS A POTENTIAL SECURITY VULNERABILITY + # THIS SHOULD BE HANDLED BY THE PACKAGE MANAGER + # AND TAKEN OUT BEFORE RELEASE, INSTEAD + # SHOWING A NICE ERROR MESSAGE ABOUT THE PROBLEM + # Check to make sure options-ssl.conf is installed if not os.path.isfile(CONFIG.OPTIONS_SSL_CONF): dist_conf = pkg_resources.resource_filename( diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index aeaaf8762..a0e9b7ff9 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -47,8 +47,14 @@ class Client(object): self.csr_file = cert_signing_request self.key_file = private_key - self._validate_csr_key_cli() - + # TODO: Figure out all exceptions from this function + try: + self._validate_csr_key_cli() + except Exception as e: + # TODO: Something nice here... + logger.fatal(("%s - until the programmers get their act together, " + "we are just going to exit" % str(e))) + sys.exit(1) self.server_url = "https://%s/acme/" % self.server def authenticate(self, domains=None, redirect=None, eula=False): @@ -88,8 +94,10 @@ class Client(object): # Get key and csr to perform challenges _, csr_der = self.get_key_csr_pem() + # TODO: Handle this exception/problem if not crypto_util.csr_matches_names(self.csr_file, self.names): - raise Exception("CSR subject does not contain the specified name") + raise Exception(("CSR subject does not contain one of the " + "specified names")) # Perform Challenges responses, challenge_objs = self.verify_identity(challenge_msg) @@ -650,6 +658,10 @@ class Client(object): correspond to one another. """ + # TODO: Handle all of these problems appropriately + # 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 " @@ -658,15 +670,13 @@ class Client(object): # If CSR is provided, it must be readable and valid. try: 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) + raise Exception("The provided CSR is not a valid CSR") 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) + raise Exception("The provided key is not a valid key") except IOError: raise Exception("The provided key could not be read") @@ -680,50 +690,6 @@ class Client(object): except IOError: raise Exception("The key or CSR files could not be read") - # def choice_of_ca(self): - # choices = self.get_cas() - # message = ("Pick a Certificate Authority. " - # "They're all unique and special!") - # in_txt = ("Enter the number of a Certificate Authority " - # "(c to cancel): ") - # code, selection = display.generic_menu(message, choices, in_txt) - - # if code != display.OK: - # sys.exit(0) - - # return selection - - # Legacy Code: Although I would like to see a free and open marketplace - # in the future. The Let's Encrypt Client will not have this feature at - # launch - # def get_cas(self): - # DV_choices = [] - # OV_choices = [] - # EV_choices = [] - # choices = [] - # try: - # with open("/etc/letsencrypt/.ca_offerings") as f: - # for line in f: - # choice = line.split(";", 1) - # if 'DV' in choice[0]: - # DV_choices.append(choice) - # elif 'OV' in choice[0]: - # OV_choices.append(choice) - # else: - # EV_choices.append(choice) - - # # random.shuffle(DV_choices) - # # random.shuffle(OV_choices) - # # random.shuffle(EV_choices) - # choices = DV_choices + OV_choices + EV_choices - # choices = [(l[0], l[1]) for l in choices] - - # except IOError as e: - # logger.fatal("Unable to find .ca_offerings file") - # sys.exit(1) - - # return choices - def get_all_names(self): """Return all valid names in the configuration.""" names = list(self.config.get_all_names()) diff --git a/letsencrypt/client/crypto_util.py b/letsencrypt/client/crypto_util.py index b874ff691..2dd291d9c 100644 --- a/letsencrypt/client/crypto_util.py +++ b/letsencrypt/client/crypto_util.py @@ -14,6 +14,8 @@ from letsencrypt.client import le_util from letsencrypt.client import logger +# TODO: All of these functions need unit tests + def b64_cert_to_pem(b64_der_cert): return M2Crypto.X509.load_cert_der_string( le_util.jose_b64decode(b64_der_cert)).as_pem() @@ -84,11 +86,11 @@ def make_key(bits=CONFIG.RSA_KEY_SIZE): """ Returns new RSA key in PEM form with specified bits """ - #Python Crypto module doesn't produce any stdout + # Python Crypto module doesn't produce any stdout key = Crypto.PublicKey.RSA.generate(bits) - #rsa = M2Crypto.RSA.gen_key(bits, 65537) - #key_pem = rsa.as_pem(cipher=None) - #rsa = None # should not be freed here + # rsa = M2Crypto.RSA.gen_key(bits, 65537) + # key_pem = rsa.as_pem(cipher=None) + # rsa = None # should not be freed here return key.exportKey(format='PEM') @@ -157,7 +159,7 @@ def make_ss_cert(key_file, domains): cert.set_issuer(cert.get_subject()) cert.add_ext(M2Crypto.X509.new_extension('basicConstraints', 'CA:FALSE')) - #cert.add_ext(M2Crypto.X509.new_extension( + # cert.add_ext(M2Crypto.X509.new_extension( # 'extendedKeyUsage', 'TLS Web Server Authentication')) cert.add_ext(M2Crypto.X509.new_extension( 'subjectAltName', ", ".join(["DNS:%s" % d for d in domains]))) @@ -165,7 +167,7 @@ def make_ss_cert(key_file, domains): cert.sign(pubkey, 'sha256') assert cert.verify(pubkey) assert cert.verify() - #print check_purpose(,0 + # print check_purpose(,0 return cert.as_pem() diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index e4f63a239..07fa6054d 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -102,21 +102,21 @@ def usage(): def print_options(): print ("\nsudo ./letsencrypt.py " "(default authentication mode using pythondialog)") - options = ["privkey= (specify key file to use to generate the " + + options = [("privkey= (specify key file to use to generate the " "certificate)", - "csr= (Use a specific CSR. If this is specified, privkey " + - "must also be specified with the correct" + + "csr= (Use a specific CSR. If this is specified, privkey " + "must also be specified with the correct" " private key for the CSR)", "server (list the ACME CA server address)", "revoke (revoke a certificate)", - "view-checkpoints (Used to view available checkpoints and " + + "view-checkpoints (Used to view available checkpoints and " "see what configuration changes have been made)", "rollback=X (Revert the configuration X number of checkpoints)", - "redirect (Automatically redirect all HTTP traffic to " + + "redirect (Automatically redirect all HTTP traffic to " "HTTPS for the newly authenticated vhost)", - "no-redirect (Skip the HTTPS redirect question, " + + "no-redirect (Skip the HTTPS redirect question, " "allowing both HTTP and HTTPS)", - "agree-eula (Skip the end user agreement screen)"] + "agree-eula (Skip the end user agreement screen))")] for o in options: print " --%s" % o sys.exit(0)