Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow treating forwarded HTTPS requests over HTTP as secure #1442

Merged
merged 1 commit into from Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -42,4 +42,4 @@ Example configuration file:
custom-property2=custom-value2

Additionally, the coordinator must be configured to use password authentication
and have HTTPS enabled.
and have HTTPS enabled (or HTTPS forwarding enabled).
4 changes: 4 additions & 0 deletions presto-docs/src/main/sphinx/security/ldap.rst
Expand Up @@ -89,6 +89,10 @@ Property Description
used to secure TLS.
``http-server.https.keystore.key`` The password for the keystore. This must match the
password you specified when creating the keystore.
``http-server.authentication.allow-forwarded-https`` Enable treating forwarded HTTPS requests over HTTP
as secure. Requires the ``X-Forwarded-Proto`` header
to be set to ``https`` on forwarded requests.
Default value is ``false``.
======================================================= ======================================================

Password Authenticator Configuration
Expand Down
Expand Up @@ -14,7 +14,9 @@
package io.prestosql.server.security;

import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.net.HttpHeaders;

import javax.inject.Inject;
import javax.servlet.Filter;
Expand Down Expand Up @@ -43,12 +45,16 @@
public class AuthenticationFilter
implements Filter
{
private static final String HTTPS_PROTOCOL = "https";

private final List<Authenticator> authenticators;
private final boolean httpsForwardingEnabled;

@Inject
public AuthenticationFilter(List<Authenticator> authenticators)
public AuthenticationFilter(List<Authenticator> authenticators, SecurityConfig securityConfig)
{
this.authenticators = ImmutableList.copyOf(authenticators);
this.authenticators = ImmutableList.copyOf(requireNonNull(authenticators, "authenticators is null"));
this.httpsForwardingEnabled = requireNonNull(securityConfig, "securityConfig is null").getEnableForwardingHttps();
}

@Override
Expand All @@ -65,7 +71,7 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
HttpServletResponse response = (HttpServletResponse) servletResponse;

// skip authentication if non-secure or not configured
if (!request.isSecure() || authenticators.isEmpty()) {
if (!doesRequestSupportAuthentication(request)) {
nextFilter.doFilter(request, response);
return;
}
Expand Down Expand Up @@ -105,6 +111,17 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
response.sendError(SC_UNAUTHORIZED, Joiner.on(" | ").join(messages));
}

private boolean doesRequestSupportAuthentication(HttpServletRequest request)
{
if (authenticators.isEmpty()) {
return false;
}
if (request.isSecure()) {
return true;
}
return httpsForwardingEnabled && Strings.nullToEmpty(request.getHeader(HttpHeaders.X_FORWARDED_PROTO)).equalsIgnoreCase(HTTPS_PROTOCOL);
}

private static ServletRequest withPrincipal(HttpServletRequest request, Principal principal)
{
requireNonNull(principal, "principal is null");
Expand Down
Expand Up @@ -32,6 +32,7 @@ public class SecurityConfig
private static final Splitter SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();

private List<AuthenticationType> authenticationTypes = ImmutableList.of();
private boolean enableForwardingHttps;

public enum AuthenticationType
{
Expand Down Expand Up @@ -67,4 +68,17 @@ public SecurityConfig setAuthenticationTypes(String types)
.collect(toImmutableList());
return this;
}

public boolean getEnableForwardingHttps()
{
return enableForwardingHttps;
}

@Config("http-server.authentication.allow-forwarded-https")
@ConfigDescription("Enable forwarding HTTPS requests")
public SecurityConfig setEnableForwardingHttps(boolean enableForwardingHttps)
{
this.enableForwardingHttps = enableForwardingHttps;
return this;
}
}
Expand Up @@ -31,6 +31,7 @@ public class TestSecurityConfig
public void testDefaults()
{
assertRecordedDefaults(recordDefaults(SecurityConfig.class)
.setEnableForwardingHttps(false)
.setAuthenticationTypes(""));
}

Expand All @@ -39,9 +40,11 @@ public void testExplicitPropertyMappings()
{
Map<String, String> properties = new ImmutableMap.Builder<String, String>()
.put("http-server.authentication.type", "KERBEROS,PASSWORD")
.put("http-server.authentication.allow-forwarded-https", "true")
.build();

SecurityConfig expected = new SecurityConfig()
.setEnableForwardingHttps(true)
.setAuthenticationTypes(ImmutableList.of(KERBEROS, PASSWORD));

assertFullMapping(properties, expected);
Expand Down