Omit the account session from Hello until EAA is checked

AUTHORS

@@ -7,6 +7,7 @@
 # Individual Persons
 
 Alexandr Stelnykovych <alexandr.stelnykovych@ivpn.net>
+Sebastien Tardif <sebtardif@ncf.ca>
 
 
 # Organizations

daemon/protocol/hello_eaa_test.go

@@ -0,0 +1,99 @@
+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"
+)
+
+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)
+	}
+}
+
+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")
+	}
+}

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)
 		}

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{}

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)