Inline -Bryan
> On Aug 22, 2018, at 11:37 AM, Derek Dagit <der...@oath.com.INVALID> wrote: > > Bryan, some clarification questions: > > >> 2. 403 > > What should we do on the N+1st redirect? Do we try to resolve the target > host and 403 that too, or just return it to the client regardless of how it > resolves? > > This is why I went with "Return" by default rather than "Reject". > Sounds good, that is what we are doing now with n+1. > > >> 3. I like Lief’s proposal. > > OK I can renumber the config values if necessary. > > > >> You might want to add another option to add non-routable IPs to the list > too. > > We do prevent connections to the ANY address 0.0.0.0 globally in ATS > already (redirect or not). I can look at adding additional checks for > non-routable addresses. > > >> I would create a configuration option called >> proxy.config.http.redirect_enabled > and start with 0 as off and set proxy.config.http.number_of_redirections to > 1 as the default. > > We have recently removed this config, seeing as it is redundant with > number_of_redirections. Are you proposing we bring this config back with a > new meaning and use it to configure the Follow/Reject/Return behavior? Yes, if it is being enabled for different modes I would think it would be best to have it under and enabled option, This is how some other configuration options are done (e.g. proxy.config.log.logging_enabled). > > Setting it to 0 according to Leif's proposal would default to the current > behavior, which we are saying is not recommended. Is this what you suggest > or were you thinking Reject or Return should be the default? > > >> When checking for localhost you should resolve the ip and check to see if > it matches any of the IPs on the server. > > OK will look at this too. I am not sure there is a good solution to handle > other servers or VIPs where the VIP is not mapped to a loop-back interface. > (If the VIP is mapped to the loopback, then this is covered by checks for > loopback. If it is not mapped to loopback, then I am not sure how we can > know to treat it specially aside from an additional config—and that starts > to sound like maybe a plugin should be handling such things.) > > > > Thanks for the input. I'm fine with changing it. > > The current Pull Request is up, but keeping the discussion here until we > settle on what we want to do. > https://github.com/apache/trafficserver/pull/4145 > > > > On Wed, Aug 22, 2018 at 1:04 PM, Bryan Call <bc...@apache.org> wrote: > >> 1. Yes >> 2. 403 >> 3. I like Lief’s proposal. You might want to add another option to add >> non-routable IPs to the list too. I would create a configuration option >> called proxy.config.http.redirect_enabled and start with 0 as off and set >> proxy.config.http.number_of_redirections to 1 as the default. >> >> When checking for localhost you should resolve the ip and check to see if >> it matches any of the IPs on the server. That could be a long list if >> there servers has many IP aliases. This still doesn’t block IPs used by >> other servers in the same network. >> >> -Bryan >> >> >> >>> On Aug 6, 2018, at 5:25 PM, Derek Dagit <der...@oath.com.INVALID> wrote: >>> >>> 1. Yep >>> >>> 2. I see multiple possible answers as well... >>> >>> 3. ... and the 0,1,2-style config having either default sounds OK. >>> >>> Regarding 1)2), Could we resolve and then check if the address is a >>> loopback address? >>> 3) Hmm, not sure about the use of *cast addresses either. >>> 4) The VIP deployment I'm used to will actually set up the host such >>> that the VIP name resolves to the loopback address, so the above works. >>> That might not be good enough for all deployments though. >>> >>> On Mon, Aug 6, 2018 at 6:16 PM, Leif Hedstrom <zw...@apache.org> wrote: >>> >>>> >>>> >>>>> On Aug 6, 2018, at 4:50 PM, Alan Carroll <solidwallofc...@oath.com. >> INVALID> >>>> wrote: >>>>> >>>>> 1. Yes. >>>> >>>> Agreed. >>>> >>>>> >>>>> 2. I think a 403 >>>> >>>> I can go either way. The HTTP way would be to just return the Location >> as >>>> is (i.e. retain the redirect), following redirects is a little >> unorthodox. >>>> Maybe since we are adding new configuration(s), maybe make it such that >> the >>>> behavior can be configurable either way? >>>> >>>> Depending on if we add another configuration, or add on to the existing >>>> one, I’m thinking something in the line of >>>> >>>> 0 - Always allow follow redirect >>>> 1 - Allow follow redirect, but return the normal redirect if it’s >>>> for localhost >>>> 2 - Allow follow redirect, but give a 403 if it's to localhost >>>> >>>> >>>> My vote would be for 1) to be the default, but I can live with 2) as >> well. >>>> >>>> Question: The notion of “localhost” is a little vague here… I think >>>> there’s a number of other ways to reach “localhost”, rather than just >>>> localhost/127.0.0.1: >>>> >>>> 1) FQDNs >>>> 2) One of the possibly many IPs that are local to the box, >>>> including IPv6 link-locals >>>> 3) Possibly some broadcast or multicast addresses?? >>>> 4) The upstream VIP IP that might point back to the box(es) >>>> >>>> I think there are many ways that someone could make a follow redirect >> loop >>>> back to itself, or a peering proxy. >>>> >>>> Cheers, >>>> >>>> — Leif >>>> >>>> >>> >>> >>> -- >>> Derek >> >> > > > -- > Derek
