From e85e3083c15bc023ef7427cd54188c3f9326a136 Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 21 Dec 2023 13:44:15 +0100 Subject: [PATCH 1/4] fix: l_ctx->cacertfile placement --- c_src/quicer_ctx.c | 2 +- c_src/quicer_listener.c | 2 +- c_src/quicer_nif.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index e43c9826..f0be9b9b 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -66,8 +66,8 @@ init_l_ctx() l_ctx->config_resource = init_config_ctx(); l_ctx->acceptor_queue = AcceptorQueueNew(); l_ctx->lock = enif_mutex_create("quicer:l_ctx"); - l_ctx->cacertfile = NULL; #if defined(QUICER_USE_TRUSTED_STORE) + l_ctx->cacertfile = NULL; l_ctx->trusted_store = NULL; #endif l_ctx->is_closed = TRUE; diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 9f7b4d0c..f879d9cb 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -320,7 +320,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) if (is_verify && cacertfile) { - l_ctx->cacertfile = cacertfile; + // We do our own certificate verification against the certificates // in cacertfile // @see QUIC_CONNECTION_EVENT_PEER_CERTIFICATE_RECEIVED diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 436b692a..35ac915f 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -851,7 +851,7 @@ resource_listener_dealloc_callback(__unused_parm__ ErlNifEnv *env, void *obj) #if defined(QUICER_USE_TRUSTED_STORE) if (l_ctx->cacertfile) { - CXPLAT_FREE(l_ctx->cacertfile, QUICER_CACERTFILE); + free(l_ctx->cacertfile); l_ctx->cacertfile = NULL; } #endif // QUICER_USE_TRUSTED_STORE From 97b08b252d613f1ef9968063aa9e701e3e4a11c9 Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 21 Dec 2023 14:42:44 +0100 Subject: [PATCH 2/4] fix: remove l_ctx->cacertfile --- c_src/quicer_ctx.c | 1 - c_src/quicer_listener.c | 9 +++++++-- c_src/quicer_nif.c | 8 -------- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index f0be9b9b..7cc1410f 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -67,7 +67,6 @@ init_l_ctx() l_ctx->acceptor_queue = AcceptorQueueNew(); l_ctx->lock = enif_mutex_create("quicer:l_ctx"); #if defined(QUICER_USE_TRUSTED_STORE) - l_ctx->cacertfile = NULL; l_ctx->trusted_store = NULL; #endif l_ctx->is_closed = TRUE; diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index f879d9cb..f053f1e6 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -282,6 +282,8 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // Start build CredConfig from with listen opts QUIC_CREDENTIAL_CONFIG CredConfig; + + // change from here CxPlatZeroMemory(&CredConfig, sizeof(CredConfig)); CredConfig.Flags = QUIC_CREDENTIAL_FLAG_NONE; @@ -328,8 +330,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) #if defined(QUICER_USE_TRUSTED_STORE) CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; - l_ctx->cacertfile = cacertfile; - if (!build_trustedstore(l_ctx->cacertfile, &l_ctx->trusted_store)) + if (!build_trustedstore(cacertfile, &l_ctx->trusted_store)) { ret = ERROR_TUPLE_2(ATOM_CERT_ERROR); goto exit; @@ -411,6 +412,10 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) &l_ctx->config_resource->Configuration, &CredConfig); +#if defined(QUICER_USE_TRUSTED_STORE) + free(cacertfile); +#endif // QUICER_USE_TRUSTED_STORE + if (!IS_SAME_TERM(ATOM_OK, estatus)) { ret = ERROR_TUPLE_3(ATOM_CONFIG_ERROR, estatus); diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 35ac915f..92fa47e6 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -847,14 +847,6 @@ resource_listener_dealloc_callback(__unused_parm__ ErlNifEnv *env, void *obj) { TP_CB_3(skip, (uintptr_t)l_ctx->Listener, 0); } - -#if defined(QUICER_USE_TRUSTED_STORE) - if (l_ctx->cacertfile) - { - free(l_ctx->cacertfile); - l_ctx->cacertfile = NULL; - } -#endif // QUICER_USE_TRUSTED_STORE deinit_l_ctx(l_ctx); // @TODO notify acceptors that the listener is closed TP_CB_3(end, (uintptr_t)l_ctx->Listener, 0); From 33e2353bf477810ec67a906f41d126f526bfc4e1 Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 21 Dec 2023 17:07:08 +0100 Subject: [PATCH 3/4] fix: remove cacertfile from l_ctx --- c_src/quicer_ctx.h | 1 - 1 file changed, 1 deletion(-) diff --git a/c_src/quicer_ctx.h b/c_src/quicer_ctx.h index 75e7a5ac..b1f454c1 100644 --- a/c_src/quicer_ctx.h +++ b/c_src/quicer_ctx.h @@ -65,7 +65,6 @@ typedef struct QuicerListenerCTX ErlNifMonitor owner_mon; ErlNifEnv *env; ErlNifMutex *lock; - char *cacertfile; #if defined(QUICER_USE_TRUSTED_STORE) X509_STORE *trusted_store; #endif From 18c30e0cfed97ebfbeea181a25365f5cec0d7d4c Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 21 Dec 2023 18:31:55 +0100 Subject: [PATCH 4/4] refactor: new fun for build cred config --- c_src/quicer_listener.c | 99 ++++++++++------------------------ c_src/quicer_tls.c | 97 +++++++++++++++++++++++++++++++++ c_src/quicer_tls.h | 6 +++ src/quicer_listener.erl | 2 + test/quicer_SUITE.erl | 4 +- test/quicer_listener_SUITE.erl | 12 +++++ 6 files changed, 149 insertions(+), 71 deletions(-) diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index f053f1e6..00126f6c 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -265,7 +265,6 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) QUIC_ADDR Address = {}; HQUIC Registration = NULL; - char *cacertfile = NULL; QuicerRegistrationCTX *target_r_ctx = NULL; @@ -283,29 +282,16 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // Start build CredConfig from with listen opts QUIC_CREDENTIAL_CONFIG CredConfig; - // change from here - CxPlatZeroMemory(&CredConfig, sizeof(CredConfig)); - - CredConfig.Flags = QUIC_CREDENTIAL_FLAG_NONE; - - if (!parse_cert_options(env, options, &CredConfig)) - { - return ERROR_TUPLE_2(ATOM_QUIC_TLS); - } +#if defined(QUICER_USE_TRUSTED_STORE) + X509_STORE *trusted_store = NULL; + ret = eoptions_to_cred_config(env, options, &CredConfig, &trusted_store); +#else + ret = eoptions_to_cred_config(env, options, &CredConfig, NULL); +#endif // QUICER_USE_TRUSTED_STORE - BOOLEAN is_verify = FALSE; - if (!parse_verify_options(env, options, &CredConfig, TRUE, &is_verify)) + if (!IS_SAME_TERM(ret, ATOM_OK)) { - return ERROR_TUPLE_2(ATOM_VERIFY); - } - - if (!parse_cacertfile_option(env, options, &cacertfile)) - { - // TLS opt error not file content error - free(cacertfile); - cacertfile = NULL; - free_certificate(&CredConfig); - return ERROR_TUPLE_2(ATOM_CACERTFILE); + return ERROR_TUPLE_2(ret); } // Now build l_ctx @@ -313,47 +299,18 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) if (!l_ctx) { - free(cacertfile); - cacertfile = NULL; free_certificate(&CredConfig); - return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); - } - CxPlatRefInitialize(&l_ctx->ref_count); - - if (is_verify && cacertfile) - { - - // We do our own certificate verification against the certificates - // in cacertfile - // @see QUIC_CONNECTION_EVENT_PEER_CERTIFICATE_RECEIVED - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_INDICATE_CERTIFICATE_RECEIVED; - #if defined(QUICER_USE_TRUSTED_STORE) - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; - if (!build_trustedstore(cacertfile, &l_ctx->trusted_store)) - { - ret = ERROR_TUPLE_2(ATOM_CERT_ERROR); - goto exit; - } -#else - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_SET_CA_CERTIFICATE_FILE; - CredConfig.CaCertificateFile = cacertfile; -#if defined(__APPLE__) - // This seems only needed for macOS - CredConfig.Flags - |= QUIC_CREDENTIAL_FLAG_USE_TLS_BUILTIN_CERTIFICATE_VALIDATION; -#endif // __APPLE__ + X509_STORE_free(trusted_store); #endif // QUICER_USE_TRUSTED_STORE + return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); } - else - { // NO verify peer -#if !defined(QUICER_USE_TRUSTED_STORE) - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; +#if defined(QUICER_USE_TRUSTED_STORE) + l_ctx->trusted_store = trusted_store; #endif // QUICER_USE_TRUSTED_STORE - // since we don't use cacertfile, free it - free(cacertfile); - cacertfile = NULL; - } + CxPlatRefInitialize(&l_ctx->ref_count); + + // ********* ANY ERROR below this line should goto `exit` ************** // Set owner for l_ctx if (!enif_self(env, &(l_ctx->listenerPid))) @@ -405,20 +362,15 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } // Now load server config - ERL_NIF_TERM estatus - = ServerLoadConfiguration(env, + ret = ServerLoadConfiguration(env, &options, Registration, &l_ctx->config_resource->Configuration, &CredConfig); - -#if defined(QUICER_USE_TRUSTED_STORE) - free(cacertfile); -#endif // QUICER_USE_TRUSTED_STORE - - if (!IS_SAME_TERM(ATOM_OK, estatus)) + if (!IS_SAME_TERM(ATOM_OK, ret)) { - ret = ERROR_TUPLE_3(ATOM_CONFIG_ERROR, estatus); + // @TODO unsure 3 elem tuple is the best way to return error + ret = ERROR_TUPLE_3(ATOM_CONFIG_ERROR, ret); goto exit; } @@ -440,19 +392,23 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) &l_ctx->Listener))) { // Server Configuration should be destroyed + // @FIXME here leaks config? + // enif_release_resource(l_ctx->config_resource); l_ctx->config_resource->Configuration = NULL; ret = ERROR_TUPLE_3(ATOM_LISTENER_OPEN_ERROR, ATOM_STATUS(Status)); goto exit; } l_ctx->is_closed = FALSE; - // Link to registration + // Link to registration only when ListenerOpen success if (target_r_ctx) { enif_mutex_lock(target_r_ctx->lock); CxPlatListInsertTail(&target_r_ctx->Listeners, &l_ctx->RegistrationLink); enif_mutex_unlock(target_r_ctx->lock); } + + // Now try to start listener unsigned alpn_buffer_length = 0; QUIC_BUFFER alpn_buffers[MAX_ALPN]; @@ -482,13 +438,16 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) ret = ERROR_TUPLE_3(ATOM_LISTENER_START_ERROR, ATOM_STATUS(Status)); goto exit; } - ERL_NIF_TERM listenHandle = enif_make_resource(env, l_ctx); + ERL_NIF_TERM listenHandle = enif_make_resource(env, l_ctx); + // @TODO move it to earlier? free_certificate(&CredConfig); return OK_TUPLE_2(listenHandle); exit: // errors.. - free(cacertfile); +#if defined(QUICER_USE_TRUSTED_STORE) + X509_STORE_free(trusted_store); +#endif // QUICER_USE_TRUSTED_STORE free_certificate(&CredConfig); destroy_l_ctx(l_ctx); return ret; diff --git a/c_src/quicer_tls.c b/c_src/quicer_tls.c index 2dd41422..ec1922a8 100644 --- a/c_src/quicer_tls.c +++ b/c_src/quicer_tls.c @@ -298,3 +298,100 @@ parse_sslkeylogfile_option(ErlNifEnv *env, c_ctx->TlsSecrets = TlsSecrets; c_ctx->ssl_keylogfile = keylogfile; } + +/* +** Convert eterm options (a map) to QUIC_CREDENTIAL_CONFIG +** +** @NOTE We zero reset CredConfig +** @NOTE Also build trusted store if needed +*/ +ERL_NIF_TERM +eoptions_to_cred_config(ErlNifEnv *env, + ERL_NIF_TERM eoptions, + QUIC_CREDENTIAL_CONFIG *CredConfig, + X509_STORE **trusted_store) +{ + BOOLEAN is_verify = FALSE; + char *cacertfile = NULL; + ERL_NIF_TERM ret = ATOM_OK; + + CXPLAT_FRE_ASSERT(CredConfig); + +#if defined(QUICER_USE_TRUSTED_STORE) + CXPLAT_FRE_ASSERT(trusted_store); +#else + CXPLAT_FRE_ASSERT(trusted_store == NULL); +#endif // QUICER_USE_TRUSTED_STORE + + CxPlatZeroMemory(CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); + + CredConfig->Flags = QUIC_CREDENTIAL_FLAG_NONE; + + // Handle the certificate, key, password options + if (!parse_cert_options(env, eoptions, CredConfig)) + { + return ATOM_QUIC_TLS; + } + + // Handle the `verify` options + if (!parse_verify_options(env, eoptions, CredConfig, TRUE, &is_verify)) + { + ret = ATOM_VERIFY; + goto exit; + ; + } + + // Hanlde the `cacertfile` options + if (!parse_cacertfile_option(env, eoptions, &cacertfile)) + { + // TLS opt error not file content error + ret = ATOM_CACERTFILE; + goto exit; + } + + // Set flags for certificate verification + if (is_verify && cacertfile) + { // === START of verify peer with cacertfile === // + + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_INDICATE_CERTIFICATE_RECEIVED; + +#if defined(QUICER_USE_TRUSTED_STORE) + // We do our own verification with the cacert in trusted_store + // @see QUIC_CONNECTION_EVENT_PEER_CERTIFICATE_RECEIVED + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; + if (!build_trustedstore(cacertfile, trusted_store)) + { + ret = ATOM_CERT_ERROR; + goto exit; + } + free(cacertfile); + cacertfile = NULL; +#else + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_SET_CA_CERTIFICATE_FILE; + CredConfig->CaCertificateFile = cacertfile; +#if defined(__APPLE__) + // This seems only needed for macOS + CredConfig->Flags + |= QUIC_CREDENTIAL_FLAG_USE_TLS_BUILTIN_CERTIFICATE_VALIDATION; +#endif // __APPLE__ +#endif // QUICER_USE_TRUSTED_STORE + } // === END of verify peer with cacertfile === // + else + { // NO verify peer +#if !defined(QUICER_USE_TRUSTED_STORE) + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; +#endif // QUICER_USE_TRUSTED_STORE + // since we don't use cacertfile, free it + free(cacertfile); + cacertfile = NULL; + } + return ATOM_OK; + +exit: +#if defined(QUICER_USE_TRUSTED_STORE) + free(cacertfile); + cacertfile = NULL; +#endif // QUICER_USE_TRUSTED_STORE + free_certificate(CredConfig); + return ret; +} diff --git a/c_src/quicer_tls.h b/c_src/quicer_tls.h index 3aa5e724..90636b30 100644 --- a/c_src/quicer_tls.h +++ b/c_src/quicer_tls.h @@ -42,4 +42,10 @@ void free_certificate(QUIC_CREDENTIAL_CONFIG *cc); void parse_sslkeylogfile_option(ErlNifEnv *env, ERL_NIF_TERM options, QuicerConnCTX *c_ctx); + +ERL_NIF_TERM +eoptions_to_cred_config(ErlNifEnv *env, + ERL_NIF_TERM eoptions, + QUIC_CREDENTIAL_CONFIG *CredConfig, + X509_STORE **trusted_store); #endif // QUICER_TLS_H_ diff --git a/src/quicer_listener.erl b/src/quicer_listener.erl index 4490b28c..048c3d98 100644 --- a/src/quicer_listener.erl +++ b/src/quicer_listener.erl @@ -147,6 +147,8 @@ handle_cast(_Request, State) -> | {noreply, NewState :: term(), Timeout :: timeout()} | {noreply, NewState :: term(), hibernate} | {stop, Reason :: normal | term(), NewState :: term()}. +handle_info({quic, listener_stopped, L}, #state{listener = L} = State) -> + {stop, normal, State}; handle_info(_Info, State) -> {noreply, State}. diff --git a/test/quicer_SUITE.erl b/test/quicer_SUITE.erl index 6ac8af0b..be8ca603 100644 --- a/test/quicer_SUITE.erl +++ b/test/quicer_SUITE.erl @@ -2911,7 +2911,9 @@ tc_peercert_server(Config) -> PeerCert = receive {SPid, peercert, Cert} -> - Cert + Cert; + {quic, transport_shutdown, Conn, _} = M -> + ct:fail("conn fail : ~p", [M]) end, OTPCert = public_key:pkix_decode_cert(PeerCert, otp), ct:pal("client cert is ~p", [OTPCert]), diff --git a/test/quicer_listener_SUITE.erl b/test/quicer_listener_SUITE.erl index ada3d3a5..120d1b39 100644 --- a/test/quicer_listener_SUITE.erl +++ b/test/quicer_listener_SUITE.erl @@ -370,6 +370,18 @@ tc_stop_start_listener(Config) -> ok = snabbkaffe:retry(100, 10, fun() -> ok = quicer:start_listener(L, Port, LConf) end), ok = quicer:close_listener(L). +tc_stop_start_listener_with_new_port(Config) -> + Port = select_port(), + LConf = default_listen_opts(Config), + {ok, L} = quicer:listen(Port, LConf), + ok = quicer:stop_listener(L), + Port2 = select_port(), + ok = snabbkaffe:retry(100, 10, fun() -> ok = quicer:start_listener(L, Port2, LConf) end), + {ok, Sock1} = gen_udp:open(Port), + ?assertMatch({error, eaddrinuse}, gen_udp:open(Port2)), + gen_udp:close(Sock1), + ok = quicer:close_listener(L). + tc_stop_close_listener(Config) -> Port = select_port(), {ok, L} = quicer:listen(Port, default_listen_opts(Config)),