commit 7b1124674a2de5add307982f02652e752724c844
author Alexander Afanasyev <aa@cs.fiu.edu> Wed Oct 17 11:51:52 2018 -0400
committer Alexander Afanasyev <aa@cs.fiu.edu> Thu Oct 18 14:50:54 2018 -0400
tree e82bd9417fa08159fb70a0189327fecfafe3cb31
parent f714d1b52099183ea3cf70dbc310aee201aa45af

security: Throw exception when loading on orphaned ValidationPolicyConfig

ValidationPolicyConfig must be assigned to a Validator instance prior to
loading and it is an API violation to load configuration otherwise.

Change-Id: Ib372cd3b6ab7c7ce1e37039f25b97e1d2f1511f9
Refs: #4758

src/security/v2/validation-policy-config.cpp

diff --git a/src/security/v2/validation-policy-config.cpp b/src/security/v2/validation-policy-config.cpp
index ff3f7bf..763bef1 100644
--- a/src/security/v2/validation-policy-config.cpp
+++ b/src/security/v2/validation-policy-config.cpp
@@ -75,6 +75,9 @@
 void
 ValidationPolicyConfig::load(const ConfigSection& configSection, const std::string& filename)
 {
+  if (m_validator == nullptr) {
+    BOOST_THROW_EXCEPTION(Error("Validator instance not assigned on the policy"));
+  }
   if (m_isConfigured) {
     m_shouldBypass = false;
     m_dataRules.clear();

src/security/v2/validation-policy-config.hpp

diff --git a/src/security/v2/validation-policy-config.hpp b/src/security/v2/validation-policy-config.hpp
index 7690cc1..d2ed69c 100644
--- a/src/security/v2/validation-policy-config.hpp
+++ b/src/security/v2/validation-policy-config.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -44,15 +44,31 @@
 public:
   ValidationPolicyConfig();
 
+  /**
+   * @brief Load policy from file @p filename
+   * @throw Error Validator instance not assigned to the policy (m_validator == nullptr)
+   */
   void
   load(const std::string& filename);
 
+  /**
+   * @brief Load policy from direct @p input
+   * @throw Error Validator instance not assigned to the policy (m_validator == nullptr)
+   */
   void
   load(const std::string& input, const std::string& filename);
 
+  /**
+   * @brief Load policy from direct @p input
+   * @throw Error Validator instance not assigned to the policy (m_validator == nullptr)
+   */
   void
   load(std::istream& input, const std::string& filename);
 
+  /**
+   * @brief Load policy from @p configSection
+   * @throw Error Validator instance not assigned to the policy (m_validator == nullptr)
+   */
   void
   load(const ConfigSection& configSection, const std::string& filename);
 

src/security/v2/validation-policy.hpp

diff --git a/src/security/v2/validation-policy.hpp b/src/security/v2/validation-policy.hpp
index 0f86c20..6921984 100644
--- a/src/security/v2/validation-policy.hpp
+++ b/src/security/v2/validation-policy.hpp
@@ -40,11 +40,6 @@
   using ValidationContinuation = std::function<void(const shared_ptr<CertificateRequest>& certRequest,
                                                     const shared_ptr<ValidationState>& state)>;
 
-  ValidationPolicy()
-    : m_validator(nullptr)
-  {
-  }
-
   virtual
   ~ValidationPolicy() = default;
 
@@ -148,7 +143,7 @@
   }
 
 NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PROTECTED:
-  Validator* m_validator;
+  Validator* m_validator = nullptr;
   unique_ptr<ValidationPolicy> m_innerPolicy;
 };
 

tests/unit-tests/security/v2/validation-policy-config.t.cpp

diff --git a/tests/unit-tests/security/v2/validation-policy-config.t.cpp b/tests/unit-tests/security/v2/validation-policy-config.t.cpp
index 31cba27..cca3e7a 100644
--- a/tests/unit-tests/security/v2/validation-policy-config.t.cpp
+++ b/tests/unit-tests/security/v2/validation-policy-config.t.cpp
@@ -505,6 +505,29 @@
   VALIDATE_FAILURE(packet, "Should fail, as the trust anchor should no longer exist");
 }
 
+BOOST_FIXTURE_TEST_CASE(OrphanedPolicyLoad, HierarchicalValidatorFixture<ValidationPolicyConfig>) // Bug #4758
+{
+  ValidationPolicyConfig policy1;
+  BOOST_CHECK_THROW(policy1.load("trust-anchor { type any }", "test-config"), Error);
+
+  // Reloading would have triggered a segfault
+  BOOST_CHECK_THROW(policy1.load("trust-anchor { type any }", "test-config"), Error);
+
+  ValidationPolicyConfig policy2;
+
+  std::string config = R"CONF(
+      trust-anchor
+      {
+        type dir
+        dir keys
+        refresh 1h
+      }
+    )CONF";
+
+  // Inserting trust anchor would have triggered a segfault
+  BOOST_CHECK_THROW(policy2.load(config, "test-config"), Error);
+}
+
 BOOST_AUTO_TEST_SUITE_END() // TestValidationPolicyConfig
 BOOST_AUTO_TEST_SUITE_END() // V2
 BOOST_AUTO_TEST_SUITE_END() // Security