Skip to content

Commit

Permalink
Implement run function for docker_services
Browse files Browse the repository at this point in the history 
* DBUS Services required for GNOI Containerz StartContainer.

* Add a placeholder function that prevent arbitrary request to run any container.

* Update to only allow running known images.

* Rewrite image validation so it get recognized by semgrep.

* Rewrite command validation so it get recognize by semgrep.

* maybe semgrep will recognize inline function

* semgrep only allow hardcoded image name. We need to bypass it.

* add documentation.

* documentation need to be before nosemgrep

* Add allowed_image_name and use it to verify the run function.

* address copilot comment.

* address comment and reformat.

* increase test coverage and address comment.

* Seperate allowed images and allowed containers.

* Address comment.

* add bmp container to allowed list.

* add validation to kwargs in run.

* fix test error.

* update allow list
  • Loading branch information
@hdwhdw
hdwhdw authored Dec 16, 2024
1 parent b0b3ca5 commit 25bd8ff
Show file tree
Hide file tree
Showing 2 changed files with 255 additions and 65 deletions.
240 changes: 175 additions & 65 deletions host_modules/docker_service.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,108 @@
import docker
import signal
import errno
import logging

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"),
]


def is_allowed_container(container):
ALLOWED_CONTAINERS = {
"bgp",
"bmp",
"database",
"dhcp_relay",
"eventd",
"gnmi",
"lldp",
"pmon",
"radv",
"restapi",
"snmp",
"swss",
"syncd",
"teamd",
"telemetry",
}

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


def is_allowed_image(image):
"""
Check if the container is allowed to be managed by this service.
Check if the image is allowed to be managed by this service.
Args:
container (str): The container name.
image (str): The image name.
Returns:
bool: True if the container is allowed, False otherwise.
bool: True if the image is allowed, False otherwise.
"""
for _, allowed_container in ALLOWED_CONTAINERS:
if container == allowed_container:
return True
return False
image_name = image.split(":")[0] # Remove tag if present
return image_name in ALLOWED_IMAGES


def get_sonic_container(container_id):
"""
Get a Sonic docker container by name. If the container is not a Sonic container, raise PermissionError.
"""
client = docker.from_env()
if container_id not in ALLOWED_CONTAINERS:
raise PermissionError(
"Container {} is not allowed to be managed by this service.".format(
container_id
)
)
container = client.containers.get(container_id)
return container


def validate_docker_run_options(kwargs):
"""
Validate the keyword arguments passed to the Docker container run API.
"""
# Validate the keyword arguments here if needed
# Disallow priviledge mode for security reasons
if kwargs.get("privileged", False):
raise ValueError("Privileged mode is not allowed for security reasons.")
# Disallow sensitive directories to be mounted.
sensitive_dirs = ["/etc", "/var", "/usr"]
for bind in kwargs.get("volumes", {}).keys():
for sensitive_dir in sensitive_dirs:
if bind.startswith(sensitive_dir):
raise ValueError(
"Mounting sensitive directories is not allowed for security reasons."
)
# Disallow running containers as root.
if kwargs.get("user", None) == "root":
raise ValueError(
"Running containers as root is not allowed for security reasons."
)
# Disallow cap_add for security reasons.
if kwargs.get("cap_add", None):
raise ValueError(
"Adding capabilities to containers is not allowed for security reasons."
)
# Disallow access to sensitive devices.
if kwargs.get("devices", None):
raise ValueError("Access to devices is not allowed for security reasons.")


class DockerService(host_service.HostModule):
Expand All @@ -52,92 +116,138 @@ class DockerService(host_service.HostModule):
@host_service.method(
host_service.bus_name(MOD_NAME), in_signature="s", out_signature="is"
)
def stop(self, container):
def stop(self, container_id):
"""
Stop a running Docker container.
Args:
container (str): The name or ID of the Docker container.
container_id (str): The name of the Docker container.
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_container(container):
return (
errno.EPERM,
"Container {} is not allowed to be managed by this service.".format(
container
),
)
container = client.containers.get(container)
container = get_sonic_container(container_id)
container.stop()
return 0, "Container {} has been stopped.".format(container.name)
except PermissionError:
msg = "Container {} is not allowed to be managed by this service.".format(
container_id
)
logging.error(msg)
return errno.EPERM, msg
except docker.errors.NotFound:
return errno.ENOENT, "Container {} does not exist.".format(container)
msg = "Container {} does not exist.".format(container_id)
logging.error(msg)
return errno.ENOENT, msg
except Exception as e:
return 1, "Failed to stop container {}: {}".format(container, str(e))
msg = "Failed to stop container {}: {}".format(container_id, str(e))
logging.error(msg)
return 1, msg

@host_service.method(
host_service.bus_name(MOD_NAME), in_signature="si", out_signature="is"
)
def kill(self, container, signal=signal.SIGKILL):
def kill(self, container_id, signal=signal.SIGKILL):
"""
Kill or send a signal to a running Docker container.
Args:
container (str): The name or ID of the Docker container.
container_id (str): The name or ID of the Docker container.
signal (int): The signal to send. Defaults to SIGKILL.
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_container(container):
return (
errno.EPERM,
"Container {} is not allowed to be managed by this service.".format(
container
),
)
container = client.containers.get(container)
container = get_sonic_container(container_id)
container.kill(signal=signal)
return 0, "Container {} has been killed with signal {}.".format(
container.name, signal
)
except PermissionError:
msg = "Container {} is not allowed to be managed by this service.".format(
container_id
)
logging.error(msg)
return errno.EPERM, msg
except docker.errors.NotFound:
return errno.ENOENT, "Container {} does not exist.".format(container)
msg = "Container {} does not exist.".format(container_id)
logging.error(msg)
return errno.ENOENT, msg
except Exception as e:
return 1, "Failed to kill container {}: {}".format(container, str(e))
return 1, "Failed to kill container {}: {}".format(container_id, str(e))

@host_service.method(
host_service.bus_name(MOD_NAME), in_signature="s", out_signature="is"
)
def restart(self, container):
def restart(self, container_id):
"""
Restart a running Docker container.
Args:
container (str): The name or ID of the Docker container.
container_id (str): The name or ID of the Docker container.
Returns:
tuple: A tuple containing the exit code (int) and a message indicating the result of the operation.
"""
try:
container = get_sonic_container(container_id)
container.restart()
return 0, "Container {} has been restarted.".format(container.name)
except PermissionError:
return (
errno.EPERM,
"Container {} is not allowed to be managed by this service.".format(
container_id
),
)
except docker.errors.NotFound:
return errno.ENOENT, "Container {} does not exist.".format(container_id)
except Exception as e:
return 1, "Failed to restart container {}: {}".format(container_id, str(e))

@host_service.method(
host_service.bus_name(MOD_NAME), in_signature="ssa{sv}", out_signature="is"
)
def run(self, image, command, kwargs):
"""
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_container(container):

if not is_allowed_image(image):
return (
errno.EPERM,
"Container {} is not allowed to be managed by this service.".format(
container
"Image {} is not allowed to be managed by this service.".format(
image
),
)
container = client.containers.get(container)
container.restart()
return 0, "Container {} has been restarted.".format(container.name)
except docker.errors.NotFound:
return errno.ENOENT, "Container {} does not exist.".format(container)

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

validate_docker_run_options(kwargs)

# 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 created.".format(container.name)
except ValueError as e:
return errno.EINVAL, "Invalid argument.".format(str(e))
except docker.errors.ImageNotFound:
return errno.ENOENT, "Image {} not found.".format(image)
except Exception as e:
return 1, "Failed to restart container {}: {}".format(container, str(e))
return 1, "Failed to run image {}: {}".format(image, str(e))
80 changes: 80 additions & 0 deletions tests/host_modules/docker_service_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,83 @@ 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"
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"

0 comments on commit 25bd8ff

Please sign in to comment.