On 04/09/2023 15:41, Jonathan S. Fisher wrote:
Mark thank you again for your leadership and setting expectations. I'm
going to commit to working on this with anyone else that wants to help with
the goal of a patch by year end. I want to nail the patch with minimal
rework that meets Tomcat project quality standards. To that end, I'll
attempt to summarize what you expect here and if you could comment and
correct my understanding that would be appreciated.

It sounds like you're satisfied with the ubiquity of the Proxy protocol and
that it has an RFC
We'll target just implementing the latest version of the Proxy protocol
We'll implement a "TrustedProxies" feature similar to what the Remote IP
Valve does
We'll implement a, or modify the RemoteIp, valve to be able to set the
remote IP from Proxy protocol headers
We'll follow the RFC spec and reject any request that does a proper Proxy
protocol header
I'm particularly interested in the Proxy protocol over Unix Domain Sockets,
so expect to see a lot of the work focused on this, but accepting Proxy
Protocol over TCP looks to be quite important from the comments on this
email chain

If I may ask two things:
Can you summarize your desired implementation? What point in the stack
should we target to implement this?

See my response earlier in this thread that suggested it sits alongside SNI processing. I still think that makes sense. If during implementation you reach a different conclusion then make the case for the alternative approach on list.

One thing I'm not familiar with on Tomcat is the testing expectations. If
you can point to a set of unit tests and a set of integration tests and say
"Do it like this"

Something like (only a guide)

https://github.com/apache/tomcat/blob/main/test/org/apache/tomcat/util/net/TestTLSClientHelloExtractor.java

to test the implementation directly and probably something based on SimpleHttpClient see

https://github.com/apache/tomcat/blob/main/test/org/apache/coyote/http11/TestHttp11Processor.java

for various examples. The main thing is I suspect you'll need control of the individual bytes and SimpleHttpClient provides a reasonably simple basis for that.

What we often do when we want to test things like setting remote IP addresses etc. is echo the value in the response body and then check that value in the client.

Anything else on the original patch you liked/didn't like? (
https://bz.apache.org/bugzilla/show_bug.cgi?id=57830)

It helps if you enable Checkstyle for your local build. It helps keep things in roughly the same coding style (we are slowly tightening up on that). Ideally, use the clean-up and formatting configurations we have for Eclipse in res/ide-support/eclipse .

This is sufficiently complex that I am expecting several iterations to be required. if it is simpler for you to manage with a PR then that is fine and probably easier to work with than a patch in Bugzilla.

Mark


Thank you,


On Tue, Aug 29, 2023 at 3:13 PM Mark Thomas <ma...@apache.org> wrote:

On 28/08/2023 18:44, Amit Pande wrote:
Oh, sure. So, what would be the best way to get some conclusion on this
thread?

Provide a patch for review based on the feedback provided here and in
the BZ issue.

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830 The state of the
ticket isn't updated for long. Perhaps add comments/ask the folks on user
list to vote?

That is more likely to irritate folks rather than encourage them to help
you progress your patch.

Mark



Thanks,
Amit

-----Original Message-----
From: Mark Thomas <ma...@apache.org>
Sent: Monday, August 28, 2023 11:20 AM
To: Tomcat Users List <users@tomcat.apache.org>
Subject: Re: [External] Re: Supporting Proxy Protocol in Tomcat


CAUTION: This email originated from outside the organization. Do not
click links or open attachments unless you recognize the sender and know
the content is safe. If you believe this is a phishing email, use the
Report to Cybersecurity icon in Outlook.



28 Aug 2023 17:11:20 Amit Pande <amit.pa...@veritas.com.INVALID>:

Mark,

Just checking - Did this issue get discussed in any of the core
members' meeting?

There are no such meetings. Discussion happens on the mailing lists.

Mark



Thanks,
Amit

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

Yes, understood.

Thank you for clarifying. Even I was referring to initial consensus
without any timeline or approach conclusion.

Thanks,
Amit

-----Original Message-----
From: Mark Thomas <ma...@apache.org>
Sent: Friday, July 28, 2023 2:48 PM
To: users@tomcat.apache.org
Subject: Re: [External] Re: Supporting Proxy Protocol in Tomcat

On 28/07/2023 19:21, Amit Pande wrote:
Thank you all for the valuable discussion on this topic.

Is it okay to say that we're agreeing to adding proxy protocol
support in Tomcat?

I think that is a little too strong. At this point there is a proposed
approach and no one is objecting but until there is an actual patch to
discuss...

Keep in mind that any committer can veto a change.

My sense is that it should be possible to implement this feature while
addressing any concerns that may be raised but it is not guaranteed.

Mark



Thanks,
Amit

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

All,

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

Seems reasonable.

We should probably look at what httpd did for all of this.

-chris

    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://ww/
w.haproxy.com%2Fblog%2Fuse-the-proxy-protocol-to-preserve-a-client
s
-
ip-address&data=05%7C01%7CAmit.Pande%40veritas.com%7C51dbcc5eeac14
f
a
b5aa708db8ee67aae%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C0%7C638
2
6
0892775883704%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
V
2
luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=RWHWpIL
a
0
rLRM0xPgFAeXdk0y1l2ob%2BNcQHZP55fQDg%3D&reserved=0
).

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://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2
Fhttpd.apache.org
%2Fdocs%2F2.4%2Fmod%2Fmod_remoteip.html&data=05%7C01%7CAmit.Pande%
40veritas.com%7Cf200a998e3514a7fdba008dba7e2d4a5%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C0%7C638288364940284915%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=1and8JWbfUqI%2F1vR1ZZPZEi%2FSWVNlqUpBb8bg668TcA%3D&reserved=0
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://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbz.apache.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D57830&data=05%7C01%7CAmit.Pande%40veritas.com%7Cf200a998e3514a7fdba008dba7e2d4a5%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C0%7C638288364940284915%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jW2B%2BbjpdRSh3NW7%2BhgvcekqSDcXes7asGUabXbkjvU%3D&reserved=0
?
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.
c
om
%7Ca
85e610757b348137b4008db8c6d8156%7Cfc8e13c0422c4c55b3eaca318e6c
a
c
32%7C0
%7C0%7C638258174209955308%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
L
j
AwMDAi
LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
&
s
data=W
NEV4UQ5q4Nl8SEFHMz7C%2Fj3Qr7pCHpfyvQLeBn56uQ%3D&reserved=0
which supports the proxy protocol.

Since there is not much action on this
https://nam12.safelinks.protection.outlook.com/?url=https%3A%2
F
%25
2Fbz.a%2F&data=05%7C01%7CAmit.Pande%40veritas.com%7C51dbcc5eea
c
1
4fab5aa708db8ee67aae%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C
0
%
7C638260892775883704%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
D
A
iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7
C
&
sdata=PqTzx9i99HLy8g0qX0WpmWsW3sYDqkW0i522q74RApY%3D&reserved=
0
pache.org
%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D57830&data=05%7C01%7CAmit.Pande%
40veritas.com%7Ca85e610757b348137b4008db8c6d8156%7Cfc8e13c0422c4c5
5
b
3eaca318e6cac32%7C0%7C0%7C638258174209955308%7CUnknown%7CTWFpbGZsb
3
d
8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
D
%
7C3000%7C%7C%7C&sdata=mH7TRJny1YUOsG%2BeFXno4xdvsLAjz%2BRkQgCnLfeh
X v Q%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%7C07ebe3c927ed4
b
7
87206
08
db519ccce8%7Cfc8e13c0422c4c55b3eaca318e6cac32%7C0%7C0%7C6381
9
3
50613
56
24269%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
l
u
MzIiL
CJ
BTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3UFyiGJ9Zg
t
L
qUzY9
JM
CK2MfwKN3OAOKdr6JmTUGkPw%3D&reserved=0
pache.org
%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D57830&data=05%7C01%7CAmit.
P
ande%40veritas.com%7Cab789327b86845e8ad7208db50046f55%7Cfc8e
1
3
c0422
c4
c
55b3eaca318e6cac32%7C0%7C0%7C638191752206669206%7CUnknown%7C
T
W
FpbGZ
sb
3
d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
I
6
Mn0%3
D%
7
C3000%7C%7C%7C&sdata=6TXyKzlyjY3AIi6zQMFn2j9BhtwYo6Jkrd1V3nO
l
4
mY%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


---------------------------------------------------------------------
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


---------------------------------------------------------------------
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