From 44f071ff20e7310fc5a680d84ab95b5f6ff77fbd Mon Sep 17 00:00:00 2001 From: Uwez Khan Date: Mon, 5 Oct 2026 16:40:58 +0530 Subject: [PATCH 1/3] fix dangling local raw_chain in GnuTLS ssl_update_certs_info The local and peer raw chains are both stored in info_pool, but the pool was reset again before the peer chain was stored, leaving local_cert_info.raw_chain pointing at released memory. Reset the pool once per update so both chains stay valid. --- pjlib/src/pj/ssl_sock_gtls.c | 11 ++++++++--- pjlib/src/pjlib-test/ssl_sock.c | 23 ++++++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/pjlib/src/pj/ssl_sock_gtls.c b/pjlib/src/pj/ssl_sock_gtls.c index 02a1e53576..d2bf5149b8 100644 --- a/pjlib/src/pj/ssl_sock_gtls.c +++ b/pjlib/src/pj/ssl_sock_gtls.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2018 Teluu Inc. (http://www.teluu.com) + * Copyright (C) 2018-2026 Teluu Inc. (http://www.teluu.com) * Copyright (C) 2014-2017 Savoir-faire Linux. * (https://www.savoirfairelinux.com) * @@ -1144,6 +1144,13 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock) pj_assert(ssock->ssl_state == SSL_STATE_ESTABLISHED); + /* Both raw chains live in info_pool, so release them together */ + pj_pool_reset(ssock->info_pool); + pj_bzero(&ssock->local_cert_info.raw_chain, + sizeof(ssock->local_cert_info.raw_chain)); + pj_bzero(&ssock->remote_cert_info.raw_chain, + sizeof(ssock->remote_cert_info.raw_chain)); + /* Get active local certificate */ us = gnutls_certificate_get_ours(gssock->session); if (!us) @@ -1159,7 +1166,6 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock) goto us_out; tls_cert_get_info(ssock->pool, &ssock->local_cert_info, cert); - pj_pool_reset(ssock->info_pool); tls_cert_get_chain_raw(ssock->info_pool, &ssock->local_cert_info, us, 1); us_out: @@ -1187,7 +1193,6 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock) goto peer_out; tls_cert_get_info(ssock->pool, &ssock->remote_cert_info, cert); - pj_pool_reset(ssock->info_pool); tls_cert_get_chain_raw(ssock->info_pool, &ssock->remote_cert_info, certs, certslen); diff --git a/pjlib/src/pjlib-test/ssl_sock.c b/pjlib/src/pjlib-test/ssl_sock.c index cab1145941..b85c5da55b 100644 --- a/pjlib/src/pjlib-test/ssl_sock.c +++ b/pjlib/src/pjlib-test/ssl_sock.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2011 Teluu Inc. (http://www.teluu.com) + * Copyright (C) 2008-2026 Teluu Inc. (http://www.teluu.com) * Copyright (C) 2003-2008 Benny Prijono * * This program is free software; you can redistribute it and/or modify @@ -117,6 +117,21 @@ static void dump_ssl_info(const pj_ssl_sock_info *si) } } +/* The local and remote raw chain must not share storage */ +static pj_status_t check_raw_chain(const pj_ssl_sock_info *si) +{ + const pj_ssl_cert_info *lci = si->local_cert_info; + const pj_ssl_cert_info *rci = si->remote_cert_info; + + if (lci && rci && lci->raw_chain.cnt && rci->raw_chain.cnt && + lci->raw_chain.cert_raw == rci->raw_chain.cert_raw) + { + return PJ_EBUG; + } + + return PJ_SUCCESS; +} + static pj_bool_t ssl_on_connect_complete(pj_ssl_sock_t *ssock, pj_status_t status) @@ -138,6 +153,12 @@ static pj_bool_t ssl_on_connect_complete(pj_ssl_sock_t *ssock, goto on_return; } + status = check_raw_chain(&info); + if (status != PJ_SUCCESS) { + app_perror("...ERROR local/remote raw chain overlap", status); + goto on_return; + } + pj_sockaddr_print((pj_sockaddr_t*)&info.local_addr, buf1, sizeof(buf1), 1); pj_sockaddr_print((pj_sockaddr_t*)&info.remote_addr, buf2, sizeof(buf2), 1); PJ_LOG(3, ("", "...Connected %s -> %s!", buf1, buf2)); From 6ef4d030d37230750d3580fc5169b23ecffeff46 Mon Sep 17 00:00:00 2001 From: Uwez Khan Date: Wed, 7 Oct 2026 14:43:38 +0530 Subject: [PATCH 2/3] gnutls: zero cert info on update failure, strengthen raw chain test ssl_update_certs_info() zeroed the whole pj_ssl_cert_info only when gnutls_x509_crt_init() had not produced a cert. Where init succeeded but both imports failed, the identity fields stayed from the previous handshake while the raw chain was already released, so the two disagreed. Track whether each half completed and zero the whole struct otherwise: a cert info is now either fully refreshed or fully cleared, which also makes the separate raw_chain clearing redundant. The pjlib-test check now compares the local chain against the DER of the configured certificate rather than only testing that it does not share an array with the peer chain, and it runs on the accepted socket too so a regression on the server side is caught. It reports through the parent test state instead of closing the accepted socket, because on_handshake_complete() still dereferences ssock->parent after the accept callback returns. --- pjlib/src/pj/ssl_sock_gtls.c | 12 +++---- pjlib/src/pjlib-test/ssl_sock.c | 53 +++++++++++++++++++++++++--- pjlib/src/pjlib-test/ssl_sock_test.h | 1 + 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/pjlib/src/pj/ssl_sock_gtls.c b/pjlib/src/pj/ssl_sock_gtls.c index d2bf5149b8..5f319f2020 100644 --- a/pjlib/src/pj/ssl_sock_gtls.c +++ b/pjlib/src/pj/ssl_sock_gtls.c @@ -1141,15 +1141,12 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock) const gnutls_datum_t *certs; unsigned int certslen = 0; int ret = GNUTLS_CERT_INVALID; + pj_bool_t updated = PJ_FALSE; pj_assert(ssock->ssl_state == SSL_STATE_ESTABLISHED); /* Both raw chains live in info_pool, so release them together */ pj_pool_reset(ssock->info_pool); - pj_bzero(&ssock->local_cert_info.raw_chain, - sizeof(ssock->local_cert_info.raw_chain)); - pj_bzero(&ssock->remote_cert_info.raw_chain, - sizeof(ssock->remote_cert_info.raw_chain)); /* Get active local certificate */ us = gnutls_certificate_get_ours(gssock->session); @@ -1167,15 +1164,17 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock) tls_cert_get_info(ssock->pool, &ssock->local_cert_info, cert); tls_cert_get_chain_raw(ssock->info_pool, &ssock->local_cert_info, us, 1); + updated = PJ_TRUE; us_out: tls_last_error = ret; if (cert) gnutls_x509_crt_deinit(cert); - else + if (!updated) pj_bzero(&ssock->local_cert_info, sizeof(pj_ssl_cert_info)); cert = NULL; + updated = PJ_FALSE; /* Get active remote certificate */ certs = gnutls_certificate_get_peers(gssock->session, &certslen); @@ -1195,12 +1194,13 @@ static void ssl_update_certs_info(pj_ssl_sock_t *ssock) tls_cert_get_info(ssock->pool, &ssock->remote_cert_info, cert); tls_cert_get_chain_raw(ssock->info_pool, &ssock->remote_cert_info, certs, certslen); + updated = PJ_TRUE; peer_out: tls_last_error = ret; if (cert) gnutls_x509_crt_deinit(cert); - else + if (!updated) pj_bzero(&ssock->remote_cert_info, sizeof(pj_ssl_cert_info)); } diff --git a/pjlib/src/pjlib-test/ssl_sock.c b/pjlib/src/pjlib-test/ssl_sock.c index b85c5da55b..87885f9911 100644 --- a/pjlib/src/pjlib-test/ssl_sock.c +++ b/pjlib/src/pjlib-test/ssl_sock.c @@ -117,18 +117,51 @@ static void dump_ssl_info(const pj_ssl_sock_info *si) } } -/* The local and remote raw chain must not share storage */ -static pj_status_t check_raw_chain(const pj_ssl_sock_info *si) +/* The local raw chain must hold the local certificate, in storage of its + * own rather than the peer chain's. Backends that don't report the local + * raw chain, e.g. OpenSSL, leave cnt zero and are skipped. + */ +static pj_status_t check_raw_chain(const pj_ssl_sock_info *si, + pj_pool_t *pool) { const pj_ssl_cert_info *lci = si->local_cert_info; const pj_ssl_cert_info *rci = si->remote_cert_info; + pj_str_t der_file = pj_str(CERT_DER_FILE); + pj_oshandle_t fd = NULL; + pj_ssize_t size; + pj_str_t der; + pj_status_t status; + + if (!lci || lci->raw_chain.cnt == 0) + return PJ_SUCCESS; - if (lci && rci && lci->raw_chain.cnt && rci->raw_chain.cnt && + if (rci && rci->raw_chain.cnt && lci->raw_chain.cert_raw == rci->raw_chain.cert_raw) { return PJ_EBUG; } + size = (pj_ssize_t)pj_file_size(der_file.ptr); + if (size <= 0) + return PJ_ENOTFOUND; + + status = pj_file_open(pool, der_file.ptr, PJ_O_RDONLY, &fd); + if (status != PJ_SUCCESS) + return status; + + der.ptr = (char*)pj_pool_alloc(pool, size); + status = pj_file_read(fd, der.ptr, &size); + der.slen = size; + pj_file_close(fd); + if (status != PJ_SUCCESS) + return status; + + if (lci->raw_chain.cert_raw[0].slen != der.slen || + pj_memcmp(lci->raw_chain.cert_raw[0].ptr, der.ptr, der.slen)) + { + return PJ_EBUG; + } + return PJ_SUCCESS; } @@ -153,9 +186,9 @@ static pj_bool_t ssl_on_connect_complete(pj_ssl_sock_t *ssock, goto on_return; } - status = check_raw_chain(&info); + status = check_raw_chain(&info, st->pool); if (status != PJ_SUCCESS) { - app_perror("...ERROR local/remote raw chain overlap", status); + app_perror("...ERROR invalid local raw chain", status); goto on_return; } @@ -246,6 +279,16 @@ static pj_bool_t ssl_on_accept_complete(pj_ssl_sock_t *ssock, goto on_return; } + status = check_raw_chain(&info, st->pool); + if (status != PJ_SUCCESS) { + app_perror("...ERROR invalid local raw chain", status); + /* Report through the parent, don't close from this callback: + * on_handshake_complete() still uses ssock->parent afterwards. + */ + parent_st->err = status; + return PJ_TRUE; + } + pj_sockaddr_print(src_addr, buf, sizeof(buf), 1); PJ_LOG(3, ("", "...Accepted connection from %s", buf)); diff --git a/pjlib/src/pjlib-test/ssl_sock_test.h b/pjlib/src/pjlib-test/ssl_sock_test.h index cd2a41acbd..a3bdad29a0 100644 --- a/pjlib/src/pjlib-test/ssl_sock_test.h +++ b/pjlib/src/pjlib-test/ssl_sock_test.h @@ -15,6 +15,7 @@ # define CERT_CA_FILE CERT_DIR "cacert.pem" #endif #define CERT_FILE CERT_DIR "cacert.pem" +#define CERT_DER_FILE CERT_DIR "cacert.der" #define CERT_PRIVKEY_FILE CERT_DIR "privkey.pem" #define CERT_PRIVKEY_PASS "privkeypass" From 748cf61c16ce5b0b2657539cbec8d72b9e1bf114 Mon Sep 17 00:00:00 2001 From: Uwez Khan Date: Thu, 8 Oct 2026 16:03:32 +0530 Subject: [PATCH 3/3] pjlib-test: give the SSL client its own certificate, keep the server echoing The raw chain check now compares the local chain against the DER of the certificate that end actually presents, which catches the dangling chain whatever address the pool hands back. The accepted socket records the error through the parent and runs the connection to completion instead of returning idle, and echo_test() closes it when its own callbacks have not. --- pjlib/build/clicert.der | Bin 0 -> 1090 bytes pjlib/build/clicert.pem | 25 ++++++++ pjlib/build/cliprivkey.pem | 30 ++++++++++ pjlib/src/pjlib-test/ssl_sock.c | 86 +++++++++++++++++++++++---- pjlib/src/pjlib-test/ssl_sock_test.h | 19 ++++++ 5 files changed, 147 insertions(+), 13 deletions(-) create mode 100644 pjlib/build/clicert.der create mode 100644 pjlib/build/clicert.pem create mode 100644 pjlib/build/cliprivkey.pem diff --git a/pjlib/build/clicert.der b/pjlib/build/clicert.der new file mode 100644 index 0000000000000000000000000000000000000000..55fa38aa0dc366ffdffcee4d4374852c8a13a30c GIT binary patch literal 1090 zcmXqLVzD!5Vp3bc%*4n9L*3`7it34oS&VGeC7gQomE-!K0Y*jhQc+Q5EA6lb3N;YOjpSB7M zbAEnmo6P(tv*xm;9Gs>xZQAB&`MT8;8{TO;PS8>)$g^;I=N_#cc6&yb!44VQ|0mlR zSMF!sDjh1Y@v+zR;AG%X*y(&j<=DCtXTRsi-v4?^-4of_@AYKM#FIOPW^TW9YVY%3*3m1C zZ?w!+nWe(3bl-c~hsWDzo^$8=`aR^D;bC|G0KwbeSkxZR`(eKS?SbyIcM96I+v;U4FpvjHE3-%#h&71Bt)4S4fAfjp(sl21OeCVxmz;9l zjvQ~m)WO`u#K@qa_jMgZ^UE&H=PA7tEyQ`te{c0p-8zwX)he+MT(j&C&yZp>G2(ui zxMDiL(k{*aN4<*+%QDyUAK27#TFzfqF}%*~=8@isD?YV6f1lny#m{=9iteNhs+tq> zZ(Mlc*;*3DB$MbsYgUJWpXKJt6p;4;%!7Ns`xo_dF z2kJ?m?(4N(a&To5;U0m8KYm=IDvA=wIk-}P$OB%Lk85}ll zar0eQec8%##{HyH?i;6>BRsQjzBk?6`|gOswDyyE5=ISrg27=qnFq8dG(eem5p z+5Sx1q^I%fB5#|1PubD0c>b9E%a5govA0eOhyJab;W0g7muIrmog3R$8{J;X%Ca?U zcB5FCb*Asr9SPsWk|hG$_NjfUI4Hedr)J76tJ50do>uR+FSHSEJG0yM`>Z)4H*=1Q z@UB~SHBvnL_p#lMy~q4_hhGXmF}YB4v1?A>#!3rkoB!dVx2Cj8rX33VET!rE@we4x zhs92rK3Z$poOT>(<9VQFxp!)mSfn*)*Y-yx1t)f{=v@~*^<`!#qsIB@Ge6$wX)V9X z_x5R-*7(__ j)-45ITpslocal_cert_info; const pj_ssl_cert_info *rci = si->remote_cert_info; - pj_str_t der_file = pj_str(CERT_DER_FILE); + pj_str_t der_file; pj_oshandle_t fd = NULL; pj_ssize_t size; pj_str_t der; @@ -141,6 +146,10 @@ static pj_status_t check_raw_chain(const pj_ssl_sock_info *si, return PJ_EBUG; } + if (!der_name) + return PJ_SUCCESS; + + der_file = pj_str((char*)der_name); size = (pj_ssize_t)pj_file_size(der_file.ptr); if (size <= 0) return PJ_ENOTFOUND; @@ -186,7 +195,7 @@ static pj_bool_t ssl_on_connect_complete(pj_ssl_sock_t *ssock, goto on_return; } - status = check_raw_chain(&info, st->pool); + status = check_raw_chain(&info, st->pool, st->cert_der); if (status != PJ_SUCCESS) { app_perror("...ERROR invalid local raw chain", status); goto on_return; @@ -272,6 +281,7 @@ static pj_bool_t ssl_on_accept_complete(pj_ssl_sock_t *ssock, /* Track accepted socket for cleanup */ parent_st->accepted_ssock = newsock; + parent_st->accepted_st = st; status = pj_ssl_sock_get_info(newsock, &info); if (status != PJ_SUCCESS) { @@ -279,14 +289,15 @@ static pj_bool_t ssl_on_accept_complete(pj_ssl_sock_t *ssock, goto on_return; } - status = check_raw_chain(&info, st->pool); + status = check_raw_chain(&info, st->pool, st->cert_der); if (status != PJ_SUCCESS) { app_perror("...ERROR invalid local raw chain", status); - /* Report through the parent, don't close from this callback: - * on_handshake_complete() still uses ssock->parent afterwards. + /* Report through the parent and keep echoing: closing from this + * callback leaves on_handshake_complete() using a released + * ssock->parent, and perf_test() waits for every client to finish. */ parent_st->err = status; - return PJ_TRUE; + status = PJ_SUCCESS; } pj_sockaddr_print(src_addr, buf, sizeof(buf), 1); @@ -755,6 +766,7 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, pj_sockaddr addr, listen_addr; pj_ssl_cipher ciphers[1]; pj_ssl_cert_t *cert = NULL; + pj_ssl_cert_t *cli_cert = NULL; pj_status_t status; pool = pj_pool_create(mem, "ssl_echo", 256, 256, NULL); @@ -795,6 +807,7 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, state_serv.echo = PJ_TRUE; state_serv.is_server = PJ_TRUE; state_serv.is_verbose = PJ_TRUE; + state_serv.cert_der = CERT_DER_FILE; status = pj_ssl_sock_create(pool, ¶m, &ssock_serv); if (status != PJ_SUCCESS) { @@ -898,6 +911,7 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, state_cli.pool = pool; state_cli.check_echo = PJ_TRUE; state_cli.is_verbose = PJ_TRUE; + state_cli.cert_der = CERT_DER_FILE; { /* srand() should be done centrally (blp) @@ -939,6 +953,8 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, } #else { + cli_cert = cert; + if (!client_provide_cert) { pj_str_t ca_file = pj_str(CERT_CA_FILE); pj_str_t null_str = pj_str(""); @@ -947,11 +963,12 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, #if (defined(TEST_LOAD_FROM_FILES) && TEST_LOAD_FROM_FILES==1) /* Reset any certificate & private keys previously set */ status = pj_ssl_cert_load_from_files(pool, &ca_file, &null_str, - &null_str, &null_str, &cert); + &null_str, &null_str, + &cli_cert); /* Also reset any direct backend objects */ pj_bzero(&cert_direct, sizeof(cert_direct)); - pj_ssl_cert_load_direct(pool, &cert_direct, &cert); + pj_ssl_cert_load_direct(pool, &cert_direct, &cli_cert); #else pj_ssl_cert_buffer null_buf, ca_buf; @@ -963,15 +980,53 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, } status = pj_ssl_cert_load_from_buffer(pool, &ca_buf, &null_buf, - &null_buf, &null_str, &cert); + &null_buf, &null_str, + &cli_cert); #endif if (status != PJ_SUCCESS) { goto on_return; } } +#if TEST_CLI_OWN_CERT + else { + /* A credential of its own, so each end's local certificate + * chain differs from the peer's in content as well as storage. + */ + pj_str_t ca_file = pj_str(CERT_CA_FILE); + pj_str_t cert_file = pj_str(CERT_CLI_FILE); + pj_str_t privkey_file = pj_str(CERT_CLI_PRIVKEY_FILE); + pj_str_t privkey_pass = pj_str(CERT_PRIVKEY_PASS); + pj_ssl_cert_t *own_cert = NULL; - status = pj_ssl_sock_set_certificate(ssock_cli, pool, cert); +#if (defined(TEST_LOAD_FROM_FILES) && TEST_LOAD_FROM_FILES==1) + status = pj_ssl_cert_load_from_files(pool, &ca_file, &cert_file, + &privkey_file, &privkey_pass, + &own_cert); +#else + pj_ssl_cert_buffer ca_buf, cert_buf, privkey_buf; + + status = load_cert_to_buf(pool, &ca_file, &ca_buf); + if (status == PJ_SUCCESS) + status = load_cert_to_buf(pool, &cert_file, &cert_buf); + if (status == PJ_SUCCESS) + status = load_cert_to_buf(pool, &privkey_file, &privkey_buf); + if (status == PJ_SUCCESS) { + status = pj_ssl_cert_load_from_buffer(pool, &ca_buf, &cert_buf, + &privkey_buf, + &privkey_pass, &own_cert); + } +#endif + if (status != PJ_SUCCESS) { + goto on_return; + } + + cli_cert = own_cert; + state_cli.cert_der = CERT_CLI_DER_FILE; + } +#endif + + status = pj_ssl_sock_set_certificate(ssock_cli, pool, cli_cert); if (status != PJ_SUCCESS) { goto on_return; } @@ -1042,6 +1097,11 @@ static int echo_test(pj_ssl_sock_proto srv_proto, pj_ssl_sock_proto cli_proto, if (ssock_serv) pj_ssl_sock_close(ssock_serv); + if (state_serv.accepted_st && !state_serv.accepted_st->err && + !state_serv.accepted_st->done) + { + pj_ssl_sock_close(state_serv.accepted_ssock); + } if (ssock_cli && !state_cli.err && !state_cli.done) pj_ssl_sock_close(ssock_cli); if (ioqueue) diff --git a/pjlib/src/pjlib-test/ssl_sock_test.h b/pjlib/src/pjlib-test/ssl_sock_test.h index a3bdad29a0..a43e30de98 100644 --- a/pjlib/src/pjlib-test/ssl_sock_test.h +++ b/pjlib/src/pjlib-test/ssl_sock_test.h @@ -19,6 +19,25 @@ #define CERT_PRIVKEY_FILE CERT_DIR "privkey.pem" #define CERT_PRIVKEY_PASS "privkeypass" +/* A second certificate, issued by the CA above, for the client to present + * when the server requires one. Having each end on its own certificate + * lets a test tell the local certificate chain from the peer's by content. + */ +#define CERT_CLI_FILE CERT_DIR "clicert.pem" +#define CERT_CLI_DER_FILE CERT_DIR "clicert.der" +#define CERT_CLI_PRIVKEY_FILE CERT_DIR "cliprivkey.pem" + +/* The Apple backends take the private key from the Keychain, where only the + * server key is stored, so there the client keeps reusing the server + * certificate. + */ +#if (PJ_SSL_SOCK_IMP == PJ_SSL_SOCK_IMP_DARWIN) || \ + (PJ_SSL_SOCK_IMP == PJ_SSL_SOCK_IMP_APPLE) +# define TEST_CLI_OWN_CERT 0 +#else +# define TEST_CLI_OWN_CERT 1 +#endif + #define TEST_LOAD_FROM_FILES 1