Skip to content

Document the new xff_num_trusted_hops feature#479

Merged
htuch merged 4 commits intoenvoyproxy:masterfrom
brian-pane:xff-doc/2503
Feb 13, 2018
Merged

Document the new xff_num_trusted_hops feature#479
htuch merged 4 commits intoenvoyproxy:masterfrom
brian-pane:xff-doc/2503

Conversation

@brian-pane
Copy link
Copy Markdown
Contributor

Envoy issue #2503
Corresponding code PR #2559

Signed-off-by: Brian Pane bpane@pinterest.com

Brian Pane added 2 commits February 9, 2018 03:28
Signed-off-by: Brian Pane <bpane@pinterest.com>
Signed-off-by: Brian Pane <bpane@pinterest.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the excellent docs. I wasn't involved in the original xff_num_trusted_hops reviews, so I should be a good sounding board to make sure this makes sense to the casual reader.

It is a common case where a service wants to perform analytics based on the origin client's IP
address. Per the lengthy discussion on :ref:`XFF <config_http_conn_man_headers_x-forwarded-for>`,
this can get quite complicated, so Envoy simplifies this by setting *x-envoy-external-address*
to the *trusted client address* (as defined in that XFF discussion) if the request is from
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an anchor there and just link with :ref: rather than with description?

additional addresses from XFF:

* If *use_remote_address* is false and *xff_num_trusted_hops* is set to a value *N* that is
greater than zero, the trusted client address is the *N+1* th address from the right end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if N is larger than the length of the XFF list? Can you state what happens then?


Example 4: Envoy as internal proxy, with the edge proxy from Example 3 in front of it
Settings:
| use_remote_address = true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be false if we're an internal proxy?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's a bug on my part; I'll fix it.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great docs, some comments on top of @htuch comments.

greater than zero, the trusted client address is the *N+1* th address from the right end
of XFF.
* If *use_remote_address* is true and *xff_num_trusted_hops* is set to a value *N* that is
greater than zero, the trusted client address is the *N* th address from the right end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "Nth"

Copy link
Copy Markdown
Contributor Author

@brian-pane brian-pane Feb 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sphinx noob question: how do I do Nth in Sphinx markup? The doc generation engine didn't like it when I tried *N*th

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry no idea. There is probably some escape character, or just do "Nth". It's not a big deal either way.


Request details:
| Downstream IP address = 192.0.2.5
| XFF = "198.51.100.128, 198.51.100.10, 198.51.100.1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: I might use something other than "198" when talking about external addresses, just because it's pretty close to "192" for the quick reader. I would use something like "50" or whatever.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, especially in this context where differentiating internal and external addresses is important. I picked addresses in the 198.51.100.0/24 range reserved for documentation in RFC5737, but that RFC also allows 203.0.113.0/24 so I'll switch to that.

Result:
| Trusted client address = 192.0.2.5 (last address in XFF is trusted)
| X-Envoy-External-Address is not modified
| Request type = internal
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an internal request, but in this case x-envoy-internal will be set to false, because XFF has multiple addresses in it. I realize this is confusing, potentially wrong, etc., but it's the way it is. Not sure how you want to discuss in docs, but I think actually saying what x-envoy-internal would be in each case is probably worth it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that! I'll fix the example and add x-envoy-internal.

Signed-off-by: Brian Pane <bpane@pinterest.com>
greater than zero, the trusted client address is the Nth address from the right end
of XFF. (If the XFF contains fewer than N addresses, Envoy falls back to using the
immediate downstream connection's source address as trusted client address.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So first off, thank you for this amazing documentation, it makes it really clear what is going on over in Envoy #2559

The goal here is to preserve existing Envoy behavior while allowing configuring xff_num_trusted_hops, right? The one thing I'm still struggling with is the use of the combinations and permutations. If we were designing this from scratch, would there be a gain in having the two separate config options, or would we only want one, where we always considered the first hop to be the direct connected host?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the discussion in Envoy #2559, I created a follow-up issue 2590 to simplify the configuration.

htuch
htuch previously approved these changes Feb 13, 2018
@htuch
Copy link
Copy Markdown
Member

htuch commented Feb 13, 2018

@brian-pane LGTM, can you merge master?

Conflicts:
	docs/root/configuration/http_conn_man/headers.rst

Signed-off-by: Brian Pane <bpane@pinterest.com>
@htuch htuch merged commit d3af0e5 into envoyproxy:master Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants