[tor-commits] [snowflake/master] Use ListenAndServe{TLS} rather than separate Listen and Serve.

dcf at torproject.org dcf at torproject.org
Wed Mar 14 02:21:20 UTC 2018


commit 19b317e7814cc0cfaab8b20bc61c3663005c02dc
Author: David Fifield <david at 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-exploiting-ssl-30.html
-	// https://code.google.com/p/go/source/detail?r=ad9e191a51946e43f1abac8b6a2fefbf2291eea7
-	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#http-challenge
 	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





More information about the tor-commits mailing list