From 918bc1add7453cf944cb09f7560be6d736d44da3 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Wed, 23 Sep 2026 10:16:08 -0700 Subject: [PATCH 1/2] fix: omit the account session from Hello until EAA is checked Hello is on the EAA skip list, and the reply included the API session token. Drop that token and the account id for a connection that has not passed the password check. After the first successful check, send a full Hello so the UI can load the session. Signed-off-by: Sebastien Tardif --- AUTHORS | 1 + daemon/protocol/hello_eaa_test.go | 32 +++++++++++++++++++++++++++++ daemon/protocol/protocol.go | 2 +- daemon/protocol/protocol_private.go | 15 ++++++++++++++ daemon/protocol/send.go | 21 +++++++++++++++++-- 5 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 daemon/protocol/hello_eaa_test.go diff --git a/AUTHORS b/AUTHORS index 3ce229f9d..31666395f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -7,6 +7,7 @@ # Individual Persons Alexandr Stelnykovych +Sebastien Tardif # Organizations diff --git a/daemon/protocol/hello_eaa_test.go b/daemon/protocol/hello_eaa_test.go new file mode 100644 index 000000000..a9b2dd874 --- /dev/null +++ b/daemon/protocol/hello_eaa_test.go @@ -0,0 +1,32 @@ +package protocol + +import ( + "testing" + + "github.com/ivpn/desktop-app/daemon/protocol/types" +) + +func TestHelloForClientOmitsSessionUntilEAA(t *testing.T) { + full := &types.HelloResp{} + full.Session.AccountID = "acct" + full.Session.Session = "session-token" + full.ParanoidMode.IsEnabled = true + + got := helloForClient(full, false, true).(*types.HelloResp) + if got.Session.Session != "" || got.Session.AccountID != "" { + t.Fatalf("unauthenticated Hello kept session AccountID=%q Session=%q", got.Session.AccountID, got.Session.Session) + } + if full.Session.Session != "session-token" { + t.Fatal("redaction mutated the original Hello") + } + + authed := helloForClient(full, true, true).(*types.HelloResp) + if authed.Session.Session != "session-token" { + t.Fatalf("authenticated Hello Session = %q", authed.Session.Session) + } + + disabled := helloForClient(full, false, false).(*types.HelloResp) + if disabled.Session.Session != "session-token" { + t.Fatalf("EAA-disabled Hello Session = %q", disabled.Session.Session) + } +} diff --git a/daemon/protocol/protocol.go b/daemon/protocol/protocol.go index 22b99449d..cd183a50b 100644 --- a/daemon/protocol/protocol.go +++ b/daemon/protocol/protocol.go @@ -532,7 +532,7 @@ func (p *Protocol) processRequest(conn net.Conn, message string) { helloResponse.ServiceBinary, _ = os.Executable() helloResponse.ServiceBinary, _ = filepath.EvalSymlinks(helloResponse.ServiceBinary) } - p.sendResponse(conn, helloResponse, req.Idx) + p.sendResponse(conn, helloForClient(helloResponse, p.clientIsAuthenticated(conn), p._eaa.IsEnabled()), req.Idx) if req.SendResponseToAllClients { p.notifyClients(helloResponse) } diff --git a/daemon/protocol/protocol_private.go b/daemon/protocol/protocol_private.go index fbba7f170..dec3ce2bd 100644 --- a/daemon/protocol/protocol_private.go +++ b/daemon/protocol/protocol_private.go @@ -173,8 +173,16 @@ func (p *Protocol) notifyClientsDaemonExiting() { p._connections = make(map[net.Conn]*connectionInfo) } +func (p *Protocol) clientIsAuthenticated(c net.Conn) bool { + p._connectionsMutex.RLock() + defer p._connectionsMutex.RUnlock() + info := p._connections[c] + return info != nil && info.IsAuthenticated +} + func (p *Protocol) clientSetAuthenticated(c net.Conn) { // separate anonymous function for correct mutex unlock + justAuthenticated := false func() { p._connectionsMutex.Lock() defer p._connectionsMutex.Unlock() @@ -183,6 +191,7 @@ func (p *Protocol) clientSetAuthenticated(c net.Conn) { if !cInfo.IsAuthenticated { // connected client (first authentication) cInfo.IsAuthenticated = true + justAuthenticated = true go func() { // notifying service about authenticated client (autoconnect if needed) @@ -192,6 +201,12 @@ func (p *Protocol) clientSetAuthenticated(c net.Conn) { } }() + // The first Hello is sent before this flag is set when EAA is on. + // Push a full Hello after the password check so the UI can load the session. + if justAuthenticated && p._eaa.IsEnabled() { + p.sendResponse(c, p.createHelloResponse(), 0) + } + if len(p._lastConnectionErrorToNotifyClient) > 0 { log.Info("Sending delayed error to client: ", p._lastConnectionErrorToNotifyClient) delayedErr := ivpnclient.ErrorRespDelayed{} diff --git a/daemon/protocol/send.go b/daemon/protocol/send.go index 4c0b9041c..634c5dad6 100644 --- a/daemon/protocol/send.go +++ b/daemon/protocol/send.go @@ -67,11 +67,28 @@ func Send(conn net.Conn, cmd ICommandBase, idx uint32) error { func (p *Protocol) notifyClients(cmd ICommandBase) { p._connectionsMutex.RLock() defer p._connectionsMutex.RUnlock() - for conn := range p._connections { - p.sendResponse(conn, cmd, 0) + for conn, info := range p._connections { + authenticated := info != nil && info.IsAuthenticated + p.sendResponse(conn, helloForClient(cmd, authenticated, p._eaa.IsEnabled()), 0) } } +// helloForClient drops the account session from a Hello reply until EAA +// has been checked on this connection. Other commands are unchanged. +func helloForClient(cmd ICommandBase, authenticated bool, eaaEnabled bool) ICommandBase { + if authenticated || !eaaEnabled { + return cmd + } + hello, ok := cmd.(*types.HelloResp) + if !ok || hello == nil { + return cmd + } + redacted := *hello + redacted.Session.Session = "" + redacted.Session.AccountID = "" + return &redacted +} + func (p *Protocol) sendError(conn net.Conn, errorText string, cmdIdx uint32) { log.Error(errorText) p.sendResponse(conn, &ivpnclient.ErrorResp{ErrorMessage: errorText}, cmdIdx) From 6380717df934750a5c9456bf3a3ca0a509a62382 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Wed, 23 Sep 2026 11:14:13 -0700 Subject: [PATCH 2/2] test: check notifyClients drops the session before EAA An unauthenticated connection receives a Hello whose account id and session token are empty on the wire. The unit test of helloForClient does not call notifyClients. Signed-off-by: Sebastien Tardif --- daemon/protocol/hello_eaa_test.go | 67 +++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/daemon/protocol/hello_eaa_test.go b/daemon/protocol/hello_eaa_test.go index a9b2dd874..3a40dd01b 100644 --- a/daemon/protocol/hello_eaa_test.go +++ b/daemon/protocol/hello_eaa_test.go @@ -1,8 +1,15 @@ package protocol import ( + "bufio" + "encoding/json" + "net" + "os" + "path/filepath" "testing" + "time" + "github.com/ivpn/desktop-app/daemon/protocol/eaa" "github.com/ivpn/desktop-app/daemon/protocol/types" ) @@ -30,3 +37,63 @@ func TestHelloForClientOmitsSessionUntilEAA(t *testing.T) { t.Fatalf("EAA-disabled Hello Session = %q", disabled.Session.Session) } } + +func TestNotifyClientsRedactsUnauthenticatedHello(t *testing.T) { + secretFile := filepath.Join(t.TempDir(), "eaa") + if err := os.WriteFile(secretFile, []byte("hash"), 0600); err != nil { + t.Fatal(err) + } + + client, server := net.Pipe() + defer client.Close() + defer server.Close() + + p := &Protocol{ + _connections: map[net.Conn]*connectionInfo{ + server: {IsAuthenticated: false}, + }, + _eaa: eaa.Init(secretFile), + } + + hello := &types.HelloResp{} + hello.Session.AccountID = "acct" + hello.Session.Session = "session-token" + + lineCh := make(chan string, 1) + errCh := make(chan error, 1) + go func() { + line, err := bufio.NewReader(client).ReadString('\n') + if err != nil { + errCh <- err + return + } + lineCh <- line + }() + + p.notifyClients(hello) + + var line string + select { + case line = <-lineCh: + case err := <-errCh: + t.Fatal(err) + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for Hello") + } + + var msg struct { + Session struct { + AccountID string + Session string + } + } + if err := json.Unmarshal([]byte(line), &msg); err != nil { + t.Fatal(err) + } + if msg.Session.AccountID != "" || msg.Session.Session != "" { + t.Fatalf("wire session AccountID=%q Session=%q", msg.Session.AccountID, msg.Session.Session) + } + if hello.Session.Session != "session-token" { + t.Fatal("notify mutated the original Hello") + } +}