From 5fefd5d8bd68bb87a2bad16138dfc0ee121eddcf Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 12 Aug 2016 11:39:36 +1000 Subject: [PATCH] Issue #185 Implement RFC 7239 Improved test harness Added more configurability Fixed SSL session and certificate bugs --- .../main/config/etc/jetty-http-forwarded.xml | 9 +- .../main/config/modules/http-forwarded.mod | 8 +- .../server/ForwardedRequestCustomizer.java | 94 +++++++++++++++---- .../ForwardedRequestCustomizerTest.java | 91 +++++++++++++++++- 4 files changed, 176 insertions(+), 26 deletions(-) diff --git a/jetty-server/src/main/config/etc/jetty-http-forwarded.xml b/jetty-server/src/main/config/etc/jetty-http-forwarded.xml index 0aacbb2468b..a842e0a4eff 100644 --- a/jetty-server/src/main/config/etc/jetty-http-forwarded.xml +++ b/jetty-server/src/main/config/etc/jetty-http-forwarded.xml @@ -4,14 +4,17 @@ + + + - - + + + - diff --git a/jetty-server/src/main/config/modules/http-forwarded.mod b/jetty-server/src/main/config/modules/http-forwarded.mod index 0915eece9e2..435a68690e6 100644 --- a/jetty-server/src/main/config/modules/http-forwarded.mod +++ b/jetty-server/src/main/config/modules/http-forwarded.mod @@ -11,10 +11,14 @@ etc/jetty-http-forwarded.xml [ini-template] ### ForwardedRequestCustomizer Configuration +# jetty.httpConfig.forwardedOnly=false +# jetty.httpConfig.forwardedProxyAsAuthority=false +# jetty.httpConfig.forwardedHeader=Forwarded # jetty.httpConfig.forwardedHostHeader=X-Forwarded-Host # jetty.httpConfig.forwardedServerHeader=X-Forwarded-Server # jetty.httpConfig.forwardedProtoHeader=X-Forwarded-Proto # jetty.httpConfig.forwardedForHeader=X-Forwarded-For -# jetty.httpConfig.forwardedSslSessionIdHeader= -# jetty.httpConfig.forwardedCipherSuiteHeader= +# jetty.httpConfig.forwardedHttpsHeader=X-Proxied-Https +# jetty.httpConfig.forwardedSslSessionIdHeader=Proxy-ssl-id +# jetty.httpConfig.forwardedCipherSuiteHeader=Proxy-auth-cert diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java index da362580608..48289da495c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java @@ -54,7 +54,6 @@ import org.eclipse.jetty.util.StringUtil; */ public class ForwardedRequestCustomizer implements Customizer { - private HostPortHttpField _forcedHost; private String _forwardedHeader = HttpHeader.FORWARDED.toString(); private String _forwardedHostHeader = HttpHeader.X_FORWARDED_HOST.toString(); @@ -62,11 +61,11 @@ public class ForwardedRequestCustomizer implements Customizer private String _forwardedForHeader = HttpHeader.X_FORWARDED_FOR.toString(); private String _forwardedProtoHeader = HttpHeader.X_FORWARDED_PROTO.toString(); private String _forwardedHttpsHeader = "X-Proxied-Https"; - private String _forwardedCipherSuiteHeader; - private String _forwardedSslSessionIdHeader; + private String _forwardedCipherSuiteHeader = "Proxy-auth-cert"; + private String _forwardedSslSessionIdHeader = "Proxy-ssl-id"; private boolean _proxyAsAuthority=false; + private boolean _sslIsSecure=true; - /** * @return true if the proxy address obtained via * X-Forwarded-Server or RFC7239 "by" is used as @@ -77,7 +76,6 @@ public class ForwardedRequestCustomizer implements Customizer return _proxyAsAuthority; } - /** * @param proxyAsAuthority if true, use the proxy address obtained via * X-Forwarded-Server or RFC7239 "by" as the request authority. @@ -87,15 +85,19 @@ public class ForwardedRequestCustomizer implements Customizer _proxyAsAuthority = proxyAsAuthority; } - /** * Configure to only support the RFC7239 Forwarded header and to - * not support any X-Forwarded- headers. + * not support any X-Forwarded- headers. This convenience method + * clears all the non RFC headers if passed true and sets them to + * the default values (if not already set) if passed false. */ public void setForwardedOnly(boolean rfc7239only) { if (rfc7239only) { + if (_forwardedHeader==null) + _forwardedHeader=HttpHeader.FORWARDED.toString(); + _forwardedHostHeader=null; _forwardedHostHeader=null; _forwardedServerHeader=null; _forwardedForHeader=null; @@ -104,15 +106,19 @@ public class ForwardedRequestCustomizer implements Customizer } else { - _forwardedHostHeader = HttpHeader.X_FORWARDED_HOST.toString(); - _forwardedServerHeader = HttpHeader.X_FORWARDED_SERVER.toString(); - _forwardedForHeader = HttpHeader.X_FORWARDED_FOR.toString(); - _forwardedProtoHeader = HttpHeader.X_FORWARDED_PROTO.toString(); - _forwardedHttpsHeader = "X-Proxied-Https"; + if (_forwardedHostHeader==null) + _forwardedHostHeader = HttpHeader.X_FORWARDED_HOST.toString(); + if (_forwardedServerHeader==null) + _forwardedServerHeader = HttpHeader.X_FORWARDED_SERVER.toString(); + if (_forwardedForHeader==null) + _forwardedForHeader = HttpHeader.X_FORWARDED_FOR.toString(); + if (_forwardedProtoHeader==null) + _forwardedProtoHeader = HttpHeader.X_FORWARDED_PROTO.toString(); + if (_forwardedHttpsHeader==null) + _forwardedHttpsHeader = "X-Proxied-Https"; } } - public String getForcedHost() { return _forcedHost.getValue(); @@ -129,6 +135,23 @@ public class ForwardedRequestCustomizer implements Customizer _forcedHost = new HostPortHttpField(hostAndPort); } + /** + * @return The header name for RFC forwarded (default Forwarded) + */ + public String getForwardedHeader() + { + return _forwardedHeader; + } + + /** + * @param forwardedHeader + * The header name for RFC forwarded (default Forwarded) + */ + public void setForwardedHeader(String forwardedHeader) + { + _forwardedHeader = forwardedHeader; + } + public String getForwardedHostHeader() { return _forwardedHostHeader; @@ -199,7 +222,7 @@ public class ForwardedRequestCustomizer implements Customizer } /** - * @return The header name holding a forwarded cipher suite (default null) + * @return The header name holding a forwarded cipher suite (default Proxy-auth-cert) */ public String getForwardedCipherSuiteHeader() { @@ -208,7 +231,7 @@ public class ForwardedRequestCustomizer implements Customizer /** * @param forwardedCipherSuite - * The header name holding a forwarded cipher suite (default null) + * The header name holding a forwarded cipher suite (default Proxy-auth-cert) */ public void setForwardedCipherSuiteHeader(String forwardedCipherSuite) { @@ -216,7 +239,7 @@ public class ForwardedRequestCustomizer implements Customizer } /** - * @return The header name holding a forwarded SSL Session ID (default null) + * @return The header name holding a forwarded SSL Session ID (default Proxy-ssl-id) */ public String getForwardedSslSessionIdHeader() { @@ -225,7 +248,7 @@ public class ForwardedRequestCustomizer implements Customizer /** * @param forwardedSslSessionId - * The header name holding a forwarded SSL Session ID (default null) + * The header name holding a forwarded SSL Session ID (default Proxy-ssl-id) */ public void setForwardedSslSessionIdHeader(String forwardedSslSessionId) { @@ -247,6 +270,24 @@ public class ForwardedRequestCustomizer implements Customizer { _forwardedHttpsHeader = forwardedHttpsHeader; } + + /** + * @return true if the presence of a SSL session or certificate header is sufficient + * to indicate a secure request (default is true) + */ + public boolean isSslIsSecure() + { + return _sslIsSecure; + } + + /** + * @param sslIsSecure true if the presence of a SSL session or certificate header is sufficient + * to indicate a secure request (default is true) + */ + public void setSslIsSecure(boolean sslIsSecure) + { + _sslIsSecure = sslIsSecure; + } @Override public void customize(Connector connector, HttpConfiguration config, Request request) @@ -265,11 +306,25 @@ public class ForwardedRequestCustomizer implements Customizer { String name = field.getName(); - if (getForwardedCipherSuiteHeader()!=null && httpFields.get(getForwardedCipherSuiteHeader()).equalsIgnoreCase(name)) + if (getForwardedCipherSuiteHeader()!=null && getForwardedCipherSuiteHeader().equalsIgnoreCase(name)) + { request.setAttribute("javax.servlet.request.cipher_suite",field.getValue()); + if (isSslIsSecure()) + { + request.setSecure(true); + request.setScheme(config.getSecureScheme()); + } + } - if (getForwardedSslSessionIdHeader()!=null && httpFields.get(getForwardedSslSessionIdHeader()).equalsIgnoreCase(name)) + if (getForwardedSslSessionIdHeader()!=null && getForwardedSslSessionIdHeader().equalsIgnoreCase(name)) + { request.setAttribute("javax.servlet.request.ssl_session_id", field.getValue()); + if (isSslIsSecure()) + { + request.setSecure(true); + request.setScheme(config.getSecureScheme()); + } + } if (forwardedHost==null && _forwardedHostHeader!=null && _forwardedHostHeader.equalsIgnoreCase(name)) forwardedHost = getLeftMost(field.getValue()); @@ -444,5 +499,4 @@ public class ForwardedRequestCustomizer implements Customizer } } } - } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index aa47e8fc8f0..8315c539b63 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -19,11 +19,15 @@ package org.eclipse.jetty.server; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.util.ArrayDeque; import java.util.Deque; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -34,6 +38,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.IO; import org.hamcrest.Matchers; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -43,6 +48,11 @@ public class ForwardedRequestCustomizerTest private LocalConnector _connector; private RequestHandler _handler; final Deque _results = new ArrayDeque<>(); + final AtomicBoolean _wasSecure = new AtomicBoolean(false); + final AtomicReference _sslSession = new AtomicReference<>(); + final AtomicReference _sslCertificate = new AtomicReference<>(); + + ForwardedRequestCustomizer _customizer; @Before public void init() throws Exception @@ -53,7 +63,7 @@ public class ForwardedRequestCustomizerTest http.getHttpConfiguration().setRequestHeaderSize(512); http.getHttpConfiguration().setResponseHeaderSize(512); http.getHttpConfiguration().setOutputBufferSize(2048); - http.getHttpConfiguration().addCustomizer(new ForwardedRequestCustomizer()); + http.getHttpConfiguration().addCustomizer(_customizer=new ForwardedRequestCustomizer()); _connector = new LocalConnector(_server,http); _server.addConnector(_connector); _handler = new RequestHandler(); @@ -64,6 +74,9 @@ public class ForwardedRequestCustomizerTest @Override public boolean check(HttpServletRequest request,HttpServletResponse response) { + _wasSecure.set(request.isSecure()); + _sslSession.set(String.valueOf(request.getAttribute("javax.servlet.request.ssl_session_id"))); + _sslCertificate.set(String.valueOf(request.getAttribute("javax.servlet.request.cipher_suite"))); _results.add(request.getScheme()); _results.add(request.getServerName()); _results.add(Integer.toString(request.getServerPort())); @@ -203,7 +216,83 @@ public class ForwardedRequestCustomizerTest assertEquals("443",_results.poll()); assertEquals("0.0.0.0",_results.poll()); assertEquals("0",_results.poll()); + assertTrue(_wasSecure.get()); } + + @Test + public void testSslSession() throws Exception + { + _customizer.setSslIsSecure(false); + String response=_connector.getResponse( + "GET / HTTP/1.1\n"+ + "Host: myhost\n"+ + "Proxy-Ssl-Id: Wibble\n"+ + "\n"); + + assertThat(response, Matchers.containsString("200 OK")); + assertEquals("http",_results.poll()); + assertEquals("myhost",_results.poll()); + assertEquals("80",_results.poll()); + assertEquals("0.0.0.0",_results.poll()); + assertEquals("0",_results.poll()); + assertFalse(_wasSecure.get()); + assertEquals("Wibble",_sslSession.get()); + + _customizer.setSslIsSecure(true); + response=_connector.getResponse( + "GET / HTTP/1.1\n"+ + "Host: myhost\n"+ + "Proxy-Ssl-Id: 0123456789abcdef\n"+ + "\n"); + + assertThat(response, Matchers.containsString("200 OK")); + assertEquals("https",_results.poll()); + assertEquals("myhost",_results.poll()); + assertEquals("443",_results.poll()); + assertEquals("0.0.0.0",_results.poll()); + assertEquals("0",_results.poll()); + assertTrue(_wasSecure.get()); + assertEquals("0123456789abcdef",_sslSession.get()); + } + + @Test + public void testSslCertificate() throws Exception + { + _customizer.setSslIsSecure(false); + String response=_connector.getResponse( + "GET / HTTP/1.1\n"+ + "Host: myhost\n"+ + "Proxy-auth-cert: Wibble\n"+ + "\n"); + + assertThat(response, Matchers.containsString("200 OK")); + assertEquals("http",_results.poll()); + assertEquals("myhost",_results.poll()); + assertEquals("80",_results.poll()); + assertEquals("0.0.0.0",_results.poll()); + assertEquals("0",_results.poll()); + assertFalse(_wasSecure.get()); + assertEquals("Wibble",_sslCertificate.get()); + + + _customizer.setSslIsSecure(true); + response=_connector.getResponse( + "GET / HTTP/1.1\n"+ + "Host: myhost\n"+ + "Proxy-auth-cert: 0123456789abcdef\n"+ + "\n"); + + assertThat(response, Matchers.containsString("200 OK")); + assertEquals("https",_results.poll()); + assertEquals("myhost",_results.poll()); + assertEquals("443",_results.poll()); + assertEquals("0.0.0.0",_results.poll()); + assertEquals("0",_results.poll()); + assertTrue(_wasSecure.get()); + assertEquals("0123456789abcdef",_sslCertificate.get()); + } + + interface RequestTester {