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

proposal: net/http: support TLS interface for HTTP/2 #64359

Closed
ErikPelli opened this issue Nov 23, 2023 · 2 comments
Closed

proposal: net/http: support TLS interface for HTTP/2 #64359

ErikPelli opened this issue Nov 23, 2023 · 2 comments
Labels
Milestone

Comments

@ErikPelli
Copy link
Contributor

Proposal Details

In my job I had a task where it was required to intercept and manually parse all the raw HTTP data that a server received from the client in a HTTPS GET request (without relying on *http.Request).
However, we don't use the internal TLS implementation of http.Server and we rely on an external function that spawns a *tls.Conn with a different underlying connection.

For HTTP/1.0 and 1.1 the logger it's straightforward, just return a custom connection wrapper in the listener passed to Serve() and overwrite the Read() method , something like:

type CustomListener struct {
	net.Listener
}

func (c *CustomListener) Accept() (net.Conn, error) {
	conn, err := c.Listener.Accept()
	if err != nil {
		return nil, err
	}
	tlsConn, ok := conn.(*tls.Conn)
	if !ok {
		return nil, errors.New("invalid TLS connection")
	}
	return &CustomLogger{Conn: tlsConn}, nil
}

type CustomLogger struct {
      *tls.Conn
}

func (c *CustomLogger) Read(b []byte) (n int, err error) {
      n, err = c.Conn.Read(b)
      if err != nil {
        return n, err
      }
      // here we can save the data and parse them later (b[:n])
}

Now, it comes the tricky part. For HTTP/2 there is a constraint specified in the Serve documentation:

func Serve(l net.Listener, handler Handler) error

Serve accepts incoming HTTP connections on the listener l, creating a new service goroutine for each. The service goroutines read requests and then call handler to reply to them.

The handler is typically nil, in which case the DefaultServeMux is used.

HTTP/2 support is only enabled if the Listener returns *tls.Conn connections and they were configured with "h2" in the TLS Config.NextProtos.

Serve always returns a non-nil error. 

"h2" protocol can be easily configured in the TLSConfig, but there is a strong dependency on the concrete *tls.Conn type, and if we use our wrapped connection as the connection returned in the listener, HTTP/2 will be disabled.
Luckily I found a fork of "net/http", oohttp that adds exactly this single functionality that I need, basically the author, @bassosimone, replaced *tls.Conn with an interface TLSConn defined like this:

type TLSConn interface {
	// net.Conn is the underlying interface
	net.Conn

	// ConnectionState returns the ConnectionState according
	// to the standard library.
	ConnectionState() tls.ConnectionState

	// HandshakeContext performs an TLS handshake bounded
	// in time by the given context.
	HandshakeContext(ctx context.Context) error

	// NetConn returns the underlying net.Conn
	NetConn() net.Conn
}

They included in this interface the TLS functions used in the http library but changed the check from a *tls.Conn to a connection that implement this interface, to support their external tls handshake.

This code has been very useful to me, and I propose to integrate something like this in the main "net/http" library.
Since this is an advanced usage of the functionality, maybe we can add a flag in the Client/Server configuration that chooses to check for *tls.Conn (default) or an interface similar to this TLSConn.
*tls.Conn actually implements by default the interface TLSConn so this wouldn't be a breaking change for external APIs, so maybe we can get rid of *tls.Conn checks totally (open to discuss all of this here).

@gopherbot gopherbot added this to the Proposal milestone Nov 23, 2023
@seankhliao
Copy link
Member

see previously #21753 folded into #5465

@ErikPelli
Copy link
Contributor Author

Oh wow, no one touched them in the last 6 to 10 years

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants