diff --git a/src/MetricLabels.h b/src/MetricLabels.h index 78dea217b..f7dd55b8c 100644 --- a/src/MetricLabels.h +++ b/src/MetricLabels.h @@ -5,11 +5,23 @@ #include #include +#include namespace visor { using LabelMap = std::map; +// OTLP, Prometheus and JSON all require label values to be UTF-8, but values +// taken from packets, such as DNS names, can hold any byte. utf8_label_value +// writes each byte that is not part of a well-formed UTF-8 sequence as \DDD, +// the decimal escape of DNS presentation format, so one such value cannot make +// a whole export or scrape fail. A backslash followed by three digits becomes +// \092, so two different values never render the same. Any other value is +// returned unchanged; label_value_needs_escape tells the two cases apart. +bool is_utf8(std::string_view value); +bool label_value_needs_escape(std::string_view value); +std::string utf8_label_value(std::string_view value); + // Process-global static labels (e.g. instance). Storage defined in Metrics.cpp. LabelMap &prometheus_static_labels_mutable(); inline const LabelMap &prometheus_static_labels() { return prometheus_static_labels_mutable(); } diff --git a/src/Metrics.cpp b/src/Metrics.cpp index 0aba2fe73..dbde83dc9 100644 --- a/src/Metrics.cpp +++ b/src/Metrics.cpp @@ -27,11 +27,7 @@ void Counter::to_opentelemetry(metrics::v1::ScopeMetrics &scope, timespec &start gauge_data_point->set_as_int(_value); gauge_data_point->set_start_time_unix_nano(timespec_to_uint64(start)); gauge_data_point->set_time_unix_nano(timespec_to_uint64(end)); - for (const auto &label: add_labels) { - auto attribute = gauge_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(gauge_data_point, add_labels); } void Rate::to_json(json &j, bool include_live) const @@ -85,11 +81,7 @@ void Cardinality::to_opentelemetry(metrics::v1::ScopeMetrics &scope, timespec &s gauge_data_point->set_as_int(lround(_set.get_estimate())); gauge_data_point->set_start_time_unix_nano(timespec_to_uint64(start)); gauge_data_point->set_time_unix_nano(timespec_to_uint64(end)); - for (const auto &label: add_labels) { - auto attribute = gauge_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(gauge_data_point, add_labels); } LabelMap &prometheus_static_labels_mutable() @@ -98,6 +90,100 @@ LabelMap &prometheus_static_labels_mutable() return labels; } +// Length of the well-formed UTF-8 sequence that starts at value[i], or 0 when +// none does. Follows Unicode Table 3-7, so overlong forms, surrogates and code +// points above U+10FFFF are rejected, as Go and protobuf decoders reject them. +static size_t utf8_sequence_length(std::string_view value, size_t i) +{ + const auto lead = static_cast(value[i]); + if (lead < 0x80) { + return 1; + } + size_t len; + unsigned char lo = 0x80, hi = 0xBF; + if (lead >= 0xC2 && lead <= 0xDF) { + len = 2; + } else if (lead >= 0xE0 && lead <= 0xEF) { + len = 3; + lo = lead == 0xE0 ? 0xA0 : 0x80; + hi = lead == 0xED ? 0x9F : 0xBF; + } else if (lead >= 0xF0 && lead <= 0xF4) { + len = 4; + lo = lead == 0xF0 ? 0x90 : 0x80; + hi = lead == 0xF4 ? 0x8F : 0xBF; + } else { + return 0; + } + if (value.size() - i < len) { + return 0; + } + const auto second = static_cast(value[i + 1]); + if (second < lo || second > hi) { + return 0; + } + for (size_t k = 2; k < len; ++k) { + const auto next = static_cast(value[i + k]); + if (next < 0x80 || next > 0xBF) { + return 0; + } + } + return len; +} + +// True when value[i] is a backslash followed by three ASCII digits, which would +// read as an escaped byte unless it is escaped itself. +static bool backslash_before_digits(std::string_view value, size_t i) +{ + auto digit = [&](size_t k) { return value[k] >= '0' && value[k] <= '9'; }; + return value[i] == '\\' && value.size() - i > 3 && digit(i + 1) && digit(i + 2) && digit(i + 3); +} + +bool is_utf8(std::string_view value) +{ + for (size_t i = 0; i < value.size();) { + const auto len = utf8_sequence_length(value, i); + if (!len) { + return false; + } + i += len; + } + return true; +} + +bool label_value_needs_escape(std::string_view value) +{ + for (size_t i = 0; i < value.size();) { + const auto len = utf8_sequence_length(value, i); + if (!len || backslash_before_digits(value, i)) { + return true; + } + i += len; + } + return false; +} + +std::string utf8_label_value(std::string_view value) +{ + if (!label_value_needs_escape(value)) { + return std::string(value); + } + std::string out; + out.reserve(value.size()); + for (size_t i = 0; i < value.size();) { + if (backslash_before_digits(value, i)) { + out.append("\\092"); + ++i; + } else if (const auto len = utf8_sequence_length(value, i)) { + out.append(value.substr(i, len)); + i += len; + } else { + fmt::format_to(std::back_inserter(out), "\\{:03d}", static_cast(value[i])); + ++i; + } + } + return out; +} + void Metric::name_json_assign(json &j, const json &val) const { json *j_part = &j; diff --git a/src/Metrics.h b/src/Metrics.h index 0067cb56e..47534f9a0 100644 --- a/src/Metrics.h +++ b/src/Metrics.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #define HIST_MIN_EXP -9 @@ -61,6 +62,16 @@ static inline uint64_t timespec_to_uint64(timespec &stamp) return stamp.tv_sec * 1000000000ULL + stamp.tv_nsec; } +template +inline void add_label_attributes(DataPoint *data_point, const LabelMap &labels) +{ + for (const auto &label : labels) { + auto attribute = data_point->add_attributes(); + attribute->set_key(label.first); + attribute->mutable_value()->set_string_value(utf8_label_value(label.second)); + } +} + // Single source of truth for the SLA quantiles pktvisor reports, in the order // get_quantiles() produces them. Each emit path (JSON, Prometheus, OpenTelemetry) // derives its label/rank from this table, so values and labels can never desync. @@ -338,11 +349,7 @@ class Histogram final : public Metric } hist_data_point->set_count(_sketch.get_n()); - for (const auto &label : add_labels) { - auto attribute = hist_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(hist_data_point, add_labels); } }; @@ -481,11 +488,7 @@ class Quantile final : public Metric quantile->set_quantile(QUANTILES[i].rank); quantile->set_value(quantiles[i]); } - for (const auto &label : add_labels) { - auto attribute = summary_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(summary_data_point, add_labels); } }; @@ -534,11 +537,7 @@ class TopN final : public Metric data_point->set_as_int(value); data_point->set_start_time_unix_nano(start); data_point->set_time_unix_nano(end); - for (const auto &label : l) { - auto attribute = data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(data_point, l); } public: @@ -668,7 +667,11 @@ class TopN final : public Metric auto threshold = _get_threshold(items); for (uint64_t i = 0; i < std::min(_top_count, items.size()); i++) { if (items[i].get_estimate() >= threshold) { - section[i]["name"] = items[i].get_item(); + if constexpr (std::is_same_v) { + section[i]["name"] = utf8_label_value(items[i].get_item()); + } else { + section[i]["name"] = items[i].get_item(); + } section[i]["estimate"] = items[i].get_estimate(); } else { break; diff --git a/src/PrometheusSerializer.h b/src/PrometheusSerializer.h index 3d53a254d..a3f2a760b 100644 --- a/src/PrometheusSerializer.h +++ b/src/PrometheusSerializer.h @@ -92,10 +92,17 @@ class PrometheusSerializer } } - // Label-value escaping: backslash, double-quote, newline. + // Label-value escaping: backslash, double-quote, newline. A value that needs it + // is first rendered by utf8_label_value, as the OTLP output does. static void append_escaped(std::string &out, const std::string &v) { - for (char c : v) { + std::string rendered; + std::string_view text = v; + if (label_value_needs_escape(v)) { + rendered = utf8_label_value(v); + text = rendered; + } + for (char c : text) { switch (c) { case '\\': out.append("\\\\"); break; case '"': out.append("\\\""); break; diff --git a/src/handlers/dns/v2/tests/test_dns_layer.cpp b/src/handlers/dns/v2/tests/test_dns_layer.cpp index d6db6198f..aa23e9059 100644 --- a/src/handlers/dns/v2/tests/test_dns_layer.cpp +++ b/src/handlers/dns/v2/tests/test_dns_layer.cpp @@ -1059,3 +1059,53 @@ TEST_CASE("DNS v2 specialized_merge aggregates two buckets", "[dns][unit]") target->to_opentelemetry(scope_after, st, et, {}); CHECK(otel_gauge_sum(scope_after, "dns_xacts") == pre_b1 + pre_b2); } + +TEST_CASE("DNS qname that is not UTF-8 is escaped in every output", "[pcap][dns]") +{ + PcapInputStream stream{"pcap-test"}; + stream.config_set("pcap_file", "tests/fixtures/dns_udp_non_utf8_qname.pcap"); + stream.config_set("bpf", ""); + + visor::Config c; + auto stream_proxy = stream.add_event_proxy(c); + c.config_set("num_periods", 1); + DnsStreamHandler dns_handler{"dns-test", stream_proxy, &c}; + + dns_handler.start(); + stream.start(); + dns_handler.stop(); + stream.stop(); + + // The fixture's qname is www.bad<0xff>name.test; byte 0xff is written as \255. + const std::string escaped = R"(.bad\255name.test)"; + + json j; + dns_handler.metrics()->bucket(0)->to_json(j); + CHECK(j["unknown"]["top_qname2_xacts"][0]["name"] == escaped); + CHECK_NOTHROW(j.dump()); + + visor::PrometheusSerializer prom; + dns_handler.metrics()->bucket(0)->to_prometheus(prom, {}); + CHECK(prom.finalize().find(R"(qname=".bad\\255name.test")") != std::string::npos); + + opentelemetry::proto::metrics::v1::ScopeMetrics scope; + timespec start_ts{}, end_ts{}; + dns_handler.metrics()->bucket(0)->to_opentelemetry(scope, start_ts, end_ts, {}); + std::string qname; + for (const auto &m : scope.metrics()) { + if (m.name() == "dns_top_qname2_xacts") { + for (const auto &point : m.gauge().data_points()) { + for (const auto &a : point.attributes()) { + if (a.key() == "qname") { + qname = a.value().string_value(); + } + } + } + } + } + CHECK(qname == escaped); + + // A strict decoder, like the agent's, refuses the whole export when one string is not UTF-8. + opentelemetry::proto::metrics::v1::ScopeMetrics decoded; + CHECK(decoded.ParseFromString(scope.SerializeAsString())); +} diff --git a/src/tests/fixtures/dns_udp_non_utf8_qname.pcap b/src/tests/fixtures/dns_udp_non_utf8_qname.pcap new file mode 100644 index 000000000..47508514e Binary files /dev/null and b/src/tests/fixtures/dns_udp_non_utf8_qname.pcap differ diff --git a/src/tests/test_metrics.cpp b/src/tests/test_metrics.cpp index e0f9ec0d6..3249c8126 100644 --- a/src/tests/test_metrics.cpp +++ b/src/tests/test_metrics.cpp @@ -601,3 +601,92 @@ TEST_CASE("Rate metrics", "[metrics][rate]") r.to_opentelemetry(scope, stamp, stamp, {{"policy", "default"}}); } } + +static std::string otel_attribute(const metrics::v1::NumberDataPoint &point, const std::string &key) +{ + for (const auto &attribute : point.attributes()) { + if (attribute.key() == key) { + return attribute.value().string_value(); + } + } + return {}; +} + +TEST_CASE("Label values that are not UTF-8", "[metrics][utf8]") +{ + SECTION("valid UTF-8 is unchanged") + { + for (const auto &value : std::vector{"", "dns.example", "caf\xc3\xa9", "\xe0\xa0\x80", "\xed\x9f\xbf", "\xee\x80\x80", "\xf0\x90\x80\x80", "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", std::string("a\0b", 3)}) { + CHECK(is_utf8(value)); + CHECK_FALSE(label_value_needs_escape(value)); + CHECK(utf8_label_value(value) == value); + } + } + + SECTION("a backslash is escaped only before three digits") + { + for (const auto &value : std::vector{R"(\Device\NPF_{0})", R"(a\12)", R"(a\12b)", "\\"}) { + CHECK_FALSE(label_value_needs_escape(value)); + CHECK(utf8_label_value(value) == value); + } + CHECK(utf8_label_value(R"(a\255b)") == R"(a\092255b)"); + CHECK(utf8_label_value("\\\xff") == R"(\\255)"); + // The literal text \255 and the byte 0xff must not render the same. + CHECK(utf8_label_value(R"(\255)") != utf8_label_value("\xff")); + } + + SECTION("each byte outside a UTF-8 sequence gets a decimal escape") + { + const std::vector> cases{ + {"\xff", R"(\255)"}, + {"a\x80", R"(a\128)"}, // lone continuation byte + {"\xc3(", R"(\195()"}, // truncated sequence, next byte kept + {"\xe2\x82", R"(\226\130)"}, // truncated at the end of the value + {"\xf0\x9f\x98", R"(\240\159\152)"}, // truncated at the end of the value + {"\xc0\xaf", R"(\192\175)"}, // overlong form + {"\xc1\x81", R"(\193\129)"}, // overlong form + {"\xe0\x80\xaf", R"(\224\128\175)"}, // overlong form + {"\xf0\x8f\xbf\xbf", R"(\240\143\191\191)"}, // overlong form + {"\xed\xa0\x80", R"(\237\160\128)"}, // surrogate + {"\xf4\x90\x80\x80", R"(\244\144\128\128)"}, // above U+10FFFF + {"\xf5\x80\x80\x80", R"(\245\128\128\128)"}, + {"caf\xc3\xa9\xff", "caf\xc3\xa9\\255"}, + }; + for (const auto &[value, escaped] : cases) { + CHECK_FALSE(is_utf8(value)); + CHECK(label_value_needs_escape(value)); + CHECK(utf8_label_value(value) == escaped); + CHECK(is_utf8(utf8_label_value(value))); + } + } + + SECTION("every output escapes them") + { + Metric::reset_static_labels(); + const std::string raw = "bad\xff" + "name"; + const std::string escaped = R"(bad\255name)"; + TopN top("root", "string", {"test", "metric"}, "A topn test metric"); + top.update(raw); + + json j; + top.to_json(j); + CHECK(j["test"]["metric"][0]["name"] == escaped); + CHECK_NOTHROW(j.dump()); + + PrometheusSerializer ser; + top.to_prometheus(ser, {{"policy", "default"}}); + CHECK(ser.finalize().find(R"(string="bad\\255name")") != std::string::npos); + + metrics::v1::ScopeMetrics scope; + timespec stamp{}; + top.to_opentelemetry(scope, stamp, stamp, {{"policy", "default"}}); + Counter c("root", {"counter"}, "A counter test metric"); + c.to_opentelemetry(scope, stamp, stamp, {{"policy", raw}}); + CHECK(otel_attribute(scope.metrics(0).gauge().data_points(0), "string") == escaped); + CHECK(otel_attribute(scope.metrics(1).gauge().data_points(0), "policy") == escaped); + + metrics::v1::ScopeMetrics decoded; + CHECK(decoded.ParseFromString(scope.SerializeAsString())); + } +} diff --git a/src/tests/test_prometheus_serializer.cpp b/src/tests/test_prometheus_serializer.cpp index fee40f724..b5ca92d26 100644 --- a/src/tests/test_prometheus_serializer.cpp +++ b/src/tests/test_prometheus_serializer.cpp @@ -71,6 +71,19 @@ TEST_CASE("label values are escaped", "[prometheus][serializer]") "m{desc=\"a\\\"b\\\\c\\nd\"} 1\n"); } +TEST_CASE("label values that are not UTF-8 get decimal byte escapes", "[prometheus][serializer]") +{ + Metric::reset_static_labels(); + PrometheusSerializer s; + s.write("m", Type::Gauge, "d", {}, {{"desc", "a\"b\xff"}}, 1); + s.write("m", Type::Gauge, "d", {}, {{"desc", "a\\255"}}, 2); + CHECK(s.finalize() == + "# HELP m d\n" + "# TYPE m gauge\n" + "m{desc=\"a\\\"b\\\\255\"} 1\n" + "m{desc=\"a\\\\092255\"} 2\n"); +} + TEST_CASE("numeric formatting matches ostream operator<<", "[prometheus][serializer]") { Metric::reset_static_labels();