Skip to content

Commit eb4cb0b

Browse files
fix(pass): trim relay password consistently so trailing whitespace doesn't break the handshake (#1153)
determinePass only trimmed the password when it was read from a file; a password supplied via --pass / CROC_PASS with a trailing newline was left untrimmed. The relay then trimmed the received password but compared it against the untrimmed stored password, so 'pass123\n' != 'pass123' and the handshake failed with 'bad password' / 'password mismatch' (regression from v10.4.5). Trim in determinePass regardless of source, and trim both sides of the relay comparison. Adds regression tests for both paths.
1 parent e89255e commit eb4cb0b

4 files changed

Lines changed: 32 additions & 2 deletions

File tree

src/cli/cli.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,9 @@ func determinePass(c *cli.Context) (pass string) {
277277
pass = c.String("pass")
278278
b, err := os.ReadFile(pass)
279279
if err == nil {
280-
pass = strings.TrimSpace(string(b))
280+
pass = string(b)
281281
}
282+
pass = strings.TrimSpace(pass)
282283
return
283284
}
284285

src/cli/cli_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,29 @@
11
package cli
22

33
import (
4+
"flag"
45
"reflect"
56
"testing"
7+
8+
"github.com/schollz/cli/v2"
69
)
710

11+
// determinePass should trim a pass from --pass/CROC_PASS, not just from a file.
12+
func TestDeterminePassTrimsEnvValue(t *testing.T) {
13+
set := flag.NewFlagSet("test", flag.ContinueOnError)
14+
set.String("pass", "", "")
15+
if err := set.Set("pass", "pass123\n"); err != nil {
16+
t.Fatalf("failed to set flag: %v", err)
17+
}
18+
ctx := cli.NewContext(nil, set, nil)
19+
20+
got := determinePass(ctx)
21+
want := "pass123"
22+
if got != want {
23+
t.Errorf("determinePass(%q) = %q, want %q", "pass123\\n", got, want)
24+
}
25+
}
26+
827
func TestParseRelayPorts(t *testing.T) {
928
tests := []struct {
1029
name string

src/tcp/tcp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ func (s *server) clientCommunication(c *comm.Comm) (room string, err error) {
325325
if err != nil {
326326
return
327327
}
328-
if strings.TrimSpace(string(passwordBytes)) != s.password {
328+
if strings.TrimSpace(string(passwordBytes)) != strings.TrimSpace(s.password) {
329329
err = fmt.Errorf("bad password")
330330
enc, _ := crypt.Encrypt([]byte(err.Error()), strongKeyForEncryption)
331331
if err = c.Send(enc); err != nil {

src/tcp/tcp_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,16 @@ func TestWrongPassword(t *testing.T) {
170170
assert.Contains(t, err.Error(), "bad password")
171171
}
172172

173+
// A relay password with trailing whitespace should still accept a trimmed client password.
174+
func TestPasswordWithTrailingWhitespace(t *testing.T) {
175+
log.SetLevel("error")
176+
go Run("debug", "127.0.0.1", "8395", "pass123 ", "8396")
177+
time.Sleep(100 * time.Millisecond)
178+
179+
_, _, _, err := ConnectToTCPServer("127.0.0.1:8395", "pass123", "testRoom")
180+
assert.Nil(t, err)
181+
}
182+
173183
func TestRoomIsolation(t *testing.T) {
174184
log.SetLevel("error")
175185
go Run("debug", "127.0.0.1", "8387", "pass123", "8388")

0 commit comments

Comments
 (0)