From: Remi Gacogne Date: Thu, 20 Feb 2020 11:11:34 +0000 (+0100) Subject: Add proxy protocol unit tests, fix some parsing issues X-Git-Tag: dnsdist-1.5.0-alpha1~12^2~25 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9a6a9e15f81c07c3f43b5e230f3e3d0e3178d14d;p=thirdparty%2Fpdns.git Add proxy protocol unit tests, fix some parsing issues --- diff --git a/pdns/proxy-protocol.cc b/pdns/proxy-protocol.cc index 2f7ea79f49..d99ffa84cd 100644 --- 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(header.at(14)) << 8) + static_cast(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(header.at(pos)) << 8) + static_cast(header.at(pos+1))); pos = pos + sizeof(uint16_t); - destination.setPort((header.at(pos) << 8) + header.at(pos+1)); + destination.setPort((static_cast(header.at(pos)) << 8) + static_cast(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(header.at(pos)); pos += sizeof(uint8_t); - uint16_t len = (header.at(pos) << 8) + header.at(pos + 1); + uint16_t len = (static_cast(header.at(pos)) << 8) + static_cast(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 b8497f1ebf..e61f978761 100644 --- 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 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 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 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 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 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 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 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()