Tidy

pdns/ednscookies.cc

@@ -19,11 +19,11 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
-#ifdef HAVE_CONFIG_H
+
 #include "config.h"
-#endif
+
 #include "ednscookies.hh"
-#include "misc.hh"
+#include "iputils.hh"
 
 #ifdef HAVE_CRYPTO_SHORTHASH
 #include <sodium.h>
@@ -54,23 +54,27 @@ bool EDNSCookiesOpt::makeFromString(const char* option, unsigned int len)
 string EDNSCookiesOpt::makeOptString() const
 {
   string ret;
-  if (!isWellFormed())
+  if (!isWellFormed()) {
     return ret;
+  }
   ret.assign(client);
-  if (server.length() != 0)
+  if (server.length() != 0) {
     ret.append(server);
+  }
   return ret;
 }
 
 void EDNSCookiesOpt::getEDNSCookiesOptFromString(const char* option, unsigned int len)
 {
   client.clear();
   server.clear();
-  if (len < 8)
+  if (len < 8) {
     return;
-  client = string(option, 8);
+  }
+  const std::string tmp(option, len);
+  client = tmp.substr(0, 8);
   if (len > 8) {
-    server = string(option + 8, len - 8);
+    server = tmp.substr(8);
   }
 }
 
@@ -84,16 +88,16 @@ bool EDNSCookiesOpt::isValid([[maybe_unused]] const string& secret, [[maybe_unus
     // Version is not 1, can't verify
     return false;
   }
-  uint32_t ts;
-  memcpy(&ts, &server[4], sizeof(ts));
-  ts = ntohl(ts);
+  uint32_t timestamp{};
+  memcpy(&timestamp, &server[4], sizeof(timestamp));
+  timestamp = ntohl(timestamp);
   // coverity[store_truncates_time_t]
-  uint32_t now = static_cast<uint32_t>(time(nullptr));
+  auto now = static_cast<uint32_t>(time(nullptr));
   // RFC 9018 section 4.3:
   //    The DNS server
   //    SHOULD allow cookies within a 1-hour period in the past and a
   //    5-minute period into the future
-  if (rfc1982LessThan(now + 300, ts) && rfc1982LessThan(ts + 3600, now)) {
+  if (rfc1982LessThan(now + 300, timestamp) && rfc1982LessThan(timestamp + 3600, now)) {
     return false;
   }
   if (secret.length() != crypto_shorthash_KEYBYTES) {
@@ -103,11 +107,13 @@ bool EDNSCookiesOpt::isValid([[maybe_unused]] const string& secret, [[maybe_unus
   string toHash = client + server.substr(0, 8) + source.toByteString();
   string hashResult;
   hashResult.resize(8);
-  crypto_shorthash(
-    reinterpret_cast<unsigned char*>(&hashResult[0]),
-    reinterpret_cast<const unsigned char*>(&toHash[0]),
-    toHash.length(),
-    reinterpret_cast<const unsigned char*>(&secret[0]));
+
+  // NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast)
+  crypto_shorthash(reinterpret_cast<unsigned char*>(hashResult.data()),
+                   reinterpret_cast<const unsigned char*>(toHash.data()),
+                   toHash.length(),
+                   reinterpret_cast<const unsigned char*>(secret.data()));
+  // NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast)
   return constantTimeStringEquals(server.substr(8), hashResult);
 #else
   return false;
@@ -119,30 +125,30 @@ bool EDNSCookiesOpt::shouldRefresh() const
   if (server.size() < 16) {
     return true;
   }
-  uint32_t ts;
-  memcpy(&ts, &server[4], sizeof(ts));
-  ts = ntohl(ts);
+  uint32_t timestamp{};
+  memcpy(&timestamp, &server.at(4), sizeof(timestamp));
+  timestamp = ntohl(timestamp);
   // coverity[store_truncates_time_t]
-  uint32_t now = static_cast<uint32_t>(time(nullptr));
+  auto now = static_cast<uint32_t>(time(nullptr));
   // RFC 9018 section 4.3:
   //    The DNS server
   //    SHOULD allow cookies within a 1-hour period in the past and a
   //    5-minute period into the future
   // If this is not the case, we need to refresh
-  if (rfc1982LessThan(now + 300, ts) && rfc1982LessThan(ts + 3600, now)) {
+  if (rfc1982LessThan(now + 300, timestamp) && rfc1982LessThan(timestamp + 3600, now)) {
     return true;
   }
 
   // RFC 9018 section 4.3:
   //    The DNS server SHOULD generate a new Server Cookie at least if the
   //     received Server Cookie from the client is more than half an hour old
-  return rfc1982LessThan(ts + 1800, now);
+  return rfc1982LessThan(timestamp + 1800, now);
 }
 
 bool EDNSCookiesOpt::makeServerCookie([[maybe_unused]] const string& secret, [[maybe_unused]] const ComboAddress& source)
 {
 #ifdef HAVE_CRYPTO_SHORTHASH
-  static_assert(EDNSCookieSecretSize == crypto_shorthash_KEYBYTES * 2, "The EDNSCookieSecretSize is not twice crypto_shorthash_KEYBYTES");
+  static_assert(EDNSCookieSecretSize == crypto_shorthash_KEYBYTES * static_cast<size_t>(2), "The EDNSCookieSecretSize is not twice crypto_shorthash_KEYBYTES");
 
   if (isValid(secret, source) && !shouldRefresh()) {
     return true;
@@ -158,18 +164,19 @@ bool EDNSCookiesOpt::makeServerCookie([[maybe_unused]] const string& secret, [[m
   server.resize(4, '\0'); // 3 reserved bytes
   // coverity[store_truncates_time_t]
   uint32_t now = htonl(static_cast<uint32_t>(time(nullptr)));
+  // NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast)
   server += string(reinterpret_cast<const char*>(&now), sizeof(now));
   server.resize(8);
 
   string toHash = client;
   toHash += server;
   toHash += source.toByteString();
   server.resize(16);
-  crypto_shorthash(
-    reinterpret_cast<unsigned char*>(&server[8]),
-    reinterpret_cast<const unsigned char*>(&toHash[0]),
-    toHash.length(),
-    reinterpret_cast<const unsigned char*>(&secret[0]));
+  crypto_shorthash(reinterpret_cast<unsigned char*>(&server.at(8)),
+                   reinterpret_cast<const unsigned char*>(toHash.data()),
+                   toHash.length(),
+                   reinterpret_cast<const unsigned char*>(secret.data()));
+  // NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast)
   return true;
 #else
   return false;

pdns/ednscookies.hh

@@ -20,8 +20,10 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 #pragma once
-#include "namespaces.hh"
-#include "iputils.hh"
+
+#include <string>
+
+union ComboAddress;
 
 struct EDNSCookiesOpt
 {
@@ -35,38 +37,39 @@ struct EDNSCookiesOpt
   bool makeFromString(const std::string& option);
   bool makeFromString(const char* option, unsigned int len);
 
-  size_t size() const
+  [[nodiscard]] size_t size() const
   {
     return server.size() + client.size();
   }
 
-  bool isWellFormed() const
+  [[nodiscard]] bool isWellFormed() const
   {
     // RFC7873 section 5.2.2
     //    In summary, valid cookie lengths are 8 and 16 to 40 inclusive.
+    // That's the total size. We account for client and server size seperately
     return (
-      client.size() == 8 && (server.size() == 0 || (server.size() >= 8 && server.size() <= 32)));
+      client.size() == 8 && (server.empty() || (server.size() >= 8 && server.size() <= 32)));
   }
 
-  bool isValid(const string& secret, const ComboAddress& source) const;
-  bool makeServerCookie(const string& secret, const ComboAddress& source);
-  string makeOptString() const;
-  string getServer() const
+  [[nodiscard]] bool isValid(const std::string& secret, const ComboAddress& source) const;
+  bool makeServerCookie(const std::string& secret, const ComboAddress& source);
+  [[nodiscard]] std::string makeOptString() const;
+  [[nodiscard]] std::string getServer() const
   {
     return server;
   }
-  string getClient() const
+  [[nodiscard]] std::string getClient() const
   {
     return client;
   }
 
 private:
-  bool shouldRefresh() const;
+  [[nodiscard]] bool shouldRefresh() const;
 
   // the client cookie
-  string client;
+  std::string client;
   // the server cookie
-  string server;
+  std::string server;
 
   void getEDNSCookiesOptFromString(const char* option, unsigned int len);
 };

pdns/recursordist/Makefile.am

@@ -141,6 +141,7 @@ pdns_recursor_SOURCES = \
 	dnswriter.cc dnswriter.hh \
 	dolog.hh \
 	ednsextendederror.cc ednsextendederror.hh \
+	ednscookies.cc ednscookies.hh \
 	ednsoptions.cc ednsoptions.hh \
 	ednspadding.cc ednspadding.hh \
 	ednssubnet.cc ednssubnet.hh \
@@ -358,6 +359,7 @@ testrunner_SOURCES = \
 	test-dnsparser_hh.cc \
 	test-dnsrecordcontent.cc \
 	test-dnsrecords_cc.cc \
+	test-ednscookie_cc.cc \
 	test-ednsoptions_cc.cc \
 	test-filterpo_cc.cc \
 	test-histogram_hh.cc \

pdns/recursordist/meson.build

@@ -107,6 +107,7 @@ common_sources += files(
   src_dir / 'dnsrecords.cc',
   src_dir / 'dnssecinfra.cc',
   src_dir / 'dnswriter.cc',
+  src_dir / 'ednscookies.cc',
   src_dir / 'ednsextendederror.cc',
   src_dir / 'ednsoptions.cc',
   src_dir / 'ednspadding.cc',
@@ -445,7 +446,6 @@ tools = {
 
 test_sources = []
 test_sources += files(
-      src_dir / 'ednscookies.cc',
       src_dir / 'test-aggressive_nsec_cc.cc',
       src_dir / 'test-arguments_cc.cc',
       src_dir / 'test-base32_cc.cc',
@@ -457,6 +457,7 @@ test_sources += files(
       src_dir / 'test-dnsparser_hh.cc',
       src_dir / 'test-dnsrecordcontent.cc',
       src_dir / 'test-dnsrecords_cc.cc',
+      src_dir / 'test-ednscookie_cc.cc',
       src_dir / 'test-ednsoptions_cc.cc',
       src_dir / 'test-filterpo_cc.cc',
       src_dir / 'test-histogram_hh.cc',

pdns/recursordist/test-ednscookie_cc.cc

@@ -0,0 +1 @@
+../test-ednscookie_cc.cc

pdns/test-ednscookie_cc.cc

@@ -25,17 +25,18 @@
 
 #define BOOST_TEST_NO_MAIN
 
-#ifdef HAVE_CONFIG_H
 #include "config.h"
-#endif
-#include "ednscookies.hh"
 
+#include <string>
 #include <boost/test/unit_test.hpp>
 
+#include "ednscookies.hh"
+#include "iputils.hh"
+
 BOOST_AUTO_TEST_SUITE(test_ednscookie)
 BOOST_AUTO_TEST_CASE(test_getEDNSCookiesOptFromString)
 {
-  string cookie("");
+  std::string cookie;
   EDNSCookiesOpt eco(cookie);
   // Length 0
   BOOST_CHECK(!eco.isWellFormed());
@@ -73,7 +74,7 @@ BOOST_AUTO_TEST_CASE(test_getEDNSCookiesOptFromString)
 
 BOOST_AUTO_TEST_CASE(test_ctor)
 {
-  string cookie("");
+  std::string cookie;
   auto eco = EDNSCookiesOpt(cookie);
   BOOST_CHECK(!eco.isWellFormed());
 
@@ -91,7 +92,7 @@ BOOST_AUTO_TEST_CASE(test_createEDNSServerCookie)
   BOOST_CHECK(eco.isWellFormed());
 
   // wrong keysize (not 128 bits)
-  string secret = "blablablabla";
+  std::string secret = "blablablabla";
   BOOST_CHECK(!eco.makeServerCookie(secret, remote));
   BOOST_CHECK(eco.isWellFormed());
   BOOST_CHECK(!eco.isValid(secret, remote));