-
Notifications
You must be signed in to change notification settings - Fork 75
In a container, sshd should not run as root #152
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
Conversation
Add the option to choose the owner of configurations files and their locations. Signed-off-by: Michée Lengronne <[email protected]>
64f4b91 to
5dc04cb
Compare
Signed-off-by: Michée Lengronne <[email protected]>
c3ab7b3 to
9305835
Compare
Signed-off-by: Michée Lengronne <[email protected]>
d240832 to
1081581
Compare
|
I am not sure how to handle the groups though. https://github.com/dev-sec/ssh-baseline/blob/micheelengronne-patch-2/controls/ssh_spec.rb#L37 |
Signed-off-by: Michée Lengronne <[email protected]>
b1c61d6 to
365f2f9
Compare
It should be 'no' Signed-off-by: Michée Lengronne <[email protected]>
7f026a3 to
5fafd0a
Compare
Signed-off-by: Michée Lengronne <[email protected]>
cc9e43d to
d47a8eb
Compare
Will test this rubocop one. Maybe it is the other. Signed-off-by: Michée Lengronne <[email protected]>
57fb69b to
7fda767
Compare
|
@artem-sidorenko is it ok for you ? Is there smthg I did wrong ? |
|
@chris-rock is it ok to merge ? |
chris-rock
left a comment
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 think it is a great addition @micheelengronne Can you elaborate the use case a bit more? I'd like to understand why a different path is required.
controls/ssh_spec.rb
Outdated
| it { should be_file } | ||
| it { should be_owned_by 'root' } | ||
| it { should be_grouped_into os.darwin? ? 'wheel' : 'root' } | ||
| it { should be_owned_by custom_user } |
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 think that needs to be ssh_custom_user
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 normally fixed that in the last commit.
Signed-off-by: Michée Lengronne <[email protected]>
ffb0a13 to
37a07a4
Compare
|
Yes. In the case of a sshd server in a container, the root user should not be used as a container process should not run as root. In that case, it is not very good to use a reserved root path like There is no clear definition of where a configurations ssh folder should be present when a non
As there is no clear definition, I think each user should have the choice. |
chris-rock
left a comment
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.
Thank you for the clarification. Great improvement @micheelengronne
|
Thank you. Can you trigger a release please ? |
|
Just out of curiosity: does this mean that the container ensures the security of the host here? If yes, wouldn't this miss the point of privilege separation and, well, containment? |
|
The problem is that unprivileged in the container world (particularly docker, less for podman) is not so unprivileged.
|
|
It is generally a bad pattern to write a |
|
Thanks for the explanation. Sorry in case I am missing obvious things here, as I primarily use LXC/LXD. If the container is within a user namespace ( But we don't need to get too philosophical here. I just wanted, for the record, to raise the concern about the container's containment qualities if a root process therein threatens the host security (as in: running nothing as root inside the container is the wrong thing to fix). |
|
for some references:
The problem is that the main used container provider is
|
Add the option to choose the owner of configurations files and their locations.