Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: [PATCH] relayd client certificate validation again
To:
Sören Tempel <soeren@soeren-tempel.net>
Cc:
tech@openbsd.org, markus.l2ll@gmail.com, rivo@elnit.ee, brian@planetunix.net
Date:
Mon, 28 Oct 2024 19:22:10 +0100

Download raw body.

Thread
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.

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;