Merge ~pjdc/ubuntu-mirror-charm/+git/ubuntu-mirror-charm:tls into ubuntu-mirror-charm:master

[Paul Collins]

support TLS and listening on specific addresses

This is so that we can avoid opening port 443 on archive.ubuntu.com.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Joel Sing (jsing)]

LGTM, but see comments/questions inline.

review: Approve (+1)

[Paul Collins (pjdc)]

Most comments addressed in upcoming push. Replies inline re ports.conf template.

[Paul Collins (pjdc)]

Pushed new commit to generate ports.conf a little more straightforwardly.

[Joel Sing (jsing)]

LGTM

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 2e6f364321c6f3e58e109b60d18c03905dc351cf

diff --git a/config.yaml b/config.yaml
index 915332b..bd8370e 100644
--- a/config.yaml
+++ b/config.yaml
@@ -14,7 +14,35 @@ options:
   role_map:
     default: ''
     type: string
-    description: 'JSON-formatted list of which type of mirror is enabled on which unit.  Format is: {"hostname1":["cdimage","ubuntu"],"hostname2":["releases"]}'
+    description: >-
+      JSON document describing which mirror is enabled on which machine.
+
+      There are two formats.  The first format is simple:
+
+        {"hostname1": ["cdimage", "ubuntu"], "hostname2": ["releases"]}
+
+      In the second more complex format each hostname's value is a
+      dictionary rather than a list of roles.  For example:
+
+        {
+           "hostname1": {
+              "releases": {
+                 "addresses": ["1.2.3.4"],
+                 "https": true
+              },
+              "ubuntu": null
+           },
+           "hostname2": [
+              "releases",
+              "ubuntu"
+           ]
+        }
+
+      Here, hostname1's ubuntu role will result in an HTTP VirtualHost
+      that listens on *:80 and serves the Ubuntu archive.
+
+      The releases role will result in HTTP and HTTPS VirtualHosts
+      that listen on 1.2.3.4:80 and 1.2.3.4:443 respectively.
   mirror_ubuntu_name:
     default: "archive.ubuntu.com"
     type: string
diff --git a/hooks/hooks.py b/hooks/hooks.py
index 8b47b2c..bc7a9d2 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -8,13 +8,14 @@ import base64
 import json
 import os
 import os.path
+import platform
 import pwd
 import shutil
 import socket
 import subprocess
 import sys
+import types
 import yaml
-import platform
 
 from charmhelpers.core.host import (
     adduser,
@@ -313,6 +314,8 @@ def configure_apache():
 
     wanted_vhosts = []
 
+    all_addresses = set()
+    all_ports = set()
     # Create vhost configs for all active roles
     for role in roles[hostname]:
         try:
@@ -320,11 +323,19 @@ def configure_apache():
         except ValueError:
             log("CHARM: Missing details - skipping {}".format(role))
         else:
+            if isinstance(roles[hostname], types.DictType) and roles[hostname][role]:
+                role_config = roles[hostname][role]
+            else:
+                role_config = {}
             wanted_vhosts.append(role)
             sites_available = os.path.join(available_dir, role + ".conf")
             sites_enabled = os.path.join(enabled_dir, role + ".conf")
             tmpl_data = mirror
             tmpl_data["logdir"] = apache_logdir
+            tmpl_data["addresses"] = role_config.get('addresses', ['*'])
+            all_addresses.update(tmpl_data["addresses"])
+            tmpl_data["ports"] = [80, 443] if role_config.get('https') else [80]
+            all_ports.update(tmpl_data["ports"])
             tmpl_file = str("apache-{}.tmpl".format(role))
             file_from_template(tmpl_file, sites_available, tmpl_data)
             symlink(sites_available, sites_enabled)
@@ -345,6 +356,18 @@ def configure_apache():
             os.chown(mirror["path"], mirror_userinfo.pw_uid, mirror_userinfo.pw_gid)
             symlink(mirror["path"], linkdest)
 
+    # Update ports file
+    all_addresses.discard('*')  # archive.ubuntu.com must not open port 443.
+    file_from_template('apache-listen-ports.conf.tmpl', '/etc/apache2/ports.conf',
+                       {'addresses': sorted(all_addresses), 'ports': sorted(all_ports)})
+
+    # Enable ssl module, if not already enabled
+    restart_for_ssl = False
+    if 443 in all_ports:
+        if check_call(['/usr/sbin/a2query', '-m', 'ssl']) != 0:
+            check_call(['/usr/sbin/a2enmod', 'ssl'])
+            restart_for_ssl = True
+
     # Remove all other vhosts
     unwanted_vhosts = set(conf.role_list()) - set(wanted_vhosts)
     for vhost in unwanted_vhosts:
@@ -356,7 +379,7 @@ def configure_apache():
                 log("CHARM: Unable to delete vhost {}".format(vhost))
 
     # Set up server limits
-    restart_needed = configure_apache_mpm()
+    restart_for_mpm = configure_apache_mpm()
 
     # Configure status module
     tmpl_data = {}
@@ -385,7 +408,7 @@ def configure_apache():
         override_file = '{}/override.conf'.format(systemd_apachedir)
         file_from_template("systemd-apache-override.tmpl", override_file, tmpl_data)
 
-    if restart_needed:
+    if restart_for_mpm or restart_for_ssl:
         log("CHARM: Restarting apache")
         service_restart("apache2")
     else:
diff --git a/templates/apache-cdimage.tmpl b/templates/apache-cdimage.tmpl
index d36cf64..ea841dc 100644
--- a/templates/apache-cdimage.tmpl
+++ b/templates/apache-cdimage.tmpl
@@ -1,4 +1,6 @@
-<VirtualHost *:80>
+#for $port in $ports
+#set $sockets = ' '.join(sorted(['{}:{}'.format(address, port) for address in $addresses]))
+<VirtualHost ${sockets}>
 ${apache_early_extra}
     ServerName ${name}
     #for $alias in $aliases
@@ -8,6 +10,13 @@ ${apache_early_extra}
     CustomLog ${logdir}/${name}-access.log combined
     ErrorLog ${logdir}/${name}-error.log
 
+#if $port == 443
+    SSLEngine On
+    SSLCertificateFile /etc/apache2/ssl/${name}.crt
+    SSLCertificateKeyFile /etc/apache2/ssl/${name}.key
+    SSLCertificateChainFile /etc/apache2/ssl/${name}_chain.crt
+
+#end if
     DocumentRoot /srv/${name}/www
     Alias /cdimage "/srv/${name}/www/"
 
@@ -21,3 +30,5 @@ ${apache_early_extra}
     </Directory>
 ${apache_late_extra}
 </VirtualHost>
+
+#end for
diff --git a/templates/apache-cloud-images.tmpl b/templates/apache-cloud-images.tmpl
index 9c59a8b..04f11cc 100644
--- a/templates/apache-cloud-images.tmpl
+++ b/templates/apache-cloud-images.tmpl
@@ -1,4 +1,6 @@
-<VirtualHost *:80>
+#for $port in $ports
+#set $sockets = ' '.join(sorted(['{}:{}'.format(address, port) for address in $addresses]))
+<VirtualHost ${sockets}>
 ${apache_early_extra}
     ServerName ${name}
     #for $alias in $aliases
@@ -9,6 +11,13 @@ ${apache_early_extra}
     ErrorLog ${logdir}/${name}-error.log
     LogFormat "%h %D %u %t \"%r\" %>s %O \"%{Referer}i\" \"%{User-Agent}i\"" combined-ext
 
+#if $port == 443
+    SSLEngine On
+    SSLCertificateFile /etc/apache2/ssl/${name}.crt
+    SSLCertificateKeyFile /etc/apache2/ssl/${name}.key
+    SSLCertificateChainFile /etc/apache2/ssl/${name}_chain.crt
+
+#end if
     DocumentRoot /srv/${name}/www
     Alias /cloud-images "/srv/${name}/www/"
 
@@ -22,3 +31,5 @@ ${apache_early_extra}
     </Directory>
 ${apache_late_extra}
 </VirtualHost>
+
+#end for
diff --git a/templates/apache-listen-ports.conf.tmpl b/templates/apache-listen-ports.conf.tmpl
new file mode 100644
index 0000000..7a633b9
--- /dev/null
+++ b/templates/apache-listen-ports.conf.tmpl
@@ -0,0 +1,15 @@
+#if 80 in $ports
+Listen 80
+
+#end if
+#if len($addresses) > 0 and len($ports) > 1
+# We only want to listen for non-HTTP on specific addresses
+# so that we don't accidentally expose port 443 for archive.
+#for $port in $ports
+#if $port != 80
+#for $address in $addresses
+Listen ${address}:${port}
+#end for
+#end if
+#end for
+#end if
diff --git a/templates/apache-ports.tmpl b/templates/apache-ports.tmpl
index 4ce54db..f24823e 100644
--- a/templates/apache-ports.tmpl
+++ b/templates/apache-ports.tmpl
@@ -1,4 +1,6 @@
-<VirtualHost *:80>
+#for $port in $ports
+#set $sockets = ' '.join(sorted(['{}:{}'.format(address, port) for address in $addresses]))
+<VirtualHost ${sockets}>
 ${apache_early_extra}
     ServerName ${name}
     #for $alias in $aliases
@@ -8,6 +10,13 @@ ${apache_early_extra}
     CustomLog ${logdir}/${name}-access.log combined
     ErrorLog ${logdir}/${name}-error.log
 
+#if $port == 443
+    SSLEngine On
+    SSLCertificateFile /etc/apache2/ssl/${name}.crt
+    SSLCertificateKeyFile /etc/apache2/ssl/${name}.key
+    SSLCertificateChainFile /etc/apache2/ssl/${name}_chain.crt
+
+#end if
     DocumentRoot /srv/${name}/www
     Alias /cdimage "/srv/${name}/www/"
 
@@ -20,3 +29,5 @@ ${apache_early_extra}
     </Directory>
 ${apache_late_extra}
 </VirtualHost>
+
+#end for
diff --git a/templates/apache-releases.tmpl b/templates/apache-releases.tmpl
index 730d30c..aeae97c 100644
--- a/templates/apache-releases.tmpl
+++ b/templates/apache-releases.tmpl
@@ -1,4 +1,6 @@
-<VirtualHost *:80>
+#for $port in $ports
+#set $sockets = ' '.join(sorted(['{}:{}'.format(address, port) for address in $addresses]))
+<VirtualHost ${sockets}>
 ${apache_early_extra}
     ServerName ${name}
     #for $alias in $aliases
@@ -9,6 +11,13 @@ ${apache_early_extra}
     ErrorLog ${logdir}/${name}-error.log
     LogFormat "%h %D %u %t \"%r\" %>s %O \"%{Referer}i\" \"%{User-Agent}i\"" combined-ext
 
+#if $port == 443
+    SSLEngine On
+    SSLCertificateFile /etc/apache2/ssl/${name}.crt
+    SSLCertificateKeyFile /etc/apache2/ssl/${name}.key
+    SSLCertificateChainFile /etc/apache2/ssl/${name}_chain.crt
+
+#end if
     DocumentRoot /srv/${name}/www
     Alias /releases "/srv/${name}/www/"
 
@@ -22,3 +31,5 @@ ${apache_early_extra}
     </Directory>
 ${apache_late_extra}
 </VirtualHost>
+
+#end for
diff --git a/templates/apache-simple-streams.tmpl b/templates/apache-simple-streams.tmpl
index ea4a16d..8e4b613 100644
--- a/templates/apache-simple-streams.tmpl
+++ b/templates/apache-simple-streams.tmpl
@@ -1,4 +1,6 @@
-<VirtualHost *:80>
+#for $port in $ports
+#set $sockets = ' '.join(sorted(['{}:{}'.format(address, port) for address in $addresses]))
+<VirtualHost ${sockets}>
 ${apache_early_extra}
     ServerName ${name}
     #for $alias in $aliases
@@ -9,6 +11,13 @@ ${apache_early_extra}
     ErrorLog ${logdir}/${name}-error.log
     LogFormat "%h %D %u %t \"%r\" %>s %O \"%{Referer}i\" \"%{User-Agent}i\"" combined-ext
 
+#if $port == 443
+    SSLEngine On
+    SSLCertificateFile /etc/apache2/ssl/${name}.crt
+    SSLCertificateKeyFile /etc/apache2/ssl/${name}.key
+    SSLCertificateChainFile /etc/apache2/ssl/${name}_chain.crt
+
+#end if
     DocumentRoot /srv/${name}/www
     Alias /simple-streams "/srv/${name}/www/"
 
@@ -22,3 +31,5 @@ ${apache_early_extra}
     </Directory>
 ${apache_late_extra}
 </VirtualHost>
+
+#end for
diff --git a/templates/apache-ubuntu.tmpl b/templates/apache-ubuntu.tmpl
index bfe69e6..0887271 100644
--- a/templates/apache-ubuntu.tmpl
+++ b/templates/apache-ubuntu.tmpl
@@ -1,4 +1,6 @@
-<VirtualHost *:80>
+#for $port in $ports
+#set $sockets = ' '.join(sorted(['{}:{}'.format(address, port) for address in $addresses]))
+<VirtualHost ${sockets}>
 ${apache_early_extra}
     ServerName ${name}
     #for $alias in $aliases
@@ -8,6 +10,13 @@ ${apache_early_extra}
     CustomLog ${logdir}/${name}-access.log combined
     ErrorLog ${logdir}/${name}-error.log
 
+#if $port == 443
+    SSLEngine On
+    SSLCertificateFile /etc/apache2/ssl/${name}.crt
+    SSLCertificateKeyFile /etc/apache2/ssl/${name}.key
+    SSLCertificateChainFile /etc/apache2/ssl/${name}_chain.crt
+
+#end if
     DocumentRoot /srv/${name}/www
     # Munge the Cache-Control/Expires response headers to try make
     # transparent proxy servers DTRT.
@@ -39,3 +48,5 @@ ${apache_early_extra}
     </Directory>
 ${apache_late_extra}
 </VirtualHost>
+
+#end for