net/http: add logging before returning error 400 HTTP request to HTTPS server

[1f604]

Proposal Details

Proposal

This proposal is specifically talking about this line of code here:

go/src/net/http/server.go

Line 1926 in c2c4a32

io.WriteString(re.Conn, "HTTP/1.0 400 Bad Request\r\n\r\nClient sent an HTTP request to an HTTPS server.\n")

My proposal is to add a line writing some error message to logs before returning the 400 error.

Motivation

Spent almost an hour today tracking down a bug which turned out to be due to Cloudflare proxy sending plain HTTP traffic (decrypted from TLS traffic) to my Go server which serves TLS.

It turns out that ListenAndServeTLS doesn't log any error messages when it receives a HTTP request, instead it just sends error: "Client sent an HTTP request to an HTTPS server." back to the client.

The problem is that my web browsers (tried both Firefox and Chrome, same result) don't display the message at all, it just shows an error 400 and that's it.

And in my Go program, there is no indication that it received any packets - no logging and there is no ability for me to add any logging to it at all.

I think the ideal solution would be to allow users to specify whether they want logging here, since some users might want it and some users might not.

But the simplest fix is to just add the logging for now.

[gopherbot]

Change https://go.dev/cl/573995 mentions this issue: net/http: log error when HTTPS server receives HTTP request

[seankhliao]

cc @neild

[1f604]

Hi @seankhliao could you share your thoughts on this proposal?

I know you're not obligated to explain downvotes but other people would also benefit from you sharing your perspectives on this matter.

If the problem is that my proposal doesn't contain enough information then I'd be happy to add more information.

[AlexanderYastrebov]

Previous related #56028

[mitar]

I think this is a duplicate of #49310 for which a draft CL already exists.

[1f604]

I think this is a duplicate of #49310 for which a draft CL already exists.

No, that proposal is about modifying the HTTP(S?) response to the client, whereas this proposal is about printing something to server's console via stdout/stderr.

[mitar]

That proposal is of having a hook when this happens. You can also log in that hook, or not. You yourself wrote:

no logging and there is no ability for me to add any logging to it at all.

#49310 gives you that ability, or if current proposal in that issue is lacking/preventing you from using it to do logging, make a comment there.

[1f604]

That proposal is of having a hook when this happens. You can also log in that hook, or not. You yourself wrote:

no logging and there is no ability for me to add any logging to it at all.

#49310 gives you that ability, or if current proposal in that issue is lacking/preventing you from using it to do logging, make a comment there.

Okay, I see so the idea is to allow users to write their own HttpOnHttpsPortErrorHandler func(conn net.Conn, requestBytes []byte) function that would have access to the net.Conn which would have the RemoteAddr.

Yes you're right that would allow me to achieve what I want here and it would be a more flexible solution compared to what I want to do.

[neild]

We have an existing log line for TLS handshake errors, we just suppress it for requests that look like HTTP:

if re, ok := err.(tls.RecordHeaderError); ok && re.Conn != nil && tlsRecordHeaderLooksLikeHTTP(re.RecordHeader) {
  io.WriteString(re.Conn, "HTTP/1.0 400 Bad Request\r\n\r\nClient sent an HTTP request to an HTTPS server.\n")
  re.Conn.Close()
  return
}
c.server.logf("http: TLS handshake error from %s: %v", c.rwc.RemoteAddr(), err)

https://go.googlesource.com/go/+/refs/tags/go1.22.1/src/net/http/server.go#1930

If we're going to log some TLS handshake errors, I don't see any reason we shouldn't be logging all of them.

[neild]

Unmarked as proposal, since I don't think this requires going through the proposal process. It's not an API change, and the behavioral change isn't particularly significant.

[gopherbot]

Change https://go.dev/cl/573979 mentions this issue: net/http: also log TLS errors which look like HTTP sent to an HTTPS port