From 2bb7df8555f4ed948918465b1088ffdae6430df8 Mon Sep 17 00:00:00 2001 From: eugene Date: Thu, 10 Oct 2024 11:27:12 -0400 Subject: [PATCH 1/2] make sure redirect socket is timed out if user fumbles external auth --- library/oidc.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/library/oidc.c b/library/oidc.c index acc767ba..0b43de6c 100644 --- a/library/oidc.c +++ b/library/oidc.c @@ -411,14 +411,43 @@ static int set_blocking(uv_os_sock_t sock) { #if _WIN32 #define close_socket(s) closesocket(s) +#define sock_error WSAGetLastError() #else #define close_socket(s) close(s) +#define sock_error errno #endif +#define OIDC_ACCEPT_TIMEOUT 30 +#define OIDC_REQ_TIMEOUT 5 static void ext_accept(uv_work_t *wr) { struct ext_link_req *elr = (struct ext_link_req *) wr; + + fd_set fds; + FD_ZERO(&fds); + FD_SET(elr->sock, &fds); + int rc = select(elr->sock + 1, &fds, NULL, NULL, &(struct timeval){ + .tv_sec = OIDC_ACCEPT_TIMEOUT, + }); + if (rc == 0) { + elr->err = ETIMEDOUT; + ZITI_LOG(WARN, "redirect_uri was not called in time"); + return; + } else if (rc < 0) { + elr->err = sock_error; + return; + } + uv_os_sock_t clt = accept(elr->sock, NULL, NULL); - set_blocking(clt); + FD_ZERO(&fds); + FD_SET(clt, &fds); + rc = select(clt + 1, &fds, NULL, NULL, &(struct timeval){ + .tv_sec = OIDC_REQ_TIMEOUT, + }); + if (rc <= 0) { + elr->err = rc == 0 ? ETIMEDOUT : sock_error; + close_socket(clt); + return; + } char buf[1024]; ssize_t c; @@ -428,12 +457,7 @@ static void ext_accept(uv_work_t *wr) { c = read(clt, buf, sizeof(buf) - 1); #endif if (c < 0) { - int err; -#if _WIN32 - err = WSAGetLastError(); -#else - err = errno; -#endif + int err = sock_error; ZITI_LOG(ERROR, "read failed: %d/%s", err, strerror(err)); elr->err = err; close_socket(clt); From 365ccd66bbb3bd3ffb1b6ca8ecdf300ae7b5321f Mon Sep 17 00:00:00 2001 From: eugene Date: Thu, 10 Oct 2024 11:27:40 -0400 Subject: [PATCH 2/2] verify correct oidc configuration --- library/ziti.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/library/ziti.c b/library/ziti.c index 26d958a9..011e1a66 100644 --- a/library/ziti.c +++ b/library/ziti.c @@ -1588,6 +1588,29 @@ void ziti_queue_work(ziti_context ztx, ztx_work_f w, void *data) { uv_async_send(&ztx->w_async); } +static void copy_oidc(ziti_context ztx, const ziti_jwt_signer *oidc) { + if (oidc == NULL) return; + if (oidc->provider_url == NULL) { + ZITI_LOG(ERROR, "invalid OIDC config `externalAuthUrl` is missing"); + return; + } + if (oidc->client_id == NULL) { + ZITI_LOG(ERROR, "invalid OIDC config `clientId` is missing"); + return; + } + + ztx->config.id.oidc = calloc(1, sizeof(*oidc)); + ztx->config.id.oidc->client_id = strdup(oidc->client_id); + ztx->config.id.oidc->provider_url = strdup(oidc->provider_url); + if (oidc->audience) { + ztx->config.id.oidc->audience = strdup(oidc->audience); + } + const char *scope; + MODEL_LIST_FOREACH(scope, oidc->scopes) { + model_list_append(&ztx->config.id.oidc->scopes, strdup(scope)); + } +} + int ziti_context_init(ziti_context *ztx, const ziti_config *config) { if (config == NULL || (config->controller_url == NULL && @@ -1636,16 +1659,7 @@ int ziti_context_init(ziti_context *ztx, const ziti_config *config) { } if (config->id.key) ctx->config.id.key = strdup(config->id.key); if (config->id.cert) ctx->config.id.cert = strdup(config->id.cert); - if (config->id.oidc) { - ctx->config.id.oidc = calloc(1, sizeof(*ctx->config.id.oidc)); - ctx->config.id.oidc->client_id = strdup(config->id.oidc->client_id); - ctx->config.id.oidc->provider_url = strdup(config->id.oidc->provider_url); - ctx->config.id.oidc->audience = strdup(config->id.oidc->audience); - const char *scope; - MODEL_LIST_FOREACH(scope, config->id.oidc->scopes) { - model_list_append(&ctx->config.id.oidc->scopes, strdup(scope)); - } - } + copy_oidc(ctx, config->id.oidc); ctx->opts = default_options;