commit 19b317e7814cc0cfaab8b20bc61c3663005c02dc Author: David Fifield david@bamsoftware.com Date: Mon Mar 12 13:34:57 2018 -0700

Use ListenAndServe{TLS} rather than separate Listen and Serve.

This is a port of commit cea86c937dc278ba6b2100c238b1d5206bbae2f0 from meek. Its purpose is to remove the need to copy-paste parts of net/http.Server.ListenAndServeTLS. Here is a copy of the commit message from meek:

The net/http package provides ListenAndServe and ListenAndServeTLS functions, but it doesn't provide a way to set up a listener without also entering an infinite serve loop. This matters for ListenAndServeTLS, which sets up a lot of magic behind the scenes for TLS and HTTP/2 support. Formerly, we had copy-pasted code from ListenAndServeTLS, but that code has only gotten more complicated in upstream net/http.

The price we pay for this is that it's no longer possible for a server bindaddr to ask to listen on port 0 (i.e., a random ephemeral port). That's because we never get a change to find out what the listening address is, before entering the serve loop.

What we gain is HTTP/2 support; formerly our copy-pasted code had the side effect of disabling HTTP/2, because it was copied from an older version and did things like config.NextProtos = []string{"http/1.1"}

The new code calls http2.ConfigureServer first, but that's not what's providing HTTP/2 support. HTTP/2 support happens by default. The reason we call http2.ConfigureServer is because we need to set TLSConfig.GetCertificate, and http2.ConfigureServer is a convenient way to initialize TLSConfig in a way that is guaranteed to work with HTTP/2. --- server/server.go | 116 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 60 insertions(+), 56 deletions(-)

diff --git a/server/server.go b/server/server.go index 5ec3ff9..8fd703a 100644 --- a/server/server.go +++ b/server/server.go @@ -22,6 +22,7 @@ import ( "git.torproject.org/pluggable-transports/goptlib.git" "git.torproject.org/pluggable-transports/websocket.git/websocket" "golang.org/x/crypto/acme/autocert" + "golang.org/x/net/http2" )

const ptMethodName = "snowflake" @@ -171,64 +172,63 @@ func webSocketHandler(ws *websocket.WebSocket) { proxy(or, &conn) }

-func listenTLS(network string, addr *net.TCPAddr, m *autocert.Manager) (net.Listener, error) { - // This is cribbed from the source of net/http.Server.ListenAndServeTLS. - // We have to separate the Listen and Serve parts because we need to - // report the listening address before entering Serve (which is an - // infinite loop). +func initServer(addr *net.TCPAddr, + getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error), + listenAndServe func(*http.Server)) (*http.Server, error) { + // We're not capable of listening on port 0 (i.e., an ephemeral port + // unknown in advance). The reason is that while the net/http package + // exposes ListenAndServe and ListenAndServeTLS, those functions never + // return, so there's no opportunity to find out what the port number + // is, in between the Listen and Serve steps. // https://groups.google.com/d/msg/Golang-nuts/3F1VRCCENp8/3hcayZiwYM8J - config := &tls.Config{} - config.NextProtos = []string{"http/1.1"} - config.GetCertificate = m.GetCertificate + if addr.Port == 0 { + return nil, fmt.Errorf("cannot listen on port %d; configure a port using ServerTransportListenAddr", addr.Port) + }

- conn, err := net.ListenTCP(network, addr) + var config websocket.Config + config.MaxMessageSize = maxMessageSize + server := &http.Server{ + Addr: addr.String(), + Handler: config.Handler(webSocketHandler), + ReadTimeout: requestTimeout, + } + // We need to override server.TLSConfig.GetCertificate--but first + // server.TLSConfig needs to be non-nil. If we just create our own new + // &tls.Config, it will lack the default settings that the net/http + // package sets up for things like HTTP/2. Therefore we first call + // http2.ConfigureServer for its side effect of initializing + // server.TLSConfig properly. An alternative would be to make a dummy + // net.Listener, call Serve on it, and let it return. + // https://github.com/golang/go/issues/16588#issuecomment-237386446 + err := http2.ConfigureServer(server, nil) if err != nil { - return nil, err + return server, err } + server.TLSConfig.GetCertificate = getCertificate

- // Additionally disable SSLv3 because of the POODLE attack. - // http://googleonlinesecurity.blogspot.com/2014/10/this-poodle-bites-exploitin... - // https://code.google.com/p/go/source/detail?r=ad9e191a51946e43f1abac8b6a2fefb... - config.MinVersion = tls.VersionTLS10 - - tlsListener := tls.NewListener(conn, config) - - return tlsListener, nil -} + go listenAndServe(server)

-func startListener(network string, addr *net.TCPAddr) (net.Listener, error) { - ln, err := net.ListenTCP(network, addr) - if err != nil { - return nil, err - } - log.Printf("listening with plain HTTP on %s", ln.Addr()) - return startServer(ln) + return server, nil }

-func startListenerTLS(network string, addr *net.TCPAddr, m *autocert.Manager) (net.Listener, error) { - ln, err := listenTLS(network, addr, m) - if err != nil { - return nil, err - } - log.Printf("listening with HTTPS on %s", ln.Addr()) - return startServer(ln) +func startServer(addr *net.TCPAddr) (*http.Server, error) { + return initServer(addr, nil, func(server *http.Server) { + log.Printf("listening with plain HTTP on %s", addr) + err := server.ListenAndServe() + if err != nil { + log.Printf("error in ListenAndServe: %s", err) + } + }) }

-func startServer(ln net.Listener) (net.Listener, error) { - go func() { - defer ln.Close() - var config websocket.Config - config.MaxMessageSize = maxMessageSize - s := &http.Server{ - Handler: config.Handler(webSocketHandler), - ReadTimeout: requestTimeout, - } - err := s.Serve(ln) +func startServerTLS(addr *net.TCPAddr, getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error)) (*http.Server, error) { + return initServer(addr, getCertificate, func(server *http.Server) { + log.Printf("listening with HTTPS on %s", addr) + err := server.ListenAndServeTLS("", "") if err != nil { - log.Printf("http.Serve: %s", err) + log.Printf("error in ListenAndServeTLS: %s", err) } - }() - return ln, nil + }) }

func getCertificateCacheDir() (string, error) { @@ -303,7 +303,7 @@ func main() { // https://github.com/ietf-wg-acme/acme/blob/master/draft-ietf-acme-acme.md#htt... needHTTP01Listener := !disableTLS

- listeners := make([]net.Listener, 0) + servers := make([]*http.Server, 0) for _, bindaddr := range ptInfo.Bindaddrs { if bindaddr.MethodName != ptMethodName { pt.SmethodError(bindaddr.MethodName, "no such method") @@ -320,32 +320,36 @@ func main() { pt.SmethodError(bindaddr.MethodName, "HTTP-01 ACME listener: "+err.Error()) continue } + server := &http.Server{ + Addr: addr.String(), + Handler: certManager.HTTPHandler(nil), + } go func() { - log.Fatal(http.Serve(lnHTTP01, certManager.HTTPHandler(nil))) + log.Fatal(server.Serve(lnHTTP01)) }() - listeners = append(listeners, lnHTTP01) + servers = append(servers, server) needHTTP01Listener = false }

- var ln net.Listener + var server *http.Server args := pt.Args{} if disableTLS { args.Add("tls", "no") - ln, err = startListener("tcp", bindaddr.Addr) + server, err = startServer(bindaddr.Addr) } else { args.Add("tls", "yes") for _, hostname := range acmeHostnames { args.Add("hostname", hostname) } - ln, err = startListenerTLS("tcp", bindaddr.Addr, certManager) + server, err = startServerTLS(bindaddr.Addr, certManager.GetCertificate) } if err != nil { log.Printf("error opening listener: %s", err) pt.SmethodError(bindaddr.MethodName, err.Error()) continue } - pt.SmethodArgs(bindaddr.MethodName, ln.Addr(), args) - listeners = append(listeners, ln) + pt.SmethodArgs(bindaddr.MethodName, bindaddr.Addr, args) + servers = append(servers, server) } pt.SmethodsDone()

@@ -366,8 +370,8 @@ func main() {

// signal received, shut down log.Printf("caught signal %q, exiting", sig) - for _, ln := range listeners { - ln.Close() + for _, server := range servers { + server.Close() } for n := range handlerChan { numHandlers += n