software/erp5: use a caucase managed certificate for balancer

Since 0.9.6 caucase stopped using the 128bits OID arc that caddy/golang does
not support, so nothing prevents us from using a caucase certiciate now.

software/erp5/test/setup.py

@@ -50,6 +50,7 @@ setup(name=name,
         'mysqlclient',
         'backports.lzma',
         'cryptography',
+        'pexpect',
         'pyOpenSSL',
         'typing; python_version<"3"',
       ],

software/erp5/test/test/test_balancer.py

@@ -5,15 +5,18 @@ import logging
 import os
 import re
 import shutil
+import socket
 import subprocess
 import tempfile
 import time
 import urlparse
 from BaseHTTPServer import BaseHTTPRequestHandler
-from typing import Dict
+from typing import Any, Dict, Optional
 
+import idna
 import mock
 import OpenSSL.SSL
+import pexpect
 import requests
 from cryptography import x509
 from cryptography.hazmat.backends import default_backend
@@ -102,6 +105,20 @@ class CaucaseService(ManagedResource):
     self._caucased_process.wait()
     shutil.rmtree(self.directory)
 
+  @property
+  def ca_crt_path(self):
+    # type: () -> str
+    """Path of the CA certificate from this caucase.
+    """
+    ca_crt_path = os.path.join(self.directory, 'ca.crt.pem')
+    if not os.path.exists(ca_crt_path):
+      with open(ca_crt_path, 'w') as f:
+        f.write(
+            requests.get(urlparse.urljoin(
+                self.url,
+                '/cas/crt/ca.crt.pem',
+            )).text)
+    return ca_crt_path
 
 class BalancerTestCase(ERP5InstanceTestCase):
 
@@ -333,6 +350,84 @@ class TestBalancer(BalancerTestCase):
         'backend_web_server1')
 
 
+class TestTLS(BalancerTestCase):
+  """Check TLS
+  """
+  __partition_reference__ = 's'
+
+  def _getServerCertificate(self, hostname, port):
+    # type: (Optional[str], Optional[int]) -> Any
+    hostname_idna = idna.encode(hostname)
+    sock = socket.socket()
+
+    sock.connect((hostname, port))
+    ctx = OpenSSL.SSL.Context(OpenSSL.SSL.SSLv23_METHOD)
+    ctx.check_hostname = False
+    ctx.verify_mode = OpenSSL.SSL.VERIFY_NONE
+
+    sock_ssl = OpenSSL.SSL.Connection(ctx, sock)
+    sock_ssl.set_connect_state()
+    sock_ssl.set_tlsext_host_name(hostname_idna)
+    sock_ssl.do_handshake()
+    cert = sock_ssl.get_peer_certificate()
+    crypto_cert = cert.to_cryptography()
+    sock_ssl.close()
+    sock.close()
+    return crypto_cert
+
+  def test_certificate_validates_with_caucase_ca(self):
+    # type: () -> None
+    caucase = self.getManagedResource("caucase", CaucaseService)
+    requests.get(self.default_balancer_url, verify=caucase.ca_crt_path)
+
+  def test_certificate_renewal(self):
+    # type: () -> None
+    caucase = self.getManagedResource("caucase", CaucaseService)
+    balancer_parsed_url = urlparse.urlparse(self.default_balancer_url)
+    certificate_before_renewal = self._getServerCertificate(
+        balancer_parsed_url.hostname,
+        balancer_parsed_url.port)
+
+    # run caucase updater 90 days in the future, so that certificate is
+    # renewed.
+    caucase_updater = os.path.join(
+        self.computer_partition_root_path,
+        'etc',
+        'service',
+        'caucase-updater',
+    )
+    process = pexpect.spawnu(
+       "faketime +90days %s" % caucase_updater,
+        env=dict(os.environ, PYTHONPATH=''),
+    )
+    logger = self.logger
+    class DebugLogFile:
+      def write(self, msg):
+        logger.info("output from caucase_updater: %s", msg)
+      def flush(self):
+        pass
+    process.logfile = DebugLogFile()
+    process.expect(u"Renewing .*\nNext wake-up.*")
+    process.terminate()
+    process.wait()
+
+    # wait for server to use new certificate
+    for _ in range(30):
+      certificate_after_renewal = self._getServerCertificate(
+          balancer_parsed_url.hostname,
+          balancer_parsed_url.port)
+      if certificate_after_renewal.not_valid_before > certificate_before_renewal.not_valid_before:
+        break
+      time.sleep(.5)
+
+    self.assertGreater(
+        certificate_after_renewal.not_valid_before,
+        certificate_before_renewal.not_valid_before,
+    )
+
+    # requests are served properly after cert renewal
+    requests.get(self.default_balancer_url, verify=caucase.ca_crt_path).raise_for_status()
+
 
 class ContentTypeHTTPServer(ManagedHTTPServer):
   """An HTTP Server which reply with content type from path.

software/erp5/test/test/test_erp5.py

@@ -31,6 +31,7 @@ import glob
 import urlparse
 import socket
 import time
+import tempfile
 
 import psutil
 import requests
@@ -43,7 +44,7 @@ setUpModule # pyflakes
 class TestPublishedURLIsReachableMixin(object):
   """Mixin that checks that default page of ERP5 is reachable.
   """
-  def _checkERP5IsReachable(self, url):
+  def _checkERP5IsReachable(self, url, verify):
     # What happens is that instanciation just create the services, but does not
     # wait for ERP5 to be initialized. When this test run ERP5 instance is
     # instanciated, but zope is still busy creating the site and haproxy replies
@@ -51,7 +52,7 @@ class TestPublishedURLIsReachableMixin(object):
     # erp5 site is not created, with 500 when mysql is not yet reachable, so we
     # retry in a loop until we get a succesful response.
     for i in range(1, 60):
-      r = requests.get(url, verify=False)  # XXX can we get CA from caucase already ?
+      r = requests.get(url, verify=verify)
       if r.status_code != requests.codes.ok:
         delay = i * 2
         self.logger.warn("ERP5 was not available, sleeping for %ds and retrying", delay)
@@ -62,19 +63,36 @@ class TestPublishedURLIsReachableMixin(object):
 
     self.assertIn("ERP5", r.text)
 
+  def _getCaucaseServiceCACertificate(self):
+    ca_cert = tempfile.NamedTemporaryFile(
+        prefix="ca.crt.pem",
+        mode="w",
+        delete=False,
+    )
+    ca_cert.write(
+        requests.get(
+            urlparse.urljoin(
+                self.getRootPartitionConnectionParameterDict()['caucase-http-url'],
+                '/cas/crt/ca.crt.pem',
+            )).text)
+    self.addCleanup(os.unlink, ca_cert.name)
+    return ca_cert.name
+
   def test_published_family_default_v6_is_reachable(self):
     """Tests the IPv6 URL published by the root partition is reachable.
     """
     param_dict = self.getRootPartitionConnectionParameterDict()
     self._checkERP5IsReachable(
-      urlparse.urljoin(param_dict['family-default-v6'], param_dict['site-id']))
+      urlparse.urljoin(param_dict['family-default-v6'], param_dict['site-id']),
+      self._getCaucaseServiceCACertificate())
 
   def test_published_family_default_v4_is_reachable(self):
     """Tests the IPv4 URL published by the root partition is reachable.
     """
     param_dict = self.getRootPartitionConnectionParameterDict()
     self._checkERP5IsReachable(
-      urlparse.urljoin(param_dict['family-default'], param_dict['site-id']))
+      urlparse.urljoin(param_dict['family-default'], param_dict['site-id']),
+      self._getCaucaseServiceCACertificate())
 
 
 class TestDefaultParameters(ERP5InstanceTestCase, TestPublishedURLIsReachableMixin):

stack/erp5/buildout.hash.cfg

@@ -90,7 +90,7 @@ md5sum = 2f3ddd328ac1c375e483ecb2ef5ffb57
 
 [template-balancer]
 filename = instance-balancer.cfg.in
-md5sum = bb9a953ce22f7d5188385f0171b6198e
+md5sum = ecf119142e6b5cd85a2ba397552d2142
 
 [template-haproxy-cfg]
 filename = haproxy.cfg.in

stack/erp5/instance-balancer.cfg.in

@@ -18,19 +18,52 @@ per partition. No more (undefined result), no less (IndexError).
 recipe = slapos.recipe.template:jinja2
 mode = 644
 
+[balancer-csr-request-config]
+< = jinja2-template-base
+template = inline:
+    [req]
+    prompt = no
+    req_extensions = req_ext
+    distinguished_name = dn
+    [ dn ]
+    CN = example.com
+    [ req_ext ]
+    subjectAltName = @alt_names
+    [ alt_names ]
+    IP.1 =  {{ ipv4 }}
+    {% if ipv6_set -%}
+    IP.2 =  {{ ipv6 }}
+    {% endif %}
+rendered = ${buildout:parts-directory}/${:_buildout_section_name_}/${:_buildout_section_name_}.txt
+
+[balancer-csr-request]
+recipe = plone.recipe.command
+command = {{ parameter_dict["openssl"] }}/bin/openssl req \
+  -newkey rsa:2048 \
+  -batch \
+  -new \
+  -nodes \
+  -keyout '${apache-conf-ssl:key}' \
+  -config '${balancer-csr-request-config:rendered}' \
+  -out '${:csr}'
+stop-on-error = true
+csr = ${directory:etc}/${:_buildout_section_name_}.csr.pem
+
+
 {{ caucase.updater(
      prefix='caucase-updater',
      buildout_bin_directory=parameter_dict['bin-directory'],
      updater_path='${directory:services-on-watch}/caucase-updater',
      url=ssl_parameter_dict['caucase-url'],
      data_dir='${directory:srv}/caucase-updater',
-     crt_path='${apache-conf-ssl:caucase-cert}',
+     crt_path='${apache-conf-ssl:cert}',
      ca_path='${directory:srv}/caucase-updater/ca.crt',
      crl_path='${directory:srv}/caucase-updater/crl.pem',
-     key_path='${apache-conf-ssl:caucase-key}',
+     key_path='${apache-conf-ssl:key}',
      on_renew='${apache-graceful:output}',
      max_sleep=ssl_parameter_dict.get('max-crl-update-delay', 1.0),
      template_csr_pem=ssl_parameter_dict.get('csr'),
+     template_csr=None if ssl_parameter_dict.get('csr') else '${balancer-csr-request:csr}',
      openssl=parameter_dict['openssl'] ~ '/bin/openssl',
 )}}
 {% do section('caucase-updater') -%}
@@ -176,9 +209,6 @@ hash-files = ${haproxy-cfg:rendered}
 [apache-conf-ssl]
 cert = ${directory:apache-conf}/apache.crt
 key = ${directory:apache-conf}/apache.pem
-# XXX caucase certificate is not supported by caddy for now
-caucase-cert = ${directory:apache-conf}/apache-caucase.crt
-caucase-key = ${directory:apache-conf}/apache-caucase.pem
 {% if frontend_caucase_url_list -%}
 depends = ${caucase-updater-housekeeper-run:recipe}
 ca-cert-dir = ${directory:apache-ca-cert-dir}
@@ -201,19 +231,6 @@ context = key content {{content_section_name}}:content
 mode = {{ mode }}
 {%- endmacro %}
 
-[apache-ssl]
-{% if ssl_parameter_dict.get('key') -%}
-key = ${apache-ssl-key:rendered}
-cert = ${apache-ssl-cert:rendered}
-{{ simplefile('apache-ssl-key', '${apache-conf-ssl:key}', ssl_parameter_dict['key']) }}
-{{ simplefile('apache-ssl-cert', '${apache-conf-ssl:cert}', ssl_parameter_dict['cert']) }}
-{% else %}
-recipe = plone.recipe.command
-command = "{{ parameter_dict['openssl'] }}/bin/openssl" req -newkey rsa -batch -new -x509 -days 3650 -nodes -keyout "${:key}" -out "${:cert}"
-key = ${apache-conf-ssl:key}
-cert = ${apache-conf-ssl:cert}
-{%- endif %}
-
 [apache-conf-parameter-dict]
 backend-list = {{ dumps(apache_dict.values()) }}
 zope-virtualhost-monster-backend-dict = {{ dumps(zope_virtualhost_monster_backend_dict) }}
@@ -225,8 +242,8 @@ access-log = ${directory:log}/apache-access.log
 # Apache 2.4's default value (60 seconds) can be a bit too short
 timeout = 300
 # Basic SSL server configuration
-cert = ${apache-ssl:cert}
-key = ${apache-ssl:key}
+cert = ${apache-conf-ssl:cert}
+key = ${apache-conf-ssl:key}
 cipher =
 ssl-session-cache = ${directory:log}/apache-ssl-session-cache
 {% if frontend_caucase_url_list -%}

[Jérome Perrin]

Also, this is also semantically incorrect, this command should only generate a template of certificate, it should not generate the key, because the key is supposed to be generated by caucase-rerequester.

maybe a patch like this should be applied to keep the semantic, but I think this whole commit is wrong

iff --git a/stack/erp5/instance-balancer.cfg.in b/stack/erp5/instance-balancer.cfg.in
index 212291b69..d012f5d32 100644
--- a/stack/erp5/instance-balancer.cfg.in
+++ b/stack/erp5/instance-balancer.cfg.in
@@ -16,7 +16,7 @@ per partition. No more (undefined result), no less (IndexError).
 recipe = slapos.recipe.template:jinja2
 mode = 644
 
-[balancer-csr-request-config]
+[balancer-csr-template-config]
 < = jinja2-template-base
 template = inline:
     [req]
@@ -34,15 +34,15 @@ template = inline:
     {% endif %}
 rendered = ${buildout:parts-directory}/${:_buildout_section_name_}/${:_buildout_section_name_}.txt
 
-[balancer-csr-request]
+[balancer-csr-template]
 recipe = plone.recipe.command
 command = {{ parameter_dict["openssl"] }}/bin/openssl req \
   -newkey rsa:2048 \
   -batch \
   -new \
   -nodes \
-  -keyout '${tls:certificate}' \
-  -config '${balancer-csr-request-config:rendered}' \
+  -keyout /dev/null \
+  -config '${balancer-csr-template-config:rendered}' \
   -out '${:csr}'
 stop-on-error = true
 csr = ${directory:etc}/${:_buildout_section_name_}.csr.pem
@@ -61,7 +61,7 @@ csr = ${directory:etc}/${:_buildout_section_name_}.csr.pem
      on_renew='${haproxy-reload:output}',
      max_sleep=ssl_parameter_dict.get('max-crl-update-delay', 1.0),
      template_csr_pem=ssl_parameter_dict.get('csr'),
-     template_csr=None if ssl_parameter_dict.get('csr') else '${balancer-csr-request:csr}',
+     template_csr=None if ssl_parameter_dict.get('csr') else '${balancer-csr-template:csr}',
      openssl=parameter_dict['openssl'] ~ '/bin/openssl',
 )}}
 {% do section('caucase-updater') -%}

[Jérome Perrin]

This is not OK, caucase will not sign another CSR if it is for another key, this profile should carefully keep the same key.

Until now this was never one correctly I think, in the past I noticed some errors like:

2020-11-06 01:22:55 caucase-updater.py[10473] ERROR ERROR Certificate '/srv/slapgrid/slappart9/srv/slapos/inst/slappart6/etc/apache/apache-caucase.crt' does not match key '/srv/slapgrid/slappart9/srv/slapos/inst/slappart6/etc/apache/apache-caucase.pem'

This is because a new key was regenerated. The openssl command we are using to generate the key just overwrite the key everytime it runs, so in place upgrade of ERP5 software release was not possible for the last few releases.

One workaround I use locally is to remove caucase sqlite database and restart it.

I'm now trying to write an upgrade test of ERP5 software release and I'll also change this profile so that it stop overriding keys from now on.

[Jérome Perrin]

I have the beginning of a software release upgrade test and I think it reveals that this commit is wrong.

In fact it's much more complicated, we can not start using template_csr in caucase.updater like this once a key was already generated without template_csr, also the CSR template passed to caucase.updater can not change, if it becomes different, caucase-rerequester generates a new key and make a CSR with this new key ... and since it's a new key and the caucase only sign one service cert automatically, scenarios like IP change are also impossible.

I still have to understand more, but this change of using a template_csr like this is definitely wrong. I'll probably revert this commit ( @tomo please wait a bit if you have to make a release )

Maybe one way is to not use template_csr which would generate a certificate for example.com and when we verify the certificate we ignore hostname errors ... @luke is it how frontend is supposed to validate backend certificate ?

[Jérome Perrin]

In reality this commit was not so problematic, because certificates generated for ERP5 balancer by macros from stack/caucase were anyway lost at each software release upgrade. In !854 (closed) there's a fix for this problem ("stack/caucase: generate key / csr only once") and also fixes so that ERP5 software release can be updated. The case of ip address change is a bit more complex and is not supported at this point.

[Jérome Perrin]

@luke @tomo I prepared a revert for now in !857 (merged)