On 27/07/2023 16:27, Jonathan S. Fisher wrote:
On the topic of security, may we consider a trustedProxies setting?

Seems reasonable.

Mark

 This
would be an analog to the internalProxies setting on RemoteIpValve. It
would need to be able to function with APR/NIO listening in a Unix Domain
Socket.

I'm not sure if this is super useful, but the goal would be an added layer
of security to prevent Proxy Protocol header injection.

On Thu, Jul 27, 2023 at 3:47 AM Mark Thomas <ma...@apache.org> wrote:

On 26/07/2023 21:53, Christopher Schultz wrote:
Mark,

On 7/26/23 13:58, Mark Thomas wrote:
I'm not a huge fan of this feature in general. I prefer supporting
features backed by specifications rather than vendor specific hacks.

I think the PROXY protocol is fairly standard, even if it's not backed
by an RFC. It's published by haproxy, but supported by nginx,
(obviously) haproxy, AWS, httpd[1], and a whole bunch of others
(
https://www.haproxy.com/blog/use-the-proxy-protocol-to-preserve-a-clients-ip-address
).

ACK. That reduces my concerns somewhat.

Well, the reality is that people want to use this in the real world and
this is essentially the only way to do it, barring coming up with a
whole new protocol for the purpose (I'm looking at /you/ AJP!).

Indeed.

So why not use /the/ protocol that (a) exists and (b) is supported by
every single product that currently supports this type of thing?

My support for any patch is going to depend on the specifics of the
patch.

In addition to the comments in the BZ
- exposing the data as a request attribute is inconsistent with other
    mechanisms that solve the same problem (e.g. see RemoteIpFilter)

+1

The whole point of PROXY is to kind of mix-together the capabilities of
both the RemoteIPFilter/Valve (which uses HTTP headers for
source-information) and the top-level idea of a Connector (something
that binds to a socket and pushes bytes around).

The confusing thing here is that those two jobs are performed at
relatively different levels in Tomcat at the moment, as I understand
things.

Yes and no. RemoteIP[Filter|Valve] insert/modify the data at a higher
level because that is where they sit but the data originates from the
SocketWrapper.

If some kind of UberConnector could be built which essentially does
something like the following, it would be ideal:

public void accept(Socket s) {
    ProxyHeader proxyHeader = readProxyHeader(s);

    Connector realConnector = getRealConnector();

    realConnector.setRemoteIP(proxyHeader.getRemoteIP());
    realConnector.setRemotePort(proxyHeader.getRemotePort());

    realConnector.takeItAway(s);
}

I'm sure there are other pieces of information that would be good to
pass-through, but the identity of the remote client is the most
interesting one.

Yes, that is the general idea. Just a couple of minor tweaks to use the
SocketWrapper rather than the Connector and to do it in a slightly
different place. The Acceptor is too early as we want to do as little as
possible on the Acceptor thread.

- needs to be implemented for all Connectors

I hope not. The connectors should be able to just have a thin layer in
front of them "sipping" the header off the beginning of the connection.
I am *way* out of my depth here when it comes to Tomcat internals and so
I don't want to appear to be telling you (Mark) "how it works/should
work", but conceptually it "seems easy". That may not translate into
"easy implementation" or it may mean "tons of refactoring that we
wouldn't need if we didn't care that much."

My point was that the provided patch only implements this for NIO. It
needs to implement it for NIO2 as well. APR/Native looks to be a lot
more difficult to implement and I'd be happy not implementing it for
APR/Native.

- I'd expect it to look more like the SNI processing

SNI processing is very connector-dependent, of course, because it's
HTTPS-only. PROXY should allow HTTP, HTTPS, AJP, SFTP, JDBC, anything.
So if it can be implemented as something that can just "sit in front of"
*any* connector now or in the future of Tomcat, that would be ideal. It
could definitely be implemented as an "optional feature" on a
Connector-by-Connector basis, but my sense is that it can be done
separately and globally.

Ah. You are thinking Connector as in protocol (HTTP, AJP, etc) whereas I
am thinking in terms of implementation (NIO, NIO2, etc).

SNI is handled independently of implementation and I think PROXY should
be handled the same way. They also sit at almost the same point in the
processing (PROXY needs to be first). PROXY parsing could be implemented
within the existing handshake() method but I think it would be much
cleaner in a separate method.

Without looking at it too closely I think the implementation would look
something like:

- a new method on SocketWrapperBase that
    - checks if PROXY is enabled
    - returns immediately if PROXY is not enabled or has already
      been parsed
    - uses a new utility class (or classes) to parse the header
      (reading via the read() methods on SocketWrapperBase)
    - sets the cached values for remoteAddr, remoteHost,
      remotePort etc
- The SocketProcessor.doRun() implementations add a call to this new
    method just before the TLS handshake

If we want to support the TLS information then a little additional
refactoring will be required (probably to cache the result of
SocketWrapperBase.getSslSupport) so the new utility classes can insert a
PROXY specific SSLSupport implementation.

Again, I'm speaking from a position of profound ignorance, here. Please
don't hear me say "oh, this is easy, Mark... just go do it!" :)

:)

Actually with the patch that has already been provided and the suggested
implementation outline above I don't think there is too much work to do.

Generally, I don't think implementing this is going to be possible as
some sort of plug-in.

+1 Unless the plug-in is "a whole new set of protocol/endpoint/etc.
handlers" which is a rather serious commitment.

On reflection, with the approach above we probably could implement this
via a new plug-in framework but I am not sure it is worth the effort at
this point. Something to keep in mind if we have more things wanting to
integrate at this point in the processing chain.

Mark


-chris

[1] https://httpd.apache.org/docs/2.4/mod/mod_remoteip.html search for
"haproxy"

On 26/07/2023 17:44, Amit Pande wrote:
Missed to ask this:

Looking the patch, it involves modifying Tomcat code.
Was wondering if it would be possible to refactor this patch and/or
allow Tomcat core code to extend and plug-in the proxy protocol
support?

Thanks,
Amit

-----Original Message-----
From: Amit Pande
Sent: Wednesday, July 26, 2023 11:43 AM
To: Tomcat Users List <users@tomcat.apache.org>
Subject: RE: [External] Re: Supporting Proxy Protocol in Tomcat

Chris, Mark,

Any thoughts on this?

Mark, if we clean up the patch and re-submit, do you will have any
concerns (specially security wise)?

Thanks,
Amit

-----Original Message-----
From: Jonathan S. Fisher <exabr...@gmail.com>
Sent: Monday, July 24, 2023 12:41 PM
To: Tomcat Users List <users@tomcat.apache.org>
Subject: Re: [External] Re: Supporting Proxy Protocol in Tomcat

Just a side note, because we're also very interested in this patch!

Awhile back, I was successfully able to apply this patch and
terminate TCP/TLS using HaProxy. We then had Tomcat listen on a unix
domain socket and the Proxy protocol provided *most *of the
relevant/required information to tomcat. I believe we had to add a
Valve to tomcat to set the Remote IP however as the patch didn't
handle that case.

I can find my notes from that experiment, but I do remember getting a
significant boost in throughput and decrease in latency.

+1 for this patch and willing to help out!

On Mon, Jul 24, 2023 at 11:22 AM Amit Pande
<amit.pa...@veritas.com.invalid>
wrote:

Thank you, Chris, again for inputs.
And sorry to circle back on this, late.

One related question is - does it make sense to use the patch attached
in
https://bz.apache.org/bugzilla/show_bug.cgi?id=57830 ?
And potentially, get it integrated into Tomcat versions?

There are concerns from Mark about using the patch in its current
state, but I see last comment (#24) on the issue and looks like there
are some more points to be concluded.

Thanks,
Amit

-----Original Message-----
From: Christopher Schultz <ch...@christopherschultz.net>
Sent: Wednesday, May 10, 2023 4:21 PM
To: users@tomcat.apache.org
Subject: Re: [External] Re: Supporting Proxy Protocol in Tomcat

Amit,

On 5/10/23 12:59, Amit Pande wrote:
Yes, we intended to have Tomcat run behind a (transparent) TCP
proxy e.g.

https://www/.
envoyproxy.io
%2Fdocs%2Fenvoy%2Flatest%2Fintro%2Farch_overview%2Fother_
features%2Fip_transparency&data=05%7C01%7CAmit.Pande%40veritas.com
%7Ca
85e610757b348137b4008db8c6d8156%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0
%7C0%7C638258174209955308%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=W
NEV4UQ5q4Nl8SEFHMz7C%2Fj3Qr7pCHpfyvQLeBn56uQ%3D&reserved=0
which supports the proxy protocol.

Since there is not much action on this
https://bz.a/
pache.org
%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D57830&data=05%7C01%7CAmit.Pande%
40veritas.com%7Ca85e610757b348137b4008db8c6d8156%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C0%7C638258174209955308%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=mH7TRJny1YUOsG%2BeFXno4xdvsLAjz%2BRkQgCnLfehXvQ%3D&reserved=0,
does it imply that most of the times Tomcat is running behind HTTP proxies
and not TCP proxies?
Or does it mean that, Tomcat or applications running in Tomcat does
not
need the remote client address information?

I can't speak for anybody else, but I use Apache httpd as my
reverse-proxy and I do terminate TLS. I also use it for
load-balancing/fail-over, caching, some authorization, etc. I wouldn't
be able to use a TCP load-balancer because I hide multiple services
behind my reverse-proxy which run in different places. It's not just
s dumb pass-through.

Hope that helps,
-chris

-----Original Message-----
From: Christopher Schultz <ch...@christopherschultz.net>
Sent: Monday, May 8, 2023 3:40 PM
To: users@tomcat.apache.org
Subject: [External] Re: Supporting Proxy Protocol in Tomcat

Amit,

On 5/4/23 16:07, Amit Pande wrote:
We have a similar requirement as mentioned in the below enhancement
request.

https://bz/.
a%2F&data=05%7C01%7CAmit.Pande%40veritas.com%7C07ebe3c927ed4b787206
08
db519ccce8%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C0%7C63819350613
56
24269%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
CJ
BTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3UFyiGJ9ZgtLqUzY9
JM
CK2MfwKN3OAOKdr6JmTUGkPw%3D&reserved=0
pache.org
%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D57830&data=05%7C01%7CAmit.
P
ande%40veritas.com%7Cab789327b86845e8ad7208db50046f55%7Cfc8e13c0422
c4
c
55b3eaca318e6cac32%7C0%7C0%7C638191752206669206%7CUnknown%7CTWFpbGZ
sb
3
d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
D%
7
C3000%7C%7C%7C&sdata=6TXyKzlyjY3AIi6zQMFn2j9BhtwYo6Jkrd1V3nOl4mY%3D
&r
e
served=0

Is there any plan to add this support in Tomcat in future releases?

Nothing at the moment that I know of.

I thought that markt had looked at this a while back and said it
didn't
look too difficult. It does require Tomcat to handle the stream
directly and not just rely on Java's SSLServerSocket. I thought that
had been done at some point, but it may not have. Handling the stream
directly may have some other advantages as well, though it definitely
makes the code more complicated.

Also, since this was requested long time back and there is no
update, are there any other alternatives to pass the client
information from load balancer to Tomcat in situations where there
is no SSL termination at load balancer?
You mean like a network load balancer where the lb is just proxying
bytes and not looking at the data at all? The PROXY protocol really is
the best way to do that, honestly.

-chris

--------------------------------------------------------------------
- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


--------------------------------------------------------------------
- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org



--
Jonathan | exabr...@gmail.com
Pessimists, see a jar as half empty. Optimists, in contrast, see it
as half full.
Engineers, of course, understand the glass is twice as big as it
needs to be.

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to