Build a secure ESP docker image run as non-root (#563)

* Move server_config path to /home/nginx

* start_esp add a flag --server_config_dir

* fix start_esp test

* create home folder for nginx user in deb.preinst

* Build a separate secure image

* Move default config_dir back to /etc/nginx

Jenkinsfile

@@ -360,6 +360,16 @@ def espServerlessDockerImage() {
   return espGenericDockerImage('-serverless')
 }
 
+def espSecureDockerImage() {
+  if (isRelease()) {
+    if (getParam('USE_LATEST_RELEASE', false)) {
+      return "gcr.io/endpoints-release/endpoints-runtime-secure:1"
+    }
+    return "gcr.io/endpoints-release/endpoints-runtime-secure:${ESP_RUNTIME_VERSION}"
+  }
+  return espGenericDockerImage('-secure')
+}
+
 def espFlexDockerImage() {
   return espGenericDockerImage('-flex')
 }
@@ -419,11 +429,13 @@ def buildPackages() {
     def espDebianPackage = espDebianPackage()
     def espImgFlex = espFlexDockerImage()
     def espImgServerless = espServerlessDockerImage()
+    def espImgSecure = espSecureDockerImage()
 
     sh("script/robot-release " +
         "-m ${espImgFlex} " +
         "-g ${espImgGeneric} " +
         "-h ${espImgServerless} " +
+        "-s ${espImgSecure} " +
         "-d ${espDebianPackage} " +
         "${serverConfigFlag} -s")
     // local perf builds its own esp binary package.

docker/secure/Dockerfile.template

@@ -0,0 +1,13 @@
+# Build more secure ESP docker image running as non-root
+# and allow root filesystem as read-only.
+
+FROM ${PARENT_IMAGE}
+
+RUN rm -rf /var/log/nginx && \
+    mkdir -p /var/log/nginx /var/cache/nginx && \
+    chown nginx:nginx /var/log/nginx /var/cache/nginx && \
+    chmod 777 /var/log/nginx /var/cache/nginx
+
+USER nginx
+
+ENTRYPOINT ["/usr/sbin/start_esp", "--server_config_dir=/home/nginx", "--config_dir=/home/nginx/endpoints"]

script/linux-build-docker

@@ -34,12 +34,13 @@ PLATFORM="flex"
 DEB="${ROOT}/bazel-bin/src/nginx/main/endpoints-server-proxy.deb"
 DOCKERFILE="Dockerfile"
 
-while getopts :c:d:i:s:p: arg; do
+while getopts :c:d:i:s:t:p: arg; do
   case ${arg} in
     c) CONFIG="${OPTARG}";;
     d) DEB="${OPTARG}";;
     i) IMAGE="${OPTARG}";;
     s) SERVERLESS_IMAGE="${OPTARG}";;
+    t) SECURE_IMAGE="${OPTARG}";;
     p) PLATFORM="${OPTARG}";;
     *) error_exit "Unrecognized argument -${OPTARG}";;
   esac
@@ -84,10 +85,25 @@ retry -n 10 -s 10 \
     gcloud docker -- push "${IMAGE}" \
   || error_exit "Failed to upload Docker image to gcr."
 
+# Build secure docker image
+if  [[ -n "${SECURE_IMAGE}" ]]; then
+  echo "Building secure docker image."
+  sed -e "s|\${PARENT_IMAGE}|${IMAGE}|g" \
+     "${ROOT}/docker/secure/Dockerfile.template" > "${ROOT}/docker/secure/Dockerfile"
+
+  retry -n 3 docker build --no-cache  -t "${SECURE_IMAGE}" ./docker/secure/  \
+    || error_exit "Failed to build secure Docker Image."
+
+  echo "Pushing secure Docker image: ${SECURE_IMAGE}"
+  retry -n 10 -s 10 \
+      gcloud docker -- push "${SECURE_IMAGE}" \
+    || error_exit "Failed to upload secure Docker image to gcr."
+fi
+
 # Build serverless docker image
-if  [[ -n "${SERVERLESS_IMAGE}" ]]; then
+if  [[ -n "${SERVERLESS_IMAGE}" && -n "${SECURE_IMAGE}" ]]; then
   echo "Building serverless docker image."
-  sed -e "s|\${PARENT_IMAGE}|${IMAGE}|g" \
+  sed -e "s|\${PARENT_IMAGE}|${SECURE_IMAGE}|g" \
      "${ROOT}/docker/serverless/Dockerfile.template" > "${ROOT}/docker/serverless/Dockerfile"
 
   retry -n 3 docker build --no-cache  -t "${SERVERLESS_IMAGE}" ./docker/serverless/  \
@@ -98,3 +114,4 @@ if  [[ -n "${SERVERLESS_IMAGE}" ]]; then
       gcloud docker -- push "${SERVERLESS_IMAGE}" \
     || error_exit "Failed to upload serverless Docker image to gcr."
 fi
+

script/robot-release

@@ -41,13 +41,15 @@ RELEASE_GENERIC_IMAGE_FORMAT='gcr.io/endpointsv2/endpoints-runtime'
 RELEASE_GENERIC_IMAGE_FORMAT+=':debian-git-%H'
 RELEASE_SERVERLESS_IMAGE_FORMAT='gcr.io/endpointsv2/endpoints-runtime-serverless'
 RELEASE_SERVERLESS_IMAGE_FORMAT+=':debian-git-%H'
+RELEASE_SECURE_IMAGE_FORMAT='gcr.io/endpointsv2/endpoints-runtime-secure'
+RELEASE_SECURE_IMAGE_FORMAT+=':debian-git-%H'
 
 ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 . ${ROOT}/script/all-utilities || { echo 'Cannot load Bash utilities'; exit 1; }
 
 CONFIG=''
 
-while getopts :c:d:f:g:h:j:m:r:st:v: arg; do
+while getopts :c:d:f:g:h:j:m:r:s:t:v: arg; do
   case ${arg} in
     c) CONFIG="${OPTARG}";;
     d) RELEASE_DEB="${OPTARG}";;
@@ -56,7 +58,7 @@ while getopts :c:d:f:g:h:j:m:r:st:v: arg; do
     h) RELEASE_SERVERLESS_IMAGE_FORMAT="${OPTARG}";;
     j) RELEASE_JSON="${OPTARG}";;
     m) RELEASE_FLEX_IMAGE_FORMAT="${OPTARG}";;
-    s) SKIP_TESTS='true';;
+    s) RELEASE_SECURE_IMAGE_FORMAT="${OPTARG}";;
     t) RELEASE_TAR="${OPTARG}";;
     *) echo "Invalid argument -${OPTARG}, ignoring.";;
   esac
@@ -74,16 +76,6 @@ done
 ${BAZEL} ${BAZEL_ARGS} clean \
   || error_exit 'Failed to ${BAZEL} ${BAZEL_ARGS} clean before the release build.'
 
-if [[ "${SKIP_TESTS}" == 'false' ]]; then
-  echo 'Running pre-release tests.'
-  retry -n 3 ${BAZEL} ${BAZEL_ARGS} fetch //src/... //third_party:all \
-  && retry -n 2 ${BAZEL} ${BAZEL_ARGS} build ${BAZEL_BUILD_ARGS} \
-    --config=release //src/... //third_party:all \
-  && ${BAZEL} ${BAZEL_ARGS} test --config=release \
-      //src/... //third_party:all \
-    || error_exit 'Failed release tests.'
-fi
-
 read ENDPOINTS_RUNTIME_VERSION < ${ROOT}/src/nginx/version
 BAZEL_VERSION="$(${BAZEL} ${BAZEL_ARGS} version \
   | grep '^Build label: ' | sed 's/^Build label: //')"
@@ -138,8 +130,11 @@ if  [[ -n "${RELEASE_GENERIC_IMAGE_FORMAT}" ]]; then
     --pretty=format:"${RELEASE_GENERIC_IMAGE_FORMAT}")"
   SERVERLESS_IMAGE="$(git show -q HEAD \
     --pretty=format:"${RELEASE_SERVERLESS_IMAGE_FORMAT}")"
+  SECURE_IMAGE="$(git show -q HEAD \
+    --pretty=format:"${RELEASE_SECURE_IMAGE_FORMAT}")"
   ${ROOT}/script/linux-build-docker -c "${CONFIG}" \
-    -d "${TMP_DEBIAN_PACKAGE}" -i "${GENERIC_IMAGE}" -s "${SERVERLESS_IMAGE}" -p generic \
+    -d "${TMP_DEBIAN_PACKAGE}" -i "${GENERIC_IMAGE}" \
+    -s "${SERVERLESS_IMAGE}" -t "${SECURE_IMAGE}" -p generic \
     || error_exit 'Failed to build a generic Docker Image.'
 fi
 

src/nginx/main/BUILD

@@ -250,7 +250,7 @@ pkg_deb(
     package = "endpoints-runtime",
     postinst = "@nginx_pkgoss//:debian_postinst",
     postrm = "@nginx_pkgoss//:debian_postrm",
-    preinst = "@nginx_pkgoss//:debian_preinst",
+    preinst = "nginx.preinst",
     prerm = "@nginx_pkgoss//:debian_prerm",
     priority = "extra",
     section = "httpd",

src/nginx/main/nginx.preinst

@@ -0,0 +1,60 @@
+#! /bin/sh
+# preinst script for nginx
+
+set -e
+
+addnginxuser() {
+    # creating nginx group if he isn't already there
+    if ! getent group nginx >/dev/null; then
+        addgroup --system nginx >/dev/null
+    fi
+
+    # creating nginx user if he isn't already there
+    if ! getent passwd nginx >/dev/null; then
+        adduser \
+          --system \
+          --disabled-login \
+          --ingroup nginx \
+          --home /home/nginx \
+          --gecos "nginx user" \
+          nginx  >/dev/null
+    fi
+}
+
+case "$1" in
+    install)
+        addnginxuser
+        cat <<BANNER
+----------------------------------------------------------------------
+
+Thanks for using nginx!
+
+Please find the official documentation for nginx here:
+* http://nginx.org/en/docs/
+
+Please subscribe to nginx-announce mailing list to get
+the most important news about nginx:
+* http://nginx.org/en/support.html
+
+Commercial subscriptions for nginx are available on:
+* http://nginx.com/products/
+
+----------------------------------------------------------------------
+BANNER
+        ;;
+    upgrade)
+        addnginxuser
+        ;;
+
+    abort-upgrade)
+        ;;
+
+    *)
+        echo "preinst called with unknown argument \`$1'" >&2
+        exit 0
+        ;;
+esac
+
+
+
+exit 0

start_esp/nginx-auto.conf.template

@@ -136,7 +136,7 @@ http {
       # Begin Endpoints v2 Support
       endpoints {
         on;
-        server_config /etc/nginx/server_config.pb.txt;
+        server_config ${server_config_path};
 % if service_account:
         google_authentication_secret ${service_account};
 % endif

start_esp/start_esp.py

@@ -50,9 +50,9 @@
 NGINX_CONF_TEMPLATE = "/etc/nginx/nginx-auto.conf.template"
 SERVER_CONF_TEMPLATE = "/etc/nginx/server-auto.conf.template"
 # Custom nginx config used by customers are hardcoded to this path
-SERVER_CONF = "/etc/nginx/server_config.pb.txt"
-# Default service config output directory when multiple services are proxied
-SERVER_CONF_DIR = "/etc/nginx/"
+# Flex nginx config is hardcoded to use this path
+SERVER_CONF_DIR = "/etc/nginx"
+SERVER_CONF_NAME = "/server_config.pb.txt"
 
 # Location of generated config files
 CONFIG_DIR = "/etc/nginx/endpoints"
@@ -90,7 +90,7 @@
 DEFAULT_XFF_TRUSTED_PROXY_LIST = "0.0.0.0/0, 0::/0"
 
 # Default PID file location (for nginx as a daemon)
-DEFAULT_PID_FILE = "/var/run/nginx.pid"
+DEFAULT_PID_FILE = "/home/nginx/nginx.pid"
 
 # Default nginx worker_processes
 DEFAULT_WORKER_PROCESSES = "1"
@@ -125,7 +125,7 @@ def write_pid_file(args):
         logging.error(err.strerror)
         sys.exit(3)
 
-def write_template(ingress, nginx_conf, args):
+def write_nginx_conf(ingress, nginx_conf, args):
     # Load template
     try:
         template = Template(filename=args.template)
@@ -160,6 +160,7 @@ def write_template(ingress, nginx_conf, args):
             cors_allow_credentials=args.cors_allow_credentials,
             cors_expose_headers=args.cors_expose_headers,
             ssl_protocols=args.ssl_protocols,
+            server_config_path=args.server_config_path,
             experimental_proxy_backend_host_header=args.experimental_proxy_backend_host_header,
             enable_strict_transport_security=args.enable_strict_transport_security,
             google_cloud_platform=(args.non_gcp==False))
@@ -662,6 +663,16 @@ def make_argparser():
         default=CONFIG_DIR,
         help=argparse.SUPPRESS)
 
+    # The server_config dir to save server configs
+    parser.add_argument('--server_config_dir',
+        default=SERVER_CONF_DIR,
+        help='''Sets the folder for writting server config file.
+        The server config file is passed to ESP in Nginx config.
+        If you are using your own nginx config,
+        please be sure its server_config path matches this one.
+        If you want to make esp container root file system read-only
+        for security, please change this folder to /home/nginx.''')
+
     # nginx.conf template
     parser.add_argument('--template',
         default=NGINX_CONF_TEMPLATE,
@@ -926,10 +937,15 @@ def enforce_conflict_args(args):
             return "Flag --enable_backend_routing cannot be used together with --dns."
         if args.ssl_protocols:
             return "Flag --enable_backend_routing cannot be used together with --ssl_protocols."
+        if args.server_config_dir != SERVER_CONF_DIR:
+            return "Flag --enable_backend_routing cannot be used together with --server_config_dir."
 
         # When --enable_backend_routing is specified, set some default value to some of its conflicting flags.
         args.disable_cloud_trace_auto_sampling = True
         args.access_log = 'off'
+        # For backend routing eabled config, use /home/nginx directly.
+        args.server_config_dir = "/home/nginx"
+        args.config_dir = "/home/nginx/endpoints"
     return None
 
 if __name__ == '__main__':
@@ -980,6 +996,7 @@ def enforce_conflict_args(args):
 
     # Generate server_config
     args.metadata_attributes = fetch.fetch_metadata_attributes(args.metadata)
+    args.server_config_path = args.server_config_dir + SERVER_CONF_NAME
     if args.generate_config_file_only:
         # When generate_config_file_only, metadata_attributes is set as empty
         # to have consistent test results on local bazel test and jenkins test
@@ -991,9 +1008,10 @@ def enforce_conflict_args(args):
         else:
             write_server_config_template(args.server_config_generation_path, args)
     else:
-        server_config_path = SERVER_CONF
+        ensure(args.server_config_dir)
+        server_config_path = args.server_config_path
         if args.service and '|' in args.service:
-            server_config_path = SERVER_CONF_DIR
+            server_config_path = args.server_config_dir + "/"
         if args.server_config_generation_path:
             server_config_path = args.server_config_generation_path
         write_server_config_template(server_config_path, args)
@@ -1004,7 +1022,7 @@ def enforce_conflict_args(args):
         ingress = make_ingress(args)
         nginx_conf = args.config_dir + "/nginx.conf"
         ensure(args.config_dir)
-        write_template(ingress, nginx_conf, args)
+        write_nginx_conf(ingress, nginx_conf, args)
 
     if args.generate_config_file_only:
         exit(0)

start_esp/test/BUILD

@@ -85,8 +85,9 @@ py_test(
         "testdata/expected_metadata_nginx.conf",
         "testdata/expected_rewrite_server.json",
         "testdata/expected_rollout_strategy_server.json",
-        "testdata/expected_service_control_url_override_server.json",
+        "testdata/expected_server_config_dir_nginx.conf",
         "testdata/expected_service_control_log_entries.json",
+        "testdata/expected_service_control_url_override_server.json",
         "testdata/expected_ssl_port_nginx.conf",
         "testdata/expected_ssl_protocols_nginx.conf",
         "testdata/expected_status_port_nginx.conf",

start_esp/test/start_esp_test.py

@@ -211,6 +211,11 @@ def test_enable_strict_transport_security_is_as_expected(self):
         config_generator = self.basic_config_generator + " --enable_strict_transport_security"
         self.run_test_with_expectation(expected_config_file, self.generated_nginx_config_file, config_generator)
 
+    def test_server_config_dir_is_as_expected(self):
+        expected_config_file = "./start_esp/test/testdata/expected_server_config_dir_nginx.conf"
+        config_generator = self.basic_config_generator + " --server_config_dir /home/nginx"
+        self.run_test_with_expectation(expected_config_file, self.generated_nginx_config_file, config_generator)
+
     ########## The tests for generating the server configuration file start from here ##########
 
     def test_service_control_url_override_arg_output_is_as_expected(self):