xs/ lint cleanups

.golangci.yml

@@ -25,6 +25,8 @@ linters-settings:
       #- style
       #- opinionated
     disabled-checks:
+      - commentFormatting
+      - commentedOutCode
       - dupImport # https://github.com/go-critic/go-critic/issues/845
       - ifElseChain
       - octalLiteral

xs/termsize_unix.go

@@ -31,7 +31,7 @@ func handleTermResizes(conn *xsnet.Conn) {
 				log.Println(err)
 			}
 			termSzPacket := fmt.Sprintf("%d %d", rows, cols)
-			conn.WritePacket([]byte(termSzPacket), xsnet.CSOTermSize)
+			conn.WritePacket([]byte(termSzPacket), xsnet.CSOTermSize) //nolint:errcheck
 		}
 	}()
 	ch <- syscall.SIGWINCH // Initial resize.

xs/xs.go

@@ -14,7 +14,6 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"log"
 	"math/rand"
 	"os"
@@ -57,6 +56,18 @@ var (
 
 ////////////////////////////////////////////////////
 
+const (
+	CmdExitedEarly               = 2
+	XSNetDialFailed              = 3
+	ErrReadingAuthReply          = 253
+	ServerRejectedSecureProposal = 254
+	GeneralProtocolErr           = 255
+)
+
+const (
+	DeadCharPrefix = 0x1d
+)
+
 // Praise Bob. Do not remove, lest ye lose Slack.
 const bob = string("\r\n\r\n" +
 	"@@@@@@@^^~~~~~~~~~~~~~~~~~~~~^@@@@@@@@@\r\n" +
@@ -154,7 +165,7 @@ func copyBuffer(dst io.Writer, src io.Reader, buf []byte) (written int64, err er
 		if rt, ok := dst.(io.ReaderFrom); ok {
 			return rt.ReadFrom(src)
 		}
-	*/
+	*/ //nolint:gocritic,nolintlint
 	if buf == nil {
 		size := 32 * 1024
 		if l, ok := src.(*io.LimitedReader); ok && int64(size) > l.N {
@@ -175,8 +186,8 @@ func copyBuffer(dst io.Writer, src io.Reader, buf []byte) (written int64, err er
 			// A repeat of 4 keys (conveniently 'dead' chars for most
 			// interactive shells; here CTRL-]) shall introduce
 			// some special responses or actions on the client side.
-			if seqPos < 4 {
+			if seqPos < 4 { //nolint:gomnd
-				if buf[0] == 0x1d {
+				if buf[0] == DeadCharPrefix {
 					seqPos++
 				}
 			} else {
@@ -223,7 +234,7 @@ func GetSize() (cols, rows int, err error) {
 
 	if err != nil {
 		log.Println(err)
-		cols, rows = 80, 24 //failsafe
+		cols, rows = 80, 24 // failsafe
 	} else {
 		n, err := fmt.Sscanf(string(out), "%d %d\n", &rows, &cols)
 		if n < 2 ||
@@ -257,7 +268,7 @@ func buildCmdRemoteToLocal(copyQuiet bool, copyLimitBPS uint, destPath string) (
 	} else {
 		// TODO: Query remote side for total file/dir size
 		bandwidthInBytesPerSec := " -L " + fmt.Sprintf("%d ", copyLimitBPS)
-		displayOpts := " -pre "
+		displayOpts := " -pre " //nolint:goconst
 		cmd = xs.GetTool("bash")
 		args = []string{"-c", "pv " + displayOpts + bandwidthInBytesPerSec + "| tar -xz -C " + destPath}
 	}
@@ -305,7 +316,7 @@ func buildCmdLocalToRemote(copyQuiet bool, copyLimitBPS uint, files string) (cap
 	} else {
 		captureStderr = copyQuiet
 		bandwidthInBytesPerSec := " -L " + fmt.Sprintf("%d", copyLimitBPS)
-		displayOpts := " -pre "
+		displayOpts := " -pre " //nolint:goconst,nolintlint
 		cmd = xs.GetTool("bash")
 		args = []string{"-c", xs.GetTool("tar") + " -cz -f /dev/stdout "}
 		files = strings.TrimSpace(files)
@@ -355,9 +366,9 @@ func doCopyMode(conn *xsnet.Conn, remoteDest bool, files string, copyQuiet bool,
 
 		var c *exec.Cmd
 
-		//os.Clearenv()
+		// os.Clearenv()
-		//os.Setenv("HOME", u.HomeDir)
+		// os.Setenv("HOME", u.HomeDir)
-		//os.Setenv("TERM", "vt102") // TODO: server or client option?
+		// os.Setenv("TERM", "vt102") // TODO: server or client option?
 
 		captureStderr, cmdName, cmdArgs := buildCmdLocalToRemote(copyQuiet, copyLimitBPS, strings.TrimSpace(files))
 		c = exec.Command(cmdName, cmdArgs...) // #nosec
@@ -390,7 +401,7 @@ func doCopyMode(conn *xsnet.Conn, remoteDest bool, files string, copyQuiet bool,
 		if err != nil {
 			fmt.Println("cmd exited immediately. Cannot get cmd.Wait().ExitStatus()")
 			err = errors.New("cmd exited prematurely")
-			exitStatus = uint32(2)
+			exitStatus = uint32(CmdExitedEarly)
 		} else {
 			if err = c.Wait(); err != nil {
 				if exiterr, ok := err.(*exec.ExitError); ok {
@@ -410,7 +421,7 @@ func doCopyMode(conn *xsnet.Conn, remoteDest bool, files string, copyQuiet bool,
 			}
 			// send CSOExitStatus to inform remote (server) end cp is done
 			log.Println("Sending local exitStatus:", exitStatus)
-			r := make([]byte, 4)
+			r := make([]byte, 4) //nolint:gomnd
 			binary.BigEndian.PutUint32(r, exitStatus)
 			_, we := conn.WritePacket(r, xsnet.CSOExitStatus)
 			if we != nil {
@@ -418,7 +429,7 @@ func doCopyMode(conn *xsnet.Conn, remoteDest bool, files string, copyQuiet bool,
 			}
 
 			// Do a final read for remote's exit status
-			s := make([]byte, 4)
+			s := make([]byte, 4) //nolint:gomnd
 			_, remErr := conn.Read(s)
 			if remErr != io.EOF &&
 				!strings.Contains(remErr.Error(), "use of closed network") &&
@@ -478,8 +489,8 @@ func doCopyMode(conn *xsnet.Conn, remoteDest bool, files string, copyQuiet bool,
 // doShellMode begins an xs shell session (one-shot command or
 // interactive).
 func doShellMode(isInteractive bool, conn *xsnet.Conn, oldState *xs.State, rec *xs.Session) {
-	//client reader (from server) goroutine
+	// Client reader (from server) goroutine
-	//Read remote end's stdout
+	// Read remote end's stdout
 
 	wg.Add(1)
 	// #gv:s/label=\"doShellMode\$1\"/label=\"shellRemoteToStdin\"/
@@ -602,7 +613,7 @@ func parseNonSwitchArgs(a []string) (user, host, path string, isDest bool, other
 		if strings.Contains(arg, ":") || strings.Contains(arg, "@") {
 			fancyArg := strings.Split(flag.Arg(i), "@")
 			var fancyHostPath []string
-			if len(fancyArg) < 2 {
+			if len(fancyArg) < 2 { //nolint:gomnd
 				//TODO: no user specified, use current
 				fancyUser = "[default:getUser]"
 				fancyHostPath = strings.Split(fancyArg[0], ":")
@@ -628,8 +639,8 @@ func parseNonSwitchArgs(a []string) (user, host, path string, isDest bool, other
 	return fancyUser, fancyHost, fancyPath, isDest, otherArgs
 }
 
-func launchTuns(conn *xsnet.Conn, remoteHost string, tuns string) {
+func launchTuns(conn *xsnet.Conn /*remoteHost string,*/, tuns string) {
-	/*remAddrs, _ := net.LookupHost(remoteHost)*/
+	/*remAddrs, _ := net.LookupHost(remoteHost)*/ //nolint:gocritic,nolintlint
 
 	if tuns == "" {
 		return
@@ -678,12 +689,12 @@ func main() { //nolint: funlen, gocyclo
 	var (
 		isInteractive bool
 		vopt          bool
-		gopt          bool //login via password, asking server to generate authToken
+		gopt          bool // true: login via password, asking server to generate authToken
 		dbg           bool
-		shellMode     bool   // if true act as shell, else file copier
+		shellMode     bool // true: act as shell, false: file copier
-		cipherAlg     string //cipher alg
+		cipherAlg     string
-		hmacAlg       string //hmac alg
+		hmacAlg       string
-		kexAlg        string //KEX/KEM alg
+		kexAlg        string
 		server        string
 		port          uint
 		cmdStr        string
@@ -703,7 +714,7 @@ func main() { //nolint: funlen, gocyclo
 		op []byte
 	)
 
-	//=== Common (xs and xc) option parsing
+	// === Common (xs and xc) option parsing
 
 	flag.BoolVar(&vopt, "v", false, "show version")
 	flag.BoolVar(&dbg, "d", false, "debug logging")
@@ -731,18 +742,18 @@ func main() { //nolint: funlen, gocyclo
       KEX_FRODOKEM_1344SHAKE
       KEX_FRODOKEM_976AES
       KEX_FRODOKEM_976SHAKE`)
-	flag.StringVar(&kcpMode, "K", "unused", "KCP `alg`, one of [KCP_NONE | KCP_AES | KCP_BLOWFISH | KCP_CAST5 | KCP_SM4 | KCP_SALSA20 | KCP_SIMPLEXOR | KCP_TEA | KCP_3DES | KCP_TWOFISH | KCP_XTEA] to use KCP (github.com/xtaci/kcp-go) reliable UDP instead of TCP")
+	flag.StringVar(&kcpMode, "K", "unused", "KCP `alg`, one of [KCP_NONE | KCP_AES | KCP_BLOWFISH | KCP_CAST5 | KCP_SM4 | KCP_SALSA20 | KCP_SIMPLEXOR | KCP_TEA | KCP_3DES | KCP_TWOFISH | KCP_XTEA] to use KCP (github.com/xtaci/kcp-go) reliable UDP instead of TCP") //nolint:lll
-	flag.UintVar(&port, "p", 2000, "``port")
+	flag.UintVar(&port, "p", 2000, "``port")                                                                                                                                                                                                                            //nolint:gomnd,lll
-	//flag.StringVar(&authCookie, "a", "", "auth cookie")
+	//nolint:gocritic,nolintlint // flag.StringVar(&authCookie, "a", "", "auth cookie")
 	flag.BoolVar(&chaffEnabled, "e", true, "enable chaff pkts")
-	flag.UintVar(&chaffFreqMin, "f", 100, "chaff pkt freq min `msecs`")
+	flag.UintVar(&chaffFreqMin, "f", 100, "chaff pkt freq min `msecs`")  //nolint:gomnd
-	flag.UintVar(&chaffFreqMax, "F", 5000, "chaff pkt freq max `msecs`")
+	flag.UintVar(&chaffFreqMax, "F", 5000, "chaff pkt freq max `msecs`") //nolint:gomnd
-	flag.UintVar(&chaffBytesMax, "B", 64, "chaff pkt size max `bytes`")
+	flag.UintVar(&chaffBytesMax, "B", 64, "chaff pkt size max `bytes`")  //nolint:gomnd
 
 	flag.StringVar(&cpuprofile, "cpuprofile", "", "write cpu profile to <`file`>")
 	flag.StringVar(&memprofile, "memprofile", "", "write memory profile to <`file`>")
 
-	//=== xc vs. xs option parsing
+	// === xc vs. xs option parsing
 
 	// Find out what program we are (shell or copier)
 	myPath := strings.Split(os.Args[0], string(os.PathSeparator))
@@ -759,7 +770,7 @@ func main() { //nolint: funlen, gocyclo
 		flag.Usage = usageShell
 	} else {
 		flag.BoolVar(&copyQuiet, "q", false, "do not output progress bar during copy")
-		flag.UintVar(&copyLimitBPS, "L", 8589934592, "copy max rate in bytes per sec")
+		flag.UintVar(&copyLimitBPS, "L", 8589934592, "copy max rate in bytes per sec") //nolint:gomnd
 		flag.Usage = usageCp
 	}
 	flag.Parse()
@@ -769,7 +780,7 @@ func main() { //nolint: funlen, gocyclo
 		exitWithStatus(0)
 	}
 
-	//=== Profiling instrumentation
+	// === Profiling instrumentation
 
 	if cpuprofile != "" {
 		f, err := os.Create(cpuprofile)
@@ -779,19 +790,19 @@ func main() { //nolint: funlen, gocyclo
 		defer f.Close()
 		fmt.Println("StartCPUProfile()")
 		if err := pprof.StartCPUProfile(f); err != nil {
-			log.Fatal("could not start CPU profile: ", err)
+			log.Fatal("could not start CPU profile: ", err) //nolint:gocritic
 		} else {
 			defer pprof.StopCPUProfile()
 		}
 
-		go func() { http.ListenAndServe("localhost:6060", nil) }()
+		go func() { http.ListenAndServe("localhost:6060", nil) }() //nolint:errcheck,gosec
 	}
 
-	//=== User, host, port and path args for file operations, if applicable
+	// === User, host, port and path args for file operations, if applicable
 
 	remoteUser, remoteHost, tmpPath, pathIsDest, otherArgs :=
 		parseNonSwitchArgs(flag.Args())
-	//fmt.Println("otherArgs:", otherArgs)
+	//nolint:gocritic,nolintlint // fmt.Println("otherArgs:", otherArgs)
 
 	// Set defaults if user doesn't specify user, path or port
 	var uname string
@@ -809,7 +820,7 @@ func main() { //nolint: funlen, gocyclo
 		tmpPath = "."
 	}
 
-	//=== Copy mode arg and copy src/dest setup
+	// === Copy mode arg and copy src/dest setup
 
 	var fileArgs string
 	if !shellMode /*&& tmpPath != ""*/ {
@@ -842,15 +853,15 @@ func main() { //nolint: funlen, gocyclo
 		}
 	}
 
-	//=== Do some final option consistency checks
+	// === Do some final option consistency checks
 
-	//fmt.Println("server finally is:", server)
+	//nolint:gocritic,nolintlint // fmt.Println("server finally is:", server)
 	if flag.NFlag() == 0 && server == "" {
 		flag.Usage()
 		exitWithStatus(0)
 	}
 
-	if len(cmdStr) != 0 && (len(copySrc) != 0 || len(copyDst) != 0) {
+	if cmdStr != "" && (len(copySrc) != 0 || copyDst != "") {
 		log.Fatal("incompatible options -- either cmd (-x) or copy ops but not both")
 	}
 
@@ -863,18 +874,18 @@ func main() { //nolint: funlen, gocyclo
 	if dbg {
 		log.SetOutput(Log)
 	} else {
-		log.SetOutput(ioutil.Discard)
+		log.SetOutput(io.Discard)
 	}
 
-	//=== Auth token fetch for login
+	// === Auth token fetch for login
 
 	if !gopt {
 		// See if we can log in via an auth token
 		u, _ := user.Current()
-		ab, aerr := ioutil.ReadFile(fmt.Sprintf("%s/.xs_id", u.HomeDir))
+		ab, aerr := os.ReadFile(fmt.Sprintf("%s/.xs_id", u.HomeDir))
 		if aerr == nil {
 			for _, line := range strings.Split(string(ab), "\n") {
-				line = line + "\n"
+				line += "\n"
 				idx := strings.Index(line, remoteHost+":"+uname)
 				if idx >= 0 {
 					line = line[idx:]
@@ -894,8 +905,8 @@ func main() { //nolint: funlen, gocyclo
 	}
 	runtime.GC()
 
-	//=== Enforce some sane min/max vals on chaff flags
+	// === Enforce some sane min/max vals on chaff flags
-	if chaffFreqMin < 2 {
+	if chaffFreqMin < 2 { //nolint:gomnd
 		chaffFreqMin = 2
 	}
 	if chaffFreqMax == 0 {
@@ -905,7 +916,7 @@ func main() { //nolint: funlen, gocyclo
 		chaffBytesMax = 64
 	}
 
-	//=== Shell vs. Copy mode chaff and cmd setup
+	// === Shell vs. Copy mode chaff and cmd setup
 
 	if shellMode {
 		// We must make the decision about interactivity before Dial()
@@ -915,7 +926,7 @@ func main() { //nolint: funlen, gocyclo
 			op = []byte{'A'}
 			chaffFreqMin = 2
 			chaffFreqMax = 10
-		} else if len(cmdStr) == 0 {
+		} else if cmdStr == "" {
 			op = []byte{'s'}
 			isInteractive = true
 		} else {
@@ -936,18 +947,18 @@ func main() { //nolint: funlen, gocyclo
 			// client->server file copy
 			// src file list is in copySrc
 			op = []byte{'D'}
-			//fmt.Println("client->server copy:", string(copySrc), "->", copyDst)
+			//nolint:gocritic,nolintlint // fmt.Println("client->server copy:", string(copySrc), "->", copyDst)
 			cmdStr = copyDst
 		} else {
 			// server->client file copy
 			// remote src file(s) in copyDsr
 			op = []byte{'S'}
-			//fmt.Println("server->client copy:", string(copySrc), "->", copyDst)
+			//nolint:gocritic,nolintlint // fmt.Println("server->client copy:", string(copySrc), "->", copyDst)
 			cmdStr = string(copySrc)
 		}
 	}
 
-	//=== TCP / KCP Dial setup
+	// === TCP / KCP Dial setup
 
 	proto := "tcp"
 	if kcpMode != "unused" {
@@ -956,10 +967,10 @@ func main() { //nolint: funlen, gocyclo
 	conn, err := xsnet.Dial(proto, server, cipherAlg, hmacAlg, kexAlg, kcpMode)
 	if err != nil {
 		fmt.Println(err)
-		exitWithStatus(3)
+		exitWithStatus(XSNetDialFailed)
 	}
 
-	//=== Shell terminal mode (Shell vs. Copy) setup
+	// === Shell terminal mode (Shell vs. Copy) setup
 
 	// Set stdin in raw mode if it's an interactive session
 	// TODO: send flag to server side indicating this
@@ -967,7 +978,7 @@ func main() { //nolint: funlen, gocyclo
 	var oldState *xs.State
 	defer conn.Close()
 
-	//=== From this point on, conn is a secure encrypted channel
+	// === From this point on, conn is a secure encrypted channel
 
 	if shellMode {
 		if isatty.IsTerminal(os.Stdin.Fd()) {
@@ -983,20 +994,20 @@ func main() { //nolint: funlen, gocyclo
 		}
 	}
 
-	//=== Login phase
+	// === Login phase
 
 	// Start login timeout here and disconnect if user/pass phase stalls
-	//iloginImpatience := time.AfterFunc(20*time.Second, func() {
+	// iloginImpatience := time.AfterFunc(20*time.Second, func() {
-	//i	fmt.Printf(" .. [you still there? Waiting for a password.]")
+	// i	fmt.Printf(" .. [you still there? Waiting for a password.]")
-	//i})
+	// i})
-	loginTimeout := time.AfterFunc(30*time.Second, func() {
+	loginTimeout := time.AfterFunc(30*time.Second, func() { //nolint:gomnd
 		restoreTermState(oldState)
 		fmt.Printf(" .. [login timeout]\n")
 		exitWithStatus(xsnet.CSOLoginTimeout)
 	})
 
-	if len(authCookie) == 0 {
+	if authCookie == "" {
-		//No auth token, prompt for password
+		// No auth token, prompt for password
 		fmt.Printf("Gimme cookie:")
 		ab, e := xs.ReadPassword(os.Stdin.Fd())
 		fmt.Printf("\r\n")
@@ -1006,43 +1017,43 @@ func main() { //nolint: funlen, gocyclo
 		authCookie = string(ab)
 	}
 
-	//i_ = loginImpatience.Stop()
+	//nolint:gocritic,nolintlint // i_ = loginImpatience.Stop()
 	_ = loginTimeout.Stop()
 	// Security scrub
 	runtime.GC()
 
-	//=== Session param and TERM setup
+	// === Session param and TERM setup
 
 	// Set up session params and send over to server
 	rec := xs.NewSession(op, []byte(uname), []byte(remoteHost), []byte(os.Getenv("TERM")), []byte(cmdStr), []byte(authCookie), 0)
 	sendErr := sendSessionParams(&conn, rec)
 	if sendErr != nil {
 		restoreTermState(oldState)
-		rec.SetStatus(254)
+		rec.SetStatus(ServerRejectedSecureProposal)
 		fmt.Fprintln(os.Stderr, "Error: server rejected secure proposal params or login timed out")
 		exitWithStatus(int(rec.Status()))
-		//log.Fatal(sendErr)
+		//nolint:gocritic,nolintlint // log.Fatal(sendErr)
 	}
 
-	//Security scrub
+	// Security scrub
 	authCookie = "" //nolint: ineffassign
 	runtime.GC()
 
-	//=== Login Auth
+	// === Login Auth
 
-	//=== Read auth reply from server
+	// === Read auth reply from server
 	authReply := make([]byte, 1) // bool: 0 = fail, 1 = pass
 	_, err = conn.Read(authReply)
 	if err != nil {
-		//=== Exit if auth reply not received
+		// === Exit if auth reply not received
 		fmt.Fprintln(os.Stderr, "Error reading auth reply")
-		rec.SetStatus(255)
+		rec.SetStatus(ErrReadingAuthReply)
 	} else if authReply[0] == 0 {
-		//=== .. or if auth failed
+		// === .. or if auth failed
 		fmt.Fprintln(os.Stderr, rejectUserMsg())
-		rec.SetStatus(255)
+		rec.SetStatus(GeneralProtocolErr)
 	} else {
-		//=== Set up chaffing to server
+		// === Set up chaffing to server
 		conn.SetupChaff(chaffFreqMin, chaffFreqMax, chaffBytesMax) // enable client->server chaffing
 		if chaffEnabled {
 			// #gv:s/label=\"main\$2\"/label=\"deferCloseChaff\"/
@@ -1052,16 +1063,16 @@ func main() { //nolint: funlen, gocyclo
 			defer conn.ShutdownChaff()
 		}
 
-		//=== (goroutine) Start keepAliveWorker for tunnels
+		// === (goroutine) Start keepAliveWorker for tunnels
 		// #gv:s/label=\"main\$1\"/label=\"tunKeepAlive\"/
 		// TODO:.gv:main:1:tunKeepAlive
-		//[1]: better to always send tunnel keepAlives even if client didn't specify
+		// [1]: better to always send tunnel keepAlives even if client didn't specify
-		//     any, to prevent listeners from knowing this.
+		//      any, to prevent listeners from knowing this.
-		//[1] if tunSpecStr != "" {
+		// [1] if tunSpecStr != "" {
 		keepAliveWorker := func() {
 			for {
 				// Add a bit of jitter to keepAlive so it doesn't stand out quite as much
-				time.Sleep(time.Duration(2000-rand.Intn(200)) * time.Millisecond) //nolint:gosec
+				time.Sleep(time.Duration(2000-rand.Intn(200)) * time.Millisecond) //nolint:gosec,gomnd
 				// FIXME: keepAlives should probably have small random packet len/data as well
 				// to further obscure them vs. interactive or tunnel data
 				// keepAlives must be  >=2 bytes, due to processing elsewhere
@@ -1069,15 +1080,15 @@ func main() { //nolint: funlen, gocyclo
 			}
 		}
 		go keepAliveWorker()
-		//[1]}
+		// [1]}
 
-		//=== Session entry (shellMode or copyMode)
+		// === Session entry (shellMode or copyMode)
 		if shellMode {
-			//=== (shell) launch tunnels
+			// === (shell) launch tunnels
-			launchTuns(&conn, remoteHost, tunSpecStr)
+			launchTuns(&conn /*remoteHost,*/, tunSpecStr)
 			doShellMode(isInteractive, &conn, oldState, rec)
 		} else {
-			//=== (.. or file copy)
+			// === (.. or file copy)
 			s, _ := doCopyMode(&conn, pathIsDest, fileArgs, copyQuiet, copyLimitBPS, rec)
 			rec.SetStatus(s)
 		}
@@ -1093,7 +1104,7 @@ func main() { //nolint: funlen, gocyclo
 		oldState = nil
 	}
 
-	//=== Exit
+	// === Exit
 	exitWithStatus(int(rec.Status()))
 }
 
@@ -1128,7 +1139,7 @@ func exitWithStatus(status int) {
 		defer f.Close()
 		runtime.GC() // get up-to-date statistics
 		if err := pprof.WriteHeapProfile(f); err != nil {
-			log.Fatal("could not write memory profile: ", err)
+			log.Fatal("could not write memory profile: ", err) //nolint:gocritic
 		}
 	}