diff --git a/src/api/api.go b/src/api/api.go index 465fa03..1ab2478 100644 --- a/src/api/api.go +++ b/src/api/api.go @@ -724,7 +724,8 @@ func (a *Api) Receive(c *gin.Context) { } defer ws.Close() var stop = make(chan struct{}) - go a.handleSignalReceive(ws, number, stop) + // receive events carry the phone number of numbered accounts, so filter by the canonical account + go a.handleSignalReceive(ws, a.signalClient.CanonicalAccount(number), stop) go a.wsPing(ws, stop) wsPong(ws, stop) } else { diff --git a/src/client/cli.go b/src/client/cli.go index 0ec9bfd..e9c0af7 100644 --- a/src/client/cli.go +++ b/src/client/cli.go @@ -56,6 +56,18 @@ func isAccountSelector(candidate string) bool { return utils.IsPhoneNumber(candidate) } +// redactSecretArgs returns a copy of args with secret values masked, so they don't end up in the logs. +func redactSecretArgs(args []string) []string { + redacted := append([]string{}, args...) + for i := 1; i < len(redacted); i++ { + switch redacted[i-1] { + case "--recovery-key", "--totp", "--pin", "setPin": + redacted[i] = "***" + } + } + return redacted +} + func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, error) { containerId, err := getContainerId() @@ -100,7 +112,7 @@ func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, err if stdin != "" { fullCmd += "echo '" + stdin + "' | " } - fullCmd += signalCliBinary + " " + strings.Join(args, " ") + fullCmd += signalCliBinary + " " + strings.Join(redactSecretArgs(args), " ") log.Debug("*) su signal-api") log.Debug("*) ", fullCmd) diff --git a/src/client/client.go b/src/client/client.go index 6dc0d1f..e28da34 100644 --- a/src/client/client.go +++ b/src/client/client.go @@ -436,7 +436,7 @@ func (s *SignalClient) GetSignalCliMode() SignalCliMode { } func (s *SignalClient) Init(maxRetries int) error { - s.signalCliApiConfig = utils.NewSignalCliApiConfig() + s.signalCliApiConfig = utils.NewSignalCliApiConfig(s.signalCliConfig) err := s.signalCliApiConfig.Load(s.signalCliApiConfigPath) if err != nil { return err @@ -1093,6 +1093,11 @@ func (s *SignalClient) Receive(number string, timeout int64, ignoreAttachments b } } +// CanonicalAccount maps the ACI of a numbered account to its phone number (see utils.CanonicalAccount). +func (s *SignalClient) CanonicalAccount(account string) string { + return utils.CanonicalAccount(s.signalCliConfig, account) +} + func (s *SignalClient) GetReceiveChannel(number string) (chan JsonRpc2ReceivedMessage, string, error) { jsonRpc2Client, err := s.getJsonRpc2Client() if err != nil { diff --git a/src/client/jsonrpc2.go b/src/client/jsonrpc2.go index 0fd0093..4de9202 100644 --- a/src/client/jsonrpc2.go +++ b/src/client/jsonrpc2.go @@ -15,6 +15,7 @@ import ( "github.com/bbernhard/signal-cli-rest-api/utils" uuid "github.com/gofrs/uuid" log "github.com/sirupsen/logrus" + "github.com/tidwall/gjson" "github.com/tidwall/sjson" ) @@ -116,6 +117,16 @@ func (r *JsonRpc2Client) Dial(address string, maxRetries int) error { return nil } +// redactSecretParams returns a copy of a JSON-RPC request with secret params masked, so they don't end up in the logs. +func redactSecretParams(request []byte) []byte { + for _, key := range []string{"recoveryKey", "totp", "pin"} { + if gjson.GetBytes(request, "params."+key).Exists() { + request, _ = sjson.SetBytes(request, "params."+key, "***") + } + } + return request +} + func (r *JsonRpc2Client) getRaw(command string, account *string, args interface{}) (string, error) { type Request struct { JsonRpc string `json:"jsonrpc"` @@ -163,7 +174,7 @@ func (r *JsonRpc2Client) getRaw(command string, account *string, args interface{ } } - log.Debug("json-rpc command: ", string(fullCommandBytes)) + log.Debug("json-rpc command: ", string(redactSecretParams(fullCommandBytes))) _, err = r.conn.Write([]byte(string(fullCommandBytes) + "\n")) if err != nil { diff --git a/src/client/redact_test.go b/src/client/redact_test.go new file mode 100644 index 0000000..d735175 --- /dev/null +++ b/src/client/redact_test.go @@ -0,0 +1,37 @@ +package client + +import ( + "reflect" + "strings" + "testing" +) + +func TestRedactSecretArgs(t *testing.T) { + args := []string{"-a", "+12025550123", "register", "--recovery-key", "KEY", "--totp", "123456"} + want := []string{"-a", "+12025550123", "register", "--recovery-key", "***", "--totp", "***"} + if got := redactSecretArgs(args); !reflect.DeepEqual(got, want) { + t.Fatalf("got %q, want %q", got, want) + } + if args[4] != "KEY" { + t.Fatal("redactSecretArgs modified the arguments that are passed to signal-cli") + } + if got := redactSecretArgs([]string{"-a", "+12025550123", "setPin", "1234"}); got[3] != "***" { + t.Fatalf("setPin pin not redacted: %q", got) + } +} + +func TestRedactSecretParams(t *testing.T) { + request := []byte(`{"jsonrpc":"2.0","method":"register","params":{"account":"+12025550123","recoveryKey":"KEY","totp":"123456"}}`) + original := string(request) + + got := string(redactSecretParams(request)) + if strings.Contains(got, "KEY") || strings.Contains(got, "123456") || !strings.Contains(got, `"recoveryKey":"***"`) { + t.Fatalf("secrets not redacted: %s", got) + } + if string(request) != original { + t.Fatal("redactSecretParams modified the request that is sent to signal-cli") + } + if strings.Contains(got, `"pin"`) { + t.Fatalf("absent param was added: %s", got) + } +} diff --git a/src/go.mod b/src/go.mod index 3c9633f..08d2580 100644 --- a/src/go.mod +++ b/src/go.mod @@ -17,6 +17,7 @@ require ( github.com/swaggo/files v1.0.1 github.com/swaggo/gin-swagger v1.6.0 github.com/swaggo/swag v1.16.4 + github.com/tidwall/gjson v1.14.4 github.com/tidwall/sjson v1.2.5 github.com/yuin/gopher-lua v1.1.2 gopkg.in/yaml.v2 v2.4.0 @@ -51,7 +52,6 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/pelletier/go-toml/v2 v2.2.3 // indirect - github.com/tidwall/gjson v1.14.4 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect diff --git a/src/utils/account_identifier.go b/src/utils/account_identifier.go index 4095668..0668b50 100644 --- a/src/utils/account_identifier.go +++ b/src/utils/account_identifier.go @@ -1,7 +1,10 @@ package utils import ( + "encoding/json" "errors" + "os" + "path/filepath" "regexp" "strings" ) @@ -16,21 +19,53 @@ const ( ) var ( - e164AccountPattern = regexp.MustCompile(`^\+?[1-9]\d{1,14}$`) - aciAccountPattern = regexp.MustCompile(`(?i)^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`) + e164AccountPattern = regexp.MustCompile(`^\+[1-9]\d{1,14}$`) + aciAccountPattern = regexp.MustCompile(`^[0-9a-f]{32}$`) ) -// ParseAccountIdentifier accepts an E.164 phone number or an ACI UUID. -// ACI values are returned in lowercase, which is the form signal-cli stores. +// ParseAccountIdentifier accepts an E.164 phone number (signal-cli requires the leading +) or an ACI. +// Like signal-cli, an ACI may be given with or without dashes and whitespace (e.g. the Account Key +// shown by Signal Android). It is returned as a lowercase UUID with dashes, the form signal-cli stores. func ParseAccountIdentifier(s string) (IdentifierType, string, error) { s = strings.TrimSpace(s) if e164AccountPattern.MatchString(s) { return IdentifierE164, s, nil } - if aciAccountPattern.MatchString(s) { - return IdentifierACI, strings.ToLower(s), nil + compact := strings.ToLower(strings.ReplaceAll(strings.Join(strings.Fields(s), ""), "-", "")) + if aciAccountPattern.MatchString(compact) { + return IdentifierACI, compact[:8] + "-" + compact[8:12] + "-" + compact[12:16] + "-" + compact[16:20] + "-" + compact[20:], nil } - return 0, "", errors.New("Couldn't process request - invalid account identifier (expected an E.164 phone number or an ACI UUID)") + return 0, "", errors.New("Couldn't process request - invalid account identifier (expected an E.164 phone number with leading + or an ACI)") +} + +// CanonicalAccount returns the phone number of the local account whose ACI is id, otherwise id unchanged. +// signal-cli reports numbered accounts by phone number (e.g. in receive events), so mapping the ACI +// to that number gives every account a single key in the API's own bookkeeping. +func CanonicalAccount(signalCliConfigDir string, id string) string { + if typ, _, err := ParseAccountIdentifier(id); err != nil || typ != IdentifierACI { + return id + } + + data, err := os.ReadFile(filepath.Join(signalCliConfigDir, "data", "accounts.json")) + if err != nil { + return id + } + var stored struct { + Accounts []struct { + Number string `json:"number"` + Uuid string `json:"uuid"` + } `json:"accounts"` + } + if json.Unmarshal(data, &stored) != nil { + return id + } + + for _, account := range stored.Accounts { + if account.Number != "" && strings.EqualFold(account.Uuid, id) { + return account.Number + } + } + return id } // SignalCliAccountSelector returns the global signal-cli account flag and value. diff --git a/src/utils/account_identifier_test.go b/src/utils/account_identifier_test.go index 6ccfad6..94d0500 100644 --- a/src/utils/account_identifier_test.go +++ b/src/utils/account_identifier_test.go @@ -1,9 +1,13 @@ package utils -import "testing" +import ( + "os" + "path/filepath" + "testing" +) func TestParseAccountIdentifierE164(t *testing.T) { - cases := []string{"+12025550123", "+12", "12025550123"} + cases := []string{"+12025550123", "+12"} for _, raw := range cases { typ, value, err := ParseAccountIdentifier(raw) if err != nil { @@ -50,6 +54,62 @@ func TestParseAccountIdentifierACI(t *testing.T) { } } +// signal-cli also accepts the Account Key form: 32 hex characters, optionally grouped by whitespace. +func TestParseAccountIdentifierACIWithoutDashes(t *testing.T) { + want := "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee" + for _, raw := range []string{"AAAAAAAABBBB4CCC8DDDEEEEEEEEEEEE", "aaaa aaaa bbbb 4ccc 8ddd eeee eeee eeee"} { + typ, value, err := ParseAccountIdentifier(raw) + if err != nil { + t.Fatalf("%q: %v", raw, err) + } + if typ != IdentifierACI || value != want { + t.Fatalf("%q: type=%v value=%q, want %q", raw, typ, value, want) + } + } +} + +// writeAccountsJson creates a signal-cli config dir with one numbered and one numberless account. +func writeAccountsJson(t *testing.T) string { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "data"), 0700); err != nil { + t.Fatal(err) + } + accounts := `{"accounts":[ + {"path":"1","environment":"LIVE","number":"+12025550123","uuid":"aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"}, + {"path":"2","environment":"LIVE","number":null,"uuid":"11111111-1111-4111-8111-111111111111"} + ],"version":2}` + if err := os.WriteFile(filepath.Join(dir, "data", "accounts.json"), []byte(accounts), 0600); err != nil { + t.Fatal(err) + } + return dir +} + +func TestCanonicalAccount(t *testing.T) { + dir := writeAccountsJson(t) + cases := map[string]string{ + "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee": "+12025550123", // ACI of a numbered account + "11111111-1111-4111-8111-111111111111": "11111111-1111-4111-8111-111111111111", // numberless account + "22222222-2222-4222-8222-222222222222": "22222222-2222-4222-8222-222222222222", // unknown account + "+12025550123": "+12025550123", + } + for id, want := range cases { + if got := CanonicalAccount(dir, id); got != want { + t.Fatalf("%q: got %q, want %q", id, got, want) + } + } + if got := CanonicalAccount(t.TempDir(), "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee"); got != "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee" { + t.Fatalf("missing accounts.json: got %q", got) + } +} + +func TestTrustModeSharedByNumberAndACI(t *testing.T) { + config := NewSignalCliApiConfig(writeAccountsJson(t)) + config.SetTrustModeForNumber("aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", AlwaysTrust) + if trustMode, err := config.GetTrustModeForNumber("+12025550123"); err != nil || trustMode != AlwaysTrust { + t.Fatalf("got %v %v, want AlwaysTrust", trustMode, err) + } +} + func TestParseAccountIdentifierInvalid(t *testing.T) { cases := []string{ "", @@ -58,9 +118,9 @@ func TestParseAccountIdentifierInvalid(t *testing.T) { "+1", "+1234567890123456", "+12 345", + "12025550123", "not-an-account", "11111111-1111-4111-8111-11111111111", - "11111111111141118111111111111111", "GGGGGGGG-1111-4111-8111-111111111111", } for _, raw := range cases { diff --git a/src/utils/api_config.go b/src/utils/api_config.go index be02ba9..33cc1c7 100644 --- a/src/utils/api_config.go +++ b/src/utils/api_config.go @@ -85,12 +85,14 @@ type SignalCliApiConfigEntries struct { } type SignalCliApiConfig struct { - config SignalCliApiConfigEntries - path string + config SignalCliApiConfigEntries + path string + signalCliConfigDir string } -func NewSignalCliApiConfig() *SignalCliApiConfig { - return &SignalCliApiConfig{} +// NewSignalCliApiConfig creates the API config. signalCliConfigDir is used to key accounts canonically (see CanonicalAccount). +func NewSignalCliApiConfig(signalCliConfigDir string) *SignalCliApiConfig { + return &SignalCliApiConfig{signalCliConfigDir: signalCliConfigDir} } func (c *SignalCliApiConfig) Load(path string) error { @@ -110,7 +112,10 @@ func (c *SignalCliApiConfig) Load(path string) error { return nil } +// GetTrustModeForNumber and SetTrustModeForNumber key by CanonicalAccount, so the setting +// applies whether a numbered account is addressed by its phone number or its ACI. func (c *SignalCliApiConfig) GetTrustModeForNumber(number string) (SignalCliTrustMode, error) { + number = CanonicalAccount(c.signalCliConfigDir, number) if val, ok := c.config.Entries[number]; ok { return val.TrustMode, nil } @@ -119,6 +124,7 @@ func (c *SignalCliApiConfig) GetTrustModeForNumber(number string) (SignalCliTrus } func (c *SignalCliApiConfig) SetTrustModeForNumber(number string, trustMode SignalCliTrustMode) { + number = CanonicalAccount(c.signalCliConfigDir, number) if c.config.Entries == nil { c.config.Entries = make(map[string]SignalCliApiConfigEntry) }