Skip to content

Commit

Permalink
B #6759: Add extra checks for ports in SecurityGroup validation (#3319)
Browse files Browse the repository at this point in the history 
Signed-off-by: Valentyn Bohdan <[email protected]>
  • Loading branch information
@vvbohdan
vvbohdan authored Dec 11, 2024
1 parent 146dfe6 commit 5aebf69
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 62 deletions.
48 changes: 43 additions & 5 deletions include/NebulaUtil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <set>
#include <algorithm>
#include <random>
#include <regex>
#include <mutex>

#include <openssl/crypto.h>
Expand Down Expand Up @@ -141,12 +142,8 @@ namespace one_util
*
* @param st string to split
* @param delim delimiter character
* @param clean_empty true to clean empty split parts.
* Example for st "a::b:c"
* clean_empty true will return ["a", "b", "c"]
* clean_empty fase will return ["a", "", "b", "c"]
* @param parts where the result will be saved
*
* @return a vector containing the resulting substrings
*/
template <class T>
void split(const std::string &st, char delim, std::vector<T> &parts)
Expand Down Expand Up @@ -176,6 +173,19 @@ namespace one_util
}
}

/**
* Splits a string, using the given delimiter
*
* @param st string to split
* @param delim delimiter character
* @param clean_empty true to clean empty split parts.
* Example for st "a::b:c"
* clean_empty true will return ["a", "b", "c"]
* clean_empty fase will return ["a", "", "b", "c"]
*
* @return a vector containing the resulting substrings
*/

std::vector<std::string> split(const std::string& st, char delim,
bool clean_empty = true);

Expand Down Expand Up @@ -364,6 +374,34 @@ namespace one_util
template <>
bool str_cast(const std::string& str, std::string& value);

/**
* Converts string into unsigned integer type
* @param str Input string
*
* @return Unsigned integer value on success, 0 on failure.
* @note If value in string is greater than typename T can hold,
* maximum possible value will be returned
*/
template <typename T>
T string_to_unsigned(const std::string& str)
{
T value;

if (std::regex_search(str, std::regex("[^0-9]")))
{
return 0;
}

std::istringstream iss(str);
iss >> value;

if (iss.fail() || !iss.eof())
{
value = std::numeric_limits<T>::max();
}

return value;
}
} // namespace one_util

#endif /* _NEBULA_UTIL_H_ */
96 changes: 63 additions & 33 deletions src/secgroup/SecurityGroup.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ int SecurityGroup::insert(SqlDB *db, string& error_str)

if (name.empty())
{
goto error_name;
error_str = "No NAME in template for Security Group.";
return -1;
}

get_template_attribute("RULE", rules);
Expand All @@ -72,27 +73,18 @@ int SecurityGroup::insert(SqlDB *db, string& error_str)
{
if (!is_valid(rule, error_str))
{
goto error_valid;
return -1;
}
}

remove_duplicates(rules);

if ( insert_replace(db, false, error_str) != 0 )
{
goto error_db;
return -1;
}

return 0;

error_name:
error_str = "No NAME in template for Security Group.";
goto error_common;

error_valid:
error_db:
error_common:
return -1;
}

/* -------------------------------------------------------------------------- */
Expand Down Expand Up @@ -302,18 +294,12 @@ void SecurityGroup::get_rules(vector<VectorAttribute*>& result) const

bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const
{
string value, ip, proto;

unsigned int ivalue;

int id;

// -------------------------------------------------------------------------
// Check PROTOCOL and extensions
// - RANGE for TCP and UDP
// - ICMP_TYPE for ICMP
// -------------------------------------------------------------------------
proto = rule->vector_value("PROTOCOL");
auto proto = rule->vector_value("PROTOCOL");

one_util::toupper(proto);

Expand All @@ -325,7 +311,7 @@ bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const
return false;
}

value = rule->vector_value("RANGE");
auto value = rule->vector_value("RANGE");

if (!value.empty() && proto != "TCP" && proto != "UDP")
{
Expand All @@ -335,24 +321,64 @@ bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const

if (!value.empty())
{
const char *range_pattern = "^(([[:digit:]]+|[[:digit:]]+:[[:digit:]]+),)*([[:digit:]]+|[[:digit:]]+:[[:digit:]]+)$";
if (one_util::regex_match(range_pattern, value.c_str()) != 0)
constexpr auto ports_list_pattern =
"^(([[:digit:]]+|[[:digit:]]+:[[:digit:]]+),)*([[:digit:]]+|[[:digit:]]+:[[:digit:]]+)$";

if (one_util::regex_match(ports_list_pattern, value.c_str()) != 0)
{
error = "Invalid RANGE specification.";
return false;
}

// Check all port numbers are between 0-65535
const char *big_port_pattern = "([1-9][[:digit:]]{5,}|"
"[7-9][[:digit:]]{4,}|"
"6[6-9][[:digit:]]{3,}|"
"65[6-9][[:digit:]]{2,}|"
"655[4-9][[:digit:]]|"
"6553[6-9])";
if (one_util::regex_match(big_port_pattern, value.c_str()) == 0)
std::size_t port_counter = 0;

const auto port_rules = one_util::split(value, ',');

for (const auto& port_rule : port_rules)
{
error = "RANGE out of bounds 0-65536.";
return false;
constexpr auto range_pattern = "([[:digit:]]+:[[:digit:]]+)";

if (one_util::regex_match(range_pattern, port_rule.c_str()) == 0)
{
const auto port_values = one_util::split(port_rule, ':');
if (port_values.size() != 2)
{
error = "Invalid RANGE specification.";
return false;
}

const auto end_value = one_util::string_to_unsigned<std::uint32_t>(port_values[1]);

if (end_value > 65535)
{
error = "RANGE out of bounds 0-65536.";
return false;
}

if (end_value < one_util::string_to_unsigned<std::uint32_t>(port_values[0]))
{
error = "RANGE BEGIN value is greater than RANGE END value";
return false;
}

port_counter += 2;
}
else
{
if (one_util::string_to_unsigned<std::uint32_t>(port_rule) > 65535)
{
error = "RANGE out of bounds 0-65536.";
return false;
}

++port_counter;
}

if (port_counter > 15)
{
error = "Not more than 15 ports can be specified in a RULE";
return false;
}
}
}

Expand All @@ -366,6 +392,7 @@ bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const
return false;
}

unsigned int ivalue = 0;
if (rule->vector_value("ICMP_TYPE", ivalue) != 0)
{
error = "Wrong ICMP_TYPE, it must be integer";
Expand All @@ -383,6 +410,7 @@ bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const
return false;
}

unsigned int ivalue = 0;
if (rule->vector_value("ICMPV6_TYPE", ivalue) != 0)
{
error = "Wrong ICMPV6_TYPE, it must be integer";
Expand All @@ -408,12 +436,13 @@ bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const
// Check IP, SIZE and NETWORK_ID
// -------------------------------------------------------------------------

ip = rule->vector_value("IP");
const auto ip = rule->vector_value("IP");

if (!ip.empty()) //Target as IP & SIZE
{
struct in6_addr ip_addr;

unsigned int ivalue = 0;
if (rule->vector_value("SIZE", ivalue) != 0)
{
error = "Wrong or empty SIZE.";
Expand All @@ -431,6 +460,7 @@ bool SecurityGroup::is_valid(const VectorAttribute * rule, string& error) const
}
else //Target is ANY or NETWORK_ID
{
int id = 0;
if (rule->vector_value("NETWORK_ID", value) == 0 &&
rule->vector_value("NETWORK_ID", id) != 0)
{
Expand Down
28 changes: 4 additions & 24 deletions src/vmm/LibVirtDriverKVM.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "Nebula.h"
#include "Image.h"
#include "DatastorePool.h"
#include "NebulaUtil.h"

#include <regex>
#include <exception>
Expand Down Expand Up @@ -105,27 +106,6 @@ static int to_int(const string& s)
return val;
}

template <class T>
T to_unsigned(const std::string& str)
{
T value;

if (std::regex_search(str, std::regex("[^0-9]")))
{
return 0;
}

std::istringstream iss(str);
iss >> value;

if (iss.fail() || !iss.eof())
{
value = std::numeric_limits<T>::max();
}

return value;
}

template<typename T>
static void insert_sec(ofstream& file, const string& base, const string& s,
const string& sm, const string& sml)
Expand All @@ -134,15 +114,15 @@ static void insert_sec(ofstream& file, const string& base, const string& s,

if (!s.empty())
{
s_i = to_unsigned<T>(s);
s_i = one_util::string_to_unsigned<T>(s);

file << "\t\t\t\t<" << base << "_sec>" << one_util::escape_xml(std::to_string(s_i))
<< "</" << base << "_sec>\n";
}

if (!sm.empty())
{
const auto sm_i = to_unsigned<T>(sm);
const auto sm_i = one_util::string_to_unsigned<T>(sm);

if ( sm_i > s_i)
{
Expand All @@ -152,7 +132,7 @@ static void insert_sec(ofstream& file, const string& base, const string& s,

if (!sml.empty())
{
const auto sml_i = to_unsigned<T>(sml);
const auto sml_i = one_util::string_to_unsigned<T>(sml);

file << "\t\t\t\t<" << base << "_sec_max_length>"
<< one_util::escape_xml(std::to_string(sml_i))
Expand Down

0 comments on commit 5aebf69

Please sign in to comment.