From 667328a91c9aca30ff6a6644553fcf7e5eb5d1cf Mon Sep 17 00:00:00 2001 From: Russ Magee Date: Sat, 24 Sep 2022 20:01:16 -0700 Subject: [PATCH] xs/ lint cleanups --- .golangci.yml | 2 + xs/termsize_unix.go | 2 +- xs/xs.go | 183 +++++++++++++++++++++++--------------------- 3 files changed, 100 insertions(+), 87 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 38507bb..a4cad3d 100644 --- a/.golangci.yml +++ b/.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 diff --git a/xs/termsize_unix.go b/xs/termsize_unix.go index c435d9a..112947a 100644 --- a/xs/termsize_unix.go +++ b/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. diff --git a/xs/xs.go b/xs/xs.go index 2e142e1..3f8823f 100755 --- a/xs/xs.go +++ b/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 buf[0] == 0x1d { + if seqPos < 4 { //nolint:gomnd + 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.Setenv("HOME", u.HomeDir) - //os.Setenv("TERM", "vt102") // TODO: server or client option? + // os.Clearenv() + // os.Setenv("HOME", u.HomeDir) + // 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 - //Read remote end's stdout + // Client reader (from server) goroutine + // 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) { - /*remAddrs, _ := net.LookupHost(remoteHost)*/ +func launchTuns(conn *xsnet.Conn /*remoteHost string,*/, tuns string) { + /*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 - cipherAlg string //cipher alg - hmacAlg string //hmac alg - kexAlg string //KEX/KEM alg + shellMode bool // true: act as shell, false: file copier + cipherAlg string + hmacAlg string + 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.UintVar(&port, "p", 2000, "``port") - //flag.StringVar(&authCookie, "a", "", "auth cookie") + 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") //nolint:gomnd,lll + //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(&chaffFreqMax, "F", 5000, "chaff pkt freq max `msecs`") - flag.UintVar(&chaffBytesMax, "B", 64, "chaff pkt size max `bytes`") + flag.UintVar(&chaffFreqMin, "f", 100, "chaff pkt freq min `msecs`") //nolint:gomnd + flag.UintVar(&chaffFreqMax, "F", 5000, "chaff pkt freq max `msecs`") //nolint:gomnd + 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(©Quiet, "q", false, "do not output progress bar during copy") - flag.UintVar(©LimitBPS, "L", 8589934592, "copy max rate in bytes per sec") + flag.UintVar(©LimitBPS, "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 - if chaffFreqMin < 2 { + // === Enforce some sane min/max vals on chaff flags + 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() { - //i fmt.Printf(" .. [you still there? Waiting for a password.]") - //i}) - loginTimeout := time.AfterFunc(30*time.Second, func() { + // iloginImpatience := time.AfterFunc(20*time.Second, func() { + // i fmt.Printf(" .. [you still there? Waiting for a password.]") + // i}) + loginTimeout := time.AfterFunc(30*time.Second, func() { //nolint:gomnd restoreTermState(oldState) fmt.Printf(" .. [login timeout]\n") exitWithStatus(xsnet.CSOLoginTimeout) }) - if len(authCookie) == 0 { - //No auth token, prompt for password + if authCookie == "" { + // 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 - // any, to prevent listeners from knowing this. - //[1] if tunSpecStr != "" { + // [1]: better to always send tunnel keepAlives even if client didn't specify + // any, to prevent listeners from knowing this. + // [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 - launchTuns(&conn, remoteHost, tunSpecStr) + // === (shell) launch tunnels + 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 } }