plaisthos has uploaded this change for review. ( http://gerrit.openvpn.net/c/openvpn/+/1800?usp=email )
Change subject: Allow extracing other integers from peer info and add a unit test for it ...................................................................... Allow extracing other integers from peer info and add a unit test for it This also ensure that we only really extract the variable we are looking for. The old code would also consider UV_IV_NCP to be IV_NCP. Change-Id: Iad94e7a9d0b3be8a8db09e9a20eaac6041470ab6 Signed-off-by: Arne Schwabe <[email protected]> --- M CMakeLists.txt M src/openvpn/push.c M src/openvpn/ssl_ncp.c M src/openvpn/ssl_util.c M src/openvpn/ssl_util.h M src/openvpn/ssl_verify.c M tests/unit_tests/openvpn/test_push_update_msg.c M tests/unit_tests/openvpn/test_ssl.c 8 files changed, 124 insertions(+), 41 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/00/1800/1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 7473f15..b886414 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -906,6 +906,7 @@ tests/unit_tests/openvpn/mock_msg.c tests/unit_tests/openvpn/mock_get_random.c src/openvpn/options_util.c + src/openvpn/ssl_util.c src/openvpn/otime.c src/openvpn/list.c ) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 633c007d..88b499f 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -718,7 +718,7 @@ /* Push our mtu to the peer if it supports pushable MTUs */ int client_max_mtu = 0; - const char *iv_mtu = extract_var_peer_info(tls_multi->peer_info, "IV_MTU=", gc); + const char *iv_mtu = extract_var_peer_info(tls_multi->peer_info, "IV_MTU", gc); if (iv_mtu && sscanf(iv_mtu, "%d", &client_max_mtu) == 1) { diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index b450de8..8148396 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -55,20 +55,10 @@ * Return the Negotiable Crypto Parameters version advertised in the peer info * string, or 0 if none specified. */ -static int +static unsigned int tls_peer_info_ncp_ver(const char *peer_info) { - const char *ncpstr = peer_info ? strstr(peer_info, "IV_NCP=") : NULL; - if (ncpstr) - { - int ncp = 0; - int r = sscanf(ncpstr, "IV_NCP=%d", &ncp); - if (r == 1) - { - return ncp; - } - } - return 0; + return peer_info_extract_uint(peer_info, "IV_NCP", 0); } /** @@ -227,7 +217,7 @@ tls_peer_ncp_list(const char *peer_info, struct gc_arena *gc) { /* Check if the peer sends the IV_CIPHERS list */ - const char *iv_ciphers = extract_var_peer_info(peer_info, "IV_CIPHERS=", gc); + const char *iv_ciphers = extract_var_peer_info(peer_info, "IV_CIPHERS", gc); if (iv_ciphers) { return iv_ciphers; @@ -358,7 +348,7 @@ { /* we use a local gc arena to keep the temporary strings needed by strsep */ struct gc_arena gc_local = gc_new(); - const char *peer_ciphers = extract_var_peer_info(peer_info, "IV_CIPHERS=", &gc_local); + const char *peer_ciphers = extract_var_peer_info(peer_info, "IV_CIPHERS", &gc_local); if (!peer_ciphers) { diff --git a/src/openvpn/ssl_util.c b/src/openvpn/ssl_util.c index 2f01f8a..a4c2910 100644 --- a/src/openvpn/ssl_util.c +++ b/src/openvpn/ssl_util.c @@ -27,6 +27,32 @@ #include "openvpn.h" #include "ssl_util.h" +static const char * +peer_info_extract_varstr(const char *peer_info, const char *field) +{ + char buf[32]; + snprintf(buf, sizeof(buf), "%s=", field); + + const char *optstr = NULL; + + /* peer info starts with this field */ + if (peer_info && strncmp(peer_info, buf, strlen(buf)) == 0) + { + optstr = peer_info; + } + else + { + snprintf(buf, sizeof(buf), "\n%s=", field); + optstr = peer_info ? strstr(peer_info, buf) : NULL; + if (optstr) + { + /* skip the \n */ + optstr++; + } + } + return optstr; +} + char * extract_var_peer_info(const char *peer_info, const char *var, struct gc_arena *gc) { @@ -35,14 +61,15 @@ return NULL; } - const char *var_start = strstr(peer_info, var); + const char *var_start = peer_info_extract_varstr(peer_info, var); if (!var_start) { /* variable not found in peer info */ return NULL; } - var_start += strlen(var); + /* one extra byte forward for the = */ + var_start += strlen(var) + 1; const char *var_end = strstr(var_start, "\n"); if (!var_end) { @@ -56,36 +83,35 @@ return var_value; } + unsigned int -extract_iv_proto(const char *peer_info) +peer_info_extract_uint(const char *peer_info, const char *field, unsigned int default_value) { - const char *optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL; + const char *optstr = peer_info_extract_varstr(peer_info, field); if (optstr) { + char buf[32]; + snprintf(buf, sizeof(buf), "%s=%%d", field); int proto = 0; - int r = sscanf(optstr, "IV_PROTO=%d", &proto); + int r = sscanf(optstr, buf, &proto); if (r == 1 && proto > 0) { return proto; } } - return 0; + return default_value; } uint32_t extract_asymmetric_peer_id(const char *peer_info) { - for (const char *p = peer_info; p && (p = strstr(p, "ID=")); p += 3) + const char *optstr = peer_info_extract_varstr(peer_info, "ID"); + if (optstr) { - /* only accept "ID=" at the start of a line, so it does not match - * substrings like "UV_ID=" or "GUID=" */ - if (p == peer_info || p[-1] == '\n') + uint32_t peer_id = 0; + if (sscanf(optstr, "ID=%x", &peer_id) == 1 && peer_id < MAX_PEER_ID) { - uint32_t peer_id = 0; - if (sscanf(p, "ID=%x", &peer_id) == 1 && peer_id < MAX_PEER_ID) - { - return peer_id; - } + return peer_id; } } return MAX_PEER_ID; diff --git a/src/openvpn/ssl_util.h b/src/openvpn/ssl_util.h index 1a69e6f..49faefa 100644 --- a/src/openvpn/ssl_util.h +++ b/src/openvpn/ssl_util.h @@ -33,11 +33,22 @@ #include "buffer.h" /** + * Extracts the named variable and returns its value or default_value + * if it cannot be extracted. + * + * @param peer_info peer info string to search in + * @parm field name of the field to be extracted + * @parm default_value value to return if not found or cannot be extracted + */ +unsigned int +peer_info_extract_uint(const char *peer_info, const char *field, unsigned int default_value); + +/** * Extracts a variable from peer info, the returned string will be allocated * using the supplied gc_arena * * @param peer_info The peer's peer_info - * @param var The variable *including* =, e.g. IV_CIPHERS= + * @param var The variable *excluding* =, e.g. IV_CIPHERS * @param gc GC arena to allocate return value in * * @return The content of the variable as NULL terminated string or NULL if the @@ -51,8 +62,11 @@ * * @param peer_info peer info string to search for IV_PROTO */ -unsigned int extract_iv_proto(const char *peer_info); - +static inline unsigned int +extract_iv_proto(const char *peer_info) +{ + return peer_info_extract_uint(peer_info, "IV_PROTO", 0); +} /** * Extracts the ID variable and returns its value or diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 162f68f..3317380 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -861,7 +861,7 @@ { struct gc_arena gc = gc_new(); - char *iv_sso = extract_var_peer_info(peer_info, "IV_SSO=", &gc); + char *iv_sso = extract_var_peer_info(peer_info, "IV_SSO", &gc); if (!iv_sso) { gc_free(&gc); diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c index 219b476..8f42483 100644 --- a/tests/unit_tests/openvpn/test_push_update_msg.c +++ b/tests/unit_tests/openvpn/test_push_update_msg.c @@ -188,12 +188,6 @@ { return true; } - -unsigned int -extract_iv_proto(const char *peer_info) -{ - return IV_PROTO_PUSH_UPDATE; -} #endif /* ifdef ENABLE_MANAGEMENT */ /* tests */ diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c index 0e9cecf..9c2a99d 100644 --- a/tests/unit_tests/openvpn/test_ssl.c +++ b/tests/unit_tests/openvpn/test_ssl.c @@ -48,7 +48,9 @@ #include "buffer.h" #include "cert_data.h" #include "packet_id.h" +#include "ssl_util.h" #include "ssl_verify.h" +#include "openvpn.h" /* Mock function to be allowed to include win32.c which is required for * getting the temp directory */ @@ -829,6 +831,61 @@ free_certificate(cert); } +void +ssl_test_extract_peer_info(void **state) +{ + const char *peer_info_normal = + "IV_VER=2.6_git\nIV_PLAT=mac\nIV_TCPNL=1\nIV_NCP=2\n" + "IV_CIPHERS=AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305\n" + "IV_PROTO=94\nIV_LZO_STUB=1\nIV_COMP_STUB=1\nP=78\nIV_COMP_STUBv2=1\n" + "IV_SSL=OpenSSL_3.0.5_5_Jul_2022\nIV_LZ4v2=1"; + + const char *empty = ""; + const char *invalid_proto = "IV_PROTO=seven\nIV_SSL=7\nP=300\nID=xyz"; + const char *test_prefix = "UV_IV_PROTO=773\nNP=112\nPD=8\n"; + + + assert_int_equal(extract_iv_proto(peer_info_normal), 94); + assert_int_equal(extract_iv_proto(empty), 0); + assert_int_equal(extract_iv_proto(invalid_proto), 0); + /* This should not pick up the UV_IV_PROTO that has the extra prefix */ + assert_int_equal(extract_iv_proto(test_prefix), 0); + + assert_int_equal(peer_info_extract_uint(peer_info_normal, "IV_COMP_STUB", 0), 1); + assert_int_equal(peer_info_extract_uint(empty, "IV_COMP_STUB", 0xfe0d0d), 0xfe0d0d); + assert_int_equal(peer_info_extract_uint(invalid_proto, "IV_COMP_STUB", 5), 5); + + assert_int_equal(peer_info_extract_uint(test_prefix, "NP", 23), 112); + assert_int_equal(peer_info_extract_uint(test_prefix, "PD", 23), 8); + assert_int_equal(peer_info_extract_uint(test_prefix, "P", 0xfe0d0d), 0xfe0d0d); + assert_int_equal(peer_info_extract_uint(peer_info_normal, "P", 23), 78); + assert_int_equal(peer_info_extract_uint(test_prefix, "UV_IV_PROTO", 23), 773); + + struct gc_arena gc = gc_new(); + + const char *peer_ciphers = extract_var_peer_info(peer_info_normal, "IV_CIPHERS", &gc); + assert_string_equal(peer_ciphers, "AES-256-GCM:AES-128-GCM:CHACHA20-POLY1305"); + + /* with the extra = this should not extract anything */ + const char *proto = extract_var_peer_info(peer_info_normal, "IV_PROTO=", &gc); + assert_null(proto); + + const char *double_eq_info = "FOO=7\nFOO==new"; + const char *foo_eq = extract_var_peer_info(double_eq_info, "FOO=", &gc); + const char *foo = extract_var_peer_info(double_eq_info, "FOO", &gc); + assert_string_equal(foo, "7"); + assert_string_equal(foo_eq, "new"); + + assert_int_equal(extract_asymmetric_peer_id(double_eq_info), MAX_PEER_ID); + assert_int_equal(extract_asymmetric_peer_id(peer_info_normal), MAX_PEER_ID); + assert_int_equal(extract_asymmetric_peer_id(invalid_proto), MAX_PEER_ID); + assert_int_equal(extract_asymmetric_peer_id("ID=f7"), 0xf7); + assert_int_equal(extract_asymmetric_peer_id("X=foo\nID=12ab"), 0x12ab); + assert_int_equal(extract_asymmetric_peer_id("X=foo\nID=34dd\nY=bar"), 0x34dd); + assert_int_equal(extract_asymmetric_peer_id("X=foo\nID=12345678"), MAX_PEER_ID); + + gc_free(&gc); +} int main(void) @@ -853,7 +910,8 @@ cmocka_unit_test(test_data_channel_roundtrip_bf_cbc), cmocka_unit_test(test_data_channel_known_vectors_epoch), cmocka_unit_test(test_data_channel_known_vectors_shortpktid), - cmocka_unit_test(crypto_test_print_cert_details) + cmocka_unit_test(crypto_test_print_cert_details), + cmocka_unit_test(ssl_test_extract_peer_info) }; -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1800?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iad94e7a9d0b3be8a8db09e9a20eaac6041470ab6 Gerrit-Change-Number: 1800 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <[email protected]> Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
