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

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

Closed
Freeaqingme opened this issue Jul 8, 2017 · 11 comments
Closed
Labels
FeatureRequest FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Freeaqingme
Copy link

Freeaqingme commented Jul 8, 2017

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
Copy link
Contributor

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

@bradfitz bradfitz added FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 14, 2017
@bradfitz bradfitz changed the title net/http: Support passing data from net.Conn to http.Request net/http: support way to modify Request context data from net.Conn etc Jul 14, 2017
@bradfitz bradfitz added this to the Go1.10 milestone Jul 14, 2017
@tombergan
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

bradfitz commented Nov 1, 2017

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.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 1, 2017
@odeke-em
Copy link
Member

@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 bradfitz modified the milestones: Go1.11, Unplanned Jun 22, 2018
@bradfitz
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants