Some nginx TLS tests started failing with LibreSSL 2.5.3

[jirutka@…]

After we updated LibreSSL from 2.4.4. to 2.5.3 in Alpine Linux, we have noticed that some TLS-related tests in nginx (both 1.10.3 and 1.12.0) started failing. Moreover, most of them fail because nginx accepted certificate that should be rejected! That’s pretty bad regression.

We’re not sure if the problem is in LibreSSL, nginx or nginx-tests, so reporting it to both.

People from VoidLinux has reproduced this issue too, on glibc.

Test Summary Report
-------------------
./h2_ssl_verify_client.t             (Wstat: 256 Tests: 5 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
./mail_imap_ssl.t                    (Wstat: 512 Tests: 14 Failed: 2)
  Failed tests:  4, 10
  Non-zero exit status: 2
./proxy_bind_transparent.t           (Wstat: 512 Tests: 3 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 2
./proxy_ssl_certificate.t            (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
./ssl_crl.t                          (Wstat: 512 Tests: 5 Failed: 2)
  Failed tests:  2-3
  Non-zero exit status: 2
./ssl_verify_client.t                (Wstat: 512 Tests: 12 Failed: 2)
  Failed tests:  4-5
  Non-zero exit status: 2
./ssl_verify_depth.t                 (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
./stream_proxy_ssl_certificate.t     (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
./stream_ssl_verify_client.t         (Wstat: 512 Tests: 12 Failed: 2)
  Failed tests:  3, 5
  Non-zero exit status: 2
Files=290, Tests=3628, 300 wallclock secs ( 1.91 usr  0.46 sys + 54.82 cusr  9.07 csys = 66.26 CPU)

Complete log: ​http://tpaste.us/Ynw6

[jirutka@…]

nginx-tests log

[jirutka@…]

LibreSSL bug report: ​https://github.com/libressl-portable/portable/issues/307

[jirutka@…]

VoidLinux guys started investigating it more and according to them the problem lies in LibreSSL, not nginx.

[jirutka@…]

So, Duncaen from VoidLinux has identified potential cause in LibreSSL code, commit ​https://github.com/libressl-portable/openbsd/commit/ddd98f8ea741a122952185a36c1396c14c2fda74. I’ve tried to build LibreSSL 2.5.3 with patch reverting that commit, rebuild nginx, run tests and… all pass! So it seems that this is really the cause.

However, it also seems that the problem is even on the nginx’s side, in function ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store) (​https://github.com/nginx/nginx/blob/branches/stable-1.10/src/event/ngx_event_openssl.c#L665-L706). It always returns 1, that seems to be wrong. It should return the given first argument (ok), that’s what null_callback(int ok, X509_STORE_CTX *e) (the default no-op callback) do, what other projects like OpenSMTPD use and what the OpenSSL documentation suggests. Why nginx always returns 1 here, i.e. saying that the cert is ok?

---

I’ve tried to change ngx_ssl_verify_callback to return the first given argument (ok) instead of 1 and build with unpatched LibreSSL 2.5.3.

--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -702,7 +702,7 @@
     }
 #endif

-    return 1;
+    return ok;
 }

Here are the results:

Test Summary Report
-------------------
./addition_buffered.t                (Wstat: 0 Tests: 3 Failed: 0)
  TODO passed:   1
./h2.t                               (Wstat: 0 Tests: 142 Failed: 0)
  TODO passed:   136-137
./h2_ssl_verify_client.t             (Wstat: 65280 Tests: 3 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 5 tests but ran 3.
./mail_imap_ssl.t                    (Wstat: 65280 Tests: 5 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 14 tests but ran 5.
./proxy_ssl_certificate.t            (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
./ssl_crl.t                          (Wstat: 512 Tests: 5 Failed: 2)
  Failed tests:  2-3
  Non-zero exit status: 2
./ssl_verify_client.t                (Wstat: 65280 Tests: 5 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 12 tests but ran 5.
./ssl_verify_depth.t                 (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
./stream_proxy_ssl_certificate.t     (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
Files=290, Tests=3234, 303 wallclock secs ( 1.73 usr  0.54 sys + 54.39 cusr  8.60 csys = 65.26 CPU)
Result: FAIL

Complete log: ​http://tpaste.us/Dk8e

[Sergey Kandaurov]

Replying to jirutka@…:

However, it also seems that the problem is even on the nginx’s side, in function ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store) (​https://github.com/nginx/nginx/blob/branches/stable-1.10/src/event/ngx_event_openssl.c#L665-L706). It always returns 1, that seems to be wrong.
..

That is incorrect. According to the ​documentation:

: The return value of verify_callback controls the strategy of the
: further verification process. If verify_callback returns 0, the
: verification process is immediately stopped with "verification failed"
: state. <..> If verify_callback returns 1, the verification process is continued.

This is what was broken in LibreSSL in the aforementioned change.

[Sergey Kandaurov]

As previously noted, this is not a problem in nginx.

[jirutka@…]

Please read also the rest of the documentation:

If verify_callback always returns 1, the TLS/SSL handshake will not be terminated with respect to verification failures and the connection will be established.

[Maxim Dounin]

Yes, and this is exactly what nginx wants to happen. It then uses SSL_get_verify_result() to check if the verification was successful or not, and acts appropriately - allows request to be handled, or returns an appropriate error. Unfortunately, this is broken in the current version of LibreSSL, and it does not seem to be possible to do anything with this on nginx side.

[jirutka@…]

Replying to mdounin:

Thanks for clarification! We have discussed it on IRC and other guys agreed that you’re right. Also LibreSSL already responded on the issue:

Thanks for digging into the issue and reporting this. It seems that the change does indeed break the documented API - we're looking into solutions and will follow up soon.

[hdatma@…]

The official nginx's makefile does not include the test section.
What command did you execute to run the tests?