This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add documentation for forward proxy #10443
Add documentation for forward proxy #10443
Changes from 5 commits
526eacb
71c1d1a
569ac59
c724aee
e614189
80a5c14
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this list from someplace we already have or did you look through the code?
This seems to match my memory, but curious if we should double check!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list is from there #6239
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We updated this at some point, I think
synapse/docs/sample_config.yaml
Lines 205 to 206 in 016f085
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if I understand you.
The documentation is from Dec 2020. There was a change in Jan 2021 #9084
Does the
config.sample
needs a info for do not block if a proxy is used?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point was that the comment from #6239 didn't take into account the changes made in #9084 so I think the current list is wrong. (It says that we don't use it for federation and identity servers, but we do now).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really is not trivial.
I took a look in code, now:
synapse/synapse/server.py
Line 408 in bf72d10
synapse/synapse/server.py
Line 415 in bf72d10
class PreviewUrlResource
synapse/synapse/server.py
Line 428 in bf72d10
synapse/synapse/server.py
Line 401 in bf72d10
class IdentityHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I'm being an idiot -- I was confusing the proxy with the IP blacklisting code, which of course aren't the same. 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. This may happen.
But I am confused about the blacklist.
How can Synapse blacklist when the proxies does the DNS resolution? Synapse does not know the IP.
In this PR #10129 a warning should be added. But there is a function in code like:
synapse/synapse/server.py
Lines 415 to 418 in bf72d10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't, it only applies the blacklist if a proxy isn't being used, so
get_proxied_blacklisted_http_client
does the following:Pretty much anything that uses
get_proxied_blacklisted_http_client
orget_proxied_http_client
uses the proxy.Frankly we should probably change anywhere that manually creates
SimpleHttpClient
to call one of the above (that's part of fixing "make proxies work everywhere IMO).