diff --git a/doc/sphinx/arm/ext-radius.rst b/doc/sphinx/arm/ext-radius.rst index ca1c2b4c65..cd8f939f91 100644 --- a/doc/sphinx/arm/ext-radius.rst +++ b/doc/sphinx/arm/ext-radius.rst @@ -550,13 +550,13 @@ RADIUS dictionary. There are differences: - Yes - - since Kea 3.1.1 + - since Kea 3.1.2 * - Support for Vendor Attributes - Yes - - No + - since Kea 3.1.2 * - Attribute Names and Attribute Values diff --git a/src/hooks/dhcp/radius/client_dictionary.cc b/src/hooks/dhcp/radius/client_dictionary.cc index 92839e62f1..4a48c27b4c 100644 --- a/src/hooks/dhcp/radius/client_dictionary.cc +++ b/src/hooks/dhcp/radius/client_dictionary.cc @@ -199,7 +199,7 @@ AttrDefs::add(IntCstDefPtr def) { } void -AttrDefs::parseLine(const string& line, unsigned int depth) { +AttrDefs::parseLine(const string& line, uint32_t& vendor, unsigned int depth) { // Ignore empty lines. if (line.empty()) { return; @@ -220,7 +220,7 @@ AttrDefs::parseLine(const string& line, unsigned int depth) { if (tokens.size() != 2) { isc_throw(Unexpected, "expected 2 tokens, got " << tokens.size()); } - readDictionary(tokens[1], depth + 1); + readDictionary(tokens[1], vendor, depth + 1); return; } // Attribute definition. @@ -243,11 +243,12 @@ AttrDefs::parseLine(const string& line, unsigned int depth) { isc_throw(Unexpected, "can't parse attribute type " << type_str); } AttrValueType value_type = textToAttrValueType(tokens[3]); - if ((value_type == PW_TYPE_VSA) && (type != PW_VENDOR_SPECIFIC)) { + if ((value_type == PW_TYPE_VSA) && + ((vendor != 0) || (type != PW_VENDOR_SPECIFIC))) { isc_throw(BadValue, "only Vendor-Specific (26) attribute can " << "have the vsa data type"); } - AttrDefPtr def(new AttrDef(type, name, value_type)); + AttrDefPtr def(new AttrDef(type, name, value_type, vendor)); add(def); return; } @@ -307,11 +308,79 @@ AttrDefs::parseLine(const string& line, unsigned int depth) { add(def); return; } + // Begin vendor attribute definitions. + if (tokens[0] == "BEGIN-VENDOR") { + if (vendor != 0) { + isc_throw(Unexpected, "unsupported embedded begin vendor, " + << vendor << " is still open"); + } + if (tokens.size() != 2) { + isc_throw(Unexpected, "expected 2 tokens, got " << tokens.size()); + } + const string& vendor_str = tokens[1]; + IntCstDefPtr vendor_cst = getByName(PW_VENDOR_SPECIFIC, vendor_str); + if (vendor_cst) { + vendor = vendor_cst->value_; + return; + } + try { + int64_t val = boost::lexical_cast(vendor_str); + if ((val < numeric_limits::min()) || + (val > numeric_limits::max())) { + isc_throw(Unexpected, "not 32 bit " << vendor_str); + } + vendor = static_cast(val); + } catch (...) { + isc_throw(Unexpected, "can't parse integer value " << vendor_str); + } + if (vendor == 0) { + isc_throw(Unexpected, "0 is reserved"); + } + return; + } + // End vendor attribute definitions. + if (tokens[0] == "END-VENDOR") { + if (vendor == 0) { + isc_throw(Unexpected, "no matching begin vendor"); + } + if (tokens.size() != 2) { + isc_throw(Unexpected, "expected 2 tokens, got " << tokens.size()); + } + const string& vendor_str = tokens[1]; + IntCstDefPtr vendor_cst = getByName(PW_VENDOR_SPECIFIC, vendor_str); + if (vendor_cst) { + if (vendor_cst->value_ == vendor) { + vendor = 0; + return; + } else { + isc_throw(Unexpected, "begin vendor " << vendor + << " and end vendor " << vendor_cst->value_ + << " do not match"); + } + } + uint32_t value; + try { + int64_t val = boost::lexical_cast(vendor_str); + if ((val < numeric_limits::min()) || + (val > numeric_limits::max())) { + isc_throw(Unexpected, "not 32 bit " << vendor_str); + } + value = static_cast(val); + } catch (...) { + isc_throw(Unexpected, "can't parse integer value " << vendor_str); + } + if (vendor == value) { + vendor = 0; + return; + } + isc_throw(Unexpected, "begin vendor " << vendor + << " and end vendor " << value << " do not match"); + } isc_throw(Unexpected, "unknown dictionary entry '" << tokens[0] << "'"); } void -AttrDefs::readDictionary(const string& path, unsigned depth) { +AttrDefs::readDictionary(const string& path, uint32_t& vendor, unsigned depth) { if (depth >= 5) { isc_throw(BadValue, "Too many nested $INCLUDE"); } @@ -324,7 +393,7 @@ AttrDefs::readDictionary(const string& path, unsigned depth) { isc_throw(BadValue, "bad dictionary '" << path << "'"); } try { - readDictionary(ifs, depth); + readDictionary(ifs, vendor, depth); ifs.close(); } catch (const exception& ex) { ifs.close(); @@ -334,14 +403,14 @@ AttrDefs::readDictionary(const string& path, unsigned depth) { } void -AttrDefs::readDictionary(istream& is, unsigned int depth) { +AttrDefs::readDictionary(istream& is, uint32_t& vendor, unsigned int depth) { size_t lines = 0; string line; try { while (is.good()) { ++lines; getline(is, line); - parseLine(line, depth); + parseLine(line, vendor, depth); } if (!is.eof()) { isc_throw(BadValue, "I/O error: " << strerror(errno)); diff --git a/src/hooks/dhcp/radius/client_dictionary.h b/src/hooks/dhcp/radius/client_dictionary.h index 8af9df0f56..854fef8990 100644 --- a/src/hooks/dhcp/radius/client_dictionary.h +++ b/src/hooks/dhcp/radius/client_dictionary.h @@ -292,8 +292,10 @@ public: /// incremented by includes and limited to 5. /// /// @param path dictionary file path. + /// @param vendor reference to the current vendor id. /// @param depth recursion depth. - void readDictionary(const std::string& path, unsigned int depth = 0); + void readDictionary(const std::string& path, uint32_t& vendor, + unsigned int depth = 0); /// @brief Read a dictionary from an input stream. /// @@ -302,8 +304,10 @@ public: /// incremented by includes and limited to 5. /// /// @param is input stream. + /// @param vendor reference to the current vendor id. /// @param depth recursion depth. - void readDictionary(std::istream& is, unsigned int depth = 0); + void readDictionary(std::istream& is, uint32_t& vendor, + unsigned int depth = 0); /// @brief Check if a list of standard attribute definitions /// are available and correct. @@ -324,8 +328,10 @@ protected: /// @brief Parse a dictionary line. /// /// @param line line to parse. + /// @param vendor reference to the current vendor id. /// @param depth recursion depth. - void parseLine(const std::string& line, unsigned int depth); + void parseLine(const std::string& line, uint32_t& vendor, + unsigned int depth); /// @brief Attribute definition container. AttrDefContainer container_; diff --git a/src/hooks/dhcp/radius/radius_parsers.cc b/src/hooks/dhcp/radius/radius_parsers.cc index 20c89d1053..261a8fd448 100644 --- a/src/hooks/dhcp/radius/radius_parsers.cc +++ b/src/hooks/dhcp/radius/radius_parsers.cc @@ -87,9 +87,10 @@ const AttrDefList RadiusConfigParser::USED_STANDARD_ATTR_DEFS = { /// @brief Defaults for Radius attribute configuration. const SimpleDefaults RadiusAttributeParser::ATTRIBUTE_DEFAULTS = { - { "data", Element::string, "" }, - { "expr", Element::string, "" }, - { "raw", Element::string, "" } + { "data", Element::string, "" }, + { "expr", Element::string, "" }, + { "raw", Element::string, "" }, + { "vendor", Element::string, "" } }; void @@ -106,12 +107,17 @@ RadiusConfigParser::parse(ElementPtr& config) { // Read the dictionary if (!AttrDefs::instance().getByType(1)) { + uint32_t vendor = 0; try { - AttrDefs::instance().readDictionary(riref.dictionary_); + AttrDefs::instance().readDictionary(riref.dictionary_, vendor); } catch (const exception& ex) { isc_throw(BadValue, "can't read radius dictionary: " << ex.what()); } + if (vendor != 0) { + isc_throw(BadValue, "vendor definitions were not properly " + << "closed: vendor " << vendor << " is still open"); + } } // Check it. @@ -511,16 +517,43 @@ RadiusAttributeParser::parse(const RadiusServicePtr& service, // Set defaults. setDefaults(attr, ATTRIBUTE_DEFAULTS); + // vendor. + uint32_t vendor = 0; + const string& vendor_txt = getString(attr, "vendor"); + if (!vendor_txt.empty()) { + IntCstDefPtr vendor_cst = + AttrDefs::instance().getByName(PW_VENDOR_SPECIFIC, vendor_txt); + if (vendor_cst) { + vendor = vendor_cst->value_; + } else { + try { + int64_t val = boost::lexical_cast(vendor_txt); + if ((val < numeric_limits::min()) || + (val > numeric_limits::max())) { + isc_throw(Unexpected, "not 32 bit " << vendor_txt); + } + vendor = static_cast(val); + } catch (...) { + isc_throw(ConfigError, "can't parse vendor " << vendor_txt); + } + } + } + // name. const ConstElementPtr& name = attr->get("name"); if (name) { if (name->stringValue().empty()) { isc_throw(ConfigError, "attribute name is empty"); } - def = AttrDefs::instance().getByName(name->stringValue()); + def = AttrDefs::instance().getByName(name->stringValue(), vendor); if (!def) { - isc_throw(ConfigError, "attribute '" - << name->stringValue() << "' is unknown"); + ostringstream msg; + msg << "attribute '" << name->stringValue() << "'"; + if (vendor != 0) { + msg << " in vendor '" << vendor_txt << "'"; + } + msg << " is unknown"; + isc_throw(ConfigError, msg.str()); } } @@ -533,16 +566,26 @@ RadiusAttributeParser::parse(const RadiusServicePtr& service, } uint8_t attrib = static_cast(type->intValue()); if (def && (def->type_ != attrib)) { - isc_throw(ConfigError, name->stringValue() << " attribute has " - << "type " << static_cast(def->type_) - << ", not " << static_cast(attrib)); + ostringstream msg; + msg << "'" << name->stringValue() << "' attribute"; + if (vendor != 0) { + msg << " in vendor '" << vendor_txt << "'"; + } + msg << " has type " << static_cast(def->type_) + << ", not " << static_cast(attrib); + isc_throw(ConfigError, msg.str()); } if (!def) { - def = AttrDefs::instance().getByType(attrib); + def = AttrDefs::instance().getByType(attrib, vendor); } if (!def) { - isc_throw(ConfigError, "attribute type " - << static_cast(attrib) << " is unknown"); + ostringstream msg; + msg << "attribute type " << static_cast(attrib); + if (vendor != 0) { + msg << " in vendor '" << vendor_txt << "'"; + } + msg << " is unknown"; + isc_throw(ConfigError, msg.str()); } } diff --git a/src/hooks/dhcp/radius/tests/attribute_test.h b/src/hooks/dhcp/radius/tests/attribute_test.h index c75d14a173..537fcb0cfb 100644 --- a/src/hooks/dhcp/radius/tests/attribute_test.h +++ b/src/hooks/dhcp/radius/tests/attribute_test.h @@ -19,7 +19,9 @@ class AttributeTest : public ::testing::Test { public: /// @brief Constructor. AttributeTest() { - AttrDefs::instance().readDictionary(TEST_DICTIONARY); + uint32_t vendor = 0; + AttrDefs::instance().readDictionary(TEST_DICTIONARY, vendor); + EXPECT_EQ(0, vendor); } /// @brief Destructor. diff --git a/src/hooks/dhcp/radius/tests/config_unittests.cc b/src/hooks/dhcp/radius/tests/config_unittests.cc index 2eb232db37..4753d208e4 100644 --- a/src/hooks/dhcp/radius/tests/config_unittests.cc +++ b/src/hooks/dhcp/radius/tests/config_unittests.cc @@ -756,7 +756,7 @@ TEST_F(ConfigTest, attribute) { attr->set("name", Element::create("User-Name")); attr->set("type", Element::create(123)); EXPECT_THROW_MSG(parser.parse(srv, attr), ConfigError, - "User-Name attribute has type 1, not 123"); + "'User-Name' attribute has type 1, not 123"); // Type must be between 0 and 255. attr = Element::createMap(); diff --git a/src/hooks/dhcp/radius/tests/dictionary_unittests.cc b/src/hooks/dhcp/radius/tests/dictionary_unittests.cc index 6d2115e39d..9b16e7051d 100644 --- a/src/hooks/dhcp/radius/tests/dictionary_unittests.cc +++ b/src/hooks/dhcp/radius/tests/dictionary_unittests.cc @@ -53,23 +53,33 @@ public: /// @brief Parse a line. /// /// @param line line to parse. + /// @param before vendor id on input (default to 0). + /// @param after expected vendor id on output (default to 0). /// @param depth recursion depth. - void parseLine(const string& line, unsigned int depth = 0) { + void parseLine(const string& line, uint32_t before = 0, + uint32_t after = 0, unsigned int depth = 0) { istringstream is(line + "\n"); - AttrDefs::instance().readDictionary(is, depth); + uint32_t vendor = before; + AttrDefs::instance().readDictionary(is, vendor, depth); + EXPECT_EQ(after, vendor); } /// @brief Parse a list of lines. /// /// @param lines list of lines. + /// @param before vendor id on input (default to 0). + /// @param after expected vendor id on output (default to 0). /// @param depth recursion depth. - void parseLines(const list& lines, unsigned int depth = 0) { + void parseLines(const list& lines, uint32_t before = 0, + uint32_t after = 0, unsigned int depth = 0) { string content; for (auto const& line : lines) { content += line + "\n"; } istringstream is(content); - AttrDefs::instance().readDictionary(is, depth); + uint32_t vendor = before; + AttrDefs::instance().readDictionary(is, vendor, depth); + EXPECT_EQ(after, vendor); } /// @brief writes specified content to a file. @@ -92,7 +102,10 @@ const char* DictionaryTest::TEST_DICT = "test-dict"; // Verifies standards definitions can be read from the dictionary. TEST_F(DictionaryTest, standard) { - ASSERT_NO_THROW_LOG(AttrDefs::instance().readDictionary(TEST_DICTIONARY)); + uint32_t vendor = 0; + ASSERT_NO_THROW_LOG(AttrDefs::instance().readDictionary(TEST_DICTIONARY, + vendor)); + EXPECT_EQ(0, vendor); } // Verifies parseLine internal routine. @@ -142,11 +155,19 @@ TEST_F(DictionaryTest, parseLine) { "expected 3 tokens, got 4 at line 1"); EXPECT_THROW_MSG(parseLine("VENDOR my-vendor 0"), BadValue, "0 is reserved at line 1"); + EXPECT_THROW_MSG(parseLine("BEGIN-VENDOR"), BadValue, + "expected 2 tokens, got 1 at line 1"); + EXPECT_THROW_MSG(parseLine("BEGIN-VENDOR my-vendor 1"), BadValue, + "expected 2 tokens, got 3 at line 1"); + EXPECT_THROW_MSG(parseLine("END-VENDOR", 1), BadValue, + "expected 2 tokens, got 1 at line 1"); + EXPECT_THROW_MSG(parseLine("END-VENDOR my-vendor 1", 1), BadValue, + "expected 2 tokens, got 3 at line 1"); - EXPECT_THROW_MSG(parseLine("BEGIN-VENDOR my-vendor"), BadValue, - "unknown dictionary entry 'BEGIN-VENDOR' at line 1"); - EXPECT_THROW_MSG(parseLine("END-VENDOR my-vendor"), BadValue, - "unknown dictionary entry 'END-VENDOR' at line 1"); + EXPECT_THROW_MSG(parseLine("BEGIN-TLV my-vendor"), BadValue, + "unknown dictionary entry 'BEGIN-TLV' at line 1"); + EXPECT_THROW_MSG(parseLine("END-TLV my-vendor"), BadValue, + "unknown dictionary entry 'END-TLV' at line 1"); } // Verifies sequences attribute of (re)definitions. @@ -275,12 +296,90 @@ TEST_F(DictionaryTest, vendorId) { EXPECT_THROW_MSG(parseLines(new_value), BadValue, expected); } +// Verifies begin and end vendor entries. +TEST_F(DictionaryTest, beginEndVendor) { + // Begin already open. + list begin_unknown = { + "BEGIN-VENDOR foo" + }; + string expected = "unsupported embedded begin vendor, "; + expected += "1 is still open at line 1"; + EXPECT_THROW_MSG(parseLines(begin_unknown, 1), BadValue, expected); + // Value must be a known name or integer. + EXPECT_THROW_MSG(parseLines(begin_unknown), BadValue, + "can't parse integer value foo at line 1"); + // End not yet open. + list end_unknown = { + "END-VENDOR foo" + }; + EXPECT_THROW_MSG(parseLines(end_unknown), BadValue, + "no matching begin vendor at line 1"); + EXPECT_THROW_MSG(parseLines(end_unknown, 1), BadValue, + "can't parse integer value foo at line 1"); + // 0 is reserved. + list begin0 = { + "BEGIN-VENDOR 0" + }; + EXPECT_THROW_MSG(parseLines(begin0), BadValue, + "0 is reserved at line 1"); + + // Positive using a name. + list positive = { + "VENDOR DSL-Forum 3561", + "BEGIN-VENDOR DSL-Forum", + "ATTRIBUTE Agent-Circuit-Id 1 string" + }; + EXPECT_NO_THROW_LOG(parseLines(positive, 0, 3561)); + auto aci = AttrDefs::instance().getByName("Agent-Circuit-Id", 3561); + ASSERT_TRUE(aci); + EXPECT_EQ(1, aci->type_); + EXPECT_EQ(PW_TYPE_STRING, aci->value_type_); + EXPECT_EQ("Agent-Circuit-Id", aci->name_); + EXPECT_EQ(3561, aci->vendor_); + + // Positive using an integer. + list positive_n = { + "BEGIN-VENDOR 3561", + "ATTRIBUTE Actual-Data-Rate-Upstream 129 integer" + }; + EXPECT_NO_THROW_LOG(parseLines(positive_n, 0, 3561)); + auto adru = AttrDefs::instance().getByType(129, 3561); + ASSERT_TRUE(adru); + EXPECT_EQ(129, adru->type_); + EXPECT_EQ(PW_TYPE_INTEGER, adru->value_type_); + EXPECT_EQ("Actual-Data-Rate-Upstream", adru->name_); + EXPECT_EQ(3561, adru->vendor_); + + // End using a name. + list end_name = { + "END-VENDOR DSL-Forum" + }; + EXPECT_NO_THROW_LOG(parseLines(end_name, 3561, 0)); + + // End using an integer. + list end_int = { + "END-VENDOR 3561" + }; + EXPECT_NO_THROW_LOG(parseLines(end_int, 3561, 0)); + + // Not matching. + list no_match = { + "BEGIN-VENDOR 1234", + "END-VENDOR 2345" + }; + expected = "begin vendor 1234 and end vendor 2345 do not match at line 2"; + EXPECT_THROW_MSG(parseLines(no_match), BadValue, expected); +} + // Verifies errors from bad dictionary files. TEST_F(DictionaryTest, badFile) { string expected = "can't open dictionary '/does-not-exist': "; expected += "No such file or directory"; - EXPECT_THROW_MSG(AttrDefs::instance().readDictionary("/does-not-exist"), + uint32_t vendor = 0; + EXPECT_THROW_MSG(AttrDefs::instance().readDictionary("/does-not-exist", + vendor), BadValue, expected); + EXPECT_EQ(0, vendor); list bad_include = { "$INCLUDE /does-not-exist" }; @@ -292,7 +391,10 @@ TEST_F(DictionaryTest, badFile) { // Verifies that the dictionary correctly defines used standard attributes. TEST_F(DictionaryTest, hookAttributes) { - ASSERT_NO_THROW_LOG(AttrDefs::instance().readDictionary(TEST_DICTIONARY)); + uint32_t vendor = 0; + ASSERT_NO_THROW_LOG(AttrDefs::instance().readDictionary(TEST_DICTIONARY, + vendor)); + EXPECT_EQ(0, vendor); EXPECT_NO_THROW_LOG(AttrDefs::instance(). checkStandardDefs(RadiusConfigParser::USED_STANDARD_ATTR_DEFS)); } @@ -312,7 +414,7 @@ TEST_F(DictionaryTest, include) { EXPECT_EQ(2495, isc->value_); // max depth is 5. - EXPECT_THROW_MSG(parseLines(include, 4), BadValue, + EXPECT_THROW_MSG(parseLines(include, 0, 0, 4), BadValue, "Too many nested $INCLUDE at line 2"); } @@ -353,11 +455,13 @@ TEST_F(DictionaryTest, DISABLED_readDictionaries) { const string path_regex("/usr/share/freeradius/*"); Glob g(path_regex); AttrDefs& defs(AttrDefs::instance()); + uint32_t vendor = 0; for (size_t i = 0; i < g.glob_buffer_.gl_pathc; ++i) { const string file_name(g.glob_buffer_.gl_pathv[i]); SCOPED_TRACE(file_name); - EXPECT_NO_THROW_LOG(defs.readDictionary(file_name)); + EXPECT_NO_THROW_LOG(defs.readDictionary(file_name, vendor)); } + EXPECT_EQ(0, vendor); } // Verifies attribute definitions.