From 0a924bc76a0d40c18825af9244dba0194f0fd47b Mon Sep 17 00:00:00 2001
From: Michel Thebeau <michel.thebeau@windriver.com>
Date: Wed, 5 Sep 2018 08:59:44 -0400
Subject: [PATCH] deployment: libvirt: make use of rm command safer

Add sanity around the use of 'rm' in script.  Make sure the file to be
deleted is the correct type, and that its content also meets
expectation.

An example of dangerous outcome in this code from using "sudo rm -rf" is
that if the CONTROLLER or COMPUTE variables are preceded by a space
character then the directory /var/lib/libvirt/images/ will be deleted.

Refrain from using recursive and force options, and in this case because
they are not needed.

Related-Bug: #1790716

Change-Id: I76797133589d993dca2b2aac3c97184bac0457ca
Signed-off-by: Michel Thebeau <michel.thebeau@windriver.com>
---
 deployment/libvirt/cleanup_network.sh         |  6 ++-
 deployment/libvirt/destroy_allinone.sh        | 10 +++--
 .../libvirt/destroy_standard_controller.sh    | 16 +++++---
 deployment/libvirt/functions.sh               | 38 +++++++++++++++++++
 4 files changed, 60 insertions(+), 10 deletions(-)
 create mode 100644 deployment/libvirt/functions.sh

diff --git a/deployment/libvirt/cleanup_network.sh b/deployment/libvirt/cleanup_network.sh
index ebace0bb..a783a8c0 100755
--- a/deployment/libvirt/cleanup_network.sh
+++ b/deployment/libvirt/cleanup_network.sh
@@ -2,13 +2,17 @@
 #
 # cleanup_network.sh - Cleans up network interfaces - not safe to run blindly!
 
+MY_WORKING_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}" )" )"
+
+source ${MY_WORKING_DIR}/functions.sh
+
 NETWORK_DEFAULT=${NETWORK_DEFAULT:-default}
 BRIDGE_INTERFACE=${BRIDGE_INTERFACE=stxbr0}
 
 if virsh net-list --name | grep ${NETWORK_DEFAULT} ; then
     sudo virsh net-destroy ${NETWORK_DEFAULT}
     sudo virsh net-undefine ${NETWORK_DEFAULT}
-    sudo rm -rf /etc/libvirt/qemu/networks/autostart/${NETWORK_DEFAULT}.xml
+    delete_xml /etc/libvirt/qemu/networks/autostart/${NETWORK_DEFAULT}.xml
 fi
 
 if [ -d "/sys/class/net/${BRIDGE_INTERFACE}" ]; then
diff --git a/deployment/libvirt/destroy_allinone.sh b/deployment/libvirt/destroy_allinone.sh
index 9389c16b..7bed6be1 100755
--- a/deployment/libvirt/destroy_allinone.sh
+++ b/deployment/libvirt/destroy_allinone.sh
@@ -1,5 +1,9 @@
 #!/usr/bin/env bash
 
+MY_WORKING_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}" )" )"
+
+source ${MY_WORKING_DIR}/functions.sh
+
 BRIDGE_INTERFACE=${BRIDGE_INTERFACE:-stxbr}
 CONTROLLER=${CONTROLLER:-controller-allinone}
 DOMAIN_DIRECTORY=vms
@@ -14,8 +18,8 @@ for i in {0..1}; do
             sudo virsh destroy ${CONTROLLER_NODE}
         fi
         sudo virsh undefine ${CONTROLLER_NODE}
-        sudo rm -rf /var/lib/libvirt/images/${CONTROLLER_NODE}-0.img
-        sudo rm -rf /var/lib/libvirt/images/${CONTROLLER_NODE}-1.img
-        [ -e ${DOMAIN_FILE} ] && rm ${DOMAIN_FILE}
+        delete_disk /var/lib/libvirt/images/${CONTROLLER_NODE}-0.img
+        delete_disk /var/lib/libvirt/images/${CONTROLLER_NODE}-1.img
+        [ -e ${DOMAIN_FILE} ] && delete_xml ${DOMAIN_FILE}
     fi
 done
diff --git a/deployment/libvirt/destroy_standard_controller.sh b/deployment/libvirt/destroy_standard_controller.sh
index d74292c5..0d65ce91 100755
--- a/deployment/libvirt/destroy_standard_controller.sh
+++ b/deployment/libvirt/destroy_standard_controller.sh
@@ -1,5 +1,9 @@
 #!/usr/bin/env bash
 
+MY_WORKING_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}" )" )"
+
+source ${MY_WORKING_DIR}/functions.sh
+
 BRIDGE_INTERFACE=${BRIDGE_INTERFACE:-stxbr}
 CONTROLLER=${CONTROLLER:-controller}
 COMPUTE=${COMPUTE:-compute}
@@ -15,9 +19,9 @@ for i in {0..1}; do
             sudo virsh destroy ${CONTROLLER_NODE}
         fi
         sudo virsh undefine ${CONTROLLER_NODE}
-        sudo rm -rf /var/lib/libvirt/images/${CONTROLLER_NODE}-0.img
-        sudo rm -rf /var/lib/libvirt/images/${CONTROLLER_NODE}-1.img
-        [ -e ${DOMAIN_FILE} ] && rm ${DOMAIN_FILE}
+        delete_disk /var/lib/libvirt/images/${CONTROLLER_NODE}-0.img
+        delete_disk /var/lib/libvirt/images/${CONTROLLER_NODE}-1.img
+        [ -e ${DOMAIN_FILE} ] && delete_xml ${DOMAIN_FILE}
     fi
 done
 
@@ -31,8 +35,8 @@ for i in {0..1}; do
             sudo virsh destroy ${COMPUTE_NODE}
         fi
         sudo virsh undefine ${COMPUTE_NODE}
-        sudo rm -rf /var/lib/libvirt/images/${COMPUTE_NODE}-0.img
-        sudo rm -rf /var/lib/libvirt/images/${COMPUTE_NODE}-1.img
-        [ -e ${DOMAIN_FILE} ] && rm ${DOMAIN_FILE}
+        delete_disk /var/lib/libvirt/images/${COMPUTE_NODE}-0.img
+        delete_disk /var/lib/libvirt/images/${COMPUTE_NODE}-1.img
+        [ -e ${DOMAIN_FILE} ] && delete_xml ${DOMAIN_FILE}
     fi
 done
diff --git a/deployment/libvirt/functions.sh b/deployment/libvirt/functions.sh
new file mode 100644
index 00000000..63179d1f
--- /dev/null
+++ b/deployment/libvirt/functions.sh
@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+# delete a node's disk file in a safe way
+delete_disk() {
+    local fpath="$1"
+
+    if [ ! -f "$fpath" ]; then
+        echo "file to delete is not a regular file: $fpath" >&2
+        return 1
+    fi
+
+    file -b "$fpath" | grep -q "^QEMU QCOW Image (v3),"
+    if [ $? -ne 0 ]; then
+        echo "file to delete is not QEMU QCOW Image (v3): $fpath" >&2
+        return 1
+    fi
+
+    sudo rm "$fpath"
+}
+
+# delete an xml file in a safe way
+delete_xml() {
+    local fpath="$1"
+
+    if [ ! -f "$fpath" ]; then
+        echo "file to delete is not a regular file: $fpath" >&2
+        return 1
+    fi
+
+    file -b "$fpath" | grep -q "^ASCII text$"
+    if [ $? -ne 0 ]; then
+        echo "file to delete is not ASCII text: $fpath" >&2
+        return 1
+    fi
+
+    sudo rm "$fpath"
+}
+