Hi there, A year or two ago I submitted a patch for adding TLS client certificate validation to relayd. At the time it didn't make it in, and I stopped pursuing it further. (https://marc.info/?l=openbsd-tech&m=154509330608643&w=2)
I'd still like to see this landed, if at all possible. I'm continuing to use this feature on my own personal websites, and it works well. The latest diff is attached, or can be viewed online here: https://github.com/openbsd/src/compare/master...kivikakk:relayd-client-verification.patch I've added a test that confirms client failure to connect without a certificate at regress/usr.sbin/relayd/args-ssl-client-verify-fail.pl -- it's a bit awkward. Let me know if I can redo it better. Best, Ashe --- >From c63bca7ba7889b43e0a9317e807499eb8ca0db55 Mon Sep 17 00:00:00 2001 From: Asherah Connor <a...@kivikakk.ee> Date: Thu, 15 Oct 2020 17:23:15 +1100 Subject: [PATCH] TLS client certificate validation --- regress/usr.sbin/relayd/Client.pm | 13 ++++++++++ regress/usr.sbin/relayd/Makefile | 18 ++++++++++++- regress/usr.sbin/relayd/Relayd.pm | 3 +++ .../relayd/args-ssl-client-verify-fail.pl | 25 +++++++++++++++++++ .../usr.sbin/relayd/args-ssl-client-verify.pl | 19 ++++++++++++++ usr.sbin/relayd/config.c | 21 ++++++++++++++++ usr.sbin/relayd/parse.y | 15 ++++++++++- usr.sbin/relayd/relay.c | 21 ++++++++++++++++ usr.sbin/relayd/relayd.c | 9 +++++++ usr.sbin/relayd/relayd.conf.5 | 4 +++ usr.sbin/relayd/relayd.h | 14 +++++++---- 11 files changed, 155 insertions(+), 7 deletions(-) create mode 100644 regress/usr.sbin/relayd/args-ssl-client-verify-fail.pl create mode 100644 regress/usr.sbin/relayd/args-ssl-client-verify.pl diff --git a/regress/usr.sbin/relayd/Client.pm b/regress/usr.sbin/relayd/Client.pm index b3e011de13d..ec6fa64274e 100644 --- a/regress/usr.sbin/relayd/Client.pm +++ b/regress/usr.sbin/relayd/Client.pm @@ -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,14 @@ 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"; + } } *STDIN = *STDOUT = $self->{cs} = $cs; diff --git a/regress/usr.sbin/relayd/Makefile b/regress/usr.sbin/relayd/Makefile index cd01aa3fb63..f2198f43cc9 100644 --- a/regress/usr.sbin/relayd/Makefile +++ b/regress/usr.sbin/relayd/Makefile @@ -96,7 +96,23 @@ 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 + +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 + +${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 diff --git a/regress/usr.sbin/relayd/Relayd.pm b/regress/usr.sbin/relayd/Relayd.pm index 98f2ada5db9..896c0b401be 100644 --- a/regress/usr.sbin/relayd/Relayd.pm +++ b/regress/usr.sbin/relayd/Relayd.pm @@ -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; diff --git a/regress/usr.sbin/relayd/args-ssl-client-verify-fail.pl b/regress/usr.sbin/relayd/args-ssl-client-verify-fail.pl new file mode 100644 index 00000000000..93c9b22a63d --- /dev/null +++ b/regress/usr.sbin/relayd/args-ssl-client-verify-fail.pl @@ -0,0 +1,25 @@ +# test client ssl certificate verification + +use strict; +use warnings; + +our %args = ( + client => { + ssl => 1, + offertlscert => 0, + up => 'failed', + down => 'connect attempt failed', + dryrun => 1, + nocheck => 1, + }, + relayd => { + listenssl => 1, + verifyclient => 1, + }, + server => { + noserver => 1, + nocheck => 1, + }, +); + +1; diff --git a/regress/usr.sbin/relayd/args-ssl-client-verify.pl b/regress/usr.sbin/relayd/args-ssl-client-verify.pl new file mode 100644 index 00000000000..c5cc3110724 --- /dev/null +++ b/regress/usr.sbin/relayd/args-ssl-client-verify.pl @@ -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; diff --git a/usr.sbin/relayd/config.c b/usr.sbin/relayd/config.c index 3e60d63ef52..5b409aa1134 100644 --- a/usr.sbin/relayd/config.c +++ b/usr.sbin/relayd/config.c @@ -956,6 +956,15 @@ config_setrelay(struct relayd *env, struct relay *rlay) rlay->rl_conf.name); return (-1); } + if (rlay->rl_tls_client_ca_fd != -1 && + config_setrelayfd(ps, id, n, 0, + 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 " @@ -989,6 +998,10 @@ config_setrelay(struct relayd *env, struct relay *rlay) close(rlay->rl_s); rlay->rl_s = -1; } + if (rlay->rl_tls_client_ca_fd != -1) { + close(rlay->rl_tls_client_ca_fd); + rlay->rl_tls_client_ca_fd = -1; + } if (rlay->rl_tls_cacert_fd != -1) { close(rlay->rl_tls_cacert_fd); rlay->rl_tls_cacert_fd = -1; @@ -1014,6 +1027,10 @@ config_setrelay(struct relayd *env, struct relay *rlay) cert->cert_ocsp_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); } @@ -1036,6 +1053,7 @@ config_getrelay(struct relayd *env, struct imsg *imsg) rlay->rl_s = imsg->fd; 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) @@ -1165,6 +1183,9 @@ config_getrelayfd(struct relayd *env, struct imsg *imsg) 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; } DPRINTF("%s: %s %d received relay fd %d type %d for relay %s", __func__, diff --git a/usr.sbin/relayd/parse.y b/usr.sbin/relayd/parse.y index ce219245731..a524447aa72 100644 --- a/usr.sbin/relayd/parse.y +++ b/usr.sbin/relayd/parse.y @@ -179,7 +179,7 @@ typedef struct { %token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT TRAP URL WITH TTL RTABLE %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDHE %token EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES CHECKS -%token WEBSOCKETS +%token WEBSOCKETS CLIENT %token <v.string> STRING %token <v.number> NUMBER %type <v.string> context hostname interface table value optstring path @@ -1373,6 +1373,16 @@ tlsflags : SESSION TICKETS { proto->tickets = 1; } name->name = $2; TAILQ_INSERT_TAIL(&proto->tlscerts, name, entry); } + | 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; } ; @@ -1830,6 +1840,7 @@ relay : RELAY STRING { r->rl_conf.dstretry = 0; 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"); @@ -2423,6 +2434,7 @@ lookup(char *s) { "check", CHECK }, { "checks", CHECKS }, { "ciphers", CIPHERS }, + { "client", CLIENT }, { "code", CODE }, { "connection", CONNECTION }, { "context", CONTEXT }, @@ -3408,6 +3420,7 @@ relay_inherit(struct relay *ra, struct relay *rb) if (!(rb->rl_conf.flags & F_TLS)) { rb->rl_tls_cacert_fd = -1; rb->rl_tls_ca_fd = -1; + rb->rl_tls_client_ca_fd = -1; } TAILQ_INIT(&rb->rl_tables); diff --git a/usr.sbin/relayd/relay.c b/usr.sbin/relayd/relay.c index 43b5c377fa5..2f9f293368c 100644 --- a/usr.sbin/relayd/relay.c +++ b/usr.sbin/relayd/relay.c @@ -2259,6 +2259,27 @@ relay_tls_ctx_create(struct relay *rlay) } rlay->rl_tls_cacert_fd = -1; + if (rlay->rl_tls_client_ca_fd != -1) { + if ((buf = relay_load_fd(rlay->rl_tls_client_ca_fd, + &len)) == + NULL) { + log_warn( + "failed to read tls client CA certificate"); + goto err; + } + + if (tls_config_set_ca_mem(tls_cfg, buf, len) != 0) { + log_warnx( + "failed to set tls client CA cert: %s", + tls_config_error(tls_cfg)); + goto err; + } + purge_key(&buf, len); + + tls_config_verify_client(tls_cfg); + } + rlay->rl_tls_client_ca_fd = -1; + tls = tls_server(); if (tls == NULL) { log_warnx("unable to allocate TLS context"); diff --git a/usr.sbin/relayd/relayd.c b/usr.sbin/relayd/relayd.c index 9390b2e4606..52093f6cdc9 100644 --- a/usr.sbin/relayd/relayd.c +++ b/usr.sbin/relayd/relayd.c @@ -1359,6 +1359,15 @@ relay_load_certfiles(struct relayd *env, struct relay *rlay, const char *name) if ((rlay->rl_conf.flags & F_TLS) == 0) return (0); + if (strlen(proto->tlsclientca) && + rlay->rl_tls_client_ca_fd == -1) { + 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); + } + if (name == NULL && print_host(&rlay->rl_conf.ss, hbuf, sizeof(hbuf)) == NULL) goto fail; diff --git a/usr.sbin/relayd/relayd.conf.5 b/usr.sbin/relayd/relayd.conf.5 index 491fc216b71..fe6dff92174 100644 --- a/usr.sbin/relayd/relayd.conf.5 +++ b/usr.sbin/relayd/relayd.conf.5 @@ -946,6 +946,10 @@ will be used (strong crypto cipher suites without anonymous DH). See the CIPHERS section of .Xr openssl 1 for information about SSL/TLS cipher suites and preference lists. +.It Ic client ca Ar path +Require TLS client certificates whose authenticity can be verified +against the CA certificate(s) in the specified file in order to +proceed beyond the TLS handshake. .It Ic client-renegotiation Allow client-initiated renegotiation. To mitigate a potential DoS risk, diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h index 354c4e7d01f..5d7c71dcbff 100644 --- a/usr.sbin/relayd/relayd.h +++ b/usr.sbin/relayd/relayd.h @@ -139,11 +139,12 @@ struct ctl_relaytable { }; enum fd_type { - RELAY_FD_CERT = 1, - RELAY_FD_CACERT = 2, - RELAY_FD_CAFILE = 3, - RELAY_FD_KEY = 4, - RELAY_FD_OCSP = 5 + RELAY_FD_CERT = 1, + RELAY_FD_CACERT = 2, + RELAY_FD_CAFILE = 3, + RELAY_FD_KEY = 4, + RELAY_FD_OCSP = 5, + RELAY_FD_CLIENTCACERT = 6 }; struct ctl_relayfd { @@ -401,6 +402,7 @@ union hashkey { #define F_TLSINSPECT 0x04000000 #define F_HASHKEY 0x08000000 #define F_AGENTX_TRAPONLY 0x10000000 +#define F_TLSVERIFY 0x20000000 #define F_BITS \ "\10\01DISABLE\02BACKUP\03USED\04DOWN\05ADD\06DEL\07CHANGED" \ @@ -744,6 +746,7 @@ struct protocol { char tlscacert[PATH_MAX]; char tlscakey[PATH_MAX]; char *tlscapass; + char tlsclientca[PATH_MAX]; struct keynamelist tlscerts; char name[MAX_NAME_SIZE]; int tickets; @@ -833,6 +836,7 @@ struct relay { int rl_tls_ca_fd; int rl_tls_cacert_fd; + int rl_tls_client_ca_fd; EVP_PKEY *rl_tls_pkey; X509 *rl_tls_cacertx509; char *rl_tls_cakey;
