Hi Jack, I'm not a developer (just a contributor), but I worked on httpd client certs a year ago, too. (https://marc.info/?t=145285926100003&r=1&w=2)
I got a private response from a developer, who had an own similar diff in preparation. He told me that is better to name configuration option "client ca <file>" instead of "ca <file>". You could adapt my regression test to make sure the feature does not break in future development: https://marc.info/?l=openbsd-tech&m=146289968117988&w=2 I had not time to test your diff, but I like it. I also need this feature in httpd. In response of your second mail: I prefer the Idea to handle revocations by a CRL which is checked by libtls in case of client certificates. btw: As far as I know, there is a source tree lock at the moment. Thus, it may take some time to integrate you diff. Just my two cent, Jan On Thu, Mar 30, 2017 at 10:17:46PM +1030, Jack Burton wrote: > One of our sites has a need to require/verify TLS client certs, > without the overhead & complexity of apache / nginx, etc. > > OpenBSD's httpd seemed the obvious candidate, and I figured that the > feature would be useful to others too -- see attached diff for an > initial implementation. > > Of course, there are two parts to verifying TLS client certs: > > 1. Checking that the certificate was issued by a locally-trusted CA and > has not yet expired. That's fairly straightforward and (I would hope) > non-controversial. The attached diff adds that to httpd and also > exposes as new fastcgi variables some of the more useful client cert > data that libtls provides (since fastcgi responders are likely where > identity is of most interest). > > 2. Validating that the certificate has not yet been revoked. The > attached diff does NOT address that part, but that's what I intend to > turn my attention to next, if there's any interest in adding TLS client > cert support to httpd. I'll send a separate post with my thoughts on > that shortly (as there's a bunch of ways to do it, so some discussion > on which to prefer is probably in order). > > So, my question here for the devs is: does this fit in with the intended > direction of httpd(8)? (if so, I'll work on part 2 next; if not, I'll > stop bothering you) > > This is the first time I've submitted a diff with a proposed new feature > (as opposed to just a bug fix) for any part of OpenBSD, so if I've gone > about any part of it in the wrong way, please correct me & I'll try to > do better next time. > > Index: config.c > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/config.c,v > retrieving revision 1.51 > diff -u -p -r1.51 config.c > --- config.c 25 Mar 2017 17:25:34 -0000 1.51 > +++ config.c 30 Mar 2017 10:03:42 -0000 > @@ -326,6 +326,28 @@ config_settls(struct httpd *env, struct > } > } > > + if (srv_conf->tls_ca_len != 0) { > + DPRINTF("%s: sending ca cert(s) for \"%s[%u]\" to %s fd %d", > + __func__, srv_conf->name, srv_conf_id, > + ps->ps_title[PROC_SERVER], srv->srv_s); > + > + memset(&tls, 0, sizeof(tls)); > + tls.id = srv_conf->id; > + tls.tls_ca_len = srv_conf->tls_ca_len; > + > + c = 0; > + iov[c].iov_base = &tls; > + iov[c++].iov_len = sizeof(tls); > + iov[c].iov_base = srv_conf->tls_ca; > + iov[c++].iov_len = srv_conf->tls_ca_len; > + > + if (proc_composev(ps, PROC_SERVER, IMSG_CFG_TLS, iov, c) != 0) { > + log_warn("%s: failed to compose IMSG_CFG_TLS imsg for " > + "`%s'", __func__, srv_conf->name); > + return (-1); > + } > + } > + > return (0); > } > > @@ -644,6 +666,13 @@ config_gettls(struct httpd *env, struct > tls_conf.tls_ocsp_staple_len)) == NULL) > goto fail; > s += tls_conf.tls_ocsp_staple_len; > + } > + if (tls_conf.tls_ca_len != 0) { > + srv_conf->tls_ca_len = tls_conf.tls_ca_len; > + if ((srv_conf->tls_ca = get_data(p + s, > + tls_conf.tls_ca_len)) == NULL) > + goto fail; > + s += tls_conf.tls_ca_len; > } > > return (0); > Index: httpd.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v > retrieving revision 1.81 > diff -u -p -r1.81 httpd.conf.5 > --- httpd.conf.5 25 Mar 2017 17:25:34 -0000 1.81 > +++ httpd.conf.5 30 Mar 2017 10:03:43 -0000 > @@ -342,6 +342,18 @@ The revision of the HTTP specification u > .It Ic SERVER_SOFTWARE > The server software name of > .Xr httpd 8 . > +.It Ic TLS_PEER_EXPIRES > +The time at which the TLS client certificate, if any, expires, > +in seconds since the epoch. > +.It Ic TLS_PEER_HASH > +A hash of the TLS client certificate, if any. See > +.Xr tls_peer_cert_hash 3 . > +.It Ic TLS_PEER_ISSUER > +The issuer of the TLS client certificate, if any. > +.It Ic TLS_PEER_OCSP_URL > +The OCSP URL from the TLS client certificate, if any. > +.It Ic TLS_PEER_SUBJECT > +The subject of the TLS client certificate, if any. > .El > .It Ic hsts Oo Ar option Oc > Enable HTTP Strict Transport Security. > @@ -511,6 +523,14 @@ Set the TLS configuration for the server > These options are only used if TLS has been enabled via the listen directive. > Valid options are: > .Bl -tag -width Ds > +.It Ic ca Ar file > +Specify the CA certificate(s) for this server to use when verifying client > +certificates. > +The > +.Ar file > +should contain one or more PEM encoded CA certificates. > +The default is > +.Pa /etc/ssl/ca.crt . > .It Ic certificate Ar file > Specify the certificate to use for this server. > The > @@ -571,6 +591,11 @@ session lifetime. > It is possible to set > .Ar seconds > to default to use the httpd default timeout of 2 hours. > +.It Ic verify > +Require a valid TLS client certificate (checked using the CA certificate(s) > +specified by the > +.Ic tls ca > +option) for a TLS connection to proceed beyond the handshake. > .El > .El > .Sh TYPES > Index: httpd.h > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v > retrieving revision 1.131 > diff -u -p -r1.131 httpd.h > --- httpd.h 25 Mar 2017 17:25:34 -0000 1.131 > +++ httpd.h 30 Mar 2017 10:03:43 -0000 > @@ -55,6 +55,7 @@ > #define HTTPD_ERROR_LOG "error.log" > #define HTTPD_DEFAULT_TYPE { "bin", "application", "octet-stream", NULL } > #define HTTPD_LOGVIS VIS_NL|VIS_TAB|VIS_CSTYLE > +#define HTTPD_TLS_CA "/etc/ssl/ca.crt" > #define HTTPD_TLS_CERT "/etc/ssl/server.crt" > #define HTTPD_TLS_KEY "/etc/ssl/private/server.key" > #define HTTPD_TLS_CIPHERS "compat" > @@ -475,10 +476,14 @@ struct server_config { > uint32_t maxrequests; > size_t maxrequestbody; > > + uint8_t *tls_ca; > + char *tls_ca_file; > + size_t tls_ca_len; > uint8_t *tls_cert; > size_t tls_cert_len; > char *tls_cert_file; > char tls_ciphers[NAME_MAX]; > + int tls_client_verify; > char tls_dhe_params[NAME_MAX]; > char tls_ecdhe_curve[NAME_MAX]; > uint8_t *tls_key; > @@ -521,6 +526,7 @@ TAILQ_HEAD(serverhosts, server_config); > struct tls_config { > uint32_t id; > > + size_t tls_ca_len; > size_t tls_cert_len; > size_t tls_key_len; > size_t tls_ocsp_staple_len; > @@ -586,6 +592,7 @@ int cmdline_symset(char *); > /* server.c */ > void server(struct privsep *, struct privsep_proc *); > int server_tls_cmp(struct server *, struct server *, int); > +int server_tls_load_ca(struct server *); > int server_tls_load_keypair(struct server *); > int server_tls_load_ocsp(struct server *); > void server_generate_ticket_key(struct server_config *); > Index: parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v > retrieving revision 1.90 > diff -u -p -r1.90 parse.y > --- parse.y 25 Mar 2017 17:25:34 -0000 1.90 > +++ parse.y 30 Mar 2017 10:03:43 -0000 > @@ -129,11 +129,12 @@ typedef struct { > > %} > > -%token ACCESS ALIAS AUTO BACKLOG BODY BUFFER CERTIFICATE CHROOT > CIPHERS COMMON > -%token COMBINED CONNECTION DHE DIRECTORY ECDHE ERR FCGI INDEX IP KEY > LIFETIME > -%token LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON > PORT PREFORK > -%token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG > TCP TICKET > -%token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD > REQUEST > +%token ACCESS ALIAS AUTO BACKLOG BODY BUFFER CA CERTIFICATE CHROOT > CIPHERS > +%token COMMON COMBINED CONNECTION DHE DIRECTORY ECDHE ERR FCGI INDEX > IP KEY > +%token LIFETIME LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY > OCSP ON > +%token PORT PREFORK PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP > STYLE > +%token SYSLOG TCP TICKET TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS > VERIFY > +%token DEFAULT PRELOAD REQUEST > %token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS > %token <v.string> STRING > %token <v.number> NUMBER > @@ -254,6 +255,9 @@ server : SERVER optmatch STRING { > s->srv_conf.flags |= SRVFLAG_SERVER_MATCH; > s->srv_conf.logformat = LOG_FORMAT_COMMON; > s->srv_conf.tls_protocols = TLS_PROTOCOLS_DEFAULT; > + if ((s->srv_conf.tls_ca_file = > + strdup(HTTPD_TLS_CA)) == NULL) > + fatal("out of memory"); > if ((s->srv_conf.tls_cert_file = > strdup(HTTPD_TLS_CERT)) == NULL) > fatal("out of memory"); > @@ -344,6 +348,14 @@ server : SERVER optmatch STRING { > YYERROR; > } > > + if (server_tls_load_ca(srv) == -1) { > + yyerror("server \"%s\": failed to load " > + "ca cert(s)", srv->srv_conf.name); > + serverconfig_free(srv_conf); > + free(srv); > + YYERROR; > + } > + > if (server_tls_load_ocsp(srv) == -1) { > yyerror("server \"%s\": failed to load " > "ocsp staple", srv->srv_conf.name); > @@ -727,6 +739,15 @@ tlsopts : CERTIFICATE STRING { > fatal("out of memory"); > free($2); > } > + | CA STRING { > + free(srv_conf->tls_ca_file); > + if ((srv_conf->tls_ca_file = strdup($2)) == NULL) > + fatal("out of memory"); > + free($2); > + } > + | VERIFY { > + srv_conf->tls_client_verify = 1; > + } > | CIPHERS STRING { > if (strlcpy(srv_conf->tls_ciphers, $2, > sizeof(srv_conf->tls_ciphers)) >= > @@ -1217,6 +1238,7 @@ lookup(char *s) > { "block", BLOCK }, > { "body", BODY }, > { "buffer", BUFFER }, > + { "ca", CA }, > { "certificate", CERTIFICATE }, > { "chroot", CHROOT }, > { "ciphers", CIPHERS }, > @@ -1269,6 +1291,7 @@ lookup(char *s) > { "tls", TLS }, > { "type", TYPE }, > { "types", TYPES }, > + { "verify", VERIFY }, > { "with", WITH } > }; > const struct keywords *p; > @@ -2043,6 +2066,9 @@ server_inherit(struct server *src, struc > > /* Copy the source server and assign a new Id */ > memcpy(&dst->srv_conf, &src->srv_conf, sizeof(dst->srv_conf)); > + if ((dst->srv_conf.tls_ca_file = > + strdup(src->srv_conf.tls_ca_file)) == NULL) > + fatal("out of memory"); > if ((dst->srv_conf.tls_cert_file = > strdup(src->srv_conf.tls_cert_file)) == NULL) > fatal("out of memory"); > @@ -2080,6 +2106,7 @@ server_inherit(struct server *src, struc > dst->srv_conf.flags |= SRVFLAG_TLS; > else > dst->srv_conf.flags &= ~SRVFLAG_TLS; > + dst->srv_conf.tls_client_verify = src->srv_conf.tls_client_verify; > > /* Don't inherit the "match" option, use it from the alias */ > dst->srv_conf.flags &= ~SRVFLAG_SERVER_MATCH; > @@ -2087,6 +2114,14 @@ server_inherit(struct server *src, struc > > if (server_tls_load_keypair(dst) == -1) { > yyerror("failed to load public/private keys " > + "for server %s", dst->srv_conf.name); > + serverconfig_free(&dst->srv_conf); > + free(dst); > + return (NULL); > + } > + > + if (server_tls_load_ca(dst) == -1) { > + yyerror("failed to load ca cert(s) " > "for server %s", dst->srv_conf.name); > serverconfig_free(&dst->srv_conf); > free(dst); > Index: server.c > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/server.c,v > retrieving revision 1.108 > diff -u -p -r1.108 server.c > --- server.c 25 Mar 2017 17:25:34 -0000 1.108 > +++ server.c 30 Mar 2017 10:03:44 -0000 > @@ -197,6 +197,26 @@ server_tls_load_ocsp(struct server *srv) > } > > int > +server_tls_load_ca(struct server *srv) > +{ > + if ((srv->srv_conf.flags & SRVFLAG_TLS) == 0 || > + srv->srv_conf.tls_client_verify == 0) > + return (0); > + > + if (srv->srv_conf.tls_ca_file == NULL) > + return (0); > + > + if ((srv->srv_conf.tls_ca = tls_load_file( > + srv->srv_conf.tls_ca_file, > + &srv->srv_conf.tls_ca_len, NULL)) == NULL) > + return (-1); > + log_debug("%s: using ca cert(s) from %s", __func__, > + srv->srv_conf.tls_ca_file); > + > + return (0); > +} > + > +int > server_tls_init(struct server *srv) > { > struct server_config *srv_conf; > @@ -254,6 +274,15 @@ server_tls_init(struct server *srv) > return (-1); > } > > + if (srv->srv_conf.tls_ca != NULL && srv->srv_conf.tls_client_verify) { > + if (tls_config_set_ca_mem(srv->srv_tls_config, > + srv->srv_conf.tls_ca, srv->srv_conf.tls_ca_len) != 0) { > + log_warnx("%s: failed to add ca cert(s)", __func__); > + return (-1); > + } > + tls_config_verify_client(srv->srv_tls_config); > + } > + > TAILQ_FOREACH(srv_conf, &srv->srv_hosts, entry) { > if (srv_conf->tls_cert == NULL || srv_conf->tls_key == NULL) > continue; > @@ -267,6 +296,16 @@ server_tls_init(struct server *srv) > log_warnx("%s: failed to add tls keypair", __func__); > return (-1); > } > + if (srv_conf->tls_ca != NULL) { > + log_debug("%s: adding ca cert(s) for server %s", > + __func__, srv->srv_conf.name); > + if (tls_config_set_ca_mem(srv->srv_tls_config, > + srv_conf->tls_ca, srv_conf->tls_ca_len) != 0) { > + log_warnx("%s: failed to add ca cert(s)", > + __func__); > + return (-1); > + } > + } > } > > /* set common session ID among all processes */ > @@ -414,6 +453,8 @@ void > serverconfig_free(struct server_config *srv_conf) > { > free(srv_conf->return_uri); > + free(srv_conf->tls_ca_file); > + free(srv_conf->tls_ca); > free(srv_conf->tls_cert_file); > free(srv_conf->tls_key_file); > free(srv_conf->tls_ocsp_staple_file); > @@ -435,6 +476,8 @@ serverconfig_reset(struct server_config > { > srv_conf->auth = NULL; > srv_conf->return_uri = NULL; > + srv_conf->tls_ca = NULL; > + srv_conf->tls_ca_file = NULL; > srv_conf->tls_cert = NULL; > srv_conf->tls_cert_file = NULL; > srv_conf->tls_key = NULL; > Index: server_fcgi.c > =================================================================== > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v > retrieving revision 1.74 > diff -u -p -r1.74 server_fcgi.c > --- server_fcgi.c 21 Jan 2017 11:32:04 -0000 1.74 > +++ server_fcgi.c 30 Mar 2017 10:03:44 -0000 > @@ -282,11 +282,38 @@ server_fcgi(struct httpd *env, struct cl > goto fail; > } > > - if (srv_conf->flags & SRVFLAG_TLS) > + if (srv_conf->flags & SRVFLAG_TLS) { > if (fcgi_add_param(¶m, "HTTPS", "on", clt) == -1) { > errstr = "failed to encode param"; > goto fail; > } > + if (srv_conf->tls_client_verify) { > + if (fcgi_add_param(¶m, "TLS_PEER_SUBJECT", > + tls_peer_cert_subject(clt->clt_tls_ctx), clt) == -1 > + || fcgi_add_param(¶m, "TLS_PEER_ISSUER", > + tls_peer_cert_issuer(clt->clt_tls_ctx), clt) == -1 > + || fcgi_add_param(¶m, "TLS_PEER_HASH", > + tls_peer_cert_hash(clt->clt_tls_ctx), clt) == -1) { > + errstr = "failed to encode param"; > + goto fail; > + } > + if (asprintf(&str, "%lld", > + tls_peer_cert_notafter(clt->clt_tls_ctx)) == -1 > + || fcgi_add_param(¶m, "TLS_PEER_EXPIRES", str, > + clt) == -1) { > + errstr = "failed to encode param"; > + goto fail; > + } > + free(str); > + str = NULL; > + if (tls_peer_ocsp_url(clt->clt_tls_ctx) != NULL > + && fcgi_add_param(¶m, "TLS_PEER_OCSP_URL", > + tls_peer_ocsp_url(clt->clt_tls_ctx), clt) == -1) { > + errstr = "failed to encode param"; > + goto fail; > + } > + } > + } > > (void)print_host(&clt->clt_ss, hbuf, sizeof(hbuf)); > if (fcgi_add_param(¶m, "REMOTE_ADDR", hbuf, clt) == -1) { >
