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

feat(adr): Docker container guidelines #220

Merged
merged 1 commit into from
Oct 26, 2020
Merged
Changes from all 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
146 changes: 146 additions & 0 deletions docs_src/design/adr/system-management/001-docker-image-guidelines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# Docker image guidelines

## Status

Pending

## Context

When deploying the EdgeX Docker containers some security measures are recommended to ensure the integrity of the software stack.

## Decision

When deploying Docker images, the following flags should be set for heightened security.

- To avoid escalation of privileges each docker container should use the `no-new-privileges` option in their Docker compose file (example below). More details about this flag can be found [here](https://docs.docker.com/engine/reference/run/#security-configuration). This follows Rule #4 for Docker security found [here](https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html#rule-4-add-no-new-privileges-flag).
Copy link
Member

Choose a reason for hiding this comment

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

We've also had instances of device services using the --privileged flag in order to access DBus and/or actual hardware devices. It'd be nice if this was called out explicitly as not allowed. In addition, we should provide some guidelines as to how to properly allow hardware access (e.g. writing custom AppArmor rules) from within a container.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tonyespy Jim White I believe commented on this earlier. I will repeat what I said before.

no-new-privileges doesn't prevent one from running privileged containers. What it prevents is tools like "sudo" from running in a container thus a going from user mode to root mode. It also prevents capabilities defined on extended file attributes from being effected. (For example, the capability to bind to a low-numbered port.).

Docker-compose v3 doesn't allow one to customize cgroups like the v2 did. For example, you can no longer pass in device_cgroup_rules. So if you are accessing a device that isn't in docker's built-in allow list, privileged mode is one way to allow device access to continue to function. (Curious myself if this is the only way, or maybe we should downrev the docker-compose to v2... hmmm.)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @bnevis-i. Two additional points...

  • The device-bluetooth-c (still in holding) device service recommends running the service using docker run --privileged, and on systems where AppArmor is supported, includes an example of how to pass an AppArmor profile to docker run. Do you know if compose v2 also allowed AppArmor profiles to be specified?
  • Frankly, if we're going to suggest that someone run a service with --privileged, I'm not sure why we're even bothering to run the service in a container in the first place.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@brian-intel

Are you up to adding a recommendation to prefer

security_opt: [ "apparmor:unconfined" ]

or a custom apparmor profile vs running --privileged

This doesn't negate the guidance regarding no-new-privileges, but it does add additional (and worthy) guidance to not use --privileged at all.


```docker
security_opt:
- "no-new-privileges:true"
```

> NOTE: Alternatively an AppArmor security profile can be used to isolate the docker container. More details about apparmor profiles can be found [here](https://docs.docker.com/engine/security/apparmor/)
```docker
security_opt: [ "apparmor:unconfined" ]
```

- To further prevent privilege escalation attacks the user should be set for the docker container using the `--user=<userid>` or `-u=<userid>` option in their Docker compose file (example below). More details about this flag can be found [here](https://docs.docker.com/engine/reference/run/#user). This follows Rule #2 for Docker security found [here](https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html#rule-2-set-a-user).

```docker
services:
device-virtual:
image: ${REPOSITORY}/docker-device-virtual-go${ARCH}:${DEVICE_VIRTUAL_VERSION}
user: $CONTAINER-PORT:$CONTAINER-PORT # user option using an unprivileged user
ports:
- "127.0.0.1:49990:49990"
container_name: edgex-device-virtual
hostname: edgex-device-virtual
networks:
- edgex-network
env_file:
- common.env
environment:
SERVICE_HOST: edgex-device-virtual
depends_on:
- consul
- data
- metadata
```

> NOTE: exception
Sometimes containers will require root access to perform their fuctions. For example the System Management Agent requires root access to control other Docker containers. In this case you would allow it run as default root user.

- To avoid a faulty or compromised containers from consuming excess amounts of the host of its resources `resource limits` should be set for each container. More details about `resource limits` can be found [here](https://docs.docker.com/config/containers/resource_constraints/). This follows Rule #7 for Docker security found [here](https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html#rule-7-limit-resources-memory-cpu-file-descriptors-processes-restarts).
Copy link
Member

Choose a reason for hiding this comment

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

Should this ADR give some recommendations as to what these resource limitations should be? I also imagine the limits might be different for some of our runtime dependencies (e.g. kong, redis, ...).

Copy link
Member

Choose a reason for hiding this comment

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

Don't think we can as they will be system and use case dependent.

Copy link
Member

Choose a reason for hiding this comment

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

If we're not going to provide any guidance (e.g. minimum memory requirement), then we should add some verbiage that this is an exercise left to the end-user base on system resources and use cases.

Also I'll note that the docker resource constraints link above is for the docker command itself, not docker-compose. Does compose support YAML specified resource constraints?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does compose support YAML specified resource constraints?

https://docs.docker.com/compose/compose-file/#resources


```docker
services:
device-virtual:
image: ${REPOSITORY}/docker-device-virtual-go${ARCH}:${DEVICE_VIRTUAL_VERSION}
user: 4000:4000 # user option using an unprivileged user
ports:
- "127.0.0.1:49990:49990"
container_name: edgex-device-virtual
hostname: edgex-device-virtual
networks:
- edgex-network
env_file:
- common.env
environment:
SERVICE_HOST: edgex-device-virtual
depends_on:
- consul
- data
- metadata
deploy: # Deployment resource limits
resources:
limits:
cpus: '0.001'
memory: 50M
reservations:
cpus: '0.0001'
memory: 20M
```

- To avoid attackers from writing data to the containers and modifying their files the `--read_only` flag should be set. More details about this flag can be found [here](https://docs.docker.com/compose/compose-file/#domainname-hostname-ipc-mac_address-privileged-read_only-shm_size-stdin_open-tty-user-working_dir). This follows Rule #8 for Docker security found [here](https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html#rule-8-set-filesystem-and-volumes-to-read-only).

```docker
device-rest:
image: ${REPOSITORY}/docker-device-rest-go${ARCH}:${DEVICE_REST_VERSION}
ports:
- "127.0.0.1:49986:49986"
container_name: edgex-device-rest
hostname: edgex-device-rest
read_only: true # read_only option
networks:
- edgex-network
env_file:
- common.env
environment:
SERVICE_HOST: edgex-device-rest
depends_on:
- data
- command
```

> NOTE: exception
If a container is required to have write permission to function, then this flag will not work. For example, the vault needs to run setcap in order to lock pages in memory. In this case the `--read_only` flag will not be used.

NOTE: Volumes
If writing persistent data is required then a volume can be used. A volume can be attached to the container in the following way

```docker
device-rest:
image: ${REPOSITORY}/docker-device-rest-go${ARCH}:${DEVICE_REST_VERSION}
ports:
- "127.0.0.1:49986:49986"
container_name: edgex-device-rest
hostname: edgex-device-rest
read_only: true # read_only option
networks:
- edgex-network
env_file:
- common.env
environment:
SERVICE_HOST: edgex-device-rest
depends_on:
- data
- command
volumes:
- consul-config:/consul/config:z
```

> NOTE: alternatives
If writing non-persistent data is required (ex. a config file) then a temporary filesystem mount can be used to accomplish this goal while still enforcing `--read_only`. Mounting a `tmpfs` in Docker gives the container a temporary location in the host systems memory to modify files. This location will be removed once the container is stopped. More details about `tmpfs` can be found [here](https://docs.docker.com/storage/tmpfs/)
Copy link
Member

Choose a reason for hiding this comment

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

What about our persistence later (e.g. redis)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with Tony. Should also mention plain 'old volumes for normal persistence needs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interestingly, Kubernetes might be an issue here. It just doesn't support volumes in the normal way that Docker does.


lenny-goodell marked this conversation as resolved.
Show resolved Hide resolved
for additional docker security rules and guidelines please check the Docker security [cheatsheet](https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html)


## Consequences

Create a more secure Docker environment

## References

- Docker-compose reference <https://docs.docker.com/compose/compose-file>
- OWASP Docker Recommendations <https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html>
- CIS Docker Benchmark <https://workbench.cisecurity.org/files/2433/download/2786> (registration required)