stack/erp5: support frontend-caucase-url-list option.

software/erp5/instance-erp5-input-schema.json

@@ -453,6 +453,11 @@
         "ssl": {
           "description": "HTTPS certificate generation parameters",
           "properties": {
+            "frontend-caucase-url-list": {
+              "title": "Frontend Caucase URL List",
+              "description": "List of URLs of caucase service of frontend groups.",
+              "type": "array"
+            },
             "caucase-url": {
               "title": "Caucase URL",
               "description": "URL of caucase service to use. If not set, global setting will be used.",

stack/erp5/buildout.hash.cfg

@@ -78,7 +78,7 @@ md5sum = d41d8cd98f00b204e9800998ecf8427e
 
 [template-erp5]
 filename = instance-erp5.cfg.in
-md5sum = 82dc695e212be124d60ceb1143e56b0d
+md5sum = 038c367b7c4249d854bb0535891f29b3
 
 [template-zeo]
 filename = instance-zeo.cfg.in
@@ -90,7 +90,7 @@ md5sum = 2f3ddd328ac1c375e483ecb2ef5ffb57
 
 [template-balancer]
 filename = instance-balancer.cfg.in
-md5sum = 6851e0c28a025bd26a4d3450204ae335
+md5sum = c7dd492f60300ed4ff2ef62e302c0fd3
 
 [template-haproxy-cfg]
 filename = haproxy.cfg.in

stack/erp5/instance-balancer.cfg.in

@@ -21,22 +21,42 @@ recipe = slapos.recipe.template:jinja2
 mode = 644
 
 {{ caucase.updater(
-     prefix='caucase-updater',
+     prefix='caucase-updater-0',
      buildout_bin_directory=parameter_dict['bin-directory'],
-     updater_path='${directory:services-on-watch}/caucase-updater',
+     updater_path='${directory:services-on-watch}/caucase-updater-0',
      url=ssl_parameter_dict['caucase-url'],
-     data_dir='${directory:srv}/caucase-updater',
+     data_dir='${directory:srv}/caucase-updater-0',
      crt_path='${apache-conf-ssl:caucase-cert}',
-     ca_path='${apache-conf-ssl:ca-cert}',
-     crl_path='${apache-conf-ssl:crl}',
+     ca_path='${directory:srv}/caucase-updater-0/ca.crt',
+     crl_path='${directory:srv}/caucase-updater-0/crl.pem',
      key_path='${apache-conf-ssl:caucase-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'),
      openssl=parameter_dict['openssl'] ~ '/bin/openssl',
 )}}
-{% do section('caucase-updater') -%}
-{% do section('caucase-updater-promise') -%}
+{% do section('caucase-updater-0') -%}
+{% do section('caucase-updater-0-promise') -%}
+
+{% for idx, frontend_caucase_url in enumerate(ssl_parameter_dict['frontend-caucase-url-list']) -%}
+{{   caucase.updater(
+       prefix='caucase-updater-%s' % (idx + 1),
+       buildout_bin_directory=parameter_dict['bin-directory'],
+       updater_path='${directory:services-on-watch}/caucase-updater-%s' % (idx + 1),
+       url=frontend_caucase_url,
+       data_dir='${directory:srv}/caucase-updater-%s' % (idx + 1),
+       crt_path='${apache-conf-ssl:caucase-cert}',
+       ca_path='${directory:srv}/caucase-updater-0/ca.crt',
+       crl_path='${directory:srv}/caucase-updater-0/crl.pem',
+       key_path='${apache-conf-ssl:caucase-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'),
+       openssl=parameter_dict['openssl'] ~ '/bin/openssl',
+     )}}
+{%   do section('caucase-updater-%s' % (idx + 1)) -%}
+{%   do section('caucase-updater-%s-promise' % (idx + 1)) -%}
+{% endfor -%}
 
 {% set haproxy_dict = {} -%}
 {% set apache_dict = {} -%}
@@ -176,9 +196,22 @@ wait-for-files =
 recipe = collective.recipe.template
 output = ${directory:bin}/apache-httpd-graceful
 mode = 700
-input = inline:
-  #!/bin/sh
-  kill -USR1 "$(cat '${apache-conf-parameter-dict:pid-file}')"
+input =
+  inline:
+  #!{{parameter_dict['bin-directory']}}/python2.7
+  from lock_file import LockFile
+  import os
+  import subprocess
+  with LockFile('${apache-conf-ssl:ca-cert}.lock', wait = True):
+    ca_path_list = [{% for idx in range(len(frontend_caucase_url_list) + 1) -%} '{{ '${directory:srv}/caucase-updater-%s/ca.crt' % idx }}', {% endfor -%}]
+    valid_ca_path_list = [path for path in ca_path_list
+                          if os.path.isfile(path) and subprocess.call(['openssl', 'x509', '-in', path], stdout=subprocess.PIPE) == 0]
+    file('${apache-conf-ssl:ca-cert}', 'w').write('\n'.join(file(path).read() for path in valid_ca_path_list))
+    crl_path_list = [{% for idx in range(len(frontend_caucase_url_list) + 1) -%} '{{ '${directory:srv}/caucase-updater-%s/crl.pem' % idx }}', {% endfor -%}]
+    valid_crl_path_list = [path for path in crl_path_list
+                          if os.path.isfile(path) and subprocess.call(['openssl', 'crl', '-in', path], stdout=subprocess.PIPE) == 0]
+    file('${apache-conf-ssl:crl}', 'w').write('\n'.join(file(path).read() for path in valid_crl_path_list))
+    subprocess.call(['kill',  '-USR1', file('${apache-conf-parameter-dict:pid-file}').read().strip()])
 
 [{{ section('apache-promise') }}]
 <= monitor-promise-base

stack/erp5/instance-erp5.cfg.in

@@ -98,6 +98,7 @@ backup-caucased = ${:srv}/backup/caucased
 {% do publish_dict.__setitem__('caucase-http-url', caucase_url) -%}
 {% set balancer_dict = slapparameter_dict.get('balancer', {}) -%}
 {% do balancer_dict.setdefault('ssl', {}).setdefault('caucase-url', caucase_url) -%}
+{% do balancer_dict['ssl'].setdefault('frontend-caucase-url-list', []) -%}
 
 {{ request('memcached-persistent', 'kumofs', 'kumofs', {'tcpv4-port': 2000}, {'url': True, 'monitor-base-url': False}, key_config={'monitor-passwd': 'monitor-htpasswd:passwd'}) }}
 {{ request('memcached-volatile', 'kumofs', 'memcached', {'tcpv4-port': 2010, 'ram-storage-size': 64}, {'url': True, 'monitor-base-url': False}, key_config={'monitor-passwd': 'monitor-htpasswd:passwd'}) }}

[Vincent Pelletier]

The chosen name, title and description must express that it is for frontend authentication as a client.

[Jérome Perrin]

can't we also put something like this to ensure the array items have proper type ?

"items": {
  "type": "string", 
  "format": "uri"
}

https://json-schema.org/understanding-json-schema/reference/array.html#list-validation

( if you want, we can also add the missing "format" for caucase-url just below at the same time )

[Jérome Perrin]

I'm just suggesting very "low level" thing, I don't understand what this change is about, so maybe this comment is not appropriate

[Vincent Pelletier]

+1 for adding this here and in the other parameter.

[Kazuhiko Shiozaki]

"format": "uri" added in f8f03bc7.

[Vincent Pelletier]

This breaks compatibility with existing setups. Please do not move existing configurations.

[Vincent Pelletier]

There is a mistake here before your change: the CA provided by this section must not be used for client authentication. This section is only supposed to provide server-side certification: key and certificate. crl and ca are to be ignored, and are only useful to the client (IOW, frontends) to validate backend identity.

[Kazuhiko Shiozaki]

I agree that this is wrong. But if I simply modify these parameters, we no longer have ${apache-conf-ssl:ca-cert} file and apache will not start if SSLCACertificateFile file does not exist or empty.

So even if I modify here in a dedicated commit, it should still be part of trusted-x-forwarded-for branch, instead of immediately 'fixing' master branch.

Then in another commit, SSLCACertificateFile will be set only when frontend-caucase-url-list is not empty.

Is it fine ?

[Kazuhiko Shiozaki]

Ah by using SSLCACertificatePath instead of SSLCACertificateFile, we don't care 'missing or empty' issue. So only concern is 'how to handle removing an entry from frontend-caucase-url-list' as I wrote below.

[Vincent Pelletier]

But if I simply modify these parameters, we no longer have ${apache-conf-ssl:ca-cert} file and apache will not start if SSLCACertificateFile file does not exist or empty.

But if there is no CA certificate this means client authentication is not desired, so SSLCACertificateFile (nor its ...Path variant) must not be output in apache configuration (and likewise for related configuration options).

[Kazuhiko Shiozaki]

Indeed. 36eef7b8

[Vincent Pelletier]

I think it is a bug to reuse caucase-updater-0 here.

Beyond this, and given the difference (sadly, not visible in the original version of this file) between these caucase instances and [caucase-updater], I think it would be better to use SSLCARevocationPath and SSLCACertificatePath: all new caucase-updater processes added as part of this change put these files in two shared directories. EDIT: this should remove completely the need to change apache graceful script.

[Kazuhiko Shiozaki]

Using SSLCACertificatePath sounds nice. The only concern is "removing an entry from frontend-caucase-url-list" usecase where obsolete files will remain.

[Vincent Pelletier]

Then maybe the CA & CRL can be in a per-caucase-updater folder, and symlinked to from the apache CACertificatePath folder ? Removing an entry would only remove the symlink. The CA certificate should not be removed, as it would cause a new trust bootstrap when/if the same caucase url is re-added, which is a vulnerability.

BTW, this will mean that the per-caucase-updater directory should ideally be named based on the url. What about something like srv/client-cert-ca/<caucase url hash>/cas-{ca,crl}.pem (plus all other caucase-updater files, I'm just illustrating with the expected final product here) ?

[Kazuhiko Shiozaki]

Then maybe the CA & CRL can be in a per-caucase-updater folder, and symlinked to from the apache CACertificatePath folder ?

Like this ?

$ ls -l etc/apache/ssl.crt
ca-0.crt -> (path_to)/caucase-update-0/ca.crt
ca-1.crt -> (path_to)/caucase-update-1/ca.crt
$ c_rehase etc/apache/ssl.crt
Doing etc/apache/ssl.crt
$ ls -l etc/apache/ssl.crt
01234567.0 -> ca-0.crt
12345678.0 -> ca-1.crt
ca-0.crt -> (path_to)/caucase-update-0/ca.crt
ca-1.crt -> (path_to)/caucase-update-1/ca.crt

Then once caucase-update-1 is removed,

$ ls -l etc/apache/ssl.crt
01234567.0 -> ca-0.crt
12345678.0 -> ca-1.crt
ca-0.crt -> (path_to)/caucase-update-0/ca.crt
ca-1.crt -> broken link
$ c_rehase etc/apache/ssl.crt
Doing etc/apache/ssl.crt
WARNING: ca-1.crt does not contain a certificate or CRL: skipping
$ ls -l etc/apache/ssl.crt
01234567.0 -> ca-0.crt
ca-0.crt -> (path_to)/caucase-update-0/ca.crt
ca-1.crt -> broken link

i.e. ca-1.crt is no longer effective.

The CA certificate should not be removed, as it would cause a new trust bootstrap when/if the same caucase url is re-added, which is a vulnerability.

BTW, this will mean that the per-caucase-updater directory should ideally be named based on the url.

We just copy CA / CRL from frontend cluster's caucase, so if they don't change, it will not cause a new trust bootstrap even if the same caucase url is re-added. Am I wrong ?

[Vincent Pelletier]

Then once caucase-update-1 is removed,

But it should really not be removed, because removing it makes caucase-updater forget everything about caucased. Which means, if the removal was an error and the URL gets re-added, a trust re-bootstrap happens, which is a vulnerability.

Which means

ca-1.crt -> broken link

is an issue: the link's target should still exist, but should be ignored by apache. Which I believe is easiest achieved by having slapos remove the symlink (...and then triggering whatever house-keeping mechanism to have apache use the correct CA cert set).

just copy CA / CRL from frontend cluster's caucase

This is "just" a trust bootstrap, yes: we initially know nothing about that caucase, and once we know about it we want to be able to ensure that no-one takes its place and trick us into accepting an unintended client certificate. To achieve this, the CA we retrieved must never be lost. When that caucase renews its CA, it signs it with previous CA's key well in advance of previous CA's expiration. caucase-updater verifies that signature during its periodic renewal checks, and is then able to trust that the new CA really belong to the same entity.

EDIT: Side note: caucase-updater only deletes a CA when it has a valid new version. If the CA is expired and there is no new version it is still kept. It cannot be used to validate a new CA (as it is expired), so its only value is to prevent an accidental new trust bootstrap.

Keeping a few outdated caucase-updater-created files around is quite cheap, so it should not be an issue.

And if frontend-side caucase has a problem and trust really needs to be re-bootstrapped, then it's a very serious issue, it must happen exceedingly rarely, and we have to spend admin time to explicitly go tell caucase to forget about the old trusted CA (=rm).

[Kazuhiko Shiozaki]

Reimplemented at c1c6d7e7 (housekeeping part is not included).

As we cannot calculate hash in instance-balancer.cfg.in template, I choose urlencode-ish encode. https://pypi.org/project/jinja2-ansible-filters/ will provide hash() filter, but we cannot add this egg by buildout because of https://gitlab.com/dreamer-labs/libraries/jinja2-ansible-filters/-/issues/1. Or shall we extend slapos.recipe.template to provide hash() filter ? (we anyway need to modify slapos.recipe.template even if we want to use jinja2-ansible-filters...)

[Vincent Pelletier]

we cannot calculate hash in instance-balancer.cfg.in template

It is actually possible: in instance.cfg.in, in the [dynamic-template-balancer-parameters] section's extra-context entry, you could add import hashlib hashlib, and it should then be possible in instance-balancer.cfg.in to write something like {{ hashlib.sha256(url).hexdigest() }}.

Thinking of opaque hashes, there should BTW be no need to store the non-hashed URL: caucase-produced CA certificates contain in their subject the URL they are supposed to be reached from (caucase --netloc ...), which must be correct for issued certificates to work: otherwise the CRL distribution point will not be accessible to ssl clients (whose implementation we do not have any control on: it's directly openssl, gnutls, microsoft's ssl library, ...). So retrieving the URL just means openssl x509 -subject -noout -in CA_FILE.

This said, it may be nicer to use a human-readable fs-safe representation. Cacuase URL's scheme is always "http", there is no querystring and no fragment. So you can parse the URL (urlparse.split and urlparse.unsplit) and extract only netloc + path for the folder name. In most cases path should be empty, which means only netloc would be present in the result (but please still preserve path, in case caucase gets embeded in a multi-application WSGI server). So in most cases on unix filesystems nothing would need to be escaped at all. I do not have a clear opinion on which is beter, I let you decide which one is preferable (code complexity vs. introspectability).

[Kazuhiko Shiozaki]

Thanks for tips to use hashlib !

I prefer hash way and modified in 93a6d490.

bin/caucase-updater-housekeeper script is also now implemented, that deletes obsolete symlinks and also creates missing symlinks. It is triggered by --on_new argument of caucase-updater, i.e. it will NOT be triggered automatically if an url is removed then re-added. Do you have any good idea to handle such case ? (Or we can manually invoke it in such rare usecase if there is no good way...)

[Vincent Pelletier]

Do you have any good idea to handle such case ?

Can the symlinks be created by buildout ? Then all the housekeeping script has to do is run c_rehase, if I understand correctly.

Then, if the recipe returns the path of the symlink it created, that path will be removed when that section gets uninstalled.

BTW, I think this behaviour should be acceptable for any buildout-managed symlink (as opposed to the makedirs recipe, for example, where it could be a disaster if the folders it create get deleted...).

[Vincent Pelletier]

And thinking again about multiple caucase-updater: the URL hash should also be used:

• in section names (the macro prefix parameter), so instanciation is not sensitive to changes in URL order
• supervisord entries, so instanciation does not replace entry 1 with what was entry 2, then then forgets to stop the old caucase-updater-1

[Kazuhiko Shiozaki]

Yes, I already use 'hash' everywhere instead of 'index'.

Also I made the change so that housekeeper is invoked each time instantiation happens at 1d39ba7c.

[Vincent Pelletier]

No key and certificates are needed here, these are only needed client-side.