Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Default permissions of jail root directory #347

Closed
Davis-A opened this issue Aug 25, 2017 · 9 comments
Closed

Default permissions of jail root directory #347

Davis-A opened this issue Aug 25, 2017 · 9 comments
Labels

Comments

@Davis-A
Copy link

Davis-A commented Aug 25, 2017

Default permissions for the jail root directory are 755 (rwxr-wr-w).

Potential attack. A non root host user could conspire with a jailed root user to escalate privilege.

To replicate. Have the jailed root user turn on setuid bit on the jails /bin/sh
The host non-root user can execute the jails /iocage/jail/jail-uuid/root/bin/sh can then execute to gain root access to the host.

Proposed fix. Have default jail root directory be 700 (rwx------) preventing a non-root host user from entering the jail file hierarchy.

@skarekrow
Copy link
Member

Thanks for bringing this to my attention!

@rwestlund
Copy link
Contributor

@skarekrow hey now. This breaks my use case from #204 where I have a service on the host communicating with jailed services via unix sockets.

Defaulting to 0700 is a good idea, but calling chmod on every start is overkill.

Why not just set it on the top-level /iocage or /iocage/jails directory during init and be done with it? Don't worry about applying this to existing systems; iocage isn't stable yet and leaving it alone won't break anything.

@skarekrow
Copy link
Member

@rwestlund I actually just hit breakage with plugins with this as well, I'm going to revert and circle back to this when I can test it more.

@skarekrow skarekrow reopened this Aug 29, 2017
@skarekrow
Copy link
Member

Defaulting to 700 means every user inside the jail cannot function, as they can no longer cd to anything. So that makes this fix problematic as it would make the jail completely unusable.

I'm not sure the correct course of action for this, so until someone can suggest something that can be done, I'm closing the issue as wontfix.

@skarekrow skarekrow added wontfix and removed bug labels Aug 29, 2017
@Davis-A
Copy link
Author

Davis-A commented Aug 30, 2017

I investigated how Linux LXC handles this.

It makes the directory above the jail root inaccessible to non-root users. I'm unfamiliar with the Iocage code, but I think this would break the ability for non-root users on the host to query Iocage list etc.

I'm not sure how Solaris Zones deal with it, but I discovered this issue after reading the Zones paper where the attack was described.

@skarekrow skarekrow added bug and removed wontfix labels Sep 1, 2017
@skarekrow
Copy link
Member

That's exactly what we need to do @croquagei! Great idea. One moment, I'll push a commit.

@gronke

This comment was marked as spam.

@rwestlund
Copy link
Contributor

@gronke I meant overkill in the sense that it would be forcing a particular paradigm, rather than defaulting to it. I need to have an unprivileged process on the host (NGINX) to reach into jails. It's easy for me to add chmod 755 to my create script, but it would be difficult to fight iocage on every jail start.

I don't think there's any reason to run iocage as an unprivileged user, so I don't think there's any point in having an iocage group.

@skarekrow
Copy link
Member

I reverted this again...lol. This will be left to the user to change from now on, there just isn't a good default for us to use that allows all functionality without completely breaking other things. Since I test mostly as root this one eluded me for almost a week.

This attack seems contrived, and would require both a host and jail to be comprised at the same time. I think if a user wants to harden against this, they should do so on their own. This important security feature seems difficult to implement with the current design paradigm. I'll see if @gronke has any ideas for libiocage for this feature to be re-implemented. But the main project will not be doing it in the meantime.

skarekrow added a commit that referenced this issue Sep 12, 2020
While setting it each start seems like overkill, it's an easy way to fix existing jails, in addition to we want to make sure our users are secure.

Closes #347
skarekrow added a commit that referenced this issue Sep 12, 2020
The previous one used the jails /root for the permissions which is wrong. Doing it one step above protects the jail in general and allows things to function securely.
Closes #347
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants