Add option to disable following redirects#237
Conversation
|
Fixed. |
blakestoddard
force-pushed
the
follow-redirects
branch
from
c38ffa8 to
aac14df
Compare
Codecov Report
@@ Coverage Diff @@
## main #237 +/- ##
==========================================
- Coverage 89.28% 87.89% -1.39%
==========================================
Files 6 6
Lines 681 694 +13
==========================================
+ Hits 608 610 +2
- Misses 50 59 +9
- Partials 23 25 +2
Continue to review full report at Codecov.
|
|
Can you say a little more about the use case here? If you trust the source host enough to put them in the allowHosts list, why are you concerned about them redirecting to a disallowed host? If not all URLs on the source host are trusted, should you instead be using signatures? |
|
Sure! We use imageproxy to proxy images in customer emails for HEY. We don’t have an allow list since we don’t want to purposefully block any customer content, but we do have a deny list with all of the IETF reserved ranges just for kicks (even if we run imageproxy in a secure environment). But even with a deny list, you can get around it by telling imageproxy to proxy a URL that 301 redirects back to one of the blocked hosts — the host in the redirect isn’t rechecked through the allow/deny lists, but imageproxy will proxy that content anyway (even if it wouldn’t of been allowed because of the initial lists). (We do use signatures.) |
blakestoddard
force-pushed
the
follow-redirects
branch
from
591733c to
47c5609
Compare
willnorris
left a comment
willnorris
left a comment
There was a problem hiding this comment.
Oh, that's really cool! Happy to hear that imageproxy is useful for HEY, and congrats on working out the App Store approval stuff :)
|
|
||
| msgNotAllowed = "requested URL is not allowed" | ||
| msgNotAllowed = "requested URL is not allowed" | ||
| msgNotAllowedInRedirect = "requested URL in redirect is not allowed" |
There was a problem hiding this comment.
I'm kind of meh on the wording here, happy to change it to something else.
willnorris
changed the title
When following redirects, ensure that the final URL is not in the configured DenyHosts list, but do not further enforce presence in the AllowHosts list. This was initially added in #237, and the original use case was about protecting against redirects being used to bypass denied hosts. They were using URL signatures and deny lists (for localhost, etc), but not allow lists. So really, checking against the deny list is all that was needed in that case. This came up recently for me as I was trying to proxy images on a remote host that redirects to Amazon S3. Even though the original URL was signed, the redirect was being denied because s3-us-west-2.amazonaws.com isn't on of my allowed host. But I don't want to allow all of S3, just the signed URLs.
When redirects are followed, ensure that they are still allowed per AllowHosts/DenyHosts
When following redirects, ensure that the final URL is not in the configured DenyHosts list, but do not further enforce presence in the AllowHosts list. This was initially added in willnorris#237, and the original use case was about protecting against redirects being used to bypass denied hosts. They were using URL signatures and deny lists (for localhost, etc), but not allow lists. So really, checking against the deny list is all that was needed in that case. This came up recently for me as I was trying to proxy images on a remote host that redirects to Amazon S3. Even though the original URL was signed, the redirect was being denied because s3-us-west-2.amazonaws.com isn't on of my allowed host. But I don't want to allow all of S3, just the signed URLs.
2 participants
Adds a new
FollowRedirectsflag that defaults totrue. If the user sets it tofalse, imageproxy will not follow redirects and instead will use the content of the redirect (likelytext/htmlwhich will result in a 403 due to the default allowedcontent-types list).If you are electing to follow redirects, ensure that the host being redirected to is allowed per
AllowHostsandDenyHosts. Without this, you can use a redirect to get around theallowed()logic.