Skip to content

Commit

Permalink
fixes #1588 TLS should log errors
Browse files Browse the repository at this point in the history 
This isn't complete, but it should go much further in assisting
debugging TLS related errors.
  • Loading branch information
@gdamore
gdamore committed Apr 15, 2024
1 parent 808eb51 commit b1aaa60
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 50 deletions.
40 changes: 39 additions & 1 deletion src/supplemental/tls/mbedtls/tls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "mbedtls/version.h" // Must be first in order to pick up version

#include "mbedtls/error.h"
#include "nng/nng.h"

// mbedTLS renamed this header for 2.4.0.
#if MBEDTLS_VERSION_MAJOR > 2 || MBEDTLS_VERSION_MINOR >= 4
Expand Down Expand Up @@ -99,6 +100,22 @@ tls_random(void *arg, unsigned char *buf, size_t sz)
#endif
}

static void
tls_log_err(const char *msgid, const char *context, int errnum)
{
char errbuf[256];
mbedtls_strerror(errnum, errbuf, sizeof(errbuf));
nng_log_err(msgid, "%s: %s", context, errbuf);
}

static void
tls_log_warn(const char *msgid, const char *context, int errnum)
{
char errbuf[256];
mbedtls_strerror(errnum, errbuf, sizeof(errbuf));
nng_log_warn(msgid, "%s: %s", context, errbuf);
}

// tls_mk_err converts an mbed error to an NNG error.
static struct {
int tls;
Expand Down Expand Up @@ -193,6 +210,8 @@ conn_init(nng_tls_engine_conn *ec, void *tls, nng_tls_engine_config *cfg)
mbedtls_ssl_set_bio(&ec->ctx, tls, net_send, net_recv, NULL);

if ((rv = mbedtls_ssl_setup(&ec->ctx, &cfg->cfg_ctx)) != 0) {
tls_log_warn(
"NNG-TLS-CONN-FAIL", "Failed to setup TLS connection", rv);
return (tls_mk_err(rv));
}

Expand Down Expand Up @@ -266,6 +285,7 @@ conn_handshake(nng_tls_engine_conn *ec)
return (0);

default:
tls_log_warn("NNG-TLS-HANDSHAKE", "TLS handshake failed", rv);
return (tls_mk_err(rv));
}
}
Expand Down Expand Up @@ -393,8 +413,10 @@ config_init(nng_tls_engine_config *cfg, enum nng_tls_mode mode)
rv = mbedtls_ssl_config_defaults(&cfg->cfg_ctx, ssl_mode,
MBEDTLS_SSL_TRANSPORT_STREAM, MBEDTLS_SSL_PRESET_DEFAULT);
if (rv != 0) {
tls_log_err("NNG-TLS-CONFIG-INIT-FAIL",
"Failed to initialize TLS configuration", rv);
config_fini(cfg);
return (rv);
return (tls_mk_err(rv));
}

mbedtls_ssl_conf_authmode(&cfg->cfg_ctx, auth_mode);
Expand Down Expand Up @@ -461,12 +483,16 @@ config_ca_chain(nng_tls_engine_config *cfg, const char *certs, const char *crl)
pem = (const uint8_t *) certs;
len = strlen(certs) + 1;
if ((rv = mbedtls_x509_crt_parse(&cfg->ca_certs, pem, len)) != 0) {
tls_log_err("NNG-TLS-CA-FAIL",
"Failed to parse CA certificate(s)", rv);
return (tls_mk_err(rv));
}
if (crl != NULL) {
pem = (const uint8_t *) crl;
len = strlen(crl) + 1;
if ((rv = mbedtls_x509_crl_parse(&cfg->crl, pem, len)) != 0) {
tls_log_err("NNG-TLS-CRL-FAIL",
"Failed to parse revocation list", rv);
return (tls_mk_err(rv));
}
}
Expand All @@ -493,6 +519,8 @@ config_own_cert(nng_tls_engine_config *cfg, const char *cert, const char *key,
pem = (const uint8_t *) cert;
len = strlen(cert) + 1;
if ((rv = mbedtls_x509_crt_parse(&p->crt, pem, len)) != 0) {
tls_log_err("NNG-TLS-CRT-FAIL",
"Failure parsing our own certificate", rv);
rv = tls_mk_err(rv);
goto err;
}
Expand All @@ -507,12 +535,15 @@ config_own_cert(nng_tls_engine_config *cfg, const char *cert, const char *key,
pass != NULL ? strlen(pass) : 0, tls_random, NULL);
#endif
if (rv != 0) {
tls_log_err("NNG-TLS-KEY", "Failure parsing private key", rv);
rv = tls_mk_err(rv);
goto err;
}

rv = mbedtls_ssl_conf_own_cert(&cfg->cfg_ctx, &p->crt, &p->key);
if (rv != 0) {
tls_log_err("NNG-TLS-SELF",
"Failure configuring self certificate", rv);
rv = tls_mk_err(rv);
goto err;
}
Expand All @@ -536,6 +567,8 @@ config_version(nng_tls_engine_config *cfg, nng_tls_version min_ver,
int maj = MBEDTLS_SSL_MAJOR_VERSION_3;

if (min_ver > max_ver) {
nng_log_err("TLS-CFG-VER",
"TLS maximum version must be larger than mimumum version");
return (NNG_ENOTSUP);
}
switch (min_ver) {
Expand All @@ -553,6 +586,8 @@ config_version(nng_tls_engine_config *cfg, nng_tls_version min_ver,
v1 = MBEDTLS_SSL_MINOR_VERSION_3;
break;
default:
nng_log_err(
"TLS-CFG-VER", "TLS minimum version not supported");
return (NNG_ENOTSUP);
}

Expand All @@ -575,6 +610,8 @@ config_version(nng_tls_engine_config *cfg, nng_tls_version min_ver,
// Note that this means that if we ever TLS 1.4 or 2.0,
// then this will break. That's sufficiently far out
// to justify not worrying about it.
nng_log_err(
"TLS-CFG-VER", "TLS maximum version not supported");
return (NNG_ENOTSUP);
}

Expand Down Expand Up @@ -629,6 +666,7 @@ nng_tls_engine_init_mbed(void)
mbedtls_ctr_drbg_init(&cfg->rng_ctx);
rv = mbedtls_ctr_drbg_seed(&rng_ctx, tls_get_entropy, NULL, NULL, 0);
if (rv != 0) {
tls_log_err("NNG-TLS-RNG", "Failed initializing CTR DRBG", rv);
nni_mtx_fini(&rng_lock);
return (rv);
}
Expand Down
55 changes: 29 additions & 26 deletions src/supplemental/tls/tls_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static nni_atomic_ptr tls_engine;

struct nng_tls_config {
nng_tls_engine_config_ops ops;
const nng_tls_engine * engine; // store this so we can verify
const nng_tls_engine *engine; // store this so we can verify
nni_mtx lock;
int ref;
int busy;
Expand All @@ -55,21 +55,21 @@ struct nng_tls_config {
typedef struct {
nng_stream stream;
nng_tls_engine_conn_ops ops;
nng_tls_config * cfg;
const nng_tls_engine * engine;
nng_tls_config *cfg;
const nng_tls_engine *engine;
size_t size;
nni_aio * user_aio; // user's aio for connect/accept
nni_aio *user_aio; // user's aio for connect/accept
nni_aio conn_aio; // system aio for connect/accept
nni_mtx lock;
bool closed;
bool hs_done;
nni_list send_queue;
nni_list recv_queue;
nng_stream * tcp; // lower level stream
nng_stream *tcp; // lower level stream
nni_aio tcp_send; // lower level send pending
nni_aio tcp_recv; // lower level recv pending
uint8_t * tcp_send_buf;
uint8_t * tcp_recv_buf;
uint8_t *tcp_send_buf;
uint8_t *tcp_recv_buf;
size_t tcp_recv_len;
size_t tcp_recv_off;
bool tcp_recv_pend;
Expand Down Expand Up @@ -101,7 +101,7 @@ static nni_reap_list tls_conn_reap_list = {
typedef struct {
nng_stream_dialer ops;
nng_stream_dialer *d; // underlying TCP dialer
nng_tls_config * cfg;
nng_tls_config *cfg;
nni_mtx lk; // protects the config
} tls_dialer;

Expand Down Expand Up @@ -130,7 +130,7 @@ tls_dialer_free(void *arg)
static void
tls_conn_cb(void *arg)
{
tls_conn * conn = arg;
tls_conn *conn = arg;
nng_stream *tcp;
int rv;

Expand Down Expand Up @@ -171,7 +171,7 @@ tls_dialer_dial(void *arg, nng_aio *aio)
{
tls_dialer *d = arg;
int rv;
tls_conn * conn;
tls_conn *conn;

if (nni_aio_begin(aio) != 0) {
return;
Expand Down Expand Up @@ -212,7 +212,7 @@ tls_dialer_set_config(void *arg, const void *buf, size_t sz, nni_type t)
{
int rv;
nng_tls_config *cfg;
tls_dialer * d = arg;
tls_dialer *d = arg;
nng_tls_config *old;

if ((rv = nni_copyin_ptr((void **) &cfg, buf, sz, t)) != 0) {
Expand All @@ -235,7 +235,7 @@ tls_dialer_set_config(void *arg, const void *buf, size_t sz, nni_type t)
static int
tls_dialer_get_config(void *arg, void *buf, size_t *szp, nni_type t)
{
tls_dialer * d = arg;
tls_dialer *d = arg;
nng_tls_config *cfg;
int rv;
nni_mtx_lock(&d->lk);
Expand Down Expand Up @@ -409,7 +409,7 @@ nni_tls_dialer_alloc(nng_stream_dialer **dp, const nng_url *url)
typedef struct {
nng_stream_listener ops;
nng_stream_listener *l;
nng_tls_config * cfg;
nng_tls_config *cfg;
nni_mtx lk;
} tls_listener;

Expand Down Expand Up @@ -445,7 +445,7 @@ tls_listener_accept(void *arg, nng_aio *aio)
{
tls_listener *l = arg;
int rv;
tls_conn * conn;
tls_conn *conn;

if (nni_aio_begin(aio) != 0) {
return;
Expand All @@ -469,7 +469,7 @@ tls_listener_set_config(void *arg, const void *buf, size_t sz, nni_type t)
{
int rv;
nng_tls_config *cfg;
tls_listener * l = arg;
tls_listener *l = arg;
nng_tls_config *old;

if ((rv = nni_copyin_ptr((void **) &cfg, buf, sz, t)) != 0) {
Expand All @@ -494,7 +494,7 @@ tls_listener_set_config(void *arg, const void *buf, size_t sz, nni_type t)
static int
tls_listener_get_config(void *arg, void *buf, size_t *szp, nni_type t)
{
tls_listener * l = arg;
tls_listener *l = arg;
nng_tls_config *cfg;
int rv;
nni_mtx_lock(&l->lk);
Expand Down Expand Up @@ -809,7 +809,7 @@ static const nni_option tls_options[] = {
static int
tls_set(void *arg, const char *name, const void *buf, size_t sz, nni_type t)
{
tls_conn * conn = arg;
tls_conn *conn = arg;
int rv;
nng_stream *tcp;

Expand Down Expand Up @@ -837,7 +837,7 @@ tls_get(void *arg, const char *name, void *buf, size_t *szp, nni_type t)
static int
tls_alloc(tls_conn **conn_p, nng_tls_config *cfg, nng_aio *user_aio)
{
tls_conn * conn;
tls_conn *conn;
const nng_tls_engine *eng;
size_t size;

Expand Down Expand Up @@ -1066,7 +1066,7 @@ static void
tls_tcp_send_cb(void *arg)
{
tls_conn *conn = arg;
nng_aio * aio = &conn->tcp_send;
nng_aio *aio = &conn->tcp_send;
int rv;
size_t count;

Expand Down Expand Up @@ -1098,7 +1098,7 @@ static void
tls_tcp_recv_cb(void *arg)
{
tls_conn *conn = arg;
nni_aio * aio = &conn->tcp_recv;
nni_aio *aio = &conn->tcp_recv;
int rv;

nni_mtx_lock(&conn->lock);
Expand Down Expand Up @@ -1273,9 +1273,9 @@ nng_tls_config_cert_key_file(
nng_tls_config *cfg, const char *path, const char *pass)
{
int rv;
void * data;
void *data;
size_t size;
char * pem;
char *pem;

if ((rv = nni_file_get(path, &data, &size)) != 0) {
return (rv);
Expand All @@ -1295,9 +1295,9 @@ int
nng_tls_config_ca_file(nng_tls_config *cfg, const char *path)
{
int rv;
void * data;
void *data;
size_t size;
char * pem;
char *pem;

if ((rv = nni_file_get(path, &data, &size)) != 0) {
return (rv);
Expand Down Expand Up @@ -1397,7 +1397,7 @@ nng_tls_config_auth_mode(nng_tls_config *cfg, nng_tls_auth_mode mode)
int
nng_tls_config_alloc(nng_tls_config **cfg_p, nng_tls_mode mode)
{
nng_tls_config * cfg;
nng_tls_config *cfg;
const nng_tls_engine *eng;
size_t size;
int rv;
Expand Down Expand Up @@ -1496,9 +1496,12 @@ int
nng_tls_engine_register(const nng_tls_engine *engine)
{
if (engine->version != NNG_TLS_ENGINE_VERSION) {
nng_log_err("NNG-TLS-ENGINE-VER",
"TLS Engine version mismatch: %d != %d", engine->version,
NNG_TLS_ENGINE_VERSION);
return (NNG_ENOTSUP);
}
nni_atomic_set_ptr(&tls_engine, (void *)engine);
nni_atomic_set_ptr(&tls_engine, (void *) engine);
return (0);
}

Expand Down
Loading

0 comments on commit b1aaa60

Please sign in to comment.