From 99df0b187636176c612b6218fada1d5ca22306d8 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Tue, 10 Dec 2024 16:06:05 +0100 Subject: [PATCH] Tidy --- pdns/ednscookies.cc | 67 ++++++++++++++----------- pdns/ednscookies.hh | 29 ++++++----- pdns/recursordist/Makefile.am | 2 + pdns/recursordist/meson.build | 3 +- pdns/recursordist/test-ednscookie_cc.cc | 1 + pdns/test-ednscookie_cc.cc | 13 ++--- 6 files changed, 65 insertions(+), 50 deletions(-) create mode 120000 pdns/recursordist/test-ednscookie_cc.cc diff --git a/pdns/ednscookies.cc b/pdns/ednscookies.cc index eefa0f43f76b..593d9e728cb3 100644 --- a/pdns/ednscookies.cc +++ b/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 @@ -54,11 +54,13 @@ 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; } @@ -66,11 +68,13 @@ void EDNSCookiesOpt::getEDNSCookiesOptFromString(const char* option, unsigned in { 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(×tamp, &server[4], sizeof(timestamp)); + timestamp = ntohl(timestamp); // coverity[store_truncates_time_t] - uint32_t now = static_cast(time(nullptr)); + auto now = static_cast(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(&hashResult[0]), - reinterpret_cast(&toHash[0]), - toHash.length(), - reinterpret_cast(&secret[0])); + + // NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast) + crypto_shorthash(reinterpret_cast(hashResult.data()), + reinterpret_cast(toHash.data()), + toHash.length(), + reinterpret_cast(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(×tamp, &server.at(4), sizeof(timestamp)); + timestamp = ntohl(timestamp); // coverity[store_truncates_time_t] - uint32_t now = static_cast(time(nullptr)); + auto now = static_cast(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(2), "The EDNSCookieSecretSize is not twice crypto_shorthash_KEYBYTES"); if (isValid(secret, source) && !shouldRefresh()) { return true; @@ -158,6 +164,7 @@ 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(time(nullptr))); + // NOLINTBEGIN(cppcoreguidelines-pro-type-reinterpret-cast) server += string(reinterpret_cast(&now), sizeof(now)); server.resize(8); @@ -165,11 +172,11 @@ bool EDNSCookiesOpt::makeServerCookie([[maybe_unused]] const string& secret, [[m toHash += server; toHash += source.toByteString(); server.resize(16); - crypto_shorthash( - reinterpret_cast(&server[8]), - reinterpret_cast(&toHash[0]), - toHash.length(), - reinterpret_cast(&secret[0])); + crypto_shorthash(reinterpret_cast(&server.at(8)), + reinterpret_cast(toHash.data()), + toHash.length(), + reinterpret_cast(secret.data())); + // NOLINTEND(cppcoreguidelines-pro-type-reinterpret-cast) return true; #else return false; diff --git a/pdns/ednscookies.hh b/pdns/ednscookies.hh index 47800446f318..03c5bb2ca0c4 100644 --- a/pdns/ednscookies.hh +++ b/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 + +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); }; diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am index 92dcd7583a79..03beee782b8f 100644 --- a/pdns/recursordist/Makefile.am +++ b/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 \ diff --git a/pdns/recursordist/meson.build b/pdns/recursordist/meson.build index 81d740568fe5..c5293b9f267c 100644 --- a/pdns/recursordist/meson.build +++ b/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', diff --git a/pdns/recursordist/test-ednscookie_cc.cc b/pdns/recursordist/test-ednscookie_cc.cc new file mode 120000 index 000000000000..8bb401a22c3b --- /dev/null +++ b/pdns/recursordist/test-ednscookie_cc.cc @@ -0,0 +1 @@ +../test-ednscookie_cc.cc \ No newline at end of file diff --git a/pdns/test-ednscookie_cc.cc b/pdns/test-ednscookie_cc.cc index e6c6d5bc87eb..e397e67ef64b 100644 --- a/pdns/test-ednscookie_cc.cc +++ b/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 #include +#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));