Ticket #25434: bug25434-1.patch

File bug25434-1.patch, 12.2 KB (added by dcf, 16 months ago)
  • server/server.go

    From 5308f318a06ab82c7e3525fb61c4d67b3e5f594a Mon Sep 17 00:00:00 2001
    From: David Fifield <david@bamsoftware.com>
    Date: Mon, 12 Mar 2018 13:34:57 -0700
    Subject: [PATCH 1/2] 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 b import ( 
    2222        "git.torproject.org/pluggable-transports/goptlib.git"
    2323        "git.torproject.org/pluggable-transports/websocket.git/websocket"
    2424        "golang.org/x/crypto/acme/autocert"
     25        "golang.org/x/net/http2"
    2526)
    2627
    2728const ptMethodName = "snowflake"
    func webSocketHandler(ws *websocket.WebSocket) { 
    171172        proxy(or, &conn)
    172173}
    173174
    174 func listenTLS(network string, addr *net.TCPAddr, m *autocert.Manager) (net.Listener, error) {
    175         // This is cribbed from the source of net/http.Server.ListenAndServeTLS.
    176         // We have to separate the Listen and Serve parts because we need to
    177         // report the listening address before entering Serve (which is an
    178         // infinite loop).
     175func initServer(addr *net.TCPAddr,
     176        getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error),
     177        listenAndServe func(*http.Server)) (*http.Server, error) {
     178        // We're not capable of listening on port 0 (i.e., an ephemeral port
     179        // unknown in advance). The reason is that while the net/http package
     180        // exposes ListenAndServe and ListenAndServeTLS, those functions never
     181        // return, so there's no opportunity to find out what the port number
     182        // is, in between the Listen and Serve steps.
    179183        // https://groups.google.com/d/msg/Golang-nuts/3F1VRCCENp8/3hcayZiwYM8J
    180         config := &tls.Config{}
    181         config.NextProtos = []string{"http/1.1"}
    182         config.GetCertificate = m.GetCertificate
     184        if addr.Port == 0 {
     185                return nil, fmt.Errorf("cannot listen on port %d; configure a port using ServerTransportListenAddr", addr.Port)
     186        }
    183187
    184         conn, err := net.ListenTCP(network, addr)
     188        var config websocket.Config
     189        config.MaxMessageSize = maxMessageSize
     190        server := &http.Server{
     191                Addr:        addr.String(),
     192                Handler:     config.Handler(webSocketHandler),
     193                ReadTimeout: requestTimeout,
     194        }
     195        // We need to override server.TLSConfig.GetCertificate--but first
     196        // server.TLSConfig needs to be non-nil. If we just create our own new
     197        // &tls.Config, it will lack the default settings that the net/http
     198        // package sets up for things like HTTP/2. Therefore we first call
     199        // http2.ConfigureServer for its side effect of initializing
     200        // server.TLSConfig properly. An alternative would be to make a dummy
     201        // net.Listener, call Serve on it, and let it return.
     202        // https://github.com/golang/go/issues/16588#issuecomment-237386446
     203        err := http2.ConfigureServer(server, nil)
    185204        if err != nil {
    186                 return nil, err
     205                return server, err
    187206        }
     207        server.TLSConfig.GetCertificate = getCertificate
    188208
    189         // Additionally disable SSLv3 because of the POODLE attack.
    190         // http://googleonlinesecurity.blogspot.com/2014/10/this-poodle-bites-exploiting-ssl-30.html
    191         // https://code.google.com/p/go/source/detail?r=ad9e191a51946e43f1abac8b6a2fefbf2291eea7
    192         config.MinVersion = tls.VersionTLS10
    193 
    194         tlsListener := tls.NewListener(conn, config)
    195 
    196         return tlsListener, nil
    197 }
     209        go listenAndServe(server)
    198210
    199 func startListener(network string, addr *net.TCPAddr) (net.Listener, error) {
    200         ln, err := net.ListenTCP(network, addr)
    201         if err != nil {
    202                 return nil, err
    203         }
    204         log.Printf("listening with plain HTTP on %s", ln.Addr())
    205         return startServer(ln)
     211        return server, nil
    206212}
    207213
    208 func startListenerTLS(network string, addr *net.TCPAddr, m *autocert.Manager) (net.Listener, error) {
    209         ln, err := listenTLS(network, addr, m)
    210         if err != nil {
    211                 return nil, err
    212         }
    213         log.Printf("listening with HTTPS on %s", ln.Addr())
    214         return startServer(ln)
     214func startServer(addr *net.TCPAddr) (*http.Server, error) {
     215        return initServer(addr, nil, func(server *http.Server) {
     216                log.Printf("listening with plain HTTP on %s", addr)
     217                err := server.ListenAndServe()
     218                if err != nil {
     219                        log.Printf("error in ListenAndServe: %s", err)
     220                }
     221        })
    215222}
    216223
    217 func startServer(ln net.Listener) (net.Listener, error) {
    218         go func() {
    219                 defer ln.Close()
    220                 var config websocket.Config
    221                 config.MaxMessageSize = maxMessageSize
    222                 s := &http.Server{
    223                         Handler:     config.Handler(webSocketHandler),
    224                         ReadTimeout: requestTimeout,
    225                 }
    226                 err := s.Serve(ln)
     224func startServerTLS(addr *net.TCPAddr, getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error)) (*http.Server, error) {
     225        return initServer(addr, getCertificate, func(server *http.Server) {
     226                log.Printf("listening with HTTPS on %s", addr)
     227                err := server.ListenAndServeTLS("", "")
    227228                if err != nil {
    228                         log.Printf("http.Serve: %s", err)
     229                        log.Printf("error in ListenAndServeTLS: %s", err)
    229230                }
    230         }()
    231         return ln, nil
     231        })
    232232}
    233233
    234234func getCertificateCacheDir() (string, error) {
    func main() { 
    303303        // https://github.com/ietf-wg-acme/acme/blob/master/draft-ietf-acme-acme.md#http-challenge
    304304        needHTTP01Listener := !disableTLS
    305305
    306         listeners := make([]net.Listener, 0)
     306        servers := make([]*http.Server, 0)
    307307        for _, bindaddr := range ptInfo.Bindaddrs {
    308308                if bindaddr.MethodName != ptMethodName {
    309309                        pt.SmethodError(bindaddr.MethodName, "no such method")
    func main() { 
    320320                                pt.SmethodError(bindaddr.MethodName, "HTTP-01 ACME listener: "+err.Error())
    321321                                continue
    322322                        }
     323                        server := &http.Server{
     324                                Addr:    addr.String(),
     325                                Handler: certManager.HTTPHandler(nil),
     326                        }
    323327                        go func() {
    324                                 log.Fatal(http.Serve(lnHTTP01, certManager.HTTPHandler(nil)))
     328                                log.Fatal(server.Serve(lnHTTP01))
    325329                        }()
    326                         listeners = append(listeners, lnHTTP01)
     330                        servers = append(servers, server)
    327331                        needHTTP01Listener = false
    328332                }
    329333
    330                 var ln net.Listener
     334                var server *http.Server
    331335                args := pt.Args{}
    332336                if disableTLS {
    333337                        args.Add("tls", "no")
    334                         ln, err = startListener("tcp", bindaddr.Addr)
     338                        server, err = startServer(bindaddr.Addr)
    335339                } else {
    336340                        args.Add("tls", "yes")
    337341                        for _, hostname := range acmeHostnames {
    338342                                args.Add("hostname", hostname)
    339343                        }
    340                         ln, err = startListenerTLS("tcp", bindaddr.Addr, certManager)
     344                        server, err = startServerTLS(bindaddr.Addr, certManager.GetCertificate)
    341345                }
    342346                if err != nil {
    343347                        log.Printf("error opening listener: %s", err)
    344348                        pt.SmethodError(bindaddr.MethodName, err.Error())
    345349                        continue
    346350                }
    347                 pt.SmethodArgs(bindaddr.MethodName, ln.Addr(), args)
    348                 listeners = append(listeners, ln)
     351                pt.SmethodArgs(bindaddr.MethodName, bindaddr.Addr, args)
     352                servers = append(servers, server)
    349353        }
    350354        pt.SmethodsDone()
    351355
    func main() { 
    366370
    367371        // signal received, shut down
    368372        log.Printf("caught signal %q, exiting", sig)
    369         for _, ln := range listeners {
    370                 ln.Close()
     373        for _, server := range servers {
     374                server.Close()
    371375        }
    372376        for n := range handlerChan {
    373377                numHandlers += n
  • server/server.go

    -- 
    2.16.1
    
    
    From 503ce80cd39c918b0d6b6493e0a0fad956b1087c Mon Sep 17 00:00:00 2001
    From: David Fifield <david@bamsoftware.com>
    Date: Mon, 12 Mar 2018 14:21:11 -0700
    Subject: [PATCH 2/2] Wait briefly after calling ListenAndServe{TLS} to see if
     it errors.
    
    This is a port of commit e3f3054f8b74caa639a6d9be09702693af9a70e7 from
    meek.
    
    In the previous commit, we changed from separate Listen and Serve steps
    to always calling ListenAndServe. However, we would really like to
    immediately get feedback if any errors happen in the Listen step inside
    the call, because it's much better for debugging if those errors get
    reported to tor through SMETHOD-ERROR--rather than reporting success to
    tor and actually logging an error only in the snowflake log. So we wait
    100 ms for an error to occur before deciding that the Listen succeeded.
    
    We don't need to apply this hack to the ACME HTTP-01 listener, because
    it's a plaintext listener. Unlike in the TLS case, there isn't any
    internal magic that the net library does that we have to rely on. We
    just call net.ListenTCP and check for an error.
    ---
     server/server.go | 30 +++++++++++++++++++++++++-----
     1 file changed, 25 insertions(+), 5 deletions(-)
    
    diff --git a/server/server.go b/server/server.go
    index 8fd703a..0136fc0 100644
    a b const requestTimeout = 10 * time.Second 
    3030
    3131const maxMessageSize = 64 * 1024
    3232
     33// How long to wait for ListenAndServe or ListenAndServeTLS to return an error
     34// before deciding that it's not going to return.
     35const listenAndServeErrorTimeout = 100 * time.Millisecond
     36
    3337var ptInfo pt.ServerInfo
    3438
    3539// When a connection handler starts, +1 is written to this channel; when it
    func webSocketHandler(ws *websocket.WebSocket) { 
    174178
    175179func initServer(addr *net.TCPAddr,
    176180        getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error),
    177         listenAndServe func(*http.Server)) (*http.Server, error) {
     181        listenAndServe func(*http.Server, chan<- error)) (*http.Server, error) {
    178182        // We're not capable of listening on port 0 (i.e., an ephemeral port
    179183        // unknown in advance). The reason is that while the net/http package
    180184        // exposes ListenAndServe and ListenAndServeTLS, those functions never
    func initServer(addr *net.TCPAddr, 
    206210        }
    207211        server.TLSConfig.GetCertificate = getCertificate
    208212
    209         go listenAndServe(server)
     213        // Another unfortunate effect of the inseparable net/http ListenAndServe
     214        // is that we can't check for Listen errors like "permission denied" and
     215        // "address already in use" without potentially entering the infinite
     216        // loop of Serve. The hack we apply here is to wait a short time,
     217        // listenAndServeErrorTimeout, to see if an error is returned (because
     218        // it's better if the error message goes to the tor log through
     219        // SMETHOD-ERROR than if it only goes to the snowflake log).
     220        errChan := make(chan error)
     221        go listenAndServe(server, errChan)
     222        select {
     223        case err = <-errChan:
     224                break
     225        case <-time.After(listenAndServeErrorTimeout):
     226                break
     227        }
    210228
    211         return server, nil
     229        return server, err
    212230}
    213231
    214232func startServer(addr *net.TCPAddr) (*http.Server, error) {
    215         return initServer(addr, nil, func(server *http.Server) {
     233        return initServer(addr, nil, func(server *http.Server, errChan chan<- error) {
    216234                log.Printf("listening with plain HTTP on %s", addr)
    217235                err := server.ListenAndServe()
    218236                if err != nil {
    219237                        log.Printf("error in ListenAndServe: %s", err)
    220238                }
     239                errChan <- err
    221240        })
    222241}
    223242
    224243func startServerTLS(addr *net.TCPAddr, getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error)) (*http.Server, error) {
    225         return initServer(addr, getCertificate, func(server *http.Server) {
     244        return initServer(addr, getCertificate, func(server *http.Server, errChan chan<- error) {
    226245                log.Printf("listening with HTTPS on %s", addr)
    227246                err := server.ListenAndServeTLS("", "")
    228247                if err != nil {
    229248                        log.Printf("error in ListenAndServeTLS: %s", err)
    230249                }
     250                errChan <- err
    231251        })
    232252}
    233253