segfault with SNI and ssl_session_cache assymetrical configuration

[Antony Dovgal]

Nginx 1.2.4 segfaults in ngx_ssl_new_session():

Program terminated with signal 11, Segmentation fault.
#0 0x000000000043bdb4 in ngx_ssl_new_session (ssl_conn=0xe225760, sess=0xf53f80) at src/event/ngx_event_openssl.c:1678
1678 src/event/ngx_event_openssl.c: No such file or directory.

in src/event/ngx_event_openssl.c

(gdb) p shm_zone
$1 = (ngx_shm_zone_t *) 0x0

The crash is only reproducible when using several SSL certificates AND using Safari on Mac as a browser.
Not reproducible with Firefox/Chrome/whatever on the same Mac.

The crash is caused by SSL_CTX_get_ex_data() returning NULL, which is then dereferenced without a check. Since this check is absent in trunk, I suppose the bug is reproducible there, too.

The fix seems to be quite simple, see attached patch.

[Maxim Dounin]

Thank you for your report. It's actually known issue and manifests itself in a configuration like this:

    server {
        listen 443 default ssl;
        server_name test;
        ssl_certificate ...
        ssl_certificate_key ...
        ssl_session_cache shared:SSL:1m;
    }

    server {
        listen 443 ssl;
        server_name test2;
        ssl_certificate ...
        ssl_certificate_key ...
        ssl_session_cache none;
    }

I.e. if shared ssl_session_cache is used in a default server, but "ssl_session_cache none" (== default) in some SNI-based virtual host. The test is as follows:

openssl s_client -connect 127.0.0.1:443 -no_ticket -servername test2

Obvious workaround is to use the same session cache for all server blocks, e.g. to configure it on the http{} level.

The patch suggested will obviously fix segfault, but not the underlying problem with disagreement between SSL context used by OpenSSL to call new session callback and the context available via SSL_get_SSL_CTX(). As far as I remember there is no way to reach original context via OpenSSL API, so probably right solution would be to save original context somewhere in SNI callback.

[Antony Dovgal]

It's actually known issue

Yeah, Anight told me about that, but only after I 'fixed' and reported it to you =)

The patch suggested will obviously fix segfault, but not the underlying problem

Well, my intention was to fix the segfault, the cache issue isn't quite my priority atm.
I understand that this is just a workaround of a kind, but crashes create much more problems than non-working cache.
Anyway, thanks for the detailed explanation!

[Michał Górny]

I can reproduce this with Opera as well. Not that it really matters.

However, I agree with Antony that leaving a bug open like this is not a good solution. If it is known to cause issues at the moment, maybe nginx should output a warning or error when such a configuration is used? That may be easier to do that fixing the underlying issue while still will prevent random segfaults on people who are not aware of it.

[Nasko Oskov]

A colleague of mine and I have looked at this issue to help fix this bug. The underlying problem is how OpenSSL uses contexts and sessions with the presence of SNI extension.

The first context created will be the one used for accepting the connection and marked as the "session_ctx". When creating new sessions, it always uses the session_ctx, as session resumption is supposed to ignore extensions as specified in RFC 3546. This leads to always using the first context for dispatching new session callbacks, even though the proper context is passed in as a parameter. This effectively means that if the session callbacks are registered with the goal of having multiple session caches, this will not be possible (not sure if different ports will lead to different behavior).

In the case of this bug, the first context defines a shared cache, which leads to registering the new session callback. The second context doesn't have one, so it has a NULL shared memory region. Since the callback dispatch is based on the first context, it gets called with the correct context which has NULL pointer.

A proposed solution to this problem is to always register the new session callbacks, since they will be dispatched with the proper context as parameter. Then in the callback return early if there is no actual cache defined for the specific context. I can't attach the patch for some reason, so I'm inlining it here:

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
index cbe4136..b599ccb 100644
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -1774,18 +1774,16 @@ ngx_ssl_session_cache(ngx_ssl_t *ssl, ngx_str_t *sess_ctx,
         }
     }

-    if (shm_zone) {
-        SSL_CTX_sess_set_new_cb(ssl->ctx, ngx_ssl_new_session);
-        SSL_CTX_sess_set_get_cb(ssl->ctx, ngx_ssl_get_cached_session);
-        SSL_CTX_sess_set_remove_cb(ssl->ctx, ngx_ssl_remove_session);
+    SSL_CTX_sess_set_new_cb(ssl->ctx, ngx_ssl_new_session);
+    SSL_CTX_sess_set_get_cb(ssl->ctx, ngx_ssl_get_cached_session);
+    SSL_CTX_sess_set_remove_cb(ssl->ctx, ngx_ssl_remove_session);

-        if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_session_cache_index, shm_zone)
-            == 0)
-        {
-            ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
-                          "SSL_CTX_set_ex_data() failed");
-            return NGX_ERROR;
-        }
+    if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_session_cache_index, shm_zone)
+          == 0)
+    {
+        ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
+                      "SSL_CTX_set_ex_data() failed");
+        return NGX_ERROR;
     }

     return NGX_OK;
@@ -1884,6 +1882,9 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_conn, ngx_ssl_session_t *sess)

     ssl_ctx = SSL_get_SSL_CTX(ssl_conn);
     shm_zone = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_session_cache_index);
+    if (!shm_zone) {
+        return 0;
+    }

     cache = shm_zone->data;
     shpool = (ngx_slab_pool_t *) shm_zone->shm.addr;

OpenSSL also has a bug (id 2019) on file to fix session resumption with SNI extension, though it might not be fixed soon.

Let us know if this is good or if there is any other way to help resolve this issue.

[Maxim Dounin <mdounin@…>]

In 6261:97f102a13f33/nginx:

SSL: preserve default server context in connection (ticket #235).

This context is needed for shared sessions cache to work in configurations
with multiple virtual servers sharing the same port. Unfortunately, OpenSSL
does not provide an API to access the session context, thus storing it
separately.

In collaboration with Vladimir Homutov.

[Maxim Dounin]

Fix committed and will be available in upcoming nginx 1.9.6.

[creativeprogramming@…]

Still not fixed? I upgraded to 1.9.9 with the hope to fix, but no way...

I'm using SNI with HTTP/2 (before SPDY)
session_cache set globally to 10m

nginx -v
nginx version: nginx/1.9.9

2015/12/22 08:15:54 [alert] 7217#0: worker process 7392 exited on signal 11 (core dumped)
2015/12/22 08:16:17 [alert] 7217#0: worker process 7357 exited on signal 11 (core dumped)
2015/12/22 08:16:50 [alert] 7217#0: worker process 7364 exited on signal 11 (core dumped)
2015/12/22 08:16:54 [alert] 7217#0: worker process 7441 exited on signal 11 (core dumped)
2015/12/22 08:18:18 [alert] 7217#0: worker process 7471 exited on signal 11 (core dumped)

[vl]

Can you please provide:
1) nginx -V output
2) full configuration
3) debug log (​http://nginx.org/en/docs/debugging_log.html)
4) backtrace (​https://www.nginx.com/resources/wiki/start/topics/tutorials/debugging/)

[Maxim Dounin <mdounin@…>]

In 6343:60ae75969588/nginx:

SSL: preserve default server context in connection (ticket #235).

This context is needed for shared sessions cache to work in configurations
with multiple virtual servers sharing the same port. Unfortunately, OpenSSL
does not provide an API to access the session context, thus storing it
separately.

In collaboration with Vladimir Homutov.

[uvuv@…]

new session callback called at SSL_ST_OK, and it after ngx_http_ssl_servername;
ngx_ssl_new_session get ssl context for the current server config, not the default config.

I don't use the origin ssl context is a good idea.

Replying to mdounin:

Thank you for your report. It's actually known issue and manifests itself in a configuration like this:

    server {
        listen 443 default ssl;
        server_name test;
        ssl_certificate ...
        ssl_certificate_key ...
        ssl_session_cache shared:SSL:1m;
    }

    server {
        listen 443 ssl;
        server_name test2;
        ssl_certificate ...
        ssl_certificate_key ...
        ssl_session_cache none;
    }

I.e. if shared ssl_session_cache is used in a default server, but "ssl_session_cache none" (== default) in some SNI-based virtual host. The test is as follows:

openssl s_client -connect 127.0.0.1:443 -no_ticket -servername test2

Obvious workaround is to use the same session cache for all server blocks, e.g. to configure it on the http{} level.

The patch suggested will obviously fix segfault, but not the underlying problem with disagreement between SSL context used by OpenSSL to call new session callback and the context available via SSL_get_SSL_CTX(). As far as I remember there is no way to reach original context via OpenSSL API, so probably right solution would be to save original context somewhere in SNI callback.

[Maxim Dounin]

The particular issue is fixed. Please avoid reopening it unless you think it is not fixed.