Add loader.env.[ENVIRON] = {passthrough=true} manifest option

It is now possible to define some envvars that are passed through from
the host inside the Graphene instance (previously the only way to pass
envvars was to set `loader.insecure__use_host_env`). A new LibOS
regression test is added.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>

Documentation/manifest-syntax.rst

@@ -156,10 +156,19 @@ both of the following options:
 ::
 
    loader.env.[ENVIRON] = "[VALUE]"
+   or
+   loader.env.[ENVIRON] = { value = "[VALUE]" }
+   or
+   loader.env.[ENVIRON] = { passthrough = [true|false] }
+
    loader.env_src_file = "file:file_with_serialized_envs"
 
-``loader.env.[ENVIRON]`` adds/overwrites a single environment variable and can
-be used multiple times to specify more than one variable.
+``loader.env.[ENVIRON]`` adds/overwrites/passes a single environment variable
+and can be used multiple times to specify more than one variable. To
+add/overwrite the environment variable, specify a TOML string (``"[VALUE]"``) or
+a TOML table with the key-value pair ``{ value = "[VALUE]" }``. To pass the
+environment variable from the host, specify a TOML table with the key-value pair
+``{ passthrough = true }``.
 
 ``loader.env_src_file`` allows to specify a URI to a file containing serialized
 environment, which can be generated using :file:`Tools/argv_serializer`. This
@@ -174,7 +183,17 @@ provided at runtime from an external (trusted) source.
    variables.
 
 If the same variable is set in both, then ``loader.env.[ENVIRON]`` takes
-precedence.
+precedence. It is prohibited to specify both ``value`` and ``passthrough`` keys
+for the same environment variable. If manifest option ``insecure__use_host_env``
+is specified, then ``passthrough = true`` manifest options have no effect (they
+are "consumed" by ``insecure__use_host_env``).
+
+.. note ::
+   It is tempting to try to passthrough all environment variables using
+   ``insecure__use_host_env`` and then disallow some of them using ``passthrough
+   = false``. However, this deny list approach is intentionally prohibited.
+   Graphene loudly fails if any ``passthrough = false`` manifest options are set
+   together with ``insecure__use_host_env``.
 
 Disabling ASLR
 ^^^^^^^^^^^^^^

LibOS/shim/test/regression/.gitignore

@@ -25,6 +25,7 @@
 /double_fork
 /env_from_file
 /env_from_host
+/env_passthrough
 /epoll_epollet
 /epoll_wait_timeout
 /eventfd

LibOS/shim/test/regression/Makefile

@@ -107,6 +107,7 @@ repo_manifests = \
 	device_passthrough.manifest \
 	env_from_file.manifest \
 	env_from_host.manifest \
+	env_passthrough.manifest \
 	file_check_policy_allow_all_but_log.manifest \
 	file_check_policy_strict.manifest \
 	multi_pthread_exitless.manifest

LibOS/shim/test/regression/env_passthrough.manifest.template

@@ -0,0 +1,20 @@
+loader.preload = "file:{{ graphene.libos }}"
+loader.argv0_override = "bootstrap"
+libos.entrypoint = "bootstrap"
+
+loader.env.LD_LIBRARY_PATH = "/lib"
+
+loader.env.A = { passthrough = true }
+loader.env.B = { value = "OVERWRITTEN_VALUE" }
+loader.env.C = { passthrough = false }
+
+# loader.env.E = { passthrough = true, value = "THIS_IS_INCORRECT_SYNTAX" }
+
+fs.mount.lib.type = "chroot"
+fs.mount.lib.path = "/lib"
+fs.mount.lib.uri = "file:{{ graphene.runtimedir() }}"
+
+sgx.trusted_files.runtime = "file:{{ graphene.runtimedir() }}/"
+sgx.trusted_files.bootstrap = "file:bootstrap"
+
+sgx.nonpie_binary = true

LibOS/shim/test/regression/test_libos.py

@@ -105,15 +105,33 @@ def test_104_env_from_file(self):
         finally:
             os.remove('env_test_input')
 
+    def test_105_env_passthrough(self):
+        host_envs = {
+            'A': 'THIS_WILL_BE_PASSED',
+            'B': 'THIS_WILL_BE_OVERWRITTEN',
+            'C': 'THIS_SHOULDNT_BE_PASSED',
+            'D': 'THIS_SHOULDNT_BE_PASSED_TOO',
+        }
+        manifest_envs = {'LD_LIBRARY_PATH': '/lib'}
+        stdout, _ = self.run_binary(['env_passthrough'], env=host_envs)
+        self.assertIn('# of envs: %d\n' % (len(host_envs) - 2 + len(manifest_envs)), stdout)
+
+        # We don't enforce any specific order of envs, so we skip checking the index.
+        self.assertIn('] = LD_LIBRARY_PATH=/lib\n', stdout)
+        self.assertIn('] = A=THIS_WILL_BE_PASSED\n', stdout)
+        self.assertIn('] = B=OVERWRITTEN_VALUE\n', stdout)
+        self.assertNotIn('] = C=THIS_SHOULDNT_BE_PASSED\n', stdout)
+        self.assertNotIn('] = D=THIS_SHOULDNT_BE_PASSED_TOO\n', stdout)
+
     @unittest.skipUnless(HAS_SGX,
         'This test is only meaningful on SGX PAL because only SGX catches raw '
         'syscalls and redirects to Graphene\'s LibOS. If we will add seccomp to '
         'Linux PAL, then we should allow this test on Linux PAL as well.')
-    def test_105_basic_bootstrapping_static(self):
+    def test_106_basic_bootstrapping_static(self):
         stdout, _ = self.run_binary(['bootstrap_static'])
         self.assertIn('Hello world (bootstrap_static)!', stdout)
 
-    def test_106_basic_bootstrapping_pie(self):
+    def test_107_basic_bootstrapping_pie(self):
         stdout, _ = self.run_binary(['bootstrap_pie'])
         self.assertIn('User program started', stdout)
         self.assertIn('Local Address in Executable: 0x', stdout)