From 5a9310d6b4862ad8dc4b9b812a0ab4c8dab28e25 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 21 Aug 2025 10:57:23 +0200 Subject: [PATCH] [#3860] Checkpoint vendor in def --- src/hooks/dhcp/radius/client_dictionary.cc | 57 +++++++----- src/hooks/dhcp/radius/client_dictionary.h | 90 ++++++++++++++++--- .../dhcp/radius/tests/config_unittests.cc | 2 +- 3 files changed, 110 insertions(+), 39 deletions(-) diff --git a/src/hooks/dhcp/radius/client_dictionary.cc b/src/hooks/dhcp/radius/client_dictionary.cc index 479db62e0d..92839e62f1 100644 --- a/src/hooks/dhcp/radius/client_dictionary.cc +++ b/src/hooks/dhcp/radius/client_dictionary.cc @@ -68,9 +68,9 @@ AttrDefs::instance() { } AttrDefPtr -AttrDefs::getByType(const uint8_t type) const { +AttrDefs::getByType(const uint8_t type, const uint32_t vendor) const { auto const& idx = container_.get<0>(); - auto it = idx.find(type); + auto it = idx.find(boost::make_tuple(vendor, type)); if (it != idx.end()) { return (*it); } @@ -78,15 +78,15 @@ AttrDefs::getByType(const uint8_t type) const { } AttrDefPtr -AttrDefs::getByName(const string& name) const { +AttrDefs::getByName(const string& name, const uint32_t vendor) const { auto const& idx = container_.get<1>(); - auto it = idx.find(name); + auto it = idx.find(boost::make_tuple(vendor, name)); if (it != idx.end()) { return (*it); } - auto alias = aliases_.find(name); + auto alias = aliases_.find(boost::make_tuple(vendor, name)); if (alias != aliases_.end()) { - auto ita = idx.find(alias->second); + auto ita = idx.find(boost::make_tuple(vendor, alias->name_)); if (ita != idx.end()) { return (*ita); } @@ -100,42 +100,51 @@ AttrDefs::add(AttrDefPtr def) { return; } auto& idx1 = container_.get<1>(); - auto it1 = idx1.find(def->name_); + auto it1 = idx1.find(boost::make_tuple(def->vendor_, def->name_)); if (it1 != idx1.end()) { if ((def->type_ == (*it1)->type_) && (def->value_type_ == (*it1)->value_type_)) { // Duplicate: ignore. return; } - isc_throw(BadValue, "Illegal attribute redefinition of '" << def->name_ - << "' type " << static_cast((*it1)->type_) - << " value type " << attrValueTypeToText((*it1)->value_type_) - << " by " << static_cast(def->type_) - << " " << attrValueTypeToText(def->value_type_)); + ostringstream msg; + msg << "Illegal attribute redefinition of '" << def->name_ << "'"; + if (def->vendor_ != 0) { + msg << " vendor " << def->vendor_; + } + msg << " type " << static_cast((*it1)->type_) + << " value type " << attrValueTypeToText((*it1)->value_type_) + << " by " << static_cast(def->type_) + << " " << attrValueTypeToText(def->value_type_); + isc_throw(BadValue, msg.str()); } auto& idx0 = container_.get<0>(); - auto it0 = idx0.find(def->type_); + auto it0 = idx0.find(boost::make_tuple(def->vendor_, def->type_)); if (it0 != idx0.end()) { if (def->value_type_ == (*it0)->value_type_) { // Alias. - auto p = pair(def->name_, (*it0)->name_); - static_cast(aliases_.insert(p)); + AttrDefAlias alias(def->name_, (*it0)->name_, def->vendor_); + static_cast(aliases_.insert(alias)); return; } - isc_throw(BadValue, "Illegal attribute redefinition of '" - << (*it0)->name_ << "' type " - << static_cast((*it0)->type_) << " value type " - << attrValueTypeToText((*it0)->value_type_) - << " by '" << def->name_ << "' " - << static_cast(def->type_) << " " - << attrValueTypeToText(def->value_type_)); + ostringstream msg; + msg << "Illegal attribute redefinition of '" << (*it0)->name_ << "'"; + if (def->vendor_ != 0) { + msg << " vendor " << def->vendor_; + } + msg << " type " << static_cast((*it0)->type_) + << " value type " << attrValueTypeToText((*it0)->value_type_) + << " by '" << def->name_ << "' " + << static_cast(def->type_) << " " + << attrValueTypeToText(def->value_type_); + isc_throw(BadValue, msg.str()); } static_cast(container_.insert(def)); } string -AttrDefs::getName(const uint8_t type) const { - AttrDefPtr def = getByType(type); +AttrDefs::getName(const uint8_t type, const uint32_t vendor) const { + AttrDefPtr def = getByType(type, vendor); if (def) { return (def->name_); } diff --git a/src/hooks/dhcp/radius/client_dictionary.h b/src/hooks/dhcp/radius/client_dictionary.h index 4b6824faee..8af9df0f56 100644 --- a/src/hooks/dhcp/radius/client_dictionary.h +++ b/src/hooks/dhcp/radius/client_dictionary.h @@ -58,9 +58,10 @@ public: /// @param type attribute type. /// @param name attribute name. /// @param value_type attribute value type. + /// @param vendor vendor id (default 0). AttrDef(const uint8_t type, const std::string& name, - const AttrValueType value_type) - : type_(type), name_(name), value_type_(value_type) { + const AttrValueType value_type, const uint32_t vendor = 0) + : type_(type), name_(name), value_type_(value_type), vendor_(vendor) { } /// @brief type. @@ -71,6 +72,9 @@ public: /// @brief value_type. const AttrValueType value_type_; + + /// @brief vendor id (default 0). + const uint32_t vendor_; }; /// @brief Shared pointers to Attribute definition. @@ -79,6 +83,30 @@ typedef boost::shared_ptr AttrDefPtr; /// @brief List of Attribute definitions. typedef std::list AttrDefList; +/// @brief RADIUS attribute aliases. +class AttrDefAlias { +public: + + /// @brief Constructor. + /// + /// @param alias attribute alias name. + /// @param name attribute name. + /// @param vendor vendor id (default 0). + AttrDefAlias(const std::string& alias, const std::string& name, + const uint32_t vendor = 0) + : alias_(alias), name_(name), vendor_(vendor) { + } + + /// @brief alias. + const std::string alias_; + + /// @brief name. + const std::string name_; + + /// @brief vendor id (default 0). + const uint32_t vendor_; +}; + /// @brief RADIUS integer constant definitions. /// /// Include vendor ids with Vendor-Specific attribute. @@ -118,23 +146,53 @@ public: AttrDefPtr, // Start specification of indexes here. boost::multi_index::indexed_by< - // Hash index for by type. + // Hash index for by vendor and type. boost::multi_index::hashed_unique< - boost::multi_index::member< - AttrDef, const uint8_t, &AttrDef::type_ + boost::multi_index::composite_key< + AttrDef, + boost::multi_index::member< + AttrDef, const uint32_t, &AttrDef::vendor_ + >, + boost::multi_index::member< + AttrDef, const uint8_t, &AttrDef::type_ + > > >, - // Hash index for by name. + // Hash index for by vendor and name. boost::multi_index::hashed_unique< - boost::multi_index::member< - AttrDef, const std::string, &AttrDef::name_ + boost::multi_index::composite_key< + AttrDef, + boost::multi_index::member< + AttrDef, const uint32_t, &AttrDef::vendor_ + >, + boost::multi_index::member< + AttrDef, const std::string, &AttrDef::name_ + > > > > > AttrDefContainer; /// @brief Type of the alias table (alias -> standard name map). - typedef std::unordered_map AttrDefAliases; + typedef boost::multi_index_container< + // This container stores aliases. + AttrDefAlias, + // Start specification of indexes here. + boost::multi_index::indexed_by< + // Hash index for by vendor and alias. + boost::multi_index::hashed_unique< + boost::multi_index::composite_key< + AttrDefAlias, + boost::multi_index::member< + AttrDefAlias, const uint32_t, &AttrDefAlias::vendor_ + >, + boost::multi_index::member< + AttrDefAlias, const std::string, &AttrDefAlias::alias_ + > + > + > + > + > AttrDefAliases; /// @brief Type of the integer constant definition container. typedef boost::multi_index_container< @@ -176,17 +234,20 @@ public: /// @return the single instance. static AttrDefs& instance(); - /// @brief Get attribute definition by type. + /// @brief Get attribute definition by type and vendor. /// /// @param type type to look for. + /// @param vendor vendor id to look for (default 0). /// @return pointer to the attribute definition or null. - AttrDefPtr getByType(const uint8_t type) const; + AttrDefPtr getByType(const uint8_t type, const uint32_t vendor = 0) const; - /// @brief Get attribute definition by name. + /// @brief Get attribute definition by name and vendor. /// /// @param name name to look for. + /// @param vendor vendor id to look for (default 0). /// @return pointer to the attribute definition or null. - AttrDefPtr getByName(const std::string& name) const; + AttrDefPtr getByName(const std::string& name, + const uint32_t vendor = 0) const; /// @brief Add (or replace) an attribute definition. /// @@ -203,7 +264,8 @@ public: /// @brief Get attribute name. /// /// @param type type to look for. - std::string getName(const uint8_t type) const; + /// @param vendor vendor id to look for (default 0). + std::string getName(const uint8_t type, const uint32_t vendor = 0) const; /// @brief Get integer constant definition by attribute type and name. /// diff --git a/src/hooks/dhcp/radius/tests/config_unittests.cc b/src/hooks/dhcp/radius/tests/config_unittests.cc index d060aa21bc..2eb232db37 100644 --- a/src/hooks/dhcp/radius/tests/config_unittests.cc +++ b/src/hooks/dhcp/radius/tests/config_unittests.cc @@ -807,7 +807,7 @@ TEST_F(ConfigTest, attribute) { attr->set("data", Element::create("foobar")); attr->set("type", Element::create(26)); expected = "can't create Vendor-Specific attribute from [foobar]: "; - expected += "Can't decode vsa from text"); + expected += "Can't decode vsa from text"; EXPECT_THROW_MSG(parser.parse(srv, attr), ConfigError, expected); // One of expr, data, raw -- 2.47.3