From ffb4910b078f119ada827ae0fdc35f29666ca348 Mon Sep 17 00:00:00 2001 From: Miroslav Shubernetskiy Date: Mon, 4 Nov 2024 16:15:39 -0500 Subject: [PATCH 1/2] fix: split attestation bits from metsys plugin currently metsys plugin was reporting global metadata keys such as chalk run time as well as generating attestation bits such as artifact signature. this meant that disabling metsys plugin to avoid assigning metadata id, etc would also not report global keys such as chalk run time this was happening for docker push when pushing non-chalked image --- src/collect.nim | 7 ++++--- src/configs/base_plugins.c4m | 19 +++++++++++++++---- src/docker/push.nim | 4 ++-- src/plugins/system.nim | 14 ++++++++------ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/collect.nim b/src/collect.nim index d8044643..433724db 100644 --- a/src/collect.nim +++ b/src/collect.nim @@ -10,7 +10,7 @@ import "./docker"/[scan] import "."/[config, plugin_api] proc isSystem*(p: Plugin): bool = - return p.name in ["system", "metsys"] + return p.name in ["system", "attestation", "metsys"] proc hasSubscribedKey(p: Plugin, keys: seq[string], dict: ChalkDict): bool = # Decides whether to run a given plugin... does it export any key we @@ -45,9 +45,10 @@ proc canWrite(plugin: Plugin, key: string, decls: seq[string]): bool = if not attrGet[bool](section & ".system"): return true - case plugin.name - of "system", "metsys": + if plugin.isSystem(): return true + + case plugin.name of "conffile": if attrGet[bool](section & ".conf_as_system"): return true diff --git a/src/configs/base_plugins.c4m b/src/configs/base_plugins.c4m index c5379583..b09bbb71 100644 --- a/src/configs/base_plugins.c4m +++ b/src/configs/base_plugins.c4m @@ -58,18 +58,29 @@ are not overridable via other plugins. # # The priority field is set to high(int64). -plugin metsys { +plugin attestation { ~enabled: true pre_chalk_keys: ["METADATA_HASH", "ERR_INFO", "FAILED_KEYS", "METADATA_ID", "SIGNING", "SIGNATURE", "INJECTOR_PUBLIC_KEY"] post_chalk_keys: ["_SIGNATURES"] + ~priority: high() - 1 + doc: """ +Like the `system` module, this module is non-overridable keys added by +Chalk. It's just the ones that need to be computed at the very end of +chalk-time data collection phase, so integrity / signing and audit. +""" +} + +plugin metsys { + ~enabled: true post_run_keys: ["_OP_ERRORS", "_OP_FAILED_KEYS", "_CHALK_EXTERNAL_ACTION_AUDIT", "_CHALK_RUN_TIME", "_OP_EXIT_CODE"] ~priority: high() doc: """ Like the `system` module, this module is non-overridable keys added by Chalk. It's just the ones that need to be computed at the very end of -a data collection phase, so integrity / signing and audit. +a run-time collection phase about the whole operation +such as the overall chalk run time. """ } @@ -313,7 +324,7 @@ plugin conffile { pre_chalk_keys: ["*"] # Chalk-only keys are evaluated here. post_chalk_keys: ["*"] # Non-chalkable artifact keys here. post_run_keys: ["*"] # Post-run keys here. - priority: high() - 1 + priority: high() - 2 doc: """ This plugin is responsible for collecting any values explicitly set in the configuration file by the user. The user can set values statically @@ -392,7 +403,7 @@ plugin elf_last_resort { codec: true pre_chalk_keys: ["ARTIFACT_TYPE"] post_chalk_keys: ["_CURRENT_HASH", "_OP_ARTIFACT_TYPE"] - ~priority: high() - 1 + ~priority: high() - 3 doc: """ This codec is only used for operating on chalk marks for oddball hand-written elf-files. It works by leveraging the fact that appending diff --git a/src/docker/push.nim b/src/docker/push.nim index 38dada97..35a969e9 100644 --- a/src/docker/push.nim +++ b/src/docker/push.nim @@ -17,7 +17,7 @@ proc dockerPush*(ctx: DockerInvocation): int = if chalkOpt.isNone(): error("docker: " & ctx.foundImage & " is not found. pushing without chalk") - return ctx.runMungedDockerInvocation() + return setExitCode(ctx.runMungedDockerInvocation()) # force DOCKER_PLATFORM to be included in chalk normalization # which is required to compute unique METADATA_* keys @@ -30,7 +30,7 @@ proc dockerPush*(ctx: DockerInvocation): int = # so they create things like CHALK_ID, METADATA_ID # but we just want to report keys about the artifact # without "creating" new chalkmark so we chalk-time collection - suspendChalkCollectionFor("metsys") + suspendChalkCollectionFor("attestation") suspendChalkCollectionFor("docker") initCollection() diff --git a/src/plugins/system.nim b/src/plugins/system.nim index f102ca71..b88beff4 100644 --- a/src/plugins/system.nim +++ b/src/plugins/system.nim @@ -276,8 +276,8 @@ proc sysGetChalkTimeHostInfo*(self: Plugin): ChalkDict {.cdecl.} = let selfIdOpt = selfID if selfIdOpt.isSome(): result["INJECTOR_CHALK_ID"] = pack(selfIdOpt.get()) -proc metsysGetChalkTimeArtifactInfo*(self: Plugin, obj: ChalkObj): - ChalkDict {.cdecl.} = +proc attestationGetChalkTimeArtifactInfo*(self: Plugin, obj: ChalkObj): + ChalkDict {.cdecl.} = result = ChalkDict() # We add these directly into collectedData so that it can get @@ -300,8 +300,8 @@ proc metsysGetChalkTimeArtifactInfo*(self: Plugin, obj: ChalkObj): except: error("Cannot sign " & obj.name & ": " & getCurrentExceptionMsg()) -proc metsysGetRunTimeArtifactInfo(self: Plugin, obj: ChalkObj, insert: bool): - ChalkDict {.cdecl.} = +proc attestationGetRunTimeArtifactInfo(self: Plugin, obj: ChalkObj, insert: bool): + ChalkDict {.cdecl.} = result = ChalkDict() if insert and obj.willSignBySigStore(): try: @@ -333,7 +333,9 @@ proc loadSystem*() = rtArtCallback = RunTimeArtifactCb(sysGetRunTimeArtifactInfo), rtHostCallback = RunTimeHostCb(sysGetRunTimeHostInfo)) + newPlugin("attestation", + ctArtCallback = ChalkTimeArtifactCb(attestationGetChalkTimeArtifactInfo), + rtArtCallback = RunTimeArtifactCb(attestationGetRunTimeArtifactInfo)) + newPlugin("metsys", - ctArtCallback = ChalkTimeArtifactCb(metsysGetChalkTimeArtifactInfo), - rtArtCallback = RunTimeArtifactCb(metsysGetRunTimeArtifactInfo), rtHostCallback = RunTimeHostCb(metsysGetRunTimeHostInfo)) From 1d00168052e0dba59a9ada7011234e579f56d918 Mon Sep 17 00:00:00 2001 From: Miroslav Shubernetskiy Date: Tue, 5 Nov 2024 09:18:03 -0500 Subject: [PATCH 2/2] test: checking metsys keys are present for non-chalk pushes --- CHANGELOG.md | 3 +++ tests/functional/test_docker.py | 8 ++++++++ tests/functional/testing.c4m | 1 + tests/functional/utils/docker.py | 2 +- 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e46e4887..17238e28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ - When running `semgrep`, its always added to `PATH`, as otherwise semgrep is not able to find `pysemgrep` folder. ([#439](https://github.com/crashappsec/chalk/pull/439)) +- Docker pushing non-chalked images did not report metsys + plugin keys such as `_EXIT_CODE`, `_CHALK_RUN_TIME`. + ([#438](https://github.com/crashappsec/chalk/pull/438)) ### New Features diff --git a/tests/functional/test_docker.py b/tests/functional/test_docker.py index 860b26a4..bb74aaeb 100644 --- a/tests/functional/test_docker.py +++ b/tests/functional/test_docker.py @@ -960,6 +960,14 @@ def test_build_and_push( assert pull.find("Digest:") == f"sha256:{image_digest}" +def test_push_nonchalked(chalk: Chalk, random_hex: str): + tag_base = f"{REGISTRY}/nonchalked_{random_hex}" + tag = f"{tag_base}:latest" + Docker.build(content="FROM alpine", tag=tag) + push = chalk.docker_push(tag) + assert push.report.has(_OP_EXIT_CODE=0, _CHALK_RUN_TIME=ANY) + + @pytest.mark.parametrize("test_file", ["valid/sample_1"]) def test_push_without_buildx( chalk: Chalk, diff --git a/tests/functional/testing.c4m b/tests/functional/testing.c4m index 1ebe46a9..9df2b15a 100644 --- a/tests/functional/testing.c4m +++ b/tests/functional/testing.c4m @@ -7,6 +7,7 @@ custom_report.terminal_other_op.use_when: ["extract", "delete", "exec", "env", " report_template insertion_default { key._OP_EXIT_CODE.use = true + key._CHALK_RUN_TIME.use = true } report_template report_default { diff --git a/tests/functional/utils/docker.py b/tests/functional/utils/docker.py index 4872e98b..a56b4e26 100644 --- a/tests/functional/utils/docker.py +++ b/tests/functional/utils/docker.py @@ -54,7 +54,7 @@ def build_cmd( for t in tags: cmd += ["-t", t] if content: - stdin = content.encode() + stdin = Docker.dockerfile(content).encode() cmd += ["-f", "-"] elif dockerfile: cmd += ["-f", str(dockerfile)]