From 4286b27403017ce4de26997a49e6630a35526979 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 6 May 2022 08:55:32 +0300 Subject: [PATCH 01/13] [#562] use SubnetID from subnet_id.h in Lease --- src/lib/dhcpsrv/lease.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 448c954b07..4fbfd8a6d2 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -11,18 +11,13 @@ #include #include #include +#include #include #include namespace isc { namespace dhcp { -/// @brief Unique identifier for a subnet (both v4 and v6) -/// -/// Let's copy SubnetID definition from subnet.h. We can't include it directly, -/// because subnet.h needs Lease::Type, so it includes lease.h -typedef uint32_t SubnetID; - struct Lease; /// @brief Pointer to the lease object. -- GitLab From 1d5ee71740f163c13cb95179bc8452642209bc69 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 6 May 2022 08:55:44 +0300 Subject: [PATCH 02/13] [#562] enable range-based for loops on ClientClasses --- src/lib/dhcp/classify.h | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcp/classify.h b/src/lib/dhcp/classify.h index 93e62932f4..d304dee6cc 100644 --- a/src/lib/dhcp/classify.h +++ b/src/lib/dhcp/classify.h @@ -68,6 +68,7 @@ namespace dhcp { /// @brief Type of iterators typedef ClientClassContainer::const_iterator const_iterator; + typedef ClientClassContainer::iterator iterator; /// @brief Default constructor. ClientClasses() : container_() { @@ -104,15 +105,31 @@ namespace dhcp { return (container_.size()); } - /// @brief Iterator to the first element. + /// @brief Iterators to the first element. + /// @{ const_iterator cbegin() const { return (container_.cbegin()); } + const_iterator begin() const { + return (container_.begin()); + } + iterator begin() { + return (container_.begin()); + } + /// @} - /// @brief Iterator to the past the end element. + /// @brief Iterators to the past the end element. + /// @{ const_iterator cend() const { return (container_.cend()); } + const_iterator end() const { + return (container_.end()); + } + iterator end() { + return (container_.end()); + } + /// @} /// @brief returns if class x belongs to the defined classes /// -- GitLab From 8781fbf1bab361bbf21160e1df1b3ff85e7574a8 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 6 May 2022 08:55:54 +0300 Subject: [PATCH 03/13] [#562] template adapters to existing qualified names --- src/hooks/dhcp/mysql_cb/mysql_cb_impl.h | 2 +- src/lib/dhcp/Makefile.am | 2 ++ src/lib/dhcp/option_int.h | 2 +- src/lib/dhcp/option_int_array.h | 2 +- src/lib/dhcp/pkt_template.h | 47 +++++++++++++++++++++++++ src/lib/dhcpsrv/subnet.h | 29 ++++++++++++++- src/lib/hooks/callout_handle.cc | 20 +++++++++++ src/lib/hooks/callout_handle.h | 7 ++++ src/lib/util/Makefile.am | 2 ++ src/lib/util/dhcp_space.cc | 35 ++++++++++++++++++ src/lib/util/dhcp_space.h | 29 +++++++++++++++ 11 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 src/lib/dhcp/pkt_template.h create mode 100644 src/lib/util/dhcp_space.cc create mode 100644 src/lib/util/dhcp_space.h diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h index 3894ced649..2422a601c7 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h @@ -644,7 +644,7 @@ public: /// @brief Creates input binding for user context parameter. /// - /// @param T Type of the configuration element to which context belongs. + /// @tparam T Type of the configuration element to which context belongs. /// @param network Pointer to a shared network, subnet or other configuration /// element for which binding should be created. /// @return Pointer to the binding (possibly null binding if context is diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index 413db74eac..4734d8c170 100644 --- a/src/lib/dhcp/Makefile.am +++ b/src/lib/dhcp/Makefile.am @@ -56,6 +56,7 @@ libkea_dhcp___la_SOURCES += pkt_filter.h pkt_filter.cc libkea_dhcp___la_SOURCES += pkt_filter6.h pkt_filter6.cc libkea_dhcp___la_SOURCES += pkt_filter_inet.cc pkt_filter_inet.h libkea_dhcp___la_SOURCES += pkt_filter_inet6.cc pkt_filter_inet6.h +libkea_dhcp___la_SOURCES += pkt_template.h libkea_dhcp___la_SOURCES += socket_info.h # Utilize Linux Packet Filtering on Linux. @@ -138,6 +139,7 @@ libkea_dhcp___include_HEADERS = \ pkt_filter6.h \ pkt_filter_inet.h \ pkt_filter_inet6.h \ + pkt_template.h \ protocol_util.h \ socket_info.h \ std_option_defs.h diff --git a/src/lib/dhcp/option_int.h b/src/lib/dhcp/option_int.h index caed70aa55..6ccae3ec58 100644 --- a/src/lib/dhcp/option_int.h +++ b/src/lib/dhcp/option_int.h @@ -44,7 +44,7 @@ typedef boost::shared_ptr OptionUint32Ptr; /// - int16_t, /// - int32_t. /// -/// @param T data field type (see above). +/// @tparam T data field type (see above). template class OptionInt: public Option { private: diff --git a/src/lib/dhcp/option_int_array.h b/src/lib/dhcp/option_int_array.h index ec6fc2be8d..92dc198306 100644 --- a/src/lib/dhcp/option_int_array.h +++ b/src/lib/dhcp/option_int_array.h @@ -51,7 +51,7 @@ typedef boost::shared_ptr OptionUint32ArrayPtr; /// allow addition of sub-options but they will be ignored during /// packing and unpacking option data. /// -/// @param T data field type (see above). +/// @tparam T data field type (see above). template class OptionIntArray : public Option { private: diff --git a/src/lib/dhcp/pkt_template.h b/src/lib/dhcp/pkt_template.h new file mode 100644 index 0000000000..a018cf6164 --- /dev/null +++ b/src/lib/dhcp/pkt_template.h @@ -0,0 +1,47 @@ +// Copyright (C) 2022 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef ISC_PKT_TEMPLATE_H +#define ISC_PKT_TEMPLATE_H + +#include +#include +#include + +namespace isc { + +namespace dhcp { + +/// @brief adapters for linking templates to qualified names +/// @{ +namespace { + +template +struct adapter_Pkt {}; + +template <> +struct adapter_Pkt { + using type = Pkt4; +}; + +template <> +struct adapter_Pkt { + using type = Pkt6; +}; + +} // namespace + +template +using PktT = typename adapter_Pkt::type; + +template +using PktTPtr = boost::shared_ptr>; +/// @} + +} // namespace dhcp +} // namespace isc + +#endif // ISC_PKT_TEMPLATE_H diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 6fe9f13e40..4dfad6f3a7 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -15,7 +15,9 @@ #include #include #include +#include #include + #include #include #include @@ -25,6 +27,7 @@ #include #include #include + #include #include #include @@ -995,9 +998,33 @@ using SubnetFetcher4 = SubnetFetcher; /// @brief Type of the @c SubnetFetcher used for IPv6. using SubnetFetcher6 = SubnetFetcher; +//@} +/// @brief adapters for linking templates to qualified names +/// @{ +namespace { -//@} +template +struct adapter_Subnet {}; + +template <> +struct adapter_Subnet { + using type = Subnet4; +}; + +template <> +struct adapter_Subnet { + using type = Subnet6; +}; + +} // namespace + +template +using SubnetT = typename adapter_Subnet::type; + +template +using SubnetTPtr = boost::shared_ptr>; +/// @} } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/lib/hooks/callout_handle.cc b/src/lib/hooks/callout_handle.cc index e80760ac92..89eae865db 100644 --- a/src/lib/hooks/callout_handle.cc +++ b/src/lib/hooks/callout_handle.cc @@ -159,5 +159,25 @@ ScopedCalloutHandleState::resetState() { callout_handle_->setStatus(CalloutHandle::NEXT_STEP_CONTINUE); } +template <> +char const* queryArgument() { + return "query4"; +} + +template <> +char const* queryArgument() { + return "query6"; +} + +template <> +char const* subnetArgument() { + return "subnet4"; +} + +template <> +char const* subnetArgument() { + return "subnet6"; +} + } // namespace hooks } // namespace isc diff --git a/src/lib/hooks/callout_handle.h b/src/lib/hooks/callout_handle.h index b9e018b7c0..017ebe67ac 100644 --- a/src/lib/hooks/callout_handle.h +++ b/src/lib/hooks/callout_handle.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -501,6 +502,12 @@ private: CalloutHandlePtr callout_handle_; }; +template +char const* queryArgument(); + +template +char const* subnetArgument(); + } // namespace hooks } // namespace isc diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am index cf1f2d6c85..e7895ae9ed 100644 --- a/src/lib/util/Makefile.am +++ b/src/lib/util/Makefile.am @@ -11,6 +11,7 @@ libkea_util_la_SOURCES = boost_time_utils.h boost_time_utils.cc libkea_util_la_SOURCES += buffer.h io_utilities.h libkea_util_la_SOURCES += chrono_time_utils.h chrono_time_utils.cc libkea_util_la_SOURCES += csv_file.h csv_file.cc +libkea_util_la_SOURCES += dhcp_space.cc dhcp_space.h libkea_util_la_SOURCES += doubles.h libkea_util_la_SOURCES += file_utilities.h file_utilities.cc libkea_util_la_SOURCES += filename.h filename.cc @@ -59,6 +60,7 @@ libkea_util_include_HEADERS = \ boost_time_utils.h \ buffer.h \ csv_file.h \ + dhcp_space.h \ doubles.h \ file_utilities.h \ filename.h \ diff --git a/src/lib/util/dhcp_space.cc b/src/lib/util/dhcp_space.cc new file mode 100644 index 0000000000..c6bbe31754 --- /dev/null +++ b/src/lib/util/dhcp_space.cc @@ -0,0 +1,35 @@ +// Copyright (C) 2022 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include + +#include + +namespace isc { +namespace util { + +template <> +uint16_t integerDhcpSpace() { + return 4; +} + +template <> +uint16_t integerDhcpSpace() { + return 6; +} + +template <> +char const* cStringDhcpSpace() { + return "4"; +} + +template <> +char const* cStringDhcpSpace() { + return "6"; +} + +} // namespace util +} // namespace isc diff --git a/src/lib/util/dhcp_space.h b/src/lib/util/dhcp_space.h new file mode 100644 index 0000000000..2945eefef8 --- /dev/null +++ b/src/lib/util/dhcp_space.h @@ -0,0 +1,29 @@ +// Copyright (C) 2022 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef ISC_DHCP_SPACE_H +#define ISC_DHCP_SPACE_H 1 + +#include + +namespace isc { +namespace util { + +enum DhcpSpace { + DHCPv4, + DHCPv6, +}; + +template +constexpr uint16_t integerDhcpSpace(); + +template +constexpr char const* cStringDhcpSpace(); + +} // namespace util +} // namespace isc + +#endif // ISC_DHCP_SPACE_H -- GitLab From 5d3a85f5ebb89241f9a824ecb1ce79becb3e319a Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 6 May 2022 08:56:00 +0300 Subject: [PATCH 04/13] [#562] mention mark and argument in logger error --- src/lib/log/log_formatter.cc | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/lib/log/log_formatter.cc b/src/lib/log/log_formatter.cc index dce7fe990a..c568f86a91 100644 --- a/src/lib/log/log_formatter.cc +++ b/src/lib/log/log_formatter.cc @@ -29,19 +29,17 @@ replacePlaceholder(std::string& message, const string& arg, message.replace(pos, mark.size(), arg); pos = message.find(mark, pos + arg.size()); } while (pos != string::npos); - } + } else { #ifdef ENABLE_LOGGER_CHECKS - else { // We're missing the placeholder, so throw an exception - isc_throw(MismatchedPlaceholders, - "Missing logger placeholder in message: " << message); - } + isc_throw(MismatchedPlaceholders, "Missing logger placeholder '" << mark << "' for value '" + << arg << "' in message '" + << message << "'"); #else - else { // We're missing the placeholder, so add some complain - message.append(" @@Missing placeholder " + mark + " for '" + arg + "'@@"); - } + message.append(" @@Missing placeholder '" + mark + "' for value '" + arg + "'@@"); #endif /* ENABLE_LOGGER_CHECKS */ + } } void @@ -57,10 +55,11 @@ checkExcessPlaceholders(std::string& message, #ifdef ENABLE_LOGGER_CHECKS // Also, make sure we print the message so we can identify which // identifier has the problem. - cerr << "Message " << message << endl; - assert("Excess logger placeholders still exist in message" == NULL); + cerr << "Excess logger placeholder '" << mark << "' still exists in message '" << message + << "'." << endl; + assert(false); #else - message.append(" @@Excess logger placeholders still exist@@"); + message.append(" @@Excess logger placeholder " + mark + " still exists@@"); #endif /* ENABLE_LOGGER_CHECKS */ } } -- GitLab From fe76e0f0788560d89ee30fb1a30058da54248dcf Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 6 May 2022 08:56:19 +0300 Subject: [PATCH 05/13] [#562] add an ordered_unique index to ClientClassContainer --- src/lib/dhcp/classify.h | 16 +++++++++++++++- src/lib/dhcp/tests/pkt4_unittest.cc | 26 +++++++++++++++++++++++++- src/lib/dhcp/tests/pkt6_unittest.cc | 24 ++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcp/classify.h b/src/lib/dhcp/classify.h index d304dee6cc..4f3a0c1401 100644 --- a/src/lib/dhcp/classify.h +++ b/src/lib/dhcp/classify.h @@ -7,10 +7,12 @@ #ifndef CLASSIFY_H #define CLASSIFY_H +#include #include #include +#include #include -#include + #include /// @file classify.h @@ -55,6 +57,10 @@ namespace dhcp { boost::multi_index::hashed_unique< boost::multi_index::tag, boost::multi_index::identity + >, + // Third index orders lexicographically. + boost::multi_index::ordered_unique< + boost::multi_index::identity > > > ClientClassContainer; @@ -131,6 +137,14 @@ namespace dhcp { } /// @} + /// @brief Returns an index that allows iteration through a sorted set + /// of all the client classes. + /// + /// @return the index iterable through range-based for looping + ClientClassContainer::nth_index<2>::type const& sorted() const { + return container_.get<2>(); + } + /// @brief returns if class x belongs to the defined classes /// /// @param x client class to be checked diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index d01849891e..7029abfbd2 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/src/lib/dhcp/tests/pkt4_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -1386,4 +1386,28 @@ TEST_F(Pkt4Test, testSkipThisOptionError) { EXPECT_EQ("def", opstr->getValue()); } +// Check that sorted() allows sorted iteration. +TEST_F(Pkt4Test, sortedClientClasses) { + // Add a bunch of classes to a packet. + Pkt4 packet(DHCPDISCOVER, 1234); + packet.addClass("foo"); + packet.addClass("bar"); + packet.addClass("2"); + packet.addClass("10"); + + // Check that the usual iteration yields items in order of insertion. + std::string client_classes; + for (ClientClass const& c : packet.getClasses()) { + client_classes += c + ','; + } + EXPECT_EQ(client_classes, "ALL,foo,bar,2,10,"); + + // Check sorted() yields sorted items. + std::string sorted_client_classes; + for (ClientClass const& c : packet.getClasses().sorted()) { + sorted_client_classes += c + ','; + } + EXPECT_EQ(sorted_client_classes, "10,2,ALL,bar,foo,"); +} + } // end of anonymous namespace diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index c4878f1595..9afd8f2671 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -2115,4 +2115,28 @@ TEST_F(Pkt6Test, relayDataOption) { EXPECT_EQ(orig_data, clone_data); } +// Check that sorted() allows sorted iteration. +TEST_F(Pkt6Test, sortedClientClasses) { + // Add a bunch of classes to a packet. + Pkt6 packet(DHCPV6_SOLICIT, 1234); + packet.addClass("foo"); + packet.addClass("bar"); + packet.addClass("2"); + packet.addClass("10"); + + // Check that the usual iteration yields items in order of insertion. + std::string client_classes; + for (ClientClass const& c : packet.getClasses()) { + client_classes += c + ','; + } + EXPECT_EQ(client_classes, "ALL,foo,bar,2,10,"); + + // Check sorted() yields sorted items. + std::string sorted_client_classes; + for (ClientClass const& c : packet.getClasses().sorted()) { + sorted_client_classes += c + ','; + } + EXPECT_EQ(sorted_client_classes, "10,2,ALL,bar,foo,"); } + +} // namespace -- GitLab From 68c7be26bf8aff4f1702d213b0b627e06ddaf028 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 6 May 2022 08:56:07 +0300 Subject: [PATCH 06/13] [#562] document rate limiting --- doc/sphinx/arm/hooks-limits.rst | 53 +++++++++++++++++++++++++++++++ doc/sphinx/arm/hooks.rst | 2 +- doc/sphinx/arm/rst_arm_sources.mk | 1 + doc/sphinx/conf.py | 1 + 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 doc/sphinx/arm/hooks-limits.rst diff --git a/doc/sphinx/arm/hooks-limits.rst b/doc/sphinx/arm/hooks-limits.rst new file mode 100644 index 0000000000..b7330a9a1f --- /dev/null +++ b/doc/sphinx/arm/hooks-limits.rst @@ -0,0 +1,53 @@ +.. _hooks-limits: + +``limits``: Rate Limiting +========================= + +This hook library enables limiting the rate at which packets are being processed. +The limits hook library is part of the subscription package. + +Configuration +~~~~~~~~~~~~~ + +The library can be loaded by both ``kea-dhcp4`` and ``kea-dhcp6`` servers by adding its path along with any parameters to the ``"hooks-libraries"`` element of the server's configuration. Here is an example: + +.. code-block:: json + + { + "DhcpX": { + "hooks-libraries": [ + { + "library": "/usr/local/lib/libdhcp_limits.so" + "limits": [ + { + "client-classes": ["ALL"], + "rate-limit": "1000 packets per second" + }, + { + "client-classes": [ "bronze" ], + "rate-limit": "100 packets per minute" + } + { + "subnet-ids": [ 1, 2 ], + "rate-limit": "1 packet per second" + } + ] + } + ] + } + } + +There are two possible packet identification criteria: client classes and subnet IDs. For easier +configuration, you may provide multiple such criteria to a single rate limit. + +The rate limit can be specified in the format ``"

packets per "``. ``

`` is any +number that can be represented by an unsigned integer on 32 bits i.e. between ``0`` and +``4,294,967,295``. ```` can be any of ``second``, ``minute``, ``hour``, ``day``, +``week``, ``month``, ``year``. ``month`` is considered 30 days for simplicity. Similarly, ``year`` +is considered 365 days. This syntax covers a high range of rates from one lease per year to four +billion leases per second. + +The configured value of ``0`` packets can be a convenient way of disabling packet processing for +certain clients entirely. As such, it means its literary value and is not a special value for +disabling rate limiting. Disabling limiting altogether is achieved by removing the leaf +configuration entry, the map around it or the entire hook library configuration. diff --git a/doc/sphinx/arm/hooks.rst b/doc/sphinx/arm/hooks.rst index 2b3a995dc1..ab2a7f69d8 100644 --- a/doc/sphinx/arm/hooks.rst +++ b/doc/sphinx/arm/hooks.rst @@ -3586,7 +3586,6 @@ following: The ``network6-subnet-del`` command uses exactly the same syntax for both the command and the response. - .. include:: hooks-bootp.rst .. include:: hooks-class-cmds.rst .. include:: hooks-cb-cmds.rst @@ -3597,6 +3596,7 @@ both the command and the response. .. include:: hooks-lease-query.rst .. include:: hooks-run-script.rst .. include:: hooks-ddns-tuning.rst +.. include:: hooks-limits.rst .. include:: hooks-rbac.rst .. _user-context-hooks: diff --git a/doc/sphinx/arm/rst_arm_sources.mk b/doc/sphinx/arm/rst_arm_sources.mk index 08671159aa..03ae98135c 100644 --- a/doc/sphinx/arm/rst_arm_sources.mk +++ b/doc/sphinx/arm/rst_arm_sources.mk @@ -21,6 +21,7 @@ rst_arm_sources += arm/hooks-ha.rst rst_arm_sources += arm/hooks-host-cache.rst rst_arm_sources += arm/hooks-lease-cmds.rst rst_arm_sources += arm/hooks-lease-query.rst +rst_arm_sources += arm/hooks-limits.rst rst_arm_sources += arm/hooks-radius.rst rst_arm_sources += arm/hooks-rbac.rst rst_arm_sources += arm/hooks-run-script.rst diff --git a/doc/sphinx/conf.py b/doc/sphinx/conf.py index ca3bb514f8..db3fa0f7b2 100644 --- a/doc/sphinx/conf.py +++ b/doc/sphinx/conf.py @@ -94,6 +94,7 @@ exclude_patterns = [ 'arm/hooks-host-cache.rst', 'arm/hooks-lease-cmds.rst', 'arm/hooks-lease-query.rst', + 'arm/hooks-limits.rst', 'arm/hooks-radius.rst', 'arm/hooks-rbac.rst', 'arm/hooks-run-script.rst', -- GitLab From fe38f4459196aa78c7a02b9a1cd9c289ea3e41b7 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Wed, 11 May 2022 14:23:51 +0300 Subject: [PATCH 07/13] [#562] add ChangeLog entry --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index c440cf1984..bd9e4c3596 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2020. [doc] andrei + Document the rate limiting feature of the new limits hook + library. It can apply a given limit of a certain number of + packets per time unit to a given client class or subnet. + (Gitlab #562, #1650) + 2019. [func] tmark Added new built-in class "SKIP_DDNS" which may be used in conjunction with the ddns-tuning hook library to to skip -- GitLab From 94de7d3da08b336beaacf9817c403200ec0a581c Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Thu, 19 May 2022 12:58:31 +0300 Subject: [PATCH 08/13] [#562] add LibLoadTest fixture --- src/lib/testutils/Makefile.am | 1 + src/lib/testutils/lib_load_test_fixture.h | 68 +++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 src/lib/testutils/lib_load_test_fixture.h diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index 30332da548..03d78fbab1 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -16,6 +16,7 @@ libkea_testutils_la_SOURCES += unix_control_client.cc unix_control_client.h libkea_testutils_la_SOURCES += user_context_utils.cc user_context_utils.h libkea_testutils_la_SOURCES += gtest_utils.h libkea_testutils_la_SOURCES += multi_threading_utils.h +libkea_testutils_la_SOURCES += lib_load_test_fixture.h libkea_testutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) libkea_testutils_la_LIBADD = $(top_builddir)/src/lib/asiolink/libkea-asiolink.la libkea_testutils_la_LIBADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/lib/testutils/lib_load_test_fixture.h b/src/lib/testutils/lib_load_test_fixture.h new file mode 100644 index 0000000000..3fe73ae20f --- /dev/null +++ b/src/lib/testutils/lib_load_test_fixture.h @@ -0,0 +1,68 @@ +// Copyright (C) 2022 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef ISC_TESTUTILS_LIB_LOAD_TEST_FIXTURE_H +#define ISC_TESTUTILS_LIB_LOAD_TEST_FIXTURE_H + +#include + +#include + +namespace isc { +namespace hooks { + +/// @brief Test fixture for testing loading and unloading of hook libraries. +struct LibLoadTest : ::testing::Test { + /// @brief Constructor. Unloads any previously loaded libraries. + LibLoadTest() { + unloadLibraries(); + } + + /// @brief Destructor. Unloads any previously loaded libraries. + ~LibLoadTest() { + unloadLibraries(); + } + + /// @brief Adds a library along with its parameters to the list of libraries to be loaded. + /// + /// @param library the path to the library to be loaded + /// @param parameters the library's parameters in Element format + void addLibrary(const std::string& library, isc::data::ConstElementPtr parameters) { + libraries_.push_back({library, parameters}); + } + + void clearLibraries() { + libraries_.clear(); + } + + /// @brief Load all libraries. + /// + /// @return true if all libraries loaded succesfully, false if one or more + /// libraries failed to load. + bool loadLibraries() { + bool result; + EXPECT_NO_THROW(result = HooksManager::loadLibraries(libraries_)); + return result; + } + + /// @brief Unloads all libraries. + /// + /// @return true if all libraries unloaded successfully, false if they + /// are still in memory. + bool unloadLibraries() { + bool result; + EXPECT_NO_THROW(result = HooksManager::unloadLibraries()); + return result; + } + + /// @brief Libraries + HookLibsCollection libraries_; +}; + +} // namespace hooks +} // namespace isc + +#endif // ISC_TESTUTILS_LIB_LOAD_TEST_FIXTURE_H -- GitLab From 51c3f84c09f20f4653b97af92b92937748c6d62f Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Thu, 19 May 2022 18:25:22 +0300 Subject: [PATCH 09/13] [#562] remove index used for sorting ClientClasses --- src/lib/dhcp/classify.h | 12 ------------ src/lib/dhcp/tests/pkt4_unittest.cc | 24 ------------------------ src/lib/dhcp/tests/pkt6_unittest.cc | 24 ------------------------ 3 files changed, 60 deletions(-) diff --git a/src/lib/dhcp/classify.h b/src/lib/dhcp/classify.h index 4f3a0c1401..c536ba3eb5 100644 --- a/src/lib/dhcp/classify.h +++ b/src/lib/dhcp/classify.h @@ -57,10 +57,6 @@ namespace dhcp { boost::multi_index::hashed_unique< boost::multi_index::tag, boost::multi_index::identity - >, - // Third index orders lexicographically. - boost::multi_index::ordered_unique< - boost::multi_index::identity > > > ClientClassContainer; @@ -137,14 +133,6 @@ namespace dhcp { } /// @} - /// @brief Returns an index that allows iteration through a sorted set - /// of all the client classes. - /// - /// @return the index iterable through range-based for looping - ClientClassContainer::nth_index<2>::type const& sorted() const { - return container_.get<2>(); - } - /// @brief returns if class x belongs to the defined classes /// /// @param x client class to be checked diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index 7029abfbd2..98a7d3c4b5 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/src/lib/dhcp/tests/pkt4_unittest.cc @@ -1386,28 +1386,4 @@ TEST_F(Pkt4Test, testSkipThisOptionError) { EXPECT_EQ("def", opstr->getValue()); } -// Check that sorted() allows sorted iteration. -TEST_F(Pkt4Test, sortedClientClasses) { - // Add a bunch of classes to a packet. - Pkt4 packet(DHCPDISCOVER, 1234); - packet.addClass("foo"); - packet.addClass("bar"); - packet.addClass("2"); - packet.addClass("10"); - - // Check that the usual iteration yields items in order of insertion. - std::string client_classes; - for (ClientClass const& c : packet.getClasses()) { - client_classes += c + ','; - } - EXPECT_EQ(client_classes, "ALL,foo,bar,2,10,"); - - // Check sorted() yields sorted items. - std::string sorted_client_classes; - for (ClientClass const& c : packet.getClasses().sorted()) { - sorted_client_classes += c + ','; - } - EXPECT_EQ(sorted_client_classes, "10,2,ALL,bar,foo,"); -} - } // end of anonymous namespace diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index 9afd8f2671..eca0f3fe50 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -2115,28 +2115,4 @@ TEST_F(Pkt6Test, relayDataOption) { EXPECT_EQ(orig_data, clone_data); } -// Check that sorted() allows sorted iteration. -TEST_F(Pkt6Test, sortedClientClasses) { - // Add a bunch of classes to a packet. - Pkt6 packet(DHCPV6_SOLICIT, 1234); - packet.addClass("foo"); - packet.addClass("bar"); - packet.addClass("2"); - packet.addClass("10"); - - // Check that the usual iteration yields items in order of insertion. - std::string client_classes; - for (ClientClass const& c : packet.getClasses()) { - client_classes += c + ','; - } - EXPECT_EQ(client_classes, "ALL,foo,bar,2,10,"); - - // Check sorted() yields sorted items. - std::string sorted_client_classes; - for (ClientClass const& c : packet.getClasses().sorted()) { - sorted_client_classes += c + ','; - } - EXPECT_EQ(sorted_client_classes, "10,2,ALL,bar,foo,"); -} - } // namespace -- GitLab From 1b01a6c0287e06fd52cdf20f9b61b7bb9a418e89 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Thu, 19 May 2022 19:26:34 +0300 Subject: [PATCH 10/13] [#562] remove unused dhcp_space.cc --- src/lib/util/Makefile.am | 2 +- src/lib/util/dhcp_space.cc | 35 ----------------------------------- src/lib/util/dhcp_space.h | 8 -------- 3 files changed, 1 insertion(+), 44 deletions(-) delete mode 100644 src/lib/util/dhcp_space.cc diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am index e7895ae9ed..932d21dda0 100644 --- a/src/lib/util/Makefile.am +++ b/src/lib/util/Makefile.am @@ -11,7 +11,7 @@ libkea_util_la_SOURCES = boost_time_utils.h boost_time_utils.cc libkea_util_la_SOURCES += buffer.h io_utilities.h libkea_util_la_SOURCES += chrono_time_utils.h chrono_time_utils.cc libkea_util_la_SOURCES += csv_file.h csv_file.cc -libkea_util_la_SOURCES += dhcp_space.cc dhcp_space.h +libkea_util_la_SOURCES += dhcp_space.h libkea_util_la_SOURCES += doubles.h libkea_util_la_SOURCES += file_utilities.h file_utilities.cc libkea_util_la_SOURCES += filename.h filename.cc diff --git a/src/lib/util/dhcp_space.cc b/src/lib/util/dhcp_space.cc deleted file mode 100644 index c6bbe31754..0000000000 --- a/src/lib/util/dhcp_space.cc +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (C) 2022 Internet Systems Consortium, Inc. ("ISC") -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#include - -#include - -namespace isc { -namespace util { - -template <> -uint16_t integerDhcpSpace() { - return 4; -} - -template <> -uint16_t integerDhcpSpace() { - return 6; -} - -template <> -char const* cStringDhcpSpace() { - return "4"; -} - -template <> -char const* cStringDhcpSpace() { - return "6"; -} - -} // namespace util -} // namespace isc diff --git a/src/lib/util/dhcp_space.h b/src/lib/util/dhcp_space.h index 2945eefef8..4c43a9c088 100644 --- a/src/lib/util/dhcp_space.h +++ b/src/lib/util/dhcp_space.h @@ -7,8 +7,6 @@ #ifndef ISC_DHCP_SPACE_H #define ISC_DHCP_SPACE_H 1 -#include - namespace isc { namespace util { @@ -17,12 +15,6 @@ enum DhcpSpace { DHCPv6, }; -template -constexpr uint16_t integerDhcpSpace(); - -template -constexpr char const* cStringDhcpSpace(); - } // namespace util } // namespace isc -- GitLab From a79bd5d5cd0a2948bd25d8e48ebbee3d118b2526 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 20 May 2022 14:34:54 +0300 Subject: [PATCH 11/13] [#562] document rate limiting --- doc/sphinx/arm/hooks-limits.rst | 112 +++++++++++++++++++++----------- src/lib/dhcp/packet_queue.h | 1 - 2 files changed, 74 insertions(+), 39 deletions(-) diff --git a/doc/sphinx/arm/hooks-limits.rst b/doc/sphinx/arm/hooks-limits.rst index b7330a9a1f..f7b438f820 100644 --- a/doc/sphinx/arm/hooks-limits.rst +++ b/doc/sphinx/arm/hooks-limits.rst @@ -9,45 +9,81 @@ The limits hook library is part of the subscription package. Configuration ~~~~~~~~~~~~~ -The library can be loaded by both ``kea-dhcp4`` and ``kea-dhcp6`` servers by adding its path along with any parameters to the ``"hooks-libraries"`` element of the server's configuration. Here is an example: - -.. code-block:: json - - { - "DhcpX": { - "hooks-libraries": [ - { - "library": "/usr/local/lib/libdhcp_limits.so" - "limits": [ - { - "client-classes": ["ALL"], - "rate-limit": "1000 packets per second" - }, - { - "client-classes": [ "bronze" ], - "rate-limit": "100 packets per minute" - } - { - "subnet-ids": [ 1, 2 ], - "rate-limit": "1 packet per second" - } - ] - } - ] - } - } - -There are two possible packet identification criteria: client classes and subnet IDs. For easier -configuration, you may provide multiple such criteria to a single rate limit. +The library can be loaded by both ``kea-dhcp4`` and ``kea-dhcp6`` servers by adding its path in the +``"hooks-libraries"`` element of the server's configuration. The rate limit can be specified in the format ``"

packets per "``. ``

`` is any number that can be represented by an unsigned integer on 32 bits i.e. between ``0`` and ``4,294,967,295``. ```` can be any of ``second``, ``minute``, ``hour``, ``day``, -``week``, ``month``, ``year``. ``month`` is considered 30 days for simplicity. Similarly, ``year`` -is considered 365 days. This syntax covers a high range of rates from one lease per year to four -billion leases per second. - -The configured value of ``0`` packets can be a convenient way of disabling packet processing for -certain clients entirely. As such, it means its literary value and is not a special value for -disabling rate limiting. Disabling limiting altogether is achieved by removing the leaf -configuration entry, the map around it or the entire hook library configuration. +``week``, ``month``, ``year``. ``month`` is considered to be 30 days for simplicity. Similarly, +``year`` is 365 days for all intents and purposes of limiting. This syntax covers a high range of +rates from one lease per year to four billion leases per second. + +The configured value of ``0`` packets is a convenient way of disabling packet processing for certain clients entirely. As such, it means its literary value and is not a special value for disabling +limiting altogether as it might be imagined. Disabling limiting altogether is achieved by removing +the `"rate-limit"` leaf configuration entry, the `"limits"` map around it or the user context around +it or the hook library configuration. + +There are two ways to configure what packets get limited. One is through the client classes that are +initially assigned to the packet. In this case, the limit is configured directly in the user context +of the client class definition. As such, if a client class has no dependencies on host reservations, +or on the special `"BOOTP"` or `"KNOWN"` classes, and is not marked with `"only-if-required"`, if +the client class gets assigned to the packet, and it has a configured limit, the packet will be +limited accordingly. + +.. note:: + + It can be tempting to think that assigning a rate limit of `n` packets per time unit results in + `n` DORA or `n` SARR exchanges. By default, all inbound packets are counted. That means that + a full message exchange amounts for 2 packets. To achieve the desired effect of counting an + exchange only once, you may use client class rate limiting with a test expression that binds + `pkt4.msgtype` to DHCPDISCOVER messages or `pkt6.msgtype` to SOLICIT messages. + +The other way is through the subnet that the Kea selects for the packet. To achieve this, the limit +is added to the user context of the subnet definition. + +.. note:: + + Subnet reselection mechanisms like the one that can be enabled in the RADIUS hook, can offer the + illusion that a packet should have been limited, when in reality it was not. The subnet that is + taken into account depends on which hook library is configured first. + +The following is an example that adds one rate limit on a client class and another one in a subnet +in `kea-dhcp6`. A valid configuration for `kea-dhcp4` can be easily extrapolated. + +.. code-block:: json + + { + "Dhcp6": { + "client-classes": [ + { + "name": "gold", + "user-context": { + "limits": { + "rate-limit": "1000 packets per second" + } + } + } + ], + "hooks-libraries": [ + { + "library": "/usr/local/lib/libdhcp_limits.so" + } + ], + "subnet6": [ + { + "id": 1, + "pools": [ + { + "pool": "2001:db8::/64" + } + ], + "subnet": "2001:db8::/64", + "user-context": { + "limits": { + "rate-limit": "10 packets per minute" + } + } + } + ] + } diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h index f755910246..f6ec0b5531 100644 --- a/src/lib/dhcp/packet_queue.h +++ b/src/lib/dhcp/packet_queue.h @@ -12,7 +12,6 @@ #include #include -#include #include namespace isc { -- GitLab From 07e345b672f466c01057808d932483e2672ba0e1 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 20 May 2022 09:06:08 -0400 Subject: [PATCH 12/13] [#562] Minor edits to ARM modified: hooks-limits.rst --- doc/sphinx/arm/hooks-limits.rst | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/doc/sphinx/arm/hooks-limits.rst b/doc/sphinx/arm/hooks-limits.rst index f7b438f820..67816b91c0 100644 --- a/doc/sphinx/arm/hooks-limits.rst +++ b/doc/sphinx/arm/hooks-limits.rst @@ -19,23 +19,24 @@ number that can be represented by an unsigned integer on 32 bits i.e. between `` ``year`` is 365 days for all intents and purposes of limiting. This syntax covers a high range of rates from one lease per year to four billion leases per second. -The configured value of ``0`` packets is a convenient way of disabling packet processing for certain clients entirely. As such, it means its literary value and is not a special value for disabling +The configured value of ``0`` packets is a convenient way of disabling packet processing for certain +clients entirely. As such, it means its literal value and is not a special value for disabling limiting altogether as it might be imagined. Disabling limiting altogether is achieved by removing the `"rate-limit"` leaf configuration entry, the `"limits"` map around it or the user context around it or the hook library configuration. -There are two ways to configure what packets get limited. One is through the client classes that are -initially assigned to the packet. In this case, the limit is configured directly in the user context -of the client class definition. As such, if a client class has no dependencies on host reservations, -or on the special `"BOOTP"` or `"KNOWN"` classes, and is not marked with `"only-if-required"`, if -the client class gets assigned to the packet, and it has a configured limit, the packet will be -limited accordingly. +There are two ways to configure which packets get limited. One is through the client classes that are +initially assigned to the packet. In this case, the limit is configured in the user context +in the client class definition. Class rate limits are checked early in packet processing cycle +and is thus limited to those classes which are assigned to the packet via test expression and that +do not depend on host reservations, the special "BOOTP" or "KNOWN" classes, and is not marked with +"only-if-required". .. note:: It can be tempting to think that assigning a rate limit of `n` packets per time unit results in `n` DORA or `n` SARR exchanges. By default, all inbound packets are counted. That means that - a full message exchange amounts for 2 packets. To achieve the desired effect of counting an + a full message exchange accounts for 2 packets. To achieve the desired effect of counting an exchange only once, you may use client class rate limiting with a test expression that binds `pkt4.msgtype` to DHCPDISCOVER messages or `pkt6.msgtype` to SOLICIT messages. @@ -44,9 +45,11 @@ is added to the user context of the subnet definition. .. note:: - Subnet reselection mechanisms like the one that can be enabled in the RADIUS hook, can offer the - illusion that a packet should have been limited, when in reality it was not. The subnet that is - taken into account depends on which hook library is configured first. + Subnet rate limits are enforced only on the initially selected subnet for a given packet. + If the selected subnet is subsequently changed, as may be the case for subnets in a + shared-network or when reselection is enabled in libraries such as the RADIUS hook, rate + limits on the newly selected subnet will be ignored. In other words, packets are gated + only by the rate limit on the original subnet. The following is an example that adds one rate limit on a client class and another one in a subnet in `kea-dhcp6`. A valid configuration for `kea-dhcp4` can be easily extrapolated. -- GitLab From 318cc63b118106c670bae09e29544ebd5529d62a Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 20 May 2022 20:02:35 +0300 Subject: [PATCH 13/13] [#562] fix warning about uninitialized variable in LibLoadTest fixture --- src/lib/testutils/lib_load_test_fixture.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/testutils/lib_load_test_fixture.h b/src/lib/testutils/lib_load_test_fixture.h index 3fe73ae20f..7f2a90b104 100644 --- a/src/lib/testutils/lib_load_test_fixture.h +++ b/src/lib/testutils/lib_load_test_fixture.h @@ -12,7 +12,7 @@ #include namespace isc { -namespace hooks { +namespace test { /// @brief Test fixture for testing loading and unloading of hook libraries. struct LibLoadTest : ::testing::Test { @@ -43,8 +43,8 @@ struct LibLoadTest : ::testing::Test { /// @return true if all libraries loaded succesfully, false if one or more /// libraries failed to load. bool loadLibraries() { - bool result; - EXPECT_NO_THROW(result = HooksManager::loadLibraries(libraries_)); + bool result(false); + EXPECT_NO_THROW(result = isc::hooks::HooksManager::loadLibraries(libraries_)); return result; } @@ -53,16 +53,16 @@ struct LibLoadTest : ::testing::Test { /// @return true if all libraries unloaded successfully, false if they /// are still in memory. bool unloadLibraries() { - bool result; - EXPECT_NO_THROW(result = HooksManager::unloadLibraries()); + bool result(false); + EXPECT_NO_THROW(result = isc::hooks::HooksManager::unloadLibraries()); return result; } /// @brief Libraries - HookLibsCollection libraries_; + isc::hooks::HookLibsCollection libraries_; }; -} // namespace hooks +} // namespace test } // namespace isc #endif // ISC_TESTUTILS_LIB_LOAD_TEST_FIXTURE_H -- GitLab