-
Notifications
You must be signed in to change notification settings - Fork 742
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
testcase for bgp container in container hardening #8694
Conversation
Signed-off-by: Mai Bui <[email protected]>
The pre-commit check detected issues in the files touched by this pull request. Detailed pre-commit check results: To run the pre-commit checks locally, you can follow below steps:
|
""" | ||
Test bgp container has no access to /dev/vda* | ||
""" | ||
output = duthost.shell("docker exec bgp bash -c 'ls /dev | grep vda'", module_ignore_errors=True)['stdout'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is vda
and why grep it? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/dev/vda in vs image is similar to /dev/sda
admin@vlab-03:~$ docker exec -it bgp bash -c 'ls /dev | grep vda'
admin@vlab-03:~$ docker exec -it telemetry bash -c 'ls /dev | grep vda'
vda
vda1
vda2
vda3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only working in KVM environments. In most DUT, you want to check /dev/sdaN.
You may find a flexible way to check the main disk device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I modified to a more general way to check the device, thanks!
Signed-off-by: Mai Bui <[email protected]>
Signed-off-by: Mai Bui <[email protected]>
""" | ||
Test bgp container has no access to /dev/vda* or /dev/sda* | ||
""" | ||
device = duthost.shell("docker exec bgp bash -c 'df -h | grep /etc/hosts' | awk '{print $1}'")['stdout'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked return code and valid path for device
Signed-off-by: Mai Bui <[email protected]>
|
||
def test_bgp_dev(duthost): | ||
""" | ||
Test bgp container has no access to /dev/vda* or /dev/sda* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this test related to the --privileged flag removal or net=host?
I think we should mention this and also change the name of the test to accommodate it.
Also - in the future, I think we should make this test generic for any container that wants to test their solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yarden-Z it's related to test --privileged flag, I modified the test to make it generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good improvement.
I think that in the future we can make this test even more generic.
i.e - maintain a list of containers that do not have the --privileged flag. (define this list in some global or maintain it in the section of container hardening)
Then - for each container (name) in this list, iterate on top of them and execute this test. That way - we won't need the if case and we'll be able to scale this test for each new container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed your comment, thanks
Signed-off-by: Mai Bui <[email protected]>
Signed-off-by: Mai Bui <[email protected]>
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]>
Description of PR
HLD implementation: Container Hardening (sonic-net/SONiC#1364)
Dependency: sonic-net/sonic-buildimage#14932
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
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?
Any platform specific information?
Supported testbed topology if it's a new test case?
Any topology
Documentation