Add proxy protocol unit tests, fix some parsing issues

diff --git a/pdns/proxy-protocol.cc b/pdns/proxy-protocol.cc
index 2f7ea79f496e9aab03936f242fe9d7d5cee0b750..d99ffa84cd68fb529979e0ffd1ec5f6a2fcbcf68 100644 (file)
--- a/pdns/proxy-protocol.cc
+++ b/pdns/proxy-protocol.cc
@@ -181,7 +181,7 @@ ssize_t isProxyHeaderComplete(const std::string& header, bool* proxy, bool* tcp,
     return 0;
   }
 
-  uint16_t contentlen = (header.at(14) << 8) + header.at(15);
+  uint16_t contentlen = (static_cast<uint8_t>(header.at(14)) << 8) + static_cast<uint8_t>(header.at(15));
   uint16_t expectedlen = 0;
   if (command != 0x00) {
     expectedlen = (addrSize * 2) + sizeof(ComboAddress::sin4.sin_port) + sizeof(ComboAddress::sin4.sin_port);
@@ -217,9 +217,9 @@ ssize_t parseProxyHeader(const std::string& header, bool& proxy, ComboAddress& s
     pos = pos + addrSize;
     destination = makeComboAddressFromRaw(protocol, &header.at(pos), addrSize);
     pos = pos + addrSize;
-    source.setPort((header.at(pos) << 8) + header.at(pos+1));
+    source.setPort((static_cast<uint8_t>(header.at(pos)) << 8) + static_cast<uint8_t>(header.at(pos+1)));
     pos = pos + sizeof(uint16_t);
-    destination.setPort((header.at(pos) << 8) + header.at(pos+1));
+    destination.setPort((static_cast<uint8_t>(header.at(pos)) << 8) + static_cast<uint8_t>(header.at(pos+1)));
     pos = pos + sizeof(uint16_t);
   }
 
@@ -228,7 +228,7 @@ ssize_t parseProxyHeader(const std::string& header, bool& proxy, ComboAddress& s
     /* we still have TLV values to parse */
     uint8_t type = static_cast<uint8_t>(header.at(pos));
     pos += sizeof(uint8_t);
-    uint16_t len = (header.at(pos) << 8) + header.at(pos + 1);
+    uint16_t len = (static_cast<uint8_t>(header.at(pos)) << 8) + static_cast<uint8_t>(header.at(pos + 1));
     pos += sizeof(uint16_t);
 
     if (len > 0) {

diff --git a/pdns/test-proxy_protocol_cc.cc b/pdns/test-proxy_protocol_cc.cc
index b8497f1ebfa3163747e388080e8cda2359cf9679..e61f9787618e69303a520645ee135af190dfa797 100644 (file)
--- a/pdns/test-proxy_protocol_cc.cc
+++ b/pdns/test-proxy_protocol_cc.cc
@@ -46,8 +46,182 @@ BOOST_AUTO_TEST_CASE(test_roundtrip) {
 
   BOOST_CHECK_EQUAL(proxy, true);
   BOOST_CHECK_EQUAL(ptcp2, true);
-  BOOST_CHECK(src2 == ComboAddress("65.66.67.68:18762"));
-  BOOST_CHECK(dest2 == ComboAddress("69.70.71.72:19276"));
+  BOOST_CHECK(src2 == src);
+  BOOST_CHECK(dest2 == dest);
+}
+
+BOOST_AUTO_TEST_CASE(test_local_proxy_header) {
+  auto payload = makeLocalProxyHeader();
+
+  BOOST_CHECK_EQUAL(payload, BINARY(
+    PROXYMAGIC
+    "\x20"          // version | command
+    "\x00"          // protocol family and address are set to 0
+    "\x00\x00"      // no content
+    ));
+
+  bool proxy;
+  bool tcp = false;
+  ComboAddress src, dest;
+  std::vector<ProxyProtocolValue> values;
+
+  BOOST_CHECK_EQUAL(parseProxyHeader(payload, proxy, src, dest, tcp, values), 16);
+
+  BOOST_CHECK_EQUAL(proxy, false);
+  BOOST_CHECK_EQUAL(tcp, false);
+  BOOST_CHECK_EQUAL(values.size(), 0U);
+}
+
+BOOST_AUTO_TEST_CASE(test_tlv_values_content_len_signedness) {
+  std::string largeValue;
+  /* this value will make the content length parsing fail in case of signedness mistake */
+  largeValue.resize(65128, 'A');
+  const std::vector<ProxyProtocolValue> values = { { "foo", 0 }, { largeValue, 255 }};
+
+  const bool tcp = false;
+  const ComboAddress src("[2001:db8::1]:0");
+  const ComboAddress dest("[::1]:65535");
+  const auto payload = makeProxyHeader(tcp, src, dest, values);
+
+  bool proxy;
+  bool tcp2;
+  ComboAddress src2;
+  ComboAddress dest2;
+  std::vector<ProxyProtocolValue> parsedValues;
+
+  BOOST_CHECK_EQUAL(parseProxyHeader(payload, proxy, src2, dest2, tcp2, parsedValues), 16 + 36 + 6 + 65131);
+  BOOST_CHECK_EQUAL(proxy, true);
+  BOOST_CHECK_EQUAL(tcp2, tcp);
+  BOOST_CHECK(src2 == src);
+  BOOST_CHECK(dest2 == dest);
+  BOOST_REQUIRE_EQUAL(parsedValues.size(), values.size());
+  for (size_t idx = 0; idx < values.size(); idx++) {
+    BOOST_CHECK_EQUAL(parsedValues.at(idx).type, values.at(idx).type);
+    BOOST_CHECK_EQUAL(parsedValues.at(idx).content, values.at(idx).content);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_tlv_values_length_signedness) {
+  std::string largeValue;
+  /* this value will make the TLV length parsing fail in case of signedness mistake */
+  largeValue.resize(65000, 'A');
+  const std::vector<ProxyProtocolValue> values = { { "foo", 0 }, { largeValue, 255 }};
+
+  const bool tcp = false;
+  const ComboAddress src("[2001:db8::1]:0");
+  const ComboAddress dest("[::1]:65535");
+  const auto payload = makeProxyHeader(tcp, src, dest, values);
+
+  bool proxy;
+  bool tcp2;
+  ComboAddress src2;
+  ComboAddress dest2;
+  std::vector<ProxyProtocolValue> parsedValues;
+
+  BOOST_CHECK_EQUAL(parseProxyHeader(payload, proxy, src2, dest2, tcp2, parsedValues), 16 + 36 + 6 + 65003);
+  BOOST_CHECK_EQUAL(proxy, true);
+  BOOST_CHECK_EQUAL(tcp2, tcp);
+  BOOST_CHECK(src2 == src);
+  BOOST_CHECK(dest2 == dest);
+  BOOST_REQUIRE_EQUAL(parsedValues.size(), values.size());
+  for (size_t idx = 0; idx < values.size(); idx++) {
+    BOOST_CHECK_EQUAL(parsedValues.at(idx).type, values.at(idx).type);
+    BOOST_CHECK_EQUAL(parsedValues.at(idx).content, values.at(idx).content);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(test_parsing_invalid_headers) {
+  const std::vector<ProxyProtocolValue> noValues;
+
+  const bool tcp = false;
+  const ComboAddress src("[2001:db8::1]:0");
+  const ComboAddress dest("[::1]:65535");
+  const auto payload = makeProxyHeader(tcp, src, dest, noValues);
+
+  bool proxy;
+  bool tcp2;
+  ComboAddress src2;
+  ComboAddress dest2;
+  std::vector<ProxyProtocolValue> values;
+
+  {
+    /* just checking that everything works */
+    BOOST_CHECK_EQUAL(parseProxyHeader(payload, proxy, src2, dest2, tcp2, values), 52);
+    BOOST_CHECK_EQUAL(proxy, true);
+    BOOST_CHECK_EQUAL(tcp2, tcp);
+    BOOST_CHECK(src2 == src);
+    BOOST_CHECK(dest2 == dest);
+    BOOST_CHECK_EQUAL(values.size(), 0U);
+  }
+
+  {
+    /* too short (not even full header) */
+    std::string truncated = payload;
+    truncated.resize(15);
+    BOOST_CHECK_EQUAL(parseProxyHeader(truncated, proxy, src2, dest2, tcp2, values), -1);
+  }
+
+  {
+    /* too short (missing address part) */
+    std::string truncated = payload;
+    truncated.resize(/* full header */ 16 + /* two IPv6s + port */ 36 - /* truncation */ 1);
+    BOOST_CHECK_EQUAL(parseProxyHeader(truncated, proxy, src2, dest2, tcp2, values), -1);
+  }
+
+  {
+    /* too short (missing TLV) */
+    values = { { "foo", 0 }, { "bar", 255 }} ;
+    const auto payloadWithValues = makeProxyHeader(tcp, src, dest, values);
+
+    std::string truncated = payloadWithValues;
+    truncated.resize(/* full header */ 16 + /* two IPv6s + port */ 36 + /* TLV 1 */ 6 + /* TLV 2 */ 6 - /* truncation */ 2);
+    BOOST_CHECK_EQUAL(parseProxyHeader(truncated, proxy, src2, dest2, tcp2, values), -2);
+  }
+
+  {
+    /* invalid magic */
+    std::string invalid = payload;
+    invalid.at(4) = 42;
+    BOOST_CHECK_EQUAL(parseProxyHeader(invalid, proxy, src2, dest2, tcp2, values), 0);
+  }
+
+  {
+    /* invalid version */
+    std::string invalid = payload;
+    invalid.at(12) = 0x10 | 0x01;
+    BOOST_CHECK_EQUAL(parseProxyHeader(invalid, proxy, src2, dest2, tcp2, values), 0);
+  }
+
+  {
+    /* invalid command */
+    std::string invalid = payload;
+    invalid.at(12) = 0x20 | 0x02;
+    BOOST_CHECK_EQUAL(parseProxyHeader(invalid, proxy, src2, dest2, tcp2, values), 0);
+  }
+
+  {
+    /* invalid family */
+    std::string invalid = payload;
+    invalid.at(13) = (0x04 << 4) | 0x01 /* STREAM */;
+    BOOST_CHECK_EQUAL(parseProxyHeader(invalid, proxy, src2, dest2, tcp2, values), 0);
+  }
+
+  {
+    /* invalid address */
+    std::string invalid = payload;
+    invalid.at(13) = (0x02 /* AF_INET */ << 4) | 0x03;
+    BOOST_CHECK_EQUAL(parseProxyHeader(invalid, proxy, src2, dest2, tcp2, values), 0);
+  }
+
+  {
+    /* TLV advertised len gets out of bounds */
+    values = { { "foo", 0 }, { "bar", 255 }} ;
+    const auto payloadWithValues = makeProxyHeader(tcp, src, dest, values);
+    std::string invalid = payloadWithValues;
+    /* full header (16) + two IPv6s + port (36) + TLV (6) TLV 2 (6) */
+    invalid.at(59) += 1;
+    BOOST_CHECK_EQUAL(parseProxyHeader(invalid, proxy, src2, dest2, tcp2, values), 0);
+  }
 }
 
 BOOST_AUTO_TEST_SUITE_END()