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 14 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
115 changes: 93 additions & 22 deletions host_modules/docker_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,43 @@
MOD_NAME = "docker_service"

# The set of allowed containers that can be managed by this service.
# First element is the image name, second element is the container name.
ALLOWED_CONTAINERS = [
("docker-syncd-brcm", "syncd"),
("docker-acms", "acms"),
("docker-sonic-gnmi", "gnmi"),
("docker-sonic-telemetry", "telemetry"),
("docker-snmp", "snmp"),
("docker-platform-monitor", "pmon"),
("docker-lldp", "lldp"),
("docker-dhcp-relay", "dhcp_relay"),
("docker-router-advertiser", "radv"),
("docker-teamd", "teamd"),
("docker-fpm-frr", "bgp"),
("docker-orchagent", "swss"),
("docker-sonic-restapi", "restapi"),
("docker-eventd", "eventd"),
("docker-database", "database"),
]
ALLOWED_CONTAINERS = {
"syncd",
"acms",
"gnmi",
"telemetry",
"snmp",
"pmon",
"lldp",
"dhcp_relay",
"radv",
"teamd",
"bgp",
"swss",
"restapi",
"eventd",
"database",
}

# The set of allowed images that can be managed by this service.
ALLOWED_IMAGES = {
"docker-syncd-brcm",
"docker-syncd-cisco"
hdwhdw marked this conversation as resolved.
Show resolved Hide resolved
"docker-acms",
"docker-sonic-gnmi",
"docker-sonic-telemetry",
"docker-snmp",
"docker-platform-monitor",
"docker-lldp",
"docker-dhcp-relay",
"docker-router-advertiser",
"docker-teamd",
"docker-fpm-frr",
"docker-orchagent",
"docker-sonic-restapi",
"docker-eventd",
"docker-database",
}


def is_allowed_container(container):
Expand All @@ -38,10 +57,21 @@ def is_allowed_container(container):
Returns:
bool: True if the container is allowed, False otherwise.
"""
for _, allowed_container in ALLOWED_CONTAINERS:
if container == allowed_container:
return True
return False
return container in ALLOWED_CONTAINERS


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
return image_name in ALLOWED_IMAGES


class DockerService(host_service.HostModule):
Expand Down Expand Up @@ -141,3 +171,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