Attention is currently required from: Arne Schwabe, Frank Lichtenheld, Răzvan
Cojocaru.
Hello Frank Lichtenheld, Răzvan Cojocaru,
I'd like you to reexamine a change. Please visit
https://gerrit.openvpn.net/c/openvpn/+/2130?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review-1 by Răzvan Cojocaru
Change subject: Adjust ACC_MAX_MSG_LEN, fix ACC in p2p mode and improve
documentation
......................................................................
Adjust ACC_MAX_MSG_LEN, fix ACC in p2p mode and improve documentation
We do not need to limit the maximum ACC message length to just 1280. Also
improve the comments and documentation to be clearer.
Fix that the management notification about negotiated protocol is not sent
when in p2p mode (or if a server has push-peer-info in its configuration)
Change-Id: I3a268fc602e916bf54853c950761977cf3d7f835
Signed-off-by: Arne Schwabe <[email protected]>
---
M doc/management-notes.txt
M src/openvpn/acc.c
M src/openvpn/acc.h
M src/openvpn/ssl.c
M tests/unit_tests/openvpn/test_acc.c
5 files changed, 31 insertions(+), 13 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/30/2130/4
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 6c798c0..a98b45f 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -1398,11 +1398,17 @@
next message should be concatenated to this message to form a complete
message.
- Note that the OpenVPN implementation operates here with the robustness
- principle ("be conservative in what you send, be liberal in what you
- accept" - RFC 793 and others) and allows receiving messages longer than
- maximum allowed message length. Implementations should be prepared to
- handle messages that are longer than the maximum allowed message length.
+ Note that fragment is basically a flag for allowing peers communicating
+ over ACC to indicate if multiple ACC messages should be considered to
+ be a single message. If the ACC protocol is a stream based protocol
+ then all messages can be sent without fragment flag.
+
+ Note that the OpenVPN implementation allows receiving messages longer than
+ announced maximum allowed message length. Implementations must be prepared
+ to handle messages that are longer than the maximum allowed message length.
+ Overlong messages are considered invalid and indicate that the peer is
+ misbehaving. Disconnecting/dropping the peer is considered a valid
+ response to such messages.
OpenVPN does not verify that the message is valid base64. A peer that
sends invalid base64 payload will be passed to the management interface.
diff --git a/src/openvpn/acc.c b/src/openvpn/acc.c
index 455f56d..7b04821 100644
--- a/src/openvpn/acc.c
+++ b/src/openvpn/acc.c
@@ -190,10 +190,10 @@
}
/* We do not check the length here. We make no promises to support
- * anything larger than our announced message length but according to
- * the robustness principle
- * ("be conservative in what you send, be liberal in what you accept" -
RFC 793 and others)
- * we accept also longer messages */
+ * anything larger than our announced message length but we accept also
+ * longer messages if they manage to get through all the other layers
+ * that restrict the message size like TLS_CHANNEL_BUF_SIZE.
+ */
int payload_len;
const char *payload_len_str = buf_extract_field(&buf, ',', &gc);
diff --git a/src/openvpn/acc.h b/src/openvpn/acc.h
index 9c99b11..d640898 100644
--- a/src/openvpn/acc.h
+++ b/src/openvpn/acc.h
@@ -29,7 +29,9 @@
#include "openvpn.h"
#define ACC_MIN_MSG_LEN 64
-#define ACC_MAX_MSG_LEN 1280
+/* This is a conservative limit. The theoretical maximum
+ * is much closer to the TLS_CHANNEL_BUF_SIZE */
+#define ACC_MAX_MSG_LEN (TLS_CHANNEL_BUF_SIZE - 128)
/**
* This method parses an app custom control message and delivers it to the
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index b701c0a..4976646 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2396,6 +2396,12 @@
}
determine_common_acc_protocols(multi, session);
+ if (multi->opt.mode != MODE_SERVER)
+ {
+ /* This is required to support ACC in p2p mode or when the server
+ * sends peer-info */
+ notify_client_acc_protocols(multi);
+ }
if (username_len < 0 || password_len < 0)
{
diff --git a/tests/unit_tests/openvpn/test_acc.c
b/tests/unit_tests/openvpn/test_acc.c
index 9364d12..b9b1041 100644
--- a/tests/unit_tests/openvpn/test_acc.c
+++ b/tests/unit_tests/openvpn/test_acc.c
@@ -181,7 +181,11 @@
int max_len = 0;
const char *protocols = parse_acc_parameters(info_acc_msg, " ", &max_len,
true, &gc);
assert_string_equal(protocols, "flower:happy");
- assert_int_equal(max_len, 1280);
+
+ /* Check that the ACC_MAX_MSG_LEN is 1920 to ensure that the tests get
+ * adjusted if it changes */
+ assert_int_equal(ACC_MAX_MSG_LEN, 1920);
+ assert_int_equal(max_len, ACC_MAX_MSG_LEN);
/* a client does not accept a length longer than it supports */
protocols = parse_acc_parameters(info_acc_msg, " ", &max_len, false, &gc);
@@ -250,11 +254,11 @@
determine_common_acc_protocols(multi,
&c->c2.tls_multi->session[TM_ACTIVE]);
- assert_int_equal(multi->acc_max_message_length, ACC_MAX_MSG_LEN);
+ assert_int_equal(multi->acc_max_message_length, 1400);
assert_string_equal(multi->acc_negotiated_protocols, "power:flower");
assert_string_equal(env_set_get(multi->opt.es, "acc_protocols"),
"acc_protocols=power:flower");
- assert_string_equal(env_set_get(multi->opt.es, "acc_message_len"),
"acc_message_len=1280");
+ assert_string_equal(env_set_get(multi->opt.es, "acc_message_len"),
"acc_message_len=1400");
free(multi->acc_negotiated_protocols);
gc_free(&c->options.gc);
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2130?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3a268fc602e916bf54853c950761977cf3d7f835
Gerrit-Change-Number: 2130
Gerrit-PatchSet: 4
Gerrit-Owner: Arne Schwabe <[email protected]>
Gerrit-Reviewer: Frank Lichtenheld <[email protected]>
Gerrit-Reviewer: Răzvan Cojocaru <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Răzvan Cojocaru <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
Gerrit-Attention: Frank Lichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
