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

DBUS API required for GNOI Containerz.StartContainer #182

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions host_modules/docker_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,23 @@ def is_allowed_container(container):
return False


def is_allowed_image(image):
"""
Check if the image is allowed to be managed by this service.

Args:
image (str): The image name.

Returns:
bool: True if the image is allowed, False otherwise.
"""
image_name = image.split(":")[0] # Remove tag if present
for allowed_image, _ in ALLOWED_CONTAINERS:
hdwhdw marked this conversation as resolved.
Show resolved Hide resolved
if image_name == allowed_image:
return True
return False


class DockerService(host_service.HostModule):
"""
DBus endpoint that executes the docker command
Expand Down Expand Up @@ -141,3 +158,44 @@ def restart(self, container):
return errno.ENOENT, "Container {} does not exist.".format(container)
except Exception as e:
return 1, "Failed to restart container {}: {}".format(container, str(e))

@host_service.method(
host_service.bus_name(MOD_NAME), in_signature="ssa{sv}", out_signature="is"
)
def run(self, image, command, kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kwargs

This method allows passing kwargs directly to docker.containers.run. This could lead to potential security issues if not properly validated. Consider sanitizing or restricting the kwargs that can be passed to ensure they don't introduce vulnerabilities (e.g., privileged containers or host mounts).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some initial validation. Please let me know if there is anything else you can think of.

"""
Run a Docker container.

Args:
image (str): The name of the Docker image to run.
command (str): The command to run in the container
kwargs (dict): Additional keyword arguments to pass to the Docker API.

Returns:
tuple: A tuple containing the exit code (int) and a message indicating the result of the operation.
"""
try:
client = docker.from_env()

if not is_allowed_image(image):
return (
errno.EPERM,
"Image {} is not allowed to be managed by this service.".format(
image
),
)

if command:
return (
errno.EPERM,
"Only an empty string command is allowed. Non-empty commands are not permitted by this service.",
)

# Semgrep cannot detect codes for validating image and command.
# nosemgrep: python.docker.security.audit.docker-arbitrary-container-run.docker-arbitrary-container-run
container = client.containers.run(image, command, **kwargs)
return 0, "Container {} has been started.".format(container.name)
except docker.errors.ImageNotFound:
return errno.ENOENT, "Image {} not found.".format(image)
except Exception as e:
return 1, "Failed to run image {}: {}".format(image, str(e))
81 changes: 81 additions & 0 deletions tests/host_modules/docker_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,84 @@ def test_docker_restart_fail_api_error(self, MockInit, MockBusName, MockSystemBu
assert "API error" in msg, "Message should contain 'API error'"
mock_docker_client.containers.get.assert_called_once_with("syncd")
mock_docker_client.containers.get.return_value.restart.assert_called_once()

@mock.patch("dbus.SystemBus")
@mock.patch("dbus.service.BusName")
@mock.patch("dbus.service.Object.__init__")
def test_docker_run_success(self, MockInit, MockBusName, MockSystemBus):
mock_docker_client = mock.Mock()
mock_docker_client.containers.run.return_value.name = "syncd"

with mock.patch.object(docker, "from_env", return_value=mock_docker_client):
docker_service = DockerService(MOD_NAME)
rc, msg = docker_service.run("docker-syncd-brcm:latest", "", {})

assert rc == 0, "Return code is wrong"
assert "started" in msg, "Message should contain 'started'"
mock_docker_client.containers.run.assert_called_once_with(
"docker-syncd-brcm:latest", "", **{}
)

@mock.patch("dbus.SystemBus")
@mock.patch("dbus.service.BusName")
@mock.patch("dbus.service.Object.__init__")
def test_docker_run_fail_image_not_found(
self, MockInit, MockBusName, MockSystemBus
):
mock_docker_client = mock.Mock()
mock_docker_client.containers.run.side_effect = docker.errors.ImageNotFound(
"Image not found"
)

with mock.patch.object(docker, "from_env", return_value=mock_docker_client):
docker_service = DockerService(MOD_NAME)
rc, msg = docker_service.run("docker-syncd-brcm:latest", "", {})

assert rc == errno.ENOENT, "Return code is wrong"
assert "not found" in msg, "Message should contain 'not found'"
mock_docker_client.containers.run.assert_called_once_with(
"docker-syncd-brcm:latest", "", **{}
)

@mock.patch("dbus.SystemBus")
@mock.patch("dbus.service.BusName")
@mock.patch("dbus.service.Object.__init__")
def test_docker_run_fail_api_error(self, MockInit, MockBusName, MockSystemBus):
mock_docker_client = mock.Mock()
mock_docker_client.containers.run.side_effect = docker.errors.APIError(
"API error"
)

with mock.patch.object(docker, "from_env", return_value=mock_docker_client):
docker_service = DockerService(MOD_NAME)
rc, msg = docker_service.run("docker-syncd-brcm:latest", "", {})

assert rc != 0, "Return code is wrong"
assert "API error" in msg, "Message should contain 'API error'"
mock_docker_client.containers.run.assert_called_once_with(
"docker-syncd-brcm:latest", "", **{}
)

@mock.patch("dbus.SystemBus")
@mock.patch("dbus.service.BusName")
@mock.patch("dbus.service.Object.__init__")
def test_docker_run_fail_image_not_allowed(
self, MockInit, MockBusName, MockSystemBus
):
mock_docker_client = mock.Mock()
with mock.patch.object(docker, "from_env", return_value=mock_docker_client):
docker_service = DockerService(MOD_NAME)
rc, msg = docker_service.run("wrong_image_name", "", {})
assert rc == errno.EPERM, "Return code is wrong"

@mock.patch("dbus.SystemBus")
@mock.patch("dbus.service.BusName")
@mock.patch("dbus.service.Object.__init__")
def test_docker_run_fail_non_empty_command(
self, MockInit, MockBusName, MockSystemBus
):
mock_docker_client = mock.Mock()
with mock.patch.object(docker, "from_env", return_value=mock_docker_client):
docker_service = DockerService(MOD_NAME)
rc, msg = docker_service.run("docker-syncd-brcm:latest", "rm -rf /", {})
assert rc == errno.EPERM, "Return code is wrong"
Loading