fix: ensure landscape homedir and broker.bpickle are owned by landsca…

…pe (LP: #2065879) (#264)

debian/landscape-client.postinst

@@ -30,15 +30,15 @@ case "$1" in
         if [ ! -f $CONFIG_FILE ]; then
             # Create new configuration, with private mode
             TEMPFILE=$(mktemp -p /etc/landscape)
-            cat > $TEMPFILE <<END
+            cat > "$TEMPFILE" <<END
 [client]
 log_level = info
 url = https://landscape.canonical.com/message-system
 ping_url = http://landscape.canonical.com/ping
 data_path = /var/lib/landscape/client
 END
-            chown landscape $TEMPFILE
-            mv $TEMPFILE $CONFIG_FILE
+            chown landscape:landscape "$TEMPFILE"
+            mv "$TEMPFILE" "$CONFIG_FILE"
 
             # Get configuration values from debconf
             db_get $PACKAGE/computer_title
@@ -65,7 +65,7 @@ END
             TAGS="${RET}"
 
             # If we got the needed information, actually do the registration.
-            if [ -n "$ACCOUNT_NAME" -a -n "$COMPUTER_TITLE" ]; then
+            if [ -n "$ACCOUNT_NAME" ] && [ -n "$COMPUTER_TITLE" ]; then
                 landscape-config --silent --ok-no-register \
                     --computer-title "$COMPUTER_TITLE" \
                     --account-name "$ACCOUNT_NAME" \
@@ -82,7 +82,7 @@ END
         else
             # Fix non-private permissions
             chmod 0600 $CONFIG_FILE
-            chown landscape $CONFIG_FILE
+            chown landscape:landscape $CONFIG_FILE
         fi
 
         # Remove statoverride for smart-update, if there's one
@@ -101,14 +101,14 @@ END
         # In response to bug 1508110 we need to trigger a complete update of
         # user information.  The flag file will be removed by the client when
         # the update completes.
-        DATA_PATH="`grep ^data_path /etc/landscape/client.conf | cut -d= -f2 | tr -d '[[:space:]]'`"
+        DATA_PATH="$(grep ^data_path /etc/landscape/client.conf | cut -d= -f2 | tr -d '[:space:]')"
         if [ -z "$DATA_PATH" ]; then
             DATA_PATH=/var/lib/landscape/client
         fi
-        install --owner=landscape --directory $DATA_PATH
+        install --owner=landscape --group=landscape --directory "$DATA_PATH"
         USER_UPDATE_FLAG_FILE="$DATA_PATH/user-update-flag"
-        install --owner=landscape /dev/null $USER_UPDATE_FLAG_FILE
-        echo "This file indicates that the Landscape client needs to send updated user information to the server." >> $USER_UPDATE_FLAG_FILE
+        install --owner=landscape --group=landscape /dev/null "$USER_UPDATE_FLAG_FILE"
+        echo "This file indicates that the Landscape client needs to send updated user information to the server." >> "$USER_UPDATE_FLAG_FILE"
 
         # To work around bug #1735100 we rewrite file-local landscape sources
         # with the trusted flag, as they have no release file, thus are
@@ -133,7 +133,7 @@ END
         # Migrate old sysvinit config to systemd
         DEFAULTS=/etc/default/landscape-client
         if [ -f $DEFAULTS ]; then
-            RUN=$(. $DEFAULTS; echo $RUN)
+            RUN=$(. "$DEFAULTS"; echo "$RUN")
             if [ "$RUN" = "1" ]; then
                 systemctl enable landscape-client.service
             fi

landscape/client/configuration.py

@@ -663,6 +663,8 @@ def setup(config) -> Identity:
             config.data_path,
             f"{BrokerService.service_name}.bpickle",
         ),
+        user=USER,
+        group=GROUP,
     )
 
     return Identity(config, persist)
@@ -687,7 +689,7 @@ def restart_client(config):
 def bootstrap_tree(config):
     """Create the client directories tree."""
     bootstrap_list = [
-        BootstrapDirectory("$data_path", USER, "root", 0o755),
+        BootstrapDirectory("$data_path", USER, GROUP, 0o755),
         BootstrapDirectory("$annotations_path", USER, GROUP, 0o755),
     ]
     BootstrapList(bootstrap_list).bootstrap(
@@ -760,7 +762,7 @@ def is_registered(config):
         config.data_path,
         f"{BrokerService.service_name}.bpickle",
     )
-    persist = Persist(filename=persist_filename)
+    persist = Persist(filename=persist_filename, user=USER, group=GROUP)
     identity = Identity(config, persist)
     return bool(identity.secure_id)
 
@@ -798,6 +800,8 @@ def set_secure_id(config, new_id):
             config.data_path,
             f"{BrokerService.service_name}.bpickle",
         ),
+        user=USER,
+        group=GROUP,
     )
     identity = Identity(config, persist)
     identity.secure_id = new_id
@@ -810,6 +814,8 @@ def get_secure_id(config):
             config.data_path,
             f"{BrokerService.service_name}.bpickle",
         ),
+        user=USER,
+        group=GROUP,
     )
     identity = Identity(config, persist)
     return identity.secure_id

landscape/client/deployment.py

@@ -13,6 +13,8 @@
 
 from landscape import VERSION
 from landscape.client import DEFAULT_CONFIG
+from landscape.client import GROUP
+from landscape.client import USER
 from landscape.client import snap_http
 from landscape.client.snap_utils import get_snap_info
 from landscape.client.upgraders import UPGRADE_MANAGERS
@@ -239,7 +241,11 @@ def get_versioned_persist(service):
     Load a L{Persist} database for the given C{service} and upgrade or
     mark as current, as necessary.
     """
-    persist = Persist(filename=service.persist_filename)
+    persist = Persist(
+        filename=service.persist_filename,
+        user=USER,
+        group=GROUP,
+    )
     upgrade_manager = UPGRADE_MANAGERS[service.service_name]
     if os.path.exists(service.persist_filename):
         upgrade_manager.apply(persist)

landscape/client/manager/keystonetoken.py

@@ -1,6 +1,8 @@
 import logging
 import os
 
+from landscape.client import GROUP
+from landscape.client import USER
 from landscape.client.monitor.plugin import DataWatcher
 from landscape.lib.compat import _PY3
 from landscape.lib.compat import ConfigParser
@@ -32,7 +34,11 @@ def register(self, client):
             self.registry.config.data_path,
             "keystone.bpickle",
         )
-        self._persist = Persist(filename=self._persist_filename)
+        self._persist = Persist(
+            filename=self._persist_filename,
+            user=USER,
+            group=GROUP,
+        )
         self.registry.reactor.call_every(
             self.registry.config.flush_interval,
             self.flush,

landscape/client/manager/plugin.py

@@ -4,6 +4,8 @@
 
 from twisted.internet.defer import maybeDeferred
 
+from landscape.client import GROUP
+from landscape.client import USER
 from landscape.client.broker.client import BrokerClientPlugin
 from landscape.lib.format import format_object
 from landscape.lib.log import log_failure
@@ -95,7 +97,11 @@ def register(self, registry):
             self.registry.config.data_path,
             self.message_type + '.manager.bpkl',
         )
-        self._persist = Persist(filename=self._persist_filename)
+        self._persist = Persist(
+            filename=self._persist_filename,
+            user=USER,
+            group=GROUP
+        )
         self.call_on_accepted(self.message_type, self.send_message)
 
     def run(self):

landscape/client/manager/scriptexecution.py

@@ -18,7 +18,9 @@
 from twisted.python.compat import unicode
 
 from landscape import VERSION
+from landscape.client import GROUP
 from landscape.client import IS_SNAP
+from landscape.client import USER
 from landscape.client.manager.plugin import FAILED
 from landscape.client.manager.plugin import ManagerPlugin
 from landscape.client.manager.plugin import SUCCEEDED
@@ -319,6 +321,8 @@ def run_script(
                     self.registry.config.data_path,
                     "broker.bpickle",
                 ),
+                user=USER,
+                group=GROUP,
             )
             persist = persist.root_at("registration")
             computer_id = persist.get("secure-id")

landscape/client/manager/snapmanager.py

@@ -5,7 +5,9 @@
 
 from twisted.internet import task
 
+from landscape.client import GROUP
 from landscape.client import snap_http
+from landscape.client import USER
 from landscape.client.manager.plugin import FAILED
 from landscape.client.manager.plugin import ManagerPlugin
 from landscape.client.manager.plugin import SUCCEEDED
@@ -270,7 +272,11 @@ def register(self, registry):
             self.registry.config.data_path,
             "snaps.bpickle",
         )
-        self._persist = Persist(filename=self._persist_filename)
+        self._persist = Persist(
+            filename=self._persist_filename,
+            user=USER,
+            group=GROUP,
+        )
         self.call_on_accepted(self.message_type, self._send_snap_update)
 
         registry.register_message("install-snaps", self._handle_snap_task)

landscape/client/tests/test_configuration.py

@@ -2349,6 +2349,8 @@ def test_function(self, Identity, Persist):
 
         Persist.assert_called_once_with(
             filename="/tmp/landscape/broker.bpickle",
+            user="landscape",
+            group="landscape",
         )
         Persist().save.assert_called_once_with()
         Identity.assert_called_once_with(config, Persist())

landscape/client/watchdog.py

@@ -26,6 +26,7 @@
 from twisted.internet.protocol import ProcessProtocol
 
 from landscape.client import IS_SNAP
+from landscape.client import GROUP
 from landscape.client import USER
 from landscape.client.broker.amp import RemoteBrokerConnector
 from landscape.client.broker.amp import RemoteManagerConnector
@@ -651,26 +652,26 @@ def _remove_pid(self):
 
 bootstrap_list = BootstrapList(
     [
-        BootstrapDirectory("$data_path", USER, "root", 0o755),
-        BootstrapDirectory("$data_path/package", USER, "root", 0o755),
-        BootstrapDirectory("$data_path/package/hash-id", USER, "root", 0o755),
-        BootstrapDirectory("$data_path/package/binaries", USER, "root", 0o755),
+        BootstrapDirectory("$data_path", USER, GROUP, 0o755),
+        BootstrapDirectory("$data_path/package", USER, GROUP, 0o755),
+        BootstrapDirectory("$data_path/package/hash-id", USER, GROUP, 0o755),
+        BootstrapDirectory("$data_path/package/binaries", USER, GROUP, 0o755),
         BootstrapDirectory(
             "$data_path/package/upgrade-tool",
             USER,
             "root",
             0o755,
         ),
-        BootstrapDirectory("$data_path/messages", USER, "root", 0o755),
-        BootstrapDirectory("$data_path/sockets", USER, "root", 0o750),
+        BootstrapDirectory("$data_path/messages", USER, GROUP, 0o755),
+        BootstrapDirectory("$data_path/sockets", USER, GROUP, 0o750),
         BootstrapDirectory(
             "$data_path/custom-graph-scripts",
             USER,
-            "root",
+            GROUP,
             0o755,
         ),
-        BootstrapDirectory("$log_dir", USER, "root", 0o755),
-        BootstrapFile("$data_path/package/database", USER, "root", 0o644),
+        BootstrapDirectory("$log_dir", USER, GROUP, 0o755),
+        BootstrapFile("$data_path/package/database", USER, GROUP, 0o644),
     ],
 )
 

landscape/lib/persist.py

@@ -21,6 +21,7 @@
 import copy
 import os
 import re
+import shutil
 import sys
 
 from twisted.python.compat import StringType  # Py2: basestring, Py3: str
@@ -67,7 +68,7 @@ class Persist:
 
     """
 
-    def __init__(self, backend=None, filename=None):
+    def __init__(self, backend=None, filename=None, user=None, group=None):
         """
         @param backend: The backend to use. If none is specified,
             L{BPickleBackend} will be used.
@@ -85,6 +86,8 @@ def __init__(self, backend=None, filename=None):
         self._readonly = False
         self._modified = False
         self._config = self
+        self._user = user
+        self._group = group
         self.filename = filename
         if filename is not None and os.path.exists(filename):
             self.load(filename)
@@ -170,6 +173,17 @@ def save(self, filepath=None):
             os.makedirs(dirname)
         self._backend.save(filepath, self._hardmap)
 
+        if self._user is not None or self._group is not None:
+            try:
+                if dirname:
+                    shutil.chown(dirname, user=self._user, group=self._group)
+                shutil.chown(filepath, user=self._user, group=self._group)
+            except PermissionError:
+                # A perist directory has been selected that can't be owned by
+                # landscape:landscape. This often happens in /tmp for tests,
+                # but there could be other reasons. Just leave it be.
+                pass
+
     def _traverse(self, obj, path, default=NOTHING, setvalue=NOTHING):
         if setvalue is not NOTHING:
             setvalue = self._backend.copy(setvalue)
@@ -642,6 +656,3 @@ def load(self, filepath):
     def save(self, filepath, map):
         with open(filepath, "wb") as fd:
             fd.write(self._bpickle.dumps(map))
-
-
-# vim:ts=4:sw=4:et

landscape/lib/tests/test_persist.py

@@ -355,7 +355,7 @@ def test_save_and_load(self):
             self.format(result, self.set_result),
         )
 
-    def test_save_on_unexistent_dir(self):
+    def test_save_on_nonexistent_dir(self):
         dirname = self.makePersistFile()
         filename = os.path.join(dirname, "foobar")