From: Tomek Mrugalski Date: Fri, 8 Jun 2018 14:07:14 +0000 (+0200) Subject: [5646] Dedicated checks for truncated client-id, server-id added X-Git-Tag: 136-add-global-host-reservation-examples_base~4^2~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4545bb9fe6a5c52d3532020a4095b2230580549;p=thirdparty%2Fkea.git [5646] Dedicated checks for truncated client-id, server-id added --- diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index f5ef0d8af5..5fd0b1b71a 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1260,18 +1260,23 @@ Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid, RequirementLevel serverid) { OptionCollection client_ids = pkt->getOptions(D6O_CLIENTID); switch (clientid) { - case MANDATORY: + case MANDATORY: { if (client_ids.size() != 1) { isc_throw(RFCViolation, "Exactly 1 client-id option expected in " << pkt->getName() << ", but " << client_ids.size() << " received"); } + sanityCheckDUID(client_ids.begin()->second, "client-id"); break; + } case OPTIONAL: if (client_ids.size() > 1) { isc_throw(RFCViolation, "Too many (" << client_ids.size() << ") client-id options received in " << pkt->getName()); } + if (!client_ids.empty()) { + sanityCheckDUID(client_ids.begin()->second, "client-id"); + } break; case FORBIDDEN: @@ -1294,6 +1299,7 @@ Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid, << server_ids.size() << "), exactly 1 expected in message " << pkt->getName()); } + sanityCheckDUID(server_ids.begin()->second, "server-id"); break; case OPTIONAL: @@ -1301,6 +1307,23 @@ Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid, isc_throw(RFCViolation, "Too many (" << server_ids.size() << ") server-id options received in " << pkt->getName()); } + if (!server_ids.empty()) { + sanityCheckDUID(server_ids.begin()->second, "server-id"); + } + } +} + +void Dhcpv6Srv::sanityCheckDUID(const OptionPtr& opt, const std::string& opt_name) { + if (!opt) { + isc_throw(RFCViolation, "Unable to find expected option " << opt_name); + } + + // The client-id or server-id has to have at least 3 bytes of useful data: + // two for duid type and one more for actual duid value. + uint16_t len = opt->len() - opt->getHeaderLen(); + if (len < 3) { + isc_throw(RFCViolation, "Received empty or truncated " << opt_name << " option: " + << len << " byte(s) only"); } } diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index 6debce96ac..c8a1ec93d8 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -225,6 +225,13 @@ protected: void sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid, RequirementLevel serverid); + /// @brief verifies if received DUID option (client-id or server-id) is sane + /// + /// @param opt option to be checked + /// @param opt_name text name to be printed + /// @throw RFCViolation if any issues are detected + void sanityCheckDUID(const OptionPtr& opt, const std::string& opt_name); + /// @brief Processes incoming Solicit and returns response. /// /// Processes received Solicit message and verifies that its sender diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 393d1bd4d7..8a8d1708b0 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -1167,6 +1167,56 @@ TEST_F(Dhcpv6SrvTest, sanityCheck) { EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::MANDATORY), RFCViolation); } + +// This test verifies if the sanityCheck() really checks options content, +// especially truncated client-id. +TEST_F(Dhcpv6SrvTest, truncatedClientId) { + NakedDhcpv6Srv srv(0); + + Pkt6Ptr pkt = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)); + + // Case 1: completely empty (size 0) + pkt->addOption(generateClientId(0)); + EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN), + RFCViolation); + + // Case 2: too short (at the very least 3 bytes are needed) + pkt->delOption(D6O_CLIENTID); + pkt->addOption(generateClientId(2)); + EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN), + RFCViolation); + + // Case 3: the shortest DUID possible (3 bytes) is ok: + pkt->delOption(D6O_CLIENTID); + pkt->addOption(generateClientId(3)); + EXPECT_NO_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::MANDATORY, Dhcpv6Srv::FORBIDDEN)); +} + +// This test verifies if the sanityCheck() really checks options content, +// especially truncated server-id. +TEST_F(Dhcpv6SrvTest, truncatedServerId) { + NakedDhcpv6Srv srv(0); + + Pkt6Ptr pkt = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)); + + // Case 1: completely empty (size 0) + pkt->addOption(generateServerId(0)); + EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::FORBIDDEN, Dhcpv6Srv::MANDATORY), + RFCViolation); + + // Case 2: too short (at the very least 3 bytes are needed) + pkt->delOption(D6O_SERVERID); + pkt->addOption(generateServerId(2)); + EXPECT_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::FORBIDDEN, Dhcpv6Srv::MANDATORY), + RFCViolation); + + // Case 3: the shortest DUID possible (3 bytes) is ok: + pkt->delOption(D6O_SERVERID); + pkt->addOption(generateServerId(3)); + EXPECT_NO_THROW(srv.sanityCheck(pkt, Dhcpv6Srv::FORBIDDEN, Dhcpv6Srv::MANDATORY)); +} + + // Check that the server is testing if server identifier received in the // query, matches server identifier used by the server. TEST_F(Dhcpv6SrvTest, testServerID) { diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h index da51036bb8..71a23d8db2 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -312,6 +312,12 @@ public: // Generate client-id option isc::dhcp::OptionPtr generateClientId(size_t duid_size = 32) { + if (duid_size == 0) { + return (isc::dhcp::OptionPtr + (new isc::dhcp::Option(isc::dhcp::Option::V6, D6O_CLIENTID))); + + } + isc::dhcp::OptionBuffer clnt_duid(duid_size); for (size_t i = 0; i < duid_size; i++) { clnt_duid[i] = 100 + i; @@ -325,6 +331,29 @@ public: clnt_duid.begin() + duid_size))); } + /// Generate server-id option + /// @param duid_size size of the duid + isc::dhcp::OptionPtr generateServerId(size_t duid_size = 32) { + + if (duid_size == 0) { + return (isc::dhcp::OptionPtr + (new isc::dhcp::Option(isc::dhcp::Option::V6, D6O_SERVERID))); + + } + + isc::dhcp::OptionBuffer clnt_duid(duid_size); + for (size_t i = 0; i < duid_size; i++) { + clnt_duid[i] = 100 + i; + } + + duid_ = isc::dhcp::DuidPtr(new isc::dhcp::DUID(clnt_duid)); + + return (isc::dhcp::OptionPtr + (new isc::dhcp::Option(isc::dhcp::Option::V6, D6O_SERVERID, + clnt_duid.begin(), + clnt_duid.begin() + duid_size))); + } + // Checks if server response (ADVERTISE or REPLY) includes proper // server-id. void checkServerId(const isc::dhcp::Pkt6Ptr& rsp,