Commit 8fa7ec94 authored by Jérome Perrin's avatar Jérome Perrin Committed by Levin Zimmermann

stack/erp5: serve balancer requests when client certificate is not verified

We configure haproxy with "verify optional", which makes haproxy request
a client certificate, but accept the case where client does not present
a certificate, but as described in [1], if client present a certificate
and this certificate can not be verified, handshake is aborted. This is
not what we want, we want to treat the case of a non verified
certificate same as the case of the absence of certificate.

This configures haproxy accordingly, using "crt-ignore-err all" to allow
handshake anyway.

Once this was fixed, there was a remaining problem with
client_cert_verified acl, haproxy acl are OR, but this rule was supposed
to be a AND (client present a certificate AND it is verified), this was
rewritten to use inline condition which are AND.

[1]: https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.1-verify

Also adjust test_x_forwarded_for_stripped_when_no_certificate to assert
that there is no X-Forwarded-For header at all when no client
certificate.
parent 361c990b
......@@ -811,7 +811,7 @@ class TestFrontendXForwardedFor(BalancerTestCase):
).json()
self.assertEqual(result['Incoming Headers'].get('x-forwarded-for', '').split(', ')[0], '1.2.3.4')
def test_x_forwarded_for_stripped_when_not_verified_connection(self):
def test_x_forwarded_for_stripped_when_no_certificate(self):
# type: () -> None
balancer_url = json.loads(self.computer_partition.getConnectionParameterDict()['_'])['default']
result = requests.get(
......@@ -819,7 +819,7 @@ class TestFrontendXForwardedFor(BalancerTestCase):
headers={'X-Forwarded-For': '1.2.3.4'},
verify=False,
).json()
self.assertNotEqual(result['Incoming Headers'].get('x-forwarded-for', '').split(', ')[0], '1.2.3.4')
self.assertNotIn('x-fowarded-for', [k.lower() for k in result['Incoming Headers'].keys()])
balancer_url = json.loads(self.computer_partition.getConnectionParameterDict()['_'])['default-auth']
with self.assertRaisesRegexp(Exception, "certificate required"):
requests.get(
......@@ -828,6 +828,32 @@ class TestFrontendXForwardedFor(BalancerTestCase):
verify=False,
)
def test_x_forwarded_for_stripped_when_not_verified_certificate(self):
# type: () -> None
balancer_url = json.loads(self.computer_partition.getConnectionParameterDict()['_'])['default']
# certificate from an unknown CA
another_unrelated_caucase = self.getManagedResource('another_unrelated_caucase', CaucaseService)
unknown_client_certificate = self.getManagedResource('unknown_client_certificate', CaucaseCertificate)
unknown_client_certificate.request('unknown client certificate', another_unrelated_caucase)
result = requests.get(
balancer_url,
headers={'X-Forwarded-For': '1.2.3.4'},
cert=(unknown_client_certificate.cert_file, unknown_client_certificate.key_file),
verify=False,
).json()
self.assertNotIn('x-fowarded-for', [k.lower() for k in result['Incoming Headers'].keys()])
balancer_url = json.loads(self.computer_partition.getConnectionParameterDict()['_'])['default-auth']
with self.assertRaisesRegex(Exception, "unknown ca"):
requests.get(
balancer_url,
headers={'X-Forwarded-For': '1.2.3.4'},
cert=(unknown_client_certificate.cert_file, unknown_client_certificate.key_file),
verify=False,
)
class TestServerTLSProvidedCertificate(BalancerTestCase):
"""Check that certificate and key can be provided as instance parameters.
......
......@@ -94,7 +94,7 @@ md5sum = b0751d3d12cfcc8934cb1027190f5e5e
[template-haproxy-cfg]
filename = haproxy.cfg.in
md5sum = 1645ef8990ab2b50f91a4c02f0cf8882
md5sum = 85a8c0dadf7b648ef9748b6199dcfeb6
[template-rsyslogd-cfg]
filename = rsyslogd.cfg.in
......
......@@ -154,7 +154,7 @@ defaults
{% for name, (port, _, certificate_authentication, timeout, backend_list) in sorted(six.iteritems(parameter_dict['backend-dict'])) -%}
listen family_{{ name }}
{%- if parameter_dict.get('ca-cert') -%}
{%- set ssl_auth = ' ca-file ' ~ parameter_dict['ca-cert'] ~ ' verify' ~ ( ' required' if certificate_authentication else ' optional' ) ~ ' crl-file ' ~ parameter_dict['crl'] %}
{%- set ssl_auth = ' ca-file ' ~ parameter_dict['ca-cert'] ~ ' verify' ~ ( ' required' if certificate_authentication else ' optional crt-ignore-err all' ) ~ ' crl-file ' ~ parameter_dict['crl'] %}
{%- else %}
{%- set ssl_auth = '' %}
{%- endif %}
......@@ -173,11 +173,10 @@ listen family_{{ name }}
{%- endif %}
# remove X-Forwarded-For unless client presented a verified certificate
acl client_cert_verified ssl_c_used ssl_c_verify 0
http-request del-header X-Forwarded-For unless client_cert_verified
http-request del-header X-Forwarded-For unless { ssl_c_verify 0 } { ssl_c_used 1 }
# set Remote-User if client presented a verified certificate
http-request del-header Remote-User
http-request set-header Remote-User %{+Q}[ssl_c_s_dn(cn)] if client_cert_verified
http-request set-header Remote-User %{+Q}[ssl_c_s_dn(cn)] if { ssl_c_verify 0 } { ssl_c_used 1 }
# logs
capture request header Referer len 512
......
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