Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1153?usp=email
to review the following change.
Change subject: options: Review use of positive_atoi vs atoi_constrained
......................................................................
options: Review use of positive_atoi vs atoi_constrained
Replace where it is useful.
Change-Id: Id440917f433aab1a7db608ba04fa95ba47c2ddde
Signed-off-by: Frank Lichtenheld <fr...@lichtenheld.com>
---
M src/openvpn/options.c
1 file changed, 21 insertions(+), 31 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/53/1153/1
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8edf261..7c685e2 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7007,21 +7007,15 @@
else if (streq(p[0], "tun-mtu-max") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
- int max_mtu = positive_atoi(p[1], msglevel);
- if (max_mtu < 68 || max_mtu > 65536)
- {
- msg(msglevel, "--tun-mtu-max value '%s' is invalid", p[1]);
- }
- else
- {
- options->ce.tun_mtu_max = max_mtu;
- }
+ atoi_constrained(p[1], &options->ce.tun_mtu_max, p[0],
TUN_MTU_MAX_MIN, 65536, msglevel);
}
else if (streq(p[0], "tun-mtu-extra") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
- options->ce.tun_mtu_extra = positive_atoi(p[1], msglevel);
- options->ce.tun_mtu_extra_defined = true;
+ if (atoi_constrained(p[1], &options->ce.tun_mtu_extra, p[0], 0, 65536,
msglevel))
+ {
+ options->ce.tun_mtu_extra_defined = true;
+ }
}
else if (streq(p[0], "max-packet-size") && p[1] && !p[2])
{
@@ -7053,11 +7047,8 @@
else if (streq(p[0], "fragment") && p[1] && !p[3])
{
VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
- options->ce.fragment = positive_atoi(p[1], msglevel);
-
- if (options->ce.fragment < 68)
+ if (!atoi_constrained(p[1], &options->ce.fragment, p[0], 68, 0,
msglevel))
{
- msg(msglevel, "--fragment needs to be at least 68");
goto err;
}
@@ -7723,12 +7714,14 @@
VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_CONNECTION);
if (p[1])
{
- int mssfix = positive_atoi(p[1], msglevel);
- /* can be 0, but otherwise it needs to be high enough so we can
- * substract room for headers. */
- if (mssfix != 0 && (mssfix < TLS_CHANNEL_MTU_MIN || mssfix >
UINT16_MAX))
+ int mssfix;
+ if (!atoi_constrained(p[1], &mssfix, p[0], 0, UINT16_MAX,
msglevel))
{
- msg(msglevel, "--mssfix value '%s' is invalid", p[1]);
+ goto err;
+ }
+ if (mssfix != 0 && mssfix < TLS_CHANNEL_MTU_MIN)
+ {
+ msg(msglevel, "mssfix needs to be >= %d, not %d",
TLS_CHANNEL_MTU_MIN, mssfix);
goto err;
}
@@ -7981,7 +7974,7 @@
else if (streq(p[0], "max-routes-per-client") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_INHERIT);
- options->max_routes_per_client = max_int(positive_atoi(p[1],
msglevel), 1);
+ atoi_constrained(p[1], &options->max_routes_per_client, p[0], 1, 0,
msglevel);
}
else if (streq(p[0], "client-cert-not-required") && !p[1])
{
@@ -9795,8 +9788,6 @@
}
else if (streq(p[0], "keying-material-exporter") && p[1] && p[2])
{
- int ekm_length = positive_atoi(p[2], msglevel);
-
VERIFY_PERMISSION(OPT_P_GENERAL);
if (strncmp(p[1], "EXPORTER", 8))
@@ -9810,14 +9801,14 @@
msg(msglevel,
"Keying material exporter label must not be '"
EXPORT_KEY_DATA_LABEL "'.");
}
- if (ekm_length < 16 || ekm_length > 4095)
+
+ if (!atoi_constrained(p[2], &options->keying_material_exporter_length,
+ p[0], 16, 4095, msglevel))
{
- msg(msglevel, "Invalid keying material exporter length");
goto err;
}
options->keying_material_exporter_label = p[1];
- options->keying_material_exporter_length = ekm_length;
}
else if (streq(p[0], "allow-recursive-routing") && !p[1])
{
@@ -9852,15 +9843,14 @@
}
else if (streq(p[0], "vlan-pvid") && p[1] && !p[2])
{
+ int vlan_pvid;
VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_INSTANCE);
- options->vlan_pvid = positive_atoi(p[1], msglevel);
- if (options->vlan_pvid < OPENVPN_8021Q_MIN_VID
- || options->vlan_pvid > OPENVPN_8021Q_MAX_VID)
+ if (!atoi_constrained(p[1], &vlan_pvid, p[0],
+ OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID,
msglevel))
{
- msg(msglevel, "the parameter of --vlan-pvid parameters must be >=
%u and <= %u",
- OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID);
goto err;
}
+ options->vlan_pvid = (uint16_t)vlan_pvid;
}
else
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1153?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: Id440917f433aab1a7db608ba04fa95ba47c2ddde
Gerrit-Change-Number: 1153
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
