net/http: support way to modify Request context data from net.Conn etc

[Freeaqingme]

Hi,

I've got a connection that implements the net.Conn interface. It's a custom implementation that implements the proxyprotocol v2. Part of the protocol is that the header, besides original remote address can contain additional metadata about its original form (e.g. if ssl was used, what ciphers, etc. But also things like network namespace).

I'm using the standard net/http tooling, and it provides no direct access to the underlying net.conn. That's been discussed in multiple issues, so don't want to spark that discussion yet again. I am however looking for a way to access this metadata.

The only option I'm aware of that currently exists is to Hijack() the response. That's suboptimal, however, because the reason I'm using net/http is so I don't have to implement HTTP myself :) (and there's no way to Unhijack() the connection again).

Unless there already is a workaround available that I'm not aware of, please do consider this a feature request for a way of transferring data from the underlying net.Conn to the http.Request object.

Thanks,

Dolf

• Freeaqingme

//cc @bradfitz

[bradfitz]

There's kinda a way for you to do it already. It's either gross or clever, but I'll leave that judgement up to you:

The net/http server set the Request.Context() context values http.LocalAddrContextKey and http.ServerContextKey. The LocalAddrContextKey one is set like:

func (c *conn) serve(ctx context.Context) {
        c.remoteAddr = c.rwc.RemoteAddr().String()
        ctx = context.WithValue(ctx, LocalAddrContextKey, c.rwc.LocalAddr())

Note that you control what c.rwc (your net.Conn impl) returns, and LocalAddr returns a net.Addr, an interface type. So as long as you implement Addr faithfully, you can also implement other stuff.

Related issue #16220 was about letting people adjust the base context but it fizzled out due to lack of real need.

Perhaps we could have an optional http.Server hook to let people modify the Request.Context or append context values before the Handler runs, with the hook getting the net.Conn, etc.

That would require some thought and design. I'll repurpose this bug about that if you don't mind.

/cc @tombergan

[tombergan]

Related issue #16220 was about letting people adjust the base context but it fizzled out due to lack of real need.

#18997 is going to add that support. But I don't think that well help in this example, because the hook doesn't get the net.Conn. Do you think it should actually get the net.Conn? I know we've resisted adding references to net.Conn in the past.

[bradfitz]

@tombergan, the desire to get at "the" conn never goes away, so I think it's time, with sufficiently scary names and docs. For instance, for TLS connections, do we give them both the raw connection and the TLS wrapper conn? With what names? RawEncryptedConn net.Conn sounds nice and scary for the low-level one.

   // RawEncryptedConn is the bottommost net.Conn, under the TLS connection if any.
   // It's usually of type *net.TCPConn, unless you have a weird setup.
   // You must not assume you can read or write from it, since it's likely wrapped in TLS,
   // and you don't know which HTTP protocol you're speaking. Oh, you also shouldn't
   // Close it if you don't want to break multiple requests in flight. We make no promises
   // about what will happen with this if you Read, Write, or Close it, and we reserve
   // the right to break your code if you do so. This is provided so you can get at things
   // you need to get at. We trust you to make good life decisions.
   // Enjoy.
   RawEncryptedConn net.Conn

Or something.

[Freeaqingme]

@bradfitz My suggestion would be to call it RawConn or something, so as to not make it specifically about encrypted connections. Besides that, I do like your comment :)

How can we get this ball rolling again?

[bradfitz]

I just got back to work today after some months of paternity leave. Not coincidentally, today is also the Go 1.10 feature freeze date, so this will need to be pumped to Go 1.11.

[odeke-em]

@Freeaqingme if you are interested/available, would you like to spin up a CL to get the ball rolling? The usage and tests will give folks a palatable point to start with. /cc @meirf @rs just in case y'all need this functionality too or are interested.

[bradfitz]

@Freeaqingme, was the hacky workaround I suggested in #20956 (comment) workable?

That's not to say we can't do more here, if somebody wants to pick it up.

[Freeaqingme]

This kinda fell off my radar swamped by other projects. Though there shouldn't be a reason for your suggestion not to work.

I'll see if I have some this weekend to gobble up a CL with a method to more conveniently retrieve the underlying conn (which is not necessarily encrypted but can still get a very scary name)

[spacewander]

@bradfitz
The suggestion #20956 (comment) is workable.
Here is a runnable example to access TLS parameter from *http.Request:
(ignore tha names of the structs, I didn't spend enough time on them)

package main

import (
	"crypto/rand"
	"crypto/rsa"
	"crypto/tls"
	"crypto/x509"
	"encoding/pem"
	"io"
	"math/big"
	"net"
	"net/http"
	"net/url"
)

func generateTLSConfig() *tls.Config {
	key, err := rsa.GenerateKey(rand.Reader, 1024)
	if err != nil {
		panic(err)
	}
	template := x509.Certificate{SerialNumber: big.NewInt(1)}
	certDER, err := x509.CreateCertificate(rand.Reader, &template,
		&template, &key.PublicKey, key)
	if err != nil {
		panic(err)
	}
	keyPEM := pem.EncodeToMemory(&pem.Block{
		Type:  "RSA PRIVATE KEY",
		Bytes: x509.MarshalPKCS1PrivateKey(key)},
	)
	certPEM := pem.EncodeToMemory(&pem.Block{
		Type:  "CERTIFICATE",
		Bytes: certDER,
	})

	tlsCert, err := tls.X509KeyPair(certPEM, keyPEM)
	if err != nil {
		panic(err)
	}
	return &tls.Config{
		Certificates:       []tls.Certificate{tlsCert},
		GetConfigForClient: storeClientSNI,
	}
}

var (
	tlsCfg = generateTLSConfig()
)

func storeClientSNI(chi *tls.ClientHelloInfo) (*tls.Config, error) {
	conn := chi.Conn.(*hackedInnerConn)
	conn.sni = chi.ServerName
	return nil, nil
}

type hackedListener struct {
	net.Listener
}

type hackedInnerConn struct {
	net.Conn
	sni string
}

type hackedOuterConn struct {
	net.Conn
	hic *hackedInnerConn
}

type hackedAddr struct {
	net.Addr
	hoc *hackedOuterConn
}

func (hc hackedOuterConn) LocalAddr() net.Addr {
	return hackedAddr{
		hc.Conn.LocalAddr(),
		&hc,
	}
}

func (hln hackedListener) Accept() (net.Conn, error) {
	conn, err := hln.Listener.Accept()
	if err != nil {
		panic(err)
	}
	hoc := hackedOuterConn{}
	hic := hackedInnerConn{
		conn,
		"",
	}
	hoc.Conn = tls.Server(&hic, tlsCfg)
	hoc.hic = &hic
	return hoc, nil
}

func startServer(addr string, handler http.Handler) {
	netAddr, err := url.Parse(addr)
	if err != nil {
		panic(err)
	}

	server := &http.Server{
		Handler: handler,
	}
	ln, err := net.Listen("tcp", netAddr.Host)
	if err != nil {
		panic(err)
	}
	hackedLn := hackedListener{
		ln,
	}
	err = server.Serve(hackedLn)
	if err != nil {
		panic(err)
	}
}

func mustWrite(w io.Writer, p []byte) {
	_, err := w.Write(p)
	if err != nil {
		panic(err.Error())
	}
}

func main() {
	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		haddr := r.Context().Value(http.LocalAddrContextKey).(hackedAddr)
		mustWrite(w, []byte(haddr.hoc.hic.sni+"\r\n"))
		mustWrite(w, []byte(r.Host+"\r\n"))
	})
	startServer("https://127.0.0.1:4443", handler)
}

It can be tested via a curl command: curl https://www.test.cn:4443/test -k --resolve "www.test.cn:4443:127.0.0.1"

[spacewander]

If the RawEncryptedConn solution #20956 (comment) is more preferable, I can spend some time writing a PR. Maybe in the upcoming October holiday.

But there is a question:
It looks like that we could not get the net.Conn under the TLS connection (the haddr.hoc.hic in my example) directly.
What we can get is the net.Conn returned by the net.Listener which we passed into func (srv *Server) Serve(l net.Listener) error (the haddr.hoc in my example).

[bradfitz]

As of Go 1.13, this exists: https://golang.org/pkg/net/http/#Server.ConnContext

(see #30694)

I'm pretty sure that solves these issues so closing this bug, but let me know if not.