tracing: disable regular expression matching in log filters (#1580)

hawkw · web-flow · commit 20634d917656 · 2022-04-08T11:53:18.000-07:00
[`tracing-subscriber` v0.3.10][1] introduces a new [builder API][2] for configuring the `EnvFilter` type. One of the configurations that can now be set using the builder is whether span field value filters for `fmt::Debug` fields are interpreted as precise string matching or as regular expressions. Disabling regular expressions is strongly recommended in cases where filter configurations can come from an untrusted source, as a malicious regular expression is a potential denial-of-service vector. In the proxy, we already implement a form of access control for setting the filter --- the `/admin/log-level` endpoint denies requests that did not originate from localhost, so it's only possible to set the log level when SSHed into the pod. However, it's probably wise to disable regex filters here as well, as a form of additional defense in depth. Therefore, this branch updates the `tracing-subscriber` dependency to v0.3.10, and disables regular expression filters. [1]: https://github.com/tokio-rs/tracing/releases/tag/tracing-subscriber-0.3.10 [2]: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.Builder.html
diff --git a/linkerd/tracing/Cargo.toml b/linkerd/tracing/Cargo.toml
@@ -17,6 +17,6 @@ tracing = "0.1"
 tracing-log = "0.1"
 
 [dependencies.tracing-subscriber]
-version = "0.3"
+version = "0.3.10"
 default-features = false
 features = ["env-filter", "fmt", "smallvec", "tracing-log", "json", "parking_lot", "registry"]
diff --git a/linkerd/tracing/src/level.rs b/linkerd/tracing/src/level.rs
@@ -1,10 +1,23 @@
 use linkerd_error::Error;
 use tracing::trace;
-use tracing_subscriber::{reload, EnvFilter, Registry};
+use tracing_subscriber::{
+    filter::{self, EnvFilter, LevelFilter},
+    reload, Registry,
+};
 
 #[derive(Clone)]
 pub struct Handle(reload::Handle<EnvFilter, Registry>);
 
+/// Returns an `EnvFilter` builder with the configuration used for parsing new
+/// filter strings.
+pub(crate) fn filter_builder() -> filter::Builder {
+    EnvFilter::builder()
+        .with_default_directive(LevelFilter::WARN.into())
+        // Disable regular expression matching for `fmt::Debug` fields, use
+        // exact string matching instead.
+        .with_regex(false)
+}
+
 impl Handle {
     pub(crate) fn new(handle: reload::Handle<EnvFilter, Registry>) -> Self {
         Self(handle)
@@ -18,7 +31,7 @@ impl Handle {
 
     pub fn set_level(&self, level: impl AsRef<str>) -> Result<(), Error> {
         let level = level.as_ref();
-        let filter = level.parse::<EnvFilter>()?;
+        let filter = filter_builder().parse(level)?;
         self.0.reload(filter)?;
         tracing::info!(%level, "set new log level");
         Ok(())
diff --git a/linkerd/tracing/src/lib.rs b/linkerd/tracing/src/lib.rs
@@ -16,7 +16,7 @@ use linkerd_error::Error;
 use std::str;
 use tracing::{Dispatch, Subscriber};
 use tracing_subscriber::{
-    filter::{EnvFilter, FilterFn},
+    filter::{FilterFn, LevelFilter},
     fmt::format,
     prelude::*,
     registry::LookupSpan,
@@ -60,7 +60,6 @@ pub fn init() -> Result<Handle, Error> {
 
 #[inline]
 pub(crate) fn update_max_level() {
-    use tracing::level_filters::LevelFilter;
     use tracing_log::{log, AsLog};
     log::set_max_level(LevelFilter::current().as_log());
 }
@@ -167,7 +166,11 @@ impl Settings {
     pub fn build(self) -> (Dispatch, Handle) {
         let log_level = self.filter.as_deref().unwrap_or(DEFAULT_LOG_LEVEL);
 
-        let mut filter = EnvFilter::new(log_level);
+        let mut filter = level::filter_builder()
+            // When parsing the initial filter configuration from the
+            // environment variable, use `parse_lossy` to skip any invalid
+            // filter directives and print an error.
+            .parse_lossy(log_level);
 
         // If access logging is enabled, build the access log layer.
         let access_log = if let Some(format) = self.access_log {
