Skip to content

Add support for confined users#18439

Closed
rhatdan wants to merge 2 commits intocontainers:mainfrom
rhatdan:selinux
Closed

Add support for confined users#18439
rhatdan wants to merge 2 commits intocontainers:mainfrom
rhatdan:selinux

Conversation

@rhatdan
Copy link
Copy Markdown
Member

@rhatdan rhatdan commented May 3, 2023 •
edited by cevich
Loading

The original SELinux support in Docker and Podman does not follow the default SELinux rules for how label transitions are supposed to be handled. Containers always switch their user and role to system_u:system_r, rather then maintain the collers user and role. For example
unconfined_u:unconfined_r:container_t:s0:c1,c2

Advanced SELinux administrators want to confine users but still allow them to create containers from their role, but not allow them to launch a privileged container like spc_t.

This means if a user running as
container_user_u:container_user_r:container_user_t:s0

Ran a container they would get

container_user_u:container_user_r:container_t:s0:c1,c2

If they run a privileged container they would run it with:

container_user_u:container_user_r:container_user_t:s0

If they want to force the label they would get an error

podman run --security-opt label=type:spc_t ...

Should fail. Because the container_user_r can not run with the spc_t.

SELinux rules would also prevent the user from forcing system_u user and the sytem_r role.

Does this PR introduce a user-facing change?

Confined SELinux users can not use podman

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels May 3, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented May 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels May 3, 2023
@rhatdan rhatdan mentioned this pull request May 3, 2023
Merged
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 4, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 4, 2023
@rhatdan rhatdan changed the title [WIP] Add support for confined users Add support for confined users May 4, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 4, 2023
@rhatdan rhatdan force-pushed the selinux branch 3 times, most recently from 8176030 to 74cad1e Compare May 8, 2023 01:12
@rhatdan rhatdan mentioned this pull request May 11, 2023
Merged
@lsm5
Copy link
Copy Markdown
Member

lsm5 commented May 11, 2023 •
edited
Loading

@cevich AFAICT, container-selinux still gets fetched from the default fedora repos?

We should probably switch to fetching all dependencies from rhcontainerbot/podman-next or make it easy to switch from upstream main (podman-next) to default fedora and back. There's a recent container-selinux change which hasn't been released yet but is merged on main and we should be using it for this PR. @rhatdan can correct me.

@lsm5
Copy link
Copy Markdown
Member

lsm5 commented May 11, 2023

@cevich i guess for now, I could rerun CI job with terminal and manually install the copr repo with latest container-selinux. If all goes well, that's good enough for now. That way your [CI:Next] won't be urgent.

@cevich
Copy link
Copy Markdown
Member

cevich commented May 11, 2023

As discussed on chat: Let's add a [CI:NEXT] magic string condition to setup_environment.sh. When found in the PR name, it will enable the podman-next COPR and run a dnf upgrade -y. That way changes can be validated not to break the current CI VM image set as well.

@cevich
Copy link
Copy Markdown
Member

cevich commented May 11, 2023

Opened #18545

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

TomSweeneyRedHat commented May 11, 2023

LGTM
once the tests get hip.

@cevich cevich mentioned this pull request May 16, 2023
Merged
@cevich
Copy link
Copy Markdown
Member

cevich commented May 17, 2023

Just tagged c20230517t144652z-f38f37d12 which has container-selinux-2:2.213.0-1. Feel free to bring it in along with this PR. Renovate will also open a (separate) image-update PR.

@cevich cevich mentioned this pull request May 17, 2023
Merged
1 task
@rhatdan rhatdan mentioned this pull request May 18, 2023
Merged
@lsm5
Copy link
Copy Markdown
Member

lsm5 commented May 18, 2023

@cevich does [CI:Next] need to be in the commit message, or does simply editing the title on github make it work too ?

@cevich
Copy link
Copy Markdown
Member

cevich commented May 18, 2023

If this PR is using c20230426t140447z-f38f37d12, it already has container-selinux-2:2.213.0-1. in the F38 VMs.

If you want it to update at runtime from podman-next COPR, edit the PR description and add [CI:NEXT] (case-sensitive) anywhere in it. Once that's done, turn the PR into a draft[*], then re-push some change to it.

[*]: Under the "reviewers" section at the top is a link "Still in progress? Convert to draft". Click that.

@rhatdan
Copy link
Copy Markdown
Member Author

rhatdan commented May 18, 2023 •
edited
Loading

The logs says
container-selinux-2.209.0-1.fc37-noarch
And
container-selinux-2.211.0-1.fc38-noarch

@lsm5
Copy link
Copy Markdown
Member

lsm5 commented May 29, 2023

Podman v4.5.1 fedora gating tests are failing and will continue to fail until this patch is included in a release AFAICT. @rhatdan are we ok to waive gating tests on fedora. See: https://artifacts.dev.testing-farm.io/fcd0eecd-53a9-44a1-aa9b-a492e3a00411/work-tests.ymlcm3acqme/tests-x65lqvzh/test.podman-rootless-cgroupsv2.bats.log for example.

@lsm5 lsm5 mentioned this pull request May 30, 2023
Merged
@rhatdan
Copy link
Copy Markdown
Member Author

rhatdan commented May 30, 2023

Yes I would waive the SELinux issues for now.

@rhatdan
Copy link
Copy Markdown
Member Author

rhatdan commented Jun 5, 2023

@cevich Can we create a new VM image to contain the latest container-selinux package?

@lsm5
Copy link
Copy Markdown
Member

lsm5 commented Jun 5, 2023

@cevich Can we create a new VM image to contain the latest container-selinux package?

I'll change this to draft and use [CI:NEXT]. Let's see how that goes.

@lsm5 lsm5 marked this pull request as draft June 5, 2023 18:36
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 5, 2023
@lsm5
Copy link
Copy Markdown
Member

lsm5 commented Jun 5, 2023

@rhatdan please repush to this PR. Hopefully it will pick the latest. The latest container-selinux build is ready on podman-next so repushing should just work.

@cevich
Copy link
Copy Markdown
Member

cevich commented Jun 5, 2023

Can we create a new VM image to contain the latest container-selinux package?

Newer images: #18612 (I'm guessing that fails CI b/c we need this PR?)

@cevich cevich changed the title Add support for confined users [CI:NEXT] Add support for confined users Jun 5, 2023
@cevich
Copy link
Copy Markdown
Member

cevich commented Jun 5, 2023

I'll change this to draft and use [CI:NEXT]. Let's see how that goes.

FYI - I'm pretty sure the magic string has to be in the PR title, not the description. I may be wrong though - there are several CI env. vars. on this topic and I can't remember which is which off-hand.

@cevich
Copy link
Copy Markdown
Member

cevich commented Jun 6, 2023

(I hit re-run on Build for fedora-38 CI_DESIRED_DATABASE:boltdb, hopefully was just a CI flake)

@rhatdan rhatdan changed the title [CI:NEXT] Add support for confined users Add support for confined users Jun 12, 2023
@cevich
Copy link
Copy Markdown
Member

cevich commented Jun 13, 2023

FYI: Opened containers/automation_images#282

@rhatdan rhatdan force-pushed the selinux branch 2 times, most recently from 5005751 to 3bec150 Compare June 21, 2023 18:19
@cevich
Copy link
Copy Markdown
Member

cevich commented Jun 21, 2023

ERROR: Automation spec. 'debian-12'; actual host 'debian-1313' (contrib/cirrus/setup_environment.sh:67 in main())

Uh-oh, that sounds like a "me" problem...Yeah, you're missing this yaml line:

DEBIAN_NAME: "debian-13"

@rhatdan rhatdan force-pushed the selinux branch 2 times, most recently from bef4561 to cde03c5 Compare June 27, 2023 20:07
rhatdan added 2 commits July 24, 2023 14:12
@rhatdan
Update cirrus images
c8ccd3a 
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan
Add support for confined users
a81bf57 
The original SELinux support in Docker and Podman does not follow the
default SELinux rules for how label transitions are supposed to be
handled. Containers always switch their user and role to
system_u:system_r, rather then maintain the collers user and role.
For example
unconfined_u:unconfined_r:container_t:s0:c1,c2

Advanced SELinux administrators want to confine users but still allow
them to create containers from their role, but not allow them to launch
a privileged container like spc_t.

This means if a user running as
container_user_u:container_user_r:container_user_t:s0

Ran a container they would get

container_user_u:container_user_r:container_t:s0:c1,c2

If they run a privileged container they would run it with:

container_user_u:container_user_r:container_user_t:s0

If they want to force the label they would get an error

podman run --security-opt label=type:spc_t ...

Should fail. Because the container_user_r can not run with the spc_t.

SELinux rules would also prevent the user from forcing system_u user and
the sytem_r role.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan rhatdan marked this pull request as ready for review July 24, 2023 18:12
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2023
@rhatdan rhatdan mentioned this pull request Jul 26, 2023
Closed
@rhatdan rhatdan closed this Aug 23, 2023
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Nov 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rhatdan @lsm5 @cevich @TomSweeneyRedHat @openshift-merge-robot