Commit 6073096a authored by Łukasz Nowak's avatar Łukasz Nowak Committed by Łukasz Nowak

caddy-frontend: Protect against slave's wrong certificate and key

Instead of relying on slapos.cookbook:certificate_authority recipe, which
stops buildout processing, extract the minimal implementation to runtime
key/certificate validator and reject slaves, which does not pass this test.

This commits results in TODO item being done.
parent 9872a4dc
...@@ -2,7 +2,6 @@ Generally things to be done with ``caddy-frontend``: ...@@ -2,7 +2,6 @@ Generally things to be done with ``caddy-frontend``:
* tests: add assertion with results of promises in etc/promise for each partition * tests: add assertion with results of promises in etc/promise for each partition
* check the whole frontend slave snippet with ``caddy -validate`` during buildout run, and reject if does not pass validation * check the whole frontend slave snippet with ``caddy -validate`` during buildout run, and reject if does not pass validation
* check that all options from ``instance-slave-caddy-input-schema.json`` are safe to be used
* ``apache-ca-certificate`` shall be merged with ``apache-certificate`` * ``apache-ca-certificate`` shall be merged with ``apache-certificate``
* ``apache-ca-certificate`` shall be appended to ``apache-certificate`` if not already there * ``apache-ca-certificate`` shall be appended to ``apache-certificate`` if not already there
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
# not need these here). # not need these here).
[template] [template]
filename = instance.cfg.in filename = instance.cfg.in
md5sum = 5360ac713bc1f00b2668238027dc253b md5sum = ae708bdef97812bad1223b94fae750b3
[template-common] [template-common]
filename = instance-common.cfg.in filename = instance-common.cfg.in
...@@ -26,11 +26,11 @@ md5sum = 750e2b1c922bf14511a3bc8a42468b1b ...@@ -26,11 +26,11 @@ md5sum = 750e2b1c922bf14511a3bc8a42468b1b
[template-apache-replicate] [template-apache-replicate]
filename = instance-apache-replicate.cfg.in filename = instance-apache-replicate.cfg.in
md5sum = 2f370174b18f27db5c0f9daf83df8104 md5sum = 70d923e4fd5cfdb52bc4cbd04f9e01d2
[template-slave-list] [template-slave-list]
filename = templates/apache-custom-slave-list.cfg.in filename = templates/apache-custom-slave-list.cfg.in
md5sum = c57a982d18cf7f36f5d34b80738ea726 md5sum = ed1c086f0548a908661b294e845dc008
[template-slave-configuration] [template-slave-configuration]
filename = templates/custom-virtualhost.conf.in filename = templates/custom-virtualhost.conf.in
...@@ -46,7 +46,7 @@ md5sum = 7c987ad75fcce6f5b925c7696ff41971 ...@@ -46,7 +46,7 @@ md5sum = 7c987ad75fcce6f5b925c7696ff41971
[template-custom-slave-list] [template-custom-slave-list]
filename = templates/apache-custom-slave-list.cfg.in filename = templates/apache-custom-slave-list.cfg.in
md5sum = c57a982d18cf7f36f5d34b80738ea726 md5sum = ed1c086f0548a908661b294e845dc008
[caddy-backend-url-validator] [caddy-backend-url-validator]
filename = templates/caddy-backend-url-validator.in filename = templates/caddy-backend-url-validator.in
......
...@@ -9,6 +9,7 @@ context = ...@@ -9,6 +9,7 @@ context =
raw common_profile {{ common_profile }} raw common_profile {{ common_profile }}
${:extra-context} ${:extra-context}
{% set popen = functools_module.partial(subprocess_module.Popen, stdout=subprocess_module.PIPE, stderr=subprocess_module.STDOUT, stdin=subprocess_module.PIPE) %}
{% set part_list = [] %} {% set part_list = [] %}
{% set single_type_key = 'single-' %} {% set single_type_key = 'single-' %}
{% if slap_software_type == "replicate" %} {% if slap_software_type == "replicate" %}
...@@ -80,6 +81,15 @@ context = ...@@ -80,6 +81,15 @@ context =
{% do slave_dict.__setitem__('state', False) %} {% do slave_dict.__setitem__('state', False) %}
{% endif %} {% endif %}
{% endif %} {% endif %}
{% if slave.get('ssl_key') and slave.get('ssl_crt') %}
{% set key_popen = popen([openssl, 'rsa', '-noout', '-modulus']) %}
{% set crt_popen = popen([openssl, 'x509', '-noout', '-modulus']) %}
{% set key_modulus = key_popen.communicate(slave['ssl_key'])[0] | trim %}
{% set crt_modulus = crt_popen.communicate(slave['ssl_crt'])[0] | trim %}
{% if not key_modulus or key_modulus != crt_modulus %}
{% do slave_dict.__setitem__('state', False) %}
{% endif %}
{% endif %}
{% if slave.get('custom_domain') %} {% if slave.get('custom_domain') %}
{% if not validators.domain(slave['custom_domain']) %} {% if not validators.domain(slave['custom_domain']) %}
{% do slave_dict.__setitem__('state', False) %} {% do slave_dict.__setitem__('state', False) %}
......
...@@ -48,9 +48,12 @@ depends = ${caddyprofiledeps:recipe} ...@@ -48,9 +48,12 @@ depends = ${caddyprofiledeps:recipe}
template = {{ template_caddy_replicate }} template = {{ template_caddy_replicate }}
filename = instance-caddy-replicate.cfg filename = instance-caddy-replicate.cfg
extensions = jinja2.ext.do extensions = jinja2.ext.do
openssl = {{ template_frontend_parameter_dict['openssl'] ~ '/bin/openssl' }}
extra-context = extra-context =
import subprocess_module subprocess import subprocess_module subprocess
import functools_module functools
import validators validators import validators validators
key openssl :openssl
raw caddy_backend_url_validator {{ caddy_backend_url_validator }} raw caddy_backend_url_validator {{ caddy_backend_url_validator }}
raw template_publish_slave_information {{ template_replicate_publish_slave_information }} raw template_publish_slave_information {{ template_replicate_publish_slave_information }}
# Must match the key id in [switch-softwaretype] which uses this section. # Must match the key id in [switch-softwaretype] which uses this section.
......
...@@ -169,25 +169,27 @@ value = {{ dumps(slave_instance.get(cert_name)) }} ...@@ -169,25 +169,27 @@ value = {{ dumps(slave_instance.get(cert_name)) }}
{% set cert_file = '/'.join([custom_ssl_directory, cert_title.replace('-','.')]) %} {% set cert_file = '/'.join([custom_ssl_directory, cert_title.replace('-','.')]) %}
{% set key_file = '/'.join([custom_ssl_directory, key_title.replace('-','.')]) %} {% set key_file = '/'.join([custom_ssl_directory, key_title.replace('-','.')]) %}
{% do part_list.append(cert_title) %} {% do part_list.append(cert_title) %}
{% do part_list.append(key_title) %}
{% do slave_parameter_dict.__setitem__("ssl_crt", cert_file) %} {% do slave_parameter_dict.__setitem__("ssl_crt", cert_file) %}
{% do slave_parameter_dict.__setitem__("ssl_key", key_file) %} {% do slave_parameter_dict.__setitem__("ssl_key", key_file) %}
{% do slave_instance.__setitem__('path_to_ssl_crt', cert_file) %} {% do slave_instance.__setitem__('path_to_ssl_crt', cert_file) %}
{% do slave_instance.__setitem__('path_to_ssl_key', key_file) %} {% do slave_instance.__setitem__('path_to_ssl_key', key_file) %}
[{{cert_title}}] [{{key_title}}]
recipe = slapos.cookbook:certificate_authority.request < = jinja2-template-base
#openssl-binary = ${openssl:location}/bin/openssl template = {{ empty_template }}
rendered = {{ key_file }}
requests-directory = ${cadirectory:requests}
ca-private = ${cadirectory:private}
ca-certs = ${cadirectory:certs}
ca-newcerts = ${cadirectory:newcerts}
ca-crl = ${cadirectory:crl}
key-file = {{ key_file }}
cert-file = {{ cert_file }}
key-content = {{ dumps(slave_instance.get('ssl_key')) }} key-content = {{ dumps(slave_instance.get('ssl_key')) }}
cert-content = {{ dumps(slave_instance.get('ssl_crt')) }} extra-context =
key content :key-content
[{{cert_title}}]
< = jinja2-template-base
template = {{ empty_template }}
rendered = {{ cert_file }}
cert-content = {{ dumps(slave_instance.get('ssl_crt')) }}
extra-context =
key content :cert-content
{% endif %} {% endif %}
{# ########################################## #} {# ########################################## #}
......
...@@ -3063,6 +3063,10 @@ class TestSlaveBadParameters(SlaveHttpFrontendTestCase, TestDataMixin): ...@@ -3063,6 +3063,10 @@ class TestSlaveBadParameters(SlaveHttpFrontendTestCase, TestDataMixin):
'monitor-ipv6-test-unsafe': { 'monitor-ipv6-test-unsafe': {
'monitor-ipv6-test': '${section:option}\nafternewline ipv6', 'monitor-ipv6-test': '${section:option}\nafternewline ipv6',
}, },
'ssl_key-ssl_crt-unsafe': {
'ssl_key': '${section:option}ssl_keyunsafe\nunsafe',
'ssl_crt': '${section:option}ssl_crtunsafe\nunsafe',
},
} }
def test_master_partition_state(self): def test_master_partition_state(self):
...@@ -3073,10 +3077,11 @@ class TestSlaveBadParameters(SlaveHttpFrontendTestCase, TestDataMixin): ...@@ -3073,10 +3077,11 @@ class TestSlaveBadParameters(SlaveHttpFrontendTestCase, TestDataMixin):
'monitor-base-url': None, 'monitor-base-url': None,
'domain': 'example.com', 'domain': 'example.com',
'accepted-slave-amount': '7', 'accepted-slave-amount': '7',
'rejected-slave-amount': '2', 'rejected-slave-amount': '3',
'slave-amount': '9', 'slave-amount': '10',
'rejected-slave-list': 'rejected-slave-list':
'["_server-alias-unsafe", "_custom_domain-unsafe"]'} '["_server-alias-unsafe", "_custom_domain-unsafe", '
'"_ssl_key-ssl_crt-unsafe"]'}
self.assertEqual( self.assertEqual(
expected_parameter_dict, expected_parameter_dict,
...@@ -3353,3 +3358,11 @@ class TestSlaveBadParameters(SlaveHttpFrontendTestCase, TestDataMixin): ...@@ -3353,3 +3358,11 @@ class TestSlaveBadParameters(SlaveHttpFrontendTestCase, TestDataMixin):
'-a ${section:option} afternewline ipv6', '-a ${section:option} afternewline ipv6',
subprocess.check_output(monitor_file).strip() subprocess.check_output(monitor_file).strip()
) )
def test_ssl_key_ssl_crt_unsafe(self):
parameter_dict = self.slave_connection_parameter_dict_dict[
'ssl_key-ssl_crt-unsafe']
self.assertEqual(
parameter_dict,
{}
)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment