]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[5458] Make sanityCheck call compacted.
authorMarcin Siodelski <marcin@isc.org>
Fri, 27 Apr 2018 09:20:36 +0000 (11:20 +0200)
committerMarcin Siodelski <marcin@isc.org>
Fri, 27 Apr 2018 09:20:36 +0000 (11:20 +0200)
src/bin/dhcp6/dhcp6_srv.cc
src/bin/dhcp6/dhcp6_srv.h
src/bin/dhcp6/tests/confirm_unittest.cc
src/bin/dhcp6/tests/rebind_unittest.cc

index 473039ba55863cf5b73102934cf9bc75046ccfbb..151a3e274bb87f10664faeaca6b84456477efa65 100644 (file)
@@ -647,28 +647,8 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) {
         callout_handle->getArgument("query6", query);
     }
 
-    try {
-        if (!sanityCheck(query)) {
-            // We received a packet type that we do not recognize.
-            LOG_DEBUG(bad_packet6_logger, DBG_DHCP6_BASIC,
-                      DHCP6_UNKNOWN_MSG_RECEIVED)
-                .arg(static_cast<int>(query->getType()))
-                .arg(query->getIface());
-            // Increase the statistic of dropped packets.
-            StatsMgr::instance().addValue("pkt6-receive-drop",
-                                          static_cast<int64_t>(1));
-            return;
-        }
-                
-    } catch (const RFCViolation& e) {
-        LOG_DEBUG(bad_packet6_logger, DBG_DHCP6_BASIC, DHCP6_REQUIRED_OPTIONS_CHECK_FAIL)
-            .arg(query->getName())
-            .arg(query->getRemoteAddr().toText())
-            .arg(e.what());
-
-        // Increase the statistic of dropped packets.
-        StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
-
+    // Reject the message if it doesn't pass the sanity check.
+    if (!sanityCheck(query)) {
         return;
     }
 
@@ -1249,28 +1229,44 @@ Dhcpv6Srv::appendRequestedVendorOptions(const Pkt6Ptr& question,
 
 bool
 Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt) {
-    switch (pkt->getType()) {
-    case DHCPV6_SOLICIT:
-    case DHCPV6_REBIND:
+    try {
+        switch (pkt->getType()) {
+        case DHCPV6_SOLICIT:
+        case DHCPV6_REBIND:
     case DHCPV6_CONFIRM:
-        sanityCheck(pkt, MANDATORY, FORBIDDEN);
-        return (true);
+            sanityCheck(pkt, MANDATORY, FORBIDDEN);
+            return (true);
 
-    case DHCPV6_REQUEST:
-    case DHCPV6_RENEW:
-    case DHCPV6_RELEASE:
-    case DHCPV6_DECLINE:
-        sanityCheck(pkt, MANDATORY, MANDATORY);
-        return (true);
+        case DHCPV6_REQUEST:
+        case DHCPV6_RENEW:
+        case DHCPV6_RELEASE:
+        case DHCPV6_DECLINE:
+            sanityCheck(pkt, MANDATORY, MANDATORY);
+            return (true);
 
-    case DHCPV6_INFORMATION_REQUEST:
-    case DHCPV6_DHCPV4_QUERY:
-        sanityCheck(pkt, OPTIONAL, OPTIONAL);
-        return (true);
+        case DHCPV6_INFORMATION_REQUEST:
+        case DHCPV6_DHCPV4_QUERY:
+            sanityCheck(pkt, OPTIONAL, OPTIONAL);
+            return (true);
+
+        default:
+            LOG_DEBUG(bad_packet6_logger, DBG_DHCP6_BASIC,
+                      DHCP6_UNKNOWN_MSG_RECEIVED)
+                .arg(static_cast<int>(pkt->getType()))
+                .arg(pkt->getIface());
+        }
+
+    } catch (const RFCViolation& e) {
+        LOG_DEBUG(bad_packet6_logger, DBG_DHCP6_BASIC, DHCP6_REQUIRED_OPTIONS_CHECK_FAIL)
+            .arg(pkt->getName())
+            .arg(pkt->getRemoteAddr().toText())
+            .arg(e.what());
 
-    default:
-        return (false);
     }
+
+    // Increase the statistic of dropped packets.
+    StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
+    return (false);
 }
 
 void
index 8f04aaf503c818f1e928ae53de52d6e48b3eeb23..68bdfadae2f7021e8d810f2023aac77f0c9be89c 100644 (file)
@@ -193,14 +193,14 @@ protected:
     /// not allowed according to RFC3315, section 15; true otherwise.
     bool testUnicast(const Pkt6Ptr& pkt) const;
 
-    /// @brief verifies if specified packet meets RFC requirements
+    /// @brief Verifies if specified packet meets RFC requirements
     ///
     /// Checks if mandatory option is really there, that forbidden option
     /// is not there, and that client-id or server-id appears only once.
     ///
     /// @param pkt packet to be checked
-    /// @return false if the message type is not recognized, true otherwise.
-    /// @throw RFCViolation if any issues are detected
+    /// @return false if the message should be dropped as a result of the
+    /// sanity check.
     bool sanityCheck(const Pkt6Ptr& pkt);
 
     /// @brief verifies if specified packet meets RFC requirements
index 2d3c4160e60653dd4f91cdbd2e1bc754e5283679..da7bf230631c768d386b18471203e1f37d3d7824 100644 (file)
@@ -96,16 +96,16 @@ TEST_F(ConfirmTest, sanityCheck) {
 
     // A message with no client-id should fail
     Pkt6Ptr confirm = Pkt6Ptr(new Pkt6(DHCPV6_CONFIRM, 1234));
-    EXPECT_THROW(srv.sanityCheck(confirm), RFCViolation);
+    EXPECT_FALSE(srv.sanityCheck(confirm));
 
     // A message with a single client-id should succeed
     OptionPtr clientid = generateClientId();
     confirm->addOption(clientid);
-    EXPECT_NO_THROW(srv.sanityCheck(confirm));
+    EXPECT_TRUE(srv.sanityCheck(confirm));
 
     // A message with server-id present should fail
     confirm->addOption(srv.getServerID());
-    EXPECT_THROW(srv.sanityCheck(confirm), RFCViolation);
+    EXPECT_FALSE(srv.sanityCheck(confirm));
 }
 
 // Test that directly connected client's Confirm message is processed and Reply
index d86eb686250b0d1569f9416a8dde6bf106c5618b..207db8e0f1db515bb707110b846410df90e72079 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2018 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
@@ -348,16 +348,16 @@ TEST_F(RebindTest, sanityCheck) {
 
     // A message with no client-id should fail
     Pkt6Ptr rebind = Pkt6Ptr(new Pkt6(DHCPV6_REBIND, 1234));
-    EXPECT_THROW(srv.sanityCheck(rebind), RFCViolation);
+    EXPECT_FALSE(srv.sanityCheck(rebind));
 
     // A message with a single client-id should succeed
     OptionPtr clientid = generateClientId();
     rebind->addOption(clientid);
-    EXPECT_NO_THROW(srv.sanityCheck(rebind));
+    EXPECT_TRUE(srv.sanityCheck(rebind));
 
     // A message with server-id present should fail
     rebind->addOption(srv.getServerID());
-    EXPECT_THROW(srv.sanityCheck(rebind), RFCViolation);
+    EXPECT_FALSE(srv.sanityCheck(rebind));
 }
 
 // Test that directly connected client's Rebind message is processed and Reply