From e5a027ce307702adb700a6478398f00f3b0dcb21 Mon Sep 17 00:00:00 2001 From: yan Date: Fri, 10 Apr 2015 15:21:25 -0700 Subject: [PATCH] Make nginx deploy_cert --- .../client/plugins/nginx/configurator.py | 57 +++------ letsencrypt/client/plugins/nginx/parser.py | 120 +++++++++++------- 2 files changed, 87 insertions(+), 90 deletions(-) diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py index ba4fc77e2..2d73196ef 100644 --- a/letsencrypt/client/plugins/nginx/configurator.py +++ b/letsencrypt/client/plugins/nginx/configurator.py @@ -100,18 +100,11 @@ class NginxConfigurator(object): # Entry point in main.py for installing cert def deploy_cert(self, domain, cert, key, cert_chain=None): - """Deploys certificate to specified virtual host. + """Deploys certificate to specified virtual host. Aborts if the + vhost is missing ssl_certificate or ssl_certificate_key. - Currently tries to find the last directives to deploy the cert in - the VHost associated with the given domain. If it can't find the - directives, it searches the "included" confs. The function verifies that - it has located the three directives and finally modifies them to point - to the correct destination. - - .. todo:: Make sure last directive is changed - - .. todo:: Might be nice to remove chain directive if none exists - This shouldn't happen within letsencrypt though + Nginx doesn't have a cert chain directive, so the last parameter is + always ignored. It expects the cert file to have the concatenated chain. :param str domain: domain to deploy certificate :param str cert: certificate filename @@ -120,44 +113,26 @@ class NginxConfigurator(object): """ vhost = self.choose_vhost(domain) - path = {} + directives = [['ssl_certificate', cert], ['ssl_certificate_key', key]] - path["cert_file"] = self.parser.find_dir(parser.case_i( - "SSLCertificateFile"), None, vhost.path) - path["cert_key"] = self.parser.find_dir(parser.case_i( - "SSLCertificateKeyFile"), None, vhost.path) - - # Only include if a certificate chain is specified - if cert_chain is not None: - path["cert_chain"] = self.parser.find_dir( - parser.case_i("SSLCertificateChainFile"), None, vhost.path) - - if len(path["cert_file"]) == 0 or len(path["cert_key"]) == 0: - # Throw some can't find all of the directives error" + try: + self.parser.add_server_directives(vhost.filep, vhost.names, + directives, True) + logging.info("Deployed Certificate to VirtualHost %s for %s", + vhost.filep, vhost.names) + except errors.LetsEncryptMisconfigurationError: logging.warn( - "Cannot find a cert or key directive in %s", vhost.path) + "Cannot find a cert or key directive in %s for %s", + vhost.filep, vhost.names) logging.warn("VirtualHost was not modified") # Presumably break here so that the virtualhost is not modified return False - logging.info("Deploying Certificate to VirtualHost %s", vhost.filep) - - self.aug.set(path["cert_file"][0], cert) - self.aug.set(path["cert_key"][0], key) - if cert_chain is not None: - if len(path["cert_chain"]) == 0: - self.parser.add_dir( - vhost.path, "SSLCertificateChainFile", cert_chain) - else: - self.aug.set(path["cert_chain"][0], cert_chain) - self.save_notes += ("Changed vhost at %s with addresses of %s\n" % (vhost.filep, ", ".join(str(addr) for addr in vhost.addrs))) - self.save_notes += "\tSSLCertificateFile %s\n" % cert - self.save_notes += "\tSSLCertificateKeyFile %s\n" % key - if cert_chain: - self.save_notes += "\tSSLCertificateChainFile %s\n" % cert_chain + self.save_notes += "\tssl_certificate %s\n" % cert + self.save_notes += "\tssl_certificate_key %s\n" % key ####################### # Vhost parsing methods @@ -291,7 +266,7 @@ class NginxConfigurator(object): filename, names, [['listen', '443 ssl'], ['ssl_certificate', '/etc/ssl/certs/ssl-cert-snakeoil.pem'], - ['ssl_key', '/etc/ssl/private/ssl-cert-snakeoil.key'], + ['ssl_certificate_key', '/etc/ssl/private/ssl-cert-snakeoil.key'], ['include', self.parser.loc["ssl_options"]]]) def get_all_certs_keys(self): diff --git a/letsencrypt/client/plugins/nginx/parser.py b/letsencrypt/client/plugins/nginx/parser.py index 4ff29962c..39cdc09d6 100644 --- a/letsencrypt/client/plugins/nginx/parser.py +++ b/letsencrypt/client/plugins/nginx/parser.py @@ -81,7 +81,8 @@ class NginxParser(object): :rtype: bool """ - return (entry[0] == 'include' and len(entry) == 2 and + return (type(entry) == list and + entry[0] == 'include' and len(entry) == 2 and type(entry[1]) == str) def get_vhosts(self): @@ -214,39 +215,6 @@ class NginxParser(object): raise errors.LetsEncryptNoInstallationError( "Could not find configuration root") - def add_dir(self, aug_conf_path, directive, arg): - """Appends directive to the end fo the file given by aug_conf_path. - - .. note:: Not added to AugeasConfigurator because it may depend - on the lens - - :param str aug_conf_path: Augeas configuration path to add directive - :param str directive: Directive to add - :param str arg: Value of the directive. ie. Listen 443, 443 is arg - - """ - pass - - def find_dir(self, directive, arg=None, start=None): - """Finds directive in the configuration. - - Recursively searches through config files to find directives - - .. todo:: Add order to directives returned. Last directive comes last.. - .. todo:: arg should probably be a list - - :param str directive: Directive to look for - - :param arg: Specific value directive must have, None if all should - be considered - :type arg: str or None - - :param str start: Beginning Augeas path to begin looking - :rtype: list - - """ - return [] - def filedump(self, ext='tmp'): """Dumps parsed configurations into files. @@ -264,29 +232,83 @@ class NginxParser(object): except IOError: logging.error("Could not open file for writing: %s" % filename) - def add_server_directives(self, filename, names, directives): - """Adds directives to a server block whose server_name set is 'names'. + def _has_server_names(self, entry, names): + """Checks if a server block has the given set of server_names. This + is the primary way of identifying server blocks in the configurator. + Returns false if 'entry' doesn't look like a server block at all. - :param str filename: The absolute filename of the config file - :param str names: The server_name to match - :param list directives: The directives to add + ..todo :: Doesn't match server blocks whose server_name directives are + split across multiple conf files. + + :param list entry: The block to search + :param set names: The names to match + :rtype: bool """ if len(names) == 0: # Nothing to identify blocks with return False - def has_server_names(entry): - # Checks if a server block has the given names - # TODO: Make this work if some of the names are in included files - server_names = set() - for item in entry: - if item[0] == 'server_name': - server_names.update((' ').split(item[1])) - return server_names == names + if type(entry) != list: + # Can't be a server block + return False - do_for_subarray(self.parsed[filename], lambda x: has_server_names(x), - lambda x: x.extend(directives)) + server_names = set() + for item in entry: + if type(item) != list: + # Can't be a server block + return False + + if item[0] == 'server_name': + server_names.update((' ').split(item[1])) + + return server_names == names + + def _replace_directives(self, block, directives): + """Replaces directives in a block. If the directive doesn't exist in + the entry already, raises a misconfiguration error. + + ..todo :: Find directives that are in included files. + + :param list block: The block to replace in + :param list directives: The new directives. + """ + for directive in directives: + changed = False + if len(directive) == 0: + continue + for line in block: + if len(line) > 0 and line[0] == directive[0]: + line = directive + changed = True + if not changed: + raise errors.LetsEncryptMisconfigurationError( + 'LetsEncrypt expected directive for %s in the Nginx config ' + 'but did not find it.' % directive[0]) + + def add_server_directives(self, filename, names, directives, + replace=False): + """Add or replace directives in server blocks whose server_name set + is 'names'. If replace is True, this raises a misconfiguration error + if the directive does not already exist. + + ..todo :: Doesn't match server blocks whose server_name directives are + split across multiple conf files. + + :param str filename: The absolute filename of the config file + :param str names: The server_name to match + :param list directives: The directives to add + :param bool replace: Whether to only replace existing directives + + """ + if replace: + do_for_subarray(self.parsed[filename], + lambda x: self._has_server_names(x, names), + lambda x: self._replace_directives(x, directives)) + else: + do_for_subarray(self.parsed[filename], + lambda x: self._has_server_names(x, names), + lambda x: x.extend(directives)) def do_for_subarray(entry, condition, func):