Skip to content

Commit

Permalink
[python] Move from toml library to tomli + tomli-w
Browse files Browse the repository at this point in the history
`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 <dmitrii.kuvaiskii@intel.com>
  • Loading branch information
dimakuv committed Nov 25, 2022
1 parent 67b1b9f commit 740e169
Show file tree
Hide file tree
Showing 18 changed files with 93 additions and 56 deletions.
3 changes: 2 additions & 1 deletion .ci/ubuntu18.04.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down
3 changes: 2 additions & 1 deletion .ci/ubuntu20.04.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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'

Expand Down
8 changes: 4 additions & 4 deletions Documentation/devel/building.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand Down
2 changes: 1 addition & 1 deletion Documentation/requirements.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down
5 changes: 0 additions & 5 deletions libos/test/regression/attestation.manifest.template
Original file line number Diff line number Diff line change
Expand Up @@ -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) }}/",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
4 changes: 0 additions & 4 deletions libos/test/regression/device_passthrough.manifest.template
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
]
15 changes: 6 additions & 9 deletions libos/test/regression/file_check_policy_strict.manifest.template
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
]
4 changes: 4 additions & 0 deletions libos/test/regression/test_libos.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])

Expand Down
1 change: 1 addition & 0 deletions libos/test/regression/tests.toml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ manifests = [
"sysfs_common",
"tcp_ipv6_v6only",
"tcp_msg_peek",
"toml_parsing",
"udp",
"uid_gid",
"unix",
Expand Down
1 change: 1 addition & 0 deletions libos/test/regression/tests_musl.toml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ manifests = [
"sysfs_common",
"tcp_ipv6_v6only",
"tcp_msg_peek",
"toml_parsing",
"udp",
"uid_gid",
"unix",
Expand Down
51 changes: 51 additions & 0 deletions libos/test/regression/toml_parsing.manifest.template
Original file line number Diff line number Diff line change
@@ -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"
6 changes: 3 additions & 3 deletions python/gramine-gen-depend
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion python/gramine-manifest
Original file line number Diff line number Diff line change
Expand Up @@ -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)')
Expand Down
2 changes: 1 addition & 1 deletion python/gramine-sgx-sign
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions python/graminelibos/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
import os
import pathlib

import toml
import tomli
import tomli_w

from . import _env

Expand Down Expand Up @@ -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', [])
Expand All @@ -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:
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions python/graminelibos/util_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import subprocess
import sys

import toml
import tomli

from . import ninja_syntax, _CONFIG_SYSLIBDIR, _CONFIG_PKGLIBDIR

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 740e169

Please sign in to comment.