Skip to content

dockerd-rootless.sh: prohibit running as root#42034

Merged
thaJeztah merged 1 commit into
moby:masterfrom
AkihiroSuda:prohibit-rootless-as-root
Feb 24, 2021
Merged

dockerd-rootless.sh: prohibit running as root#42034
thaJeztah merged 1 commit into
moby:masterfrom
AkihiroSuda:prohibit-rootless-as-root

Conversation

@AkihiroSuda

Copy link
Copy Markdown
Member

- What I did

Prohibit running dockerd-rootless.sh as root

- How I did it

Added if [ $(id -u) = 0 ]; then echo "(error)"; exit 1; fi

- How to verify it

$ sudo dockerd-rootless.sh 
...
Must not run as root
+ exit 1

- A picture of a cute animal (not mandatory but encouraged)
🐧

@AkihiroSuda AkihiroSuda added area/rootless Rootless Mode kind/refactor PR's that refactor, or clean-up code labels Feb 17, 2021
Comment thread contrib/dockerd-rootless.sh Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought I left a comment, but I guess I forgot.

Does this only apply to this part of the code, or to the script as a whole? If it's for the whole script, perhaps something like;

Suggested change
echo "Must not run as root"
echo "This script must be executed as a non-privileged user"

(and in that case, I think it would make sense to put it at the top of the script, together with the other checks at the start)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the error message.

put it at the top of the script, together with the other checks at the start

No. The script "reexecs" itself inside a user namespace, so the UID flips to be zero.

@AkihiroSuda
AkihiroSuda force-pushed the prohibit-rootless-as-root branch from 7b228ea to 9351e19 Compare February 24, 2021 04:22

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah

Copy link
Copy Markdown
Member

@tianon ptal 🤗

@tianon tianon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@thaJeztah thaJeztah added this to the 21.xx milestone Feb 24, 2021
@thaJeztah
thaJeztah merged commit e8227b1 into moby:master Feb 24, 2021
@thaJeztah

Copy link
Copy Markdown
Member

@AkihiroSuda have you seen many users running into this? Did you want this backported?

@AkihiroSuda

Copy link
Copy Markdown
Member Author

Yes, backport PR: #42072

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

Labels

area/rootless Rootless Mode kind/refactor PR's that refactor, or clean-up code process/cherry-picked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants