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

Merged
merged 19 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
57 changes: 57 additions & 0 deletions host_modules/docker_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ def is_allowed_container(container):
return True
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):
"""
Expand Down Expand Up @@ -141,3 +157,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 empty command is allowed to be managed 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 container {}: {}".format(image, str(e))
58 changes: 58 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,61 @@ 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()
mock_docker_client.images.list.return_value = [mock.Mock(tags=["image_name:tag"])]

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", "command", {})
assert rc == errno.EPERM, "Return code is wrong"
assert "not allowed" in msg, "Message should contain 'not allowed'"
Loading