commit 19b317e7814cc0cfaab8b20bc61c3663005c02dc
Author: David Fifield <da...@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



_______________________________________________
tor-commits mailing list
tor-commits@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits

Reply via email to