cron2 has uploaded a new patch set (#3) to the change originally created by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/1255?usp=email )
The following approvals got outdated and were removed: Code-Review+1 by ordex, Code-Review+2 by cron2 Change subject: PUSH_UPDATE server: check IV_PROTO before sending the message to the client ...................................................................... PUSH_UPDATE server: check IV_PROTO before sending the message to the client Before sending the PUSH_UPDATE message to the client, we must verify that the client has actually sent IV_PROTO_PUSH_UPDATE to the server, declaring that it supports push-updates. Also fixed a gc_arena memory leak in one of the error paths and asserted mi->context.c2.tls_multi . Change-Id: I7c28da72be11c7efbed3068fbfc65f2959227bec Signed-off-by: Marco Baffo <[email protected]> Acked-by: Gert Doering <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1255 Message-Id: <[email protected]> URL: https://sourceforge.net/p/openvpn/mailman/message/59244566/ Signed-off-by: Gert Doering <[email protected]> --- M src/openvpn/push_util.c M tests/unit_tests/openvpn/test_push_update_msg.c 2 files changed, 36 insertions(+), 8 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/55/1255/3 diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c index b475d2e..25c6ebe 100644 --- a/src/openvpn/push_util.c +++ b/src/openvpn/push_util.c @@ -7,6 +7,7 @@ #ifdef ENABLE_MANAGEMENT #include "multi.h" +#include "ssl_util.h" #endif #if defined(__GNUC__) || defined(__clang__) @@ -177,20 +178,32 @@ buf_string_compare_advance(&tmp_msg, push_update_cmd); if (process_incoming_push_update(c, pull_permission_mask(c), option_types_found, &tmp_msg, true) == PUSH_MSG_ERROR) { - msg(M_WARN, "Failed to process push update message sent to client ID: %u", - c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX); + msg(M_WARN, "Failed to process push update message sent to client ID: %u", c->c2.tls_multi->peer_id); continue; } c->options.push_option_types_found |= *option_types_found; if (!options_postprocess_pull(&c->options, c->c2.es)) { - msg(M_WARN, "Failed to post-process push update message sent to client ID: %u", - c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX); + msg(M_WARN, "Failed to post-process push update message sent to client ID: %u", c->c2.tls_multi->peer_id); } } return true; } +/* Return true if the client supports push-update */ +static bool +support_push_update(struct multi_instance *mi) +{ + ASSERT(mi->context.c2.tls_multi); + const unsigned int iv_proto_peer = extract_iv_proto(mi->context.c2.tls_multi->peer_info); + if (!(iv_proto_peer & IV_PROTO_PUSH_UPDATE)) + { + return false; + } + + return true; +} + int send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const int push_bundle_size) { @@ -231,9 +244,17 @@ if (!mi) { + gc_free(&gc); return -ENOENT; } + if (!support_push_update(mi)) + { + msg(M_CLIENT, "PUSH_UPDATE: not sending message to unsupported peer with ID: %u", mi->context.c2.tls_multi->peer_id); + gc_free(&gc); + return 0; + } + const char *old_ip = mi->context.options.ifconfig_local; const char *old_ipv6 = mi->context.options.ifconfig_ipv6_local; if (!mi->halt @@ -262,7 +283,7 @@ { struct multi_instance *curr_mi = he->value; - if (curr_mi->halt) + if (curr_mi->halt || !support_push_update(curr_mi)) { continue; } @@ -273,8 +294,7 @@ const char *old_ipv6 = curr_mi->context.options.ifconfig_ipv6_local; if (!send_single_push_update(&curr_mi->context, msgs, &option_types_found)) { - msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated", - curr_mi->context.c2.tls_multi ? curr_mi->context.c2.tls_multi->peer_id : UINT32_MAX); + msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated", curr_mi->context.c2.tls_multi->peer_id); continue; } if (option_types_found & OPT_P_UP) diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c index 6e49f14..60596ed 100644 --- a/tests/unit_tests/openvpn/test_push_update_msg.c +++ b/tests/unit_tests/openvpn/test_push_update_msg.c @@ -144,7 +144,13 @@ { return true; } -#endif /* ifndef ENABLE_MANAGEMENT */ + +unsigned int +extract_iv_proto(const char *peer_info) +{ + return IV_PROTO_PUSH_UPDATE; +} +#endif /* ifdef ENABLE_MANAGEMENT */ /* tests */ @@ -464,6 +470,7 @@ struct multi_context *m = calloc(1, sizeof(struct multi_context)); m->instances = calloc(1, sizeof(struct multi_instance *)); struct multi_instance *mi = calloc(1, sizeof(struct multi_instance)); + mi->context.c2.tls_multi = calloc(1, sizeof(struct tls_multi)); *(m->instances) = mi; m->top.options.disable_dco = true; *state = m; @@ -474,6 +481,7 @@ teardown2(void **state) { struct multi_context *m = *state; + free((*(m->instances))->context.c2.tls_multi); free(*(m->instances)); free(m->instances); free(m); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1255?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I7c28da72be11c7efbed3068fbfc65f2959227bec Gerrit-Change-Number: 1255 Gerrit-PatchSet: 3 Gerrit-Owner: mrbff <[email protected]> Gerrit-Reviewer: cron2 <[email protected]> Gerrit-Reviewer: flichtenheld <[email protected]> Gerrit-Reviewer: ordex <[email protected]> Gerrit-Reviewer: plaisthos <[email protected]> Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
