Remove address limitation for direct-tcpip for now

Now listen sockets should always be closed. I observed that on rare
occasions, listen sockets were not closed. My assumption is that a
client was sending a listen request, but also closed right after that,
and the closing code ran before the listen request code, so the last
listen request was processed after the listen-shutdown code was run; so
the last one was never freed.

At least, that is my current assumption.

README.rst

@@ -17,3 +17,18 @@ Same as OpenSSH authorized_keys format.
 The options field contains the ports that are allowed to be forwarded, colon separated::
 
     ports=3333:4444 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHPWEWu85yECrbmtL38wlFua3tBSqxTekCX/aU+dku+w COMMENTHERE
+
+Running as non-root user
+========================
+
+You should not run this program as root. Due to the way Go is implemented,
+setuid is non-trivial, so instead you need to set the CAP_NET_BIND_SERVICE
+capability on the resulting binary:
+
+    setcap 'cap_net_bind_service=+ep' go-sshd
+
+Init script
+===========
+
+There is an init script for gentoo/alpine (OpenRC) users. SSHD_LISTEN needs to
+be set in /etc/conf.d/go-sshd and the init-script goes in /etc/init.d/go-sshd

TODO

@@ -1,4 +1,8 @@
 * Make sure to not run this as root (setuid doesn't work well), so use NET capabilities
+* Allow limiting the hosts that one can connect to use direct-tcpip (right now
+  all hosts are allowed)
+* Allow lifting restrictions on what clients can bind on with forwarded-tcpip
+
 * Check assertions and TODOs.
 * Look if/where we want to set deadlines on open sockets
 * Go through all log.Println calls, and make sure they are unique(?) and

alpine/go-sshd

@@ -0,0 +1 @@
+gentoo/go-sshd

gentoo/go-sshd

@@ -0,0 +1,39 @@
+#!/sbin/openrc-run
+# Copyright 1999-2017 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+description="Go Secure Shell server"
+description_reload="Reload configuration"
+
+extra_started_commands="reload"
+
+: ${SSHD_PIDFILE:=/run/${SVCNAME}.pid}
+: ${SSHD_BINARY:=/usr/local/bin/go-sshd}
+: ${SSHD_LISTEN:="-listenaddr :1 -listenport 8822"}
+: ${SSHD_LOG:="/var/log/mcs/${SVCNAME}"}
+
+start() {
+        ebegin "Starting ${SVCNAME}"
+        start-stop-daemon --start --exec "${SSHD_BINARY}" \
+            --make-pidfile --pidfile "${SSHD_PIDFILE}" \
+            --background \
+            --user ${SSHD_USER} --group ${SSHD_GROUP} \
+            --stderr "${SSHD_LOG}" \
+            -- ${SSHD_OPTS} ${SSHD_LISTEN} -hostkey /etc/go-sshd/tunnel \
+            -authorisedkeys /etc/go-sshd/authorized_keys
+        eend $?
+}
+
+stop() {
+        ebegin "Stopping ${SVCNAME}"
+        start-stop-daemon --stop --exec "${SSHD_BINARY}" \
+            --pidfile "${SSHD_PIDFILE}" --quiet
+        eend $?
+}
+
+reload() {
+        ebegin "Reloading ${SVCNAME}"
+        start-stop-daemon --signal USR1 \
+            --exec "${SSHD_BINARY}" --pidfile "${SSHD_PIDFILE}"
+        eend $?
+}

sshd.go

@@ -39,6 +39,8 @@ type sshClient struct {
 	Listeners          map[string]net.Listener
 	AllowedLocalPorts  []uint32
 	AllowedRemotePorts []uint32
+	Stopping           bool
+	ListenMutex        sync.Mutex
 }
 
 type bindInfo struct {
@@ -87,9 +89,9 @@ func main() {
 
 	config := &ssh.ServerConfig{
 		PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) {
+			authmutex.Lock()
+			defer authmutex.Unlock()
 			if deviceinfo, found := authorisedKeys[string(key.Marshal())]; found {
-				authmutex.Lock()
-				defer authmutex.Unlock()
 				return &ssh.Permissions{
 					CriticalOptions: map[string]string{"name": deviceinfo.Comment,
 						"localports":  deviceinfo.LocalPorts,
@@ -123,16 +125,16 @@ func main() {
 		go func() {
 			sshConn, chans, reqs, err := ssh.NewServerConn(tcpConn, config)
 			if err != nil {
-				log.Printf("Failed to handshake (%s)", err)
+				log.Printf("Failed to handshake: %s (rip: %v)", err, tcpConn.RemoteAddr())
 				return
 			}
 
-			client := sshClient{sshConn.Permissions.CriticalOptions["name"], sshConn, make(map[string]net.Listener), nil, nil}
+			client := sshClient{sshConn.Permissions.CriticalOptions["name"], sshConn, make(map[string]net.Listener), nil, nil, false, sync.Mutex{}}
 			allowedLocalPorts := sshConn.Permissions.CriticalOptions["localports"]
 			allowedRemotePorts := sshConn.Permissions.CriticalOptions["remoteports"]
 
 			if *verbose {
-				log.Printf("Connection from \"%s\", %s (%s). Allowed local ports: %s remote ports: %s", client.Name, sshConn.RemoteAddr(), sshConn.ClientVersion(), allowedLocalPorts, allowedRemotePorts)
+				log.Printf("[%s] Connection from %s (%s). Allowed local ports: %s remote ports: %s", client.Name, sshConn.RemoteAddr(), sshConn.ClientVersion(), allowedLocalPorts, allowedRemotePorts)
 			}
 
 			// Parsing a second time should not error, so we can ignore the error
@@ -142,17 +144,19 @@ func main() {
 
 			go func() {
 				err := client.Conn.Wait()
+				client.ListenMutex.Lock()
+				client.Stopping = true
+
 				if *verbose {
-					log.Printf("SSH connection closed for client %s: %s", client.Name, err)
+					log.Printf("[%s] SSH connection closed: %s", client.Name, err)
 				}
-				// TODO: Make this safe? Is it impossible for cancel code to be
-				// running at this point?
 				for bind, listener := range client.Listeners {
 					if *verbose {
-						log.Printf("Closing listener bound to %s", bind)
+						log.Printf("[%s] Closing listener bound to %s", client.Name, bind)
 					}
 					listener.Close()
 				}
+				client.ListenMutex.Unlock()
 			}()
 
 			go handleRequest(&client, reqs)
@@ -170,14 +174,14 @@ func handleChannels(client *sshClient, chans <-chan ssh.NewChannel) {
 
 func handleChannel(client *sshClient, newChannel ssh.NewChannel) {
 	if *verbose {
-		log.Println("Channel type:", newChannel.ChannelType())
+		log.Printf("[%s] Channel type: %v", client.Name, newChannel.ChannelType())
 	}
 	if t := newChannel.ChannelType(); t == "direct-tcpip" {
 		handleDirect(client, newChannel)
 		return
 	}
 
-	newChannel.Reject(ssh.Prohibited, fmt.Sprintf("Only \"direct-tcpip\" is accepted"))
+	newChannel.Reject(ssh.Prohibited, fmt.Sprintf("Only \"direct-tcpip\", \"forwarded-tcpip\" and \"cancel-tcpip-forward\" are accepted"))
 	/*
 		// TODO: USE THIS ONLY FOR USING SSH ESCAPE SEQUENCES
 		c, _, err := newChannel.Accept()
@@ -196,39 +200,42 @@ func handleChannel(client *sshClient, newChannel ssh.NewChannel) {
 func handleDirect(client *sshClient, newChannel ssh.NewChannel) {
 	var payload directTCPPayload
 	if err := ssh.Unmarshal(newChannel.ExtraData(), &payload); err != nil {
-		log.Printf("Could not unmarshal extra data: %s\n", err)
+		log.Printf("[%s] Could not unmarshal extra data: %s", client.Name, err)
 
 		newChannel.Reject(ssh.Prohibited, fmt.Sprintf("Bad payload"))
 		return
 	}
 
-	if payload.Addr != "localhost" {
-		log.Printf("Tried to connect to prohibited host: %s", payload.Addr)
-		newChannel.Reject(ssh.Prohibited, fmt.Sprintf("Bad addr"))
-		return
-	}
+	/*
+		// XXX: Is this sensible?
+		if payload.Addr != "localhost" && payload.Addr != "::1" && payload.Addr != "127.0.0.1" {
+			log.Printf("[%s] Tried to connect to prohibited host: %s", client.Name, payload.Addr)
+			newChannel.Reject(ssh.Prohibited, fmt.Sprintf("Bad addr"))
+			return
+		}
+	*/
 
 	if !portPermitted(payload.Port, client.AllowedLocalPorts) {
 		newChannel.Reject(ssh.Prohibited, fmt.Sprintf("Bad port"))
-		log.Printf("Tried to connect to prohibited port: %d", payload.Port)
+		log.Printf("[%s] Tried to connect to prohibited port: %d", client.Name, payload.Port)
 		return
 	}
 
 	connection, requests, err := newChannel.Accept()
 	if err != nil {
-		log.Printf("Could not accept channel (%s)", err)
+		log.Printf("[%s] Could not accept channel (%s)", client.Name, err)
 		return
 	}
 	go ssh.DiscardRequests(requests)
 
-	addr := fmt.Sprintf("%s:%d", payload.Addr, payload.Port)
+	addr := fmt.Sprintf("[%s]:%d", payload.Addr, payload.Port)
 	if *verbose {
-		log.Println("Dialing:", addr)
+		log.Printf("[%s] Dialing: %s", client.Name, addr)
 	}
 
 	rconn, err := net.Dial("tcp", addr)
 	if err != nil {
-		log.Printf("Could not dial remote (%s)", err)
+		log.Printf("[%s] Could not dial remote (%s)", client.Name, err)
 		connection.Close()
 		return
 	}
@@ -239,24 +246,24 @@ func handleDirect(client *sshClient, newChannel ssh.NewChannel) {
 func handleTcpIpForward(client *sshClient, req *ssh.Request) (net.Listener, *bindInfo, error) {
 	var payload tcpIpForwardPayload
 	if err := ssh.Unmarshal(req.Payload, &payload); err != nil {
-		log.Println("Unable to unmarshal payload")
+		log.Printf("[%s] Unable to unmarshal payload", client.Name)
 		req.Reply(false, []byte{})
 		return nil, nil, fmt.Errorf("Unable to parse payload")
 	}
 
 	if *verbose {
-		log.Println("Request:", req.Type, req.WantReply, payload)
-		log.Printf("Request to listen on %s:%d", payload.Addr, payload.Port)
+		log.Printf("[%s] Request: %s %v %v", client.Name, req.Type, req.WantReply, payload)
+		log.Printf("[%s] Request to listen on %s:%d", client.Name, payload.Addr, payload.Port)
 	}
 
 	if payload.Addr != "localhost" && payload.Addr != "" {
-		log.Printf("Payload address is not \"localhost\" or empty")
+		log.Printf("[%s] Payload address is not \"localhost\" or empty: %s", client.Name, payload.Addr)
 		req.Reply(false, []byte{})
 		return nil, nil, fmt.Errorf("Address is not permitted")
 	}
 
 	if !portPermitted(payload.Port, client.AllowedRemotePorts) {
-		log.Printf("Port is not permitted.")
+		log.Printf("[%s] Port is not permitted: %d", client.Name, payload.Port)
 		req.Reply(false, []byte{})
 		return nil, nil, fmt.Errorf("Port is not permitted")
 	}
@@ -267,7 +274,7 @@ func handleTcpIpForward(client *sshClient, req *ssh.Request) (net.Listener, *bin
 	bind := fmt.Sprintf("%s:%d", laddr, lport)
 	ln, err := net.Listen("tcp", bind)
 	if err != nil {
-		log.Printf("Listen failed for %s", bind)
+		log.Printf("[%s] Listen failed for %s", client.Name, bind)
 		req.Reply(false, []byte{})
 		return nil, nil, err
 	}
@@ -287,11 +294,11 @@ func handleListener(client *sshClient, bindinfo *bindInfo, listener net.Listener
 		if err != nil {
 			neterr := err.(net.Error)
 			if neterr.Timeout() {
-				log.Println("Accept failed with timeout:", err)
+				log.Printf("[%s] Accept failed with timeout: %s", client.Name, err)
 				continue
 			}
 			if neterr.Temporary() {
-				log.Println("Accept failed with temporary:", err)
+				log.Printf("[%s] Accept failed with temporary: %s", client.Name, err)
 				continue
 			}
 
@@ -313,13 +320,12 @@ func handleForwardTcpIp(client *sshClient, bindinfo *bindInfo, lconn net.Conn) {
 	// Open channel with client
 	c, requests, err := client.Conn.OpenChannel("forwarded-tcpip", mpayload)
 	if err != nil {
-		log.Printf("Error: %s", err)
-		log.Println("Unable to get channel. Hanging up requesting party!")
+		log.Printf("[%s] Unable to get channel: %s. Hanging up requesting party!", client.Name, err)
 		lconn.Close()
 		return
 	}
 	if *verbose {
-		log.Printf("Channel opened for client %s", client.Name)
+		log.Printf("[%s] Channel opened for client", client.Name)
 	}
 	go ssh.DiscardRequests(requests)
 
@@ -328,11 +334,11 @@ func handleForwardTcpIp(client *sshClient, bindinfo *bindInfo, lconn net.Conn) {
 
 func handleTcpIPForwardCancel(client *sshClient, req *ssh.Request) {
 	if *verbose {
-		log.Println("Cancel called by client", client)
+		log.Printf("[%s] \"cancel-tcpip-forward\" called by client", client.Name)
 	}
 	var payload tcpIpForwardCancelPayload
 	if err := ssh.Unmarshal(req.Payload, &payload); err != nil {
-		log.Println("Unable to unmarshal cancel payload")
+		log.Printf("[%s] Unable to unmarshal cancel payload", client.Name)
 		req.Reply(false, []byte{})
 	}
 
@@ -354,7 +360,7 @@ func serve(cssh ssh.Channel, conn net.Conn, client *sshClient) {
 		cssh.Close()
 		conn.Close()
 		if *verbose {
-			log.Printf("Channel closed for client: %s", client.Name)
+			log.Printf("[%s] Channel closed.", client.Name)
 		}
 	}
 
@@ -435,8 +441,12 @@ func registerReloadSignal() {
 
 	go func() {
 		for sig := range c {
-			log.Printf("Received signal: \"%s\". Reloading authorised keys.", sig.String())
-			loadAuthorisedKeys(*authorisedkeys)
+			if sig == syscall.SIGUSR1 {
+				log.Printf("Received signal: SIGUSR1. Reloading authorised keys.")
+				loadAuthorisedKeys(*authorisedkeys)
+			} else {
+				log.Printf("Received unexpected signal: \"%s\".", sig.String())
+			}
 		}
 
 	}()
@@ -445,21 +455,34 @@ func registerReloadSignal() {
 func handleRequest(client *sshClient, reqs <-chan *ssh.Request) {
 	for req := range reqs {
 		if *verbose {
-			log.Println("Out of band request:", req.Type, req.WantReply)
+			log.Printf("[%s] Out of band request: %v %v", client.Name, req.Type, req.WantReply)
 		}
 
 		// RFC4254: 7.1 for forwarding
 		if req.Type == "tcpip-forward" {
+			client.ListenMutex.Lock()
+			/* If we are closing, do not set up a new listener */
+			if client.Stopping {
+				client.ListenMutex.Unlock()
+				req.Reply(false, []byte{})
+				continue
+			}
+
 			listener, bindinfo, err := handleTcpIpForward(client, req)
 			if err != nil {
+				client.ListenMutex.Unlock()
 				continue
 			}
 
 			client.Listeners[bindinfo.Bound] = listener
+			client.ListenMutex.Unlock()
+
 			go handleListener(client, bindinfo, listener)
 			continue
 		} else if req.Type == "cancel-tcpip-forward" {
+			client.ListenMutex.Lock()
 			handleTcpIPForwardCancel(client, req)
+			client.ListenMutex.Unlock()
 			continue
 		} else {
 			// Discard everything else