From 13232452f8041603213f2dde8daba2547446227b Mon Sep 17 00:00:00 2001 From: yan Date: Mon, 6 Apr 2015 18:00:21 -0700 Subject: [PATCH] Add recursive 'include' parsing to nginx parser --- .../client/plugins/nginx/configurator.py | 21 +--- letsencrypt/client/plugins/nginx/dvsni.py | 4 +- .../client/plugins/nginx/nginxparser.py | 2 +- letsencrypt/client/plugins/nginx/obj.py | 9 +- letsencrypt/client/plugins/nginx/parser.py | 113 ++++++++++++++++-- .../plugins/nginx/tests/testdata/nginx.conf | 6 +- .../nginx/tests/testdata/nginx.new.conf | 58 --------- 7 files changed, 117 insertions(+), 96 deletions(-) diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py index 64d07d717..a15d42eb2 100644 --- a/letsencrypt/client/plugins/nginx/configurator.py +++ b/letsencrypt/client/plugins/nginx/configurator.py @@ -97,7 +97,7 @@ class NginxConfigurator(object): self.version = self._get_version() # Get all of the available vhosts - self.vhosts = self._get_vhosts() + self.vhosts = self.parser._get_vhosts() temp_install(self.config.nginx_mod_ssl_conf) @@ -239,25 +239,6 @@ class NginxConfigurator(object): return all_names - # TODO: make "sites-available" a configurable directory - def _get_vhosts(self): - """Returns list of virtual hosts found in the Nginx configuration. - - :returns: List of - :class:`~letsencrypt.client.plugins.nginx.obj.VirtualHost` objects - found in configuration - :rtype: list - - """ - # Search sites-available/, conf.d/, nginx.conf for possible vhosts - paths = self.parser.get_conf_files() - vhs = [] - - for path in paths: - vhs.append(self.parser.get_vhosts(path)) - - return vhs - def _make_vhost_ssl(self, nonssl_vhost): # pylint: disable=too-many-locals """Makes an ssl_vhost version of a nonssl_vhost. diff --git a/letsencrypt/client/plugins/nginx/dvsni.py b/letsencrypt/client/plugins/nginx/dvsni.py index 960352831..c20ce1c0e 100644 --- a/letsencrypt/client/plugins/nginx/dvsni.py +++ b/letsencrypt/client/plugins/nginx/dvsni.py @@ -42,6 +42,7 @@ class NginxDvsni(object): """ + def __init__(self, configurator): self.configurator = configurator self.achalls = [] @@ -83,9 +84,6 @@ class NginxDvsni(object): logging.error("Please specify servernames in the Nginx config") return None - # TODO - @jdkasten review this code to make sure it makes sense - self.configurator.make_server_sni_ready(vhost, default_addr) - for addr in vhost.addrs: if "_default_" == addr.get_addr(): addresses.append([default_addr]) diff --git a/letsencrypt/client/plugins/nginx/nginxparser.py b/letsencrypt/client/plugins/nginx/nginxparser.py index 3d01d7ad4..2182ef6a7 100644 --- a/letsencrypt/client/plugins/nginx/nginxparser.py +++ b/letsencrypt/client/plugins/nginx/nginxparser.py @@ -1,4 +1,4 @@ -"""An nginx config parser based on pyparsing.""" +"""Very low-level nginx config parser based on pyparsing.""" import string from pyparsing import ( diff --git a/letsencrypt/client/plugins/nginx/obj.py b/letsencrypt/client/plugins/nginx/obj.py index 69e0d6b20..85a7fa003 100644 --- a/letsencrypt/client/plugins/nginx/obj.py +++ b/letsencrypt/client/plugins/nginx/obj.py @@ -47,7 +47,6 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods """Represents an Nginx Virtualhost. :ivar str filep: file path of VH - :ivar str path: Augeas path to virtual host :ivar set addrs: Virtual Host addresses (:class:`set` of :class:`Addr`) :ivar set names: Server names/aliases of vhost (:class:`list` of :class:`str`) @@ -57,11 +56,10 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods """ - def __init__(self, filep, path, addrs, ssl, enabled, names=None): + def __init__(self, filep, addrs, ssl, enabled, names=None): # pylint: disable=too-many-arguments """Initialize a VH.""" self.filep = filep - self.path = path self.addrs = addrs self.names = set() if names is None else set(names) self.ssl = ssl @@ -74,16 +72,15 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods def __str__(self): addr_str = ", ".join(str(addr) for addr in self.addrs) return ("file: %s\n" - "vh_path: %s\n" "addrs: %s\n" "names: %s\n" "ssl: %s\n" - "enabled: %s" % (self.filep, self.path, addr_str, + "enabled: %s" % (self.filep, addr_str, self.names, self.ssl, self.enabled)) def __eq__(self, other): if isinstance(other, self.__class__): - return (self.filep == other.filep and self.path == other.path and + return (self.filep == other.filep and self.addrs == other.addrs and self.names == other.names and self.ssl == other.ssl and self.enabled == other.enabled) diff --git a/letsencrypt/client/plugins/nginx/parser.py b/letsencrypt/client/plugins/nginx/parser.py index dfb091881..6fc5bef53 100644 --- a/letsencrypt/client/plugins/nginx/parser.py +++ b/letsencrypt/client/plugins/nginx/parser.py @@ -6,6 +6,7 @@ import re import pyparsing from letsencrypt.client import errors +from letsencrypt.client.plugins.nginx import obj from letsencrypt.client.plugins.nginx.nginxparser import dump, load @@ -22,11 +23,101 @@ class NginxParser(object): self.parsed = {} self.root = os.path.abspath(root) self.loc = self._set_locations(ssl_options) - self._parse_file(self.loc["root"]) - # Must also attempt to parse sites-available or equivalent - # Sites-available is not included naturally in configuration - self._parse_file(os.path.join(self.root, "sites-available") + "/*.conf") + # Parse nginx.conf and included files. + # TODO: Check sites-available/ as well. For now, the configurator does + # not enable sites from there. + self._parse_recursively(self.loc["root"]) + + def _parse_recursively(self, filepath): + """Parses nginx config files recursively by looking at 'include' + directives inside 'http' and 'server' blocks. Note that this only + reads Nginx files that potentially declare a virtual host. + + .. todo:: Can Nginx 'virtual hosts' be defined somewhere other than in + the server context? + + """ + trees = self._parse_files(filepath) + for tree in trees: + for entry in tree: + if self._is_include_directive(entry): + # Parse the top-level included file + self._parse_recursively(entry[1]) + elif entry[0] == ['http'] or entry[0] == ['server']: + # Look for includes in the top-level 'http'/'server' context + for subentry in entry[1]: + if self._is_include_directive(subentry): + self._parse_recursively(subentry[1]) + elif entry[0] == ['http'] and subentry[0] == ['server']: + # Look for includes in a 'server' context within + # an 'http' context + for server_entry in subentry[1]: + if self._is_include_directive(server_entry): + self._parse_recursively(server_entry[1]) + + def _is_include_directive(self, entry): + """Checks if an nginx parsed entry is an 'include' directive. + + :param list entry: the parsed entry + :returns: Whether it's an 'include' directive + :rtype: bool + + """ + return (entry[0] == 'include' and len(entry) == 2 and + type(entry[1]) == str) + + def _get_names(self, entry): + """Gets server names from nginx parsed entry. + + :param list entry: the parsed entry + :returns: Set of server names + :rtype: set + + """ + return set() + + def _get_addrs(self, entry): + """Gets addresses from nginx parsed entry. + + :param list entry: the parsed entry + :returns: Set of + :class:`~letsencrypt.client.plugins.nginx.obj.Addr` objects + :rtype: set + + """ + return set() + + def _get_ssl(self, entry): + """Gets whether the nginx parsed entry is SSL-enabled. + + :param list entry: the parsed entry + :returns: Whether it's SSL-enabled + :rtype: bool + + """ + return False + + def get_vhosts(self): + """Gets list of all 'virtual hosts' found in Nginx configuration. + Technically this is a misnomer because Nginx does not have virtual + hosts, it has 'server blocks'. + + :returns: List of + :class:`~letsencrypt.client.plugins.nginx.obj.VirtualHost` objects + found in configuration + :rtype: list + + """ + enabled = True # We only look at enabled vhosts for now + vhosts = [] + for filename, tree in self.parsed: + vhost = obj.VirtulHost(filename, + self._get_addrs(tree), + self._get_ssl(tree), + enabled, + self._get_names(tree)) + vhosts.append(vhost) def add_dir_to_ifmodssl(self, aug_conf_path, directive, val): """Adds directive and value to IfMod ssl block. @@ -200,7 +291,7 @@ class NginxParser(object): # TODO: Test if Nginx allows ../ or ~/ for Includes # Attempts to add a transform to the file if one does not already exist - self._parse_file(arg) + self._parse_files(arg) # Argument represents an fnmatch regular expression, convert it # Split up the path and convert each into an Augeas accepted regex @@ -246,20 +337,28 @@ class NginxParser(object): regex = regex + letter return regex - def _parse_file(self, filepath): + def _parse_files(self, filepath): """Parse file :param str filepath: Nginx config file path + :returns: list of parsed tree structures + :rtype: list """ files = glob.glob(filepath) + trees = [] for f in files: + if f in self.parsed: + continue try: - self.parsed[f] = load(open(f)) + parsed = load(open(f)) + self.parsed[f] = parsed + trees.append(parsed) except IOError: logging.warn("Could not parse file: %s" % f) except pyparsing.ParseException: logging.warn("Could not parse file: %s" % f) + return trees def _add_httpd_transform(self, incl): """Add a transform to Augeas. diff --git a/letsencrypt/client/plugins/nginx/tests/testdata/nginx.conf b/letsencrypt/client/plugins/nginx/tests/testdata/nginx.conf index 057edba6f..67566604e 100644 --- a/letsencrypt/client/plugins/nginx/tests/testdata/nginx.conf +++ b/letsencrypt/client/plugins/nginx/tests/testdata/nginx.conf @@ -14,6 +14,7 @@ events { worker_connections 1024; } +include foo.conf http { include mime.types; @@ -84,7 +85,7 @@ http { server { listen 8000; listen somename:8080; - server_name somename alias another.alias; + include server.conf; location / { root html; @@ -114,4 +115,7 @@ http { # } #} + include conf.d/test.conf; + include sites-enabled/*; + } diff --git a/letsencrypt/client/plugins/nginx/tests/testdata/nginx.new.conf b/letsencrypt/client/plugins/nginx/tests/testdata/nginx.new.conf index 610ded391..210861593 100644 --- a/letsencrypt/client/plugins/nginx/tests/testdata/nginx.new.conf +++ b/letsencrypt/client/plugins/nginx/tests/testdata/nginx.new.conf @@ -6,64 +6,6 @@ error_log logs/error.log info; pid logs/nginx.pid; events { worker_connections 1024; -} -http { - include mime.types; - default_type application/octet-stream; - log_format main '$remote_addr - $remote_user [$time_local] "$request" ' - '$status $body_bytes_sent "$http_referer" ' - '"$http_user_agent" "$http_x_forwarded_for"'; - access_log logs/access.log main; - sendfile on; - tcp_nopush on; - keepalive_timeout 0; - keepalive_timeout 65; - gzip on; - - server { - listen 8080; - server_name localhost; - charset koi8-r; - access_log logs/host.access.log main; - - location / { - root html; - index index.html index.htm; - } - error_page 404 /404.html; - error_page 500 502 503 504 /50x.html; - - location = /50x.html { - root html; - } - - location ~ \.php$ { - proxy_pass http://127.0.0.1; - } - - location ~ \.php$ { - root html; - fastcgi_pass 127.0.0.1:9000; - fastcgi_index index.php; - fastcgi_param SCRIPT_FILENAME /scripts$fastcgi_script_name; - include fastcgi_params; - } - - location ~ /\.ht { - deny all; - } - } - - server { - listen 8000; - listen somename:8080; - server_name somename alias another.alias; - - location / { - root html; - index index.html index.htm; - } - } server { listen 443 ssl;