Skip to content

Conversation

@pweil-
Copy link

@pweil- pweil- commented Jul 18, 2017

Removes the requirement to drop chroot.

@smarterclayton

@pweil-
Copy link
Author

pweil- commented Jul 18, 2017

@openshift/sig-security

@php-coder
Copy link
Contributor

@bparees Should we allow this cap also to the building containers? Is it possible that we would need it?

@pweil-
Copy link
Author

pweil- commented Jul 18, 2017

It is in the default set of caps from docker but we were requiring it to be dropped prior to this PR.

@php-coder
Copy link
Contributor

@pweil- Was the previous comment, an answer to my question? I was asking about should we also remove it from this list:

@pweil-
Copy link
Author

pweil- commented Jul 18, 2017

I was asking about should we also remove it from this list:

Seems reasonable to remove it there if we're removing it from restricted

@bparees
Copy link
Contributor

bparees commented Jul 18, 2017

what's the basis for us not requiring it to be dropped any longer? s2i is using that list of caps to drop because it runs the container directly, so it needs to manage the caps independently.

@pweil-
Copy link
Author

pweil- commented Jul 18, 2017

sys_chroot is required by some of our containers that do scanning which led to our discussion of whether or not it should really be required as a drop. Ultimately, the chroot call is not considered a security feature of the kernel and folks are still relying on DAC and SELinux in order to protect the system.

Some references for posterity:

The basic idea is that you can run a process inside of a chroot where it will not have access to various system resources; however, chroot is not a security feature.

In particular, it is not intended to be used
for any kind of security purpose, neither to fully sandbox a process
nor to restrict filesystem system calls.

@bparees
Copy link
Contributor

bparees commented Jul 18, 2017

@pweil- ok in that case can you remove it from the spot @php-coder noted as well?

@pweil-
Copy link
Author

pweil- commented Jul 18, 2017

updated

@bparees
Copy link
Contributor

bparees commented Jul 18, 2017

s2i change lgtm

@pweil-
Copy link
Author

pweil- commented Jul 18, 2017

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 2d59f8e

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3230/) (Base Commit: 20e72f7) (PR Branch Commit: 2d59f8e)

@openshift-merge-robot openshift-merge-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 24, 2017
@bparees bparees removed their assignment Jul 25, 2017
@openshift-merge-robot openshift-merge-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 28, 2017
@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2017
@pweil-
Copy link
Author

pweil- commented Aug 23, 2017

/assign @smarterclayton

bump for review

@openshift-merge-robot openshift-merge-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 30, 2017
@php-coder
Copy link
Contributor

PTAL @smarterclayton

@smarterclayton
Copy link
Contributor

/lgtm

as well

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 15, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pweil-, smarterclayton

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@0xmichalis
Copy link
Contributor

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 15834, 16321, 16353, 15298, 15433)

@openshift-merge-robot openshift-merge-robot merged commit 79c3950 into openshift:master Sep 18, 2017
@enoodle
Copy link

enoodle commented Feb 12, 2018

@pweil- @bparees @php-coder
Hello,
Is this available in Openshift 3.7 (oc v3.7.27)? I am getting operation not permitted when trying to chroot inside a non privileged container.

@php-coder
Copy link
Contributor

@enoodle Yes, it is:

$ git tag --contains 2d59f8e4c059df09e339e2bb0355cdbc6a78279e | grep 3.7
v3.7.0
v3.7.0-rc.0
v3.7.1

I'd suggest you to do the following:

  • check by what SCC a pod was admitted (oc get pod <name> -o yaml | grep scc)
  • check whether the output of the oc adm policy reconcile-sccs command is empty
  • check whether a pod is requesting SYS_CHROOT capability in its securityContext

HTH

@enoodle
Copy link

enoodle commented Feb 13, 2018

Thanks for your help @php-coder I indeed had to request this capability directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants