RouterInfo: refactor signature creation and verification

src/core/router/info.cc

@@ -164,15 +164,7 @@ void RouterInfo::ReadFromBuffer(bool verify_signature)
       // Verify signature
       if (verify_signature)
         {
-          // Note: signature length is guaranteed to be no less than buffer length
-          std::uint16_t const len =
-              m_Buffer.size() - m_RouterIdentity.GetSignatureLen();
-          if (!m_RouterIdentity.Verify(
-                  m_Buffer.data(), len, m_Buffer.data() + len))
-            {
-              LOG(error) << "RouterInfo: signature verification failed";
-              m_IsUnreachable = true;
-            }
+          Verify();
           m_RouterIdentity.DropVerifier();
         }
     }
@@ -700,14 +692,29 @@ void RouterInfo::CreateBuffer(const PrivateKeys& private_keys)
           reinterpret_cast<const std::uint8_t*>(router_info.Str().c_str()),
           router_info.Str().size());
 
-      // Signature
-      // TODO(anonimal): signing should be done when creating RI, not after. Requires other refactoring.
-      private_keys.Sign(
-          m_Buffer.data(),
-          m_Buffer.size(),
-          m_Buffer.data() + m_Buffer.size());
+      // Verify signature
+      Verify();
+    }
+  catch (...)
+    {
+      m_Exception.Dispatch(__func__);
+      throw;
+    }
+}
 
-      m_Buffer(m_Buffer.size() + private_keys.GetPublic().GetSignatureLen());
+void RouterInfo::Verify()
+{
+  try
+    {
+      // Get RI length without signature
+      std::size_t const len =
+          m_Buffer.size() - m_RouterIdentity.GetSignatureLen();
+
+      // Confirm if valid and usable
+      const std::uint8_t* buf = m_Buffer.data();
+      if (len < Size::MinUnsignedBuffer
+          || !m_RouterIdentity.Verify(buf, len, &buf[len]))
+        m_IsUnreachable = true;
     }
   catch (...)
     {
@@ -879,7 +886,17 @@ void RouterInfo::CreateRouterInfo(
   // Write remaining options to RI
   router_info.Write(options.Str().c_str(), options.Str().size());
 
-  // TODO(anonimal): we should implement RI signing *here*
+  // Ensure signature has proper capacity
+  std::vector<std::uint8_t> sig_buf(private_keys.GetPublic().GetSignatureLen());
+
+  // Sign RI
+  private_keys.Sign(
+      reinterpret_cast<const std::uint8_t*>(router_info.Str().c_str()),
+      router_info.Str().size(),
+      sig_buf.data());
+
+  // Write signature to RI
+  router_info.Write(sig_buf.data(), sig_buf.size());
 
   LOG(debug) << "RouterInfo: " << __func__
              << " total RI size: " << router_info.Str().size();

src/core/router/info.h

@@ -69,6 +69,7 @@ struct RouterInfoTraits
   {
     MinBuffer = core::DSA_SIGNATURE_LENGTH,  // TODO(unassigned): see #498
     MaxBuffer = 2048,  // TODO(anonimal): review if arbitrary
+    MinUnsignedBuffer = 399,  // Minimum RouterInfo length w/o signature, see spec
     // TODO(unassigned): algorithm to dynamically determine cost
     NTCPCost = 10,  // NTCP *should* have priority over SSU
     SSUCost = 5,
@@ -523,6 +524,11 @@ class RouterInfo : public RouterInfoTraits, public RoutingDestination
   ///   (and subsequently sign the RI with)
   void CreateBuffer(const PrivateKeys& private_keys);
 
+  /// @brief Verify RI signature
+  /// @throws std::length_error if unsigned buffer length is below minimum
+  /// @throws std::runtime_error if signature verification fails
+  void Verify();
+
   /// @brief Save RI to file
   /// @param path Full RI path of file to save to
   void SaveToFile(const std::string& path);

tests/unit_tests/CMakeLists.txt

@@ -19,6 +19,7 @@ add_executable(kovri-tests
   core/crypto/util/x509.cc
   core/router/identity.cc
   core/router/net_db/impl.cc
+  core/router/info.cc
   core/router/transports/ssu/packet.cc
   core/util/buffer.cc
   core/util/byte_stream.cc

tests/unit_tests/core/router/info.cc

@@ -0,0 +1,58 @@
+/**
+ * Copyright (c) 2015-2018, The Kovri I2P Router Project
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are
+ * permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this list of
+ *    conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice, this list
+ *    of conditions and the following disclaimer in the documentation and/or other
+ *    materials provided with the distribution.
+ *
+ * 3. Neither the name of the copyright holder nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software without specific
+ *    prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#define BOOST_TEST_DYN_LINK
+
+#include <boost/test/unit_test.hpp>
+
+#include "core/router/identity.h"
+
+struct RouterInfoFixture
+{
+  core::PrivateKeys keys = core::PrivateKeys::CreateRandomKeys(
+      core::SIGNING_KEY_TYPE_EDDSA_SHA512_ED25519);
+};
+
+BOOST_FIXTURE_TEST_SUITE(RouterInfoTests, RouterInfoFixture)
+
+BOOST_AUTO_TEST_CASE(ValidSignature)
+{
+  BOOST_CHECK_NO_THROW(core::RouterInfo r(keys, {{"127.0.0.1", 10701}}, {}));
+}
+
+BOOST_AUTO_TEST_CASE(InvalidSignature)
+{
+  // If RI is not built completely, insufficient data will throw
+  core::RouterInfo router;
+  BOOST_CHECK_THROW(router.Verify(), std::exception);
+  BOOST_CHECK_THROW(router.CreateBuffer(keys), std::exception);
+}
+
+BOOST_AUTO_TEST_SUITE_END()