Merge pull request #3366 from cgwalters/unconditional-cfs

docs/composefs.md

@@ -21,22 +21,20 @@ At the current time, integration of composefs and ostree is experimental.
 
 ### Enabling composefs (unsigned)
 
-When building a disk image *or* to transition an existing system, run:
+If ostree is compiled with composefs support, then a composefs file
+corresponding to the deployment tree will be generated by default.
+
+The `ostree-prepare-root` binary will look for `ostree/prepare-root.conf` in `/etc` and
+`/usr/lib` in the initramfs. Using that configuration file you can enable composefs.
+This configuration will enable an "unsigned" mode, which does not require fsverity,
+but does make the system more resilient to accidental mutation.
 
 ```
-ostree config --repo=/ostree/repo set ex-integrity.composefs true
+[composefs]
+enabled = yes
 ```
 
-This will ensure that any future deployments (e.g. created by `ostree admin upgrade`)
-have a `.ostree.cfs` file in the deployment directory which is a mountable
-composefs metadata file, with a "backing store" directory that is
-shared with the current `/ostree/repo/objects`.
-
-### composefs configuration
-
-The `ostree-prepare-root` binary will look for `ostree/prepare-root.conf` in `/etc` and
-`/usr/lib` in the initramfs. Using that configuration file you can enable composefs,
-and specify an Ed25519 public key to validate the booted commit.
+You can also specify an Ed25519 public key to validate the booted commit.
 
 See the manpage for `ostree-prepare-root` for details of how to configure it.
 

man/ostree-prepare-root.xml

@@ -151,7 +151,12 @@ License along with this library. If not, see <https://www.gnu.org/licenses/>.
                 the integrity of its backing OSTree object is validated by the digest stored in the image.
                 Additionally, if set to <literal>signed</literal>, boot will fail if the image cannot be
                 validated by a public key.
-                Setting this to <literal>maybe</literal> is currently equivalent to <literal>no</literal>.
+                Setting this to <literal>maybe</literal> will cause composefs to be used at runtime only
+                if the deployment has a composefs generated, which causes unpredicable and confusing semantics
+                and is not recommended. In practice with the <emphasis>current</emphasis> version of ostree,
+                in the case where composefs is enabled at build time for both the version that made the
+                deployment (often an older OS version), this will be equivalent to <literal>yes</literal>.
+                But in general one either wants composefs or not, so choose an explicit value for that.
                 </para></listitem>
             </varlistentry>
             <varlistentry>

src/libostree/ostree-sysroot-deploy.c

@@ -669,33 +669,28 @@ checkout_deployment_tree (OstreeSysroot *sysroot, OstreeRepo *repo, OstreeDeploy
   guint64 composefs_start_time = 0;
   guint64 composefs_end_time = 0;
 #ifdef HAVE_COMPOSEFS
-  if (composefs_enabled != OT_TRISTATE_NO)
-    {
-      composefs_start_time = g_get_monotonic_time ();
-      // TODO: Clean up our mess around composefs/fsverity...we have duplication
-      // between the repo config and the sysroot config, *and* we need to better
-      // handle skew between repo config and repo state (e.g. "post-copy" should
-      // support transitioning verity on and off in general).
-      // For now we configure things such that the fsverity digest is only added
-      // if present on disk in the unsigned case, and in the signed case unconditionally
-      // require it.
-      g_auto (GVariantBuilder) cfs_checkout_opts_builder
-          = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
-      guint32 composefs_requested = 1;
-      if (composefs_config->require_verity)
-        composefs_requested = 2;
-      g_variant_builder_add (&cfs_checkout_opts_builder, "{sv}", "verity",
-                             g_variant_new_uint32 (composefs_requested));
-      g_debug ("composefs requested: %u", composefs_requested);
-      g_autoptr (GVariant) cfs_checkout_opts
-          = g_variant_ref_sink (g_variant_builder_end (&cfs_checkout_opts_builder));
-      if (!ostree_repo_checkout_composefs (repo, cfs_checkout_opts, ret_deployment_dfd,
-                                           OSTREE_COMPOSEFS_NAME, csum, cancellable, error))
-        return FALSE;
-      composefs_end_time = g_get_monotonic_time ();
-    }
-  else
-    g_debug ("not using composefs");
+  composefs_start_time = g_get_monotonic_time ();
+  // TODO: Clean up our mess around composefs/fsverity...we have duplication
+  // between the repo config and the sysroot config, *and* we need to better
+  // handle skew between repo config and repo state (e.g. "post-copy" should
+  // support transitioning verity on and off in general).
+  // For now we configure things such that the fsverity digest is only added
+  // if present on disk in the unsigned case, and in the signed case unconditionally
+  // require it.
+  g_auto (GVariantBuilder) cfs_checkout_opts_builder
+      = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT);
+  guint32 composefs_requested = 1;
+  if (composefs_config->require_verity)
+    composefs_requested = 2;
+  g_variant_builder_add (&cfs_checkout_opts_builder, "{sv}", "verity",
+                         g_variant_new_uint32 (composefs_requested));
+  g_debug ("composefs requested: %u", composefs_requested);
+  g_autoptr (GVariant) cfs_checkout_opts
+      = g_variant_ref_sink (g_variant_builder_end (&cfs_checkout_opts_builder));
+  if (!ostree_repo_checkout_composefs (repo, cfs_checkout_opts, ret_deployment_dfd,
+                                       OSTREE_COMPOSEFS_NAME, csum, cancellable, error))
+    return FALSE;
+  composefs_end_time = g_get_monotonic_time ();
 #else
   if (composefs_enabled == OT_TRISTATE_YES)
     return glnx_throw (error, "composefs: enabled at runtime, but support is not compiled in");

src/libotcore/otcore-prepare-root.c

@@ -188,8 +188,8 @@ otcore_load_composefs_config (const char *cmdline, GKeyFile *config, gboolean lo
       ret->is_signed = false;
     }
   else if (!ot_keyfile_get_tristate_with_default (config, OTCORE_PREPARE_ROOT_COMPOSEFS_KEY,
-                                                  OTCORE_PREPARE_ROOT_ENABLED_KEY,
-                                                  OT_TRISTATE_MAYBE, &ret->enabled, error))
+                                                  OTCORE_PREPARE_ROOT_ENABLED_KEY, OT_TRISTATE_NO,
+                                                  &ret->enabled, error))
     return NULL;
 
   // Look for a key - we default to the initramfs binding path.

tests/test-admin-deploy-composefs.sh

@@ -38,10 +38,10 @@ cd -
 ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string version=1.composefs -b testos/buildmain/x86_64-runtime osdata
 ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmain/x86_64-runtime
 
+# We generate the blob now, even if it's explicitly runtime disabled
 ${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=LABEL=foo --karg=testkarg=1 testos:testos/buildmain/x86_64-runtime
-if test -f sysroot/ostree/deploy/testos/deploy/*.0/.ostree.cfs; then
-    fatal "found composefs unexpectedly"
-fi
+cfs_count=$(ls sysroot/ostree/deploy/testos/deploy/*.0/.ostree.cfs | wc -l)
+assert_streq "${cfs_count}" "1"
 
 # check explicit enablement
 cd osdata
@@ -55,7 +55,8 @@ ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-str
 ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmain/x86_64-runtime
 
 ${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=LABEL=foo --karg=testkarg=1 testos:testos/buildmain/x86_64-runtime
-ls sysroot/ostree/deploy/testos/deploy/*.0/.ostree.cfs
+cfs_count=$(ls sysroot/ostree/deploy/testos/deploy/*.0/.ostree.cfs | wc -l)
+assert_streq "${cfs_count}" "2"
 
 tap_ok composefs