On Fri, Dec 14, 2018 at 10:58:06AM +0100, Sebastian Benoit wrote:
> 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.

Sounds good.


> As for style, please make lines not longer than 80 chars.

Thanks for the pick up, I'll be sure to keep that in mind in future.

Revised patch follows (includes mandoc changes).




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.8.1
diff -u -p -u -r1.36 -r1.36.8.1
--- usr.sbin/relayd/config.c    29 Nov 2017 15:24:50 -0000      1.36
+++ usr.sbin/relayd/config.c    30 Nov 2018 21:10:06 -0000      1.36.8.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.4.1
diff -u -p -u -r1.230 -r1.230.4.1
--- usr.sbin/relayd/parse.y     1 Nov 2018 00:18:44 -0000       1.230
+++ usr.sbin/relayd/parse.y     30 Nov 2018 21:10:06 -0000      1.230.4.1
@@ -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.6.2
diff -u -p -u -r1.241 -r1.241.6.2
--- usr.sbin/relayd/relay.c     19 Sep 2018 11:28:02 -0000      1.241
+++ usr.sbin/relayd/relay.c     12 Dec 2018 16:25:44 -0000      1.241.6.2
@@ -2176,6 +2176,26 @@ 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)) ==
+                           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);
+               }
+
                tls = tls_server();
                if (tls == NULL) {
                        log_warnx("unable to allocate TLS context");
@@ -2198,6 +2218,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 +2699,13 @@ 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);
+               }
        }
 
        if ((rlay->rl_conf.flags & F_TLS) == 0)
@@ -2712,6 +2740,14 @@ 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);
+       }
 
        return (0);
 }
Index: usr.sbin/relayd/relayd.conf.5
===================================================================
RCS file: /home/kivikakk/cvsync/root/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.187
retrieving revision 1.187.6.1
diff -u -p -u -r1.187 -r1.187.6.1
--- usr.sbin/relayd/relayd.conf.5       6 Aug 2018 18:26:29 -0000       1.187
+++ usr.sbin/relayd/relayd.conf.5       30 Nov 2018 21:10:06 -0000      
1.187.6.1
@@ -939,6 +939,10 @@ will be used (strong crypto cipher suite
 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,
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.6.1
diff -u -p -u -r1.251 -r1.251.6.1
--- usr.sbin/relayd/relayd.h    9 Sep 2018 21:06:51 -0000       1.251
+++ usr.sbin/relayd/relayd.h    30 Nov 2018 21:10:06 -0000      1.251.6.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 -u -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   12 Dec 2018 16:25:42 -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,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: /home/kivikakk/cvsync/root/src/regress/usr.sbin/relayd/Makefile,v
retrieving revision 1.15
retrieving revision 1.15.6.2
diff -u -p -u -r1.15 -r1.15.6.2
--- regress/usr.sbin/relayd/Makefile    6 Oct 2018 10:52:24 -0000       1.15
+++ regress/usr.sbin/relayd/Makefile    12 Dec 2018 16:25:42 -0000      1.15.6.2
@@ -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
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.1
diff -u -p -u -r1.17 -r1.17.4.1
--- regress/usr.sbin/relayd/Relayd.pm   20 Oct 2018 10:49:09 -0000      1.17
+++ regress/usr.sbin/relayd/Relayd.pm   30 Nov 2018 21:09:49 -0000      1.17.4.1
@@ -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   30 Nov 2018 21:09:49 
-0000      1.1.2.1
@@ -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;

Reply via email to