Block requests with multiple Host headers #1060
Conversation
| } else if (zuulRequest.getHeaders().getAll(HttpHeaderNames.HOST.toString()).size() > 1) { | ||
| LOG.debug( | ||
| "Duplicate Host headers. clientRequest = {} , uri = {}, info = {}", | ||
| clientRequest.toString(), |
| zuulRequest.getContext(), | ||
| ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST); | ||
| zuulRequest.getContext().setError(ze); | ||
| zuulRequest.getContext().setShouldSendErrorResponse(true); |
There was a problem hiding this comment.
This will continue the filter chain processing, and actually write a response to the client.
Is that desirable?
There was a problem hiding this comment.
Yes, I believe so, but interested in hearing your thoughts. The other option would be to close the connection without a response (we do this a few lines above). It seems best to let the client know what they did wrong.
There was a problem hiding this comment.
I'm not aware of a good spec for this case.
My general preference is to not continue filter processing and expend resources writing back a response, when we treat a request as malicious.
There was a problem hiding this comment.
For this change, I'm inclined to provide the client a 400 response given that it could impact non-malicious requests. For example, a client or upstream proxy may be accidentally adding the Host header twice. Closing the connection without a response would make it very difficult to troubleshoot this change when it rolls out. I think the better client experience outweighs the extra resource spend here.
jfernandez
changed the title
| @@ -159,6 +158,19 @@ private void channelReadInternal(final ChannelHandlerContext ctx, Object msg) th | |||
| ZuulStatusCategory.FAILURE_CLIENT_BAD_REQUEST); | |||
| zuulRequest.getContext().setError(ze); | |||
| zuulRequest.getContext().setShouldSendErrorResponse(true); | |||
| } else if (zuulRequest.getHeaders().getAll(HttpHeaderNames.HOST.toString()).size() > 1) { | |||
There was a problem hiding this comment.
This should be HOST.getName()
There was a problem hiding this comment.
HOST is an AsciiString, that method doesn't resolve for it.
There was a problem hiding this comment.
edit: com.netflix.zuul.message.http.HttpHeaderNames
No description provided.