Ashe Connor([email protected]) on 2018.11.26 05:29:45 +0000:
> On Fri, Nov 23, 2018 at 04:41:21PM +0100, Sebastian Benoit wrote:
> > > It appears that relayd doesn't support TLS client certificate validation
> > > (in the manner that httpd does with "tls client ca [cafile]"). Would
> > > there be interest in a patch that added such support?
> >
> > yes, a patch to support client certificates would be welcome.
> >
> > /Benno
>
> Wonderful. Here's a first pass at such a patch.
>
> Ashe
Sorry to keep you waiting.
The parse and config bits look good.
And the use of tls* looks ok to me too, but I would like to have someone
more familiar with it to give an ok though.
As for style, please make lines not longer than 80 chars.
> Index: usr.sbin/relayd/config.c
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/usr.sbin/relayd/config.c,v
> retrieving revision 1.36
> retrieving revision 1.36.6.1
> diff -u -p -r1.36 -r1.36.6.1
> --- usr.sbin/relayd/config.c 29 Nov 2017 15:24:50 -0000 1.36
> +++ usr.sbin/relayd/config.c 24 Nov 2018 16:15:37 -0000 1.36.6.1
> @@ -900,6 +900,15 @@ config_setrelay(struct relayd *env, stru
> rlay->rl_conf.name);
> return (-1);
> }
> + if (rlay->rl_tls_client_ca_fd != -1 &&
> + config_setrelayfd(ps, id, n,
> + rlay->rl_conf.id, RELAY_FD_CLIENTCACERT,
> + rlay->rl_tls_client_ca_fd) == -1) {
> + log_warn("%s: fd passing failed for "
> + "`%s'", __func__,
> + rlay->rl_conf.name);
> + return (-1);
> + }
> /* Prevent fd exhaustion in the parent. */
> if (proc_flush_imsg(ps, id, n) == -1) {
> log_warn("%s: failed to flush "
> @@ -945,6 +954,10 @@ config_setrelay(struct relayd *env, stru
> close(rlay->rl_tls_ca_fd);
> rlay->rl_tls_ca_fd = -1;
> }
> + if (rlay->rl_tls_client_ca_fd != -1) {
> + close(rlay->rl_tls_client_ca_fd);
> + rlay->rl_tls_client_ca_fd = -1;
> + }
>
> return (0);
> }
> @@ -968,6 +981,7 @@ config_getrelay(struct relayd *env, stru
> rlay->rl_tls_cert_fd = -1;
> rlay->rl_tls_ca_fd = -1;
> rlay->rl_tls_cacert_fd = -1;
> + rlay->rl_tls_client_ca_fd = -1;
>
> if (ps->ps_what[privsep_process] & CONFIG_PROTOS) {
> if (rlay->rl_conf.proto == EMPTY_ID)
> @@ -1084,6 +1098,9 @@ config_getrelayfd(struct relayd *env, st
> break;
> case RELAY_FD_CAFILE:
> rlay->rl_tls_cacert_fd = imsg->fd;
> + break;
> + case RELAY_FD_CLIENTCACERT:
> + rlay->rl_tls_client_ca_fd = imsg->fd;
> break;
> }
>
> Index: usr.sbin/relayd/parse.y
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.230
> retrieving revision 1.230.2.2
> diff -u -p -r1.230 -r1.230.2.2
> --- usr.sbin/relayd/parse.y 1 Nov 2018 00:18:44 -0000 1.230
> +++ usr.sbin/relayd/parse.y 24 Nov 2018 16:15:37 -0000 1.230.2.2
> @@ -175,7 +175,7 @@ typedef struct {
> %token SNMP SOCKET SPLICE SSL STICKYADDR STYLE TABLE TAG TAGGED TCP
> TIMEOUT TLS
> %token TO ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL
> RTABLE
> %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE
> PASSWORD ECDHE
> -%token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
> +%token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
> CLIENT
> %token <v.string> STRING
> %token <v.number> NUMBER
> %type <v.string> hostname interface table value optstring
> @@ -1246,6 +1246,16 @@ tlsflags : SESSION TICKETS { proto->tick
> }
> free($3);
> }
> + | CLIENT CA STRING {
> + if (strlcpy(proto->tlsclientca, $3,
> + sizeof(proto->tlsclientca)) >=
> + sizeof(proto->tlsclientca)) {
> + yyerror("tlsclientca truncated");
> + free($3);
> + YYERROR;
> + }
> + free($3);
> + }
> | NO flag { proto->tlsflags &= ~($2); }
> | flag { proto->tlsflags |= $1; }
> ;
> @@ -1687,6 +1697,7 @@ relay : RELAY STRING {
> r->rl_tls_cert_fd = -1;
> r->rl_tls_ca_fd = -1;
> r->rl_tls_cacert_fd = -1;
> + r->rl_tls_client_ca_fd = -1;
> TAILQ_INIT(&r->rl_tables);
> if (last_relay_id == INT_MAX) {
> yyerror("too many relays defined");
> @@ -2241,6 +2252,7 @@ lookup(char *s)
> { "check", CHECK },
> { "checks", CHECKS },
> { "ciphers", CIPHERS },
> + { "client", CLIENT },
> { "code", CODE },
> { "connection", CONNECTION },
> { "cookie", COOKIE },
> @@ -3224,6 +3236,7 @@ relay_inherit(struct relay *ra, struct r
> rb->rl_tls_cert_fd = -1;
> rb->rl_tls_cacert_fd = -1;
> rb->rl_tls_ca_fd = -1;
> + rb->rl_tls_client_ca_fd = -1;
> rb->rl_tls_key = NULL;
> rb->rl_conf.tls_key_len = 0;
> }
> Index: usr.sbin/relayd/relay.c
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.241
> retrieving revision 1.241.4.1
> diff -u -p -r1.241 -r1.241.4.1
> --- usr.sbin/relayd/relay.c 19 Sep 2018 11:28:02 -0000 1.241
> +++ usr.sbin/relayd/relay.c 24 Nov 2018 16:15:37 -0000 1.241.4.1
> @@ -2176,6 +2176,24 @@ relay_tls_ctx_create(struct relay *rlay)
> goto err;
> }
>
> + if (rlay->rl_tls_client_ca_fd != -1) {
> + if ((buf = relay_load_fd(rlay->rl_tls_client_ca_fd,
> &len)) ==
length
> + NULL) {
> + log_warn("failed to read tls client CA
> certificate");
length
> + goto err;
> + }
> +
> + if (tls_config_set_ca_mem(tls_cfg, buf, len) !=
> + 0) {
> + log_warnx("failed to set tls client CA
> certificate: %s",
length
> + tls_config_error(tls_cfg));
> + goto err;
> + }
> + purge_key(&buf, len);
> +
> + tls_config_verify_client(tls_cfg);
> + }
> +
> tls = tls_server();
> if (tls == NULL) {
> log_warnx("unable to allocate TLS context");
> @@ -2198,6 +2216,7 @@ relay_tls_ctx_create(struct relay *rlay)
> close(rlay->rl_tls_cert_fd);
> close(rlay->rl_tls_cacert_fd);
> close(rlay->rl_tls_ca_fd);
> + close(rlay->rl_tls_client_ca_fd);
>
> if (rlay->rl_tls_client_cfg == NULL)
> tls_config_free(tls_client_cfg);
> @@ -2678,6 +2697,12 @@ relay_load_certfiles(struct relay *rlay)
> log_debug("%s: using ca key %s", __func__,
> proto->tlscakey);
> }
> + if (strlen(proto->tlsclientca)) {
> + if ((rlay->rl_tls_client_ca_fd =
> + open(proto->tlsclientca, O_RDONLY)) == -1)
> + return (-1);
> + log_debug("%s: using client ca %s", __func__,
> proto->tlsclientca);
length
> + }
> }
>
> if ((rlay->rl_conf.flags & F_TLS) == 0)
> @@ -2712,6 +2737,13 @@ relay_load_certfiles(struct relay *rlay)
> &rlay->rl_conf.tls_key_len, NULL)) == NULL)
> return (-1);
> log_debug("%s: using private key %s", __func__, certfile);
> +
> + if (strlen(proto->tlsclientca)) {
> + if ((rlay->rl_tls_client_ca_fd =
> + open(proto->tlsclientca, O_RDONLY)) == -1)
> + return (-1);
> + log_debug("%s: using client ca %s", __func__,
> proto->tlsclientca);
length
> + }
>
> return (0);
> }
> Index: usr.sbin/relayd/relayd.h
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/usr.sbin/relayd/relayd.h,v
> retrieving revision 1.251
> retrieving revision 1.251.4.1
> diff -u -p -r1.251 -r1.251.4.1
> --- usr.sbin/relayd/relayd.h 9 Sep 2018 21:06:51 -0000 1.251
> +++ usr.sbin/relayd/relayd.h 24 Nov 2018 16:15:37 -0000 1.251.4.1
> @@ -141,9 +141,10 @@ struct ctl_relayfd {
> objid_t relayid;
> int type;
> };
> -#define RELAY_FD_CERT 1
> -#define RELAY_FD_CACERT 2
> -#define RELAY_FD_CAFILE 3
> +#define RELAY_FD_CERT 1
> +#define RELAY_FD_CACERT 2
> +#define RELAY_FD_CAFILE 3
> +#define RELAY_FD_CLIENTCACERT 4
>
> struct ctl_script {
> objid_t host;
> @@ -393,6 +394,7 @@ union hashkey {
> #define F_TLSINSPECT 0x04000000
> #define F_HASHKEY 0x08000000
> #define F_SNMP_TRAPONLY 0x10000000
> +#define F_TLSVERIFY 0x20000000
>
> #define F_BITS
> \
> "\10\01DISABLE\02BACKUP\03USED\04DOWN\05ADD\06DEL\07CHANGED" \
> @@ -719,6 +721,7 @@ struct protocol {
> char tlscacert[PATH_MAX];
> char tlscakey[PATH_MAX];
> char *tlscapass;
> + char tlsclientca[PATH_MAX];
> char name[MAX_NAME_SIZE];
> int tickets;
> enum prototype type;
> @@ -798,6 +801,7 @@ struct relay {
> int rl_tls_cert_fd;
> int rl_tls_ca_fd;
> int rl_tls_cacert_fd;
> + int rl_tls_client_ca_fd;
> char *rl_tls_key;
> EVP_PKEY *rl_tls_pkey;
> X509 *rl_tls_cacertx509;
> Index: regress/usr.sbin/relayd/Client.pm
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/regress/usr.sbin/relayd/Client.pm,v
> retrieving revision 1.12
> retrieving revision 1.12.12.2
> diff -u -p -r1.12 -r1.12.12.2
> --- regress/usr.sbin/relayd/Client.pm 22 Sep 2016 01:16:29 -0000 1.12
> +++ regress/usr.sbin/relayd/Client.pm 24 Nov 2018 16:28:45 -0000
> 1.12.12.2
> @@ -57,6 +57,11 @@ sub child {
> PeerAddr => $self->{connectaddr},
> PeerPort => $self->{connectport},
> SSL_verify_mode => SSL_VERIFY_NONE,
> + SSL_use_cert => $self->{offertlscert} ? 1 : 0,
> + SSL_cert_file => $self->{offertlscert} ?
> + "client.crt" : "",
> + SSL_key_file => $self->{offertlscert} ?
> + "client.key" : "",
> ) or die ref($self), " $iosocket socket connect failed: $!,$SSL_ERROR";
> if ($self->{sndbuf}) {
> setsockopt($cs, SOL_SOCKET, SO_SNDBUF,
> @@ -86,6 +91,12 @@ sub child {
> print STDERR "ssl cipher: ",$cs->get_cipher(),"\n";
> print STDERR "ssl peer certificate:\n",
> $cs->dump_peer_certificate();
> +
> + if ($self->{offertlscert}) {
> + print STDERR "ssl client certificate:\n";
> + print STDERR "Subject Name:
> ${\$cs->sock_certificate('subject')}\n";
> + print STDERR "Issuer Name:
> ${\$cs->sock_certificate('issuer')}\n";
length
> + }
> }
>
> *STDIN = *STDOUT = $self->{cs} = $cs;
> Index: regress/usr.sbin/relayd/Makefile
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/regress/usr.sbin/relayd/Makefile,v
> retrieving revision 1.15
> retrieving revision 1.15.6.3
> diff -u -p -r1.15 -r1.15.6.3
> --- regress/usr.sbin/relayd/Makefile 6 Oct 2018 10:52:24 -0000 1.15
> +++ regress/usr.sbin/relayd/Makefile 24 Nov 2018 16:36:09 -0000 1.15.6.3
> @@ -96,7 +96,16 @@ server.req:
> server.crt: ca.crt server.req
> openssl x509 -CAcreateserial -CAkey ca.key -CA ca.crt -req -in
> server.req -out server.crt
>
> -${REGRESS_TARGETS:M*ssl*} ${REGRESS_TARGETS:M*https*}: server.crt
> +client-ca.crt:
> + openssl req -batch -new -subj
> /L=OpenBSD/O=relayd-regress/OU=client-ca/CN=root/ -nodes -newkey rsa -keyout
> client-ca.key -x509 -out client-ca.crt
> +
> +client.req:
> + openssl req -batch -new -subj
> /L=OpenBSD/O=relayd-regress/OU=client/CN=localhost/ -nodes -newkey rsa
> -keyout client.key -out client.req
length
> +
> +client.crt: client-ca.crt client.req
> + openssl x509 -CAcreateserial -CAkey client-ca.key -CA client-ca.crt
> -req -in client.req -out client.crt
length
> +
> +${REGRESS_TARGETS:M*ssl*} ${REGRESS_TARGETS:M*https*}: server.crt client.crt
> .if empty (REMOTE_SSH)
> ${REGRESS_TARGETS:M*ssl*} ${REGRESS_TARGETS:M*https*}: 127.0.0.1.crt
> .else
> Index: regress/usr.sbin/relayd/Relayd.pm
> ===================================================================
> RCS file: /home/kivikakk/cvsync/root/src/regress/usr.sbin/relayd/Relayd.pm,v
> retrieving revision 1.17
> retrieving revision 1.17.4.3
> diff -u -p -r1.17 -r1.17.4.3
> --- regress/usr.sbin/relayd/Relayd.pm 20 Oct 2018 10:49:09 -0000 1.17
> +++ regress/usr.sbin/relayd/Relayd.pm 24 Nov 2018 16:36:09 -0000 1.17.4.3
> @@ -84,6 +84,9 @@ sub new {
> print $fh "\n\ttls ca cert ca.crt";
> print $fh "\n\ttls ca key ca.key password ''";
> }
> + if ($self->{verifyclient}) {
> + print $fh "\n\ttls client ca client-ca.crt";
> + }
> # substitute variables in config file
> foreach (@protocol) {
> s/(\$[a-z]+)/$1/eeg;
> Index: regress/usr.sbin/relayd/args-ssl-client-verify.pl
> ===================================================================
> RCS file: regress/usr.sbin/relayd/args-ssl-client-verify.pl
> diff -N regress/usr.sbin/relayd/args-ssl-client-verify.pl
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ regress/usr.sbin/relayd/args-ssl-client-verify.pl 24 Nov 2018 15:29:47
> -0000 1.1.2.2
> @@ -0,0 +1,19 @@
> +# test client ssl certificate verification
> +
> +use strict;
> +use warnings;
> +
> +our %args = (
> + client => {
> + ssl => 1,
> + offertlscert => 1,
> + },
> + relayd => {
> + listenssl => 1,
> + verifyclient => 1,
> + },
> + len => 251,
> + md5 => "bc3a3f39af35fe5b1687903da2b00c7f",
> +);
> +
> +1;
>