Thanks, working on it. On Thu, Aug 23, 2018 at 6:19 PM, Bryan Call <bc...@apache.org> wrote:
> 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 > > -- Derek
