[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

Lapo Luchini  changed:

   What|Removed |Added

 CC||l...@lapo.it

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #24 from Tim  ---
I just re-read the spec and I see that the Proxy Protocol header must be
required on every request.

I'm not convinced requiring the Proxy Protocol header on every request
increases security, especially not in the narrow context of HTTP and HTTPS
connections. As I understood the argument, requiring the header always be
present would somehow prevent spoofing. I don't see any way it prevents a
malicious actor from sending a request with a falsified header. I think the
correct guard against malicious actors sending falsified headers is to limit
the range of IP's that can send requests with the Proxy Protocol header.

In the context of supporting Proxy Protocol in Tomcat, I wouldn't want an
implementation unless it provided the ability to limit the allowed senders of
the Proxy Protocol header with an allow list that specified either a list of
allowed IP addresses or one or more CIDRs. It is probably better to support the
spec as written, but I would support an 'optional' argument to autodetect
presence of the Proxy Protocol header if it didn't meet fierce resistance from
the Tomcat team and if it rejected requests with a Proxy Protocol header that
did not come from an address in the allow list.

I think logs should show both the "claimed" IP as well as the actual IP a
request came from. This is the approach I've taken when processing
X-Request-For headers; log both the source of tcp connectoin and the
X-Request-For address so anybody investigating a breach can verify if the
request actually came from an authorized load balancer.

Lastly, I think the implementation should make the "claimed" IP and port from
the Proxy Protocol header available in the X-Forwarded-For and X-Forwarded-Port
headers when handing the request off to the Servlet. This would preserve the
data necessary to log both the "claimed" ip and the actual IP. In the tomcat
access log, I think the %a and %h flags should print both the actual source of
the connection (e.g. load balancer), and the "claimed" IP and port.

I'm not strongly invested in any of my positions I layed out here. I'm just
trying to get a better handle on what sort of patch would be acceptable.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #23 from Mark Thomas  ---
(In reply to Tim from comment #22)
> Mark Thomas: Why do you object to the optional config?

Because it is insecure. It is for this reason that the PROXY spec explicitly
states that "The receiver ... MUST not try to guess whether the protocol header
is present or not."

Looking at a diff against the 8.5.23, I'm also wondering if the implementation
has been made at the right point. That the broadly the same implementation
would need to be repeated across multiple connectors suggests it should be
better implemented further up the stack.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #22 from Tim  ---
Mark Thomas: Why do you object to the optional config?

I'm asking because we have our servers behind a load balancer but we often send
test requests to individual servers. It would make sense to me to accept proxy
protocol requests from the load balancer and also accept requests without the
proxy protocol. This would seem to be the case that the optional config
handles.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #21 from Mark Thomas  ---
In its current form? Unlikely. A quick look identifies several issues:

- The patch no longer applies cleanly. It needs to be updated to work with the
latest 9.0.x code.
- The optional configuration is inherently insecure. I'd consider vetoing a
commit that implemented this.
- The IP address parsing is not robust. Tomcat provides robust parsers in
org.apache.tomcat.util.http.parser
- This looks to be significantly more complex to implement for APR/Native. That
may not be an issue on the basis APR/Native may be dropped in Tomcat 10
onwards.
- No documentation
- I haven't looked at the overall design because the patch didn't apply. There
may be additional concerns.

Generally, when there appears to be a low level of interest in an enhancement
the closer a patch is to being able to be simply reviewed and committed without
additional changes the more likely it is to be applied.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #20 from Emanuel  ---
hi, are there any plans to commit these changes into new tomcat releases?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

Emanuel  changed:

   What|Removed |Added

 CC||emanuel.ga...@hws-gruppe.de

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

Matafagafo  changed:

   What|Removed |Added

 CC||matafag...@yahoo.com

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

asanc...@mga.es changed:

   What|Removed |Added

 CC|asanc...@mga.es |

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

asanc...@mga.es changed:

   What|Removed |Added

 CC||asanc...@mga.es

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #19 from kycro...@gmail.com ---
Created attachment 35535
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35535=edit
Source and JAR for proxy protocol support for 8.5.23

I've attached a new tomcat-coyote jar, with source code, that supports proxy
protocol in tomcat 8.5.23.  Test/use/ignore as you like.

(And yes, this work was done for support with AWS ELB.)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #18 from Christopher Schultz  ---
The PROXY protocol should be "easy" to roll into an existing class: just have a
look at the first few bytes of a request to see if it's got a valid "PROXY"
command in there. If so, validate the source IP (the proxy), set all of the
source IP information (the client's actual IP, port, etc.), and consume the
"PROXY" command so that the following code just sees the raw request.

I'm not familiar enough with the channel-unwrapping internals of Tomcat to know
how best to implement it in the current pipeline. The best I would be able to
do is validate that the patch actually works. Which I have not yet done :(

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #17 from Rob  ---
Would it make sense to create a new protocol object either containing or
derived from Http11NioProtocol (e.g. ProxyHttp11NioProtocol)?  That would avoid
put the proxy protocol code in a class by itself.  I have no idea if this
would, work for all configurations.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #16 from Axel U  ---
Is there an update on adding PROXY protocol support?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #15 from Christopher Schultz  ---
My work on this stalled while I worked with Dan @ httpd on his patches.
Ironically, as he was trying to merge his patch set, he discovered that someone
else had built the same capabilities and committed them a few hours earlier :(

So I'm going to get back involved, here, first working with httpd and then it
should be easy to validate that the Tomcat patch is working as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #14 from Igor Cicimov  ---
Hi,

Just wonder about the status on this?

Thanks,
Igor

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #13 from Christopher Schultz  ---
Since I'm working with Daniel Ruggeri @ httpd, I've got an AWS lb set up for
testing. I should be able to apply your patch and test it... soon. Sorry for
the delay on this. Lots to do at $work.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

bon...@gmail.com changed:

   What|Removed |Added

 CC||bon...@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

SATOH Fumiyasu  changed:

   What|Removed |Added

 CC||fumiyas-u-apa...@sfo.jp

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #12 from Christopher Schultz  ---
Awesome. I should be able to do some testing using Amazon ELB, which I expect
was the impetus for this work.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #11 from kycro...@gmail.com ---
Created attachment 33738
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33738=edit
SVN patch, source code, and jar file

+1

Also

I've attached a zip file containing:
tomcat-coyote-pp-src.zip (a maven project extending tomcat-coyote.jar)
tomcat-coyote-pp.patch   (an SVN patch for tomcat-coyote's source)
tomcat-coyote-pp.jar (the jar built from tomcat-coyote-pp-src)

This extension is based on tomcat-coyote.jar version 8.0.32, but applying the
changes to 8.0.33 works as well.  It enables support for human readable Proxy
Protocol (according to the spec at
http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt) in the
Http11NioProtocol connector.

Simply add 'proxyProtocol="off|on|optional"' to server.xml, eg:


This will cause the connector to add two attributes to an incoming request:
"ProxyProtocolInfo" -> org.apache.tomcat.util.net.ProxyProtocolInfo   a class
that contains the source and destination addresses and ports extracted from the
proxy protocol header
"ProxyProtocolSourceAddress" -> String containing the source IP address



I've tested this using our own deployment+webapps+integration tests, and it
appears to work.  I'd love to see this (or any other support) added to tomcat. 
If anyone on the apache team wants to look at this, I'm receptive to feedback,
criticism, further instructions, and hate mail.

This is an extension to tomcat code, so it is licensed under the Apache License
v2.0.  If you feel like using this jar provided by "some guy on the internet",
feel free :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #10 from Christopher Schultz  ---
Whoops, sorry. I confused this with a similar httpd enhancement request. Please
ignore comment #9 (and this one, too).

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #9 from Christopher Schultz  ---
I think Daniel Ruggeri did some work on this. He's been waiting for some
feedback from me. Maybe I should get on that!

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #8 from Axel Fontaine  ---
+1

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #7 from Bill Barker billbar...@apache.org ---
(In reply to Christopher Schultz from comment #6)
 (In reply to Bill Barker from comment #5)
  Ok, so I miss read the spec.  After reading the spec again, I have lost all
  interest in this issue.
 
 I'm curious: does this simply not interest you, or do you actively think
 this is a bad idea, in general, or a bad idea to implement in Tomcat?

It just doesn't interest me to support such low-grade proxy servers. Other then
that they are popular on a certain well-known hosting site, it doesn't seem
worth my effort. 

If I thought it was a bad idea then I would have expressed a vote. But if this
is an itch that somebody wants to scratch, by all means scratch away :).

I haven't been able to see how to theoretically break this if it is implemented
properly according to the spec, as long as you also have a competent network
admin (which presumably applies to the above-mentioned well-known hosting
site). If I can sit as man-in-the-middle before the proxy and inject my own
PROXY ... line, then Tomcat gets two such lines (assuming the proxy is
working properly) and mine is rejected just like Tomcat does now.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #6 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to Bill Barker from comment #5)
 Ok, so I miss read the spec.  After reading the spec again, I have lost all
 interest in this issue.

I'm curious: does this simply not interest you, or do you actively think this
is a bad idea, in general, or a bad idea to implement in Tomcat?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #5 from Bill Barker billbar...@apache.org ---
Ok, so I miss read the spec.  After reading the spec again, I have lost all
interest in this issue.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #4 from Christopher Schultz ch...@christopherschultz.net ---
(In reply to Bill Barker from comment #3)
 That means that the PROXY ... line is encrypted
 over SSL/TSL just like everything else in the payload.  This in turn means
 that all of the code that is required to support this comes at the point of
 reading the initial request line and no special handling is required for
 SSL/TSL.

That's not my reading of this spec. The proxy prepends the header information
to the front of whatever the client is sending. The fact that the original
connection is encrypted is irrelevant. The PROXY protocol allows anything to be
tunneled, including non-encrypted configurations.

 It would be a huge security hole if the proxy was allowed to inject a plain
 text (for version 1) payload in front of the encrypted payload.

Why?

 Both of them would have to be encrypted if you are using an encrypted
 connection between the proxy and the receiver.

There is no requirement to encrypt the traffic between the proxy and the
receiver.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

Mark Thomas ma...@apache.org changed:

   What|Removed |Added

   Severity|normal  |enhancement

--- Comment #1 from Mark Thomas ma...@apache.org ---
Moving this to an enhancement request.

I can see the benefit of this but is would be non-trivial to implement -
particularly for HTTPS.

For NIO and NIO2 (and much like SNI support) it would require the server to
process some data before passing it to the SSL engine. Unlike SNI, the data
would not be left in the bufffer for the SSL engine to process.

APR/native needs further investigation to determine what changes would be
required to implement this.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #2 from Christopher Schultz ch...@christopherschultz.net ---
+1 to adding this enhancement.

Without this feature, AWS ELB won't send anything about an ELB-terminated TLS
connection through to the server except for the protocol (e.g. TLSv1) and the
remote client's IP address. That means you can't do client certificates,
further-validate cipher strength, etc.

As for the difference between the recently-added SNI code, is it not possible
to read a block of data and then discard some of it (after taking-out the
relevant data, of course) before passing it down the chain to the SSL handshake
processor?

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

[Bug 57830] Add support for ProxyProtocol

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=57830

--- Comment #3 from Bill Barker billbar...@apache.org ---
(In reply to Mark Thomas from comment #1)
 Moving this to an enhancement request.
 
 I can see the benefit of this but is would be non-trivial to implement -
 particularly for HTTPS.
 
 For NIO and NIO2 (and much like SNI support) it would require the server to
 process some data before passing it to the SSL engine. Unlike SNI, the data
 would not be left in the bufffer for the SSL engine to process.
 
 APR/native needs further investigation to determine what changes would be
 required to implement this.

My reading of the ProxyProtocol spec linked above states that it is agnostic to
the transport protocol between the proxy server and the receiver. Otherwise it
can't work if you are using SSH tunneling which is one of the target
configurations. That means that the PROXY ... line is encrypted over SSL/TSL
just like everything else in the payload.  This in turn means that all of the
code that is required to support this comes at the point of reading the initial
request line and no special handling is required for SSL/TSL.

It would be a huge security hole if the proxy was allowed to inject a plain
text (for version 1) payload in front of the encrypted payload. Both of them
would have to be encrypted if you are using an encrypted connection between the
proxy and the receiver.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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