[
https://issues.apache.org/jira/browse/YARN-7221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16401161#comment-16401161
]
Eric Badger commented on YARN-7221:
-----------------------------------
bq. Eric Badger Are you running sudo -U ebadger -n -l docker as root user or
yarn? When container-executor runs this check, it is using root privileges to
check, thus password prompt is omitted. The sudo session doesn't exist beyond
the check.
I was running as ebadger, which is why I saw that. Makes sense on why I was
seeing what I saw.
{noformat}
+ char tmpl[] = "id -G -n %s";
+ char buffer[4096];
+ if (fork()==0) {
+ char *cmd = (char *) alloc_and_clear_memory(strlen(tmpl) + strlen(user),
sizeof(char));
+ sprintf(cmd, tmpl, user);
{noformat}
Is there a reason for tmpl? It doesn't seem to be necessary here. We can just
put it into the sprintf. And even more, is there a reason we can't use
{{getgroups()}} instead of calling {{id}}? Seems unnecessary to shell out the
call to {{id}} that we can do in C land.
{noformat}
+ if (fp == NULL) {
+ exit(127);
+ }
{noformat}
Missing a free for {{cmd}} here.
{noformat}
+ if (strcmp(token, "root")==0 || strcmp(token, "docker")==0) {
+ pclose(fp);
+ free(cmd);
{noformat}
Missing a free for {{token}} here.
{noformat}
+ wait(&statval);
+ if (WIFEXITED(statval)) {
+ if (WEXITSTATUS(statval)==0) {
+ return 1;
+ }
+ }
+ }
+ return 0;
{noformat}
Since returning 1 is "success" in this case, I think a comment might be useful.
Just a simple "//success" or something like that, since returning 1 usually
implies failure when the only options are 0 and 1.
> Add security check for privileged docker container
> --------------------------------------------------
>
> Key: YARN-7221
> URL: https://issues.apache.org/jira/browse/YARN-7221
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: security
> Affects Versions: 3.0.0, 3.1.0
> Reporter: Eric Yang
> Assignee: Eric Yang
> Priority: Major
> Attachments: YARN-7221.001.patch, YARN-7221.002.patch,
> YARN-7221.003.patch, YARN-7221.004.patch, YARN-7221.005.patch,
> YARN-7221.006.patch
>
>
> When a docker is running with privileges, majority of the use case is to have
> some program running with root then drop privileges to another user. i.e.
> httpd to start with privileged and bind to port 80, then drop privileges to
> www user.
> # We should add security check for submitting users, to verify they have
> "sudo" access to run privileged container.
> # We should remove --user=uid:gid for privileged containers.
>
> Docker can be launched with --privileged=true, and --user=uid:gid flag. With
> this parameter combinations, user will not have access to become root user.
> All docker exec command will be drop to uid:gid user to run instead of
> granting privileges. User can gain root privileges if container file system
> contains files that give user extra power, but this type of image is
> considered as dangerous. Non-privileged user can launch container with
> special bits to acquire same level of root power. Hence, we lose control of
> which image should be run with --privileges, and who have sudo rights to use
> privileged container images. As the result, we should check for sudo access
> then decide to parameterize --privileged=true OR --user=uid:gid. This will
> avoid leading developer down the wrong path.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
