Attention is currently required from: cron2, plaisthos.
Hello cron2,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/268?usp=email
to review the following change.
Change subject: Change type of frame.mss_fix to uint16_t
......................................................................
Change type of frame.mss_fix to uint16_t
Since in the end this always ends up as an uint16_t
anyway, just make the conversion much earlier. Cleans
up the code and removes some -Wconversion warnings.
v2:
- proper error handling in options.c
Change-Id: Id8321dfbb8ad8d79f4bb2a9da61f8cd6b6c6ee26
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/mss.c
M src/openvpn/mss.h
M src/openvpn/mtu.c
M src/openvpn/mtu.h
M src/openvpn/options.c
5 files changed, 23 insertions(+), 15 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/268/3
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index dbd3681..d7ee4c2 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -44,7 +44,7 @@
* if yes, hand to mss_fixup_dowork()
*/
void
-mss_fixup_ipv4(struct buffer *buf, int maxmss)
+mss_fixup_ipv4(struct buffer *buf, uint16_t maxmss)
{
const struct openvpn_iphdr *pip;
int hlen;
@@ -72,7 +72,7 @@
struct openvpn_tcphdr *tc = (struct openvpn_tcphdr *)
BPTR(&newbuf);
if (tc->flags & OPENVPN_TCPH_SYN_MASK)
{
- mss_fixup_dowork(&newbuf, (uint16_t) maxmss);
+ mss_fixup_dowork(&newbuf, maxmss);
}
}
}
@@ -84,7 +84,7 @@
* (IPv6 header structure is sufficiently different from IPv4...)
*/
void
-mss_fixup_ipv6(struct buffer *buf, int maxmss)
+mss_fixup_ipv6(struct buffer *buf, uint16_t maxmss)
{
const struct openvpn_ipv6hdr *pip6;
struct buffer newbuf;
@@ -130,7 +130,7 @@
struct openvpn_tcphdr *tc = (struct openvpn_tcphdr *) BPTR(&newbuf);
if (tc->flags & OPENVPN_TCPH_SYN_MASK)
{
- mss_fixup_dowork(&newbuf, (uint16_t) maxmss-20);
+ mss_fixup_dowork(&newbuf, maxmss-20);
}
}
}
@@ -191,13 +191,14 @@
{
continue;
}
- mssval = (opt[2]<<8)+opt[3];
+ mssval = opt[2] << 8;
+ mssval += opt[3];
if (mssval > maxmss)
{
- dmsg(D_MSS, "MSS: %d -> %d", (int) mssval, (int) maxmss);
+ dmsg(D_MSS, "MSS: %" PRIu16 " -> %" PRIu16, mssval,
maxmss);
accumulate = htons(mssval);
- opt[2] = (maxmss>>8)&0xff;
- opt[3] = maxmss&0xff;
+ opt[2] = (uint8_t)((maxmss>>8)&0xff);
+ opt[3] = (uint8_t)(maxmss&0xff);
accumulate -= htons(maxmss);
ADJUST_CHECKSUM(accumulate, tc->check);
}
@@ -291,7 +292,7 @@
{
/* we subtract IPv4 and TCP overhead here, mssfix method will add the
* extra 20 for IPv6 */
- frame->mss_fix = options->ce.mssfix - (20 + 20);
+ frame->mss_fix = (uint16_t)(options->ce.mssfix - (20 + 20));
return;
}
@@ -325,7 +326,7 @@
/* This is the target value our payload needs to be smaller */
unsigned int target = options->ce.mssfix - overhead;
- frame->mss_fix = adjust_payload_max_cbc(kt, target) - payload_overhead;
+ frame->mss_fix = (uint16_t)(adjust_payload_max_cbc(kt, target) -
payload_overhead);
}
diff --git a/src/openvpn/mss.h b/src/openvpn/mss.h
index 1c4704b..b2a68cf 100644
--- a/src/openvpn/mss.h
+++ b/src/openvpn/mss.h
@@ -29,9 +29,9 @@
#include "mtu.h"
#include "ssl_common.h"
-void mss_fixup_ipv4(struct buffer *buf, int maxmss);
+void mss_fixup_ipv4(struct buffer *buf, uint16_t maxmss);
-void mss_fixup_ipv6(struct buffer *buf, int maxmss);
+void mss_fixup_ipv6(struct buffer *buf, uint16_t maxmss);
void mss_fixup_dowork(struct buffer *buf, uint16_t maxmss);
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index 132f93c..389d140 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -210,7 +210,7 @@
buf_printf(&out, "%s ", prefix);
}
buf_printf(&out, "[");
- buf_printf(&out, " mss_fix:%d", frame->mss_fix);
+ buf_printf(&out, " mss_fix:%" PRIu16, frame->mss_fix);
#ifdef ENABLE_FRAGMENT
buf_printf(&out, " max_frag:%d", frame->max_fragment_size);
#endif
diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h
index b602b86..c64398d 100644
--- a/src/openvpn/mtu.h
+++ b/src/openvpn/mtu.h
@@ -115,7 +115,7 @@
* decryption/encryption or compression. */
} buf;
- unsigned int mss_fix; /**< The actual MSS value that should be
+ uint16_t mss_fix; /**< The actual MSS value that should be
* written to the payload packets. This
* is the value for IPv4 TCP packets. For
* IPv6 packets another 20 bytes must
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 90d85be..bdef703 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7253,9 +7253,16 @@
VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
if (p[1])
{
+ int mssfix = positive_atoi(p[1]);
+ if (mssfix > UINT16_MAX)
+ {
+ msg(msglevel, "--mssfix value '%s' is invalid", p[1]);
+ goto err;
+ }
+
/* value specified, assume encapsulation is not
* included unless "mtu" follows later */
- options->ce.mssfix = positive_atoi(p[1]);
+ options->ce.mssfix = mssfix;
options->ce.mssfix_encap = false;
options->ce.mssfix_default = false;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/268?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id8321dfbb8ad8d79f4bb2a9da61f8cd6b6c6ee26
Gerrit-Change-Number: 268
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: cron2
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: cron2
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
