genpolicy: validate each exec command line arg

Generate policy that validates each exec command line argument, instead
of joining those args and validating the resulting string. Joining the
args ignored the fact that some of the args might include space
characters.

The older format from genpolicy-settings.json was similar to:

    "ExecProcessRequest": {
      "commands": [
                "sh -c cat /proc/self/status"
        ],
      "regex": []
    },

That format will not be supported anymore. genpolicy will detect if its
users are trying to use the older "commands" field and will exit with
a relevant error message in that case.

The new settings format is:

    "ExecProcessRequest": {
      "allowed_commands": [
        [
          "sh",
          "-c",
          "cat /proc/self/status"
        ]
      ],
      "regex": []
    },

Signed-off-by: Dan Mihai <dmihai@microsoft.com>

src/tools/genpolicy/genpolicy-settings.json

@@ -299,7 +299,7 @@
             "^$(cpath)/"
         ],
         "ExecProcessRequest": {
-            "commands": [],
+            "allowed_commands": [],
             "regex": []
         },
         "CloseStdinRequest": false,

src/tools/genpolicy/rules.rego

@@ -1111,12 +1111,9 @@ CreateSandboxRequest {
 ExecProcessRequest {
     print("ExecProcessRequest 1: input =", input)
 
-    i_command = concat(" ", input.process.Args)
+    some p_command in policy_data.request_defaults.ExecProcessRequest.allowed_commands
-    print("ExecProcessRequest 1: i_command =", i_command)
-
-    some p_command in policy_data.request_defaults.ExecProcessRequest.commands
     print("ExecProcessRequest 1: p_command =", p_command)
-    p_command == i_command
+    p_command == input.process.Args
 
     print("ExecProcessRequest 1: true")
 }

src/tools/genpolicy/src/policy.rs

@@ -313,8 +313,12 @@ pub struct CreateContainerRequestDefaults {
 /// ExecProcessRequest settings from genpolicy-settings.json.
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct ExecProcessRequestDefaults {
+    /// Allow these commands to be executed. This field has been deprecated - use allowed_commands instead.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub commands: Option<Vec<String>>,
+
     /// Allow these commands to be executed.
-    commands: Vec<String>,
+    pub allowed_commands: Vec<Vec<String>>,
 
     /// Allow commands matching these regexes to be executed.
     regex: Vec<String>,

src/tools/genpolicy/src/settings.rs

@@ -73,6 +73,7 @@ impl Settings {
         if let Ok(file) = File::open(json_settings_path) {
             let settings: Self = serde_json::from_reader(file).unwrap();
             debug!("settings = {:?}", &settings);
+            Self::validate_settings(&settings);
             settings
         } else {
             panic!("Cannot open file {}. Please copy it to the current directory or specify the path to it using the -j parameter.",
@@ -87,4 +88,13 @@ impl Settings {
             &self.other_container
         }
     }
+
+    fn validate_settings(settings: &Self) {
+        if let Some(commands) = &settings.request_defaults.ExecProcessRequest.commands {
+            if !commands.is_empty() {
+                panic!("The settings field <request_defaults.ExecProcessRequest.commands> has been deprecated. \
+                    Please use <request_defaults.ExecProcessRequest.allowed_commands> instead.");
+            }
+        }
+    }
 }

tests/integration/kubernetes/tests_common.sh

@@ -236,23 +236,15 @@ add_exec_to_policy_settings() {
 	auto_generate_policy_enabled || return 0
 
 	local -r settings_dir="$1"
-
-	# TODO: teach genpolicy to work with an array of args, instead of joining the args here.
 	shift
-	if [ "${#@}" -gt "1" ]; then
-		# Join all the exec args.
-		local allowed_exec=$(printf '%s ' "${@}")
 
-		# Remove the trailing space character.
+	# Create a JSON array of strings containing all the args of the command to be allowed.
-		allowed_exec="${allowed_exec::-1}"
+	local exec_args=$(printf "%s\n" "$@" | jq -R | jq -sc)
-	else
-		local -r allowed_exec="$1"
-	fi
 
 	# Change genpolicy settings to allow kubectl to exec the command specified by the caller.
-	info "${settings_dir}/genpolicy-settings.json: allowing exec: ${allowed_exec}"
+	local jq_command=".request_defaults.ExecProcessRequest.allowed_commands |= . + [${exec_args}]"
-	jq --arg allowed_exec "${allowed_exec}" \
+	info "${settings_dir}/genpolicy-settings.json: executing jq command: ${jq_command}"
-		'.request_defaults.ExecProcessRequest.commands |= . + [$allowed_exec]' \
+	jq "${jq_command}" \
 		"${settings_dir}/genpolicy-settings.json" > \
 		"${settings_dir}/new-genpolicy-settings.json"
 	mv "${settings_dir}/new-genpolicy-settings.json" \
@@ -281,29 +273,28 @@ add_requests_to_policy_settings() {
 # Change genpolicy settings to allow executing on the Guest VM the commands
 # used by "kubectl cp" from the Host to the Guest.
 add_copy_from_host_to_policy_settings() {
-	declare -r genpolicy_settings_dir="$1"
+	local -r genpolicy_settings_dir="$1"
 
-	exec_command="test -d /tmp"
+	local exec_command=(test -d /tmp)
-	add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command}"
+	add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command[@]}"
-	exec_command="tar -xmf - -C /tmp"
+	exec_command=(tar -xmf - -C /tmp)
-	add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command}"
+	add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command[@]}"
 }
 
 # Change genpolicy settings to allow executing on the Guest VM the commands
 # used by "kubectl cp" from the Guest to the Host.
 add_copy_from_guest_to_policy_settings() {
-	declare -r genpolicy_settings_dir="$1"
+	local -r genpolicy_settings_dir="$1"
-	declare -r copied_file="$2"
+	local -r copied_file="$2"
 
-	exec_command="tar cf - ${copied_file}"
+	exec_command=(tar cf - "${copied_file}")
-	add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command}"
+	add_exec_to_policy_settings "${policy_settings_dir}" "${exec_command[@]}"
 }
 
-# Change genpolicy settings to allow "kubectl exec" to execute a command
+# Change genpolicy settings to use a pod namespace different than "default".
-# and to read console output from a test pod.
 set_namespace_to_policy_settings() {
-	declare -r settings_dir="$1"
+	local -r settings_dir="$1"
-	declare -r namespace="$2"
+	local -r namespace="$2"
 
 	auto_generate_policy_enabled || return 0