From f1c377195a1febda39d6d99ecfbc647ffcd5d740 Mon Sep 17 00:00:00 2001
From: Max Asnaashari <max.asnaashari@canonical.com>
Date: Wed, 21 Feb 2024 00:13:43 +0000
Subject: [PATCH 1/2] lxd/rsync: Merge sendSetup and Send functions

sendSetup can use `RunWrapper` to run rsync from an apparmor profile,
but needs to clean up the profile after the command is done. We track
the running command both in `Send` and `sendSetup` which makes managing
when to begin the cleanup more complex than necessary.

Since this function isn't used anywhere else, and is so tightly coupled
to the Send process, this commit merges the two functions so that we
don't have to track the running command from 2 separate functions before
deleting the profile.

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
(cherry picked from commit 96cee1e58d3fb84f1dc3a27f91f81cbc03261327)
---
 lxd/rsync/rsync.go | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/lxd/rsync/rsync.go b/lxd/rsync/rsync.go
index 71ec66dc1a24..5e7ab24e0861 100644
--- a/lxd/rsync/rsync.go
+++ b/lxd/rsync/rsync.go
@@ -120,7 +120,9 @@ func LocalCopy(source string, dest string, bwlimit string, xattrs bool, rsyncArg
 	return msg, nil
 }
 
-func sendSetup(name string, path string, bwlimit string, execPath string, features []string, rsyncArgs ...string) (*exec.Cmd, net.Conn, io.ReadCloser, error) {
+// Send sets up the sending half of an rsync, to recursively send the
+// directory pointed to by path over the websocket.
+func Send(name string, path string, conn io.ReadWriteCloser, tracker *ioprogress.ProgressTracker, features []string, bwlimit string, execPath string, rsyncArgs ...string) error {
 	/*
 	 * The way rsync works, it invokes a subprocess that does the actual
 	 * talking (given to it by a -E argument). Since there isn't an easy
@@ -144,7 +146,7 @@ func sendSetup(name string, path string, bwlimit string, execPath string, featur
 
 	l, err := net.Listen("unix", auds)
 	if err != nil {
-		return nil, nil, nil, err
+		return err
 	}
 
 	defer func() { _ = l.Close() }()
@@ -189,7 +191,7 @@ func sendSetup(name string, path string, bwlimit string, execPath string, featur
 	if RunWrapper != nil {
 		cleanup, err := RunWrapper(cmd, path, "")
 		if err != nil {
-			return nil, nil, nil, err
+			return err
 		}
 
 		defer cleanup()
@@ -197,55 +199,45 @@ func sendSetup(name string, path string, bwlimit string, execPath string, featur
 
 	stderr, err := cmd.StderrPipe()
 	if err != nil {
-		return nil, nil, nil, err
+		return err
 	}
 
 	err = cmd.Start()
 	if err != nil {
-		return nil, nil, nil, err
+		return err
 	}
 
-	var conn *net.Conn
+	var ncConn *net.Conn
 	chConn := make(chan *net.Conn, 1)
 
 	go func() {
-		conn, err := l.Accept()
+		ncConn, err := l.Accept()
 		if err != nil {
 			chConn <- nil
 			return
 		}
 
-		chConn <- &conn
+		chConn <- &ncConn
 	}()
 
 	select {
-	case conn = <-chConn:
-		if conn == nil {
+	case ncConn = <-chConn:
+		if ncConn == nil {
 			output, _ := io.ReadAll(stderr)
 			_ = cmd.Process.Kill()
 			_ = cmd.Wait()
-			return nil, nil, nil, fmt.Errorf("Failed to connect to rsync socket (%s)", string(output))
+			return fmt.Errorf("Failed to ncConnect to rsync socket (%s)", string(output))
 		}
 
 	case <-time.After(10 * time.Second):
 		output, _ := io.ReadAll(stderr)
 		_ = cmd.Process.Kill()
 		_ = cmd.Wait()
-		return nil, nil, nil, fmt.Errorf("rsync failed to spawn after 10s (%s)", string(output))
-	}
-
-	return cmd, *conn, stderr, nil
-}
-
-// Send sets up the sending half of an rsync, to recursively send the
-// directory pointed to by path over the websocket.
-func Send(name string, path string, conn io.ReadWriteCloser, tracker *ioprogress.ProgressTracker, features []string, bwlimit string, execPath string, rsyncArgs ...string) error {
-	cmd, netcatConn, stderr, err := sendSetup(name, path, bwlimit, execPath, features, rsyncArgs...)
-	if err != nil {
-		return err
+		return fmt.Errorf("rsync failed to spawn after 10s (%s)", string(output))
 	}
 
 	// Setup progress tracker.
+	netcatConn := *ncConn
 	readNetcatPipe := io.ReadCloser(netcatConn)
 	if tracker != nil {
 		readNetcatPipe = &ioprogress.ProgressReader{

From e1a37a5c240e068ffaff4fe20fa8af612e4143fe Mon Sep 17 00:00:00 2001
From: Max Asnaashari <max.asnaashari@canonical.com>
Date: Wed, 21 Feb 2024 17:52:49 +0000
Subject: [PATCH 2/2] lxd/rsync: Add description of cleanup function to
 RunWrappers

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
(cherry picked from commit ae63aec32fe70c5167d6bfafbc532b2b1286fc09)
---
 lxd/apparmor/rsync.go | 1 +
 lxd/rsync/rsync.go    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lxd/apparmor/rsync.go b/lxd/apparmor/rsync.go
index cae545587fba..25c0e0b3bbb7 100644
--- a/lxd/apparmor/rsync.go
+++ b/lxd/apparmor/rsync.go
@@ -72,6 +72,7 @@ profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) {
 `))
 
 // RsyncWrapper is used as a RunWrapper in the rsync package.
+// It returns a cleanup function that deletes the AppArmor profile that the command is running in.
 func RsyncWrapper(sysOS *sys.OS, cmd *exec.Cmd, sourcePath string, dstPath string) (func(), error) {
 	if !sysOS.AppArmorAvailable {
 		return func() {}, nil
diff --git a/lxd/rsync/rsync.go b/lxd/rsync/rsync.go
index 5e7ab24e0861..2f274fb4d836 100644
--- a/lxd/rsync/rsync.go
+++ b/lxd/rsync/rsync.go
@@ -23,6 +23,7 @@ import (
 var Debug bool
 
 // RunWrapper is an optional function that's used to wrap rsync, useful for confinement like AppArmor.
+// It returns a cleanup function that will close the wrapper's environment, and should be called after the command has completed.
 var RunWrapper func(cmd *exec.Cmd, source string, destination string) (func(), error)
 
 // rsync is a wrapper for the rsync command which will respect RunWrapper.