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

SONiC Container Hardening #1364

Merged
merged 9 commits into from
Feb 29, 2024
Merged

SONiC Container Hardening #1364

merged 9 commits into from
Feb 29, 2024

Conversation

Yarden-Z
Copy link
Contributor

@Yarden-Z Yarden-Z commented Jun 19, 2023

Create the container hardening initial proposal for SONiC.
During this PR - we would like to present the container hardening criteria and items for SONiC and future requirements in the future for all SONiC containers

Preview:
Containers is a method of creating virtualization and abstraction of an OS for a subset of processes/service on top of a single host with the purpose of giving it an environment to run and execute its tasks without effect of nearby containers/processes.
In SONiC, we are deploying container with full visibility and capabilities as the host Linux.
This poses a security risk and vulnerability as 1 breached container means that the whole system is breached.
Addressing this issue – we have composed this doc for container hardening, containing the security hardening requirements and definitions for all containers on top of SONiC

Repo PR title State
sonic-buildimage [docker-fpm-frr] limit privileged flag for bgp container GitHub issue/pull request detail
sonic-buildimage [docker-lldp] limit privileged flag for lldp container GitHub issue/pull request detail
sonic-buildimage [docker-teamd] limit privileged flag for teamd container GitHub issue/pull request detail
sonic-buildimage [telemetry] limit privileged flag for telemetry container GitHub issue/pull request detail
sonic-buildimage [docker-mux] limit privileged flag for mux container GitHub issue/pull request detail
sonic-buildimage [docker-eventd] limit privileged flag for eventd container GitHub issue/pull request detail
sonic-buildimage [docker-radv] limit privileged flag for radv container GitHub issue/pull request detail
sonic-buildimage [docker-dhcp-relay] limit privileged flag for dhcp_relay container GitHub issue/pull request detail
sonic-buildimage [docker-sflow] limit privileged flag for sflow container GitHub issue/pull request detail
sonic-buildimage [docker-snmp] limit privileged flag for snmp container GitHub issue/pull request detail
sonic-buildimage limit privileged flag for mgmt-framework container GitHub issue/pull request detail
sonic-buildimage [docker-restapi] limit privileged flag for restapi container GitHub issue/pull request detail
sonic-buildimage [docker-macsec] fix privileged and volumes settings GitHub issue/pull request detail
sonic-buildimage [docker-nat] limit privileged flag for nat container GitHub issue/pull request detail
sonic-buildimage [docker-p4rt] limit privileged flag for p4rt container GitHub issue/pull request detail
sonic-buildimage [docker-iccpd] limit privileged flag for iccpd container GitHub issue/pull request detail
sonic-buildimage Container net host remove GitHub issue/pull request detail
sonic-snmpagent SNMP hardening - redis socket connect GitHub issue/pull request detail
sonic-buildimage [docker-database] limit privileged flag for database container GitHub issue/pull request detail

sonic-mgmt PRs

Repo PR title State
sonic-mgmt testcase for bgp container in container hardening GitHub issue/pull request detail
sonic-mgmt [container hardening] include multiasic scenario, add lldp and teamd containers to testcase GitHub issue/pull request detail
sonic-mgmt [container hardening] include more containers to test case GitHub issue/pull request detail

Revert related PR

Repo PR title State
sonic-buildimage Remove privileged flag for database and snmp docker GitHub issue/pull request detail
sonic-buildimage Revert "Remove privileged flag for database and snmp docker" GitHub issue/pull request detail

syncd, swss cannot be reduced privileged

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 19, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

maipbui added 2 commits July 3, 2023 17:22
Signed-off-by: Mai Bui <[email protected]>
Signed-off-by: Mai Bui <[email protected]>
@maipbui
Copy link
Contributor

maipbui commented Jul 3, 2023

@Yarden-Z I made some changes directly to the HLD, please check again


### 3.2 net=host

Removing the `net=host` is required to prevent the container from accessing the full network scope of the host and system.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe, BGP container needs full network access in order to function properly, do we have the list of containers that can be migrated without net=host? Some of the container (DHCP relay, SNMP..etc) directly work on the Linux networking, removing net=host, might require additional changes at the application level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the SNMP container we have managed to remove the net=host.
For BGP - we also have a PR up with @maipbui changes that we can review.

For DHCP - we'll need to check this.
I have a composed list of containers and the suggestions on whether we can remove the flags or not, I'll add it to the HLD as an appendix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please help review code PRs which we included in this HLD PR's description. @venkatmahalingam Yes, BGP container needs host network mode.

Signed-off-by: Mai Bui <[email protected]>

### 4.2 net=host optimization

Here we will provide a detailed example of how to switch from the `--net=host` configuration (host network) to the `--net=bridge` configuration paired with port forwarding in a specific container. We are using the database container as an example for this item.
Copy link
Contributor

@Blueve Blueve Jul 4, 2023

Choose a reason for hiding this comment

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

Will this design cover SDK change? SONiC containers are massively interactive with Redis database, it is required to let our SDK adapt bridge mode container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why does this require SDK changes?
Regarding the SDK - since it resides in a single container, we have decided against reducing the privileges and capabilities of this container, as this will require massive SDK re-design (and this is not our purpose).
Also - the SDK container is not at a high risk of compromise since it does not interact with the outside network.

Copy link
Contributor

@qiluo-msft qiluo-msft Jul 5, 2023

Choose a reason for hiding this comment

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

@Blueve Which SDK? Is it switch ASIC vendor SDK or swss-common library?
Could you give specific sample function in the SDK you are worrying about?

Copy link
Contributor

Choose a reason for hiding this comment

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

@qiluo-msft swss-common, do we still have a unified way to interact with Redis database with this design?

Current we can use two approaches to interact Redis in sdk:

  • local IP
  • Unix sock file

With this design, both of them are not available I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both are still applicable with this design (tested).
UDS should not be affected, since reducing the privileges of the redis container does not affect the Unix socket (on a different level of permissions - file access permissions).
Local IP might require a slight change, but you can check it in the relevant PR and it is currently working.

StormLiangMS pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 14, 2023
Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)

Work item tracking
Microsoft ADO (number only): 14807420
How I did it
Reduce linux capabilities in privileged flag, retain NET_ADMIN and SYS_ADMIN capabilities

How to verify it
Install new image to DUT, verify bgp container is up
Run bgp sonic-mgmt kvmtest
maipbui added a commit to sonic-net/sonic-mgmt that referenced this pull request Jul 21, 2023
Description of PR
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
Dependency: sonic-net/sonic-buildimage#14932
#### What is the motivation for this PR?
Check bgp container has access to /dev/sda* or /dev/vda* after limiting privileged flag to less Linux capabilities.
#### How did you do it?
#### How did you verify/test it?
```
container_hardening/test_container_hardening.py::test_bgp_dev PASSED                                                                            [100%]
```
Signed-off-by: Mai Bui <[email protected]>
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Aug 15, 2023
#### Why I did it
Reduce docker privilege 
This is part of HLD sonic-net/SONiC#1364

#### How I did it
Remove flag --privileged
#### How to verify it
docker exec -it database bash
root@0048b82b460b:/# ip link add dummy0 type dummy
RTNETLINK answers: Operation not permitted
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Aug 15, 2023
#### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420

#### How I did it
Reduce linux capabilities in privileged flag, retain NET_ADMIN capability
| SYS_PTRACE | Trace arbitrary processes using ptrace(2). |
| SYS_RAWIO | Perform I/O port operations (iopl(2) and ioperm(2)). |
| SYS_RESOURCE | Override resource Limits |
| SYS_TIME | Set system clock (settimeofday(2), stime(2), adjtimex(2)); set real-time (hardware) clock. |
Copy link
Contributor

Choose a reason for hiding this comment

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

how do we know if the appliation does not use SYS_TIME?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't, but if a specific container requires this - it can add it as a specific flag only for this container.

## 10. Open/Action items - if any

Currently, Nvidia and MSFT have scoped commitment for specific containers.
Redis and SNMP already have these adjustments.
Copy link
Collaborator

@venkatmahalingam venkatmahalingam Sep 11, 2023

Choose a reason for hiding this comment

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

What if SNMP container wants to listen on some particular interface on the host? container hardening will be disabled by default for now? can this be driven based on FEATURE table config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the SNMP container would like to listen to a specific interface - you can check this:
https://github.com/sonic-net/sonic-buildimage/pull/15176/files

I think that this PR will showcase your specific use-case.

@zhangyanzhao
Copy link
Collaborator

-p 6379:6379 \
{%- endif %}

#### How to check?
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the user experience with this change with the current container capabilities? What is the plan to mitigate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

User should not experience any changes.
If there are, then one of the following should occur:

  1. Adjust the user action/container action so that it does not require the elevated privileges or specific net host capabilities.
  2. Add to the container the required capabilities it needs to perform.

We should go with option 1 first, and only after we have decided option 1 is not feasible - option 2 is on the table.

N/A, this feature will be checked on a system level.

### 9.2 System Test cases

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any impact w.r.t memory and cpu as it is running in different network name spaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Negligible, as it is part of the infra.
I'll be more precise - the impact is as much of how much the Linux ACL takes, but this should not be visible to any user.


### 3.1 Root privileges

When removing the root privileges from a specific container - we are required to remove the `--privileged` flag and add the required missing Linux capabilities to the docker, or alternatively adjust the container so that it does not require root privileges to perform any action.
Copy link
Contributor

Choose a reason for hiding this comment

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

There is similar work done with Multi ASIC design, it is better to converge before making any decision changing net=host -> bridge etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not familiar with it, what is the work being done there?

N/A

### 6.2. CLI/YANG model Enhancements

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a tool that knows what containers are running privileged mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There might be, requires a bit of research.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you can check by docker inspect container_name | grep Privileged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might be applicable for the --privileged flag, but this HLD is also discussing host network.
So while this might work for one of the HLD item (removing containers from privilege mode) it does not scope the whole feature list.
So this command might be a good start, but we should look at a more encompassing approach to check this (and not stop here).

StormLiangMS pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Nov 16, 2023
Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)

Work item tracking
Microsoft ADO (number only): 14807420
How I did it
Reduce linux capabilities in privileged flag

How to verify it
Run snmp sonic-mgmt tests
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.

admin@vlab-01:~$ docker inspect snmp | grep Privi
            "Privileged": false,

admin@vlab-01:~$ docker exec -it snmp bash
root@vlab-01:/# capsh --print
Current: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
Bounding set =cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_se
xumia pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Nov 21, 2023
Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)

Work item tracking
Microsoft ADO (number only): 14807420
How I did it
Reduce linux capabilities in privileged flag

How to verify it
Run restapi sonic-mgmt tests on sn4600c
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
xumia pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Nov 28, 2023
…k container (#17217)

Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)

Work item tracking
Microsoft ADO (number only): 14807420
How I did it
Reduce linux capabilities in privileged flag

How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
@liat-grozovik
Copy link
Collaborator

@qiluo-msft @lguohan any further comments or we can approve and merge this HLD?
BTW we are left with 2 PRs only, my recommendation is to close the review asap and include it in 202311.

yxieca pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Dec 4, 2023
…k container (#17217)

Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)

Work item tracking
Microsoft ADO (number only): 14807420
How I did it
Reduce linux capabilities in privileged flag

How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
@liat-grozovik
Copy link
Collaborator

@zhangyanzhao could you please help to close the loop and get this HLD PR merged?

qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 19, 2024
### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420
#### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-01:~$ docker inspect iccpd | grep Privi
            "Privileged": false,


admin@vlab-01:~$ docker exec -it iccpd bash
root@vlab-01:/# capsh --print
Current: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
```
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 23, 2024
### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420
#### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-01:~$ docker inspect p4rt | grep Privi
            "Privileged": false,


admin@vlab-01:~$ docker exec -it p4rt bash
root@vlab-01:/# capsh --print
Current: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
```
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
Description of PR
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
Dependency: sonic-net/sonic-buildimage#14932
#### What is the motivation for this PR?
Check bgp container has access to /dev/sda* or /dev/vda* after limiting privileged flag to less Linux capabilities.
#### How did you do it?
#### How did you verify/test it?
```
container_hardening/test_container_hardening.py::test_bgp_dev PASSED                                                                            [100%]
```
Signed-off-by: Mai Bui <[email protected]>
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 26, 2024
### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-01:~$ docker inspect nat | grep Privi
            "Privileged": false,


admin@vlab-01:~$ docker exec -it nat bash
root@vlab-01:/# capsh --print
Current: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
```
@fastiuk
Copy link
Contributor

fastiuk commented Jan 29, 2024

@Blueve @lguohan @venkatmahalingam @maipbui @madhupalu @qiluo-msft please let us know if everything looks good and approve if so

lixiaoyuner pushed a commit to lixiaoyuner/sonic-buildimage that referenced this pull request Feb 6, 2024
…iner

[docker-acms] limit privileged flag for acms container

#### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420

#### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Run acms sonic-mgmt tests
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-01:~$ docker inspect acms | grep Privilege
            "Privileged": false,
admin@vlab-01:~$ docker exec -it acms bash
root@vlab-01:/# capsh --print
Current: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
```

Signed-off-by: Mai Bui <[email protected]>
@liat-grozovik
Copy link
Collaborator

@zhangyanzhao this HLD has been shared long time ago and most of the code PRs were already merged.
I believe there was enough time to provide feedback. as comments were addressed and IMO it is time to merge this PR and ensure the rest of the PRs get in master soon

Added note regarding user-defined bridges. Not in the scope of this HLD at the moment
@liat-grozovik liat-grozovik merged commit 417a096 into sonic-net:master Feb 29, 2024
1 check passed
qiluo-msft pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 22, 2024
#### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420
#### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-08:~$ docker inspect database0 | jq '.[0] | {CapAdd: .HostConfig.CapAdd, Privileged: .HostConfig.Privileged, AppArmor: .AppArmorProfile, Security
Opt: .HostConfig.SecurityOpt, Devices: .HostConfig.Devices, MaskedPath: .HostConfig.MaskedPaths, ReadonlyPaths: .HostConfig.ReadonlyPaths}'
{
  "CapAdd": null,
  "Privileged": false,
  "AppArmor": "unconfined",
  "SecurityOpt": [
    "apparmor=unconfined"
  ],
  "Devices": [],
  "MaskedPath": [],
  "ReadonlyPaths": []
}
```
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
…-net#19248)

#### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420
#### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-08:~$ docker inspect database0 | jq '.[0] | {CapAdd: .HostConfig.CapAdd, Privileged: .HostConfig.Privileged, AppArmor: .AppArmorProfile, Security
Opt: .HostConfig.SecurityOpt, Devices: .HostConfig.Devices, MaskedPath: .HostConfig.MaskedPaths, ReadonlyPaths: .HostConfig.ReadonlyPaths}'
{
  "CapAdd": null,
  "Privileged": false,
  "AppArmor": "unconfined",
  "SecurityOpt": [
    "apparmor=unconfined"
  ],
  "Devices": [],
  "MaskedPath": [],
  "ReadonlyPaths": []
}
```
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Aug 1, 2024
…-net#19248)

#### Why I did it
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
##### Work item tracking
- Microsoft ADO **(number only)**: 14807420
#### How I did it
Reduce linux capabilities in privileged flag

#### How to verify it
Check container's settings: Privileged is false and container only has default Linux caps, does not have extended caps.
```
admin@vlab-08:~$ docker inspect database0 | jq '.[0] | {CapAdd: .HostConfig.CapAdd, Privileged: .HostConfig.Privileged, AppArmor: .AppArmorProfile, Security
Opt: .HostConfig.SecurityOpt, Devices: .HostConfig.Devices, MaskedPath: .HostConfig.MaskedPaths, ReadonlyPaths: .HostConfig.ReadonlyPaths}'
{
  "CapAdd": null,
  "Privileged": false,
  "AppArmor": "unconfined",
  "SecurityOpt": [
    "apparmor=unconfined"
  ],
  "Devices": [],
  "MaskedPath": [],
  "ReadonlyPaths": []
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.