jpillora / chisel

A fast TCP/UDP tunnel over HTTP
MIT License
12.69k stars 1.34k forks source link

chisel client - Avoiding plain-text password when http_proxy #340

Open yaakov-berkovitch opened 2 years ago

yaakov-berkovitch commented 2 years ago

Hello,

This is not a real issue or bug, but a question, even if I think it violates some basic security rules.

We are using chisel v1.7.6. I'm wondering if there is a way to avoid passing a plain-text password in the chisel client command line. After starting the chisel client, anyone that list the running processes will see the command arguments and so the clear password.

I didn't find a way to hide this command or the password from users, so any help with this concern will be very helpful.

Thanks.

jpillora commented 2 years ago

Good point, at this stage, no way to hide it from the process args (ps aux will display it)

Easiest change to fix this is to use an environment variable fallback for the --proxy option, like CHISEL_HTTP_PROXY and then define this env var in a systemd unit file (or similar). Feel free to raise a PR, will merge it

yaakov-berkovitch commented 2 years ago

Edited. Thanks @jpillora, but your workaround is not working. The environment variable is evaluated and then passed to the command. So the password still appears . There are 2 simple solutions I would propose:

What do you think ?

tman204 commented 2 years ago

I am new to Go, so my coding skills are weak, but I had this same problem, and solved it with these edits. This may or may not fit your use case.

$ git diff share/settings/user.go
diff --git a/share/settings/user.go b/share/settings/user.go
index 40b2c4e..46a007b 100644
--- a/share/settings/user.go
+++ b/share/settings/user.go
@@ -2,7 +2,11 @@ package settings

 import (
        "regexp"
+       "syscall"
+       "fmt"
        "strings"
+
+       "golang.org/x/term"
 )

 var UserAllowAll = regexp.MustCompile("")
@@ -10,7 +14,21 @@ var UserAllowAll = regexp.MustCompile("")
 func ParseAuth(auth string) (string, string) {
        if strings.Contains(auth, ":") {
                pair := strings.SplitN(auth, ":", 2)
-               return pair[0], pair[1]
+               if pair[1] == "" {
+                 fmt.Print("*** Enter session password: ")
+                 bytePassword, err := term.ReadPassword(int(syscall.Stdin))
+                 fmt.Printf("\n\n")
+                 if err != nil {
+                   return "", ""
+                 }
+                 var password string = string(bytePassword)
+                 return pair[0], strings.TrimSpace(password)
+               } else {
+                 var password string = pair[1]
+                 return pair[0], strings.TrimSpace(password)
+               }
+               //fmt.Println("user", pair[0])
+               //fmt.Println("pass", strings.TrimSpace(password))
        }
        return "", ""
 }

It allows one to specify --user "user:pass" as usual, however, if you specify --user "user:" then it will prompt you for a session password to use. This prevents this password from being discovered via ps. HTH

yaakov-berkovitch commented 2 years ago

@tman204 your solution is also a good approach, and by reading the password from stdin will indeed make it invisible using ps. I didn't find a PR for this change, why not creating a PR and having your changes part of releases ? @jpillora your advise would be very appreciated.

ayelet-ack commented 2 years ago

Added a PR to fix this issue: https://github.com/jpillora/chisel/pull/361 And also another PR to fix an issue we found with the re-connection mechanism: https://github.com/jpillora/chisel/pull/362 @jpillora could you please review & merge them? Thanks!

ayelet-ack commented 2 years ago

@jpillora I opened a PR with a fix for this issue a while ago: https://github.com/jpillora/chisel/pull/361 could you please merge? Thanks! @yaakov-berkovitch FYI