launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1042865 into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1042865 in MAAS: "maas-import-pxe-files sets incorrect permissions for commissioning dir"
  https://bugs.launchpad.net/maas/+bug/1042865

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1042865/+merge/121990

If the import scripts run with an overly strict umask, maas-import-ephemerals may produce a "commissioning" image directory that the TFTP server is not allowed to read.

The reason seems to be that the download code creates a temporary directory to download in, then sets a more permissive umask, and finally calls into maas-provision to install the new image — which copies the temporary directory including its permissions from the time with the stricter umask.

In this branch I address that in two ways:

1. The umask really didn't belong in the inner loop, since it affects global script state.  I hoisted it up to a global level, so that the temporary directory is created with the new umask.

2. Before copying the image, I make it world-readable.  The "X" flag on chmod pertains to the "search" permission.  It sets the "x" bit on directories ("search") but not on files ("execute").

One open question is what happens to systems that had a non-world-readable directory created by an import script run from an older maas version.  That might be worth fixing up in postinst for now.  We don't need to carry it around forever: a permissions problem with the image go away as soon as the script downloads the first image update.  (Also, chmod -R a+rX is much easier to do in shell than in python.)

Sadly, the ephemerals import script has not been made testable and is not tested.  I can't even run it locally.  We'll have to see in Q/A whether this really fixes the problem.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1042865/+merge/121990
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/bug-1042865 into lp:maas.

=== modified file 'scripts/maas-import-ephemerals'
--- scripts/maas-import-ephemerals	2012-08-15 12:54:49 +0000
+++ scripts/maas-import-ephemerals	2012-08-30 06:22:19 +0000
@@ -295,10 +295,6 @@
         cp -- "$src/$filename" "$tmpdir/"
     done
 
-    # All files we create here are public.  The TFTP user will need to be
-    # able to read them.
-    umask a+r
-
     debug 1 "maas-provision install-pxe-image --arch=$arch --subarch=$subarch --release=$release --purpose=commissioning --image=$tmpdir"
     maas-provision install-pxe-image \
         --arch=$arch --subarch=$subarch --release=$release \
@@ -346,6 +342,12 @@
 EOF
 fi
 
+
+# All files we create here are public.  The TFTP user will need to be
+# able to read them.
+umask a+r
+
+
 updates=0
 for release in $RELEASES; do
     for arch in $ARCHES; do
@@ -372,6 +374,7 @@
                 "$r_serial" "$r_arch" "$r_url" "$r_name" ||
                 fail "failed to prepare image for $release/$arch"
 
+            chmod -R a+rX "$wd"
             install_tftp_image "$wd" "$r_arch" "generic" "$r_release"
 
             target_name="${TARGET_NAME_PREFIX}${r_name}"