diff mbox series

[OpenWrt-Devel] uclient-fetch and ustream-mbedtls certificate verification

Message ID trinity-0c56705d-7e2c-482a-a0b5-a3230d3e75b2-1533383113431@3c-app-gmx-bs62
State Superseded
Delegated to: Petr Štetiar
Headers show
Series [OpenWrt-Devel] uclient-fetch and ustream-mbedtls certificate verification | expand

Commit Message

p.wassi@gmx.at Aug. 4, 2018, 11:45 a.m. UTC 
Dear list,

for SSL connections, uclient offers some options for certificate verification:
-) --no-check-certificate (i.e. ignore verification errors)
-) --ca-certificate=/path/to/file.crt
If "--ca-certificate" is not given, uclient internally uses /etc/ssl/certs/*.crt
(probably populated by the "ca-certificates" package).

I installed ustream-mbedtls for use with uclient-fetch, and observed - I'd say
not optimal behaviour.

When you run 
> uclient-fetch https://server/file.txt
(so no explicit certificate is given) and have *not* installed ca-certificates,
mbedtls obviously can't do verification since no root certificates available.
But then it simply ignores the issue and continues SSL handshake without warning.

Further, if you run it like
> uclient-fetch --ca-certificate=/etc/ssl/foo.crt https://server/file.txt
but foo.crt can't be read (filename misspelled, wrong certificate format, ...),
ustream-mbedtls also does not do verification at all (gives no warning either).

For me the issue is that ustream starts with verification disabled at first,
and enables verification as soon as a *valid* root certificate is given.
(No certificate or unparsable cert -> no verification and no warning/error)
However, in client mode it should always verify (but ignore the error, if
--no-check-certificate is given in uclient).
Not providing a root certificate should immediately lead to an error when
mbedtls is supposed to do verification for us.

I propose the attached change for the ustream-ssl repo:

-) leave verification disabled in server mode (is mbedtls' default anyway)
-) make verification optional* in client mode at initialisation step

* "optional" means that the code calling the mbedtls-library itself
must take care of calling verify(), see manual at [1]

What about using "required" like stated in the mbedtls reference?
This would probably need some tweaking, as --no-check-certificated needs
to be passed to ustream-* ...

Best regards,
Paul


[1]:
https://tls.mbed.org/api/ssl_8h.html#a5695285c9dbfefec295012b566290f37
diff mbox series

Patch

diff --git a/ustream-mbedtls.c b/ustream-mbedtls.c
index 347c600..262167c 100644
--- a/ustream-mbedtls.c
+++ b/ustream-mbedtls.c
@@ -144,14 +144,15 @@  __ustream_ssl_context_new(bool server)
 
        mbedtls_ssl_config_defaults(conf, ep, MBEDTLS_SSL_TRANSPORT_STREAM,
                                    MBEDTLS_SSL_PRESET_DEFAULT);
-       mbedtls_ssl_conf_authmode(conf, MBEDTLS_SSL_VERIFY_NONE);
        mbedtls_ssl_conf_rng(conf, _urandom, NULL);
 
        if (server) {
+               mbedtls_ssl_conf_authmode(conf, MBEDTLS_SSL_VERIFY_NONE);
                mbedtls_ssl_conf_ciphersuites(conf, default_ciphersuites_server);
                mbedtls_ssl_conf_min_version(conf, MBEDTLS_SSL_MAJOR_VERSION_3,
                                             MBEDTLS_SSL_MINOR_VERSION_3);
        } else
+               mbedtls_ssl_conf_authmode(conf, MBEDTLS_SSL_VERIFY_OPTIONAL);
                mbedtls_ssl_conf_ciphersuites(conf, default_ciphersuites_client);
 
 #if defined(MBEDTLS_SSL_CACHE_C)
@@ -189,7 +190,6 @@  __hidden int __ustream_ssl_add_ca_crt_file(struct ustream_ssl_ctx *ctx, const ch
                return -1;
 
        mbedtls_ssl_conf_ca_chain(&ctx->conf, &ctx->ca_cert, NULL);
-       mbedtls_ssl_conf_authmode(&ctx->conf, MBEDTLS_SSL_VERIFY_OPTIONAL);
        return 0;
 }