Skip to content

Add more sophisticated realip middleware #708

@jawnsy

Description

@jawnsy

Thanks so much for this excellent library! I would like to propose the addition of a more sophisticated realip middleware, perhaps as a separate project, similar to httprate.

I'd be happy to open a pull request or contribute a repository, but wanted to check with you first. Thanks in advance for your consideration!

Current behavior

The included realip middleware is a very simple implementation, and serves as a great example for implementing custom middlewares. However, there are some limitations (by design):

  1. It allows spoofing if clients are able to access the chi router directly, bypassing the frontend reverse proxy

    If the client connects directly to the router and passes X-Forwarded-For: 127.0.0.1, then chi will update the RemoteAddr to 127.0.0.1

  2. It allows spoofing if the frontend reverse proxy does not appropriately sanitize (filter out) other headers

    If the client connects to a reverse proxy and passes X-Forwarded-For: 127.0.0.1 and X-Real-IP: 10.0.0.1, and the reverse proxy is configured to ignore/replace X-Forwarded-For, but not configured to filter out X-Real-IP, then the proxy will pass through: X-Forwarded-For: 100.100.100.100 (the real client address) as well as X-Real-IP: 10.0.0.1, and due to the search order, the untrusted X-Real-IP will take precedence over the correct X-Forwarded-For header.

  3. It does not indicate whether the original client request protocol was HTTP or HTTPS

    If the client connects to the reverse proxy over HTTPS and the reverse proxy terminates TLS, then it is not possible to tell that the client connected over TLS, since X-Forwarded-Proto is not considered by the middleware. This means that the server may use incorrect logic, omitting HTTPS-only headers like Strict-Transport-Security, avoiding Secure cookies, etc.

  4. It does not preserve the original RemoteAddr information for downstream clients

    If clients need to get the original RemoteAddr (the address/port of the reverse proxy), there is currently no way to get that information.

  5. It does not sanitize headers from the original request, so spoofing may be an issue if the request is proxied to a different service, such as using httputil.NewSingleHostReverseProxy()

    The original http.Request is not modified, so if users pass to a different service without modification (such as through the built-in reverse proxy handler), then the headers will be propagated to the backing service, which can lead to spoofing attacks downstream of chi.

    Moreover, if users pass multiple values for a given header (e.g. two X-Forwarded-For headers), chi will accept the first header and proxied services may have different logic (e.g. last-value-wins), again leading to a spoofing problem. So it is desirable in this case to coalesce the headers (e.g. accept only the first value, and only propagate the first value to downstream proxied connections)

Implementation options

There are two straightforward approaches that can address some of the above limitations, with different tradeoffs:

  1. Opt-in approach: Create a handler that adds updated state to the request context.

    The advantage of this is that we completely avoid the need to think about what port number to use if we want to create a RemoteAddr that "looks like" the usual 10.0.0.1:1234 value. This also ensures that downstream code "knows" how to handle things correctly, since they are effectively "opting in" to the originating client address information that the middleware is adding.

    The disadvantages of this approach are that: (1) all code needs to be modified to look in the request context for the IP address, or would have incorrect information otherwise; and (2) code has a tight coupling to the middleware, since it needs to know the context key containing the "correct" address information.

  2. Opt-out approach: Create a handler that updates the request in-place, and preserves the old state in the request context.

    The advantage of this is that most code will operate without modifications, especially if they do not attempt to parse the RemoteAddr (e.g. logging client address information as a string). As long as consumers are not parsing the RemoteAddr, then the code will behave correctly whether or not the middleware is installed/enabled. Moreover, we can modify the req.TLS to set a dummy tls.ConnectionState in order to indicate whether the originating protocol was HTTP or HTTPS.

Proposed behavior

I would propose creating a middleware that:

  1. Accepts configuration for a list of trusted headers (which may be a predefined set, such as X-Forwarded-For, X-Real-IP, Forwarded, CF-Connecting-IP, True-Client-IP, etc.)
  2. Accepts configuration for a list of trusted origin networks (specified as CIDRs corresponding to the source addresses of the reverse proxy; in case the headers are supplied from untrusted networks, the headers will be silently dropped to prevent spoofing)
  3. Performs either the opt-out or opt-in approach
  4. Provides a utility function to sanitize & coalesce headers (e.g. req.Header.Set("X-Forwarded-For", req.Header.Get("X-Forwarded-For"))), on an opt-in basis (since this is only required in situations where we're using both chi and a reverse proxy)

Open questions

  1. For implementing the trusted origin network check, we will likely need to check whether the source address is contained in a given CIDR range. Go provides the net.IPNet type for this purpose, however, Go 1.18 introduces a new type, which has a function net/netip.Prefix to do the same containment check. These types were introduced to provide better performance, so it is preferable to use these if available.

    Given that chi supports Go versions as old as 1.14, this means we either need to stick to the old API, or use the new one conditionally (providing alternate implementations with build tags). This would also constrain the way that we need to design the middleware.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions