Misc. fixes to end-of-session conn handling. Outstanding bug w/client chaff enabled & truncated client data

hkexnet/hkexnet.go

@@ -439,7 +439,12 @@ func (hc Conn) Read(b []byte) (n int, err error) {
 			log.Printf("[TermSize pkt: rows %v cols %v]\n", hc.Rows, hc.Cols)
 			hc.WinCh <- WinSize{hc.Rows, hc.Cols}
 		} else if ctrlStatOp == CSOExitStatus {
-			*hc.closeStat = uint8(payloadBytes[0])
+			if len(payloadBytes) > 0 {
+				*hc.closeStat = uint8(payloadBytes[0])
+			} else {
+				log.Println("[truncated payload, cannot determine CSOExitStatus]")
+				*hc.closeStat = 99
+			}
 		} else {
 			hc.dBuf.Write(payloadBytes)
 			//log.Printf("hc.dBuf: %s\n", hex.Dump(hc.dBuf.Bytes()))
@@ -450,10 +455,14 @@ func (hc Conn) Read(b []byte) (n int, err error) {
 		hTmp := hc.rm.Sum(nil)[0:4]
 		log.Printf("<%04x) HMAC:(i)%s (c)%02x\r\n", decryptN, hex.EncodeToString([]byte(hmacIn[0:])), hTmp)
 
-		// Log alert if hmac didn't match, corrupted channel
-		if !bytes.Equal(hTmp, []byte(hmacIn[0:])) /*|| hmacIn[0] > 0xf8*/ {
-			fmt.Println("** ALERT - detected HMAC mismatch, possible channel tampering **")
-			_, _ = hc.c.Write([]byte{CSOHmacInvalid})
+		if *hc.closeStat == 99 {
+			log.Println("[cannot verify HMAC]")
+		} else {
+			// Log alert if hmac didn't match, corrupted channel
+			if !bytes.Equal(hTmp, []byte(hmacIn[0:])) /*|| hmacIn[0] > 0xf8*/ {
+				fmt.Println("** ALERT - detected HMAC mismatch, possible channel tampering **")
+				_, _ = hc.c.Write([]byte{CSOHmacInvalid})
+			}
 		}
 	}
 

hkexsh/hkexsh.go

@@ -30,7 +30,7 @@ type cmdSpec struct {
 	who        []byte
 	cmd        []byte
 	authCookie []byte
-	status     int // though UNIX shell exit status is uint8, os.Exit() wants int
+	status     int // UNIX exit status is uint8, but os.Exit() wants int
 }
 
 var (
@@ -86,13 +86,13 @@ func parseNonSwitchArgs(a []string, dp string) (user, host, port, path string, i
 				fancyPort = dp
 			}
 
-			if fancyPath == "" {
-				fancyPath = "."
-			}
+			//if fancyPath == "" {
+			//	fancyPath = "."
+			//}
 
 			if i == len(a)-1 {
 				isDest = true
-				fmt.Println("isDest")
+				fmt.Println("remote path isDest")
 			}
 			fmt.Println("fancyArgs: user:", fancyUser, "host:", fancyHost, "port:", fancyPort, "path:", fancyPath)
 		} else {
@@ -118,6 +118,7 @@ func main() {
 	version := "0.1pre (NO WARRANTY)"
 	var vopt bool
 	var dbg bool
+	var shellMode bool // if true act as shell, else file copier
 	var cAlg string
 	var hAlg string
 	var server string
@@ -154,6 +155,7 @@ func main() {
 		// hkexsh accepts a command (-x) but not
 		// a srcpath (-r) or dstpath (-t)
 		flag.StringVar(&cmdStr, "x", "", "command to run (default empty - interactive shell)")
+		shellMode = true
 	} // else {
 	//// hkexcp accepts srcpath (-r) and dstpath (-t), but not
 	//// a command (-x)
@@ -162,19 +164,19 @@ func main() {
 	//}
 	flag.Parse()
 
-	fancyUser, fancyHost, fancyPort, fancyPath, pathIsDest, otherArgs :=
+	tmpUser, tmpHost, tmpPort, tmpPath, pathIsDest, otherArgs :=
 		parseNonSwitchArgs(flag.Args(), defPort /* defPort */)
 	fmt.Println("otherArgs:", otherArgs)
-	//fmt.Println("fancyHost:", fancyHost)
-	fmt.Println("fancyPath:", fancyPath)
-	if fancyUser != "" {
-		altUser = fancyUser
+	//fmt.Println("tmpHost:", tmpHost)
+	//fmt.Println("tmpPath:", tmpPath)
+	if tmpUser != "" {
+		altUser = tmpUser
 	}
-	if fancyHost != "" {
-		server = fancyHost + ":" + fancyPort
-		//fmt.Println("fancyHost sets server to", server)
+	if tmpHost != "" {
+		server = tmpHost + ":" + tmpPort
+		//fmt.Println("tmpHost sets server to", server)
 	}
-	if fancyPath != "" {
+	if tmpPath != "" {
 		// -if pathIsSrc && len(otherArgs) > 1 ERROR
 		// -else flatten otherArgs into space-delim list => copySrc
 		if pathIsDest {
@@ -183,17 +185,18 @@ func main() {
 				copySrc = append(copySrc, v...)
 			}
 			fmt.Println(">> copySrc:", string(copySrc))
-			copyDst = fancyPath
+			copyDst = tmpPath
 		} else {
 			if len(otherArgs) > 1 {
 				log.Fatal("ERROR: cannot specify more than one dest path for copy")
 			}
-			copySrc = []byte(fancyPath)
+			copySrc = []byte(tmpPath)
 		}
 	}
 
-	//fmt.Println("server finally is:", server)
+	// Do some more option consistency checks
 
+	//fmt.Println("server finally is:", server)
 	if flag.NFlag() == 0 && server == "" {
 		flag.Usage()
 		os.Exit(0)
@@ -208,12 +211,33 @@ func main() {
 		log.Fatal("incompatible options -- either cmd (-x) or copy ops but not both")
 	}
 
+	//-------------------------------------------------------------------
+	// Here we have parsed all options and can now carry out
+	// either the shell session or copy operation.
+	_ = shellMode
+
 	if dbg {
 		log.SetOutput(os.Stdout)
 	} else {
 		log.SetOutput(ioutil.Discard)
 	}
 
+	// We must make the decision about interactivity before Dial()
+	// as it affects chaffing behaviour. 20180805
+	if len(cmdStr) == 0 {
+		op = []byte{'s'}
+		isInteractive = true
+	} else {
+		op = []byte{'c'}
+		// non-interactive cmds may complete quickly, so chaff earlier/faster
+		// to help ensure there's some cover to the brief traffic.
+		// (ignoring cmdline values)
+		//!DEBUG
+		//chaffEnabled = false
+		chaffFreqMin = 2
+		chaffFreqMax = 10
+	}
+
 	conn, err := hkexnet.Dial("tcp", server, cAlg, hAlg)
 	if err != nil {
 		fmt.Println("Err!")
@@ -226,14 +250,16 @@ func main() {
 	// TODO: send flag to server side indicating this
 	//  affects shell command used
 	var oldState *hkexsh.State
-	if isatty.IsTerminal(os.Stdin.Fd()) {
-		oldState, err = hkexsh.MakeRaw(int(os.Stdin.Fd()))
-		if err != nil {
-			panic(err)
+	if shellMode {
+		if isatty.IsTerminal(os.Stdin.Fd()) {
+			oldState, err = hkexsh.MakeRaw(int(os.Stdin.Fd()))
+			if err != nil {
+				panic(err)
+			}
+			defer func() { _ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) }() // Best effort.
+		} else {
+			log.Println("NOT A TTY")
 		}
-		defer func() { _ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) }() // Best effort.
-	} else {
-		log.Println("NOT A TTY")
 	}
 
 	var uname string
@@ -244,18 +270,6 @@ func main() {
 		uname = altUser
 	}
 
-	if len(cmdStr) == 0 {
-		op = []byte{'s'}
-		isInteractive = true
-	} else {
-		op = []byte{'c'}
-		// non-interactive cmds may complete quickly, so chaff earlier/faster
-		// to help ensure there's some cover to the brief traffic.
-		// (ignoring cmdline values)
-		chaffFreqMin = 2
-		chaffFreqMax = 10
-	}
-
 	if len(authCookie) == 0 {
 		fmt.Printf("Gimme cookie:")
 		ab, err := hkexsh.ReadPassword(int(os.Stdin.Fd()))
@@ -288,32 +302,25 @@ func main() {
 	conn.SetupChaff(chaffFreqMin, chaffFreqMax, chaffBytesMax) // enable client->server chaffing
 	if chaffEnabled {
 		conn.EnableChaff()
+		//defer conn.DisableChaff()
+		//defer conn.ShutdownChaff()
 	}
-	defer conn.DisableChaff()
-	defer conn.ShutdownChaff()
 
 	//client reader (from server) goroutine
+	//Read remote end's stdout
 	wg.Add(1)
 	go func() {
 		// By deferring a call to wg.Done(),
 		// each goroutine guarantees that it marks
 		// its direction's stream as finished.
-		//
-		// Whichever direction's goroutine finishes first
-		// will call wg.Done() once more, explicitly, to
-		// hang up on the other side, so that this client
-		// exits immediately on an EOF from either side.
-		defer wg.Done()
 
-		// io.Copy() expects EOF so this will
+		// io.Copy() expects EOF so normally this will
 		// exit with inerr == nil
 		_, inerr := io.Copy(os.Stdout, conn)
 		if inerr != nil {
-			if inerr.Error() != "EOF" {
-				fmt.Println(inerr)
-				_ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort.
-				os.Exit(1)
-			}
+			fmt.Println(inerr)
+			_ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort.
+			os.Exit(1)
 		}
 
 		rec.status = int(conn.GetStatus())
@@ -322,41 +329,43 @@ func main() {
 		if isInteractive {
 			log.Println("[* Got EOF *]")
 			_ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort.
-			wg.Done()
-			//os.Exit(rec.status)
 		}
+		wg.Done()
 	}()
 
+	// Only look for data from stdin to send to remote end
+	// for interactive sessions.
 	if isInteractive {
 		handleTermResizes(conn)
 
 		// client writer (to server) goroutine
+		// Write local stdin to remote end
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-
 			// Copy() expects EOF so this will
 			// exit with outerr == nil
 			//!_, outerr := io.Copy(conn, os.Stdin)
 			_, outerr := func(conn *hkexnet.Conn, r io.Reader) (w int64, e error) {
-				return io.Copy(conn, r)
+				w, e = io.Copy(conn, r)
+				return w, e
 			}(conn, os.Stdin)
 
 			if outerr != nil {
 				log.Println(outerr)
-				if outerr.Error() != "EOF" {
-					fmt.Println(outerr)
-					_ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort.
-					os.Exit(255)
-				}
+				fmt.Println(outerr)
+				_ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort.
+				os.Exit(255)
 			}
 			log.Println("[Sent EOF]")
-			wg.Done() // client hung up, close WaitGroup to exit client
+			wg.Done()
 		}()
 	}
 
 	// Wait until both stdin and stdout goroutines finish
 	wg.Wait()
+	conn.DisableChaff()
+	conn.ShutdownChaff()
 
 	_ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort.
 	os.Exit(rec.status)

hkexshd/hkexshd.go

@@ -18,6 +18,7 @@ import (
 	"os/user"
 	"runtime"
 	"strings"
+	"sync"
 	"syscall"
 
 	"blitter.com/go/goutmp"
@@ -83,6 +84,7 @@ func runCmdAs(who string, cmd string, conn hkex.Conn) (err error) {
 //
 // Uses ptys to support commands which expect a terminal.
 func runShellAs(who string, cmd string, interactive bool, conn hkexnet.Conn, chaffing bool) (err error, exitStatus int) {
+	var wg sync.WaitGroup
 	u, _ := user.Lookup(who)
 	var uid, gid uint32
 	fmt.Sscanf(u.Uid, "%d", &uid)
@@ -135,15 +137,16 @@ func runShellAs(who string, cmd string, interactive bool, conn hkexnet.Conn, cha
 				log.Printf("[Setting term size to: %v %v]\n", sz.Rows, sz.Cols)
 				pty.Setsize(ptmx, &pty.Winsize{Rows: sz.Rows, Cols: sz.Cols})
 			}
+			fmt.Println("*** WinCh goroutine done ***")
 		}()
 
 		// Copy stdin to the pty.. (bgnd goroutine)
 		go func() {
 			_, e := io.Copy(ptmx, conn)
 			if e != nil {
-				log.Printf("** std->pty ended **\n")
-				return
+				log.Println("** stdin->pty ended **:", e.Error())
 			}
+			fmt.Println("*** stdin->pty goroutine done ***")
 		}()
 
 		if chaffing {
@@ -153,17 +156,26 @@ func runShellAs(who string, cmd string, interactive bool, conn hkexnet.Conn, cha
 		defer conn.ShutdownChaff()
 
 		// ..and the pty to stdout.
+		// This may take some time exceeding that of the
+		// actual command's lifetime, so the c.Wait() below
+		// must synchronize with the completion of this goroutine
+		// to ensure all stdout data gets to the client before
+		// connection is closed.
+		wg.Add(1)
 		go func() {
+			defer wg.Done()
 			_, e := io.Copy(conn, ptmx)
 			if e != nil {
-				log.Printf("** pty->stdout ended **\n")
-				return
+				log.Println("** pty->stdout ended **:", e.Error())
+				//wg.Done() //!return
 			}
 			// The above io.Copy() will exit when the command attached
 			// to the pty exits
+			fmt.Println("*** pty->stdout goroutine done ***")
 		}()
 
 		if err := c.Wait(); err != nil {
+				fmt.Println("*** c.Wait() done ***")
 			if exiterr, ok := err.(*exec.ExitError); ok {
 				// The program has exited with an exit code != 0
 
@@ -177,6 +189,9 @@ func runShellAs(who string, cmd string, interactive bool, conn hkexnet.Conn, cha
 				}
 			}
 		}
+		wg.Wait() // Wait on pty->stdout completion to client
+		//conn.DisableChaff()
+		//conn.ShutdownChaff()
 	}
 	return
 }