machine-id-setup: rework writing of /etc/machine-id around chase()

Let's make sure we properly stay inside of the root fs if one is
provided, i.e. stop using prefix_roota() at one more place.

diff --git a/src/shared/machine-id-setup.c b/src/shared/machine-id-setup.c
index 0659bced1277d341d0bcf0f8f9d80abcaf632e68..ee7e08b086a834588bdb9f29a909f3331c6c3159 100644 (file)
--- a/src/shared/machine-id-setup.c
+++ b/src/shared/machine-id-setup.c
@@ -131,54 +131,89 @@ static int acquire_machine_id(const char *root, bool machine_id_from_firmware, s
 }
 
 int machine_id_setup(const char *root, sd_id128_t machine_id, MachineIdSetupFlags flags, sd_id128_t *ret) {
-        const char *etc_machine_id, *run_machine_id;
-        _cleanup_close_ int fd = -EBADF;
+        _cleanup_free_ char *etc_machine_id = NULL, *run_machine_id = NULL;
         bool writable, write_run_machine_id = true;
+        _cleanup_close_ int fd = -EBADF, run_fd = -EBADF;
+        bool unlink_run_machine_id = false;
         int r;
 
-        etc_machine_id = prefix_roota(root, "/etc/machine-id");
-
         WITH_UMASK(0000) {
-                /* We create this 0444, to indicate that this isn't really
-                 * something you should ever modify. Of course, since the file
-                 * will be owned by root it doesn't matter much, but maybe
-                 * people look. */
+                _cleanup_close_ int inode_fd = -EBADF;
 
-                (void) mkdir_parents(etc_machine_id, 0755);
-                fd = open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444);
-                if (fd < 0) {
-                        int old_errno = errno;
+                r = chase("/etc/machine-id", root, CHASE_PREFIX_ROOT, &etc_machine_id, &inode_fd);
+                if (r == -ENOENT) {
+                        _cleanup_close_ int etc_fd = -EBADF;
+                        _cleanup_free_ char *etc = NULL;
+
+                        r = chase("/etc/", root, CHASE_PREFIX_ROOT|CHASE_MKDIR_0755, &etc, &etc_fd);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to open '/etc/': %m");
 
-                        fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY);
+                        etc_machine_id = path_join(etc, "machine-id");
+                        if (!etc_machine_id)
+                                return log_oom();
+
+                        /* We create this 0444, to indicate that this isn't really something you should ever
+                         * modify. Of course, since the file will be owned by root it doesn't matter much, but maybe
+                         * people look. */
+
+                        fd = openat(etc_fd, "machine-id", O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW|O_CLOEXEC, 0444);
                         if (fd < 0) {
-                                if (old_errno == EROFS && errno == ENOENT)
+                                if (errno == EROFS)
                                         return log_error_errno(errno,
-                                                  "System cannot boot: Missing /etc/machine-id and /etc is mounted read-only.\n"
-                                                  "Booting up is supported only when:\n"
-                                                  "1) /etc/machine-id exists and is populated.\n"
-                                                  "2) /etc/machine-id exists and is empty.\n"
-                                                  "3) /etc/machine-id is missing and /etc is writable.\n");
-                                else
-                                        return log_error_errno(errno, "Cannot open %s: %m", etc_machine_id);
+                                                               "System cannot boot: Missing %s and %s/ is read-only.\n"
+                                                               "Booting up is supported only when:\n"
+                                                               "1) /etc/machine-id exists and is populated.\n"
+                                                               "2) /etc/machine-id exists and is empty.\n"
+                                                               "3) /etc/machine-id is missing and /etc/ is writable.",
+                                                               etc_machine_id,
+                                                               etc);
+
+                                return log_error_errno(errno, "Cannot create '%s': %m", etc_machine_id);
                         }
 
-                        writable = false;
-                } else
+                        log_debug("Successfully opened new '%s' file.", etc_machine_id);
                         writable = true;
+                } else if (r < 0)
+                        return log_error_errno(r, "Cannot open '/etc/machine-id': %m");
+                else {
+                        /* We pinned the inode, now try to convert it into a writable file */
+
+                        fd = xopenat_full(inode_fd, /* path= */ NULL, O_RDWR|O_CLOEXEC, XO_REGULAR, 0444);
+                        if (fd < 0) {
+                                log_debug_errno(fd, "Failed to topen '%s' in writable mode, retrying in read-only mode: %m", etc_machine_id);
+
+                                /* If that didn't work, convert it into a readable file */
+                                fd = xopenat_full(inode_fd, /* path= */ NULL, O_RDONLY|O_CLOEXEC, XO_REGULAR, MODE_INVALID);
+                                if (fd < 0)
+                                        return log_error_errno(fd, "Cannot open '%s' in neither writable nor read-only mode: %m", etc_machine_id);
+
+                                log_debug("Successfully opened existing '%s' file in read-only mode.", etc_machine_id);
+                                writable = false;
+                        } else {
+                                log_debug("Successfully opened existing '%s' file in writable mode.", etc_machine_id);
+                                writable = true;
+                        }
+                }
         }
 
         /* A we got a valid machine ID argument, that's what counts */
         if (sd_id128_is_null(machine_id) || FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_FIRMWARE)) {
 
                 /* Try to read any existing machine ID */
-                if (id128_read_fd(fd, ID128_FORMAT_PLAIN, &machine_id) >= 0)
+                r = id128_read_fd(fd, ID128_FORMAT_PLAIN, &machine_id);
+                if (r >= 0)
                         goto finish;
 
+                log_debug_errno(r, "Unable to read current machine ID, acquiring new one: %m");
+
                 /* Hmm, so, the id currently stored is not useful, then let's acquire one. */
                 r = acquire_machine_id(root, FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_FIRMWARE), &machine_id);
                 if (r < 0)
                         return r;
-                write_run_machine_id = !r;
+
+                write_run_machine_id = !r; /* acquire_machine_id() returns 1 in case we read this machine ID
+                                            * from /run/machine-id */
         }
 
         if (writable) {
@@ -209,37 +244,49 @@ int machine_id_setup(const char *root, sd_id128_t machine_id, MachineIdSetupFlag
                 }
         }
 
-        fd = safe_close(fd);
+        /* Hmm, we couldn't or shouldn't write the machine-id to /etc/? So let's write it to /run/machine-id
+         * as a replacement */
 
-        /* Hmm, we couldn't or shouldn't write the machine-id to /etc?
-         * So let's write it to /run/machine-id as a replacement */
+        if (write_run_machine_id) {
+                _cleanup_free_ char *run = NULL;
 
-        run_machine_id = prefix_roota(root, "/run/machine-id");
+                r = chase("/run/", root, CHASE_PREFIX_ROOT|CHASE_MKDIR_0755, &run, &run_fd);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to open '/run/': %m");
 
-        if (write_run_machine_id) {
-                WITH_UMASK(0022)
-                        r = id128_write(run_machine_id, ID128_FORMAT_PLAIN, machine_id);
-                if (r < 0) {
-                        (void) unlink(run_machine_id);
-                        return log_error_errno(r, "Cannot write %s: %m", run_machine_id);
+                run_machine_id = path_join(run, "machine-id");
+                if (!run_machine_id)
+                        return log_oom();
+
+                WITH_UMASK(0022) {
+                        r = id128_write_at(run_fd, "machine-id", ID128_FORMAT_PLAIN, machine_id);
+                        if (r < 0)
+                                return log_error_errno(r, "Cannot write '%s': %m", run_machine_id);
                 }
+
+                unlink_run_machine_id = true;
+        } else {
+                r = chase("/run/machine-id", root, CHASE_PREFIX_ROOT, &run_machine_id, /* ret_inode_fd= */ NULL);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to open '/run/machine-id': %m");
         }
 
         /* And now, let's mount it over */
-        r = mount_follow_verbose(LOG_ERR, run_machine_id, etc_machine_id, NULL, MS_BIND, NULL);
-        if (r < 0) {
-                (void) unlink(run_machine_id);
+        r = mount_follow_verbose(LOG_ERR, run_machine_id, FORMAT_PROC_FD_PATH(fd), /* fstype= */ NULL, MS_BIND, /* options= */ NULL);
+        if (r < 0)
                 return r;
-        }
 
-        log_full(FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_TRANSIENT) ? LOG_DEBUG : LOG_INFO, "Installed transient %s file.", etc_machine_id);
+        log_full(FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_TRANSIENT) ? LOG_DEBUG : LOG_INFO, "Installed transient '%s' file.", etc_machine_id);
 
-        /* Mark the mount read-only */
-        r = mount_follow_verbose(LOG_WARNING, NULL, etc_machine_id, NULL, MS_BIND|MS_RDONLY|MS_REMOUNT, NULL);
-        if (r < 0)
-                return r;
+        unlink_run_machine_id = false;
+
+        /* Mark the mount read-only (note: we are not going via FORMAT_PROC_FD_PATH() here because that fd is not updated to our new bind mount) */
+        (void) mount_follow_verbose(LOG_WARNING, /* source= */ NULL, etc_machine_id, /* fstype= */ NULL, MS_BIND|MS_RDONLY|MS_REMOUNT, /* options= */ NULL);
 
 finish:
+        if (unlink_run_machine_id)
+                (void) unlinkat(ASSERT_FD(run_fd), "machine-id", /* flags= */ 0);
+
         if (!in_initrd())
                 (void) sd_notifyf(/* unset_environment= */ false, "X_SYSTEMD_MACHINE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(machine_id));