Skip to content

Commit

Permalink
Fix ACL input attributes validation. (sonic-net#178)
Browse files Browse the repository at this point in the history 
- Move string to int converters to swss-common repo
- Use common converters in Everflow and ACL code
  • Loading branch information
@oleksandrivantsiv @marian-pritsak
oleksandrivantsiv authored and marian-pritsak committed Apr 4, 2017
1 parent c8a53f1 commit ee14e9e
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 103 deletions.
150 changes: 78 additions & 72 deletions orchagent/aclorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "logger.h"
#include "schema.h"
#include "ipprefix.h"
#include "converter.h"

using namespace std;
using namespace swss;
Expand Down Expand Up @@ -121,75 +122,75 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value)

sai_attribute_value_t value;

if (aclMatchLookup.find(attr_name) == aclMatchLookup.end())
try
{
return false;
}
else if(attr_name == MATCH_IP_TYPE)
{
if (!processIpType(attr_value, value.aclfield.data.u32))
if (aclMatchLookup.find(attr_name) == aclMatchLookup.end())
{
SWSS_LOG_ERROR("Invalid IP type %s", attr_value.c_str());
return false;
}
else if(attr_name == MATCH_IP_TYPE)
{
if (!processIpType(attr_value, value.aclfield.data.u32))
{
SWSS_LOG_ERROR("Invalid IP type %s", attr_value.c_str());
return false;
}

value.aclfield.mask.u32 = 0xFFFFFFFF;
}
else if(attr_name == MATCH_TCP_FLAGS)
{
vector<string> flagsData;
string flags, mask;
int val;
char *endp = NULL;
errno = 0;
value.aclfield.mask.u32 = 0xFFFFFFFF;
}
else if(attr_name == MATCH_TCP_FLAGS)
{
vector<string> flagsData;
string flags, mask;
int val;
char *endp = NULL;
errno = 0;

split(attr_value, flagsData, '/');
split(attr_value, flagsData, '/');

if (flagsData.size() != 2) // expect two parts flags and mask separated with '/'
{
SWSS_LOG_ERROR("Invalid TCP flags format %s", attr_value.c_str());
return false;
}
if (flagsData.size() != 2) // expect two parts flags and mask separated with '/'
{
SWSS_LOG_ERROR("Invalid TCP flags format %s", attr_value.c_str());
return false;
}

flags = trim(flagsData[0]);
mask = trim(flagsData[1]);
flags = trim(flagsData[0]);
mask = trim(flagsData[1]);

val = strtol(flags.c_str(), &endp, 0);
if (errno || (endp != flags.c_str() + flags.size()) ||
(val < 0) || (val > UCHAR_MAX))
{
SWSS_LOG_ERROR("TCP flags parse error, value: %s(=%d), errno: %d", flags.c_str(), val, errno);
return false;
}
value.aclfield.data.u8 = val;

val = strtol(flags.c_str(), &endp, 0);
if (errno || (endp != flags.c_str() + flags.size()) ||
(val < 0) || (val > UCHAR_MAX))
val = strtol(mask.c_str(), &endp, 0);
if (errno || (endp != mask.c_str() + mask.size()) ||
(val < 0) || (val > UCHAR_MAX))
{
SWSS_LOG_ERROR("TCP mask parse error, value: %s(=%d), errno: %d", mask.c_str(), val, errno);
return false;
}
value.aclfield.mask.u8 = val;
}
else if(attr_name == MATCH_ETHER_TYPE || attr_name == MATCH_L4_SRC_PORT || attr_name == MATCH_L4_DST_PORT)
{
SWSS_LOG_ERROR("TCP flags parse error, value: %s(=%d), errno: %d", flags.c_str(), val, errno);
return false;
value.aclfield.data.u16 = to_uint<uint16_t>(attr_value);
value.aclfield.mask.u16 = 0xFFFF;
}
value.aclfield.data.u8 = val;

val = strtol(mask.c_str(), &endp, 0);
if (errno || (endp != mask.c_str() + mask.size()) ||
(val < 0) || (val > UCHAR_MAX))
else if(attr_name == MATCH_DSCP)
{
SWSS_LOG_ERROR("TCP mask parse error, value: %s(=%d), errno: %d", mask.c_str(), val, errno);
return false;
value.aclfield.data.u8 = to_uint<uint8_t>(attr_value, 0, 63);
value.aclfield.mask.u8 = 0xFF;
}
value.aclfield.mask.u8 = val;
}
else if((attr_name == MATCH_ETHER_TYPE) || (attr_name == MATCH_DSCP) ||
(attr_name == MATCH_L4_SRC_PORT) || (attr_name == MATCH_L4_DST_PORT) ||
(attr_name == MATCH_IP_PROTOCOL))
{
char *endp = NULL;
errno = 0;
value.aclfield.data.u32 = strtol(attr_value.c_str(), &endp, 0);
if (errno || (endp != attr_value.c_str() + attr_value.size()))
else if(attr_name == MATCH_IP_PROTOCOL)
{
SWSS_LOG_ERROR("Integer parse error. Attribute: %s, value: %s(=%d), errno: %d", attr_name.c_str(), attr_value.c_str(), value.aclfield.data.u32, errno);
return false;
value.aclfield.data.u8 = to_uint<uint8_t>(attr_value);
value.aclfield.mask.u8 = 0xFF;
}

value.aclfield.mask.u32 = 0xFFFFFFFF;
}
else if (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP)
{
try
else if (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP)
{
IpPrefix ip(attr_value);

Expand All @@ -204,28 +205,33 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value)
memcpy(value.aclfield.mask.ip6, ip.getMask().getV6Addr(), 16);
}
}
catch(...)
else if ((attr_name == MATCH_L4_SRC_PORT_RANGE) || (attr_name == MATCH_L4_DST_PORT_RANGE))
{
SWSS_LOG_ERROR("IpPrefix exception. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str());
return false;
if (sscanf(attr_value.c_str(), "%d-%d", &value.u32range.min, &value.u32range.max) != 2)
{
SWSS_LOG_ERROR("Range parse error. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str());
return false;
}

// check boundaries
if ((value.u32range.min > USHRT_MAX) ||
(value.u32range.max > USHRT_MAX) ||
(value.u32range.min > value.u32range.max))
{
SWSS_LOG_ERROR("Range parse error. Invalid range value. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str());
return false;
}
}
}
else if ((attr_name == MATCH_L4_SRC_PORT_RANGE) || (attr_name == MATCH_L4_DST_PORT_RANGE))
catch (exception &e)
{
if (sscanf(attr_value.c_str(), "%d-%d", &value.u32range.min, &value.u32range.max) != 2)
{
SWSS_LOG_ERROR("Range parse error. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str());
return false;
}

// check boundaries
if ((value.u32range.min > USHRT_MAX) ||
(value.u32range.max > USHRT_MAX) ||
(value.u32range.min > value.u32range.max))
{
SWSS_LOG_ERROR("Range parse error. Invalid range value. Attribute: %s, value: %s", attr_name.c_str(), attr_value.c_str());
return false;
}
SWSS_LOG_ERROR("Failed to parse %s attribute %s value. Error: %s", attr_name.c_str(), attr_value.c_str(), e.what());
return false;
}
catch (...)
{
SWSS_LOG_ERROR("Failed to parse %s attribute %s value.", attr_name.c_str(), attr_value.c_str());
return false;
}

value.aclfield.enable = true;
Expand Down
36 changes: 5 additions & 31 deletions orchagent/mirrororch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "logger.h"
#include "swssnet.h"
#include "converter.h"
#include "mirrororch.h"

#define MIRROR_SESSION_STATUS "status"
Expand All @@ -28,33 +29,6 @@ extern sai_mirror_api_t *sai_mirror_api;

using namespace std::rel_ops;

static inline uint64_t to_uint64(string str, uint64_t min = numeric_limits<uint64_t>::min(), uint64_t max = numeric_limits<uint64_t>::max())
{
size_t idx = 0;
uint64_t ret = stoul(str, &idx, 0);
if (str[idx])
{
throw invalid_argument("failed to convert " + str + " value to uint64_t type");
}

if (ret < min || ret > max)
{
throw invalid_argument("failed to convert " + str + " value is not in range " + to_string(min) + " - " + to_string(max));
}

return ret;
}

static inline uint16_t to_uint16(string str, uint16_t min = numeric_limits<uint16_t>::min(), uint16_t max = numeric_limits<uint16_t>::max())
{
return static_cast<uint16_t>(to_uint64(str, min, max));
}

static inline uint8_t to_uint8(string str, uint8_t min = numeric_limits<uint8_t>::min(), uint8_t max = numeric_limits<uint8_t>::max())
{
return static_cast<uint8_t>(to_uint64(str, min, max));
}

MirrorOrch::MirrorOrch(DBConnector *db, string tableName,
PortsOrch *portOrch, RouteOrch *routeOrch, NeighOrch *neighOrch, FdbOrch *fdbOrch) :
Orch(db, tableName),
Expand Down Expand Up @@ -212,19 +186,19 @@ void MirrorOrch::createEntry(const string& key, const vector<FieldValueTuple>& d
}
else if (fvField(i) == MIRROR_SESSION_GRE_TYPE)
{
entry.greType = to_uint16(fvValue(i));
entry.greType = to_uint<uint16_t>(fvValue(i));
}
else if (fvField(i) == MIRROR_SESSION_DSCP)
{
entry.dscp = to_uint8(fvValue(i), MIRROR_SESSION_DSCP_MIN, MIRROR_SESSION_DSCP_MAX);
entry.dscp = to_uint<uint8_t>(fvValue(i), MIRROR_SESSION_DSCP_MIN, MIRROR_SESSION_DSCP_MAX);
}
else if (fvField(i) == MIRROR_SESSION_TTL)
{
entry.ttl = to_uint8(fvValue(i));
entry.ttl = to_uint<uint8_t>(fvValue(i));
}
else if (fvField(i) == MIRROR_SESSION_QUEUE)
{
entry.queue = to_uint8(fvValue(i));
entry.queue = to_uint<uint8_t>(fvValue(i));
}
else if (fvField(i) == MIRROR_SESSION_STATUS)
{
Expand Down

0 comments on commit ee14e9e

Please sign in to comment.