relay: remove SSRF protections for insecure localhost connections#1217
relay: remove SSRF protections for insecure localhost connections#1217
Conversation
| if !s.config.AllowInsecureHosts { | ||
| err = s.relay.HostChecker.CheckHost(ctx, hostURL) | ||
| } else { | ||
| hc := relay.NewHostClient("") |
There was a problem hiding this comment.
It sounds simpler to set hc.Client = http.DefaultClient in NewRelay() if config.AllowInsecureHosts is set.
There was a problem hiding this comment.
That would impact all use of the HostChecker though. The current setup only allows the bypass when doing the request using admin authentication.
I flipped the ordering and added another admin boolean check to make it clearer to read and stricter in implementation.
There was a problem hiding this comment.
According to the code intelligence, relay.HostChecker is literally only used on that line, do you know of other users?
There was a problem hiding this comment.
Oh, I see. Yeah, with the additional check on admin it makes sense 👍
There was a problem hiding this comment.
But maybe use NewHostClient(s.config.UserAgent) like in NewRelay()?
There was a problem hiding this comment.
This PR starts using some of the account checking behavior: #1215. I guess that will probably not work with localhost stuff now that I think of it.
This is honestly a big messy tangle, I kind of want to just be secure all the time and not support the insecure/localhost mode at all.
|
Oh neat! Thanks for this, took a little debugging yesterday to help the reporter figure out what was going wrong on this! |
|
The way I thought of handling this was like the safeFetch implementation in the atproto repo. |
3 participants
Closes: #1216