From 0d9f02197e54ff025410e0af8933f83da5165773 Mon Sep 17 00:00:00 2001 From: Eugene K Date: Fri, 29 Dec 2023 12:49:59 -0500 Subject: [PATCH 1/3] remove ziti context timeout setting - connect timeout is available via ziti_dial_opts --- includes/ziti/ziti.h | 22 ++-------------------- library/ziti.c | 12 ------------ 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/includes/ziti/ziti.h b/includes/ziti/ziti.h index fdb633e0..cf64f32f 100644 --- a/includes/ziti/ziti.h +++ b/includes/ziti/ziti.h @@ -1,9 +1,9 @@ -// Copyright (c) 2022-2023. NetFoundry Inc. +// Copyright (c) 2022-2023. NetFoundry Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. -// You may obtain a copy of the License at // +// You may obtain a copy of the License at // https://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software @@ -550,24 +550,6 @@ extern const ziti_identity *ziti_get_identity(ziti_context ztx); ZITI_FUNC extern void ziti_get_transfer_rates(ziti_context ztx, double *up, double *down); -/** - * @brief Sets connect and write timeouts(in millis). - * - * The #ZITI_DEFAULT_TIMEOUT is used if this function is not invoked prior to initializing connections. This value is only - * referenced when initializing new connections via ziti_conn_init(). Any connection initialized before this function will - * have the whatever timeout was set at the time of initialization. - * - * Note: There is no check to verify the timeout specified is not "too small". Setting this value to a very small value - * may lead to a large number of timeouts. - * - * @param ztx the Ziti Edge identity context to set a timeout on - * @param timeout the value in milliseconds of the timeout (must be > 0) - * - * @return #ZITI_OK or corresponding #ZITI_ERRORS - */ -ZITI_FUNC -extern int ziti_set_timeout(ziti_context ztx, int timeout); - /** * @brief Shutdown Ziti Edge identity context and reclaim the memory from the provided #ziti_context. * diff --git a/library/ziti.c b/library/ziti.c index 79c13b97..957df37d 100644 --- a/library/ziti.c +++ b/library/ziti.c @@ -236,7 +236,6 @@ int ziti_init_opts(ziti_options *options, uv_loop_t *loop) { ctx->opts = *options; ctx->tlsCtx = tls; ctx->loop = loop; - ctx->ziti_timeout = ZITI_DEFAULT_TIMEOUT; ctx->ctrl_status = ZITI_WTF; STAILQ_INIT(&ctx->w_queue); @@ -538,15 +537,6 @@ void ziti_get_transfer_rates(ziti_context ztx, double *up, double *down) { *down = metrics_rate_get(&ztx->down_rate); } -int ziti_set_timeout(ziti_context ztx, int timeout) { - if (timeout > 0) { - ztx->ziti_timeout = timeout; - } else { - ztx->ziti_timeout = ZITI_DEFAULT_TIMEOUT; - } - return ZITI_OK; -} - static void free_ztx(uv_handle_t *h) { ziti_context ztx = h->data; @@ -758,7 +748,6 @@ int ziti_conn_init(ziti_context ztx, ziti_connection *conn, void *data) { NEWP(c, struct ziti_conn); c->ziti_ctx = ztx; c->data = data; - c->timeout = ctx->ziti_timeout; c->conn_id = ztx->conn_seq++; *conn = c; @@ -1918,7 +1907,6 @@ int ziti_context_run(ziti_context ztx, uv_loop_t *loop) { ztx->tlsCtx = tls; ztx->loop = loop; - ztx->ziti_timeout = ZITI_DEFAULT_TIMEOUT; ztx->ctrl_status = ZITI_WTF; STAILQ_INIT(&ztx->w_queue); From 16ba7abedcc05ad084a743832258705c0ad4656f Mon Sep 17 00:00:00 2001 From: Eugene K Date: Fri, 29 Dec 2023 12:51:37 -0500 Subject: [PATCH 2/3] use conn_req.dial_options.connect_timeout_seconds remove write timeouts --- inc_internal/zt_internal.h | 5 -- library/connect.c | 111 +++++++++++-------------------------- 2 files changed, 32 insertions(+), 84 deletions(-) diff --git a/inc_internal/zt_internal.h b/inc_internal/zt_internal.h index c4a84379..0c185557 100644 --- a/inc_internal/zt_internal.h +++ b/inc_internal/zt_internal.h @@ -128,7 +128,6 @@ struct ziti_write_req_s { struct message_s *message; ziti_write_cb cb; - uv_timer_t *timeout; uint64_t start_ts; void *ctx; @@ -204,7 +203,6 @@ struct ziti_conn { bool fin_sent; int fin_recv; // 0 - not received, 1 - received, 2 - called app data cb bool disconnecting; - int timeout; TAILQ_HEAD(, message_s) in_q; buffer *inbound; @@ -303,9 +301,6 @@ struct ziti_ctx { uint32_t conn_seq; - /* options */ - int ziti_timeout; - /* context wide metrics */ uint64_t start; rate_t up_rate; diff --git a/library/connect.c b/library/connect.c index 5cf38294..23777c1a 100644 --- a/library/connect.c +++ b/library/connect.c @@ -27,6 +27,9 @@ static const int MAX_CONNECT_RETRY = 3; conn->ziti_ctx->id, conn->conn_id, (int)sizeof(conn->marker), conn->marker, conn_state_str[conn->state], ##__VA_ARGS__) +#define DEFAULT_DIAL_OPTS (ziti_dial_opts){ \ + .connect_timeout_seconds = ZITI_DEFAULT_TIMEOUT/1000, \ + } static const char *conn_state_str[] = { #define state_str(ST) #ST , @@ -39,8 +42,7 @@ struct ziti_conn_req { char *service_id; ziti_net_session *session; ziti_conn_cb cb; - ziti_dial_opts *dial_opts; - ziti_listen_opts *listen_opts; + ziti_dial_opts dial_opts; int retry_count; uv_timer_t *conn_timeout; @@ -84,30 +86,19 @@ static void conn_set_state(struct ziti_conn *conn, enum conn_state state) { } } -static ziti_dial_opts *clone_ziti_dial_opts(const ziti_dial_opts *dial_opts) { - if (dial_opts == NULL) { - ZITI_LOG(TRACE, "refuse to clone NULL dial_opts"); - return NULL; - } - ziti_dial_opts *c = calloc(1, sizeof(ziti_dial_opts)); - memcpy(c, dial_opts, sizeof(ziti_dial_opts)); - if (dial_opts->identity != NULL) c->identity = strdup(dial_opts->identity); +static void clone_ziti_dial_opts(ziti_dial_opts *dest, const ziti_dial_opts *dial_opts) { + memcpy(dest, dial_opts, sizeof(ziti_dial_opts)); + if (dial_opts->identity != NULL) dest->identity = strdup(dial_opts->identity); if (dial_opts->app_data != NULL) { - c->app_data = malloc(dial_opts->app_data_sz); - c->app_data_sz = dial_opts->app_data_sz; - memcpy(c->app_data, dial_opts->app_data, dial_opts->app_data_sz); + dest->app_data = malloc(dial_opts->app_data_sz); + dest->app_data_sz = dial_opts->app_data_sz; + memcpy(dest->app_data, dial_opts->app_data, dial_opts->app_data_sz); } - return c; } static void free_ziti_dial_opts(ziti_dial_opts *dial_opts) { - if (dial_opts == NULL) { - ZITI_LOG(TRACE, "refuse to free NULL dial_opts"); - return; - } FREE(dial_opts->identity); FREE(dial_opts->app_data); - free(dial_opts); } static ziti_listen_opts *clone_ziti_listen_opts(const ziti_listen_opts *ln_opts) { @@ -140,8 +131,7 @@ static void free_conn_req(struct ziti_conn_req *r) { FREE(r->session); } - free_ziti_dial_opts(r->dial_opts); - free_ziti_listen_opts(r->listen_opts); + free_ziti_dial_opts(&r->dial_opts); FREE(r->service_id); free(r); } @@ -223,12 +213,6 @@ void on_write_completed(struct ziti_conn *conn, struct ziti_write_req_s *req, in } CONN_LOG(TRACE, "status %d", status); - if (req->timeout != NULL) { - uv_timer_stop(req->timeout); - uv_close((uv_handle_t *) req->timeout, free_handle); - req->timeout = NULL; - } - if (status < 0) { conn_set_state(conn, Disconnected); CONN_LOG(DEBUG, "is now Disconnected due to write failure: %d", status); @@ -342,8 +326,8 @@ static void connect_timeout(uv_timer_t *timer) { CONN_LOG(WARN, "connect timeout: no suitable edge router for service[%s]", conn->service); } else { - CONN_LOG(WARN, "failed to establish connection to service[%s] in %dms on ch[%d]", - conn->service, conn->timeout, ch->id); + CONN_LOG(WARN, "failed to establish connection to service[%s] in %ds on ch[%d]", + conn->service, conn->conn_req->dial_opts.connect_timeout_seconds, ch->id); } complete_conn_req(conn, ZITI_TIMEOUT); ziti_disconnect(conn); @@ -352,7 +336,7 @@ static void connect_timeout(uv_timer_t *timer) { } } -static int ziti_connect(struct ziti_ctx *ztx, const ziti_net_session *session, struct ziti_conn *conn) { +static int ziti_connect(struct ziti_ctx *ztx, ziti_net_session *session, struct ziti_conn *conn) { // verify ziti context is still authorized if (ztx->api_session == NULL) { CONN_LOG(ERROR, "ziti context is not authenticated, cannot connect to service[%s]", conn->service); @@ -495,7 +479,7 @@ static void process_connect(struct ziti_conn *conn) { req->conn_timeout = calloc(1, sizeof(uv_timer_t)); uv_timer_init(loop, req->conn_timeout); req->conn_timeout->data = conn; - uv_timer_start(req->conn_timeout, connect_timeout, conn->timeout, 0); + uv_timer_start(req->conn_timeout, connect_timeout, req->dial_opts.connect_timeout_seconds * 1000, 0); CONN_LOG(DEBUG, "starting %s connection for service[%s] with session[%s]", ziti_session_types.name(req->session_type), conn->service, req->session->id); @@ -531,14 +515,12 @@ static int do_ziti_dial(ziti_connection conn, const char *service, ziti_dial_opt req->session_type = ziti_session_types.Dial; req->cb = conn_cb; + req->dial_opts = DEFAULT_DIAL_OPTS; if (dial_opts != NULL) { // clone dial_opts to survive the async request - req->dial_opts = clone_ziti_dial_opts(dial_opts); + clone_ziti_dial_opts(&req->dial_opts, dial_opts); // override connection timeout if set in dial_opts - if (dial_opts->connect_timeout_seconds > 0) { - conn->timeout = dial_opts->connect_timeout_seconds * 1000; - } } conn->data_cb = data_cb; @@ -567,21 +549,6 @@ int ziti_dial_with_options(ziti_connection conn, const char *service, ziti_dial_ return do_ziti_dial(conn, service, dial_opts, conn_cb, data_cb); } -static void ziti_write_timeout(uv_timer_t *t) { - struct ziti_write_req_s *req = t->data; - struct ziti_conn *conn = req->conn; - - req->timeout = NULL; - req->conn = NULL; - - if (conn->state < Disconnected) { - conn_set_state(conn, Disconnected); - req->cb(conn, ZITI_TIMEOUT, req->ctx); - } - - uv_close((uv_handle_t *) t, free_handle); -} - static void ziti_write_req(struct ziti_write_req_s *req) { struct ziti_conn *conn = req->conn; @@ -593,13 +560,6 @@ static void ziti_write_req(struct ziti_write_req_s *req) { message *m = create_message(conn, ContentTypeStateClosed, 0); send_message(conn, m, req); } else { - if (req->cb) { - req->timeout = calloc(1, sizeof(uv_timer_t)); - uv_timer_init(conn->ziti_ctx->loop, req->timeout); - req->timeout->data = req; - uv_timer_start(req->timeout, ziti_write_timeout, conn->timeout, 0); - } - message *m = req->message; if (m == NULL) { size_t total_len = req->len + (conn->encrypted ? crypto_secretstream_xchacha20poly1305_abytes() : 0); @@ -697,15 +657,8 @@ int establish_crypto(ziti_connection conn, message *msg) { uint8_t *peer_key; bool peer_key_sent = message_get_bytes_header(msg, PublicKeyHeader, &peer_key, &peer_key_len); if (!peer_key_sent) { - if (conn->encrypted) { - CONN_LOG(ERROR, "failed to establish crypto for encrypted service: did not receive peer key"); - return ZITI_CRYPTO_FAIL; - } else { - CONN_LOG(VERBOSE, "encryption not set up: peer_key_sent[%d] conn->encrypted[%d]", (int) peer_key_sent, - (int) conn->encrypted); - // service is not required to be encrypted and hosting side did not send the key - return ZITI_OK; - } + CONN_LOG(ERROR, "failed to establish crypto for encrypted service: did not receive peer key"); + return ZITI_CRYPTO_FAIL; } int rc = init_crypto(&conn->key_ex, &conn->key_pair, peer_key, conn->state == Accepting); @@ -1063,19 +1016,19 @@ static int ziti_channel_start_connection(struct ziti_conn *conn, ziti_channel_t init_key_pair(&conn->key_pair); nheaders++; } - if (req->dial_opts != NULL) { - if (req->dial_opts->identity != NULL) { - headers[nheaders].header_id = TerminatorIdentityHeader; - headers[nheaders].value = (uint8_t *) req->dial_opts->identity; - headers[nheaders].length = strlen(req->dial_opts->identity); - nheaders++; - } - if (req->dial_opts->app_data != NULL) { - headers[nheaders].header_id = AppDataHeader; - headers[nheaders].value = req->dial_opts->app_data; - headers[nheaders].length = req->dial_opts->app_data_sz; - nheaders++; - } + + if (req->dial_opts.identity != NULL) { + headers[nheaders].header_id = TerminatorIdentityHeader; + headers[nheaders].value = (uint8_t *) req->dial_opts.identity; + headers[nheaders].length = strlen(req->dial_opts.identity); + nheaders++; + } + + if (req->dial_opts.app_data != NULL) { + headers[nheaders].header_id = AppDataHeader; + headers[nheaders].value = req->dial_opts.app_data; + headers[nheaders].length = req->dial_opts.app_data_sz; + nheaders++; } req->waiter = ziti_channel_send_for_reply(ch, content_type, headers, nheaders, From 1a1a597516ab1feb819d7223941c85bfe589350a Mon Sep 17 00:00:00 2001 From: eugene Date: Tue, 2 Jan 2024 10:51:02 -0500 Subject: [PATCH 3/3] check for valid dial_opts.identity and app_data --- library/connect.c | 11 ++++++++--- tests/integ/bootstrap.exp | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/library/connect.c b/library/connect.c index 23777c1a..72c23a36 100644 --- a/library/connect.c +++ b/library/connect.c @@ -87,9 +87,14 @@ static void conn_set_state(struct ziti_conn *conn, enum conn_state state) { } static void clone_ziti_dial_opts(ziti_dial_opts *dest, const ziti_dial_opts *dial_opts) { - memcpy(dest, dial_opts, sizeof(ziti_dial_opts)); - if (dial_opts->identity != NULL) dest->identity = strdup(dial_opts->identity); - if (dial_opts->app_data != NULL) { + *dest = DEFAULT_DIAL_OPTS; + + dest->connect_timeout_seconds = dial_opts->connect_timeout_seconds; + if (dial_opts->identity != NULL && dial_opts->identity[0] != '\0') { + dest->identity = strdup(dial_opts->identity); + } + + if (dial_opts->app_data != NULL && dial_opts->app_data_sz > 0) { dest->app_data = malloc(dial_opts->app_data_sz); dest->app_data_sz = dial_opts->app_data_sz; memcpy(dest->app_data, dial_opts->app_data, dial_opts->app_data_sz); diff --git a/tests/integ/bootstrap.exp b/tests/integ/bootstrap.exp index eccdd93f..461fdda9 100644 --- a/tests/integ/bootstrap.exp +++ b/tests/integ/bootstrap.exp @@ -1,6 +1,6 @@ #!expect -set timeout 20 +set timeout 30 proc abort errs { puts "test failed: $errs" exit 2