Skip to content

Commit

Permalink
websocket: Do not allow a listener or dialer to change TLS while running
Browse files Browse the repository at this point in the history
This also covers a few test cases that we were missing.
  • Loading branch information
gdamore committed Nov 10, 2024
1 parent 0058b76 commit cbe9a27
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/sp/transport/ws/ws_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,31 @@ test_ws_recv_max(void)
NUTS_CLOSE(s1);
}

void
test_ws_no_tls(void)
{
nng_socket s0;
nng_listener l;
nng_dialer d;
char *addr;
nng_tls_config *tls;

NUTS_ADDR(addr, "ws");
NUTS_OPEN(s0);
NUTS_PASS(nng_listener_create(&l, s0, addr));
NUTS_FAIL(nng_listener_get_tls(l, &tls), NNG_ENOTSUP);

NUTS_PASS(nng_dialer_create(&d, s0, addr));
NUTS_FAIL(nng_dialer_get_tls(d, &tls), NNG_ENOTSUP);
NUTS_CLOSE(s0);
}

TEST_LIST = {
{ "ws url path filters", test_ws_url_path_filters },
{ "ws wild card port", test_wild_card_port },
{ "ws wild card host", test_wild_card_host },
{ "ws empty host", test_empty_host },
{ "ws recv max", test_ws_recv_max },
{ "ws no tls", test_ws_no_tls },
{ NULL, NULL },
};
19 changes: 19 additions & 0 deletions src/supplemental/tls/tls_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ typedef struct {
nng_stream_dialer ops;
nng_stream_dialer *d; // underlying TCP dialer
nng_tls_config *cfg;
bool started;
nni_mtx lk; // protects the config
} tls_dialer;

Expand Down Expand Up @@ -186,6 +187,9 @@ tls_dialer_dial(void *arg, nng_aio *aio)
tls_free(conn);
return;
}
nni_mtx_lock(&d->lk);
d->started = true;
nni_mtx_unlock(&d->lk);

nng_stream_dialer_dial(d->d, &conn->conn_aio);
}
Expand All @@ -198,9 +202,15 @@ tls_dialer_set_tls(void *arg, nng_tls_config *cfg)
if (cfg == NULL) {
return (NNG_EINVAL);
}

nng_tls_config_hold(cfg);

nni_mtx_lock(&d->lk);
if (d->started) {
nni_mtx_unlock(&d->lk);
nng_tls_config_free(cfg);
return (NNG_EBUSY);
}
old = d->cfg;
d->cfg = cfg;
nni_mtx_unlock(&d->lk);
Expand Down Expand Up @@ -287,6 +297,7 @@ typedef struct {
nng_stream_listener ops;
nng_stream_listener *l;
nng_tls_config *cfg;
bool started;
nni_mtx lk;
} tls_listener;

Expand Down Expand Up @@ -314,6 +325,9 @@ static int
tls_listener_listen(void *arg)
{
tls_listener *l = arg;
nni_mtx_lock(&l->lk);
l->started = true;
nni_mtx_unlock(&l->lk);
return (nng_stream_listener_listen(l->l));
}

Expand Down Expand Up @@ -352,6 +366,11 @@ tls_listener_set_tls(void *arg, nng_tls_config *cfg)
nng_tls_config_hold(cfg);

nni_mtx_lock(&l->lk);
if (l->started) {
nni_mtx_unlock(&l->lk);
nng_tls_config_free(cfg);
return (NNG_EBUSY);
}
old = l->cfg;
l->cfg = cfg;
nni_mtx_unlock(&l->lk);
Expand Down
58 changes: 58 additions & 0 deletions src/supplemental/websocket/wssfile_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
//

#include "core/nng_impl.h"
#include "nng/nng.h"
#include "nng/supplemental/tls/tls.h"

#include <nuts.h>
Expand Down Expand Up @@ -214,6 +215,62 @@ test_cert_file_not_present(void)
nng_tls_config_free(c);
}

static void
test_tls_config(void)
{
uint16_t port = nuts_next_port();
nng_socket s1;
nng_socket s2;
nng_listener l;
nng_dialer d;
char addr[40];
nng_tls_config *cfg;

(void) snprintf(addr, sizeof(addr), "wss4://:%u/test", port);

NUTS_PASS(nng_pair_open(&s1));
NUTS_PASS(nng_pair_open(&s2));
NUTS_PASS(nng_listener_create(&l, s1, addr));
NUTS_PASS(nng_listener_get_tls(l, &cfg));
nng_tls_config_hold(cfg);
NUTS_TRUE(cfg != NULL);

init_listener_wss_file(l);
NUTS_PASS(nng_listener_start(l, 0));

// make sure we cannot change the auth mode while running

NUTS_FAIL(nng_listener_set_tls(l, cfg), NNG_EBUSY);
nng_tls_config_free(cfg);

NUTS_PASS(nng_listener_get_tls(l, &cfg));

NUTS_FAIL(
nng_tls_config_auth_mode(cfg, NNG_TLS_AUTH_MODE_NONE), NNG_EBUSY);

nng_msleep(100);

snprintf(addr, sizeof(addr), "wss://127.0.0.1:%u/test", port);

// We find that sometimes this fails due to NNG_EPEERAUTH, but it
// can also fail due to NNG_ECLOSED. This seems to be timing
// dependent, based on receive vs. send timing most likely.
// Applications shouldn't really depend that much on this.
int rv;

NUTS_PASS(nng_dialer_create(&d, s2, addr));
rv = nng_dialer_start(d, 0);
NUTS_PASS(nng_dialer_get_tls(d, &cfg));
NUTS_FAIL(nng_dialer_set_tls(d, cfg), NNG_EBUSY);

NUTS_TRUE(rv != 0);
NUTS_TRUE((rv == NNG_EPEERAUTH) || (rv == NNG_ECLOSED) ||
(rv == NNG_ECRYPTO));

NUTS_PASS(nng_close(s1));
NUTS_PASS(nng_close(s2));
}

#endif

NUTS_TESTS = {
Expand All @@ -222,6 +279,7 @@ NUTS_TESTS = {
{ "wss file no verify", test_no_verify },
{ "wss file verify works", test_verify_works },
{ "wss file ca cert missing", test_cert_file_not_present },
{ "wss tls config", test_tls_config },
#endif
{ NULL, NULL },
};

0 comments on commit cbe9a27

Please sign in to comment.