Logging $ssl_session_id can crash Nginx 1.5.9 worker

[Maxim Dounin]

Hello!

On Thu, Jan 23, 2014 at 12:06:55AM +0400, Ruslan Ermilov wrote:

> On Wed, Jan 22, 2014 at 02:02:51PM -0500, mnordhoff wrote:
> > I run the nginx.org mainline packages on Ubuntu 12.04, 32- and 64-bit. I use
> > a wacky custom log format, and after 1.5.9 was released today, I enabled
> > logging the $ssl_session_id variable. I later ran an SSL Labs SSL Server
> > Test, [0] which makes numerous HTTPS requests of various sorts, and lo and
> > behold, one of my worker processes core dumped. I fooled around with my
> > configuration and determined that the problem was logging $ssl_session_id.
> > If I don't log it, it's fine. If I enable a log that contains
> > $ssl_session_id -- even only $ssl_session_id -- it crashes.
> 
> The following patch fixes this:
> 
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -2509,6 +2509,10 @@ ngx_ssl_get_session_id(ngx_connection_t 
>  
>      sess = SSL_get0_session(c->ssl->connection);
>  
> +    if (sess == NULL) {
> +        return NGX_ERROR;
> +    }
> +
>      buf = sess->session_id;
>      len = sess->session_id_length;

You were faster. :)
I think that len = 0 + NGX_OK is better than NGX_ERROR here 
though, and also in line with other similar functions like 
ngx_ssl_get_[raw_]certificate().

-- 
Maxim Dounin
http://nginx.org/