YARN-6726. Fix issues with docker commands executed by container-executor. (Shane Kumpf via wangda)

Change-Id: If1b1827345f98f0a49cc7e39d1ba41fbeed5e911

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/CMakeLists.txt

@@ -89,6 +89,7 @@ add_library(container
     main/native/container-executor/impl/configuration.c
     main/native/container-executor/impl/container-executor.c
     main/native/container-executor/impl/get_executable.c
+    main/native/container-executor/impl/utils/string-utils.c
 )
 
 add_executable(container-executor

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c

@@ -18,6 +18,7 @@
 
 #include "configuration.h"
 #include "container-executor.h"
+#include "utils/string-utils.h"
 
 #include <inttypes.h>
 #include <libgen.h>
@@ -40,6 +41,7 @@
 #include <sys/mount.h>
 #include <sys/wait.h>
 #include <getopt.h>
+#include <regex.h>
 
 #include "config.h"
 
@@ -79,6 +81,11 @@ static const char* TC_READ_STATS_OPTS [] = { "-s",  "-b", NULL};
 //struct to store the user details
 struct passwd *user_detail = NULL;
 
+//Docker container related constants.
+static const char* DOCKER_CONTAINER_NAME_PREFIX = "container_";
+static const char* DOCKER_CLIENT_CONFIG_ARG = "--config=";
+static const char* DOCKER_PULL_COMMAND = "pull";
+
 FILE* LOGFILE = NULL;
 FILE* ERRORFILE = NULL;
 
@@ -1208,6 +1215,27 @@ char** tokenize_docker_command(const char *input, int *split_counter) {
   return linesplit;
 }
 
+int execute_regex_match(const char *regex_str, const char *input) {
+  regex_t regex;
+  int regex_match;
+  if (0 != regcomp(&regex, regex_str, REG_EXTENDED|REG_NOSUB)) {
+    fprintf(LOGFILE, "Unable to compile regex.");
+    fflush(LOGFILE);
+    exit(ERROR_COMPILING_REGEX);
+  }
+  regex_match = regexec(&regex, input, (size_t) 0, NULL, 0);
+  regfree(&regex);
+  if(0 == regex_match) {
+    return 0;
+  }
+  return 1;
+}
+
+int validate_docker_image_name(const char *image_name) {
+  char *regex_str = "^(([a-zA-Z0-9.-]+)(:[0-9]+)?/)?([a-z0-9_./-]+)(:[a-zA-Z0-9_.-]+)?$";
+  return execute_regex_match(regex_str, image_name);
+}
+
 char* sanitize_docker_command(const char *line) {
   static struct option long_options[] = {
     {"name", required_argument, 0, 'n' },
@@ -1222,6 +1250,7 @@ char* sanitize_docker_command(const char *line) {
     {"cap-drop", required_argument, 0, 'o' },
     {"device", required_argument, 0, 'i' },
     {"detach", required_argument, 0, 't' },
+    {"format", required_argument, 0, 'f' },
     {0, 0, 0, 0}
   };
 
@@ -1240,6 +1269,35 @@ char* sanitize_docker_command(const char *line) {
   if(output == NULL) {
     exit(OUT_OF_MEMORY);
   }
+
+  // Handle docker client config option.
+  if(0 == strncmp(linesplit[0], DOCKER_CLIENT_CONFIG_ARG, strlen(DOCKER_CLIENT_CONFIG_ARG))) {
+    strcat(output, linesplit[0]);
+    strcat(output, " ");
+    long index = 0;
+    while(index < split_counter) {
+      linesplit[index] = linesplit[index + 1];
+      if (linesplit[index] == NULL) {
+        split_counter--;
+        break;
+      }
+      index++;
+    }
+  }
+
+  // Handle docker pull and image name validation.
+  if (0 == strncmp(linesplit[0], DOCKER_PULL_COMMAND, strlen(DOCKER_PULL_COMMAND))) {
+    if (0 != validate_docker_image_name(linesplit[1])) {
+      fprintf(ERRORFILE, "Invalid Docker image name, exiting.");
+      fflush(ERRORFILE);
+      exit(DOCKER_IMAGE_INVALID);
+    }
+    strcat(output, linesplit[0]);
+    strcat(output, " ");
+    strcat(output, linesplit[1]);
+    return output;
+  }
+
   strcat(output, linesplit[0]);
   strcat(output, " ");
   optind = 1;
@@ -1287,6 +1345,11 @@ char* sanitize_docker_command(const char *line) {
       case 't':
         quote_and_append_arg(&output, &output_size, "--detach=", optarg);
         break;
+      case 'f':
+        strcat(output, "--format=");
+        strcat(output, optarg);
+        strcat(output, " ");
+        break;
       default:
         fprintf(LOGFILE, "Unknown option in docker command, character %d %c, optionindex = %d\n", c, c, optind);
         fflush(LOGFILE);
@@ -1297,7 +1360,16 @@ char* sanitize_docker_command(const char *line) {
 
   if(optind < split_counter) {
     while(optind < split_counter) {
-      quote_and_append_arg(&output, &output_size, "", linesplit[optind++]);
+      if (0 == strncmp(linesplit[optind], DOCKER_CONTAINER_NAME_PREFIX, strlen(DOCKER_CONTAINER_NAME_PREFIX))) {
+        if (1 != validate_container_id(linesplit[optind])) {
+          fprintf(ERRORFILE, "Specified container_id=%s is invalid\n", linesplit[optind]);
+          fflush(ERRORFILE);
+          exit(DOCKER_CONTAINER_NAME_INVALID);
+        }
+        strcat(output, linesplit[optind++]);
+      } else {
+        quote_and_append_arg(&output, &output_size, "", linesplit[optind++]);
+      }
     }
   }
 
@@ -1328,8 +1400,8 @@ char* parse_docker_command_file(const char* command_file) {
   if(ret == NULL) {
     exit(ERROR_SANITIZING_DOCKER_COMMAND);
   }
-  fprintf(LOGFILE, "Using command %s\n", ret);
-  fflush(LOGFILE);
+  fprintf(ERRORFILE, "Using command %s\n", ret);
+  fflush(ERRORFILE);
 
   return ret;
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h

@@ -74,7 +74,10 @@ enum errorcodes {
   COULD_NOT_CREATE_APP_LOG_DIRECTORIES = 36,
   COULD_NOT_CREATE_TMP_DIRECTORIES = 37,
   ERROR_CREATE_CONTAINER_DIRECTORIES_ARGUMENTS = 38,
-  ERROR_SANITIZING_DOCKER_COMMAND = 39
+  ERROR_SANITIZING_DOCKER_COMMAND = 39,
+  DOCKER_IMAGE_INVALID = 40,
+  DOCKER_CONTAINER_NAME_INVALID = 41,
+  ERROR_COMPILING_REGEX = 42
 };
 
 enum operations {
@@ -309,3 +312,15 @@ int run_docker(const char *command_file);
  * Sanitize docker commands. Returns NULL if there was any failure.
 */
 char* sanitize_docker_command(const char *line);
+
+/*
+ * Compile the regex_str and determine if the input string matches.
+ * Return 0 on match, 1 of non-match.
+ */
+int execute_regex_match(const char *regex_str, const char *input);
+
+/**
+ * Validate the docker image name matches the expected input.
+ * Return 0 on success.
+ */
+int validate_docker_image_name(const char *image_name);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.c

@@ -0,0 +1,86 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <strings.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+/*
+ * if all chars in the input str are numbers
+ * return true/false
+ */
+static int all_numbers(char* input) {
+  if (0 == strlen(input)) {
+    return 0;
+  }
+
+  for (int i = 0; i < strlen(input); i++) {
+    if (input[i] < '0' || input[i] > '9') {
+      return 0;
+    }
+  }
+  return 1;
+}
+
+int validate_container_id(const char* input) {
+  /*
+   * Two different forms of container_id
+   * container_e17_1410901177871_0001_01_000005
+   * container_1410901177871_0001_01_000005
+   */
+  char* input_cpy = malloc(strlen(input));
+  strcpy(input_cpy, input);
+  char* p = strtok(input_cpy, "_");
+  int idx = 0;
+  while (p != NULL) {
+    if (0 == idx) {
+      if (0 != strcmp("container", p)) {
+        return 0;
+      }
+    } else if (1 == idx) {
+      // this could be e[n][n], or [n][n]...
+      if (!all_numbers(p)) {
+        if (strlen(p) == 0) {
+          return 0;
+        }
+        if (p[0] != 'e') {
+          return 0;
+        }
+        if (!all_numbers(p + 1)) {
+          return 0;
+        }
+      }
+    } else {
+      // otherwise, should be all numbers
+      if (!all_numbers(p)) {
+        return 0;
+      }
+    }
+
+    p = strtok(NULL, "_");
+    idx++;
+  }
+  free(input_cpy);
+
+  // We should have [5,6] elements split by '_'
+  if (idx > 6 || idx < 5) {
+    return 0;
+  }
+  return 1;
+}

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.h

@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifdef __FreeBSD__
+#define _WITH_GETLINE
+#endif
+
+#ifndef _UTILS_STRING_UTILS_H_
+#define _UTILS_STRING_UTILS_H_
+
+/*
+ * Get numbers split by comma from a input string
+ * return false/true
+ */
+int validate_container_id(const char* input);
+
+#endif

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c

@@ -17,6 +17,7 @@
  */
 #include "configuration.h"
 #include "container-executor.h"
+#include "utils/string-utils.h"
 
 #include <inttypes.h>
 #include <errno.h>
@@ -1176,7 +1177,13 @@ void test_sanitize_docker_command() {
     "run --name=$CID --user=nobody -d --workdir=/yarn/local/cdir --privileged --rm --device=/sys/fs/cgroup/device:/sys/fs/cgroup/device --detach=true --cgroup-parent=/sys/fs/cgroup/cpu/yarn/cid --net=host --hostname=test.host.name --cap-drop=ALL --cap-add=SYS_CHROOT --cap-add=MKNOD --cap-add=SETFCAP --cap-add=SETPCAP --cap-add=FSETID --cap-add=CHOWN --cap-add=AUDIT_WRITE --cap-add=SETGID --cap-add=NET_RAW --cap-add=FOWNER --cap-add=SETUID --cap-add=DAC_OVERRIDE --cap-add=KILL --cap-add=NET_BIND_SERVICE -v /sys/fs/cgroup:/sys/fs/cgroup:ro -v /yarn/local/cdir:/yarn/local/cdir -v /yarn/local/usercache/test/:/yarn/local/usercache/test/ ubuntu bash /yarn/local/usercache/test/appcache/aid/cid/launch_container.sh",
     "run --name=cname --user=nobody -d --workdir=/yarn/local/cdir --privileged --rm --device=/sys/fs/cgroup/device:/sys/fs/cgroup/device --detach=true --cgroup-parent=/sys/fs/cgroup/cpu/yarn/cid --net=host --hostname=test.host.name --cap-drop=ALL --cap-add=SYS_CHROOT --cap-add=MKNOD --cap-add=SETFCAP --cap-add=SETPCAP --cap-add=FSETID --cap-add=CHOWN --cap-add=AUDIT_WRITE --cap-add=SETGID --cap-add=NET_RAW --cap-add=FOWNER --cap-add=SETUID --cap-add=DAC_OVERRIDE --cap-add=KILL --cap-add=NET_BIND_SERVICE -v /sys/fs/cgroup:/sys/fs/cgroup:ro -v /yarn/local/cdir:/yarn/local/cdir -v /yarn/local/usercache/test/:/yarn/local/usercache/test/ ubuntu || touch /tmp/file # bash /yarn/local/usercache/test/appcache/aid/cid/launch_container.sh",
     "run --name=cname --user=nobody -d --workdir=/yarn/local/cdir --privileged --rm --device=/sys/fs/cgroup/device:/sys/fs/cgroup/device --detach=true --cgroup-parent=/sys/fs/cgroup/cpu/yarn/cid --net=host --hostname=test.host.name --cap-drop=ALL --cap-add=SYS_CHROOT --cap-add=MKNOD --cap-add=SETFCAP --cap-add=SETPCAP --cap-add=FSETID --cap-add=CHOWN --cap-add=AUDIT_WRITE --cap-add=SETGID --cap-add=NET_RAW --cap-add=FOWNER --cap-add=SETUID --cap-add=DAC_OVERRIDE --cap-add=KILL --cap-add=NET_BIND_SERVICE -v /sys/fs/cgroup:/sys/fs/cgroup:ro -v /yarn/local/cdir:/yarn/local/cdir -v /yarn/local/usercache/test/:/yarn/local/usercache/test/ ubuntu' || touch /tmp/file # bash /yarn/local/usercache/test/appcache/aid/cid/launch_container.sh",
-    "run ''''''''"
+    "run ''''''''",
+    "inspect --format='{{range(.NetworkSettings.Networks)}}{{.IPAddress}},{{end}}{{.Config.Hostname}}' container_e111_1111111111111_1111_01_111111",
+    "rm container_e111_1111111111111_1111_01_111111",
+    "stop container_e111_1111111111111_1111_01_111111",
+    "pull ubuntu",
+    "pull registry.com/user/ubuntu",
+    "--config=/yarn/local/cdir/ pull registry.com/user/ubuntu"
   };
   char *expected_output[] = {
       "run --name='cname' --user='nobody' -d --workdir='/yarn/local/cdir' --privileged --rm --device='/sys/fs/cgroup/device:/sys/fs/cgroup/device' --detach='true' --cgroup-parent='/sys/fs/cgroup/cpu/yarn/cid' --net='host' --hostname='test.host.name' --cap-drop='ALL' --cap-add='SYS_CHROOT' --cap-add='MKNOD' --cap-add='SETFCAP' --cap-add='SETPCAP' --cap-add='FSETID' --cap-add='CHOWN' --cap-add='AUDIT_WRITE' --cap-add='SETGID' --cap-add='NET_RAW' --cap-add='FOWNER' --cap-add='SETUID' --cap-add='DAC_OVERRIDE' --cap-add='KILL' --cap-add='NET_BIND_SERVICE' -v '/sys/fs/cgroup:/sys/fs/cgroup:ro' -v '/yarn/local/cdir:/yarn/local/cdir' -v '/yarn/local/usercache/test/:/yarn/local/usercache/test/' 'ubuntu' 'bash' '/yarn/local/usercache/test/appcache/aid/cid/launch_container.sh' ",
@@ -1184,12 +1191,18 @@ void test_sanitize_docker_command() {
       "run --name='cname' --user='nobody' -d --workdir='/yarn/local/cdir' --privileged --rm --device='/sys/fs/cgroup/device:/sys/fs/cgroup/device' --detach='true' --cgroup-parent='/sys/fs/cgroup/cpu/yarn/cid' --net='host' --hostname='test.host.name' --cap-drop='ALL' --cap-add='SYS_CHROOT' --cap-add='MKNOD' --cap-add='SETFCAP' --cap-add='SETPCAP' --cap-add='FSETID' --cap-add='CHOWN' --cap-add='AUDIT_WRITE' --cap-add='SETGID' --cap-add='NET_RAW' --cap-add='FOWNER' --cap-add='SETUID' --cap-add='DAC_OVERRIDE' --cap-add='KILL' --cap-add='NET_BIND_SERVICE' -v '/sys/fs/cgroup:/sys/fs/cgroup:ro' -v '/yarn/local/cdir:/yarn/local/cdir' -v '/yarn/local/usercache/test/:/yarn/local/usercache/test/' 'ubuntu' '||' 'touch' '/tmp/file' '#' 'bash' '/yarn/local/usercache/test/appcache/aid/cid/launch_container.sh' ",
       "run --name='cname' --user='nobody' -d --workdir='/yarn/local/cdir' --privileged --rm --device='/sys/fs/cgroup/device:/sys/fs/cgroup/device' --detach='true' --cgroup-parent='/sys/fs/cgroup/cpu/yarn/cid' --net='host' --hostname='test.host.name' --cap-drop='ALL' --cap-add='SYS_CHROOT' --cap-add='MKNOD' --cap-add='SETFCAP' --cap-add='SETPCAP' --cap-add='FSETID' --cap-add='CHOWN' --cap-add='AUDIT_WRITE' --cap-add='SETGID' --cap-add='NET_RAW' --cap-add='FOWNER' --cap-add='SETUID' --cap-add='DAC_OVERRIDE' --cap-add='KILL' --cap-add='NET_BIND_SERVICE' -v '/sys/fs/cgroup:/sys/fs/cgroup:ro' -v '/yarn/local/cdir:/yarn/local/cdir' -v '/yarn/local/usercache/test/:/yarn/local/usercache/test/' 'ubuntu'\"'\"'' '||' 'touch' '/tmp/file' '#' 'bash' '/yarn/local/usercache/test/appcache/aid/cid/launch_container.sh' ",
       "run ''\"'\"''\"'\"''\"'\"''\"'\"''\"'\"''\"'\"''\"'\"''\"'\"'' ",
+      "inspect --format='{{range(.NetworkSettings.Networks)}}{{.IPAddress}},{{end}}{{.Config.Hostname}}' container_e111_1111111111111_1111_01_111111",
+      "rm container_e111_1111111111111_1111_01_111111",
+      "stop container_e111_1111111111111_1111_01_111111",
+      "pull ubuntu",
+      "pull registry.com/user/ubuntu",
+      "--config=/yarn/local/cdir/ pull registry.com/user/ubuntu"
   };
 
   int input_size = sizeof(input) / sizeof(char *);
   int i = 0;
   for(i = 0;  i < input_size; i++) {
-    char *command = (char *) calloc(strlen(input[i]), sizeof(char));
+    char *command = (char *) calloc(strlen(input[i]) + 1 , sizeof(char));
     strncpy(command, input[i], strlen(input[i]));
     char *op = sanitize_docker_command(command);
     if(strncmp(expected_output[i], op, strlen(expected_output[i])) != 0) {
@@ -1200,6 +1213,102 @@ void test_sanitize_docker_command() {
   }
 }
 
+void test_validate_docker_image_name() {
+
+  char *good_input[] = {
+    "ubuntu",
+    "ubuntu:latest",
+    "ubuntu:14.04",
+    "ubuntu:LATEST",
+    "registry.com:5000/user/ubuntu",
+    "registry.com:5000/user/ubuntu:latest",
+    "registry.com:5000/user/ubuntu:0.1.2.3",
+    "registry.com/user/ubuntu",
+    "registry.com/user/ubuntu:latest",
+    "registry.com/user/ubuntu:0.1.2.3",
+    "registry.com/user/ubuntu:test-image",
+    "registry.com/user/ubuntu:test_image",
+    "registry.com/ubuntu",
+    "user/ubuntu",
+    "user/ubuntu:0.1.2.3",
+    "user/ubuntu:latest",
+    "user/ubuntu:test_image",
+    "user/ubuntu.test:test_image",
+    "user/ubuntu-test:test-image",
+    "registry.com/ubuntu/ubuntu/ubuntu"
+  };
+
+  char *bad_input[] = {
+    "UBUNTU",
+    "registry.com|5000/user/ubuntu",
+    "registry.com | 5000/user/ubuntu",
+    "ubuntu' || touch /tmp/file #",
+    "ubuntu || touch /tmp/file #",
+    "''''''''",
+    "bad_host_name:5000/user/ubuntu",
+    "registry.com:foo/ubuntu/ubuntu/ubuntu",
+    "registry.com/ubuntu:foo/ubuntu/ubuntu"
+  };
+
+  int good_input_size = sizeof(good_input) / sizeof(char *);
+  int i = 0;
+  for(i = 0; i < good_input_size; i++) {
+    int op = validate_docker_image_name(good_input[i]);
+    if(0 != op) {
+      printf("\nFAIL: docker image name %s is invalid", good_input[i]);
+      exit(1);
+    }
+  }
+
+  int bad_input_size = sizeof(bad_input) / sizeof(char *);
+  int j = 0;
+  for(j = 0; j < bad_input_size; j++) {
+    int op = validate_docker_image_name(bad_input[j]);
+    if(1 != op) {
+      printf("\nFAIL: docker image name %s is valid, expected invalid", bad_input[j]);
+      exit(1);
+    }
+  }
+}
+
+void test_validate_container_id() {
+  char *good_input[] = {
+    "container_e134_1499953498516_50875_01_000007",
+    "container_1499953498516_50875_01_000007",
+    "container_e1_12312_11111_02_000001"
+  };
+
+  char *bad_input[] = {
+    "CONTAINER",
+    "container_e1_12312_11111_02_000001 | /tmp/file"
+    "container_e1_12312_11111_02_000001 || # /tmp/file",
+    "container_e1_12312_11111_02_000001 # /tmp/file",
+    "container_e1_12312_11111_02_000001' || touch /tmp/file #",
+    "ubuntu || touch /tmp/file #",
+    "''''''''"
+  };
+
+  int good_input_size = sizeof(good_input) / sizeof(char *);
+  int i = 0;
+  for(i = 0; i < good_input_size; i++) {
+    int op = validate_container_id(good_input[i]);
+    if(1 != op) {
+      printf("FAIL: docker container name %s is invalid\n", good_input[i]);
+      exit(1);
+    }
+  }
+
+  int bad_input_size = sizeof(bad_input) / sizeof(char *);
+  int j = 0;
+  for(j = 0; j < bad_input_size; j++) {
+    int op = validate_container_id(bad_input[j]);
+    if(0 != op) {
+      printf("FAIL: docker container name %s is valid, expected invalid\n", bad_input[j]);
+      exit(1);
+    }
+  }
+}
+
 // This test is expected to be executed either by a regular
 // user or by root. If executed by a regular user it doesn't
 // test all the functions that would depend on changing the
@@ -1297,6 +1406,12 @@ int main(int argc, char **argv) {
   printf("\nTesting sanitize docker commands()\n");
   test_sanitize_docker_command();
 
+  printf("\nTesting validate_docker_image_name()\n");
+  test_validate_docker_image_name();
+
+  printf("\nTesting validate_container_id()\n");
+  test_validate_container_id();
+
   test_check_user(0);
 
 #ifdef __APPLE__