Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin: Add an unlock command #212

Closed
wants to merge 8 commits into from

Conversation

cgwalters
Copy link
Member

I'm trying to improve the developer experience on OSTree-managed
systems, and I had an epiphany (:thought_balloon:) the other day - there's no reason we
have to be absolutely against mutating the current rootfs live. The
key should be making it easy to rollback/reset to a known good state.

This new unlock command implements that by first "dup"ing the
current deployment, so you go back to it when rebooting (or with
--hotfix, have it as a rollback target).

Next, we use rofiles-fuse to point the mutable
/sysroot/ostree/deploy/.../${thisdeployment}/usr to /usr. We
choose /usr since this way we avoid the immutable bit and pretty
much everything is in /usr anyways.

And then everything immediately explodes because the default (at least
CentOS 7) SELinux policy denies tons of things (including sshd_t
access to fusefs_t). Sigh.

I'm not sure...we could try to load a policy module that allowed this?
Alternatively...we could investigate overlayfs except that itself is
currently known to be incompatible with SELinux too. Except we don't
need copy-up...

@cgwalters
Copy link
Member Author

FWIW I tested this on a current CentOS7 AH by copying /usr/lib64/libfuse* from my host into the target (underneath the writable $(ostree admin --print-current-dir)/lib64), copying the built .libs/ostree binary into ~, then running ~ostree admin unlock.

You will also need to:

  • setenforce 0
  • Edit /etc/fuse.conf and add user_allow_other

@cgwalters
Copy link
Member Author

There is another option - we could provide a gated tool like:

ostree admin unlock push-tree /path/to/fstree

which would ensure correct overwrites. It could even take .tar.gz files (and for that matter .rpm) and use libarchive to unpack them.

@cgwalters
Copy link
Member Author

Okay, see the above two commits. Right now overlayfs seems like a win, but I want to do more testing before I'm confident in it.


<listitem><para>
OSTree follows a default policy of retaining at most two deployments
per OS. This option suppresses the default collection of any previous
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default collection --> removal?
Ahh, or is it missing the word garbage?

I'm trying to improve the developer experience on OSTree-managed
systems, and I had an epiphany the other day - there's no reason we
have to be absolutely against mutating the current rootfs live.  The
key should be making it easy to rollback/reset to a known good state.

I see this command as useful for two related but distinct workflows:

 - `ostree admin unlock` will assume you're doing "development".  The
   semantics hare are that we mount an overlayfs on `/usr`, but the
   overlay data is in `/var/tmp`, and is thus discarded on reboot.
 - `ostree admin unlock --hotfix` first clones your current deployment,
   then creates an overlayfs over `/usr` persistent
   to this deployment.  Persistent in that now the initramfs switchroot
   tool knows how to mount it as well.  In this model, if you want
   to discard the hotfix, at the moment you roll back/reboot into
   the clone.

Note originally, I tried using `rofiles-fuse` over `/usr` for this,
but then everything immediately explodes because the default (at least
CentOS 7) SELinux policy denies tons of things (including `sshd_t`
access to `fusefs_t`).  Sigh.

So the switch to `overlayfs` came after experimentation.  It still
seems to have some issues...specifically `unix_chkpwd` is broken,
possibly because it's setuid?  Basically I can't ssh in anymore.

But I *can* `rpm -Uvh strace.rpm` which is handy.

NOTE: I haven't tested the hotfix path fully yet, specifically
the initramfs bits.
@cgwalters
Copy link
Member Author

Okay, squashed, and major changes in ⬆️

@cgwalters
Copy link
Member Author

One thing I'm uncertain about...should ostree admin upgrade require the user to specify a flag like --discard-hotfixes ?

* think we could do this a lot earlier and make all of the mounts
* here just be relative.
*/
orig_cwd_dfd = openat (AT_FDCWD, ".", O_RDONLY | O_NONBLOCK | O_DIRECTORY | O_CLOEXEC | O_NOCTTY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing declaration for orig_cwd_dfd

@giuseppe
Copy link
Member

I tried to make a new Rawhide tree with the updated OSTree (after adding the declaration for orig_cwd_dfd) and I get this error when it boots:

Failed to switch root: Specified root path /sysroot does not seem to be an OS tree. os-release file is missing.

@cgwalters
Copy link
Member Author

Blah, was accidentally building without dracut. Will fix.

@cgwalters
Copy link
Member Author

@giuseppe I suspect that error was actually coreos/rpm-ostree#243

if (fchdir (child_deployment_dfd) < 0)
exit (EXIT_FAILURE);
(void) close (child_deployment_dfd);
if (mount ("overlay", "/usr", "overlay", 0, ovl_options) < 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC complains about ovl_options here (-Wmaybe-uninitialized). Easy fix:

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index d4842fe..846e522 100644
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -1718,9 +1718,6 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,

   switch (unlocked_state)
     {
-    case OSTREE_DEPLOYMENT_UNLOCKED_NONE:
-      g_assert_not_reached ();
-      break;
     case OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX:
       {
         /* Create the overlayfs directories in the deployment root
@@ -1755,6 +1752,9 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
         ovl_options = glnx_strjoina ("lowerdir=usr,upperdir=", development_ovl_upper,
                                      ",workdir=", development_ovl_work);
       }
+    default:
+      g_assert_not_reached ();
+      break;
     }

   /* Here we run `mount()` in a fork()ed child because we need to use

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but then if we later add a new enum member we lose the warning that we're not handling an enum member in switch.

It's too bad gcc can't prove it's initialized here, because it is. But that gets into:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501

Anyways I pushed a fixup which initializes to NULL then tells gcc it's not-NULL after the switch.

@cgwalters
Copy link
Member Author

That, and a pile of fixups for the "hotfix" code path. You can see all of my bugs individually!

The other thing is I added some bits to drop unlocked=hotfix from the origin on upgrade silently, but I'm wavering and considering making it an error, i.e. you have to rollback. Except that potentially reintroduces the security issue transiently...so we'll need to either drop it silently or add a cmdline option.

@jlebon
Copy link
Member

jlebon commented Mar 22, 2016

Hmm, don't feel strongly either way. I guess I'd vote for the cmdline option so that admins are more conscious of what will happen.

Will give this PR a spin now! This should make testing my package-layering stuff much less painful.

@jlebon
Copy link
Member

jlebon commented Mar 22, 2016

Hmm, I can't get ostree-prepare-root to play nice. It finds the overlay and mounts it, but then everything dies (got a couple dozen services that hang to start). Still investigating.

@cgwalters
Copy link
Member Author

Hum. I'm testing on RHEL7.2 for what it's worth. Are you using Fedora?

@jlebon
Copy link
Member

jlebon commented Mar 22, 2016

Yes, F23. Testing it the proper way now (I was directly overwriting ostree files 😇 ).

@jlebon
Copy link
Member

jlebon commented Mar 22, 2016

Sweet, yes it works.

Two things:

  • It'd be great if ostree-prepare-root could output one line to mention that it found an overlayfs to mount.
  • ostree admin status still shows Unlocked: hotfix after reboot. I understand why that is, but I wonder if we should offer a distinction in the output of ostree admin status between "it's unlocked right now and you can edit stuff" (e.g. "Unlocked: hotfix") and "we're currently running on an overlay of a previously unlocked session" (e.g. "Locked: hotfix" ?).

Will test this more tomorrow, but I really like it so far.

@jlebon
Copy link
Member

jlebon commented Mar 22, 2016

Or maybe have the Version string be e.g. $BASE_VER (hotfix). So then the Unlocked line is only shown when you're currently editing /usr.

@cgwalters
Copy link
Member Author

I'm not sure what you mean - the overlayfs is mounted and writable after reboot, no? Or are you saying it shouldn't be? I could see that...but allowing "relocking" would introduce a new state into the system...

@jlebon
Copy link
Member

jlebon commented Mar 23, 2016

I'm not sure what you mean - the overlayfs is mounted and writable after reboot, no?

Ahh OK, I missed that. Yeah, I think it's fine that way. The simpler the better.

BTW, I really like the push-tree idea you mentioned earlier, although that was before you switched over to overlayfs. Still, something like that would probably make life easier for admins. E.g. then it becomes a single ansible task like ostree admin unlock --hotfix-file=/tmp/hotfix.tar --reboot. WDYT?

@cgwalters
Copy link
Member Author

Maybe...I think at the moment I really want to get the non-hotfix admin unlock out to make it easier for people to hack. The --hotfix path I was thinking of as a bonus since it wasn't too much harder to do. We can always revisit with improvements here.

I suspect for most people the biggest win is a persistent package layering, since you often want something smarter than tarballs for software delivery.

@jlebon
Copy link
Member

jlebon commented Mar 23, 2016

We can always revisit with improvements here.

Agreed! I think both @giuseppe and I gave a 👍 for this during today's meeting. Just putting it here for the record. :)

@cgwalters
Copy link
Member Author

Awesome, merged to master. Thanks for the review and testing!

@cgwalters cgwalters closed this Mar 23, 2016
@cgwalters
Copy link
Member Author

Though I should drop in here that I still need to debug "Specifically unix_chkpwd is broken,
possibly because it's setuid? Basically I can't ssh in anymore."

rfairley pushed a commit to rfairley/ostree that referenced this pull request Apr 17, 2019
travis: Disable email notifications
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants