Attention is currently required from: flichtenheld, mrbff, ordex, plaisthos.
Hello flichtenheld, ordex, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1255?usp=email
to look at the new patch set (#2).
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]>
---
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/2
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
index f306104..3fa099c 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__)
@@ -174,20 +175,32 @@
buf_string_compare_advance(&msgs[i], push_update_cmd);
if (process_incoming_push_update(c, pull_permission_mask(c),
option_types_found, &msgs[i], 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)
{
@@ -228,9 +241,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
@@ -259,7 +280,7 @@
{
struct multi_instance *curr_mi = he->value;
- if (curr_mi->halt)
+ if (curr_mi->halt || !support_push_update(curr_mi))
{
continue;
}
@@ -270,8 +291,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: 2
Gerrit-Owner: mrbff <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: cron2 <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Attention: ordex <[email protected]>
Gerrit-Attention: mrbff <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
