Index | Thread | Search

From:
Jack Burton <jack@saosce.com.au>
Subject:
Re: [diff] httpd: pass through dn from tls client cert to fcgi
To:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Cc:
Jan Klemkow <jan@openbsd.org>, tech@openbsd.org
Date:
Thu, 30 Apr 2026 21:33:05 +0930

Download raw body.

Thread
On Thu, 30 Apr 2026 13:45:43 +0200
Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:
> On Thu, Apr 30, 2026 at 08:39:19PM +0930, Jack Burton wrote:
> > On Thu, 30 Apr 2026 10:36:10 +0200
> > Claudio Jeker <cjeker@diehard.n-r-g.com> wrote:  
> > > On Thu, Apr 30, 2026 at 03:26:20PM +0930, Jack Burton wrote:  
> > > > On Wed, 29 Apr 2026 21:49:29 +0200> > +		}
> > > > +		if (tls_peer_cert_provided(clt->clt_tls_ctx)) {
> > > > +			dn =
> > > > tls_peer_cert_subject(clt->clt_tls_ctx);
> > > > +			if (dn != NULL &&
> > > > fcgi_add_param(&param,
> > > > +			    "TLS_PEER_SUBJECT", dn, clt) ==
> > > > -1) {
> > > > +				errstr = "failed to encode
> > > > param";
> > > > +				goto fail;
> > > > +			}
> > > >  		}
> > > >  	}    
> > > 
> > > Is it really an error if dn == NULL or should the code simply omit
> > > adding the TLS_PEER_SUBJECT?  
> > 
> > dn == NULL does not trigger an error.  In that case (dn != NULL) is
> > false so the goto statement is never reached.
> > 
> > Perhaps it would more readable with a single conditional statement
> > instead of two.  
> 
> I blame lack of sleep and not reading the code carefully enough.
> I often trip over
> 	if (foo != NULL && xyz(foo) == -1)
> 		fail hard;
> statements because they logic is a bit twisted. In general splitting
> this up makes it more legible to me:
> 	if (foo != NULL) {
> 		if (xyz(foo) == -1)
> 			fail hard;
> 	}
> 
> I do the same error myself and build complex if statements and later
> on I trip over them in the same way.
>  
> > How's this?  
> 
> No, for me that is worse.
> 

Ah, I see.  Okay, then it's more readable to move the middle test into
the first if, so we have clear separation between condition-to-try and
condition-to-fail, like so?

Index: httpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
diff -u -p -r1.129 httpd.conf.5
--- httpd.conf.5	18 Jan 2026 16:38:02 -0000	1.129
+++ httpd.conf.5	30 Apr 2026 11:57:32 -0000
@@ -453,6 +453,14 @@ The revision of the HTTP specification u
 .It Ic SERVER_SOFTWARE
 The server software name of
 .Xr httpd 8 .
+.It Ic TLS_PEER_SUBJECT
+The subject
+.Pq distinguished name
+of the TLS client certificate
+.Po
+omitted when certificate has no subject field or
+TLS client verification is not in use
+.Pc .
 .It Ic TLS_PEER_VERIFY
 A variable that is set to a comma separated list of TLS client verification
 features in use
Index: server_fcgi.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
diff -u -p -r1.100 server_fcgi.c
--- server_fcgi.c	2 Mar 2026 19:24:58 -0000	1.100
+++ server_fcgi.c	30 Apr 2026 11:57:32 -0000
@@ -34,6 +34,8 @@
 #include <event.h>
 #include <unistd.h>
 
+#include <tls.h>
+
 #include "httpd.h"
 #include "http.h"
 #include "log.h"
@@ -99,7 +101,7 @@ server_fcgi(struct httpd *env, struct cl
 	size_t				 scriptlen;
 	int				 pathlen;
 	int				 fd = -1, ret;
-	const char			*stripped, *alias, *errstr = NULL;
+	const char			*stripped, *alias, *dn, *errstr = NULL;
 	char				*query_alias, *str, *script = NULL;
 
 	if ((fd = socket(srv_conf->fastcgi_ss.ss_family,
@@ -272,6 +274,14 @@ server_fcgi(struct httpd *env, struct cl
 		    TLSFLAG_BITS), clt) == -1) {
 			errstr = "failed to encode param";
 			goto fail;
+		}
+		if (tls_peer_cert_provided(clt->clt_tls_ctx) &&
+		    (dn = tls_peer_cert_subject(clt->clt_tls_ctx)) != NULL) {
+			if (fcgi_add_param(&param, "TLS_PEER_SUBJECT", dn,
+			    clt) == -1) {
+				errstr = "failed to encode param";
+				goto fail;
+			}
 		}
 	}