[python] Move from toml library to tomli + tomli-w

`toml` Python library (for parsing and generating TOML files) is buggy and doesn't support the full spec of TOML 1.0.0. This commit replaces it with more robust `tomli` (for parsing) and `tomli-w` (for generating). New LibOS regression test is added that combines all tricky/legacy TOML syntax, including the checks for TOML-syntax bugs. Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
gramineproject · Nov 25, 2022 · 740e169 · 740e169
1 parent 67b1b9f
commit 740e169
Show file tree

Hide file tree

Showing 18 changed files with 93 additions and 56 deletions.
diff --git a/.ci/ubuntu18.04.dockerfile b/.ci/ubuntu18.04.dockerfile
@@ -85,7 +85,8 @@ RUN git clone https://github.com/giltene/wrk2.git \
 RUN python3 -m pip install -U \
     'Sphinx==1.8' \
     'sphinx_rtd_theme<1' \
-    'toml>=0.10' \
+    'tomli>=1.1.0' \
+    'tomli-w>=0.4.0' \
     'meson>=0.56,<0.57' \
     'docutils>=0.17,<0.18'
 

diff --git a/.ci/ubuntu20.04.dockerfile b/.ci/ubuntu20.04.dockerfile
@@ -63,7 +63,6 @@ RUN apt-get update && env DEBIAN_FRONTEND=noninteractive apt-get install -y \
     python3-pytest-xdist \
     python3-scipy \
     python3-sphinx-rtd-theme \
-    python3-toml \
     shellcheck \
     sphinx-doc \
     sqlite3 \
@@ -86,6 +85,8 @@ RUN git clone https://github.com/giltene/wrk2.git \
 # the earliest supported minor version (pip implicitly installs latest version satisfying the
 # specification)
 RUN python3 -m pip install -U \
+    'tomli>=1.1.0' \
+    'tomli-w>=0.4.0' \
     'meson>=0.56,<0.57' \
     'docutils>=0.17,<0.18'
 

diff --git a/Documentation/devel/building.rst b/Documentation/devel/building.rst
@@ -38,11 +38,11 @@ Run the following command on Ubuntu LTS to install dependencies::
     sudo apt-get install -y build-essential \
         autoconf bison gawk nasm ninja-build pkg-config python3 python3-click \
         python3-jinja2 python3-pip python3-pyelftools wget
-    sudo python3 -m pip install 'meson>=0.56' 'toml>=0.10'
+    sudo python3 -m pip install 'meson>=0.56' 'tomli>=1.1.0' 'tomli-w>=0.4.0'
 
-You can also install Meson and python3-toml from apt instead of pip, but only if
-your distro is new enough to have Meson >= 0.56 and python3-toml >= 0.10 (Debian
-11, Ubuntu 20.10).
+You can also install Meson, python3-tomli and python3-tomli-w from apt instead
+of pip, but only if your distro is new enough to have Meson >= 0.56,
+python3-tomli >= 1.1.0 and python3-tomli-w >= 0.4.0 (e.g. Ubuntu 22.04).
 
 For GDB support and to run all tests locally you also need to install::
 

diff --git a/Documentation/requirements.txt b/Documentation/requirements.txt
@@ -1,7 +1,7 @@
 sphinx==1.8.0
 breathe<4.13.0
 sphinx_rtd_theme<1
-toml>=0.10
+tomli>=1.1.0
 
 # Work around Sphinx/docutils incompatibility, see https://github.com/sphinx-doc/sphinx/issues/9727.
 # TODO: This shouldn't be necessary once we upgrade to newer Sphinx (Sphinx 4.2.0 is not yet

diff --git a/libos/test/regression/attestation.manifest.template b/libos/test/regression/attestation.manifest.template
@@ -18,11 +18,6 @@ sgx.remote_attestation = "{{ env.get('RA_TYPE', 'none') }}"
 sgx.ra_client_spid = "{{ env.get('RA_CLIENT_SPID', '') }}"
 sgx.ra_client_linkable = {{ 'true' if env.get('RA_CLIENT_LINKABLE', '0') == '1' else 'false' }}
 
-# below three entries are irrelevant, only for test purposes
-sgx.seal_key.flags_mask = "0xffffffffffffffff"
-sgx.seal_key.xfrm_mask  = "0xfffffffffff9ff1b"
-sgx.seal_key.misc_mask  = "0xffffffff"
-
 sgx.trusted_files = [
   "file:{{ gramine.libos }}",
   "file:{{ gramine.runtimedir(libc) }}/",

diff --git a/libos/test/regression/attestation_deprecated_syntax.manifest.template b/libos/test/regression/attestation_deprecated_syntax.manifest.template
@@ -6,16 +6,10 @@ libos.entrypoint = "{{ entrypoint }}"
 loader.env.LD_LIBRARY_PATH = "/lib"
 loader.insecure__use_cmdline_argv = true
 
-# Keep the deprecated `fs.mount` syntax for test purposes
-# TODO: this syntax is deprecated in v1.2 and will be removed two versions after it.
-
-fs.mount.lib.type = "chroot"
-fs.mount.lib.path = "/lib"
-fs.mount.lib.uri = "file:{{ gramine.runtimedir(libc) }}"
-
-fs.mount.entrypoint.type = "chroot"
-fs.mount.entrypoint.path = "{{ entrypoint }}"
-fs.mount.entrypoint.uri = "file:{{ binary_dir }}/{{ entrypoint }}"
+fs.mounts = [
+  { path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" },
+  { path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" },
+]
 
 sgx.insecure__protected_files_key = "ffeeddccbbaa99887766554433221100"
 

diff --git a/libos/test/regression/device_passthrough.manifest.template b/libos/test/regression/device_passthrough.manifest.template
@@ -3,10 +3,6 @@ libos.entrypoint = "{{ entrypoint }}"
 
 loader.env.LD_LIBRARY_PATH = "/lib"
 
-# the manifest option below has no significance for this specific test, it's added only so that this
-# feature has any test coverage
-libos.check_invalid_pointers = false
-
 fs.mounts = [
   { path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" },
   { path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" },

diff --git a/libos/test/regression/file_check_policy_allow_all_but_log.manifest.template b/libos/test/regression/file_check_policy_allow_all_but_log.manifest.template
@@ -22,12 +22,7 @@ sgx.file_check_policy = "allow_all_but_log"
 # entries in `sgx.trusted_files` -- this is on purpose; we want to test that
 # `allow_all_but_log` also applies to Gramine-runtime files (e.g., LibOS binary)
 
-# below entry in sgx.trusted_files is to test TOML-table syntax without `sha256`
-[[sgx.trusted_files]]
-uri = "file:{{ binary_dir }}/{{ entrypoint }}"
-
-# below entry in sgx.trusted_files is for testing purposes (trusted_testfile has
-# hard-coded contents, so we can use pre-calculated SHA256 hash)
-[[sgx.trusted_files]]
-uri = "file:trusted_testfile"
-sha256 = "41dacdf1e6d0481d3b1ab1a91f93139db02b96f29cfdd3fb0b819ba1e33cafc4"
+sgx.trusted_files = [
+  "file:{{ binary_dir }}/{{ entrypoint }}",
+  { uri = "file:trusted_testfile", sha256 = "41dacdf1e6d0481d3b1ab1a91f93139db02b96f29cfdd3fb0b819ba1e33cafc4" },
+]
diff --git a/libos/test/regression/file_check_policy_strict.manifest.template b/libos/test/regression/file_check_policy_strict.manifest.template
@@ -21,14 +21,11 @@ sgx.file_check_policy = "strict"
 sgx.trusted_files = [
   "file:{{ gramine.libos }}",
   "file:{{ gramine.runtimedir(libc) }}/",
-]
 
-# below entry in sgx.trusted_files is to test TOML-table syntax without `sha256`
-[[sgx.trusted_files]]
-uri = "file:{{ binary_dir }}/{{ entrypoint }}"
+  # test TOML inline table syntax without `sha256`
+  { uri = "file:{{ binary_dir }}/{{ entrypoint }}" },
 
-# below entry in sgx.trusted_files is for testing purposes (trusted_testfile has
-# hard-coded contents, so we can use pre-calculated SHA256 hash)
-[[sgx.trusted_files]]
-uri = "file:trusted_testfile"
-sha256 = "41dacdf1e6d0481d3b1ab1a91f93139db02b96f29cfdd3fb0b819ba1e33cafc4"
+  # test TOML inline table syntax with `sha256` (trusted_testfile has hard-coded contents, so we can
+  # use pre-calculated SHA256 hash)
+  { uri = "file:trusted_testfile", sha256 = "41dacdf1e6d0481d3b1ab1a91f93139db02b96f29cfdd3fb0b819ba1e33cafc4" },
+]
diff --git a/libos/test/regression/test_libos.py b/libos/test/regression/test_libos.py
@@ -68,6 +68,10 @@ def test_001_helloworld(self):
         stdout, _ = self.run_binary(['helloworld'])
         self.assertIn('Hello world!', stdout)
 
+    def test_002_toml_parsing(self):
+        stdout, _ = self.run_binary(['toml_parsing'])
+        self.assertIn('Hello world!', stdout)
+
     def test_100_basic_bootstrapping(self):
         stdout, _ = self.run_binary(['bootstrap'])
 

diff --git a/libos/test/regression/tests.toml b/libos/test/regression/tests.toml
@@ -109,6 +109,7 @@ manifests = [
   "sysfs_common",
   "tcp_ipv6_v6only",
   "tcp_msg_peek",
+  "toml_parsing",
   "udp",
   "uid_gid",
   "unix",

diff --git a/libos/test/regression/tests_musl.toml b/libos/test/regression/tests_musl.toml
@@ -110,6 +110,7 @@ manifests = [
   "sysfs_common",
   "tcp_ipv6_v6only",
   "tcp_msg_peek",
+  "toml_parsing",
   "udp",
   "uid_gid",
   "unix",

diff --git a/libos/test/regression/toml_parsing.manifest.template b/libos/test/regression/toml_parsing.manifest.template
@@ -0,0 +1,51 @@
+# This test contains tricky/legacy TOML syntax, just to test TOML parsing and have some coverage
+
+{% set entrypoint = "helloworld" -%}
+
+loader.entrypoint = "file:{{ gramine.libos }}"
+libos.entrypoint = "{{ entrypoint }}"
+
+loader.env.LD_LIBRARY_PATH = "/lib"
+
+# keep the deprecated `fs.mount` syntax for test purposes
+# TODO: this syntax is deprecated in v1.2 and will be removed two versions after it.
+
+fs.mount.lib.type = "chroot"
+fs.mount.lib.path = "/lib"
+fs.mount.lib.uri = "file:{{ gramine.runtimedir(libc) }}"
+
+fs.mount.entrypoint.type = "chroot"
+fs.mount.entrypoint.path = "{{ entrypoint }}"
+fs.mount.entrypoint.uri = "file:{{ binary_dir }}/{{ entrypoint }}"
+
+# the manifest option below added only so that this feature has any test coverage
+libos.check_invalid_pointers = false
+
+sgx.nonpie_binary = true
+sgx.debug = true
+
+# the manifest options below added only so that they have any test coverage
+sgx.seal_key.flags_mask = "0xffffffffffffffff"
+sgx.seal_key.xfrm_mask  = "0xfffffffffff9ff1b"
+sgx.seal_key.misc_mask  = "0xffffffff"
+
+# below format of sgx.trusted_files is to test TOML-table syntax without `sha256`
+[[sgx.trusted_files]]
+uri = "file:{{ gramine.libos }}"
+
+[[sgx.trusted_files]]
+uri = "file:{{ gramine.runtimedir(libc) }}/"
+
+[[sgx.trusted_files]]
+uri = "file:{{ binary_dir }}/{{ entrypoint }}"
+
+# below entry is to test TOML-table syntax with `sha256` (trusted_testfile has hard-coded contents,
+# so we can use pre-calculated SHA256 hash)
+[[sgx.trusted_files]]
+uri = "file:trusted_testfile"
+sha256 = "41dacdf1e6d0481d3b1ab1a91f93139db02b96f29cfdd3fb0b819ba1e33cafc4"
+
+# below entry is to test parsing of `\\x2d` sequence (previously-used `toml` Python parser had bug)
+[[sgx.trusted_files]]
+uri = "file:nonexisting\\x2dfile"
+sha256 = "0123456789012345678901234567890123456789012345678901234567890123"
diff --git a/python/gramine-gen-depend b/python/gramine-gen-depend
@@ -10,12 +10,12 @@ import click
 from graminelibos import Manifest, _CONFIG_PKGLIBDIR
 
 @click.command()
-@click.option('--manifest', '-m', 'manifest_file', type=click.File('r', encoding='utf-8'),
-              required=True, help='Input .manifest file')
+@click.option('--manifest', '-m', 'manifest_file', type=click.File('rb'), required=True,
+              help='Input .manifest file')
 @click.option('--libpal', '-l', type=click.Path(exists=True, dir_okay=False),
               default=os.path.join(_CONFIG_PKGLIBDIR, 'sgx/libpal.so'), help='Input libpal file',
               show_default=True)
-@click.option('--output', '-o', type=click.File('w', encoding='utf-8'), required=True,
+@click.option('--output', '-o', type=click.File('wb'), required=True,
               help='Output .manifest.d file')
 def main(manifest_file, libpal, output):
     manifest = Manifest.load(manifest_file)

diff --git a/python/gramine-manifest b/python/gramine-manifest
@@ -21,7 +21,7 @@ def validate_define(_ctx, _param, values):
 @click.option('--string', '-c')
 @click.option('--define', '-D', multiple=True, callback=validate_define)
 @click.argument('infile', type=click.File('r'), required=False)
-@click.argument('outfile', type=click.File('w'), default='-')
+@click.argument('outfile', type=click.File('wb'), default='-')
 def main(string, define, infile, outfile):
     if not bool(string) ^ bool(infile):
         click.get_current_context().fail('specify exactly one of (infile, -c)')

diff --git a/python/gramine-sgx-sign b/python/gramine-sgx-sign
@@ -33,7 +33,7 @@ def main(output, libpal, key, manifest_file, sigfile, depfile, verbose):
 
     expanded = manifest.expand_all_trusted_files()
 
-    with open(output, 'w', encoding='utf-8') as f:
+    with open(output, 'wb') as f:
         manifest.dump(f)
 
     if not sigfile:

diff --git a/python/graminelibos/manifest.py b/python/graminelibos/manifest.py
@@ -12,7 +12,8 @@
 import os
 import pathlib
 
-import toml
+import tomli
+import tomli_w
 
 from . import _env
 
@@ -82,7 +83,7 @@ class Manifest:
     """
 
     def __init__(self, manifest_str):
-        manifest = toml.loads(manifest_str)
+        manifest = tomli.loads(manifest_str)
 
         sgx = manifest.setdefault('sgx', {})
         sgx.setdefault('trusted_files', [])
@@ -109,7 +110,6 @@ def __init__(self, manifest_str):
             raise ValueError("Unsupported trusted files syntax, more info: " +
                   "https://gramine.readthedocs.io/en/latest/manifest-syntax.html#trusted-files")
 
-        # Current toml versions (< 1.0) do not support non-homogeneous arrays
         trusted_files = []
         for tf in sgx['trusted_files']:
             if isinstance(tf, dict) and 'uri' in tf:
@@ -155,10 +155,10 @@ def load(cls, f):
         return cls.loads(f.read())
 
     def dumps(self):
-        return toml.dumps(self._manifest)
+        return tomli_w.dumps(self._manifest)
 
     def dump(self, f):
-        toml.dump(self._manifest, f)
+        tomli_w.dump(self._manifest, f)
 
     def expand_all_trusted_files(self):
         """Expand all trusted files entries.

diff --git a/python/graminelibos/util_tests.py b/python/graminelibos/util_tests.py
@@ -8,7 +8,7 @@
 import subprocess
 import sys
 
-import toml
+import tomli
 
 from . import ninja_syntax, _CONFIG_SYSLIBDIR, _CONFIG_PKGLIBDIR
 
@@ -47,7 +47,8 @@ class TestConfig:
     def __init__(self, path):
         self.config_path = path
 
-        data = toml.load(path)
+        with open(path, "rb") as f:
+            data = tomli.load(f)
 
         self.manifests = self.get_manifests(data)
         arch = platform.machine()