Skip to content

Path traversal middleware: log the execution time in ms

David Fernandez requested to merge 413766-fix-execution-time-log into master

🌳 Context

In Rack middleware for path traversal checks (!123477 - merged), we introduced a rack middleware with this idea of checking the request path, if a path traversal is detected, reject the request. See #413766+ for more details.

Since middlewares are executed for all requests, we are not starting with the "reject the request" part. Instead, we simply log the path traversal attempt and additionally we measure the execution time of that check. This is the simulation phase.

In this phase, one of the goals is to measure the impact (execution time) on requests. We certainly don't want to add 1sec of execution time on all requests 😱.

The middleware is gated behind feature flags so that we can quickly disable it.

We started playing with it on staging and while trying to get the numbers for the execution time, we got only a collection of 0.

What happens is that we compute the execution time in seconds. However, the middleware logic is quite fast. When the duration_s gets into Kibana, there has to be some rounding or truncation and our result gets rounded to 0.

This MR attempts to fix that.

🤔 What does this MR do and why?

  • Compute the path traversal middleware execution time in ms instead of s.
  • Log the value into duration_ms instead of duration_s.
  • Update the related specs.

Changelog is not needed here as the middleware is behind a feature flag .

🖼 Screenshots or screen recordings

None

How to set up and validate locally

  1. In a rails console, enable the middleware and enable the execution time logging:
Feature.enable(:check_path_traversal_middleware)
Feature.enable(:log_execution_time_path_traversal_middleware)
  1. Start the server.
  2. Access http://gdk.test:8000/foo%2F../bar
  3. Check log/application_json.log and see:
{"severity":"WARN","time":"2023-10-05T16:57:41.195Z","correlation_id":"01HC0CC327PRP91T226C3J9HMJ","fullpath":"/foo%2F../bar","message":"Potential path traversal attempt detected","duration_ms":0.713,"class_name":"Gitlab::Middleware::PathTraversalCheck"}

Notice the duration_ms field that will report the execution time in ms.

🐎 MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by David Fernandez

Merge request reports