Make web WHOIS more resilient to malformed requests

We are seeing some web WHOIS HTTP(S) requests made to our endpoints without the Host header specified. This is an error according to the HTTP/1.1 spec. However we do not want to spam our logs with errors that are outside of our control. Do not throw and return a 400 response instead.

Also re-worked the logic a bit to only return HSTS headers if we send a redirect response, not any other error responses. The tests are re-arrange to correspond with the logical flow in the code.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=207143230
This commit is contained in:
jianglai 2018-08-02 12:14:22 -07:00
parent b8bd230061
commit 8664101687
2 changed files with 108 additions and 53 deletions

View file

@ -22,6 +22,7 @@ import static io.netty.handler.codec.http.HttpHeaderNames.LOCATION;
import static io.netty.handler.codec.http.HttpHeaderValues.KEEP_ALIVE;
import static io.netty.handler.codec.http.HttpHeaderValues.TEXT_PLAIN;
import static io.netty.handler.codec.http.HttpMethod.GET;
import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
import static io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN;
import static io.netty.handler.codec.http.HttpResponseStatus.FOUND;
import static io.netty.handler.codec.http.HttpResponseStatus.METHOD_NOT_ALLOWED;
@ -30,6 +31,7 @@ import static io.netty.handler.codec.http.HttpResponseStatus.OK;
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener;
@ -88,6 +90,8 @@ public class WebWhoisRedirectHandler extends SimpleChannelInboundHandler<HttpReq
// We only support GET, any other HTTP method should result in 405 error.
if (!msg.method().equals(GET)) {
response = new DefaultFullHttpResponse(HTTP_1_1, METHOD_NOT_ALLOWED);
} else if (Strings.isNullOrEmpty(msg.headers().get(HOST))) {
response = new DefaultFullHttpResponse(HTTP_1_1, BAD_REQUEST);
} else {
// All HTTP/1.1 request must contain a Host header with the format "host:[port]".
// See https://tools.ietf.org/html/rfc2616#section-14.23
@ -104,14 +108,16 @@ public class WebWhoisRedirectHandler extends SimpleChannelInboundHandler<HttpReq
response = new DefaultFullHttpResponse(HTTP_1_1, isHttps ? FOUND : MOVED_PERMANENTLY);
String redirectUrl = String.format("https://%s/", isHttps ? redirectHost : host);
response.headers().set(LOCATION, redirectUrl);
// Add HSTS header to HTTPS response.
if (isHttps) {
response
.headers()
.set(HSTS_HEADER_NAME, String.format("max-age=%d", HSTS_MAX_AGE.getSeconds()));
}
}
}
// Add HSTS header to HTTPS response.
if (isHttps) {
response
.headers()
.set(HSTS_HEADER_NAME, String.format("max-age=%d", HSTS_MAX_AGE.getSeconds()));
}
// Common headers that need to be set on any response.
response
.headers()
.set(CONTENT_TYPE, TEXT_PLAIN)

View file

@ -19,6 +19,7 @@ import static google.registry.proxy.TestUtils.assertHttpResponseEquivalent;
import static google.registry.proxy.TestUtils.makeHttpGetRequest;
import static google.registry.proxy.TestUtils.makeHttpPostRequest;
import static google.registry.proxy.TestUtils.makeHttpResponse;
import static io.netty.handler.codec.http.HttpHeaderNames.HOST;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.http.FullHttpRequest;
@ -35,13 +36,16 @@ public class WebWhoisRedirectHandlerTest {
private static final String REDIRECT_HOST = "www.example.com";
private static final String TARGET_HOST = "whois.nic.tld";
private WebWhoisRedirectHandler redirectHandler;
private EmbeddedChannel channel;
private FullHttpRequest request;
private FullHttpResponse response;
private void setupChannel(boolean isHttps) {
channel = new EmbeddedChannel(new WebWhoisRedirectHandler(isHttps, REDIRECT_HOST));
}
private static FullHttpResponse makeRedirectResponse(
HttpResponseStatus status, String location, boolean keepAlive, boolean isHttps) {
HttpResponseStatus status, String location, boolean keepAlive, boolean hsts) {
FullHttpResponse response = makeHttpResponse("", status);
response.headers().set("content-type", "text/plain").set("content-length", "0");
if (location != null) {
@ -50,16 +54,66 @@ public class WebWhoisRedirectHandlerTest {
if (keepAlive) {
response.headers().set("connection", "keep-alive");
}
if (isHttps) {
if (hsts) {
response.headers().set("Strict-Transport-Security", "max-age=31536000");
}
return response;
}
// HTTP redirect tests.
@Test
public void testSuccess_http_notGet() {
setupChannel(false);
request = makeHttpPostRequest("", TARGET_HOST, "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.METHOD_NOT_ALLOWED, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_http_badHost() {
setupChannel(false);
request = makeHttpGetRequest("", "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.BAD_REQUEST, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_http_noHost() {
setupChannel(false);
request = makeHttpGetRequest("", "/");
request.headers().remove(HOST);
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.BAD_REQUEST, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_http_healthCheck() {
setupChannel(false);
request = makeHttpPostRequest("", TARGET_HOST, "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.METHOD_NOT_ALLOWED, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_http_redirectToHttps() {
redirectHandler = new WebWhoisRedirectHandler(false, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
setupChannel(false);
request = makeHttpGetRequest(TARGET_HOST, "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
@ -73,8 +127,7 @@ public class WebWhoisRedirectHandlerTest {
@Test
public void testSuccess_http_redirectToHttps_hostAndPort() {
redirectHandler = new WebWhoisRedirectHandler(false, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
setupChannel(false);
request = makeHttpGetRequest(TARGET_HOST + ":80", "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
@ -88,8 +141,7 @@ public class WebWhoisRedirectHandlerTest {
@Test
public void testSuccess_http_redirectToHttps_noKeepAlive() {
redirectHandler = new WebWhoisRedirectHandler(false, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
setupChannel(false);
request = makeHttpGetRequest(TARGET_HOST, "/");
request.headers().set("connection", "close");
// No inbound message passed to the next handler.
@ -102,10 +154,11 @@ public class WebWhoisRedirectHandlerTest {
assertThat(channel.isActive()).isFalse();
}
// HTTPS redirect tests.
@Test
public void testSuccess_http_notGet() {
redirectHandler = new WebWhoisRedirectHandler(false, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
public void testSuccess_https_notGet() {
setupChannel(true);
request = makeHttpPostRequest("", TARGET_HOST, "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
@ -116,22 +169,45 @@ public class WebWhoisRedirectHandlerTest {
}
@Test
public void testSuccess_http_healthCheck() {
redirectHandler = new WebWhoisRedirectHandler(false, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
request = makeHttpPostRequest("", TARGET_HOST, "/");
public void testSuccess_https_badHost() {
setupChannel(true);
request = makeHttpGetRequest("", "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.METHOD_NOT_ALLOWED, null, true, false));
response, makeRedirectResponse(HttpResponseStatus.BAD_REQUEST, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_https_noHost() {
setupChannel(true);
request = makeHttpGetRequest("", "/");
request.headers().remove(HOST);
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.BAD_REQUEST, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_https_healthCheck() {
setupChannel(true);
request = makeHttpGetRequest("health-check.invalid", "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.FORBIDDEN, null, true, false));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_https_redirectToDestination() {
redirectHandler = new WebWhoisRedirectHandler(true, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
setupChannel(true);
request = makeHttpGetRequest(TARGET_HOST, "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
@ -144,8 +220,7 @@ public class WebWhoisRedirectHandlerTest {
@Test
public void testSuccess_https_redirectToDestination_noKeepAlive() {
redirectHandler = new WebWhoisRedirectHandler(true, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
setupChannel(true);
request = makeHttpGetRequest(TARGET_HOST, "/");
request.headers().set("connection", "close");
// No inbound message passed to the next handler.
@ -156,30 +231,4 @@ public class WebWhoisRedirectHandlerTest {
makeRedirectResponse(HttpResponseStatus.FOUND, "https://www.example.com/", false, true));
assertThat(channel.isActive()).isFalse();
}
@Test
public void testSuccess_https_notGet() {
redirectHandler = new WebWhoisRedirectHandler(true, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
request = makeHttpPostRequest("", TARGET_HOST, "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.METHOD_NOT_ALLOWED, null, true, true));
assertThat(channel.isActive()).isTrue();
}
@Test
public void testSuccess_https_healthCheck() {
redirectHandler = new WebWhoisRedirectHandler(true, REDIRECT_HOST);
channel = new EmbeddedChannel(redirectHandler);
request = makeHttpGetRequest("health-check.invalid", "/");
// No inbound message passed to the next handler.
assertThat(channel.writeInbound(request)).isFalse();
response = channel.readOutbound();
assertHttpResponseEquivalent(
response, makeRedirectResponse(HttpResponseStatus.FORBIDDEN, null, true, true));
assertThat(channel.isActive()).isTrue();
}
}