From c4411fc97eb395f5482ffda6e9c1a48e040d68b1 Mon Sep 17 00:00:00 2001 From: ekoby <7406535+ekoby@users.noreply.github.com> Date: Tue, 1 Oct 2024 14:24:12 -0400 Subject: [PATCH] OIDC redirect socket (#735) * more robust error handling of OIDC redirect * make sure OIDC redirect sockets are in blocking mode * use win32 recv/send * copy oidc.audience * update tlsuv@0.32.3 --- deps/CMakeLists.txt | 2 +- library/auth_queries.c | 30 +++++++------ library/authenticators.c | 6 ++- library/oidc.c | 93 ++++++++++++++++++++++++++++++++++------ library/posture.c | 10 +++-- library/ziti.c | 1 + 6 files changed, 110 insertions(+), 32 deletions(-) diff --git a/deps/CMakeLists.txt b/deps/CMakeLists.txt index 6621f302..019aedae 100644 --- a/deps/CMakeLists.txt +++ b/deps/CMakeLists.txt @@ -9,7 +9,7 @@ if (NOT TARGET tlsuv) else () FetchContent_Declare(tlsuv GIT_REPOSITORY https://github.com/openziti/tlsuv.git - GIT_TAG v0.32.2 + GIT_TAG v0.32.3 ) FetchContent_MakeAvailable(tlsuv) endif (tlsuv_DIR) diff --git a/library/auth_queries.c b/library/auth_queries.c index eb3869b6..30a7a89b 100644 --- a/library/auth_queries.c +++ b/library/auth_queries.c @@ -134,8 +134,8 @@ void ziti_mfa_enroll_post_internal_cb(void *empty, const ziti_error *err, void * if (err == NULL) { ziti_ctrl_get_mfa(ztx_get_controller(ztx), ziti_mfa_enroll_get_internal_cb, ctx); } else { - ZTX_LOG(ERROR, "error during create MFA call: %d - %s - %s", err->http_code, err->code, err->message); - mfa_enroll_cb_ctx->cb(mfa_enroll_cb_ctx->ztx, err->err, NULL, mfa_enroll_cb_ctx->cb_ctx); + ZTX_LOG(ERROR, "error during create MFA call: %d - %s - %s", (int)err->http_code, err->code, err->message); + mfa_enroll_cb_ctx->cb(mfa_enroll_cb_ctx->ztx, (int)err->err, NULL, mfa_enroll_cb_ctx->cb_ctx); FREE(ctx); } } @@ -146,8 +146,8 @@ void ziti_mfa_enroll_get_internal_cb(ziti_mfa_enrollment *mfa_enrollment, const if (err != NULL) { if (err->http_code != 404) { - ZTX_LOG(ERROR, "error during enroll MFA call: %d - %s - %s", err->http_code, err->code, err->message); - mfa_enroll_cb_ctx->cb(mfa_enroll_cb_ctx->ztx, err->err, NULL, mfa_enroll_cb_ctx->cb_ctx); + ZTX_LOG(ERROR, "error during enroll MFA call: %d - %s - %s", (int)err->http_code, err->code, err->message); + mfa_enroll_cb_ctx->cb(mfa_enroll_cb_ctx->ztx, (int)err->err, NULL, mfa_enroll_cb_ctx->cb_ctx); FREE(ctx); return; } @@ -182,8 +182,8 @@ void ziti_mfa_remove_internal_cb(void *empty, const ziti_error *err, void *ctx) ziti_context ztx = mfa_cb_ctx->ztx; if (err != NULL) { - ZTX_LOG(ERROR, "error during remove MFA call: %d - %s - %s", err->http_code, err->code, err->message); - mfa_cb_ctx->cb(mfa_cb_ctx->ztx, err->err, mfa_cb_ctx->cb_ctx); + ZTX_LOG(ERROR, "error during remove MFA call: %d - %s - %s", (int)err->http_code, err->code, err->message); + mfa_cb_ctx->cb(mfa_cb_ctx->ztx, (int)err->err, mfa_cb_ctx->cb_ctx); } else { mfa_cb_ctx->cb(mfa_cb_ctx->ztx, ZITI_OK, mfa_cb_ctx->cb_ctx); } @@ -212,8 +212,8 @@ void ziti_mfa_verify_internal_cb(void *empty, const ziti_error *err, void *ctx) ziti_context ztx = mfa_cb_ctx->ztx; if (err != NULL) { - ZTX_LOG(ERROR, "error during verify MFA call: %d - %s - %s", err->http_code, err->code, err->message); - mfa_cb_ctx->cb(mfa_cb_ctx->ztx, err->err, mfa_cb_ctx->cb_ctx); + ZTX_LOG(ERROR, "error during verify MFA call: %d - %s - %s", (int)err->http_code, err->code, err->message); + mfa_cb_ctx->cb(mfa_cb_ctx->ztx, (int)err->err, mfa_cb_ctx->cb_ctx); } else { ziti_force_api_session_refresh(ztx); mfa_cb_ctx->cb(mfa_cb_ctx->ztx, ZITI_OK, mfa_cb_ctx->cb_ctx); @@ -242,10 +242,13 @@ void ziti_mfa_get_recovery_codes_internal_cb(ziti_mfa_recovery_codes *rc, const ziti_mfa_recovery_codes_cb_ctx *mfa_recovery_codes_cb_ctx = ctx; ziti_context ztx = mfa_recovery_codes_cb_ctx->ztx; if (err != NULL) { - ZTX_LOG(ERROR, "error during get recovery codes MFA call: %d - %s - %s", err->http_code, err->code, err->message); - mfa_recovery_codes_cb_ctx->cb(mfa_recovery_codes_cb_ctx->ztx, err->err, NULL, mfa_recovery_codes_cb_ctx->cb_ctx); + ZTX_LOG(ERROR, "error during get recovery codes MFA call: %d - %s - %s", + (int)err->http_code, err->code, err->message); + mfa_recovery_codes_cb_ctx->cb(mfa_recovery_codes_cb_ctx->ztx, (int)err->err, NULL, + mfa_recovery_codes_cb_ctx->cb_ctx); } else { - mfa_recovery_codes_cb_ctx->cb(mfa_recovery_codes_cb_ctx->ztx, ZITI_OK, rc->recovery_codes, mfa_recovery_codes_cb_ctx->cb_ctx); + mfa_recovery_codes_cb_ctx->cb(mfa_recovery_codes_cb_ctx->ztx, ZITI_OK, rc->recovery_codes, + mfa_recovery_codes_cb_ctx->cb_ctx); free_ziti_mfa_recovery_codes(rc); } @@ -273,8 +276,9 @@ void ziti_mfa_post_recovery_codes_internal_cb(void *empty, const ziti_error *err ziti_context ztx = mfa_recovery_codes_cb_ctx->ztx; if (err != NULL) { - ZTX_LOG(ERROR, "error during create recovery codes MFA call: %d - %s - %s", err->http_code, err->code, err->message); - mfa_recovery_codes_cb_ctx->cb(mfa_recovery_codes_cb_ctx->ztx, err->err, NULL, mfa_recovery_codes_cb_ctx->cb_ctx); + ZTX_LOG(ERROR, "error during create recovery codes MFA call: %d - %s - %s", + (int)err->http_code, err->code, err->message); + mfa_recovery_codes_cb_ctx->cb(mfa_recovery_codes_cb_ctx->ztx, (int)err->err, NULL, mfa_recovery_codes_cb_ctx->cb_ctx); } else { ziti_mfa_get_recovery_codes(mfa_recovery_codes_cb_ctx->ztx, mfa_recovery_codes_cb_ctx->code, mfa_recovery_codes_cb_ctx->cb, mfa_recovery_codes_cb_ctx->cb_ctx); diff --git a/library/authenticators.c b/library/authenticators.c index 0dcfe71a..bb2f6b01 100644 --- a/library/authenticators.c +++ b/library/authenticators.c @@ -33,7 +33,8 @@ static void extend_cb(ziti_extend_cert_authenticator_resp* resp, const ziti_erro ziti_context ztx = wrapped_ctx->ztx; if(err){ - ZTX_LOG(ERROR, "error response returned when attempting to extend authenticator: %d %s: %s, calling cb", err->http_code, err->code, err->message); + ZTX_LOG(ERROR, "error response returned when attempting to extend authenticator: %d %s: %s, calling cb", + (int)err->http_code, err->code, err->message); if(err->http_code == 404) { wrapped_ctx->extend_cb(ztx, NULL, ZITI_NOT_FOUND, wrapped_ctx->ctx); } else if(strncmp(err->code, CAN_NOT_UPDATE_AUTHENTICATOR, strlen(CAN_NOT_UPDATE_AUTHENTICATOR)) == 0){ @@ -62,7 +63,8 @@ static void verify_cb(void* empty, const ziti_error* err, void* ctx){ ziti_context ztx = wrapped_ctx->ztx; if(err) { - ZTX_LOG(ERROR, "error response returned when attempting to verify extended authenticator: %d %s: %s", err->http_code, err->code, err->message); + ZTX_LOG(ERROR, "error response returned when attempting to verify extended authenticator: %d %s: %s", + (int)err->http_code, err->code, err->message); if(err->http_code == 404) { wrapped_ctx->verify_cb(ztx, ZITI_NOT_FOUND, wrapped_ctx->ctx); } else if(strncmp(err->code, CAN_NOT_UPDATE_AUTHENTICATOR, strlen(CAN_NOT_UPDATE_AUTHENTICATOR)) == 0){ diff --git a/library/oidc.c b/library/oidc.c index ed6d9b4a..c1acf10a 100644 --- a/library/oidc.c +++ b/library/oidc.c @@ -1,4 +1,4 @@ -// Copyright (c) 2023. NetFoundry Inc. +// Copyright (c) 2023-2024. NetFoundry Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ #include "ziti/errors.h" #include "ziti/ziti_buffer.h" #include "ziti/ziti_model.h" +#include "buffer.h" #define code_len 40 #define code_verifier_len sodium_base64_ENCODED_LEN(code_len, sodium_base64_VARIANT_URLSAFE_NO_PADDING) @@ -368,13 +369,60 @@ struct ext_link_req { uv_os_sock_t sock; auth_req *req; char *code; + int err; }; +static int set_blocking(uv_os_sock_t sock) { +#ifdef _WIN32 + unsigned long mode = 0; + if (ioctlsocket(sock, FIONBIO, &mode) != 0) { + int err = WSAGetLastError(); + ZITI_LOG(ERROR, "failed to set socket to blocking: %d", err); + return err; + } +#else + int flags = fcntl(sock, F_GETFL, 0); + flags = flags & ~O_NONBLOCK; + if (fcntl(sock, F_SETFL, flags) != 0) { + int err = errno; + ZITI_LOG(ERROR, "failed to set socket to blocking: %d/%s", err, strerror(err)); + return err; + } +#endif + return 0; +} + +#if _WIN32 +#define close_socket(s) closesocket(s) +#else +#define close_socket(s) close(s) +#endif + static void ext_accept(uv_work_t *wr) { struct ext_link_req *elr = (struct ext_link_req *) wr; uv_os_sock_t clt = accept(elr->sock, NULL, NULL); + set_blocking(clt); + char buf[1024]; - ssize_t c = read(clt, buf, sizeof(buf) - 1); + ssize_t c; +#if _WIN32 + c = recv(clt, buf, sizeof(buf) -1, 0); +#else + c = read(clt, buf, sizeof(buf) - 1); +#endif + if (c < 0) { + int err; +#if _WIN32 + err = WSAGetLastError(); +#else + err = errno; +#endif + ZITI_LOG(ERROR, "read failed: %d/%s", err, strerror(err)); + elr->err = err; + close_socket(clt); + return; + } + buf[c] = 0; char *cs = strstr(buf, "code="); @@ -385,9 +433,14 @@ static void ext_accept(uv_work_t *wr) { "Connection: close\r\n" "\r\n" "Unexpected auth request:
";
+#if _WIN32
+        send(clt, resp, sizeof(resp), 0);
+        send(clt, buf, c, 0);
+#else
         write(clt, resp, sizeof(resp));
         write(clt, buf, c);
-        close(clt);
+#endif
+        close_socket(clt);
         return;
     }
     cs += strlen("code=");
@@ -402,23 +455,38 @@ static void ext_accept(uv_work_t *wr) {
     memcpy(code, cs, ce - cs);
     elr->code = code;
 
-    char resp[] = "HTTP/1.1 200 OK\r\n"
-                  "Content-Type: text/html\r\n"
-                  "\r\n"
-                  ""
-                  "You may close this window";
-    write(clt, resp, sizeof(resp));
-    close(clt);
+    string_buf_t resp_buf;
+    string_buf_init(&resp_buf);
+
+    char resp_body[] = ""
+                       "You may close this window";
+#define RESP_FMT "HTTP/1.1 200 OK\r\n"\
+                  "Content-Type: text/html\r\n"\
+                  "Content-Length: %zd\r\n"\
+                  "\r\n%s"
+    string_buf_fmt(&resp_buf, RESP_FMT, strlen(resp_body), resp_body);
+    size_t resp_len;
+    char *resp = string_buf_to_string(&resp_buf, &resp_len);
+#if _WIN32
+    send(clt, resp, resp_len, 0);
+#else
+    write(clt, resp, resp_len);
+#endif
+
+    free(resp);
+    string_buf_free(&resp_buf);
+
+    close_socket(clt);
 }
 
 static void ext_done(uv_work_t *wr, int status) {
     struct ext_link_req *elr = (struct ext_link_req *) wr;
-    close(elr->sock);
+    close_socket(elr->sock);
 
     if (elr->code) {
         request_token(elr->req, elr->code);
     } else {
-        failed_auth_req(elr->req, "code not received");
+        failed_auth_req(elr->req, elr->err ? strerror(elr->err) : "code not received");
     }
 
     free(elr->code);
@@ -442,6 +510,7 @@ static void start_ext_auth(auth_req *req, const char *ep, int qc, tlsuv_http_pai
         close(sock);
         return;
     }
+    set_blocking(sock);
 
     struct ext_link_req *elr = calloc(1, sizeof(*elr));
     elr->sock = sock;
diff --git a/library/posture.c b/library/posture.c
index 8e18aa58..15803d66 100644
--- a/library/posture.c
+++ b/library/posture.c
@@ -433,7 +433,8 @@ static void handle_pr_resp_timer_events(ziti_context ztx, ziti_pr_response *pr_r
         for(service_timer = pr_resp->services; *service_timer != NULL; service_timer++){
             NEWP(val, bool);
             *val = true;
-            ZTX_LOG(DEBUG, "handle_pr_resp_timer_events: forcing service name[%s] id[%s] with timeout[%d] timeoutRemaining[%d]", (*service_timer)->name, (*service_timer)->id, *(*service_timer)->timeout, *(*service_timer)->timeoutRemaining);
+            ZTX_LOG(DEBUG, "handle_pr_resp_timer_events: forcing service name[%s] id[%s] with timeout[%d] timeoutRemaining[%d]",
+                    (*service_timer)->name, (*service_timer)->id, (int)*(*service_timer)->timeout, (int)*(*service_timer)->timeoutRemaining);
             ziti_force_service_update(ztx, (*service_timer)->id);
         }
 
@@ -452,7 +453,7 @@ static void ziti_pr_post_bulk_cb(ziti_pr_response *pr_resp, const ziti_error *er
     // if ztx is disabled this request is cancelled and posture_checks is cleared
     if (ztx->posture_checks) {
         if (err != NULL) {
-            ZTX_LOG(ERROR, "error during bulk posture response submission (%d) %s", err->http_code, err->message);
+            ZTX_LOG(ERROR, "error during bulk posture response submission (%d) %s", (int)err->http_code, err->message);
             ztx->posture_checks->must_send = true; //error, must try again
         } else {
             ztx->posture_checks->must_send = false; //did not error, can skip submissions
@@ -489,7 +490,7 @@ static void ziti_pr_post_cb(ziti_pr_response *pr_resp, const ziti_error *err, vo
     ZTX_LOG(DEBUG, "ziti_pr_post_cb: starting");
 
     if (err != NULL) {
-        ZTX_LOG(ERROR, "error during individual posture response submission (%d) %s - object: %s", err->http_code,
+        ZTX_LOG(ERROR, "error during individual posture response submission (%d) %s - object: %s", (int)err->http_code,
                  err->message, pr_ctx->info->obj);
         ziti_pr_set_info_errored(pr_ctx->ztx, pr_ctx->info->id);
     } else {
@@ -850,7 +851,8 @@ static void process_check_work(uv_work_t *w) {
 void ziti_endpoint_state_pr_cb(ziti_pr_response *pr_resp, const ziti_error *err, void *ctx) {
     ziti_context ztx = ctx;
     if (err) {
-        ZTX_LOG(ERROR, "error during endpoint state posture response submission: %d - %s", err->http_code, err->message);
+        ZTX_LOG(ERROR, "error during endpoint state posture response submission: %d - %s",
+                (int)err->http_code, err->message);
     } else {
         ZTX_LOG(INFO, "endpoint state sent");
         handle_pr_resp_timer_events(ztx, pr_resp);
diff --git a/library/ziti.c b/library/ziti.c
index 378e1c91..be3a7ea7 100644
--- a/library/ziti.c
+++ b/library/ziti.c
@@ -1609,6 +1609,7 @@ int ziti_context_init(ziti_context *ztx, const ziti_config *config) {
         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));