On Wednesday 16 May 2018 17:32:56 Jack Burton wrote:
> My attempts to get this accepted last year stalled.
>
> As best I recall, the main stumbling block was agreeing on how much /
> exactly which client cert data to pass through to fastcgi (my view was
> that passing the whole client cert chain would be ideal).
>
> So, here's a stripped down version that passes *no* client cert
> data through to fastcgi at all (but still passes the relevant status
> flags in TLS_PEER_VERIFY).
>
> This diff provides everything necessary for tls client *authentication*,
> but *none* of what's required to use tls client certs for authorisation
> or accounting.
>
> I figured that if we can agree on this much, so httpd can be used for
> the authentication-only case (which is all non-fastcgi sites would want)
> straight away, that's be a good first step -- then we can come back and
> argue the toss over how much client cert data is necessary/sufficient
> to pass through for authorisation/accounting purposes.
>
> There's also a trivial regression test (unchanged from last year), which
> I'll post again separately next.
Thanks - I've just committed this, with some minor tweaks and minus one chunk:
> Index: server.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server.c,v
> retrieving revision 1.113
> diff -u -p -r1.113 server.c
> --- server.c 29 Nov 2017 16:55:08 -0000 1.113
> +++ server.c 16 May 2018 07:59:11 -0000
> @@ -264,6 +300,27 @@ server_tls_init(struct server *srv)
> return (-1);
> }
>
> + if (srv->srv_conf.tls_ca != NULL) {
> + if (tls_config_set_ca_mem(srv->srv_tls_config,
> + srv->srv_conf.tls_ca, srv->srv_conf.tls_ca_len) != 0) {
> + log_warnx("%s: failed to add ca cert(s)", __func__);
> + return (-1);
> + }
> + if (srv->srv_conf.tls_flags & TLSFLAG_OPTIONAL)
> + tls_config_verify_client_optional(srv->srv_tls_config);
> + else
> + tls_config_verify_client(srv->srv_tls_config);
> +
> + if (srv->srv_conf.tls_crl != NULL) {
> + if (tls_config_set_crl_mem(srv->srv_tls_config,
> + srv->srv_conf.tls_crl,
> + srv->srv_conf.tls_crl_len) != 0) {
> + log_warnx("%s: failed to add crl(s)", __func__);
> + return (-1);
> + }
> + }
> + }
> +
> TAILQ_FOREACH(srv_conf, &srv->srv_hosts, entry) {
> if (srv_conf->tls_cert == NULL || srv_conf->tls_key == NULL)
> continue;
> @@ -277,6 +334,26 @@ server_tls_init(struct server *srv)
> log_warnx("%s: failed to add tls keypair", __func__);
> return (-1);
> }
> +
> + if (srv->srv_conf.tls_ca == NULL)
> + continue;
> + log_debug("%s: adding ca cert(s) for server %s", __func__,
> + srv->srv_conf.name);
> + if (tls_config_set_ca_mem(srv->srv_tls_config,
> + srv_conf->tls_ca, srv_conf->tls_ca_len) != 0) {
> + log_warnx("%s: failed to add ca cert(s)", __func__);
> + return (-1);
> + }
> +
> + if (srv->srv_conf.tls_crl == NULL)
> + continue;
> +
> + log_debug("%s: adding crl(s) for server %s", __func__,
> + srv->srv_conf.name);
> + if (tls_config_set_crl_mem(srv->srv_tls_config,
> + srv_conf->tls_crl, srv_conf->tls_crl_len) != 0) {
> + return (-1);
> + }
> }
>
> /* set common session ID among all processes */
The above chunk does not make sense, since in the case of multiple httpd
servers configured on the same port, we'll just keep on setting the CA and CRL
which overwrites the one set in the previous chunk (which means the last
configured CA and CRL win). The SNI support in libtls does not currently allow
for multiple CAs/CRLs to be provided - if we wanted to support this we'd need
to add support their first. For the time being we should add the CA and CRL
configuration to the server_tls_cmp() check so that we force it to be identical
across HTTPS servers configured on the same address/port.
