From: Francis Dupont Date: Wed, 1 Jun 2022 17:30:13 +0000 (+0200) Subject: [#1706] Added a command filtering feature X-Git-Tag: Kea-2.1.7~44 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=79dbc484395891ed2ecf27db3011765529aa22e5;p=thirdparty%2Fkea.git [#1706] Added a command filtering feature --- diff --git a/src/lib/config/cmd_response_creator.cc b/src/lib/config/cmd_response_creator.cc index 92a29012fa..c586d98b88 100644 --- a/src/lib/config/cmd_response_creator.cc +++ b/src/lib/config/cmd_response_creator.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2021-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 @@ -8,20 +8,25 @@ #include #include -#include +#include +#include #include #include #include #include +using namespace isc::config; using namespace isc::data; using namespace isc::http; +using namespace std; namespace isc { namespace config { HttpAuthConfigPtr CmdResponseCreator::http_auth_config_; +unordered_set CmdResponseCreator::command_accept_list_; + HttpRequestPtr CmdResponseCreator::createNewHttpRequest() const { return (HttpRequestPtr(new PostHttpRequestJson())); @@ -58,8 +63,7 @@ createStockHttpResponseInternal(const HttpRequestPtr& request, } HttpResponsePtr -CmdResponseCreator:: -createDynamicHttpResponse(HttpRequestPtr request) { +CmdResponseCreator::createDynamicHttpResponse(HttpRequestPtr request) { HttpResponseJsonPtr http_response; // Check the basic HTTP authentication. @@ -87,6 +91,12 @@ createDynamicHttpResponse(HttpRequestPtr request) { // to getBodyAsJson must not trigger an exception. ConstElementPtr command = request_json->getBodyAsJson(); + // Filter the command. + http_response = filterCommand(request, command, command_accept_list_); + if (http_response) { + return (http_response); + } + // Process command doesn't generate exceptions but can possibly return // null response, if the handler is not implemented properly. This is // again an internal server issue. @@ -116,5 +126,51 @@ createDynamicHttpResponse(HttpRequestPtr request) { return (http_response); } +HttpResponseJsonPtr +CmdResponseCreator::filterCommand(const HttpRequestPtr& request, + const ConstElementPtr& body, + const unordered_set& accept) { + HttpResponseJsonPtr response; + if (!body || accept.empty()) { + return (response); + } + if (body->getType() != Element::map) { + return (response); + } + ConstElementPtr elem = body->get(CONTROL_COMMAND); + if (!elem || (elem->getType() != Element::string)) { + return (response); + } + string command = elem->stringValue(); + if (command.empty() || accept.count(command)) { + return (response); + } + + // Reject the command. + LOG_DEBUG(command_logger, DBG_COMMAND, + COMMAND_HTTP_LISTENER_COMMAND_REJECTED) + .arg(command) + .arg(request->getRemote()); + // From CtrlAgentResponseCreator::createStockHttpResponseInternal. + HttpVersion http_version(request->context()->http_version_major_, + request->context()->http_version_minor_); + if ((http_version < HttpVersion(1, 0)) || + (HttpVersion(1, 1) < http_version)) { + http_version.major_ = 1; + http_version.minor_ = 0; + } + HttpStatusCode status_code = HttpStatusCode::FORBIDDEN; + response.reset(new HttpResponseJson(http_version, status_code)); + ElementPtr response_body = Element::createMap(); + uint16_t result = HttpResponse::statusCodeToNumber(status_code); + response_body->set(CONTROL_RESULT, + Element::create(static_cast(result))); + const string& text = HttpResponse::statusCodeToString(status_code); + response_body->set(CONTROL_TEXT, Element::create(text)); + response->setBodyAsJson(response_body); + response->finalize(); + return (response); +} + } // end of namespace isc::config } // end of namespace isc diff --git a/src/lib/config/cmd_response_creator.h b/src/lib/config/cmd_response_creator.h index 4b1d819713..cdb33ccec0 100644 --- a/src/lib/config/cmd_response_creator.h +++ b/src/lib/config/cmd_response_creator.h @@ -10,6 +10,7 @@ #include #include #include +#include namespace isc { namespace config { @@ -65,11 +66,29 @@ public: return (emulate_agent_response_); } + /// @brief Filter commands. + /// + /// From RBAC code: if the access list is empty or the command + /// cannot be found just return. + /// + /// @param request The HTTP request (for the HTTP version). + /// @param body The request body. + /// @param accept The accept access list. + http::HttpResponseJsonPtr + filterCommand(const http::HttpRequestPtr& request, + const data::ConstElementPtr& body, + const std::unordered_set& accept); + /// @brief The server current authentication configuration. /// /// Default to the empty HttpAuthConfigPtr. static http::HttpAuthConfigPtr http_auth_config_; + /// @brief The server command accept list. + /// + /// Default to the empty list which means to accept everything. + static std::unordered_set command_accept_list_; + private: /// @brief Creates un-finalized stock HTTP response. diff --git a/src/lib/config/config_messages.cc b/src/lib/config/config_messages.cc index 87af1265d3..f2754a1f1f 100644 --- a/src/lib/config/config_messages.cc +++ b/src/lib/config/config_messages.cc @@ -10,6 +10,7 @@ namespace config { extern const isc::log::MessageID COMMAND_ACCEPTOR_START = "COMMAND_ACCEPTOR_START"; extern const isc::log::MessageID COMMAND_DEREGISTERED = "COMMAND_DEREGISTERED"; extern const isc::log::MessageID COMMAND_EXTENDED_REGISTERED = "COMMAND_EXTENDED_REGISTERED"; +extern const isc::log::MessageID COMMAND_HTTP_LISTENER_COMMAND_REJECTED = "COMMAND_HTTP_LISTENER_COMMAND_REJECTED"; extern const isc::log::MessageID COMMAND_HTTP_LISTENER_STARTED = "COMMAND_HTTP_LISTENER_STARTED"; extern const isc::log::MessageID COMMAND_HTTP_LISTENER_STOPPED = "COMMAND_HTTP_LISTENER_STOPPED"; extern const isc::log::MessageID COMMAND_HTTP_LISTENER_STOPPING = "COMMAND_HTTP_LISTENER_STOPPING"; @@ -43,6 +44,7 @@ const char* values[] = { "COMMAND_ACCEPTOR_START", "Starting to accept connections via unix domain socket bound to %1", "COMMAND_DEREGISTERED", "Command %1 deregistered", "COMMAND_EXTENDED_REGISTERED", "Command %1 registered", + "COMMAND_HTTP_LISTENER_COMMAND_REJECTED", "Command HTTP listener rejected command '%1' from '%2'", "COMMAND_HTTP_LISTENER_STARTED", "Command HTTP listener started with %1 threads, listening on %2:%3, use TLS: %4", "COMMAND_HTTP_LISTENER_STOPPED", "Command HTTP listener for %1:%2 stopped.", "COMMAND_HTTP_LISTENER_STOPPING", "Stopping Command HTTP listener for %1:%2", diff --git a/src/lib/config/config_messages.h b/src/lib/config/config_messages.h index 06a88f5bb0..3aac871c75 100644 --- a/src/lib/config/config_messages.h +++ b/src/lib/config/config_messages.h @@ -11,6 +11,7 @@ namespace config { extern const isc::log::MessageID COMMAND_ACCEPTOR_START; extern const isc::log::MessageID COMMAND_DEREGISTERED; extern const isc::log::MessageID COMMAND_EXTENDED_REGISTERED; +extern const isc::log::MessageID COMMAND_HTTP_LISTENER_COMMAND_REJECTED; extern const isc::log::MessageID COMMAND_HTTP_LISTENER_STARTED; extern const isc::log::MessageID COMMAND_HTTP_LISTENER_STOPPED; extern const isc::log::MessageID COMMAND_HTTP_LISTENER_STOPPING; diff --git a/src/lib/config/config_messages.mes b/src/lib/config/config_messages.mes index 840f713eeb..00e6726edd 100644 --- a/src/lib/config/config_messages.mes +++ b/src/lib/config/config_messages.mes @@ -23,6 +23,10 @@ This debug message indicates that the daemon started supporting specified command. The handler for the registered command includes a parameter holding entire command to be processed. +% COMMAND_HTTP_LISTENER_COMMAND_REJECTED Command HTTP listener rejected command '%1' from '%2' +This debug messages is issued when a command is rejected. Arguments detail +the command and the address the request was received from. + % COMMAND_HTTP_LISTENER_STARTED Command HTTP listener started with %1 threads, listening on %2:%3, use TLS: %4 This debug messages is issued when an HTTP listener has been started to accept connections from Command API clients through which commands can be diff --git a/src/lib/config/tests/cmd_response_creator_factory_unittests.cc b/src/lib/config/tests/cmd_response_creator_factory_unittests.cc index a7c0325bd9..50f2dd8c46 100644 --- a/src/lib/config/tests/cmd_response_creator_factory_unittests.cc +++ b/src/lib/config/tests/cmd_response_creator_factory_unittests.cc @@ -33,6 +33,9 @@ TEST(CmdResponseCreatorFactory, createDefault) { // Authorization configuration should be an empty pointer. EXPECT_FALSE(CmdResponseCreator::http_auth_config_); + // By default all commands are accepted. + EXPECT_TRUE(CmdResponseCreator::command_accept_list_.empty()); + // Invoke create() again. CmdResponseCreatorPtr response2; ASSERT_NO_THROW(response2 = boost::dynamic_pointer_cast< @@ -60,6 +63,9 @@ TEST(CmdResponseCreatorFactory, createAgentEmulationDisabled) { // Authorization configuration should be an empty pointer. EXPECT_FALSE(CmdResponseCreator::http_auth_config_); + + // By default all commands are accepted. + EXPECT_TRUE(CmdResponseCreator::command_accept_list_.empty()); } } // end of anonymous namespace diff --git a/src/lib/config/tests/cmd_response_creator_unittests.cc b/src/lib/config/tests/cmd_response_creator_unittests.cc index d2126e5e88..7ab27c081f 100644 --- a/src/lib/config/tests/cmd_response_creator_unittests.cc +++ b/src/lib/config/tests/cmd_response_creator_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2021-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 @@ -21,6 +21,7 @@ using namespace isc; using namespace isc::config; using namespace isc::data; using namespace isc::http; +using namespace std; namespace ph = std::placeholders; namespace { @@ -41,6 +42,9 @@ public: config::CommandMgr::instance(). registerCommand("foo", std::bind(&CmdResponseCreatorTest::fooCommandHandler, this, ph::_1, ph::_2)); + // Clear class variables. + CmdResponseCreator::http_auth_config_.reset(); + CmdResponseCreator::command_accept_list_.clear(); } /// @brief Destructor. @@ -48,6 +52,8 @@ public: /// Removes registered commands from the command manager. virtual ~CmdResponseCreatorTest() { config::CommandMgr::instance().deregisterAll(); + CmdResponseCreator::http_auth_config_.reset(); + CmdResponseCreator::command_accept_list_.clear(); } /// @brief SetUp function that wraps call to initCreator. @@ -95,7 +101,7 @@ public: /// @param must_contain Text that must be present in the textual /// representation of the generated response. void testStockResponse(const HttpStatusCode& status_code, - const std::string& must_contain) { + const string& must_contain) { HttpResponsePtr response; ASSERT_NO_THROW( response = response_creator_->createStockHttpResponse(request_, @@ -106,7 +112,7 @@ public: HttpResponseJson>(response); ASSERT_TRUE(response_json); // Make sure the response contains the string specified as argument. - EXPECT_TRUE(response_json->toString().find(must_contain) != std::string::npos); + EXPECT_TRUE(response_json->toString().find(must_contain) != string::npos); } @@ -116,7 +122,7 @@ public: /// @param command_arguments Command arguments (empty). /// /// @return Returns response with a single string "bar". - ConstElementPtr fooCommandHandler(const std::string& /*command_name*/, + ConstElementPtr fooCommandHandler(const string& /*command_name*/, const ConstElementPtr& /*command_arguments*/) { ElementPtr arguments = Element::createList(); arguments->add(Element::create("bar")); @@ -189,11 +195,11 @@ TEST_F(CmdResponseCreatorTest, createDynamicHttpResponse) { // Response must be successful. EXPECT_TRUE(response_json->toString().find("HTTP/1.1 200 OK") != - std::string::npos); + string::npos); // Response must contain JSON body with "result" of 0. EXPECT_TRUE(response_json->toString().find("\"result\": 0") != - std::string::npos); + string::npos); } // Test successful server response without emulating agent response. @@ -225,11 +231,11 @@ TEST_F(CmdResponseCreatorTest, createDynamicHttpResponseNoEmulation) { // Response must be successful. EXPECT_TRUE(response_json->toString().find("HTTP/1.1 200 OK") != - std::string::npos); + string::npos); // Response must contain JSON body with "result" of 0. EXPECT_TRUE(response_json->toString().find("\"result\": 0") != - std::string::npos); + string::npos); } // This test verifies that Internal Server Error is returned when invalid C++ @@ -255,7 +261,178 @@ TEST_F(CmdResponseCreatorTest, createDynamicHttpResponseInvalidType) { // Response must contain Internal Server Error status code. EXPECT_TRUE(response_json->toString().find("HTTP/1.1 500 Internal Server Error") != - std::string::npos); + string::npos); +} + +// This test verifies command filtering. +TEST_F(CmdResponseCreatorTest, filterCommand) { + initCreator(false); + setBasicContext(request_); + // For the log message... + request_->setRemote("127.0.0.1"); + + HttpResponseJsonPtr response; + ConstElementPtr body; + unordered_set accept; + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_FALSE(response); + + accept.insert("foo"); + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_FALSE(response); + + body = Element::createList(); + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_FALSE(response); + + body = Element::createMap(); + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_FALSE(response); + + body = createCommand("foo", ConstElementPtr()); + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_FALSE(response); + + body = createCommand("bar", ConstElementPtr()); + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_TRUE(response); + EXPECT_EQ("HTTP/1.1 403 Forbidden", response->toBriefString()); + + accept.clear(); + ASSERT_NO_THROW(response = response_creator_->filterCommand(request_, body, accept)); + EXPECT_FALSE(response); +} + +// This test verifies basic HTTP authentication - reject case. +// Empty case was handled in createDynamicHttpResponseNoEmulation. +TEST_F(CmdResponseCreatorTest, basicAuthReject) { + initCreator(false); + setBasicContext(request_); + + // Body: "foo" command has been registered in the test fixture constructor. + request_->context()->body_ = "{ \"command\": \"foo\" }"; + + // Add no basic HTTP authentication. + + // All requests must be finalized before they can be processed. + ASSERT_NO_THROW(request_->finalize()); + + // Create basic HTTP authentication configuration. + CmdResponseCreator::http_auth_config_.reset(new BasicHttpAuthConfig()); + BasicHttpAuthConfigPtr basic = + boost::dynamic_pointer_cast( + CmdResponseCreator::http_auth_config_); + ASSERT_TRUE(basic); + EXPECT_NO_THROW(basic->add("test", "", "123\xa3", "")); + + // Create response from the request. + HttpResponsePtr response; + ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); + ASSERT_TRUE(response); + + // Response must not be successful. + EXPECT_EQ("HTTP/1.1 401 Unauthorized", response->toBriefString()); +} + +// This test verifies basic HTTP authentication - accept case. +// Empty case was handled in createDynamicHttpResponseNoEmulation. +TEST_F(CmdResponseCreatorTest, basicAuthAccept) { + initCreator(false); + setBasicContext(request_); + + // Body: "foo" command has been registered in the test fixture constructor. + request_->context()->body_ = "{ \"command\": \"foo\" }"; + + // Add basic HTTP authentication. + HttpHeaderContext auth("Authorization", "Basic dGVzdDoxMjPCow=="); + request_->context()->headers_.push_back(auth); + + // All requests must be finalized before they can be processed. + ASSERT_NO_THROW(request_->finalize()); + + // Create basic HTTP authentication configuration. + CmdResponseCreator::http_auth_config_.reset(new BasicHttpAuthConfig()); + BasicHttpAuthConfigPtr basic = + boost::dynamic_pointer_cast( + CmdResponseCreator::http_auth_config_); + ASSERT_TRUE(basic); + EXPECT_NO_THROW(basic->add("test", "", "123\xa3", "")); + + // Create response from the request. + HttpResponsePtr response; + ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); + ASSERT_TRUE(response); + + // Response must be convertible to HttpResponseJsonPtr. + HttpResponseJsonPtr response_json = boost::dynamic_pointer_cast< + HttpResponseJson>(response); + ASSERT_TRUE(response_json); + + // Response must be successful. + EXPECT_TRUE(response_json->toString().find("HTTP/1.1 200 OK") != + string::npos); + + // Response must contain JSON body with "result" of 0. + EXPECT_TRUE(response_json->toString().find("\"result\": 0") != + string::npos); +} + +// This test verifies command filtering at the HTTP level - reject case. +TEST_F(CmdResponseCreatorTest, filterCommandReject) { + initCreator(false); + setBasicContext(request_); + // For the log message... + request_->setRemote("127.0.0.1"); + + // Body: "bar" command has been registered in the test fixture constructor. + request_->context()->body_ = "{ \"command\": \"bar\" }"; + + // All requests must be finalized before they can be processed. + ASSERT_NO_THROW(request_->finalize()); + + // Add foo in the access list. + CmdResponseCreator::command_accept_list_.insert("foo"); + + // Create response from the request. + HttpResponsePtr response; + ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); + ASSERT_TRUE(response); + + // Response must not be successful. + EXPECT_EQ("HTTP/1.1 403 Forbidden", response->toBriefString()); +} + +// This test verifies command filtering at the HTTP level - accept case. +TEST_F(CmdResponseCreatorTest, filterCommandAccept) { + initCreator(false); + setBasicContext(request_); + + // Body: "foo" command has been registered in the test fixture constructor. + request_->context()->body_ = "{ \"command\": \"foo\" }"; + + // All requests must be finalized before they can be processed. + ASSERT_NO_THROW(request_->finalize()); + + // Add foo in the access list. + CmdResponseCreator::command_accept_list_.insert("foo"); + + // Create response from the request. + HttpResponsePtr response; + ASSERT_NO_THROW(response = response_creator_->createHttpResponse(request_)); + ASSERT_TRUE(response); + + // Response must be convertible to HttpResponseJsonPtr. + HttpResponseJsonPtr response_json = boost::dynamic_pointer_cast< + HttpResponseJson>(response); + ASSERT_TRUE(response_json); + + // Response must be successful. + EXPECT_TRUE(response_json->toString().find("HTTP/1.1 200 OK") != + string::npos); + + // Response must contain JSON body with "result" of 0. + EXPECT_TRUE(response_json->toString().find("\"result\": 0") != + string::npos); } }