Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Enable (most) wrapping when seeing USER directive #34

Merged
merged 10 commits into from
Oct 6, 2023
4 changes: 4 additions & 0 deletions src/chalk_common.nim
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ type
rawSrc*: seq[string]
rawDst*: string

DfUserInfo* = ref object of InfoBase
str*: string

LabelInfo* = ref object of InfoBase
labels*: OrderedTable[string, string]

Expand All @@ -238,6 +241,7 @@ type
entryPoint*: EntryPointInfo
cmd*: CmdInfo
shell*: ShellInfo
lastUser*: DfUserInfo

DockerInvocation* = ref object
dockerExe*: string
Expand Down
2 changes: 2 additions & 0 deletions src/commands/cmd_docker.nim
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ proc runMungedDockerInvocation*(ctx: DockerInvocation): int =
trace("Running docker: " & dockerExeLocation & " " & args.join(" "))

if ctx.dfPassOnStdin:
if not ctx.inDockerFile.endswith("\n"):
ctx.inDockerFile &= "\n"
newStdin = ctx.inDockerFile & ctx.addedInstructions.join("\n")
trace("Passing on stdin: \n" & newStdin)

Expand Down
45 changes: 38 additions & 7 deletions src/docker_base.nim
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import osproc, config, util, reporting

var
buildXVersion: float = 0 # Major and minor only
dockerVersion: string = ""

const
hashHeader* = "sha256:"
Expand Down Expand Up @@ -62,6 +63,19 @@ proc getBuildXVersion*(): float =

return buildXVersion

proc getDockerVersion*(): string =
once:
let (output, exitcode) = execCmdEx(dockerExeLocation & " version")
if exitcode == 0:
let words = output.split(" ")

for item in words:
if '.' in item:
dockerVersion = item
break

return dockerVersion

template haveBuildContextFlag*(): bool =
buildXVersion >= 0.8

Expand Down Expand Up @@ -105,12 +119,15 @@ proc makeFileAvailableToDocker*(ctx: DockerInvocation,
once:
trace("Docker injection method: --build-context")

var chmodstr = ""

if chmod:
chmodstr = "--chmod=0755 "

ctx.newCmdLine.add("--build-context")
ctx.newCmdLine.add("chalkexedir" & $(contextCounter) & "=\"" & dir & "\"")
ctx.addedInstructions.add("COPY --from=chalkexedir" & $(contextCounter) &
" " & file & " /" & newname)
if chmod:
ctx.addedInstructions.add("RUN chmod 0755 /" & newname)
ctx.addedInstructions.add("COPY " & chmodstr & "--from=chalkexedir" &
$(contextCounter) & " " & file & " /" & newname)
contextCounter += 1
if move:
registerTempFile(loc)
Expand Down Expand Up @@ -138,10 +155,24 @@ proc makeFileAvailableToDocker*(ctx: DockerInvocation,
copyFile(loc, dstLoc)
trace("Copied " & loc & " to " & dstLoc)

ctx.addedInstructions.add("COPY " & file & " " & " /" & newname)
if chmod:
if chmod and getDockerVersion().startswith("2") and
getBuildXVersion() > 0:
ctx.addedInstructions.add("COPY --chmod=0755 " & file & " " & " /" &
newname)
elif chmod:
let useDirective = ctx.dfSections[^1].lastUser

# TODO detect user from base image if possible but thats not
# trivial as what is a base image is not a trivial question
# due to multi-stage build possibilities...
if useDirective != nil:
ctx.addedInstructions.add("USER root")
ctx.addedInstructions.add("COPY " & file & " " & " /" & newname)
ctx.addedInstructions.add("RUN chmod 0755 /" & newname)

if useDirective != nil:
ctx.addedInstructions.add("USER " & useDirective.str)
else:
ctx.addedInstructions.add("COPY " & file & " " & " /" & newname)
registerTempFile(dstLoc)

except:
Expand Down
4 changes: 4 additions & 0 deletions src/dockerfile.nim
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,8 @@ proc parseAndEval*(s: Stream,
of "CMD":
firstFromCheck()
res.add(parse.parseCmd(cmd))
of "USER":
res.add(DfUserInfo(str: unicode.strip(cmd.rawArg)))
of "ONBUILD":
firstFromCheck()
res.add(parse.parseOnBuild(cmd))
Expand Down Expand Up @@ -855,6 +857,8 @@ proc evalAndExtractDockerfile*(ctx: DockerInvocation) =
section.cmd = CmdInfo(obj)
elif obj of ShellInfo:
section.shell = ShellInfo(obj)
elif obj of DfUserInfo:
section.lastUser = DfUserInfo(obj)
elif obj of LabelInfo:
for k, v in LabelInfo(obj).labels:
labels[k] = v
Expand Down
23 changes: 23 additions & 0 deletions src/docs/core-release-notes.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,26 @@
# Release Notes for Chalk version 0.1.3

## Fixes

- Docker support when installed via `Snap`.
[9](https://github.com/crashappsec/chalk/pull/9)
- Removes error log when using chalk on ARM Linux as chalk fully
runs on ARM Linux now.
[7](https://github.com/crashappsec/chalk/pull/7)
- When a `Dockerfile` uses `USER` directive, chalk can now wrap
entrypoint in that image
(`docker.wrap_entrypoint = true` in chalk config).
[34](https://github.com/crashappsec/chalk/pull/34)

## Known Issues

### Containers

- When a `Dockerfile` does not use `USER` directive but base image
uses it to change default image user, chalk cannot wrap the
image as it on legacy Docker builder (not buildx) as it will
fail to `chmod` permissions of chalk during the build.

# Release Notes for Chalk version 0.1.2

This is the first open source release of Chalk. For those who
Expand Down
13 changes: 12 additions & 1 deletion tests/chalk/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,10 @@ def docker_build(
tag: Optional[str] = None,
context: Optional[Path] = None,
expected_success: bool = True,
expecting_report: bool = True,
virtual: bool = False,
cwd: Optional[Path] = None,
args: Optional[dict[str, str]] = None,
config: Optional[Path] = None,
buildkit: bool = True,
) -> tuple[str, ChalkProgram]:
Expand All @@ -358,6 +360,7 @@ def docker_build(
tag=tag,
context=context,
dockerfile=dockerfile,
args=args,
cwd=cwd,
expected_success=expected_success,
buildkit=buildkit,
Expand All @@ -375,6 +378,7 @@ def docker_build(
tag=tag,
context=context,
dockerfile=dockerfile,
args=args,
),
expected_success=expected_success,
cwd=cwd,
Expand All @@ -384,11 +388,18 @@ def docker_build(
dockerfile = dockerfile or (
(cwd or context or Path(os.getcwd())) / "Dockerfile"
)
if expected_success:
if expecting_report and expected_success:
# sanity check that chalk mark includes basic chalk keys
assert image_hash == result.mark["_CURRENT_HASH"]
assert image_hash == result.mark["_IMAGE_ID"]
assert str(dockerfile) == result.mark["DOCKERFILE_PATH"]
elif not expecting_report:
try:
assert not result.reports
except json.JSONDecodeError:
# we are not expecting any report json to be present in output
# so this exception is expected here
pass
return image_hash, result

def docker_push(self, image: str):
Expand Down
1 change: 1 addition & 0 deletions tests/data/configs/docker_wrap.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
docker.wrap_entrypoint = true
6 changes: 6 additions & 0 deletions tests/data/dockerfiles/valid/sample_3/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FROM alpine as base

RUN adduser -D testuser
RUN id
USER testuser
RUN id
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FROM alpine as base

RUN adduser -D testuser
RUN id
USER testuser
RUN id
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ARG BASE
FROM $BASE

ENTRYPOINT ["/bin/bash", "-c"]
53 changes: 52 additions & 1 deletion tests/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from .conf import CONFIGS, DOCKERFILES, REGISTRY
from .utils.docker import Docker
from .utils.log import get_logger
from .utils.os import run


logger = get_logger()
Expand Down Expand Up @@ -69,6 +70,10 @@ def run_container(*args, **kwargs):
(DOCKERFILES / "valid" / "sample_1", None, False),
# PWD=foo && docker build -t test .
(DOCKERFILES / "valid" / "sample_1", None, True),
# PWD=foo && docker build .
(DOCKERFILES / "valid" / "sample_3", None, False),
# PWD=foo && docker build -t test .
(DOCKERFILES / "valid" / "sample_3", None, True),
# docker build -f foo/Dockerfile foo
(None, DOCKERFILES / "valid" / "sample_1" / "Dockerfile", False),
# docker build -f foo/Dockerfile -t test foo
Expand All @@ -91,16 +96,59 @@ def test_build(
cwd=cwd,
tag=random_hex if tag else None,
buildkit=buildkit,
config=CONFIGS / "docker_wrap.conf",
)
assert image_id


@pytest.mark.parametrize("buildkit", [True, False])
@pytest.mark.parametrize(
"base, test",
[
(
DOCKERFILES / "valid" / "split_dockerfiles" / "base.Dockerfile",
DOCKERFILES / "valid" / "split_dockerfiles" / "test.Dockerfile",
),
],
)
def test_composite_build(
chalk: Chalk,
base: Path,
test: Path,
buildkit: bool,
random_hex: str,
):
image_id, _ = Docker.build(
dockerfile=base,
buildkit=buildkit,
tag=random_hex,
)
assert image_id

# TODO this is a known limitation for the moment
# we EXPECT this case to fail without buildkit enabled,
# as base image adjusts USER and child Dockerfile
# does not have any indication that USER was adjusted
# and so we cannot detect USER from base image.
# In this case chalk falls back to standard docker build
# which means there is no chalk report in the output
second_image_id, result = chalk.docker_build(
dockerfile=test,
buildkit=buildkit,
args={"BASE": random_hex},
config=CONFIGS / "docker_wrap.conf",
expecting_report=buildkit,
)
assert second_image_id


@mock.patch.dict(os.environ, {"SINK_TEST_OUTPUT_FILE": "/tmp/sink_file.json"})
@pytest.mark.parametrize(
"test_file",
[
"valid/sample_1",
"valid/sample_2",
"valid/sample_3",
],
)
def test_virtual_valid(
Expand Down Expand Up @@ -173,12 +221,15 @@ def test_virtual_invalid(
).is_file(), "virtual-chalk.json should not have been created!"


@pytest.mark.parametrize("test_file", ["valid/sample_1", "valid/sample_2"])
@pytest.mark.parametrize(
"test_file", ["valid/sample_1", "valid/sample_2", "valid/sample_3"]
)
def test_nonvirtual_valid(chalk: Chalk, test_file: str, random_hex: str):
tag = f"{test_file}_{random_hex}"
image_hash, build = chalk.docker_build(
dockerfile=DOCKERFILES / test_file / "Dockerfile",
tag=tag,
config=CONFIGS / "docker_wrap.conf",
)

# artifact is the docker image
Expand Down
8 changes: 7 additions & 1 deletion tests/utils/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ def build_cmd(
tag: Optional[str],
context: Optional[Path] = None,
dockerfile: Optional[Path] = None,
args: Optional[dict[str, str]] = None,
):
cmd = ["docker", "build"]
if tag:
cmd += ["-t", tag]
if dockerfile:
cmd += ["-f", str(dockerfile)]
for name, value in (args or {}).items():
cmd += [f"--build-arg={name}={value}"]
cmd += [str(context or ".")]
return cmd

Expand All @@ -36,6 +39,7 @@ def build(
tag: Optional[str] = None,
context: Optional[Path] = None,
dockerfile: Optional[Path] = None,
args: Optional[dict[str, str]] = None,
cwd: Optional[Path] = None,
expected_success: bool = True,
buildkit: bool = True,
Expand All @@ -45,7 +49,9 @@ def build(
"""
return Docker.with_image_id(
run(
Docker.build_cmd(tag=tag, context=context, dockerfile=dockerfile),
Docker.build_cmd(
tag=tag, context=context, dockerfile=dockerfile, args=args
),
expected_exit_code=int(not expected_success),
env=Docker.build_env(buildkit=buildkit),
cwd=cwd,
Expand Down