Hi Jack,

On Wed, May 16, 2018 at 05:32:56PM +0930, Jack Burton wrote:
> I figured that if we can agree on this much, so httpd can be used for
> the authentication-only case (which is all non-fastcgi sites would want)
> straight away, that's be a good first step -- then we can come back and
> argue the toss over how much client cert data is necessary/sufficient
> to pass through for authorisation/accounting purposes.

I agree with you.  Lets commit this first step.

I tested your diff on current amd64 and sparc64 with an own PKI [1] and
the firefox browser.  Every thing works for me.  I also looked down your
source code which also seams fine to me.

I tested the optional and non-optional "client ca" configuration, as
well as the revocation and the fastcgi environment feature on both
architectures.  Everything works so far.

Hopefully, it will committed this time! :-)

Thanks!
Jan

[1]: https://github.com/younix/ca

> There's also a trivial regression test (unchanged from last year), which
> I'll post again separately next.
> 
> 
> Index: config.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/config.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 config.c
> --- config.c  19 Jul 2017 17:36:25 -0000      1.53
> +++ config.c  16 May 2018 07:59:10 -0000
> @@ -304,10 +304,18 @@ config_setserver_tls(struct httpd *env, 
>  
>       log_debug("%s: configuring tls for %s", __func__, srv_conf->name);
>  
> +     if (config_settls(env, srv, TLS_CFG_CA, "ca", srv_conf->tls_ca,
> +         srv_conf->tls_ca_len) != 0)
> +             return (-1);
> +
>       if (config_settls(env, srv, TLS_CFG_CERT, "cert", srv_conf->tls_cert,
>           srv_conf->tls_cert_len) != 0)
>               return (-1);
>  
> +     if (config_settls(env, srv, TLS_CFG_CRL, "crl", srv_conf->tls_crl,
> +         srv_conf->tls_crl_len) != 0)
> +             return (-1);
> +
>       if (config_settls(env, srv, TLS_CFG_KEY, "key", srv_conf->tls_key,
>           srv_conf->tls_key_len) != 0)
>               return (-1);
> @@ -431,6 +439,7 @@ config_getserver_config(struct httpd *en
>  
>               f = SRVFLAG_TLS;
>               srv_conf->flags |= parent->flags & f;
> +             srv_conf->tls_flags = parent->tls_flags;
>  
>               f = SRVFLAG_ACCESS_LOG;
>               if ((srv_conf->flags & f) == 0) {
> @@ -655,9 +664,21 @@ config_getserver_tls(struct httpd *env, 
>       }
>  
>       switch (tls_conf.tls_type) {
> +     case TLS_CFG_CA:
> +             if (config_gettls(env, srv_conf, &tls_conf, "ca", p, len,
> +                 &srv_conf->tls_ca, &srv_conf->tls_ca_len) != 0)
> +                     goto fail;
> +             break;
> +
>       case TLS_CFG_CERT:
>               if (config_gettls(env, srv_conf, &tls_conf, "cert", p, len,
>                   &srv_conf->tls_cert, &srv_conf->tls_cert_len) != 0)
> +                     goto fail;
> +             break;
> +
> +     case TLS_CFG_CRL:
> +             if (config_gettls(env, srv_conf, &tls_conf, "crl", p, len,
> +                 &srv_conf->tls_crl, &srv_conf->tls_crl_len) != 0)
>                       goto fail;
>               break;
>  
> Index: httpd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.90
> diff -u -p -r1.90 httpd.conf.5
> --- httpd.conf.5      11 Apr 2018 15:50:46 -0000      1.90
> +++ httpd.conf.5      16 May 2018 07:59:10 -0000
> @@ -342,6 +342,10 @@ The revision of the HTTP specification u
>  .It Ic SERVER_SOFTWARE
>  The server software name of
>  .Xr httpd 8 .
> +.It Ic TLS_PEER_VERIFY
> +A variable that is set to a comma separated list of TLS client verification
> +features in use
> +.Pq omitted when TLS client verification is not in use .
>  .El
>  .It Ic hsts Oo Ar option Oc
>  Enable HTTP Strict Transport Security.
> @@ -526,6 +530,23 @@ 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 cafile Oo Ic crl Ar crlfile Oc Op Ic optional
> +Require
> +.Po
> +or, if
> +.Ic optional
> +is specified, request but do not require
> +.Pc
> +TLS client certificates whose authenticity can be verified
> +against the CA certificate(s) in
> +.Ar cafile
> +in order to proceed beyond the TLS handshake.
> +With
> +.Ic crl
> +specified, additionally require that no certificate in the client chain be
> +listed as revoked in the CRL(s) in
> +.Ar crlfile .
> +CA certificates and CRLs should be PEM encoded.
>  .It Ic dhe Ar params
>  Specify the DHE parameters to use for DHE cipher suites.
>  Valid parameter values are none, legacy and auto.
> Index: httpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.136
> diff -u -p -r1.136 httpd.h
> --- httpd.h   11 Apr 2018 15:50:46 -0000      1.136
> +++ httpd.h   16 May 2018 07:59:10 -0000
> @@ -424,6 +424,11 @@ SPLAY_HEAD(client_tree, client);
>  #define HSTSFLAG_PRELOAD     0x02
>  #define HSTSFLAG_BITS                "\10\01SUBDOMAINS\02PRELOAD"
>  
> +#define TLSFLAG_CA           0x01
> +#define TLSFLAG_CRL          0x02
> +#define TLSFLAG_OPTIONAL     0x04
> +#define TLSFLAG_BITS         "\10\01CA\02CRL\3OPTIONAL"
> +
>  enum log_format {
>       LOG_FORMAT_COMMON,
>       LOG_FORMAT_COMBINED,
> @@ -480,12 +485,19 @@ 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[HTTPD_TLS_CONFIG_MAX];
> +     uint8_t                 *tls_crl;
> +     char                    *tls_crl_file;
> +     size_t                   tls_crl_len;
>       char                     tls_dhe_params[HTTPD_TLS_CONFIG_MAX];
>       char                     tls_ecdhe_curves[HTTPD_TLS_CONFIG_MAX];
> +     uint8_t                  tls_flags;
>       uint8_t                 *tls_key;
>       size_t                   tls_key_len;
>       char                    *tls_key_file;
> @@ -524,7 +536,9 @@ struct server_config {
>  TAILQ_HEAD(serverhosts, server_config);
>  
>  enum tls_config_type {
> +     TLS_CFG_CA,
>       TLS_CFG_CERT,
> +     TLS_CFG_CRL,
>       TLS_CFG_KEY,
>       TLS_CFG_OCSP_STAPLE,
>  };
> @@ -598,6 +612,8 @@ 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_crl(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.94
> diff -u -p -r1.94 parse.y
> --- parse.y   26 Apr 2018 14:12:19 -0000      1.94
> +++ parse.y   16 May 2018 07:59:11 -0000
> @@ -134,6 +134,7 @@ typedef struct {
>  %token       PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG 
> TCP TICKET
>  %token       TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD 
> REQUEST
>  %token       ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS
> +%token       CA CLIENT CRL OPTIONAL
>  %token       <v.string>      STRING
>  %token  <v.number>   NUMBER
>  %type        <v.port>        port
> @@ -345,6 +346,22 @@ 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_crl(srv) == -1) {
> +                             yyerror("server \"%s\": failed to load crl(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);
> @@ -587,6 +604,7 @@ serveroptsl       : LISTEN ON STRING opttls po
>                           sizeof(s->srv_conf.ss));
>                       s->srv_conf.port = srv->srv_conf.port;
>                       s->srv_conf.prefixlen = srv->srv_conf.prefixlen;
> +                     s->srv_conf.tls_flags = srv->srv_conf.tls_flags;
>  
>                       if (last_server_id == INT_MAX) {
>                               yyerror("too many servers/locations defined");
> @@ -760,6 +778,13 @@ tlsopts          : CERTIFICATE STRING            {
>                       }
>                       free($2);
>               }
> +             | CLIENT CA STRING tlsclientopt {
> +                     srv_conf->tls_flags |= TLSFLAG_CA;
> +                     free(srv_conf->tls_ca_file);
> +                     if ((srv_conf->tls_ca_file = strdup($3)) == NULL)
> +                             fatal("out of memory");
> +                     free($3);
> +             }
>               | DHE STRING                    {
>                       if (strlcpy(srv_conf->tls_dhe_params, $2,
>                           sizeof(srv_conf->tls_dhe_params)) >=
> @@ -808,6 +833,18 @@ tlsopts          : CERTIFICATE STRING            {
>               }
>               ;
>  
> +tlsclientopt : /* empty */
> +             | tlsclientopt CRL STRING       {
> +                     srv_conf->tls_flags = TLSFLAG_CRL;
> +                     free(srv_conf->tls_crl_file);
> +                     if ((srv_conf->tls_crl_file = strdup($3)) == NULL)
> +                             fatal("out of memory");
> +                     free($3);
> +             }
> +             | tlsclientopt OPTIONAL         {
> +                     srv_conf->tls_flags |= TLSFLAG_OPTIONAL;
> +             }
> +             ;
>  root         : ROOT rootflags
>               | ROOT '{' optnl rootflags_l '}'
>               ;
> @@ -1240,12 +1277,15 @@ lookup(char *s)
>               { "block",              BLOCK },
>               { "body",               BODY },
>               { "buffer",             BUFFER },
> +             { "ca",                 CA },
>               { "certificate",        CERTIFICATE },
>               { "chroot",             CHROOT },
>               { "ciphers",            CIPHERS },
> +             { "client",             CLIENT },
>               { "combined",           COMBINED },
>               { "common",             COMMON },
>               { "connection",         CONNECTION },
> +             { "crl",                CRL },
>               { "default",            DEFAULT },
>               { "dhe",                DHE },
>               { "directory",          DIRECTORY },
> @@ -1270,6 +1310,7 @@ lookup(char *s)
>               { "nodelay",            NODELAY },
>               { "ocsp",               OCSP },
>               { "on",                 ON },
> +             { "optional",           OPTIONAL },
>               { "pass",               PASS },
>               { "port",               PORT },
>               { "prefork",            PREFORK },
> @@ -2100,6 +2141,21 @@ server_inherit(struct server *src, struc
>               serverconfig_free(&dst->srv_conf);
>               free(dst);
>               return (NULL);
> +     }
> +
> +     if (server_tls_load_ca(dst) == -1) {
> +             yyerror("falied to load ca cert(s) for server %s",
> +                 dst->srv_conf.name);
> +             serverconfig_free(&dst->srv_conf);
> +             return NULL;
> +     }
> +
> +     if (server_tls_load_crl(dst) == -1) {
> +             yyerror("failed to load crl(s) for server %s",
> +                 dst->srv_conf.name);
> +             serverconfig_free(&dst->srv_conf);
> +             free(dst);
> +             return NULL;
>       }
>  
>       if (server_tls_load_ocsp(dst) == -1) {
> Index: server.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server.c,v
> retrieving revision 1.113
> diff -u -p -r1.113 server.c
> --- server.c  29 Nov 2017 16:55:08 -0000      1.113
> +++ server.c  16 May 2018 07:59:11 -0000
> @@ -134,6 +134,8 @@ server_tls_cmp(struct server *s1, struct
>       sc1 = &s1->srv_conf;
>       sc2 = &s2->srv_conf;
>  
> +     if (sc1->tls_flags != sc2->tls_flags)
> +             return (-1);
>       if (sc1->tls_protocols != sc2->tls_protocols)
>               return (-1);
>       if (sc1->tls_ticket_lifetime != sc2->tls_ticket_lifetime)
> @@ -207,6 +209,40 @@ server_tls_load_ocsp(struct server *srv)
>  }
>  
>  int
> +server_tls_load_ca(struct server *srv)
> +{
> +     if ((srv->srv_conf.tls_flags & TLSFLAG_CA) == 0 ||
> +         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_load_crl(struct server *srv)
> +{
> +     if ((srv->srv_conf.tls_flags & TLSFLAG_CA) == 0 ||
> +         srv->srv_conf.tls_crl_file == NULL)
> +             return (0);
> +
> +     if ((srv->srv_conf.tls_crl = tls_load_file(
> +         srv->srv_conf.tls_crl_file,
> +         &srv->srv_conf.tls_crl_len, NULL)) == NULL)
> +             return (-1);
> +     log_debug("%s: using crl(s) from %s", __func__,
> +         srv->srv_conf.tls_crl_file);
> +
> +     return (0);
> +}
> +
> +int
>  server_tls_init(struct server *srv)
>  {
>       struct server_config *srv_conf;
> @@ -264,6 +300,27 @@ server_tls_init(struct server *srv)
>               return (-1);
>       }
>  
> +     if (srv->srv_conf.tls_ca != NULL) {
> +             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);
> +             }
> +             if (srv->srv_conf.tls_flags & TLSFLAG_OPTIONAL)
> +                     tls_config_verify_client_optional(srv->srv_tls_config);
> +             else
> +                     tls_config_verify_client(srv->srv_tls_config);
> +
> +             if (srv->srv_conf.tls_crl != NULL) {
> +                     if (tls_config_set_crl_mem(srv->srv_tls_config,
> +                         srv->srv_conf.tls_crl,
> +                         srv->srv_conf.tls_crl_len) != 0) {
> +                             log_warnx("%s: failed to add crl(s)", __func__);
> +                             return (-1);
> +                     }
> +             }
> +     }
> +
>       TAILQ_FOREACH(srv_conf, &srv->srv_hosts, entry) {
>               if (srv_conf->tls_cert == NULL || srv_conf->tls_key == NULL)
>                       continue;
> @@ -277,6 +334,26 @@ server_tls_init(struct server *srv)
>                       log_warnx("%s: failed to add tls keypair", __func__);
>                       return (-1);
>               }
> +
> +             if (srv->srv_conf.tls_ca == NULL)
> +                     continue;
> +             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);
> +             }
> +
> +             if (srv->srv_conf.tls_crl == NULL)
> +                     continue;
> +
> +             log_debug("%s: adding crl(s) for server %s", __func__,
> +                 srv->srv_conf.name);
> +             if (tls_config_set_crl_mem(srv->srv_tls_config,
> +                 srv_conf->tls_crl, srv_conf->tls_crl_len) != 0) {
> +                     return (-1);
> +             }
>       }
>  
>       /* set common session ID among all processes */
> @@ -310,13 +387,19 @@ server_tls_init(struct server *srv)
>               return (-1);
>       }
>  
> -     /* We're now done with the public/private key... */
> +     /* We're now done with the public/private key & ca/crl... */
>       tls_config_clear_keys(srv->srv_tls_config);
>       freezero(srv->srv_conf.tls_cert, srv->srv_conf.tls_cert_len);
>       freezero(srv->srv_conf.tls_key, srv->srv_conf.tls_key_len);
> +     free(srv->srv_conf.tls_ca);
> +     free(srv->srv_conf.tls_crl);
> +     srv->srv_conf.tls_ca = NULL;
>       srv->srv_conf.tls_cert = NULL;
> +     srv->srv_conf.tls_crl = NULL;
>       srv->srv_conf.tls_key = NULL;
> +     srv->srv_conf.tls_ca_len = 0;
>       srv->srv_conf.tls_cert_len = 0;
> +     srv->srv_conf.tls_crl_len = 0;
>       srv->srv_conf.tls_key_len = 0;
>  
>       return (0);
> @@ -422,7 +505,11 @@ 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_crl_file);
> +     free(srv_conf->tls_crl);
>       free(srv_conf->tls_key_file);
>       free(srv_conf->tls_ocsp_staple_file);
>       free(srv_conf->tls_ocsp_staple);
> @@ -435,8 +522,12 @@ 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_crl = NULL;
> +     srv_conf->tls_crl_file = NULL;
>       srv_conf->tls_key = NULL;
>       srv_conf->tls_key_file = NULL;
>       srv_conf->tls_ocsp_staple = NULL;
> Index: server_fcgi.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 server_fcgi.c
> --- server_fcgi.c     31 Jul 2017 08:02:49 -0000      1.75
> +++ server_fcgi.c     16 May 2018 07:59:12 -0000
> @@ -282,11 +282,18 @@ 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(&param, "HTTPS", "on", clt) == -1) {
>                       errstr = "failed to encode param";
>                       goto fail;
>               }
> +             if (srv_conf->tls_flags != 0 && fcgi_add_param(&param,
> +                 "TLS_PEER_VERIFY", printb_flags(srv_conf->tls_flags,
> +                 TLSFLAG_BITS), clt) == -1) {
> +                     errstr = "failed to encode param";
> +                     goto fail;
> +             }
> +     }
>  
>       (void)print_host(&clt->clt_ss, hbuf, sizeof(hbuf));
>       if (fcgi_add_param(&param, "REMOTE_ADDR", hbuf, clt) == -1) {
> 

Reply via email to