Download raw body.
[PATCH] relayd client certificate validation again
On 2024-10-28 19:22 +01, Theo Buehler <tb@theobuehler.org> wrote:
> On Mon, Oct 28, 2024 at 07:17:12PM +0100, Sören Tempel wrote:
>> Hi Theo,
>>
>> Sorry to bother you again about this, but as far as I can tell the patch
>> hasn't been merged yet. Did you want me to send an updated patch with
>> the man page change? I thought that you would just change that on
>> commit. If there is anything else that needs doing, just let me know.
>
> I can't commit this without getting the ok from another committer.
> Here's the diff I have in my tree. It is
>
> ok tb
>
> if anyone wants to commit or I'm happy to take an ok.
>
reads correct to me. I trust you know what you are doing with the TLS
bits.
OK florian
> Index: usr.sbin/relayd/config.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/config.c,v
> diff -u -p -r1.45 config.c
> --- usr.sbin/relayd/config.c 17 Jan 2024 10:01:24 -0000 1.45
> +++ usr.sbin/relayd/config.c 21 Oct 2024 07:14:24 -0000
> @@ -953,6 +953,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, 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 "
> @@ -986,6 +995,10 @@ config_setrelay(struct relayd *env, stru
> 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;
> @@ -1011,6 +1024,10 @@ config_setrelay(struct relayd *env, stru
> 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);
> }
> @@ -1033,6 +1050,7 @@ config_getrelay(struct relayd *env, stru
> rlay->rl_s = imsg_get_fd(imsg);
> 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)
> @@ -1161,6 +1179,9 @@ config_getrelayfd(struct relayd *env, st
> break;
> case RELAY_FD_CAFILE:
> rlay->rl_tls_cacert_fd = imsg_get_fd(imsg);
> + break;
> + case RELAY_FD_CLIENTCACERT:
> + rlay->rl_tls_client_ca_fd = imsg->fd;
> break;
> }
>
> Index: usr.sbin/relayd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> diff -u -p -r1.257 parse.y
> --- usr.sbin/relayd/parse.y 10 Aug 2024 05:47:29 -0000 1.257
> +++ usr.sbin/relayd/parse.y 21 Oct 2024 07:14:24 -0000
> @@ -179,7 +179,7 @@ typedef struct {
> %token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT 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 PFLOG
> +%token WEBSOCKETS PFLOG CLIENT
> %token <v.string> STRING
> %token <v.number> NUMBER
> %type <v.string> context hostname interface table value path
> @@ -1351,6 +1351,16 @@ tlsflags : SESSION TICKETS { proto->tick
> 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; }
> ;
> @@ -1822,6 +1832,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");
> @@ -2411,6 +2422,7 @@ lookup(char *s)
> { "check", CHECK },
> { "checks", CHECKS },
> { "ciphers", CIPHERS },
> + { "client", CLIENT },
> { "code", CODE },
> { "connection", CONNECTION },
> { "context", CONTEXT },
> @@ -3397,6 +3409,7 @@ relay_inherit(struct relay *ra, struct r
> 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);
>
> Index: usr.sbin/relayd/relay.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> diff -u -p -r1.259 relay.c
> --- usr.sbin/relayd/relay.c 17 Jan 2024 10:01:24 -0000 1.259
> +++ usr.sbin/relayd/relay.c 21 Oct 2024 07:14:24 -0000
> @@ -2159,8 +2159,7 @@ relay_tls_ctx_create(struct relay *rlay)
> tls_config_insecure_noverifyname(tls_client_cfg);
>
> if (rlay->rl_tls_ca_fd != -1) {
> - if ((buf = relay_load_fd(rlay->rl_tls_ca_fd, &len)) ==
> - NULL) {
> + if ((buf = relay_load_fd(rlay->rl_tls_ca_fd, &len)) == NULL) {
> log_warn("failed to read root certificates");
> goto err;
> }
> @@ -2250,6 +2249,26 @@ relay_tls_ctx_create(struct relay *rlay)
> goto err;
> }
> 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) {
> Index: usr.sbin/relayd/relayd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> diff -u -p -r1.191 relayd.c
> --- usr.sbin/relayd/relayd.c 25 Jun 2023 08:07:38 -0000 1.191
> +++ usr.sbin/relayd/relayd.c 21 Oct 2024 07:14:24 -0000
> @@ -1359,6 +1359,14 @@ relay_load_certfiles(struct relayd *env,
> 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;
> Index: usr.sbin/relayd/relayd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
> diff -u -p -r1.210 relayd.conf.5
> --- usr.sbin/relayd/relayd.conf.5 21 Sep 2024 05:37:26 -0000 1.210
> +++ usr.sbin/relayd/relayd.conf.5 28 Oct 2024 18:20:51 -0000
> @@ -954,6 +954,9 @@ will be used (strong crypto cipher suite
> See the CIPHERS section of
> .Xr openssl 1
> for information about TLS cipher suites and preference lists.
> +.It Ic client ca Ar path
> +Require TLS client certificates that can be verified against the CA
> +certificates in the specified file.
> .It Ic client-renegotiation
> Allow client-initiated renegotiation.
> To mitigate a potential DoS risk,
> Index: usr.sbin/relayd/relayd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
> diff -u -p -r1.275 relayd.h
> --- usr.sbin/relayd/relayd.h 8 Oct 2024 05:28:11 -0000 1.275
> +++ usr.sbin/relayd/relayd.h 21 Oct 2024 07:14:24 -0000
> @@ -137,11 +137,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 {
> @@ -744,6 +745,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 +835,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;
> Index: regress/usr.sbin/relayd/Client.pm
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/relayd/Client.pm,v
> diff -u -p -r1.14 Client.pm
> --- regress/usr.sbin/relayd/Client.pm 22 Dec 2021 11:50:28 -0000 1.14
> +++ regress/usr.sbin/relayd/Client.pm 21 Oct 2024 07:14:24 -0000
> @@ -58,6 +58,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,
> @@ -89,6 +94,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;
> Index: regress/usr.sbin/relayd/Makefile
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/relayd/Makefile,v
> diff -u -p -r1.21 Makefile
> --- regress/usr.sbin/relayd/Makefile 30 Dec 2021 20:51:34 -0000 1.21
> +++ regress/usr.sbin/relayd/Makefile 21 Oct 2024 07:14:24 -0000
> @@ -92,7 +92,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
> Index: regress/usr.sbin/relayd/Relayd.pm
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/relayd/Relayd.pm,v
> diff -u -p -r1.19 Relayd.pm
> --- regress/usr.sbin/relayd/Relayd.pm 12 Oct 2021 05:42:39 -0000 1.19
> +++ regress/usr.sbin/relayd/Relayd.pm 21 Oct 2024 07:14:24 -0000
> @@ -85,6 +85,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;
>
--
In my defence, I have been left unsupervised.
[PATCH] relayd client certificate validation again