From 2f1f138bda84c7803ece7662bd42137d4f8bbf2c Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Wed, 8 Jul 2020 10:28:32 +0000 Subject: [PATCH] fix accidental early client.Close() --- cmd/telebit/telebit.go | 11 ++++++----- mplexer/connwrap.go | 39 +++++++++------------------------------ mplexer/listener.go | 9 ++++++--- mplexer/routemux.go | 13 +++++++------ mplexer/telebit.go | 2 +- 5 files changed, 29 insertions(+), 45 deletions(-) diff --git a/cmd/telebit/telebit.go b/cmd/telebit/telebit.go index 265ee82..6ae8013 100644 --- a/cmd/telebit/telebit.go +++ b/cmd/telebit/telebit.go @@ -251,9 +251,9 @@ func main() { fmt.Printf("[debug] Accepting API or WebSocket client %q\n", *apiHostname) listener.Feed(client) fmt.Printf("[debug] done with %q client\n", *apiHostname) - // TODO use a more correct non-error error? - // or perhaps (ok, error) or (handled, error)? - return io.EOF + // nil now means handler in-progress (go routine) + // EOF now means handler finished + return nil })) } for _, fwd := range forwards { @@ -369,7 +369,8 @@ func routeSubscribersAndClients(client net.Conn) error { labels := strings.Split(servername, ".") n := len(labels) if n < 3 { - return nil + // skip + return telebit.ErrNotHandled } for i := 1; i < n-1; i++ { wildname := "*." + strings.Join(labels[1:], ".") @@ -379,7 +380,7 @@ func routeSubscribersAndClients(client net.Conn) error { } // skip - return nil + return telebit.ErrNotHandled } // tryToServeName picks the server tunnel with the least connections, if any diff --git a/mplexer/connwrap.go b/mplexer/connwrap.go index 7e8beba..d3433ff 100644 --- a/mplexer/connwrap.go +++ b/mplexer/connwrap.go @@ -117,28 +117,21 @@ func (c *ConnWrap) Servername() string { } // this will get the servername - c.isTerminated() + _ = c.isEncrypted() return c.servername } -// isTerminated returns true if net.Conn is either a ConnWrap{ tls.Conn }, -// or a telebit.Conn with a non-encrypted `scheme` such as "tcp" or "http". -func (c *ConnWrap) isTerminated() bool { - // TODO look at SNI, may need context for Peek() timeout - /* - if nil != c.Plain { - return true - } - */ +// isEncrypted returns true if peeking at net.Conn reveals that it is TLS-encrypted +func (c *ConnWrap) isEncrypted() bool { if nil != c.encrypted { - return !*c.encrypted + return *c.encrypted } - // how to know how many bytes to read? really needs timeout + // TODO: how to allow / detect / handle protocols where the server hello happens first? c.SetDeadline(time.Now().Add(5 * time.Second)) n := 6 b, _ := c.Peek(n) - fmt.Println("Peek(n)", b, string(b)) + fmt.Println("[debug] Peek(n)", b, string(b)) defer c.SetDeadline(time.Time{}) var encrypted bool if len(b) >= n { @@ -155,30 +148,16 @@ func (c *ConnWrap) isTerminated() bool { b, err := c.Peek(n - 1 + length) if nil != err { c.encrypted = &encrypted - return !*c.encrypted + return *c.encrypted } c.servername, _ = sni.GetHostname(b) encrypted = true c.encrypted = &encrypted - return !*c.encrypted + return *c.encrypted } } c.encrypted = &encrypted - return !*c.encrypted - /* - if nil != err { - return true - } - - switch conn := c.Conn.(type) { - case *ConnWrap: - return conn.isTerminated() - case *Conn: - _, ok := encryptedSchemes[string(conn.relayTargetAddr.scheme)] - return !ok - } - return false - */ + return *c.encrypted } // LocalAddr returns the local network address. diff --git a/mplexer/listener.go b/mplexer/listener.go index 486f0a7..72ad11c 100644 --- a/mplexer/listener.go +++ b/mplexer/listener.go @@ -76,13 +76,16 @@ func Serve(listener net.Listener, mux Handler) error { } go func() { - err = mux.Serve(client) - if nil != err { + // nil means being handled + // non-nil means handled + // io.EOF means handled with success + if err := mux.Serve(client); nil != err { if io.EOF != err && io.ErrClosedPipe != err && !strings.Contains(err.Error(), errNetClosing) { fmt.Printf("client could not be served: %q\n", err.Error()) } + fmt.Println("[debug] closing original client", err) + client.Close() } - client.Close() }() } } diff --git a/mplexer/routemux.go b/mplexer/routemux.go index 955f99a..0b5f960 100644 --- a/mplexer/routemux.go +++ b/mplexer/routemux.go @@ -1,6 +1,7 @@ package telebit import ( + "errors" "fmt" "io" "net" @@ -18,6 +19,8 @@ type RouteMux struct { routes []meta } +var ErrNotHandled = errors.New("connection not handled") + type meta struct { addr string handler Handler @@ -72,11 +75,10 @@ func (m *RouteMux) Serve(client net.Conn) error { fmt.Println("Meta:", meta.addr, servername) if servername == meta.addr || "*" == meta.addr || port == meta.addr { //fmt.Println("[debug] test of route:", meta) - if err := meta.handler.Serve(wconn); nil != err { - // error should be EOF if successful + // Only keep trying handlers if ErrNotHandled was returned + if err := meta.handler.Serve(wconn); ErrNotHandled != err { return err } - // nil err means skipped } } @@ -121,9 +123,8 @@ func (m *RouteMux) HandleTLS(servername string, acme *ACME, handler Handler) err panic("HandleTLS is special in that it must receive &ConnWrap{ Conn: conn }") } - if wconn.isTerminated() { - // nil to skip - return nil + if !wconn.isEncrypted() { + return ErrNotHandled } //NewTerminator(acme, handler)(client) diff --git a/mplexer/telebit.go b/mplexer/telebit.go index aa3f5aa..c3cbd3c 100644 --- a/mplexer/telebit.go +++ b/mplexer/telebit.go @@ -253,7 +253,7 @@ func TerminateTLS(client net.Conn, acme *ACME) net.Conn { wconn := &ConnWrap{ Conn: client, } - wconn.isTerminated() + _ = wconn.isEncrypted() servername = wconn.Servername() scheme = wconn.Scheme() client = wconn