make_dev_ssd: more sanity checks

scripts/image_signing/make_dev_ssd.sh

Index: scripts/image_signing/make_dev_ssd.sh
diff --git a/scripts/image_signing/make_dev_ssd.sh b/scripts/image_signing/make_dev_ssd.sh
index 9a645deb5e46d8c0d1831d3e8321729a84582783..93533eff0a57bdd4e12adae9778ea7d9df9dee3c 100755
--- a/scripts/image_signing/make_dev_ssd.sh
+++ b/scripts/image_signing/make_dev_ssd.sh
@@ -5,7 +5,7 @@
 # found in the LICENSE file.
 #
 # This script can change key (usually developer keys) and kernel config
-# of a kernels on SSD.
+# of a kernels on an disk image (usually for SSD but also works for USB).

[Randall Spangler, 2011/04/19 20:40:12]

'a kernels' -> 'kernels'

[Hung-Te, 2011/04/20 01:30:20]

Done.

 
 SCRIPT_BASE="$(dirname "$0")"
 . "$SCRIPT_BASE/common_minimal.sh"
@@ -17,11 +17,15 @@ DEFAULT_KEYS_FOLDER="$VBOOT_BASE/devkeys"
 DEFAULT_BACKUP_FOLDER='/mnt/stateful_partition/backups'
 DEFAULT_PARTITIONS='2 4'
 
-# TODO(hungte) or use "rootdev -s" in future
-DEFAULT_IMAGE="/dev/sda"
+# TODO(hungte) The default image selection is no longer a "make_dev_ssd"... More
+# like "make_dev_image".  We may change the file name in future.
+ROOTDEV="$(rootdev -s 2>/dev/null)"
+ROOTDEV_PARTITION="$(echo $ROOTDEV | sed -n 's/.*\([0-9][0-9]*\)$/\1/p')"
+ROOTDEV_DISK="${ROOTDEV%$ROOTDEV_PARTITION}"
+ROOTDEV_KERNEL="$((ROOTDEV_PARTITION - 1))"
 
 # DEFINE_string name default_value description flag
-DEFINE_string image "$DEFAULT_IMAGE" "Path to device or image file" "i"
+DEFINE_string image "$ROOTDEV_DISK" "Path to device or image file" "i"
 DEFINE_string keys "$DEFAULT_KEYS_FOLDER" "Path to folder of dev keys" "k"
 DEFINE_boolean remove_rootfs_verification \
   $FLAGS_FALSE "Modify kernel boot config to disable rootfs verification" ""
@@ -41,6 +45,7 @@ DEFINE_boolean force "$FLAGS_FALSE" "Skip sanity checks and make the change" "f"
 FLAGS "$@" || exit 1
 ORIGINAL_PARAMS="$@"
 eval set -- "$FLAGS_ARGV"
+ORIGINAL_PARTITIONS="$FLAGS_partitions"
 
 # Globals
 # ----------------------------------------------------------------------------
@@ -107,6 +112,24 @@ cros_kernel_name() {
   esac
 }
 
+find_valid_kernel_partitions() {
+  local part_id
+  local valid_partitions=""
+  for part_id in $*; do
+    local name="$(cros_kernel_name $part_id)"
+    if [ -z "$(dump_kernel_config $FLAGS_image$part_id 2>"$EXEC_LOG")" ]; then
+      echo "INFO: $name: no kernel boot information, ignored." >&2
+    else
+      [ -z "$valid_partitions" ] &&
+        valid_partitions="$part_id" ||
+        valid_partitions="$valid_partitions $part_id"
+      continue
+    fi
+  done
+  debug_msg "find_valid_kernel_partitions: [$*] -> [$valid_partitions]"
+  echo "$valid_partitions"
+}
+
 # Resigns a kernel on SSD or image.
 resign_ssd_kernel() {
   # bs=512 is the fixed block size for dd and cgpt
@@ -235,7 +258,7 @@ resign_ssd_kernel() {
       conv=notrunc
     resigned_kernels=$(($resigned_kernels + 1))
 
-    debug_msg "Make the root filesystem writable if needed."
+    debug_msg "Make the root file system writable if needed."
     # TODO(hungte) for safety concern, a more robust way would be to:
     # (1) change kernel config to ro
     # (2) check if we can enable rw mount
@@ -273,6 +296,82 @@ resign_ssd_kernel() {
   return $resigned_kernels
 }
 
+sanity_check_live_partitions() {
+  debug_msg "Partition sanity check"
+  if [ "$FLAGS_partitions" = "$ROOTDEV_KERNEL" ]; then
+    debug_msg "only for current active partition - safe."
+    return
+  fi
+  if [ "$ORIGINAL_PARTITIONS" != "$DEFAULT_PARTITIONS" ]; then

[Randall Spangler, 2011/04/19 20:40:12]

What does this check do?

(and what if I explicitly specify --partitions='2 4'?  Wouldn't that skip this
check even though I did explicitly specify partitions?)

[Hung-Te, 2011/04/20 01:30:20]

 Changed to compare with empty (new default value).
 

[Randall Spangler, 2011/04/20 19:31:57]

Isn't the default above still '2 4'?

[Hung-Te, 2011/04/21 01:58:35]

The default partitions is '2 4', but the "default" parameter is now empty
(ORIGINAL_PARTITIONS).

And the new check code is if [ "$ORIGINAL_PARTITIONS" != "" ]

If user execute this command without giving --partitions, he will get
[ "" != "" ];

If user manually assigned --partitions "2 4", he will get
[ "2 4" != "" ];

+    debug_msg "user has assigned some option - provide more info."
+    echo "INFO: Making change to $FLAGS_partitions on $FLAGS_image."
+    return
+  fi
+  echo "
+  ERROR: YOU ARE TRYING TO MODIFY THE LIVE SYSTEM IMAGE $FLAGS_image.
+
+  The system may become unusable after that change, especially when you have
+  some auto updates in progress. To make it safer, we suggest you to only
+  change the partition you have booted with. To do that, re-execute this command
+  as:
+
+    sudo ./make_dev_ssd.sh $ORIGINAL_PARAMS --partitions $ROOTDEV_KERNEL
+
+  If you are sure to modify other partition, please invoke the command again and
+  explicitly assign only one target partition for each time  (--partitions N )
+  "
+  return $FLAGS_FALSE
+}
+
+sanity_check_live_firmware() {
+  debug_msg "Firmware compatibility sanity check"
+  if [ "$(crossystem mainfw_type)" = "developer" ]; then
+    debug_msg "developer type firmware in active."
+    return
+  fi
+  debug_msg "Loading firmware to check root key..."
+  local bios_image="$(make_temp_file)"
+  local rootkey_file="$(make_temp_file)"
+  echo "INFO: checking system firmware..."
+  sudo flashrom -p internal:bus=spi -i GBB -r "$bios_image" >/dev/null 2>&1
+  gbb_utility -g --rootkey="$rootkey_file" "$bios_image" >/dev/null 2>&1
+  if [ ! -s "$rootkey_file" ]; then
+    debug_msg "failed to read root key from system firmware..."
+  else
+    # 130 is the magic number for DEV key

[Randall Spangler, 2011/04/19 20:40:12]

Please add more explanation on how 130 was calculated.  Preferably using terms
which match how it's defined in vboot_struct.h, so that we have a better chance
of noticing that we need to update make_dev_ssd.sh if we refactor the key
format.

Or better, can you just compare the root key with the one in
/usr/share/vboot/devkeys?

[Hung-Te, 2011/04/20 01:30:20]

  That's  a great idea. 
  However the file sizes are different now, so I'm marking it as TODO.

 

+    local rootkey_hash="$(od "$rootkey_file" |
+                          head -130 | md5sum |
+                          sed 's/ .*$//' )"
+    if [ "$rootkey_hash" = "a13642246ef93daaf75bd791446fec9b" ]; then
+      debug_msg "detected DEV root key in firmware."
+      return
+    else
+      debug_msg "non-devkey hash: $rootkey_hash"
+    fi
+  fi
+
+  echo "
+  ERROR: YOU ARE NOT USING DEVELOPER FIRMWARE, AND RUNNING THIS COMMAND MAY
+  THROW YOUR CHROMEOS DEVICE INTO UN-BOOTABLE STATE.
+
+  You need to either install developer firmware, or change system root key.
+
+   - To install developer firmware: type command
+     sudo chromeos-firmwareupdate --mode=todev
+
+   - To change system rootkey: disable firmware write protection (a hardware
+     switch) and then type command:
+     sudo ./make_dev_firmware.sh
+
+  If you are sure that you want to make such image without developer
+  firmware or you've already changed system root keys, please run this
+  command again with --force paramemeter:
+
+     sudo ./make_dev_ssd.sh --force $ORIGINAL_PARAMS
+  "
+  return $FLAGS_FALSE
+}
+
 # Main
 # ----------------------------------------------------------------------------
 main() {
@@ -297,34 +396,31 @@ main() {
     "$FLAGS_image" ||
     exit 1
 
-  debug_msg "Firmware compatibility sanity check"
-  if [ "$FLAGS_force" = "$FLAGS_FALSE" ] &&
-     [ "$FLAGS_image" = "$DEFAULT_IMAGE" ] &&
-     [ "$(crossystem mainfw_type)" != "developer" ]; then
+  # checks for running on a live system image.
+  if [ "$FLAGS_image" = "$ROOTDEV_DISK" ]; then
+    debug_msg "check valid kernel partitions for live system"
+    local valid_partitions="$(find_valid_kernel_partitions $FLAGS_partitions)"
+    [ -n "$valid_partitions" ] ||
+      err_die "No valid kernel partitions on $FLAGS_image ($FLAGS_partitions)."
+    FLAGS_partitions="$valid_partitions"
 
-      # TODO(hungte) we can check if the fimware rootkey is already dev keys."
+    # Sanity checks
+    if [ "$FLAGS_force" = "$FLAGS_TRUE" ]; then
       echo "
-      ERROR: YOU ARE NOT USING DEVELOPER FIRMWARE, AND RUNNING THIS COMMAND MAY
-      THROW YOUR CHROMEOS DEVICE INTO UNBOOTABLE STATE.
-
-      You need to either install developer firmware, or change system rootkey.
-
-       - To install developer firmware: type command
-         sudo chromeos-firmwareupdate --mode=todev
-
-       - To change system rootkey: disable firmware write protection (a hardware
-         switch) and then type command:
-         sudo ./make_dev_firmware.sh
-
-      If you are sure that you want to make such image without developer
-      firmware or you've already changed system root keys, please run this
-      command again with --force param:
-
-         sudo ./make_dev_ssd.sh --force $ORIGINAL_PARAMS
-
-      YOUR IMAGE $FLAGS_image IS NOT MODIFIED.
-      "
-      exit 1
+      !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+      ! INFO: ALL SANITY CHECKS WERE BYPASSED. YOU ARE ON YOUR OWN. !
+      !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+      " >&2
+      local i
+      for i in $(seq 5 -1 1); do
+        echo -n "\rStart in $i second(s) (^C to abort)...  " >&2
+        sleep 1
+      done
+      echo ""
+    elif ! sanity_check_live_firmware ||
+         ! sanity_check_live_partitions; then
+      err_die "IMAGE $FLAGS_image IS NOT MODIFIED."
+    fi
   fi
 
   resign_ssd_kernel "$FLAGS_image" || num_signed=$?