Re: httpd logging X509 cert subject when CA option is used.

2019-02-10 Thread Sebastian Benoit
Karel Gardas(gard...@gmail.com) on 2019.02.10 14:18:00 +0100:
> 
> Any issues with the patch now?

no, i just lost track of it.

commited, thanks!

> Anything I shall improve to get that
> into acceptable/comitable state?
> 
> Thanks,
> Karel
> 
> On Fri, 1 Feb 2019 17:48:46 +0100
> Karel Gardas  wrote:
> 
> > On Fri, 1 Feb 2019 16:53:14 +0100
> > Sebastian Benoit  wrote:
> > 
> > > > +   if (clt->clt_remote_user == NULL &&
> > > > +   clt->clt_tls_ctx != NULL &&
> > > > +   (srv_conf->tls_flags & TLSFLAG_CA) &&
> > > > +   stravis(&user, 
> > > > tls_peer_cert_subject(clt->clt_tls_ctx),
> > > 
> > > tls_peer_cert_subject() can return NULL.
> > 
> > Fixed in patch below.
> > 
> > Thanks,
> > Karel
> > 
> > diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
> > index 9b13db2bca4..f0f30b93ebc 100644
> > --- a/usr.sbin/httpd/server_http.c
> > +++ b/usr.sbin/httpd/server_http.c
> > @@ -1712,6 +1712,13 @@ server_log_http(struct client *clt, unsigned int 
> > code, size_t len)
> > if (clt->clt_remote_user &&
> > stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
> > goto done;
> > +   if (clt->clt_remote_user == NULL &&
> > +   clt->clt_tls_ctx != NULL &&
> > +   (srv_conf->tls_flags & TLSFLAG_CA) &&
> > +   tls_peer_cert_subject(clt->clt_tls_ctx) != NULL &&
> > +   stravis(&user, tls_peer_cert_subject(clt->clt_tls_ctx),
> > +   HTTPD_LOGVIS) == -1)
> > +   goto done;
> > if (desc->http_version &&
> > stravis(&version, desc->http_version, HTTPD_LOGVIS) == -1)
> > goto done;
> > @@ -1730,7 +1737,7 @@ server_log_http(struct client *clt, unsigned int 
> > code, size_t len)
> > ret = evbuffer_add_printf(clt->clt_log,
> > "%s %s - %s [%s] \"%s %s%s%s%s%s\""
> > " %03d %zu \"%s\" \"%s\"\n",
> > -   srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
> > +   srv_conf->name, ip, user == NULL ? "-" :
> > user, tstamp,
> > server_httpmethod_byid(desc->http_method),
> > desc->http_path == NULL ? "" : path,
> 
> 



Re: httpd logging X509 cert subject when CA option is used.

2019-02-10 Thread Karel Gardas


Any issues with the patch now? Anything I shall improve to get that
into acceptable/comitable state?

Thanks,
Karel

On Fri, 1 Feb 2019 17:48:46 +0100
Karel Gardas  wrote:

> On Fri, 1 Feb 2019 16:53:14 +0100
> Sebastian Benoit  wrote:
> 
> > > + if (clt->clt_remote_user == NULL &&
> > > + clt->clt_tls_ctx != NULL &&
> > > + (srv_conf->tls_flags & TLSFLAG_CA) &&
> > > + stravis(&user, tls_peer_cert_subject(clt->clt_tls_ctx),
> > 
> > tls_peer_cert_subject() can return NULL.
> 
> Fixed in patch below.
> 
> Thanks,
> Karel
> 
> diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
> index 9b13db2bca4..f0f30b93ebc 100644
> --- a/usr.sbin/httpd/server_http.c
> +++ b/usr.sbin/httpd/server_http.c
> @@ -1712,6 +1712,13 @@ server_log_http(struct client *clt, unsigned int code, 
> size_t len)
>   if (clt->clt_remote_user &&
>   stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
>   goto done;
> + if (clt->clt_remote_user == NULL &&
> + clt->clt_tls_ctx != NULL &&
> + (srv_conf->tls_flags & TLSFLAG_CA) &&
> + tls_peer_cert_subject(clt->clt_tls_ctx) != NULL &&
> + stravis(&user, tls_peer_cert_subject(clt->clt_tls_ctx),
> + HTTPD_LOGVIS) == -1)
> + goto done;
>   if (desc->http_version &&
>   stravis(&version, desc->http_version, HTTPD_LOGVIS) == -1)
>   goto done;
> @@ -1730,7 +1737,7 @@ server_log_http(struct client *clt, unsigned int code, 
> size_t len)
>   ret = evbuffer_add_printf(clt->clt_log,
>   "%s %s - %s [%s] \"%s %s%s%s%s%s\""
>   " %03d %zu \"%s\" \"%s\"\n",
> - srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
> + srv_conf->name, ip, user == NULL ? "-" :
>   user, tstamp,
>   server_httpmethod_byid(desc->http_method),
>   desc->http_path == NULL ? "" : path,




Re: httpd logging X509 cert subject when CA option is used.

2019-02-01 Thread Karel Gardas
On Fri, 1 Feb 2019 16:53:14 +0100
Sebastian Benoit  wrote:

> > +   if (clt->clt_remote_user == NULL &&
> > +   clt->clt_tls_ctx != NULL &&
> > +   (srv_conf->tls_flags & TLSFLAG_CA) &&
> > +   stravis(&user, tls_peer_cert_subject(clt->clt_tls_ctx),
> 
> tls_peer_cert_subject() can return NULL.

Fixed in patch below.

Thanks,
Karel

diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
index 9b13db2bca4..f0f30b93ebc 100644
--- a/usr.sbin/httpd/server_http.c
+++ b/usr.sbin/httpd/server_http.c
@@ -1712,6 +1712,13 @@ server_log_http(struct client *clt, unsigned int code, 
size_t len)
if (clt->clt_remote_user &&
stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
goto done;
+   if (clt->clt_remote_user == NULL &&
+   clt->clt_tls_ctx != NULL &&
+   (srv_conf->tls_flags & TLSFLAG_CA) &&
+   tls_peer_cert_subject(clt->clt_tls_ctx) != NULL &&
+   stravis(&user, tls_peer_cert_subject(clt->clt_tls_ctx),
+   HTTPD_LOGVIS) == -1)
+   goto done;
if (desc->http_version &&
stravis(&version, desc->http_version, HTTPD_LOGVIS) == -1)
goto done;
@@ -1730,7 +1737,7 @@ server_log_http(struct client *clt, unsigned int code, 
size_t len)
ret = evbuffer_add_printf(clt->clt_log,
"%s %s - %s [%s] \"%s %s%s%s%s%s\""
" %03d %zu \"%s\" \"%s\"\n",
-   srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
+   srv_conf->name, ip, user == NULL ? "-" :
user, tstamp,
server_httpmethod_byid(desc->http_method),
desc->http_path == NULL ? "" : path,



Re: httpd logging X509 cert subject when CA option is used.

2019-02-01 Thread Sebastian Benoit
Karel Gardas(gard...@gmail.com) on 2019.02.01 16:28:17 +0100:
> 
> Hello,
> 
> I'd like to have X509 peer's cert subject name logged in some form when
> ca option in httpd.conf is used. That is, we do have X509 verified
> client accessing web resource. Following patch implements this
> behavior for combined logging style and for the case http connection is
> not authenticated by other means.
> 
> Thanks for review, comments and/or inclusion,

i think this is a good idea, but ...

> 
> Karel
> 
> diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
> index 9b13db2bca4..8291db52311 100644
> --- a/usr.sbin/httpd/server_http.c
> +++ b/usr.sbin/httpd/server_http.c
> @@ -1712,6 +1712,12 @@ server_log_http(struct client *clt, unsigned int code, 
> size_t len)
>   if (clt->clt_remote_user &&
>   stravis(&user, clt->clt_remote_user, HTTPD_LOGVIS) == -1)
>   goto done;
> + if (clt->clt_remote_user == NULL &&
> + clt->clt_tls_ctx != NULL &&
> + (srv_conf->tls_flags & TLSFLAG_CA) &&
> + stravis(&user, tls_peer_cert_subject(clt->clt_tls_ctx),

tls_peer_cert_subject() can return NULL.

> + HTTPD_LOGVIS) == -1)
> + goto done;
>   if (desc->http_version &&
>   stravis(&version, desc->http_version, HTTPD_LOGVIS) == -1)
>   goto done;
> @@ -1730,7 +1736,7 @@ server_log_http(struct client *clt, unsigned int code, 
> size_t len)
>   ret = evbuffer_add_printf(clt->clt_log,
>   "%s %s - %s [%s] \"%s %s%s%s%s%s\""
>   " %03d %zu \"%s\" \"%s\"\n",
> - srv_conf->name, ip, clt->clt_remote_user == NULL ? "-" :
> + srv_conf->name, ip, user == NULL ? "-" :
>   user, tstamp,
>   server_httpmethod_byid(desc->http_method),
>   desc->http_path == NULL ? "" : path,
>