From 6fbbcdadb67a5ac91a6a44370791b4a8f19d5b45 Mon Sep 17 00:00:00 2001 From: Russ Magee Date: Sun, 25 Nov 2018 10:24:10 -0800 Subject: [PATCH] Gometalinter cleanup/audit of hkexsh, hkexshd, hkexpasswd Signed-off-by: Russ Magee --- Makefile | 9 +- consts.go | 3 +- hkexauth.go | 26 +++-- hkexpasswd/Makefile | 4 +- hkexpasswd/hkexpasswd.go | 15 ++- hkexsession.go | 33 ++++++- hkexsh/Makefile | 4 +- hkexsh/hkexsh.go | 77 +++++++++------ hkexsh/termsize_linux.go | 2 +- hkexshd/Makefile | 5 +- hkexshd/hkexshd.go | 200 +++++++++++++++++++++------------------ logger/Makefile | 4 +- logger/logger_linux.go | 34 ++++++- termmode_unix.go | 3 +- 14 files changed, 269 insertions(+), 150 deletions(-) diff --git a/Makefile b/Makefile index 6281f65..347164d 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: vis clean common client server passwd subpkgs install uninstall +.PHONY: lint vis clean common client server passwd subpkgs install uninstall reinstall SUBPKGS = logger spinsult hkexnet herradurakex TOOLS = hkexpasswd hkexsh hkexshd @@ -59,6 +59,13 @@ vis: make -C hkexpasswd vis; \ fi +lint: + make -C hkexpasswd lint + make -C hkexshd lint + make -C hkexsh lint + +reinstall: uninstall install + install: cp hkexsh/hkexsh $(INSTPREFIX)/bin ifeq ($(MSYSTEM),) diff --git a/consts.go b/consts.go index ddd9d41..e8ad6cb 100644 --- a/consts.go +++ b/consts.go @@ -1,4 +1,4 @@ -// Common constants for the HKExSh +// Package hkexsh - common constants for the HKExSh // // Copyright (c) 2017-2018 Russell Magee // Licensed under the terms of the MIT license (see LICENSE.mit in this @@ -7,5 +7,6 @@ // golang implementation by Russ Magee (rmagee_at_gmail.com) package hkexsh +// Version string returned by tools const Version = "0.7pre (NO WARRANTY)" diff --git a/hkexauth.go b/hkexauth.go index 2fd43a7..a70a02c 100644 --- a/hkexauth.go +++ b/hkexauth.go @@ -24,15 +24,15 @@ import ( func userExistsOnSystem(who string) bool { _, userErr := user.Lookup(who) - if userErr != nil { - return false - } else { - return true - } + return userErr == nil } +// AuthUserByPasswd checks user login information using a password. +// This checks /etc/hkexsh.passwd for auth info, and system /etc/passwd +// to cross-check the user actually exists. +// nolint: gocyclo func AuthUserByPasswd(username string, auth string, fname string) (valid bool, allowedCmds string) { - b, e := ioutil.ReadFile(fname) + b, e := ioutil.ReadFile(fname) // nolint: gosec if e != nil { valid = false log.Println("ERROR: Cannot read hkexsh.passwd file!") @@ -60,7 +60,10 @@ func AuthUserByPasswd(username string, auth string, fname string) (valid bool, a } if username == record[0] { - tmp, _ := bcrypt.Hash(auth, record[1]) + tmp, err := bcrypt.Hash(auth, record[1]) + if err != nil { + break + } if tmp == record[2] && username != "$nosuchuser$" { valid = true } @@ -71,7 +74,6 @@ func AuthUserByPasswd(username string, auth string, fname string) (valid bool, a for i := range b { b[i] = 0 } - b = nil r = nil runtime.GC() @@ -81,6 +83,11 @@ func AuthUserByPasswd(username string, auth string, fname string) (valid bool, a return } +// AuthUserByToken checks user login information against an auth token. +// Auth tokens are stored in each user's $HOME/.hkexsh_id and are requested +// via the -g option. +// The function also check system /etc/passwd to cross-check the user +// actually exists. func AuthUserByToken(username string, connhostname string, auth string) (valid bool) { auth = strings.TrimSpace(auth) u, ue := user.Lookup(username) @@ -111,7 +118,8 @@ func AuthUserByToken(username string, connhostname string, auth string) (valid b if (connhostname == record[0]) && (auth == strings.Join([]string{record[0], record[1]}, ":")) { - return true + valid = true + break } } if !userExistsOnSystem(username) { diff --git a/hkexpasswd/Makefile b/hkexpasswd/Makefile index 96acf08..6cf6f22 100644 --- a/hkexpasswd/Makefile +++ b/hkexpasswd/Makefile @@ -1,4 +1,4 @@ -.PHONY: clean all vis +.PHONY: clean all vis lint EXTPKGS = bytes,errors,flag,fmt,internal,io,log,net,os,path,runtime,time,strings,sync,syscall,binary,encoding EXE = $(notdir $(shell pwd)) @@ -12,3 +12,5 @@ clean: vis: go-callvis -skipbrowser -png -svg -output hkexpasswd-vis -ignore $(EXTPKGS) -group pkg,type . +lint: + -gometalinter --deadline=60s | sort diff --git a/hkexpasswd/hkexpasswd.go b/hkexpasswd/hkexpasswd.go index d1b779f..8e51164 100644 --- a/hkexpasswd/hkexpasswd.go +++ b/hkexpasswd/hkexpasswd.go @@ -21,6 +21,7 @@ import ( "github.com/jameskeane/bcrypt" ) +// nolint: gocyclo func main() { var pfName string var newpw string @@ -84,7 +85,7 @@ func main() { } //fmt.Println("Salt:", salt, "Hash:", hash) - b, err := ioutil.ReadFile(pfName) + b, err := ioutil.ReadFile(pfName) // nolint: gosec if err != nil { log.Fatal(err) } @@ -100,7 +101,7 @@ func main() { } recFound := false - for i, _ := range records { + for i := range records { //fmt.Println(records[i]) if records[i][0] == uname { recFound = true @@ -124,8 +125,14 @@ func main() { w := csv.NewWriter(outFile) w.Comma = ':' //w.FieldsPerRecord = 4 // username:salt:authCookie:disallowedCmdList (a,b,...) - w.Write([]string{"#username", "salt", "authCookie"/*, "disallowedCmdList"*/}) - w.WriteAll(records) + err = w.Write([]string{"#username", "salt", "authCookie" /*, "disallowedCmdList"*/}) + if err != nil { + log.Fatal(err) + } + err = w.WriteAll(records) + if err != nil { + log.Fatal(err) + } if err = w.Error(); err != nil { log.Fatal(err) } diff --git a/hkexsession.go b/hkexsession.go index 55b81a6..74e07b9 100644 --- a/hkexsession.go +++ b/hkexsession.go @@ -13,6 +13,7 @@ import ( "runtime" ) +// Session holds essential bookkeeping info about an active session. type Session struct { op []byte who []byte @@ -29,58 +30,80 @@ func (h *Session) String() string { h.op, h.who, h.cmd, h.AuthCookie(false), h.status) } +// Op returns the op code of the Session (interactive shell, cmd, ...) func (h Session) Op() []byte { return h.op } +// SetOp stores the op code desired for a Session. func (h *Session) SetOp(o []byte) { h.op = o } +// Who returns the user associated with a Session. func (h Session) Who() []byte { return h.who } +// SetWho sets the username associated with a Session. func (h *Session) SetWho(w []byte) { h.who = w } +// ConnHost returns the connecting hostname/IP string for a Session. func (h Session) ConnHost() []byte { return h.connhost } +// SetConnHost stores the connecting hostname/IP string for a Session. func (h *Session) SetConnHost(n []byte) { h.connhost = n } +// TermType returns the TERM env variable reported by the client initiating +// a Session. func (h Session) TermType() []byte { return h.termtype } +// SetTermType stores the TERM env variable supplied by the client initiating +// a Session. func (h *Session) SetTermType(t []byte) { h.termtype = t } +// Cmd returns the command requested for execution by a client initiating +// the Session. func (h Session) Cmd() []byte { return h.cmd } +// SetCmd stores the command request by the client for execution when initiating +// the Session. func (h *Session) SetCmd(c []byte) { h.cmd = c } +// AuthCookie returns the authcookie (essentially the password) used for +// authorization of the Session. This return value is censored unless +// reallyShow is true (so dumps of Session Info do not accidentally leak it). func (h Session) AuthCookie(reallyShow bool) []byte { if reallyShow { return h.authCookie - } else { - return []byte("**REDACTED**") } + return []byte("**REDACTED**") } +// SetAuthCookie stores the authcookie (essential the password) used to +// authenticate the Session. func (h *Session) SetAuthCookie(a []byte) { h.authCookie = a } +// ClearAuthCookie attempts to scrub the Session's stored authcookie. +// +// This should of course be called as soon as possible after authentication +// and it is no longer required. func (h *Session) ClearAuthCookie() { for i := range h.authCookie { h.authCookie[i] = 0 @@ -88,14 +111,20 @@ func (h *Session) ClearAuthCookie() { runtime.GC() } +// Status returns the (current) Session status code. +// +// This usually corresponds to a UNIX shell exit code, but +// extended codes are returns at times to indicate internal errors. func (h Session) Status() uint32 { return h.status } +// SetStatus stores the current Session status code. func (h *Session) SetStatus(s uint32) { h.status = s } +// NewSession returns a new Session record. func NewSession(op, who, connhost, ttype, cmd, authcookie []byte, status uint32) *Session { return &Session{ op: op, diff --git a/hkexsh/Makefile b/hkexsh/Makefile index b4d319d..3218db5 100644 --- a/hkexsh/Makefile +++ b/hkexsh/Makefile @@ -1,4 +1,4 @@ -.PHONY: clean all vis +.PHONY: clean all vis lint EXTPKGS = bytes,errors,flag,fmt,internal,io,log,net,os,path,runtime,time,strings,sync,syscall,binary,encoding EXE = $(notdir $(shell pwd)) @@ -12,4 +12,6 @@ clean: vis: go-callvis -skipbrowser -svg -png -output hkexsh-vis -ignore $(EXTPKGS) -group pkg,type . +lint: + -gometalinter --deadline=60s | sort diff --git a/hkexsh/hkexsh.go b/hkexsh/hkexsh.go index 8deac25..69a7d31 100755 --- a/hkexsh/hkexsh.go +++ b/hkexsh/hkexsh.go @@ -66,6 +66,7 @@ func GetSize() (cols, rows int, err error) { } // doCopyMode begins a secure hkexsh local<->remote file copy operation. +// TODO: reduce gocyclo func doCopyMode(conn *hkexnet.Conn, remoteDest bool, files string, rec *hkexsh.Session) (exitStatus uint32, err error) { if remoteDest { log.Println("local files:", files, "remote filepath:", string(rec.Cmd())) @@ -299,15 +300,15 @@ func doShellMode(isInteractive bool, conn *hkexnet.Conn, oldState *hkexsh.State, } func usageShell() { - fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) - fmt.Fprintf(os.Stderr, "%s [opts] [user]@server\n", os.Args[0]) + fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) // nolint: errcheck + fmt.Fprintf(os.Stderr, "%s [opts] [user]@server\n", os.Args[0]) // nolint: errcheck flag.PrintDefaults() } func usageCp() { - fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) - fmt.Fprintf(os.Stderr, "%s [opts] srcFileOrDir [...] [user]@server[:dstpath]\n", os.Args[0]) - fmt.Fprintf(os.Stderr, "%s [opts] [user]@server[:srcFileOrDir] dstPath\n", os.Args[0]) + fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) // nolint: errcheck + fmt.Fprintf(os.Stderr, "%s [opts] srcFileOrDir [...] [user]@server[:dstpath]\n", os.Args[0]) // nolint: errcheck + fmt.Fprintf(os.Stderr, "%s [opts] [user]@server[:srcFileOrDir] dstPath\n", os.Args[0]) // nolint: errcheck flag.PrintDefaults() } @@ -323,17 +324,15 @@ func reqTunnel(hc *hkexnet.Conn, lp uint16, p string /*net.Addr*/, rp uint16) { // Write request to server so it can attempt to set up its end var bTmp bytes.Buffer if e := binary.Write(&bTmp, binary.BigEndian, lp); e != nil { - fmt.Fprintln(os.Stderr, "reqTunnel:", e) + fmt.Fprintln(os.Stderr, "reqTunnel:", e) // nolint: errcheck } if e := binary.Write(&bTmp, binary.BigEndian, rp); e != nil { - fmt.Fprintln(os.Stderr, "reqTunnel:", e) + fmt.Fprintln(os.Stderr, "reqTunnel:", e) // nolint: errcheck } - _ = logger.LogDebug(fmt.Sprintln("[Client sending CSOTunSetup]")) + _ = logger.LogDebug(fmt.Sprintln("[Client sending CSOTunSetup]")) // nolint: gosec if n, e := hc.WritePacket(bTmp.Bytes(), hkexnet.CSOTunSetup); e != nil || n != len(bTmp.Bytes()) { - fmt.Fprintln(os.Stderr, "reqTunnel:", e) + fmt.Fprintln(os.Stderr, "reqTunnel:", e) // nolint: errcheck } - - return } func parseNonSwitchArgs(a []string) (user, host, path string, isDest bool, otherArgs []string) { @@ -378,7 +377,7 @@ func parseNonSwitchArgs(a []string) (user, host, path string, isDest bool, other } func launchTuns(conn *hkexnet.Conn, remoteHost string, tuns string) { - remAddrs, _ := net.LookupHost(remoteHost) + remAddrs, _ := net.LookupHost(remoteHost) // nolint: gosec if tuns == "" { return @@ -387,7 +386,7 @@ func launchTuns(conn *hkexnet.Conn, remoteHost string, tuns string) { tunSpecs := strings.Split(tuns, ",") for _, tunItem := range tunSpecs { var lPort, rPort uint16 - _, _ = fmt.Sscanf(tunItem, "%d:%d", &lPort, &rPort) + _, _ = fmt.Sscanf(tunItem, "%d:%d", &lPort, &rPort) // nolint: gosec reqTunnel(conn, lPort, remAddrs[0], rPort) } } @@ -395,13 +394,31 @@ func launchTuns(conn *hkexnet.Conn, remoteHost string, tuns string) { func sendSessionParams(conn io.Writer /* *hkexnet.Conn*/, rec *hkexsh.Session) (e error) { _, e = fmt.Fprintf(conn, "%d %d %d %d %d %d\n", len(rec.Op()), len(rec.Who()), len(rec.ConnHost()), len(rec.TermType()), len(rec.Cmd()), len(rec.AuthCookie(true))) + if e != nil { + return + } _, e = conn.Write(rec.Op()) + if e != nil { + return + } _, e = conn.Write(rec.Who()) + if e != nil { + return + } _, e = conn.Write(rec.ConnHost()) + if e != nil { + return + } _, e = conn.Write(rec.TermType()) + if e != nil { + return + } _, e = conn.Write(rec.Cmd()) + if e != nil { + return + } _, e = conn.Write(rec.AuthCookie(true)) - return + return e } // hkexsh - a client for secure shell and file copy operations. @@ -415,6 +432,7 @@ func sendSessionParams(conn io.Writer /* *hkexnet.Conn*/, rec *hkexsh.Session) ( // setting desired; as well as the intended operation mode for the // connection (app-specific, passed through to the server to use or // ignore at its discretion). +// TODO: reduce gocyclo func main() { version := hkexsh.Version var vopt bool @@ -475,7 +493,7 @@ func main() { // Set defaults if user doesn't specify user, path or port var uname string if remoteUser == "" { - u, _ := user.Current() + u, _ := user.Current() // nolint: gosec uname = u.Username } else { uname = remoteUser @@ -541,7 +559,7 @@ func main() { // either the shell session or copy operation. _ = shellMode - Log, _ = logger.New(logger.LOG_USER|logger.LOG_DEBUG|logger.LOG_NOTICE|logger.LOG_ERR, "hkexsh") + Log, _ = logger.New(logger.LOG_USER|logger.LOG_DEBUG|logger.LOG_NOTICE|logger.LOG_ERR, "hkexsh") // nolint: errcheck,gosec hkexnet.Init(dbg, "hkexsh", logger.LOG_USER|logger.LOG_DEBUG|logger.LOG_NOTICE|logger.LOG_ERR) if dbg { log.SetOutput(Log) @@ -551,7 +569,7 @@ func main() { if !gopt { // See if we can log in via an auth token - u, _ := user.Current() + u, _ := user.Current() // nolint: gosec ab, aerr := ioutil.ReadFile(fmt.Sprintf("%s/.hkexsh_id", u.HomeDir)) if aerr == nil { idx := strings.Index(string(ab), remoteHost) @@ -574,7 +592,7 @@ func main() { // We must make the decision about interactivity before Dial() // as it affects chaffing behaviour. 20180805 if gopt { - fmt.Fprintln(os.Stderr, "[requesting authtoken from server]") + fmt.Fprintln(os.Stderr, "[requesting authtoken from server]") // nolint: errcheck op = []byte{'A'} chaffFreqMin = 2 chaffFreqMax = 10 @@ -615,7 +633,7 @@ func main() { fmt.Println(err) panic(err) } - defer conn.Close() + defer conn.Close() // nolint: errcheck // From this point on, conn is a secure encrypted channel // Set stdin in raw mode if it's an interactive session @@ -628,7 +646,7 @@ func main() { if err != nil { panic(err) } - defer func() { _ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) }() // Best effort. + defer func() { _ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) }() // nolint: errcheck,gosec } else { log.Println("NOT A TTY") } @@ -649,20 +667,23 @@ func main() { // Set up session params and send over to server rec := hkexsh.NewSession(op, []byte(uname), []byte(remoteHost), []byte(os.Getenv("TERM")), []byte(cmdStr), []byte(authCookie), 0) - sendSessionParams(&conn, rec) + sendErr := sendSessionParams(&conn, rec) + if sendErr != nil { + log.Fatal(sendErr) + } //Security scrub - authCookie = "" + authCookie = "" // nolint: ineffassign runtime.GC() // Read auth reply from server authReply := make([]byte, 1) // bool: 0 = fail, 1 = pass _, err = conn.Read(authReply) if err != nil { - fmt.Fprintln(os.Stderr, "Error reading auth reply") + fmt.Fprintln(os.Stderr, "Error reading auth reply") // nolint: errcheck rec.SetStatus(255) } else if authReply[0] == 0 { - fmt.Fprintln(os.Stderr, rejectUserMsg()) + fmt.Fprintln(os.Stderr, rejectUserMsg()) // nolint: errcheck rec.SetStatus(255) } else { @@ -678,7 +699,7 @@ func main() { go func() { for { time.Sleep(time.Duration(2) * time.Second) - conn.WritePacket([]byte{0, 0}, hkexnet.CSOTunKeepAlive) + conn.WritePacket([]byte{0, 0}, hkexnet.CSOTunKeepAlive) // nolint: errcheck,gosec } }() @@ -687,17 +708,17 @@ func main() { doShellMode(isInteractive, &conn, oldState, rec) } else { // copyMode - s, _ := doCopyMode(&conn, pathIsDest, fileArgs, rec) + s, _ := doCopyMode(&conn, pathIsDest, fileArgs, rec) // nolint: errcheck,gosec rec.SetStatus(s) } if rec.Status() != 0 { - fmt.Fprintln(os.Stderr, "Session exited with status:", rec.Status()) + fmt.Fprintln(os.Stderr, "Session exited with status:", rec.Status()) // nolint: errcheck } } if oldState != nil { - _ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // Best effort. + _ = hkexsh.Restore(int(os.Stdin.Fd()), oldState) // nolint: gosec } os.Exit(int(rec.Status())) } diff --git a/hkexsh/termsize_linux.go b/hkexsh/termsize_linux.go index bcd79cd..8120a80 100644 --- a/hkexsh/termsize_linux.go +++ b/hkexsh/termsize_linux.go @@ -29,7 +29,7 @@ func handleTermResizes(conn *hkexnet.Conn) { log.Println(err) } termSzPacket := fmt.Sprintf("%d %d", rows, cols) - conn.WritePacket([]byte(termSzPacket), hkexnet.CSOTermSize) + conn.WritePacket([]byte(termSzPacket), hkexnet.CSOTermSize) // nolint: errcheck,gosec } }() ch <- syscall.SIGWINCH // Initial resize. diff --git a/hkexshd/Makefile b/hkexshd/Makefile index 7e41914..963c18c 100644 --- a/hkexshd/Makefile +++ b/hkexshd/Makefile @@ -1,4 +1,4 @@ -.PHONY: clean all vis +.PHONY: clean all vis lint EXTPKGS = binary,bytes,crypto,encoding,errors,flag,fmt,internal,io,log,net,os,path,runtime,time,strings,sync,syscall EXE = $(notdir $(shell pwd)) @@ -12,3 +12,6 @@ clean: vis: go-callvis -skipbrowser -png -svg -output hkexshd-vis -ignore $(EXTPKGS) -group pkg,type . +lint: + -gometalinter --deadline=60s | sort + diff --git a/hkexshd/hkexshd.go b/hkexshd/hkexshd.go index 77748d3..7c31ce4 100755 --- a/hkexshd/hkexshd.go +++ b/hkexshd/hkexshd.go @@ -34,16 +34,17 @@ import ( ) var ( - Log *logger.Writer // reg. syslog output (no -d) + // Log - syslog output (with no -d) + Log *logger.Writer ) /* -------------------------------------------------------------- */ // Perform a client->server copy -func runClientToServerCopyAs(who, ttype string, conn *hkexnet.Conn, fpath string, chaffing bool) (err error, exitStatus uint32) { - u, _ := user.Lookup(who) +func runClientToServerCopyAs(who, ttype string, conn *hkexnet.Conn, fpath string, chaffing bool) (exitStatus uint32, err error) { + u, _ := user.Lookup(who) // nolint: gosec var uid, gid uint32 - fmt.Sscanf(u.Uid, "%d", &uid) - fmt.Sscanf(u.Gid, "%d", &gid) + fmt.Sscanf(u.Uid, "%d", &uid) // nolint: gosec,errcheck + fmt.Sscanf(u.Gid, "%d", &gid) // nolint: gosec,errcheck log.Println("uid:", uid, "gid:", gid) // Need to clear server's env and set key vars of the @@ -54,9 +55,9 @@ func runClientToServerCopyAs(who, ttype string, conn *hkexnet.Conn, fpath string // of client shell window used to run client. // Investigate -- rlm 2018-01-26) os.Clearenv() - os.Setenv("HOME", u.HomeDir) - os.Setenv("TERM", ttype) - os.Setenv("HKEXSH", "1") + os.Setenv("HOME", u.HomeDir) // nolint: gosec,errcheck + os.Setenv("TERM", ttype) // nolint: gosec,errcheck + os.Setenv("HKEXSH", "1") // nolint: gosec,errcheck var c *exec.Cmd cmdName := "/bin/tar" @@ -74,7 +75,7 @@ func runClientToServerCopyAs(who, ttype string, conn *hkexnet.Conn, fpath string // When args are passed in exec() format, no quoting is required // (as this isn't input from a shell) (right? -rlm 20180823) //cmdArgs := []string{"-x", "-C", destDir, `--xform=s#.*/\(.*\)#\1#`} - c = exec.Command(cmdName, cmdArgs...) + c = exec.Command(cmdName, cmdArgs...) // nolint: gosec c.Dir = destDir @@ -128,7 +129,7 @@ func runClientToServerCopyAs(who, ttype string, conn *hkexnet.Conn, fpath string // an ExitStatus() method with the same signature. if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { exitStatus = uint32(status.ExitStatus()) - err = errors.New("cmd returned nonzero status") + //err = errors.New("cmd returned nonzero status") log.Printf("Exit Status: %d\n", exitStatus) } } @@ -139,11 +140,15 @@ func runClientToServerCopyAs(who, ttype string, conn *hkexnet.Conn, fpath string } // Perform a server->client copy -func runServerToClientCopyAs(who, ttype string, conn *hkexnet.Conn, srcPath string, chaffing bool) (err error, exitStatus uint32) { - u, _ := user.Lookup(who) +func runServerToClientCopyAs(who, ttype string, conn *hkexnet.Conn, srcPath string, chaffing bool) (exitStatus uint32, err error) { + u, err := user.Lookup(who) + if err != nil { + exitStatus = 1 + return + } var uid, gid uint32 - fmt.Sscanf(u.Uid, "%d", &uid) - fmt.Sscanf(u.Gid, "%d", &gid) + _, _ = fmt.Sscanf(u.Uid, "%d", &uid) // nolint: gosec + _, _ = fmt.Sscanf(u.Gid, "%d", &gid) // nolint: gosec log.Println("uid:", uid, "gid:", gid) // Need to clear server's env and set key vars of the @@ -154,9 +159,9 @@ func runServerToClientCopyAs(who, ttype string, conn *hkexnet.Conn, srcPath stri // of client shell window used to run client. // Investigate -- rlm 2018-01-26) os.Clearenv() - os.Setenv("HOME", u.HomeDir) - os.Setenv("TERM", ttype) - os.Setenv("HKEXSH", "1") + _ = os.Setenv("HOME", u.HomeDir) // nolint: gosec + _ = os.Setenv("TERM", ttype) // nolint: gosec + _ = os.Setenv("HKEXSH", "1") // nolint: gosec var c *exec.Cmd cmdName := "/bin/tar" @@ -167,7 +172,7 @@ func runServerToClientCopyAs(who, ttype string, conn *hkexnet.Conn, srcPath stri srcDir, srcBase := path.Split(srcPath) cmdArgs := []string{"-cz", "-C", srcDir, "-f", "-", srcBase} - c = exec.Command(cmdName, cmdArgs...) + c = exec.Command(cmdName, cmdArgs...) // nolint: gosec //If os.Clearenv() isn't called by server above these will be seen in the //client's session env. @@ -197,40 +202,44 @@ func runServerToClientCopyAs(who, ttype string, conn *hkexnet.Conn, srcPath stri err = c.Start() // returns immediately if err != nil { log.Printf("Command finished with error: %v", err) - return err, hkexnet.CSEExecFail // !? - } else { - 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 + return hkexnet.CSEExecFail, err // !? + } + 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 - // This works on both Unix and Windows. Although package - // syscall is generally platform dependent, WaitStatus is - // defined for both Unix and Windows and in both cases has - // an ExitStatus() method with the same signature. - if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { - exitStatus = uint32(status.ExitStatus()) - if len(stdErrBuffer.Bytes()) > 0 { - log.Print(stdErrBuffer) - } - log.Printf("Exit Status: %d", exitStatus) + // This works on both Unix and Windows. Although package + // syscall is generally platform dependent, WaitStatus is + // defined for both Unix and Windows and in both cases has + // an ExitStatus() method with the same signature. + if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { + exitStatus = uint32(status.ExitStatus()) + if len(stdErrBuffer.Bytes()) > 0 { + log.Print(stdErrBuffer) } + log.Printf("Exit Status: %d", exitStatus) } } - //fmt.Println("*** server->client cp finished ***") - return } + //fmt.Println("*** server->client cp finished ***") + return } // Run a command (via default shell) as a specific user // // Uses ptys to support commands which expect a terminal. -func runShellAs(who, ttype string, cmd string, interactive bool, conn *hkexnet.Conn, chaffing bool) (err error, exitStatus uint32) { +// nolint: gocyclo +func runShellAs(who, ttype string, cmd string, interactive bool, conn *hkexnet.Conn, chaffing bool) (exitStatus uint32, err error) { var wg sync.WaitGroup - u, _ := user.Lookup(who) + u, err := user.Lookup(who) + if err != nil { + exitStatus = 1 + return + } var uid, gid uint32 - fmt.Sscanf(u.Uid, "%d", &uid) - fmt.Sscanf(u.Gid, "%d", &gid) + _, _ = fmt.Sscanf(u.Uid, "%d", &uid) // nolint: gosec + _, _ = fmt.Sscanf(u.Gid, "%d", &gid) // nolint: gosec log.Println("uid:", uid, "gid:", gid) // Need to clear server's env and set key vars of the @@ -241,15 +250,15 @@ func runShellAs(who, ttype string, cmd string, interactive bool, conn *hkexnet.C // of client shell window used to run client. // Investigate -- rlm 2018-01-26) os.Clearenv() - os.Setenv("HOME", u.HomeDir) - os.Setenv("TERM", ttype) - os.Setenv("HKEXSH", "1") + _ = os.Setenv("HOME", u.HomeDir) // nolint: gosec + _ = os.Setenv("TERM", ttype) // nolint: gosec + _ = os.Setenv("HKEXSH", "1") // nolint: gosec var c *exec.Cmd if interactive { - c = exec.Command("/bin/bash", "-i", "-l") + c = exec.Command("/bin/bash", "-i", "-l") // nolint: gosec } else { - c = exec.Command("/bin/bash", "-c", cmd) + c = exec.Command("/bin/bash", "-c", cmd) // nolint: gosec } //If os.Clearenv() isn't called by server above these will be seen in the //client's session env. @@ -264,10 +273,10 @@ func runShellAs(who, ttype string, cmd string, interactive bool, conn *hkexnet.C // Start the command with a pty. ptmx, err := pty.Start(c) // returns immediately with ptmx file if err != nil { - return err, hkexnet.CSEPtyExecFail + return hkexnet.CSEPtyExecFail, err } // Make sure to close the pty at the end. - defer func() { _ = ptmx.Close() }() // Best effort. + defer func() { _ = ptmx.Close() }() // nolint: gosec log.Printf("[%s]\n", cmd) if err != nil { @@ -278,7 +287,7 @@ func runShellAs(who, ttype string, cmd string, interactive bool, conn *hkexnet.C go func() { for sz := range conn.WinCh { log.Printf("[Setting term size to: %v %v]\n", sz.Rows, sz.Cols) - pty.Setsize(ptmx, &pty.Winsize{Rows: sz.Rows, Cols: sz.Cols}) + pty.Setsize(ptmx, &pty.Winsize{Rows: sz.Rows, Cols: sz.Cols}) // nolint: gosec,errcheck } log.Println("*** WinCh goroutine done ***") }() @@ -339,6 +348,8 @@ func runShellAs(who, ttype string, cmd string, interactive bool, conn *hkexnet.C return } +// GenAuthToken generates a pseudorandom auth token for a specific +// user from a specific host to allow non-interactive logins. func GenAuthToken(who string, connhost string) string { //tokenA, e := os.Hostname() //if e != nil { @@ -347,7 +358,7 @@ func GenAuthToken(who string, connhost string) string { tokenA := connhost tokenB := make([]byte, 64) - _, _ = rand.Read(tokenB) + _, _ = rand.Read(tokenB) // nolint: gosec return fmt.Sprintf("%s:%s", tokenA, hex.EncodeToString(tokenB)) } @@ -356,6 +367,7 @@ func GenAuthToken(who string, connhost string) string { // server code, save for declaring 'hkex' rather than 'net' // Listener and Conns. The KEx and encrypt/decrypt is done within the type. // Compare to 'serverp.go' in this directory to see the equivalence. +// TODO: reduce gocyclo func main() { version := hkexsh.Version @@ -388,7 +400,7 @@ func main() { } } - Log, _ = logger.New(logger.LOG_DAEMON|logger.LOG_DEBUG|logger.LOG_NOTICE|logger.LOG_ERR, "hkexshd") + Log, _ = logger.New(logger.LOG_DAEMON|logger.LOG_DEBUG|logger.LOG_NOTICE|logger.LOG_ERR, "hkexshd") // nolint: gosec hkexnet.Init(dbg, "hkexshd", logger.LOG_DAEMON|logger.LOG_DEBUG|logger.LOG_NOTICE|logger.LOG_ERR) if dbg { log.SetOutput(Log) @@ -404,17 +416,17 @@ func main() { sig := <-exitCh switch sig.String() { case "terminated": - logger.LogNotice(fmt.Sprintf("[Got signal: %s]", sig)) + logger.LogNotice(fmt.Sprintf("[Got signal: %s]", sig)) // nolint: gosec,errcheck signal.Reset() - syscall.Kill(0, syscall.SIGTERM) + syscall.Kill(0, syscall.SIGTERM) // nolint: gosec,errcheck case "interrupt": - logger.LogNotice(fmt.Sprintf("[Got signal: %s]", sig)) + logger.LogNotice(fmt.Sprintf("[Got signal: %s]", sig)) // nolint: gosec,errcheck signal.Reset() - syscall.Kill(0, syscall.SIGINT) + syscall.Kill(0, syscall.SIGINT) // nolint: gosec,errcheck case "hangup": - logger.LogNotice(fmt.Sprintf("[Got signal: %s - nop]", sig)) + logger.LogNotice(fmt.Sprintf("[Got signal: %s - nop]", sig)) // nolint:gosec,errcheck default: - logger.LogNotice(fmt.Sprintf("[Got signal: %s - ignored]", sig)) + logger.LogNotice(fmt.Sprintf("[Got signal: %s - ignored]", sig)) // nolint: gosec,errcheck } } }() @@ -425,7 +437,7 @@ func main() { if err != nil { log.Fatal(err) } - defer l.Close() + defer l.Close() // nolint: errcheck log.Println("Serving on", laddr) for { @@ -445,7 +457,7 @@ func main() { // The loop then returns to accepting, so that // multiple connections may be served concurrently. go func(hc *hkexnet.Conn) (e error) { - defer hc.Close() + defer hc.Close() // nolint: errcheck //We use io.ReadFull() here to guarantee we consume //just the data we want for the hkexsh.Session, and no more. @@ -462,7 +474,7 @@ func main() { return err } - tmp := make([]byte, len1, len1) + tmp := make([]byte, len1) _, err = io.ReadFull(hc, tmp) if err != nil { log.Println("[Bad hkexsh.Session.Op]") @@ -470,7 +482,7 @@ func main() { } rec.SetOp(tmp) - tmp = make([]byte, len2, len2) + tmp = make([]byte, len2) _, err = io.ReadFull(hc, tmp) if err != nil { log.Println("[Bad hkexsh.Session.Who]") @@ -478,7 +490,7 @@ func main() { } rec.SetWho(tmp) - tmp = make([]byte, len3, len3) + tmp = make([]byte, len3) _, err = io.ReadFull(hc, tmp) if err != nil { log.Println("[Bad hkexsh.Session.ConnHost]") @@ -486,7 +498,7 @@ func main() { } rec.SetConnHost(tmp) - tmp = make([]byte, len4, len4) + tmp = make([]byte, len4) _, err = io.ReadFull(hc, tmp) if err != nil { log.Println("[Bad hkexsh.Session.TermType]") @@ -494,7 +506,7 @@ func main() { } rec.SetTermType(tmp) - tmp = make([]byte, len5, len5) + tmp = make([]byte, len5) _, err = io.ReadFull(hc, tmp) if err != nil { log.Println("[Bad hkexsh.Session.Cmd]") @@ -502,7 +514,7 @@ func main() { } rec.SetCmd(tmp) - tmp = make([]byte, len6, len6) + tmp = make([]byte, len6) _, err = io.ReadFull(hc, tmp) if err != nil { log.Println("[Bad hkexsh.Session.AuthCookie]") @@ -526,10 +538,10 @@ func main() { // Tell client if auth was valid if valid { - hc.Write([]byte{1}) + hc.Write([]byte{1}) // nolint: gosec,errcheck } else { - logger.LogNotice(fmt.Sprintln("Invalid user", string(rec.Who()))) - hc.Write([]byte{0}) // ? required? + logger.LogNotice(fmt.Sprintln("Invalid user", string(rec.Who()))) // nolint: errcheck,gosec + hc.Write([]byte{0}) // nolint: gosec,errcheck return } @@ -539,15 +551,15 @@ func main() { // Generate automated login token addr := hc.RemoteAddr() hname := goutmp.GetHost(addr.String()) - logger.LogNotice(fmt.Sprintf("[Generating autologin token for [%s@%s]]\n", rec.Who(), hname)) + logger.LogNotice(fmt.Sprintf("[Generating autologin token for [%s@%s]]\n", rec.Who(), hname)) // nolint: gosec,errcheck token := GenAuthToken(string(rec.Who()), string(rec.ConnHost())) tokenCmd := fmt.Sprintf("echo \"%s\" | tee -a ~/.hkexsh_id", token) - runErr, cmdStatus := runShellAs(string(rec.Who()), string(rec.TermType()), tokenCmd, false, hc, chaffEnabled) + cmdStatus, runErr := runShellAs(string(rec.Who()), string(rec.TermType()), tokenCmd, false, hc, chaffEnabled) // Returned hopefully via an EOF or exit/logout; // Clear current op so user can enter next, or EOF rec.SetOp([]byte{0}) if runErr != nil { - logger.LogErr(fmt.Sprintf("[Error generating autologin token for %s@%s]\n", rec.Who(), hname)) + logger.LogErr(fmt.Sprintf("[Error generating autologin token for %s@%s]\n", rec.Who(), hname)) // nolint: gosec,errcheck } else { log.Printf("[Autologin token generation completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus) hc.SetStatus(hkexnet.CSOType(cmdStatus)) @@ -556,34 +568,34 @@ func main() { // Non-interactive command addr := hc.RemoteAddr() hname := goutmp.GetHost(addr.String()) - logger.LogNotice(fmt.Sprintf("[Running command for [%s@%s]]\n", rec.Who(), hname)) - runErr, cmdStatus := runShellAs(string(rec.Who()), string(rec.TermType()), string(rec.Cmd()), false, hc, chaffEnabled) + logger.LogNotice(fmt.Sprintf("[Running command for [%s@%s]]\n", rec.Who(), hname)) // nolint: gosec,errcheck + cmdStatus, runErr := runShellAs(string(rec.Who()), string(rec.TermType()), string(rec.Cmd()), false, hc, chaffEnabled) // Returned hopefully via an EOF or exit/logout; // Clear current op so user can enter next, or EOF rec.SetOp([]byte{0}) if runErr != nil { - logger.LogErr(fmt.Sprintf("[Error spawning cmd for %s@%s]\n", rec.Who(), hname)) + logger.LogErr(fmt.Sprintf("[Error spawning cmd for %s@%s]\n", rec.Who(), hname)) // nolint: gosec,errcheck } else { - logger.LogNotice(fmt.Sprintf("[Command completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) + logger.LogNotice(fmt.Sprintf("[Command completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) // nolint: gosec,errcheck hc.SetStatus(hkexnet.CSOType(cmdStatus)) } } else if rec.Op()[0] == 's' { // Interactive session addr := hc.RemoteAddr() hname := goutmp.GetHost(addr.String()) - logger.LogNotice(fmt.Sprintf("[Running shell for [%s@%s]]\n", rec.Who(), hname)) + logger.LogNotice(fmt.Sprintf("[Running shell for [%s@%s]]\n", rec.Who(), hname)) // nolint: gosec,errcheck utmpx := goutmp.Put_utmp(string(rec.Who()), hname) defer func() { goutmp.Unput_utmp(utmpx) }() goutmp.Put_lastlog_entry("hkexsh", string(rec.Who()), hname) - runErr, cmdStatus := runShellAs(string(rec.Who()), string(rec.TermType()), string(rec.Cmd()), true, hc, chaffEnabled) + cmdStatus, runErr := runShellAs(string(rec.Who()), string(rec.TermType()), string(rec.Cmd()), true, hc, chaffEnabled) // Returned hopefully via an EOF or exit/logout; // Clear current op so user can enter next, or EOF rec.SetOp([]byte{0}) if runErr != nil { - Log.Err(fmt.Sprintf("[Error spawning shell for %s@%s]\n", rec.Who(), hname)) + Log.Err(fmt.Sprintf("[Error spawning shell for %s@%s]\n", rec.Who(), hname)) // nolint: gosec,errcheck } else { - logger.LogNotice(fmt.Sprintf("[Shell completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) + logger.LogNotice(fmt.Sprintf("[Shell completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) // nolint: gosec,errcheck hc.SetStatus(hkexnet.CSOType(cmdStatus)) } } else if rec.Op()[0] == 'D' { @@ -591,15 +603,15 @@ func main() { log.Printf("[Client->Server copy]\n") addr := hc.RemoteAddr() hname := goutmp.GetHost(addr.String()) - logger.LogNotice(fmt.Sprintf("[Running copy for [%s@%s]]\n", rec.Who(), hname)) - runErr, cmdStatus := runClientToServerCopyAs(string(rec.Who()), string(rec.TermType()), hc, string(rec.Cmd()), chaffEnabled) + logger.LogNotice(fmt.Sprintf("[Running copy for [%s@%s]]\n", rec.Who(), hname)) // nolint: gosec,errcheck + cmdStatus, runErr := runClientToServerCopyAs(string(rec.Who()), string(rec.TermType()), hc, string(rec.Cmd()), chaffEnabled) // Returned hopefully via an EOF or exit/logout; // Clear current op so user can enter next, or EOF rec.SetOp([]byte{0}) if runErr != nil { - logger.LogErr(fmt.Sprintf("[Error running cp for %s@%s]\n", rec.Who(), hname)) + logger.LogErr(fmt.Sprintf("[Error running cp for %s@%s]\n", rec.Who(), hname)) // nolint: gosec,errcheck } else { - logger.LogNotice(fmt.Sprintf("[Command completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) + logger.LogNotice(fmt.Sprintf("[Command completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) // nolint: gosec,errcheck } hc.SetStatus(hkexnet.CSOType(cmdStatus)) @@ -607,32 +619,32 @@ func main() { s := make([]byte, 4) binary.BigEndian.PutUint32(s, cmdStatus) log.Printf("** cp writing closeStat %d at Close()\n", cmdStatus) - hc.WritePacket(s, hkexnet.CSOExitStatus) + hc.WritePacket(s, hkexnet.CSOExitStatus) // nolint: gosec,errcheck } else if rec.Op()[0] == 'S' { // File copy (src) operation - server copy to client log.Printf("[Server->Client copy]\n") addr := hc.RemoteAddr() hname := goutmp.GetHost(addr.String()) - logger.LogNotice(fmt.Sprintf("[Running copy for [%s@%s]]\n", rec.Who(), hname)) - runErr, cmdStatus := runServerToClientCopyAs(string(rec.Who()), string(rec.TermType()), hc, string(rec.Cmd()), chaffEnabled) - // Returned hopefully via an EOF or exit/logout; + logger.LogNotice(fmt.Sprintf("[Running copy for [%s@%s]]\n", rec.Who(), hname)) // nolint: gosec,errcheck + cmdStatus, runErr := runServerToClientCopyAs(string(rec.Who()), string(rec.TermType()), hc, string(rec.Cmd()), chaffEnabled) + if runErr != nil { + logger.LogErr(fmt.Sprintf("[Error spawning cp for %s@%s]\n", rec.Who(), hname)) // nolint: gosec,errcheck + } else { + // Returned hopefully via an EOF or exit/logout; + logger.LogNotice(fmt.Sprintf("[Command completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) // nolint: gosec,errcheck + } // Clear current op so user can enter next, or EOF rec.SetOp([]byte{0}) - if runErr != nil { - logger.LogErr(fmt.Sprintf("[Error spawning cp for %s@%s]\n", rec.Who(), hname)) - } else { - logger.LogNotice(fmt.Sprintf("[Command completed for %s@%s, status %d]\n", rec.Who(), hname, cmdStatus)) - } hc.SetStatus(hkexnet.CSOType(cmdStatus)) //fmt.Println("Waiting for EOF from other end.") //_, _ = hc.Read(nil /*ackByte*/) //fmt.Println("Got remote end ack.") } else { - logger.LogErr(fmt.Sprintln("[Bad hkexsh.Session]")) + logger.LogErr(fmt.Sprintln("[Bad hkexsh.Session]")) // nolint: gosec,errcheck } return - }(&conn) + }(&conn) // nolint: errcheck } // Accept() success } //endfor - logger.LogNotice(fmt.Sprintln("[Exiting]")) + //logger.LogNotice(fmt.Sprintln("[Exiting]")) // nolint: gosec,errcheck } diff --git a/logger/Makefile b/logger/Makefile index 32000d7..0287ffe 100644 --- a/logger/Makefile +++ b/logger/Makefile @@ -1,4 +1,4 @@ -.PHONY: clean all +.PHONY: clean all lint EXE = $(notdir $(shell pwd)) @@ -8,3 +8,5 @@ all: clean: $(RM) $(EXE) $(EXE).exe +lint: + gometalinter --deadline 60s | sort diff --git a/logger/logger_linux.go b/logger/logger_linux.go index 2179ddd..7c635fa 100644 --- a/logger/logger_linux.go +++ b/logger/logger_linux.go @@ -1,17 +1,20 @@ // +build linux -// -// Wrapper around UNIX syslog, so that it also may be wrapped -// with something else for Windows (Sadly, the stdlib log/syslog -// is frozen, and there is no Window implementation.) + +// Package logger is a wrapper around UNIX syslog, so that it also may +// be wrapped with something else for Windows (Sadly, the stdlib log/syslog +// is frozen, and there is no Windows implementation.) package logger import ( sl "log/syslog" ) +// Priority is the logger priority type Priority = sl.Priority +// Writer is a syslog Writer type Writer = sl.Writer +// nolint: golint const ( // Severity. @@ -27,6 +30,7 @@ const ( LOG_DEBUG ) +// nolint: golint const ( // Facility. @@ -62,39 +66,59 @@ var ( l *sl.Writer ) +// New returns a new log Writer. func New(flags Priority, tag string) (w *Writer, e error) { - w, e = sl.New(sl.Priority(flags), tag) + w, e = sl.New(flags, tag) l = w return w, e } +// Alert returns a log Alert error func Alert(s string) error { return l.Alert(s) } + +// LogClose closes the log Writer. func LogClose() error { return l.Close() } + +// LogCrit returns a log Alert error func LogCrit(s string) error { return l.Crit(s) } + +// LogDebug returns a log Debug error func LogDebug(s string) error { return l.Debug(s) } + +// LogEmerg returns a log Emerg error func LogEmerg(s string) error { return l.Emerg(s) } + +// LogErr returns a log Err error func LogErr(s string) error { return l.Err(s) } + +// LogInfo returns a log Info error func LogInfo(s string) error { return l.Info(s) } + +// LogNotice returns a log Notice error func LogNotice(s string) error { return l.Notice(s) } + +// LogWarning returns a log Warning error func LogWarning(s string) error { return l.Warning(s) } + +// LogWrite writes to the logger at default level func LogWrite(b []byte) (int, error) { return l.Write(b) } diff --git a/termmode_unix.go b/termmode_unix.go index 1d8578d..8bcb60a 100644 --- a/termmode_unix.go +++ b/termmode_unix.go @@ -20,6 +20,7 @@ const ioctlReadTermios = unix.TCGETS const ioctlWriteTermios = unix.TCSETS // From github.com/golang/crypto/blob/master/ssh/terminal/util.go + // State contains the state of a terminal. type State struct { termios unix.Termios @@ -87,7 +88,7 @@ func ReadPassword(fd int) ([]byte, error) { } defer func() { - unix.IoctlSetTermios(fd, ioctlWriteTermios, termios) + _ = unix.IoctlSetTermios(fd, ioctlWriteTermios, termios) // nolint: gosec }() return readPasswordLine(passwordReader(fd))