On Sat, 26 May 2018 22:44:56 +0930
Jack Burton <j...@saosce.com.au> wrote:
> This diff adds three new TLS_PEER_* fastcgi variables: ISSUER, SERIAL
> & SUBJECT.
> 
> That seems the logical next step in making tls client certs usable for
> authorisation & accounting. Last week's commit (thanks Joel!) took
> care of authentication.
> 
> The cert subject on its own should suffice for the simplest cases.
> 
> For anything more involved, {issuer,serial} is the bare minimum
> necessary to identify any certificate uniquely (at least within the
> realm of trusted CAs, which is all that's relevant after
> authentication).
> 
> That should be enough for any fastcgi responder to record tls client
> cert accounting data.
> 
> It should also be enough for any fastcgi responder which has
> out-of-band access to relevant CA metadata to complete tls client
> cert authorisation tasks of any complexity.
> 
> That requirement for out-of-band access to CA metadata is why this
> approach is not as flexible as passing through the raw client cert to
> fastcgi (although admittedly passing the whole chain, as I proposed
> last year, was probably overkill). It's probably still worth coming
> back to revisit passing the client cert itself once the issues
> currently blocking that approach have been resolved.
> 
> But in the meantime this approach should cover the majority of use
> cases (and has the benefit of not requiring fastcgi responders to link
> to libcrypto just for cert authorisation, as would be the case when
> passing through the raw cert).

Ping...

Here's a slightly updated diff that applies cleanly against today's
-current and deals with the one compiler warning the last one generated.

Am I on the right track with this more minimal approach?

Or would it be better to add a tls_peer_cert_serial() to libtls, then
call that from httpd's server_fcgi(), given that the unique
{issuer,serial} pair seems like a fairly common thing to need?

Or would we be better off reworking the way libtls presents client cert
chains first, then returning to the earlier proposed approach of having
httpd pass the client cert chain through to fastcgi responders instead?


Index: usr.sbin/httpd/httpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
retrieving revision 1.101
diff -u -p -r1.101 httpd.conf.5
--- usr.sbin/httpd/httpd.conf.5 20 Jun 2018 16:43:05 -0000      1.101
+++ usr.sbin/httpd/httpd.conf.5 9 Jul 2018 07:42:48 -0000
@@ -344,11 +344,27 @@ The revision of the HTTP specification u
 .It Ic SERVER_SOFTWARE
 The server software name of
 .Xr httpd 8 .
+.It Ic TLS_PEER_ISSUER
+The issuer of the TLS client certificate.
+.It Ic TLS_PEER_SERIAL
+The serial number of the TLS client certificate, expressed in hexadecimal.
+.It Ic TLS_PEER_SUBJECT
+The subject of the TLS client certificate.
 .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 .
+features in use.
 .El
+.Pp
+All
+.Ic TLS_PEER_*
+variables are omitted when TLS client certificate verification is not in use.
+All except
+.Ic TLS_PEER_VERIFY
+are also omitted if no TLS client certificate is offered when the
+.Ic optional
+argument to the
+.Ic tls client ca
+directive is in use.
 .It Ic hsts Oo Ar option Oc
 Enable HTTP Strict Transport Security.
 Valid options are:
Index: usr.sbin/httpd/server_fcgi.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.76
diff -u -p -r1.76 server_fcgi.c
--- usr.sbin/httpd/server_fcgi.c        19 May 2018 13:56:56 -0000      1.76
+++ usr.sbin/httpd/server_fcgi.c        9 Jul 2018 07:42:48 -0000
@@ -34,6 +34,13 @@
 #include <event.h>
 #include <unistd.h>
 
+#include <openssl/asn1.h>
+#include <openssl/bio.h>
+#include <openssl/bn.h>
+#include <openssl/pem.h>
+#include <openssl/x509.h>
+#include <tls.h>
+
 #include "httpd.h"
 #include "http.h"
 
@@ -87,17 +94,22 @@ int fcgi_add_param(struct server_fcgi_pa
 int
 server_fcgi(struct httpd *env, struct client *clt)
 {
+       ASN1_INTEGER                    *sn;
+       BIGNUM                          *bn = NULL;
+       BIO                             *bio = NULL;
+       X509                            *cert = NULL;
        struct server_fcgi_param         param;
        struct server_config            *srv_conf = clt->clt_srv_conf;
        struct http_descriptor          *desc = clt->clt_descreq;
        struct fcgi_record_header       *h;
        struct fcgi_begin_request_body  *begin;
        char                             hbuf[HOST_NAME_MAX+1];
-       size_t                           scriptlen;
+       size_t                           chainlen, scriptlen;
        int                              pathlen;
        int                              fd = -1, ret;
        const char                      *stripped, *p, *alias, *errstr = NULL;
-       char                            *str, *script = NULL;
+       char                            *str, *serial = NULL, *script = NULL;
+       const uint8_t                   *chain;
 
        if (srv_conf->socket[0] == ':') {
                struct sockaddr_storage  ss;
@@ -290,6 +302,36 @@ server_fcgi(struct httpd *env, struct cl
                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;
+               }
+       }
+       if ((srv_conf->flags & SRVFLAG_TLS) &&
+           tls_peer_cert_provided(clt->clt_tls_ctx)) {
+               if (fcgi_add_param(&param, "TLS_PEER_ISSUER",
+                   tls_peer_cert_issuer(clt->clt_tls_ctx), clt) == -1 ||
+                   fcgi_add_param(&param, "TLS_PEER_SUBJECT",
+                   tls_peer_cert_subject(clt->clt_tls_ctx), clt) == -1) {
+                       errstr = "failed to encode param";
+                       goto fail;
+               }
+               ret = 0;
+               chain = tls_peer_cert_chain_pem(clt->clt_tls_ctx, &chainlen);
+               if (chain == NULL ||
+                   (bio = BIO_new_mem_buf((const void *)chain,
+                   chainlen)) == NULL ||
+                   (cert = PEM_read_bio_X509(bio, NULL, NULL, NULL)) == NULL ||
+                   (sn = X509_get_serialNumber(cert)) == NULL ||
+                   (bn = ASN1_INTEGER_to_BN(sn, NULL)) == NULL ||
+                   (serial = BN_bn2hex(bn)) == NULL ||
+                   fcgi_add_param(&param, "TLS_PEER_SERIAL", serial,
+                   clt) == -1)
+                       ret = -1;
+               BIO_free(bio);
+               BN_free(bn);
+               X509_free(cert);
+               free(serial);
+               if (ret == -1) {
                        errstr = "failed to encode param";
                        goto fail;
                }

Reply via email to