From e0442a1063ea39463aaeaa1bc82a32982e9a27a3 Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 1 Sep 2023 10:56:49 +0200 Subject: [PATCH 01/12] feat(listener): new registration listen opt --- c_src/quicer_config.c | 5 ++-- c_src/quicer_config.h | 1 + c_src/quicer_ctx.c | 11 +++++++ c_src/quicer_ctx.h | 1 + c_src/quicer_listener.c | 54 ++++++++++++++++++++++++++++++---- c_src/quicer_nif.c | 2 +- include/quicer_types.hrl | 1 + test/quicer_listener_SUITE.erl | 45 +++++++++++++++++++++++++--- 8 files changed, 107 insertions(+), 13 deletions(-) diff --git a/c_src/quicer_config.c b/c_src/quicer_config.c index fde77459..239f614c 100644 --- a/c_src/quicer_config.c +++ b/c_src/quicer_config.c @@ -210,12 +210,13 @@ atom_cipher_suite(QUIC_CIPHER_SUITE suite) ERL_NIF_TERM ServerLoadConfiguration(ErlNifEnv *env, const ERL_NIF_TERM *option, + HQUIC Registration, HQUIC *Configuration, QUIC_CREDENTIAL_CONFIG *CredConfig) { QUIC_SETTINGS Settings = { 0 }; - if (!isRegistered) + if (!isRegistered && (Registration == GRegistration)) { return ATOM_REG_FAILED; } @@ -238,7 +239,7 @@ ServerLoadConfiguration(ErlNifEnv *env, // and settings. // QUIC_STATUS Status = QUIC_STATUS_SUCCESS; - if (QUIC_FAILED(Status = MsQuic->ConfigurationOpen(GRegistration, + if (QUIC_FAILED(Status = MsQuic->ConfigurationOpen(Registration, alpn_buffers, alpn_buffer_length, &Settings, diff --git a/c_src/quicer_config.h b/c_src/quicer_config.h index d92f1c7c..088d475d 100644 --- a/c_src/quicer_config.h +++ b/c_src/quicer_config.h @@ -66,6 +66,7 @@ QUIC_STATUS UpdateCredConfig(ErlNifEnv *env, ERL_NIF_TERM ServerLoadConfiguration(ErlNifEnv *env, const ERL_NIF_TERM *option, + HQUIC Registration, HQUIC *Configuration, QUIC_CREDENTIAL_CONFIG *Config); ERL_NIF_TERM ClientLoadConfiguration(ErlNifEnv *env, diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index f7f21c3a..25cd7b50 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -65,6 +65,7 @@ init_l_ctx() l_ctx->trusted_store = NULL; l_ctx->is_closed = TRUE; l_ctx->allow_insecure = FALSE; + l_ctx->r_ctx = NULL; return l_ctx; } @@ -80,6 +81,10 @@ deinit_l_ctx(QuicerListenerCTX *l_ctx) { destroy_config_ctx(l_ctx->config_resource); } + if (l_ctx->r_ctx && l_ctx->r_ctx->Registration != GRegistration) + { + enif_release_resource(l_ctx->r_ctx); + } enif_mutex_destroy(l_ctx->lock); enif_free_env(l_ctx->env); } @@ -90,6 +95,12 @@ destroy_l_ctx(QuicerListenerCTX *l_ctx) // @note, Destroy config asap as it holds rundown // ref count in registration destroy_config_ctx(l_ctx->config_resource); + + if (l_ctx->r_ctx) + { + enif_release_resource(l_ctx->r_ctx); + l_ctx->r_ctx = NULL; + } l_ctx->config_resource = NULL; enif_release_resource(l_ctx); } diff --git a/c_src/quicer_ctx.h b/c_src/quicer_ctx.h index e3b17ecb..e94ec316 100644 --- a/c_src/quicer_ctx.h +++ b/c_src/quicer_ctx.h @@ -51,6 +51,7 @@ typedef struct QuicerListenerCTX { // config_resource is allocated in 'init_l_ctx' QuicerConfigCTX *config_resource; + QuicerRegistrationCTX *r_ctx; HQUIC Listener; QUICER_ACCEPTOR_QUEUE *acceptor_queue; ErlNifPid listenerPid; diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 94419684..d7af8941 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -230,6 +230,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) ERL_NIF_TERM options = argv[1]; QUIC_ADDR Address = {}; int UdpPort = 0; + HQUIC Registration = NULL; if (!enif_is_map(env, options)) { @@ -264,6 +265,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) char cert_path[PATH_MAX + 1] = { 0 }; char key_path[PATH_MAX + 1] = { 0 }; ERL_NIF_TERM tmp_term; + QuicerRegistrationCTX *r_ctx = NULL; if (get_str_from_map(env, ATOM_CERTFILE, &options, cert_path, PATH_MAX + 1) <= 0 @@ -281,13 +283,39 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) return ERROR_TUPLE_2(ATOM_BADARG); } + // End Build CredConfig from options + + // Now build l_ctx QuicerListenerCTX *l_ctx = init_l_ctx(); + // Set owner for l_ctx if (!enif_self(env, &(l_ctx->listenerPid))) + { + destroy_l_ctx(l_ctx); + return ERROR_TUPLE_2(ATOM_BAD_PID); + } + + + // Get Reg for l_ctx + if (enif_get_map_value(env, options, ATOM_QUIC_REGISTRATION, &tmp_term)) + { + if (!enif_get_resource(env, tmp_term, ctx_reg_t, (void **)&r_ctx)) + { + destroy_l_ctx(l_ctx); + return ERROR_TUPLE_2(ATOM_BADARG); + } + enif_keep_resource(r_ctx); + Registration = r_ctx->Registration; + } + else { - return ERROR_TUPLE_2(ATOM_BAD_PID); + Registration = GRegistration; } + // NULL, if no registration in opts + l_ctx->r_ctx = r_ctx; + + // Get cert file for l_ctx ERL_NIF_TERM ecacertfile; if (enif_get_map_value(env, options, ATOM_CACERTFILE, &ecacertfile)) { @@ -365,8 +393,12 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } } - ERL_NIF_TERM estatus = ServerLoadConfiguration( - env, &options, &l_ctx->config_resource->Configuration, &CredConfig); + ERL_NIF_TERM estatus + = ServerLoadConfiguration(env, + &options, + Registration, + &l_ctx->config_resource->Configuration, + &CredConfig); // Cleanup CredConfig if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE == CredConfig.Type) @@ -394,10 +426,20 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) return ERROR_TUPLE_2(ATOM_BAD_MON); } - if (QUIC_FAILED( - Status = MsQuic->ListenerOpen( - GRegistration, ServerListenerCallback, l_ctx, &l_ctx->Listener))) + + // Now open listener + if (QUIC_FAILED(Status = MsQuic->ListenerOpen( + // Listener registration + Registration, + ServerListenerCallback, + l_ctx, + &l_ctx->Listener))) { + if (r_ctx) + { + // deref the resource + enif_release_resource(r_ctx); + } l_ctx->config_resource->Configuration = NULL; destroy_l_ctx(l_ctx); return ERROR_TUPLE_3(ATOM_LISTENER_OPEN_ERROR, ATOM_STATUS(Status)); diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index ec6712a0..c5bbc7ff 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -1121,7 +1121,7 @@ registration(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) return ERROR_TUPLE_2(ATOM_BADARG); } } - + // Open global registration if (QUIC_FAILED(status = MsQuic->RegistrationOpen(&GRegConfig, &GRegistration))) { diff --git a/include/quicer_types.hrl b/include/quicer_types.hrl index 8d42a2bb..ae76e8e4 100644 --- a/include/quicer_types.hrl +++ b/include/quicer_types.hrl @@ -90,6 +90,7 @@ , password => string() , sslkeylogfile => filelib:filename() , allow_insecure => boolean() + , quic_registration => reg_handle() , conn_acceptors => non_neg_integer() }. diff --git a/test/quicer_listener_SUITE.erl b/test/quicer_listener_SUITE.erl index e9e2576a..bb573e25 100644 --- a/test/quicer_listener_SUITE.erl +++ b/test/quicer_listener_SUITE.erl @@ -63,8 +63,12 @@ end_per_suite(_Config) -> %% Reason = term() %% @end %%-------------------------------------------------------------------- -init_per_group(_GroupName, Config) -> - Config. +init_per_group(global_reg, Config) -> + Config; +init_per_group(suite_reg, Config) -> + {ok, SReg} = quicer:new_registration(atom_to_list(?MODULE), + quic_execution_profile_max_throughput), + [{quic_registration, SReg} | Config]. %%-------------------------------------------------------------------- %% @spec end_per_group(GroupName, Config0) -> @@ -73,6 +77,8 @@ init_per_group(_GroupName, Config) -> %% Config0 = Config1 = [tuple()] %% @end %%-------------------------------------------------------------------- +end_per_group(suite_reg, Config) -> + quicer:shutdown_registration(proplists:get_value(quic_registration, Config)); end_per_group(_GroupName, _Config) -> ok. @@ -113,7 +119,10 @@ end_per_testcase(_TestCase, _Config) -> %% @end %%-------------------------------------------------------------------- groups() -> - []. + TCs = quicer_test_lib:all_tcs(?MODULE), + [ {global_reg, [], TCs} + , {suite_reg, [], TCs} + ]. %%-------------------------------------------------------------------- %% @spec all() -> GroupsAndTestCases | {skip,Reason} @@ -124,7 +133,9 @@ groups() -> %% @end %%-------------------------------------------------------------------- all() -> - quicer_test_lib:all_tcs(?MODULE). + [ {group, global_reg} + , {group, suite_reg} + ]. %%-------------------------------------------------------------------- %% @spec TestCase() -> Info @@ -190,6 +201,32 @@ tc_open_listener(Config) -> ok = gen_udp:close(P), ok. +tc_open_listener_with_inval_reg(Config) -> + Port = select_port(), + Config1 = proplists:delete(quic_registration, Config), + %% Given invalid registration + Reg = erlang:make_ref(), + %% When try to listen with the invalid registration + Res = quicer:listen(Port, default_listen_opts([{quic_registration, Reg} | Config1])), + %% Then it shall fail to listen and proper error is returned + ?assertEqual({error, quic_registration}, Res), + ok. + +tc_open_listener_with_new_reg(Config) -> + Port = select_port(), + %% Given New registration is created + {ok, Reg} = quicer:new_registration(atom_to_list(?MODULE), quic_execution_profile_max_throughput), + %% When Listener is created with the New Registration + {ok, L} = quicer:listen(Port, default_listen_opts([{quic_registration, Reg} | Config])), + {ok, {_, Port}} = quicer:sockname(L), + %% Then Listener is created successfully and port is occupied + {error, eaddrinuse} = gen_udp:open(Port), + ok = quicer:close_listener(L), + {ok, P} = gen_udp:open(Port), + ok = gen_udp:close(P), + ok = quicer:shutdown_registration(Reg), + ok. + tc_open_listener_with_cert_password(Config) -> Port = select_port(), DataDir = ?config(data_dir, Config), From 051a42cb5dff3f7fcc1e1de9a0d14afeab38928d Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 1 Sep 2023 12:02:56 +0200 Subject: [PATCH 02/12] refactor(listener): use parse_listen_on --- c_src/quicer_config.c | 28 +++++++++++++++++++++++----- c_src/quicer_listener.c | 39 ++++++++++++++------------------------- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/c_src/quicer_config.c b/c_src/quicer_config.c index 239f614c..a8197f34 100644 --- a/c_src/quicer_config.c +++ b/c_src/quicer_config.c @@ -1174,15 +1174,33 @@ bool parse_listen_on(ErlNifEnv *env, ERL_NIF_TERM elisten_on, QUIC_ADDR *Address) { char listen_on[INET6_ADDRSTRLEN + 6] = { 0 }; - if (enif_get_string( - env, elisten_on, listen_on, INET6_ADDRSTRLEN + 6, ERL_NIF_LATIN1) - > 0) + int UdpPort = 0; + + ErlNifTermType type = enif_term_type(env, elisten_on); + switch (type) { - if ((QuicAddr4FromString(listen_on, Address) - || QuicAddr6FromString(listen_on, Address))) + case ERL_NIF_TERM_TYPE_LIST: + if (enif_get_string( + env, elisten_on, listen_on, INET6_ADDRSTRLEN + 6, ERL_NIF_LATIN1) + > 0) { + if ((QuicAddr4FromString(listen_on, Address) + || QuicAddr6FromString(listen_on, Address))) + { + return TRUE; + } + } + break; + case ERL_NIF_TERM_TYPE_INTEGER: + if (enif_get_int(env, elisten_on, &UdpPort) && UdpPort >= 0) + { + QuicAddrSetFamily(Address, QUIC_ADDRESS_FAMILY_UNSPEC); + QuicAddrSetPort(Address, (uint16_t)UdpPort); return TRUE; } + break; + default: + break; } return FALSE; } diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index d7af8941..0391eab4 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -228,8 +228,8 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) ERL_NIF_TERM elisten_on = argv[0]; ERL_NIF_TERM options = argv[1]; + QUIC_ADDR Address = {}; - int UdpPort = 0; HQUIC Registration = NULL; if (!enif_is_map(env, options)) @@ -237,28 +237,13 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) return ERROR_TUPLE_2(ATOM_BADARG); } - char listen_on[INET6_ADDRSTRLEN + 6] = { 0 }; - if (enif_get_string( - env, elisten_on, listen_on, INET6_ADDRSTRLEN + 6, ERL_NIF_LATIN1) - > 0) - { - if (!(QuicAddr4FromString(listen_on, &Address) - || QuicAddr6FromString(listen_on, &Address))) - { - return ERROR_TUPLE_2(ATOM_BADARG); - } - } - else if (enif_get_int(env, elisten_on, &UdpPort) && UdpPort >= 0) - { - QuicAddrSetFamily(&Address, QUIC_ADDRESS_FAMILY_UNSPEC); - QuicAddrSetPort(&Address, (uint16_t)UdpPort); - } - else + // Check ListenOn string + if (!parse_listen_on(env, elisten_on, &Address)) { return ERROR_TUPLE_2(ATOM_BADARG); } - // Build CredConfig + // Start Build CredConfig from options QUIC_CREDENTIAL_CONFIG CredConfig; CxPlatZeroMemory(&CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); char password[256] = { 0 }; @@ -290,11 +275,10 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // Set owner for l_ctx if (!enif_self(env, &(l_ctx->listenerPid))) - { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_BAD_PID); - } - + { + destroy_l_ctx(l_ctx); + return ERROR_TUPLE_2(ATOM_BAD_PID); + } // Get Reg for l_ctx if (enif_get_map_value(env, options, ATOM_QUIC_REGISTRATION, &tmp_term)) @@ -346,6 +330,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } } + // Get password for Server CertFile if (enif_get_map_value(env, options, ATOM_PASSWORD, &tmp_term)) { if (get_str_from_map(env, ATOM_PASSWORD, &options, password, 256) <= 0) @@ -376,6 +361,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) CredConfig.Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE; } + // Get Verify for CredConfig bool Verify = load_verify(env, &options, false); if (!Verify) @@ -393,6 +379,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } } + // Now load server config ERL_NIF_TERM estatus = ServerLoadConfiguration(env, &options, @@ -418,6 +405,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } // mon will be removed when triggered or when l_ctx is dealloc. + // this is wrong scope ErlNifMonitor mon; if (0 != enif_monitor_process(env, l_ctx, &l_ctx->listenerPid, &mon)) @@ -426,7 +414,6 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) return ERROR_TUPLE_2(ATOM_BAD_MON); } - // Now open listener if (QUIC_FAILED(Status = MsQuic->ListenerOpen( // Listener registration @@ -440,6 +427,8 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // deref the resource enif_release_resource(r_ctx); } + + // Server Configuration should be destroyed l_ctx->config_resource->Configuration = NULL; destroy_l_ctx(l_ctx); return ERROR_TUPLE_3(ATOM_LISTENER_OPEN_ERROR, ATOM_STATUS(Status)); From 0237e9c52b0e158b44be8b546e6c0b1f887cdf6e Mon Sep 17 00:00:00 2001 From: William Yang Date: Sat, 2 Sep 2023 18:21:50 +0200 Subject: [PATCH 03/12] refactor: split big nif listen, part 1 --- c_src/quicer_config.c | 184 +++++++++++++++++++++++++++++++++ c_src/quicer_config.h | 14 +++ c_src/quicer_listener.c | 158 +++++++++------------------- src/quicer.erl | 4 + test/quicer_SUITE.erl | 10 +- test/quicer_listener_SUITE.erl | 9 +- 6 files changed, 262 insertions(+), 117 deletions(-) diff --git a/c_src/quicer_config.c b/c_src/quicer_config.c index a8197f34..e0a19375 100644 --- a/c_src/quicer_config.c +++ b/c_src/quicer_config.c @@ -2433,6 +2433,7 @@ set_config_opt(ErlNifEnv *env, return res; } +// @deprecated int get_str_from_map(ErlNifEnv *env, ERL_NIF_TERM key, @@ -2460,6 +2461,46 @@ get_str_from_map(ErlNifEnv *env, return enif_get_string(env, tmp_term, buff, tmp_len + 1, ERL_NIF_LATIN1); } +char * +str_from_map(ErlNifEnv *env, + ERL_NIF_TERM key, + const ERL_NIF_TERM *map, + unsigned int max_len) +{ + unsigned int len = 0; + ERL_NIF_TERM tmp_term; + + if (!enif_get_map_value(env, *map, key, &tmp_term)) + { + goto exit; + } + + if (ERL_NIF_TERM_TYPE_LIST != enif_term_type(env, tmp_term)) + { + goto exit; + } + + if (!enif_get_list_length(env, tmp_term, &len)) + { + goto exit; + } + + if (len == 0 || max_len < len) + { + goto exit; + } + + char *str_buffer = (char *)malloc(len + 1); + + if (enif_get_string(env, tmp_term, str_buffer, len + 1, ERL_NIF_LATIN1)) + { + return str_buffer; + } + +exit: + return NULL; +} + BOOLEAN build_trustedstore(const char *cacertfile, X509_STORE **trusted_store) { @@ -2493,3 +2534,146 @@ build_trustedstore(const char *cacertfile, X509_STORE **trusted_store) *trusted_store = store; return TRUE; } + +/* +** Build QUIC_CREDENTIAL_CONFIG from options, certfile, keyfile and password +*/ +BOOLEAN +parse_cert_options(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig) +{ + char *password = NULL; + char *certfile = NULL; + char *keyfile = NULL; + ERL_NIF_TERM tmp_term; + + if (!CredConfig) + { + return FALSE; + } + CxPlatZeroMemory(CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); + + if (!(certfile = str_from_map(env, ATOM_CERTFILE, &options, PATH_MAX + 1))) + { + return FALSE; + } + if (!(keyfile = str_from_map(env, ATOM_KEYFILE, &options, PATH_MAX + 1))) + { + return FALSE; + } + + // Get password for Server CertFile + if (enif_get_map_value(env, options, ATOM_PASSWORD, &tmp_term)) + { + if (!(password = str_from_map(env, ATOM_PASSWORD, &options, 256))) + { + return FALSE; + } + + QUIC_CERTIFICATE_FILE_PROTECTED *CertFile + = (QUIC_CERTIFICATE_FILE_PROTECTED *)CXPLAT_ALLOC_NONPAGED( + sizeof(QUIC_CERTIFICATE_FILE_PROTECTED), + QUICER_CERTIFICATE_FILE); + + if (!CertFile) + { + return FALSE; + } + CertFile->CertificateFile = certfile; + CertFile->PrivateKeyFile = keyfile; + CertFile->PrivateKeyPassword = password; + CredConfig->CertificateFileProtected = CertFile; + CredConfig->Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED; + } + else + { + QUIC_CERTIFICATE_FILE *CertFile + = (QUIC_CERTIFICATE_FILE *)CXPLAT_ALLOC_NONPAGED( + sizeof(QUIC_CERTIFICATE_FILE), QUICER_CERTIFICATE_FILE); + if (!CertFile) + { + return FALSE; + } + CertFile->CertificateFile = certfile; + CertFile->PrivateKeyFile = keyfile; + CredConfig->CertificateFile = CertFile; + CredConfig->Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE; + } + + return TRUE; +} + +/* + * Parse verify option for listener (server) + * verify : boolean() | undefined + */ +BOOLEAN +parse_verify_options_server(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig) +{ + + BOOLEAN verify = load_verify(env, &options, FALSE); + + if (!verify) + { + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; + } + else + { + // Server flag: + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_REQUIRE_CLIENT_AUTHENTICATION; + } + return TRUE; +} + +/* +** Parse optional cacertfile option +*/ +BOOLEAN +parse_cacertfile_option(ErlNifEnv *env, + ERL_NIF_TERM options, + char **cacertfile) +{ + ERL_NIF_TERM ecacertfile; + char *tmp = NULL; + + if (!enif_is_map(env, options)) + { + return FALSE; + } + + if (enif_get_map_value(env, options, ATOM_CACERTFILE, &ecacertfile)) + { + tmp = str_from_map(env, ATOM_CACERTFILE, &options, PATH_MAX + 1); + if (!tmp) + { + return FALSE; + } + } + *cacertfile = tmp; + return TRUE; +} + +/* + * parse optional quic_registration, and store it in r_ctx + * return TRUE if quic_registration is present and valid or not present + * */ +BOOLEAN +parse_registration(ErlNifEnv *env, + ERL_NIF_TERM options, + QuicerRegistrationCTX **r_ctx) +{ + ERL_NIF_TERM tmp_term; + assert(*r_ctx == NULL); + if (enif_get_map_value(env, options, ATOM_QUIC_REGISTRATION, &tmp_term)) + { + if (!enif_get_resource(env, tmp_term, ctx_reg_t, (void **)r_ctx)) + { + return FALSE; + } + } + + return TRUE; +} diff --git a/c_src/quicer_config.h b/c_src/quicer_config.h index 088d475d..2a8eff00 100644 --- a/c_src/quicer_config.h +++ b/c_src/quicer_config.h @@ -121,4 +121,18 @@ ERL_NIF_TERM set_connection_opt(ErlNifEnv *env, BOOLEAN build_trustedstore(const char *cacertfile, X509_STORE **trusted_store); +BOOLEAN parse_cert_options(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig); + +BOOLEAN +parse_verify_options_server(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig); + +BOOLEAN +parse_cacertfile_option(ErlNifEnv *env, + ERL_NIF_TERM options, + char **cacertfile); + #endif // __QUICER_CONFIG_H_ diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 0391eab4..e4273b44 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -21,6 +21,10 @@ limitations under the License. #include #include +BOOLEAN parse_registration(ErlNifEnv *env, + ERL_NIF_TERM options, + QuicerRegistrationCTX **r_ctx); + QUIC_STATUS ServerListenerCallback(__unused_parm__ HQUIC Listener, void *Context, @@ -231,6 +235,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) QUIC_ADDR Address = {}; HQUIC Registration = NULL; + char *cacertfile = NULL; if (!enif_is_map(env, options)) { @@ -245,138 +250,74 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) // Start Build CredConfig from options QUIC_CREDENTIAL_CONFIG CredConfig; - CxPlatZeroMemory(&CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); - char password[256] = { 0 }; - char cert_path[PATH_MAX + 1] = { 0 }; - char key_path[PATH_MAX + 1] = { 0 }; - ERL_NIF_TERM tmp_term; - QuicerRegistrationCTX *r_ctx = NULL; - - if (get_str_from_map(env, ATOM_CERTFILE, &options, cert_path, PATH_MAX + 1) - <= 0 - && get_str_from_map(env, ATOM_CERT, &options, cert_path, PATH_MAX + 1) - <= 0) + // CxPlatZeroMemory(&CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); + CredConfig.Flags = QUIC_CREDENTIAL_FLAG_NONE; + + if (!parse_cert_options(env, options, &CredConfig)) { - return ERROR_TUPLE_2(ATOM_BADARG); + return ERROR_TUPLE_2(ATOM_QUIC_TLS); } - if (get_str_from_map(env, ATOM_KEYFILE, &options, key_path, PATH_MAX + 1) - <= 0 - && get_str_from_map(env, ATOM_KEY, &options, key_path, PATH_MAX + 1) - <= 0) + if (!parse_verify_options_server(env, options, &CredConfig)) { - return ERROR_TUPLE_2(ATOM_BADARG); + return ERROR_TUPLE_2(ATOM_VERIFY); } - // End Build CredConfig from options + if (!parse_cacertfile_option(env, options, &cacertfile)) + { + // TLS opt error not file content error + return ERROR_TUPLE_2(ATOM_CACERTFILE); + } // Now build l_ctx QuicerListenerCTX *l_ctx = init_l_ctx(); - // Set owner for l_ctx - if (!enif_self(env, &(l_ctx->listenerPid))) + if (!l_ctx) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_BAD_PID); + return ERROR_TUPLE_2(ATOM_ERROR_NOT_ENOUGH_MEMORY); } - // Get Reg for l_ctx - if (enif_get_map_value(env, options, ATOM_QUIC_REGISTRATION, &tmp_term)) + if (cacertfile) { - if (!enif_get_resource(env, tmp_term, ctx_reg_t, (void **)&r_ctx)) + l_ctx->cacertfile = 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; + CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; + + if (!build_trustedstore(l_ctx->cacertfile, &l_ctx->trusted_store)) { destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_BADARG); + return ERROR_TUPLE_2(ATOM_CERT_ERROR); } - enif_keep_resource(r_ctx); - Registration = r_ctx->Registration; - } - else - { - Registration = GRegistration; } - // NULL, if no registration in opts - l_ctx->r_ctx = r_ctx; - - // Get cert file for l_ctx - ERL_NIF_TERM ecacertfile; - if (enif_get_map_value(env, options, ATOM_CACERTFILE, &ecacertfile)) + // Set owner for l_ctx + if (!enif_self(env, &(l_ctx->listenerPid))) { - unsigned len; - if (enif_get_list_length(env, ecacertfile, &len)) - { - l_ctx->cacertfile - = (char *)CXPLAT_ALLOC_NONPAGED(len + 1, QUICER_CACERTFILE); - if (!(enif_get_string(env, - ecacertfile, - l_ctx->cacertfile, - len + 1, - ERL_NIF_LATIN1) - > 0 - && build_trustedstore(l_ctx->cacertfile, - &l_ctx->trusted_store))) - { - CXPLAT_FREE(l_ctx->cacertfile, QUICER_CACERTFILE); - l_ctx->cacertfile = NULL; - enif_release_resource(l_ctx); - return ERROR_TUPLE_2(ATOM_BADARG); - } - } - else - { - enif_release_resource(l_ctx); - return ERROR_TUPLE_2(ATOM_BADARG); - } + destroy_l_ctx(l_ctx); + return ERROR_TUPLE_2(ATOM_BAD_PID); } - // Get password for Server CertFile - if (enif_get_map_value(env, options, ATOM_PASSWORD, &tmp_term)) + // Get Reg for l_ctx, quic_registration is optional + if (!parse_registration(env, options, &l_ctx->r_ctx)) { - if (get_str_from_map(env, ATOM_PASSWORD, &options, password, 256) <= 0) - { - enif_release_resource(l_ctx); - return ERROR_TUPLE_2(ATOM_BADARG); - } - - QUIC_CERTIFICATE_FILE_PROTECTED *CertFile - = (QUIC_CERTIFICATE_FILE_PROTECTED *)CXPLAT_ALLOC_NONPAGED( - sizeof(QUIC_CERTIFICATE_FILE_PROTECTED), - QUICER_CERTIFICATE_FILE); - - CertFile->CertificateFile = cert_path; - CertFile->PrivateKeyFile = key_path; - CertFile->PrivateKeyPassword = password; - CredConfig.CertificateFileProtected = CertFile; - CredConfig.Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED; + destroy_l_ctx(l_ctx); + return ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); } - else + + if (l_ctx->r_ctx) { - QUIC_CERTIFICATE_FILE *CertFile - = (QUIC_CERTIFICATE_FILE *)CXPLAT_ALLOC_NONPAGED( - sizeof(QUIC_CERTIFICATE_FILE), QUICER_CERTIFICATE_FILE); - CertFile->CertificateFile = cert_path; - CertFile->PrivateKeyFile = key_path; - CredConfig.CertificateFile = CertFile; - CredConfig.Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE; + // quic_registration is set + enif_keep_resource(l_ctx->r_ctx); + Registration = l_ctx->r_ctx->Registration; } - - // Get Verify for CredConfig - bool Verify = load_verify(env, &options, false); - - if (!Verify) - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; else { - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_REQUIRE_CLIENT_AUTHENTICATION; - if (l_ctx->cacertfile) - { - // We do our own certificate verification agains the certificates - // in cacertfile - CredConfig.Flags - |= QUIC_CREDENTIAL_FLAG_INDICATE_CERTIFICATE_RECEIVED; - CredConfig.Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; - } + // quic_registration is not set, use global registration + // msquic should reject if global registration is NULL (closed) + Registration = GRegistration; } // Now load server config @@ -388,6 +329,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) &CredConfig); // Cleanup CredConfig + // @todo clean more dynamically allocated memory such as cert paths if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE == CredConfig.Type) { CxPlatFree(CredConfig.CertificateFile, QUICER_CERTIFICATE_FILE); @@ -422,12 +364,6 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) l_ctx, &l_ctx->Listener))) { - if (r_ctx) - { - // deref the resource - enif_release_resource(r_ctx); - } - // Server Configuration should be destroyed l_ctx->config_resource->Configuration = NULL; destroy_l_ctx(l_ctx); @@ -438,7 +374,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) unsigned alpn_buffer_length = 0; QUIC_BUFFER alpn_buffers[MAX_ALPN]; - // Allow insecure + // Allow insecure, default is false ERL_NIF_TERM eisInsecure; if (enif_get_map_value(env, options, ATOM_ALLOW_INSECURE, &eisInsecure) && IS_SAME_TERM(eisInsecure, ATOM_TRUE)) diff --git a/src/quicer.erl b/src/quicer.erl index 38b7b128..e5802b46 100644 --- a/src/quicer.erl +++ b/src/quicer.erl @@ -250,6 +250,10 @@ terminate_listener(AppName) when is_atom(AppName)-> %% @end -spec listen(listen_on(), listen_opts()) -> {ok, listener_handle()} | + {error, quic_tls} | %% bad tls related opts, cacertfile, certfile, keyfile, password... + {error, cacertfile} | %% bad cacert file + {error, quic_registration} | %% wrong registration opt + {error, badarg} | {error, listener_open_error, atom_reason()} | {error, listener_start_error, atom_reason()}. listen(ListenOn, Opts) when is_list(Opts) -> diff --git a/test/quicer_SUITE.erl b/test/quicer_SUITE.erl index 0749d995..c9c2e02e 100644 --- a/test/quicer_SUITE.erl +++ b/test/quicer_SUITE.erl @@ -3208,8 +3208,14 @@ simple_conn_server_client_cert(Owner, Config, Port) -> {ok, L} = quicer:listen(Port, default_listen_opts_client_cert(Config)), Owner ! listener_ready, {ok, Conn} = quicer:accept(L, [], 1000), - {ok, Conn} = quicer:handshake(Conn), - simple_conn_server_client_cert_loop(L, Conn, Owner). + case quicer:handshake(Conn) of + {ok, Conn} -> + simple_conn_server_client_cert_loop(L, Conn, Owner); + {error, closed} -> + receive done -> + quicer:close_listener(L) + end + end. simple_conn_server_client_cert_loop(L, Conn, Owner) -> receive diff --git a/test/quicer_listener_SUITE.erl b/test/quicer_listener_SUITE.erl index bb573e25..912a7f3a 100644 --- a/test/quicer_listener_SUITE.erl +++ b/test/quicer_listener_SUITE.erl @@ -173,20 +173,21 @@ tc_open_listener_inval_parm(Config) -> tc_open_listener_inval_cacertfile_1(Config) -> Port = select_port(), - ?assertEqual({error, badarg}, + ?assertEqual({error, cacertfile}, quicer:listen(Port, [ {cacertfile, atom} | default_listen_opts(Config)])), ok. tc_open_listener_inval_cacertfile_2(Config) -> Port = select_port(), - {error, badarg} = quicer:listen(Port, [ {cacertfile, [1,2,3,4]} - | default_listen_opts(Config)]), + ?assertEqual({error, cacertfile}, + quicer:listen(Port, [ {cacertfile, <<"1,2,3,4">>} + | default_listen_opts(Config)])), ok. tc_open_listener_inval_cacertfile_3(Config) -> Port = select_port(), - ?assertEqual({error, badarg}, + ?assertEqual({error, cacertfile}, quicer:listen(Port, [ {cacertfile, [-1]} | default_listen_opts(Config)])), ok. From 03bda0aa6ee69d658e769c146ab8364fea2b9e36 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sat, 2 Sep 2023 18:53:15 +0200 Subject: [PATCH 04/12] fix: lux test --- test/example/qt.erl | 4 ++-- test/example/rev.erl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/example/qt.erl b/test/example/qt.erl index fa0716a8..3b5c03fd 100644 --- a/test/example/qt.erl +++ b/test/example/qt.erl @@ -44,8 +44,8 @@ s() -> application:ensure_all_started(quicer), Port = 4567, - LOptions = #{cert => "./server.pem", - key => "./server.key", + LOptions = #{certfile => "./server.pem", + keyfile => "./server.key", verify => none, handshake_idle_timeout_ms => 3 * ?INTERVAL, keep_alive_interval_ms => ?INTERVAL, diff --git a/test/example/rev.erl b/test/example/rev.erl index 66d4f444..4eb53a02 100644 --- a/test/example/rev.erl +++ b/test/example/rev.erl @@ -98,8 +98,8 @@ longrun() -> top_site() -> Port = 4567, - LOptions = #{cert => "./server.pem", - key => "./server.key", + LOptions = #{certfile => "./server.pem", + keyfile => "./server.key", verify => none, handshake_idle_timeout_ms => 3 * ?INTERVAL, keep_alive_interval_ms => ?INTERVAL, From 572545e5bcb4da7ca53b858c2d34beebda285c72 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sat, 2 Sep 2023 18:55:55 +0200 Subject: [PATCH 05/12] refactor: move tls funs to quicer_tls --- CMakeLists.txt | 2 + c_src/quicer_config.c | 155 ---------------------------------- c_src/quicer_config.h | 20 +---- c_src/quicer_connection.c | 1 + c_src/quicer_listener.c | 1 + c_src/quicer_tls.c | 173 ++++++++++++++++++++++++++++++++++++++ c_src/quicer_tls.h | 38 +++++++++ 7 files changed, 219 insertions(+), 171 deletions(-) create mode 100644 c_src/quicer_tls.c create mode 100644 c_src/quicer_tls.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 9beaeaf7..4a881d99 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -68,6 +68,8 @@ set(SOURCES c_src/quicer_ctx.h c_src/quicer_listener.c c_src/quicer_listener.h + c_src/quicer_tls.c + c_src/quicer_tls.h c_src/quicer_connection.c c_src/quicer_connection.h c_src/quicer_stream.c diff --git a/c_src/quicer_config.c b/c_src/quicer_config.c index e0a19375..08966ea9 100644 --- a/c_src/quicer_config.c +++ b/c_src/quicer_config.c @@ -2501,161 +2501,6 @@ str_from_map(ErlNifEnv *env, return NULL; } -BOOLEAN -build_trustedstore(const char *cacertfile, X509_STORE **trusted_store) -{ - X509_STORE *store = NULL; - X509_LOOKUP *lookup = NULL; - - if (cacertfile == NULL) - { - return FALSE; - } - - store = X509_STORE_new(); - if (store == NULL) - { - return FALSE; - } - - lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()); - if (lookup == NULL) - { - X509_STORE_free(store); - return FALSE; - } - - if (!X509_LOOKUP_load_file(lookup, cacertfile, X509_FILETYPE_PEM)) - { - X509_STORE_free(store); - return FALSE; - } - - *trusted_store = store; - return TRUE; -} - -/* -** Build QUIC_CREDENTIAL_CONFIG from options, certfile, keyfile and password -*/ -BOOLEAN -parse_cert_options(ErlNifEnv *env, - ERL_NIF_TERM options, - QUIC_CREDENTIAL_CONFIG *CredConfig) -{ - char *password = NULL; - char *certfile = NULL; - char *keyfile = NULL; - ERL_NIF_TERM tmp_term; - - if (!CredConfig) - { - return FALSE; - } - CxPlatZeroMemory(CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); - - if (!(certfile = str_from_map(env, ATOM_CERTFILE, &options, PATH_MAX + 1))) - { - return FALSE; - } - if (!(keyfile = str_from_map(env, ATOM_KEYFILE, &options, PATH_MAX + 1))) - { - return FALSE; - } - - // Get password for Server CertFile - if (enif_get_map_value(env, options, ATOM_PASSWORD, &tmp_term)) - { - if (!(password = str_from_map(env, ATOM_PASSWORD, &options, 256))) - { - return FALSE; - } - - QUIC_CERTIFICATE_FILE_PROTECTED *CertFile - = (QUIC_CERTIFICATE_FILE_PROTECTED *)CXPLAT_ALLOC_NONPAGED( - sizeof(QUIC_CERTIFICATE_FILE_PROTECTED), - QUICER_CERTIFICATE_FILE); - - if (!CertFile) - { - return FALSE; - } - CertFile->CertificateFile = certfile; - CertFile->PrivateKeyFile = keyfile; - CertFile->PrivateKeyPassword = password; - CredConfig->CertificateFileProtected = CertFile; - CredConfig->Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED; - } - else - { - QUIC_CERTIFICATE_FILE *CertFile - = (QUIC_CERTIFICATE_FILE *)CXPLAT_ALLOC_NONPAGED( - sizeof(QUIC_CERTIFICATE_FILE), QUICER_CERTIFICATE_FILE); - if (!CertFile) - { - return FALSE; - } - CertFile->CertificateFile = certfile; - CertFile->PrivateKeyFile = keyfile; - CredConfig->CertificateFile = CertFile; - CredConfig->Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE; - } - - return TRUE; -} - -/* - * Parse verify option for listener (server) - * verify : boolean() | undefined - */ -BOOLEAN -parse_verify_options_server(ErlNifEnv *env, - ERL_NIF_TERM options, - QUIC_CREDENTIAL_CONFIG *CredConfig) -{ - - BOOLEAN verify = load_verify(env, &options, FALSE); - - if (!verify) - { - CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; - } - else - { - // Server flag: - CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_REQUIRE_CLIENT_AUTHENTICATION; - } - return TRUE; -} - -/* -** Parse optional cacertfile option -*/ -BOOLEAN -parse_cacertfile_option(ErlNifEnv *env, - ERL_NIF_TERM options, - char **cacertfile) -{ - ERL_NIF_TERM ecacertfile; - char *tmp = NULL; - - if (!enif_is_map(env, options)) - { - return FALSE; - } - - if (enif_get_map_value(env, options, ATOM_CACERTFILE, &ecacertfile)) - { - tmp = str_from_map(env, ATOM_CACERTFILE, &options, PATH_MAX + 1); - if (!tmp) - { - return FALSE; - } - } - *cacertfile = tmp; - return TRUE; -} - /* * parse optional quic_registration, and store it in r_ctx * return TRUE if quic_registration is present and valid or not present diff --git a/c_src/quicer_config.h b/c_src/quicer_config.h index 2a8eff00..6d509d4d 100644 --- a/c_src/quicer_config.h +++ b/c_src/quicer_config.h @@ -102,6 +102,10 @@ int get_str_from_map(ErlNifEnv *env, const ERL_NIF_TERM *map, char *buff, unsigned max_len); +char *str_from_map(ErlNifEnv *env, + ERL_NIF_TERM key, + const ERL_NIF_TERM *map, + unsigned int max_len); ERL_NIF_TERM getopt3(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); ERL_NIF_TERM setopt4(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); @@ -119,20 +123,4 @@ ERL_NIF_TERM set_connection_opt(ErlNifEnv *env, ERL_NIF_TERM optval, ERL_NIF_TERM elevel); -BOOLEAN build_trustedstore(const char *cacertfile, X509_STORE **trusted_store); - -BOOLEAN parse_cert_options(ErlNifEnv *env, - ERL_NIF_TERM options, - QUIC_CREDENTIAL_CONFIG *CredConfig); - -BOOLEAN -parse_verify_options_server(ErlNifEnv *env, - ERL_NIF_TERM options, - QUIC_CREDENTIAL_CONFIG *CredConfig); - -BOOLEAN -parse_cacertfile_option(ErlNifEnv *env, - ERL_NIF_TERM options, - char **cacertfile); - #endif // __QUICER_CONFIG_H_ diff --git a/c_src/quicer_connection.c b/c_src/quicer_connection.c index 0a8d12c9..ca4d05e3 100644 --- a/c_src/quicer_connection.c +++ b/c_src/quicer_connection.c @@ -15,6 +15,7 @@ limitations under the License. -------------------------------------------------------------------*/ #include "quicer_connection.h" #include "quicer_ctx.h" +#include "quicer_tls.h" #include #include #include diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index e4273b44..12ede520 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -16,6 +16,7 @@ limitations under the License. #include "quicer_listener.h" #include "quicer_config.h" +#include "quicer_tls.h" #include "quicer_tp.h" #include #include diff --git a/c_src/quicer_tls.c b/c_src/quicer_tls.c new file mode 100644 index 00000000..73341fb9 --- /dev/null +++ b/c_src/quicer_tls.c @@ -0,0 +1,173 @@ +/*-------------------------------------------------------------------- +Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. + +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 + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +-------------------------------------------------------------------*/ +#include "quicer_tls.h" + +/* +** Build QUIC_CREDENTIAL_CONFIG from options, certfile, keyfile and password +*/ +BOOLEAN +parse_cert_options(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig) +{ + char *password = NULL; + char *certfile = NULL; + char *keyfile = NULL; + ERL_NIF_TERM tmp_term; + + if (!CredConfig) + { + return FALSE; + } + CxPlatZeroMemory(CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); + + if (!(certfile + = str_from_map(env, ATOM_CERTFILE, &options, NULL, PATH_MAX + 1))) + { + return FALSE; + } + if (!(keyfile + = str_from_map(env, ATOM_KEYFILE, &options, NULL, PATH_MAX + 1))) + { + return FALSE; + } + + // Get password for Server CertFile + if (enif_get_map_value(env, options, ATOM_PASSWORD, &tmp_term)) + { + if (!(password = str_from_map(env, ATOM_PASSWORD, &options, NULL, 256))) + { + return FALSE; + } + + QUIC_CERTIFICATE_FILE_PROTECTED *CertFile + = (QUIC_CERTIFICATE_FILE_PROTECTED *)CXPLAT_ALLOC_NONPAGED( + sizeof(QUIC_CERTIFICATE_FILE_PROTECTED), + QUICER_CERTIFICATE_FILE); + + if (!CertFile) + { + return FALSE; + } + CertFile->CertificateFile = certfile; + CertFile->PrivateKeyFile = keyfile; + CertFile->PrivateKeyPassword = password; + CredConfig->CertificateFileProtected = CertFile; + CredConfig->Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED; + } + else + { + QUIC_CERTIFICATE_FILE *CertFile + = (QUIC_CERTIFICATE_FILE *)CXPLAT_ALLOC_NONPAGED( + sizeof(QUIC_CERTIFICATE_FILE), QUICER_CERTIFICATE_FILE); + if (!CertFile) + { + return FALSE; + } + CertFile->CertificateFile = certfile; + CertFile->PrivateKeyFile = keyfile; + CredConfig->CertificateFile = CertFile; + CredConfig->Type = QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE; + } + + return TRUE; +} + +/* + * Parse verify option for listener (server) + * verify : boolean() | undefined + */ +BOOLEAN +parse_verify_options_server(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig) +{ + + BOOLEAN verify = load_verify(env, &options, FALSE); + + if (!verify) + { + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_NO_CERTIFICATE_VALIDATION; + } + else + { + // Server flag: + CredConfig->Flags |= QUIC_CREDENTIAL_FLAG_REQUIRE_CLIENT_AUTHENTICATION; + } + return TRUE; +} + +/* +** Parse optional cacertfile option +*/ +BOOLEAN +parse_cacertfile_option(ErlNifEnv *env, + ERL_NIF_TERM options, + char **cacertfile) +{ + ERL_NIF_TERM ecacertfile; + char *tmp = NULL; + + if (!enif_is_map(env, options)) + { + return FALSE; + } + + if (enif_get_map_value(env, options, ATOM_CACERTFILE, &ecacertfile)) + { + tmp = str_from_map(env, ATOM_CACERTFILE, &options, NULL, PATH_MAX + 1); + if (!tmp) + { + return FALSE; + } + } + *cacertfile = tmp; + return TRUE; +} + +BOOLEAN +build_trustedstore(const char *cacertfile, X509_STORE **trusted_store) +{ + X509_STORE *store = NULL; + X509_LOOKUP *lookup = NULL; + + if (cacertfile == NULL) + { + return FALSE; + } + + store = X509_STORE_new(); + if (store == NULL) + { + return FALSE; + } + + lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()); + if (lookup == NULL) + { + X509_STORE_free(store); + return FALSE; + } + + if (!X509_LOOKUP_load_file(lookup, cacertfile, X509_FILETYPE_PEM)) + { + X509_STORE_free(store); + return FALSE; + } + + *trusted_store = store; + return TRUE; +} diff --git a/c_src/quicer_tls.h b/c_src/quicer_tls.h new file mode 100644 index 00000000..4884423a --- /dev/null +++ b/c_src/quicer_tls.h @@ -0,0 +1,38 @@ +/*-------------------------------------------------------------------- +Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. + +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 + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +-------------------------------------------------------------------*/ +#ifndef QUICER_TLS_H_ +#define QUICER_TLS_H_ + +#include "quicer_nif.h" + +BOOLEAN parse_cert_options(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig); + +BOOLEAN +parse_verify_options_server(ErlNifEnv *env, + ERL_NIF_TERM options, + QUIC_CREDENTIAL_CONFIG *CredConfig); + +BOOLEAN +parse_cacertfile_option(ErlNifEnv *env, + ERL_NIF_TERM options, + char **cacertfile); + +BOOLEAN +build_trustedstore(const char *cacertfile, X509_STORE **trusted_store); + +#endif // QUICER_TLS_H_ From 9175d1a9ba31cfaf9c887d55b970ae87e3d588e4 Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 3 Sep 2023 08:21:44 +0200 Subject: [PATCH 06/12] fix: free filename in CredConfig after load --- c_src/quicer_config.c | 26 +++++++++++++++++++++----- c_src/quicer_config.h | 1 + c_src/quicer_listener.c | 6 +++++- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/c_src/quicer_config.c b/c_src/quicer_config.c index 08966ea9..05e2961e 100644 --- a/c_src/quicer_config.c +++ b/c_src/quicer_config.c @@ -2461,14 +2461,23 @@ get_str_from_map(ErlNifEnv *env, return enif_get_string(env, tmp_term, buff, tmp_len + 1, ERL_NIF_LATIN1); } +/* +** Fill str_buffer with string value of key in map. +** In case str_buffer is NULL, then new memory will be allocated, +** and caller should free it after use. +** +** Returns NULL on error. +*/ char * str_from_map(ErlNifEnv *env, ERL_NIF_TERM key, const ERL_NIF_TERM *map, + char *str_buffer, unsigned int max_len) { unsigned int len = 0; ERL_NIF_TERM tmp_term; + BOOLEAN is_alloc = str_buffer == NULL; if (!enif_get_map_value(env, *map, key, &tmp_term)) { @@ -2480,22 +2489,29 @@ str_from_map(ErlNifEnv *env, goto exit; } - if (!enif_get_list_length(env, tmp_term, &len)) + if ((!str_buffer && !enif_get_list_length(env, tmp_term, &len)) + || len > max_len) { goto exit; } - - if (len == 0 || max_len < len) + else { - goto exit; + len = max_len; } - char *str_buffer = (char *)malloc(len + 1); + if (is_alloc) + { + str_buffer = (char *)malloc(len + 1); + } if (enif_get_string(env, tmp_term, str_buffer, len + 1, ERL_NIF_LATIN1)) { return str_buffer; } + else if (is_alloc) + { + free(str_buffer); + } exit: return NULL; diff --git a/c_src/quicer_config.h b/c_src/quicer_config.h index 6d509d4d..8026e1b1 100644 --- a/c_src/quicer_config.h +++ b/c_src/quicer_config.h @@ -105,6 +105,7 @@ int get_str_from_map(ErlNifEnv *env, char *str_from_map(ErlNifEnv *env, ERL_NIF_TERM key, const ERL_NIF_TERM *map, + char *string_buffer, unsigned int max_len); ERL_NIF_TERM getopt3(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]); diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 12ede520..0a62b44b 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -330,13 +330,17 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) &CredConfig); // Cleanup CredConfig - // @todo clean more dynamically allocated memory such as cert paths if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE == CredConfig.Type) { + free((char *)CredConfig.CertificateFile->CertificateFile); + free((char *)CredConfig.CertificateFile->PrivateKeyFile); CxPlatFree(CredConfig.CertificateFile, QUICER_CERTIFICATE_FILE); } else if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED == CredConfig.Type) { + free((char *)CredConfig.CertificateFileProtected->CertificateFile); + free((char *)CredConfig.CertificateFileProtected->PrivateKeyFile); + free((char *)CredConfig.CertificateFileProtected->PrivateKeyPassword); CxPlatFree(CredConfig.CertificateFileProtected, QUICER_CERTIFICATE_FILE_PROTECTED); } From 819427d1131840f44d90353e8595eedae77eeb6f Mon Sep 17 00:00:00 2001 From: William Yang Date: Sun, 3 Sep 2023 09:06:16 +0200 Subject: [PATCH 07/12] feat(listener): release udp port when listener owner pid dead. --- c_src/quicer_ctx.c | 1 + c_src/quicer_ctx.h | 1 + c_src/quicer_listener.c | 7 ++--- c_src/quicer_nif.c | 13 ++++++-- test/quicer_listener_SUITE.erl | 57 ++++++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 7 deletions(-) diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index 25cd7b50..b6893f48 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -102,6 +102,7 @@ destroy_l_ctx(QuicerListenerCTX *l_ctx) l_ctx->r_ctx = NULL; } l_ctx->config_resource = NULL; + enif_demonitor_process(l_ctx->env, l_ctx, &l_ctx->owner_mon); enif_release_resource(l_ctx); } diff --git a/c_src/quicer_ctx.h b/c_src/quicer_ctx.h index e94ec316..c3c472c9 100644 --- a/c_src/quicer_ctx.h +++ b/c_src/quicer_ctx.h @@ -55,6 +55,7 @@ typedef struct QuicerListenerCTX HQUIC Listener; QUICER_ACCEPTOR_QUEUE *acceptor_queue; ErlNifPid listenerPid; + ErlNifMonitor owner_mon; ErlNifEnv *env; ErlNifMutex *lock; char *cacertfile; diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 0a62b44b..fb33f7a6 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -352,10 +352,9 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) } // mon will be removed when triggered or when l_ctx is dealloc. - // this is wrong scope - ErlNifMonitor mon; - - if (0 != enif_monitor_process(env, l_ctx, &l_ctx->listenerPid, &mon)) + if (0 + != enif_monitor_process( + env, l_ctx, &l_ctx->listenerPid, &l_ctx->owner_mon)) { destroy_l_ctx(l_ctx); return ERROR_TUPLE_2(ATOM_BAD_MON); diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index c5bbc7ff..2ba329a1 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -758,12 +758,19 @@ QUIC_REGISTRATION_CONFIG GRegConfig = { "quicer_nif", QUIC_EXECUTION_PROFILE_LOW_LATENCY }; void -resource_listener_down_callback(__unused_parm__ ErlNifEnv *caller_env, - __unused_parm__ void *obj, +resource_listener_down_callback(__unused_parm__ ErlNifEnv *env, + void *ctx, __unused_parm__ ErlNifPid *pid, __unused_parm__ ErlNifMonitor *mon) { - // @TODO + QuicerListenerCTX *l_ctx = (QuicerListenerCTX *)ctx; + TP_CB_3(start, (uintptr_t)l_ctx->Listener, 0); + if (l_ctx->Listener) + { + MsQuic->ListenerStop(l_ctx->Listener); + } + + TP_CB_3(end, (uintptr_t)l_ctx->Listener, 0); } void diff --git a/test/quicer_listener_SUITE.erl b/test/quicer_listener_SUITE.erl index 912a7f3a..c4ef6695 100644 --- a/test/quicer_listener_SUITE.erl +++ b/test/quicer_listener_SUITE.erl @@ -18,6 +18,7 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("stdlib/include/assert.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). -compile(export_all). -compile(nowarn_export_all). @@ -407,6 +408,62 @@ tc_get_listener(Config) -> end, Listeners), ?assertEqual({error, not_found}, quicer:listener(bad_listen_name)). +tc_listener_closed_when_owner_die_and_gc(Config) -> + Port = select_port(), + Me = self(), + %% Given when port is occupied by another process + {Pid, MRef} = spawn_monitor(fun() -> + {ok, _L} = quicer:listen(Port, default_listen_opts(Config)), + Me ! {started, self()}, + receive done -> ok end + end), + receive {started, Pid} -> ok end, + {error, listener_start_error, {unknown_quic_status, Reason}} + = quicer:listen(Port, default_listen_opts(Config)), + ?assert(Reason == 91 orelse Reason == 41), + + Pid ! done, + %% When the owner process dies + receive {'DOWN', MRef, process, Pid, normal} -> + ok + end, + %% Then port is released and new listener can be started + {ok, L} = snabbkaffe:retry(10000, 10, + fun() -> + {ok, _L0} = quicer:listen(Port, default_listen_opts(Config)) + end), + ok = quicer:close_listener(L). + +tc_listener_stopped_when_owner_die(Config) -> + Port = select_port(), + Me = self(), + {Pid, MRef} = spawn_monitor(fun() -> + {ok, L} = quicer:listen(Port, default_listen_opts(Config)), + Me ! {started, self(), L}, + receive done -> ok end + end), + %% Given a Listener Handle owned by another process + receive {started, Pid, L0} -> ok end, + {error, listener_start_error, {unknown_quic_status, Reason}} + = quicer:listen(Port, default_listen_opts(Config)), + ?assert(Reason == 91 orelse Reason == 41), + + Pid ! done, + %% When the owner process dies + receive {'DOWN', MRef, process, Pid, normal} -> + ok + end, + %% Then port is released and new listener can be started + {ok, L1} = snabbkaffe:retry(10000, 10, + fun() -> + {ok, _L0} = quicer:listen(Port, default_listen_opts(Config)) + end), + %% Then the old listener can be closed but timeout since it is already stopped + %% and no stop event is triggered + {error, timeout} = quicer:close_listener(L0, _timeout = 10), + %% Then the new listener can be closed + ok = quicer:close_listener(L1). + select_port() -> select_free_port(quic). From 3d7ccf0b9b4e23d020e1636bdd2661c866fe1759 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 4 Sep 2023 11:22:36 +0200 Subject: [PATCH 08/12] refactor(listener): move release of certificate file to quicer_tls --- c_src/quicer_listener.c | 60 ++++++++++++++++++----------------------- c_src/quicer_tls.c | 28 +++++++++++++++++++ c_src/quicer_tls.h | 2 ++ 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index fb33f7a6..8907cdb1 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -230,6 +230,7 @@ ERL_NIF_TERM listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) { QUIC_STATUS Status = QUIC_STATUS_SUCCESS; + ERL_NIF_TERM ret = ATOM_OK; ERL_NIF_TERM elisten_on = argv[0]; ERL_NIF_TERM options = argv[1]; @@ -249,9 +250,9 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) return ERROR_TUPLE_2(ATOM_BADARG); } - // Start Build CredConfig from options + // Start build CredConfig from with listen opts QUIC_CREDENTIAL_CONFIG CredConfig; - // CxPlatZeroMemory(&CredConfig, sizeof(QUIC_CREDENTIAL_CONFIG)); + CredConfig.Flags = QUIC_CREDENTIAL_FLAG_NONE; if (!parse_cert_options(env, options, &CredConfig)) @@ -289,23 +290,23 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) if (!build_trustedstore(l_ctx->cacertfile, &l_ctx->trusted_store)) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_CERT_ERROR); + ret = ERROR_TUPLE_2(ATOM_CERT_ERROR); + goto exit; } } // Set owner for l_ctx if (!enif_self(env, &(l_ctx->listenerPid))) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_BAD_PID); + ret = ERROR_TUPLE_2(ATOM_BAD_PID); + goto exit; } // Get Reg for l_ctx, quic_registration is optional if (!parse_registration(env, options, &l_ctx->r_ctx)) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); + ret = ERROR_TUPLE_2(ATOM_QUIC_REGISTRATION); + goto exit; } if (l_ctx->r_ctx) @@ -329,26 +330,10 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) &l_ctx->config_resource->Configuration, &CredConfig); - // Cleanup CredConfig - if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE == CredConfig.Type) - { - free((char *)CredConfig.CertificateFile->CertificateFile); - free((char *)CredConfig.CertificateFile->PrivateKeyFile); - CxPlatFree(CredConfig.CertificateFile, QUICER_CERTIFICATE_FILE); - } - else if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED == CredConfig.Type) - { - free((char *)CredConfig.CertificateFileProtected->CertificateFile); - free((char *)CredConfig.CertificateFileProtected->PrivateKeyFile); - free((char *)CredConfig.CertificateFileProtected->PrivateKeyPassword); - CxPlatFree(CredConfig.CertificateFileProtected, - QUICER_CERTIFICATE_FILE_PROTECTED); - } - if (!IS_SAME_TERM(ATOM_OK, estatus)) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_3(ATOM_CONFIG_ERROR, estatus); + ret = ERROR_TUPLE_3(ATOM_CONFIG_ERROR, estatus); + goto exit; } // mon will be removed when triggered or when l_ctx is dealloc. @@ -356,8 +341,8 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) != enif_monitor_process( env, l_ctx, &l_ctx->listenerPid, &l_ctx->owner_mon)) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_BAD_MON); + ret = ERROR_TUPLE_2(ATOM_BAD_MON); + goto exit; } // Now open listener @@ -370,8 +355,8 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) { // Server Configuration should be destroyed l_ctx->config_resource->Configuration = NULL; - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_3(ATOM_LISTENER_OPEN_ERROR, ATOM_STATUS(Status)); + ret = ERROR_TUPLE_3(ATOM_LISTENER_OPEN_ERROR, ATOM_STATUS(Status)); + goto exit; } l_ctx->is_closed = FALSE; @@ -388,8 +373,8 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) if (!load_alpn(env, &options, &alpn_buffer_length, alpn_buffers)) { - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_2(ATOM_ALPN); + ret = ERROR_TUPLE_2(ATOM_ALPN); + goto exit; } // Start Listener @@ -400,11 +385,18 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) TP_NIF_3(start_fail, (uintptr_t)(l_ctx->Listener), Status); MsQuic->ListenerClose(l_ctx->Listener); l_ctx->Listener = NULL; - destroy_l_ctx(l_ctx); - return ERROR_TUPLE_3(ATOM_LISTENER_START_ERROR, ATOM_STATUS(Status)); + ret = ERROR_TUPLE_3(ATOM_LISTENER_START_ERROR, ATOM_STATUS(Status)); + goto exit; } ERL_NIF_TERM listenHandle = enif_make_resource(env, l_ctx); + + free_certificate(&CredConfig); return OK_TUPLE_2(listenHandle); + +exit: //errors.. + free_certificate(&CredConfig); + destroy_l_ctx(l_ctx); + return ret; } ERL_NIF_TERM diff --git a/c_src/quicer_tls.c b/c_src/quicer_tls.c index 73341fb9..3178bd18 100644 --- a/c_src/quicer_tls.c +++ b/c_src/quicer_tls.c @@ -171,3 +171,31 @@ build_trustedstore(const char *cacertfile, X509_STORE **trusted_store) *trusted_store = store; return TRUE; } + +/* + * Free certfile/certfileprotected of QUIC_CREDENTIAL_CONFIG + * +*/ +void +free_certificate(QUIC_CREDENTIAL_CONFIG *cc) +{ + if (!cc) + { + return; + } + + if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE == cc->Type) + { + free((char *)cc->CertificateFile->CertificateFile); + free((char *)cc->CertificateFile->PrivateKeyFile); + CxPlatFree(cc->CertificateFile, QUICER_CERTIFICATE_FILE); + } + else if (QUIC_CREDENTIAL_TYPE_CERTIFICATE_FILE_PROTECTED == cc->Type) + { + free((char *)cc->CertificateFileProtected->CertificateFile); + free((char *)cc->CertificateFileProtected->PrivateKeyFile); + free((char *)cc->CertificateFileProtected->PrivateKeyPassword); + CxPlatFree(cc->CertificateFileProtected, + QUICER_CERTIFICATE_FILE_PROTECTED); + } +} diff --git a/c_src/quicer_tls.h b/c_src/quicer_tls.h index 4884423a..c8036122 100644 --- a/c_src/quicer_tls.h +++ b/c_src/quicer_tls.h @@ -35,4 +35,6 @@ parse_cacertfile_option(ErlNifEnv *env, BOOLEAN build_trustedstore(const char *cacertfile, X509_STORE **trusted_store); +void +free_certificate(QUIC_CREDENTIAL_CONFIG *cc); #endif // QUICER_TLS_H_ From d070f4897adc481a558d85014268c9373c102612 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 4 Sep 2023 11:45:19 +0200 Subject: [PATCH 09/12] chore: format code --- c_src/quicer_ctx.c | 2 +- c_src/quicer_listener.c | 2 +- c_src/quicer_tls.c | 2 +- c_src/quicer_tls.h | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/c_src/quicer_ctx.c b/c_src/quicer_ctx.c index b6893f48..b5c376c6 100644 --- a/c_src/quicer_ctx.c +++ b/c_src/quicer_ctx.c @@ -95,7 +95,7 @@ destroy_l_ctx(QuicerListenerCTX *l_ctx) // @note, Destroy config asap as it holds rundown // ref count in registration destroy_config_ctx(l_ctx->config_resource); - + if (l_ctx->r_ctx) { enif_release_resource(l_ctx->r_ctx); diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 8907cdb1..8cd93833 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -393,7 +393,7 @@ listen2(ErlNifEnv *env, __unused_parm__ int argc, const ERL_NIF_TERM argv[]) free_certificate(&CredConfig); return OK_TUPLE_2(listenHandle); -exit: //errors.. +exit: // errors.. 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 3178bd18..3e4d3212 100644 --- a/c_src/quicer_tls.c +++ b/c_src/quicer_tls.c @@ -175,7 +175,7 @@ build_trustedstore(const char *cacertfile, X509_STORE **trusted_store) /* * Free certfile/certfileprotected of QUIC_CREDENTIAL_CONFIG * -*/ + */ void free_certificate(QUIC_CREDENTIAL_CONFIG *cc) { diff --git a/c_src/quicer_tls.h b/c_src/quicer_tls.h index c8036122..ec0553fd 100644 --- a/c_src/quicer_tls.h +++ b/c_src/quicer_tls.h @@ -35,6 +35,5 @@ parse_cacertfile_option(ErlNifEnv *env, BOOLEAN build_trustedstore(const char *cacertfile, X509_STORE **trusted_store); -void -free_certificate(QUIC_CREDENTIAL_CONFIG *cc); +void free_certificate(QUIC_CREDENTIAL_CONFIG *cc); #endif // QUICER_TLS_H_ From 1aa2825e7154c7162e80f9241d88e10621969737 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 4 Sep 2023 13:37:49 +0200 Subject: [PATCH 10/12] fix(listener): mark listener stopped in resource down callback --- c_src/quicer_nif.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index 2ba329a1..aa642047 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -765,14 +765,27 @@ resource_listener_down_callback(__unused_parm__ ErlNifEnv *env, { QuicerListenerCTX *l_ctx = (QuicerListenerCTX *)ctx; TP_CB_3(start, (uintptr_t)l_ctx->Listener, 0); - if (l_ctx->Listener) + BOOLEAN to_stop = FALSE; + enif_mutex_lock(l_ctx->lock); + if (!l_ctx->is_closed && !l_ctx->is_stopped && l_ctx->Listener) { + to_stop = TRUE; + l_ctx->is_stopped = TRUE; + } + enif_mutex_unlock(l_ctx->lock); + if (to_stop) + { + // We only stop here, but not close it, because we want to + // close it in resource_listener_dealloc_callback + // Or some pid could still start the stopped listener with nif handle MsQuic->ListenerStop(l_ctx->Listener); } - TP_CB_3(end, (uintptr_t)l_ctx->Listener, 0); } +/* +** Listener NIF handle, end of world... +*/ void resource_listener_dealloc_callback(__unused_parm__ ErlNifEnv *env, void *obj) { From e1db656e56462606fd1a34b304951d6cf354c604 Mon Sep 17 00:00:00 2001 From: William Yang Date: Mon, 4 Sep 2023 17:39:09 +0200 Subject: [PATCH 11/12] ci: debug coredump bt --- .github/workflows/main.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5e864810..3e075e0f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -121,9 +121,11 @@ jobs: - name: gdb bt if: failure() run: | + set -x which gdb || sudo apt install gdb - corefile=$(find _build/test -name core) - if [ -n $corefile ]; then + corefile=$(find _build/test -name core.*) + if [ -n "$corefile" ]; then + echo "found corefile: $corefile"; gdb -ex bt $(erl -noshell -eval 'io:format(code:root_dir()),halt()')/erts-*/bin/beam.smp "${corefile}" else echo "No coredump found" From e49cc164e49aeb738efce577c8183aa32ba41b9a Mon Sep 17 00:00:00 2001 From: William Yang Date: Tue, 5 Sep 2023 11:48:02 +0200 Subject: [PATCH 12/12] feat(listener): stop listener with fine-tuned lockings In Msquic, the listener callback for stopped listener event is called in same code path of listener stop/close API. Thus we don't need to hold lock in callback, instead the caller should hold it. --- c_src/quicer_listener.c | 30 ++++++++++++++++-------------- c_src/quicer_nif.c | 16 +++++++--------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/c_src/quicer_listener.c b/c_src/quicer_listener.c index 8cd93833..25a434f5 100644 --- a/c_src/quicer_listener.c +++ b/c_src/quicer_listener.c @@ -36,11 +36,10 @@ ServerListenerCallback(__unused_parm__ HQUIC Listener, QuicerConnCTX *c_ctx = NULL; BOOLEAN is_destroy = FALSE; - enif_mutex_lock(l_ctx->lock); - // dbg("server listener event: %d", Event->Type); switch (Event->Type) { case QUIC_LISTENER_EVENT_NEW_CONNECTION: + enif_mutex_lock(l_ctx->lock); // // Note, c_ctx is newly init here, don't grab lock. // @@ -196,6 +195,9 @@ ServerListenerCallback(__unused_parm__ HQUIC Listener, break; case QUIC_LISTENER_EVENT_STOP_COMPLETE: + // **Note**, the callback in msquic for this event is called in the + // MsQuicListenerClose or MsQuicListenerStop. we assume caller should + // ensure the thread safty thus we don't hold lock env = l_ctx->env; enif_send(NULL, &(l_ctx->listenerPid), @@ -207,18 +209,20 @@ ServerListenerCallback(__unused_parm__ HQUIC Listener, if (!l_ctx->Listener) { // @NOTE This callback is part of the listener *close* process - // Listener is already closing, we can destroy the l_ctx now. + // Listener is already closing, we can destroy the l_ctx now + // as the handle is NULL no subsequent msquic API is allowed/possible assert(!l_ctx->is_stopped); is_destroy = TRUE; } enif_clear_env(env); - break; + goto Exit2; default: break; } Error: enif_mutex_unlock(l_ctx->lock); +Exit2: if (is_destroy) { destroy_l_ctx(l_ctx); @@ -445,27 +449,25 @@ stop_listener1(ErlNifEnv *env, { QuicerListenerCTX *l_ctx; ERL_NIF_TERM ret = ATOM_OK; - BOOLEAN is_stopped = FALSE; assert(argc == 1); if (!enif_get_resource(env, argv[0], ctx_listener_t, (void **)&l_ctx)) { return ERROR_TUPLE_2(ATOM_BADARG); } - enif_mutex_lock(l_ctx->lock); - HQUIC l = l_ctx->Listener; - is_stopped = l_ctx->is_stopped; - l_ctx->is_stopped = TRUE; - enif_mutex_unlock(l_ctx->lock); - if (!l) + if (!l_ctx->Listener) { - return ERROR_TUPLE_2(ATOM_CLOSED); + ret = ERROR_TUPLE_2(ATOM_CLOSED); + goto exit; } - else if (!is_stopped) + else if (!l_ctx->is_stopped) { + l_ctx->is_stopped = TRUE; // void return - MsQuic->ListenerStop(l); + MsQuic->ListenerStop(l_ctx->Listener); } +exit: + enif_mutex_unlock(l_ctx->lock); return ret; } diff --git a/c_src/quicer_nif.c b/c_src/quicer_nif.c index aa642047..594b60f8 100644 --- a/c_src/quicer_nif.c +++ b/c_src/quicer_nif.c @@ -765,21 +765,19 @@ resource_listener_down_callback(__unused_parm__ ErlNifEnv *env, { QuicerListenerCTX *l_ctx = (QuicerListenerCTX *)ctx; TP_CB_3(start, (uintptr_t)l_ctx->Listener, 0); - BOOLEAN to_stop = FALSE; + // Hold lock for the race of ListenerClose and ListenerStop call enif_mutex_lock(l_ctx->lock); if (!l_ctx->is_closed && !l_ctx->is_stopped && l_ctx->Listener) { - to_stop = TRUE; l_ctx->is_stopped = TRUE; - } - enif_mutex_unlock(l_ctx->lock); - if (to_stop) - { - // We only stop here, but not close it, because we want to - // close it in resource_listener_dealloc_callback - // Or some pid could still start the stopped listener with nif handle + // We only stop here, but not close it, because possible subsequent + // scenarios a. Some pid could still start the stopped listener with nif + // handle b. Some pid could still close the stopped listener with nif + // handle + // 3. We close it in resource_listener_dealloc_callback anyway. MsQuic->ListenerStop(l_ctx->Listener); } + enif_mutex_unlock(l_ctx->lock); TP_CB_3(end, (uintptr_t)l_ctx->Listener, 0); }