httpd logging X509 cert subject when CA option is used.

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

httpd logging X509 cert subject when CA option is used.

Karel Gardas

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,

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),
+ 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,

Reply | Threaded
Open this post in threaded view
|

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

Sebastian Benoit-3
Karel Gardas([hidden email]) 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,
>

Reply | Threaded
Open this post in threaded view
|

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

Karel Gardas
On Fri, 1 Feb 2019 16:53:14 +0100
Sebastian Benoit <[hidden email]> 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,

Reply | Threaded
Open this post in threaded view
|

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

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 <[hidden email]> wrote:

> On Fri, 1 Feb 2019 16:53:14 +0100
> Sebastian Benoit <[hidden email]> 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,


Reply | Threaded
Open this post in threaded view
|

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

Sebastian Benoit-3
Karel Gardas([hidden email]) 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 <[hidden email]> wrote:
>
> > On Fri, 1 Feb 2019 16:53:14 +0100
> > Sebastian Benoit <[hidden email]> 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,
>
>