Skip to content

Conversation

@giuseppe
Copy link
Member

@giuseppe giuseppe commented Aug 5, 2018

Lookup the current username by UID if the USER env variable is not
set.

Reported in: #1092

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

@@ -7,6 +7,7 @@ import (
"io/ioutil"
"os"
"os/exec"
"os/user"
Copy link
Member

Choose a reason for hiding this comment

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

Need blank line afterwards.

@@ -97,6 +98,13 @@ func BecomeRootInUserNS() (bool, int, error) {

var uids, gids []idtools.IDMap
username := os.Getenv("USER")
if username == "" {
user, err := user.LookupId(fmt.Sprintf("%d", os.Geteuid()))
Copy link
Member

Choose a reason for hiding this comment

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

Does this work with usernames not in /etc/passwd?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, it works only with users defined in /etc/passwd.

I've changed the error message with a pointer to the USER env variable

Copy link
Member

Choose a reason for hiding this comment

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

Do any go interfaces work with nsswitch?

Copy link
Member Author

Choose a reason for hiding this comment

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

from a quick search I didn't find anything, I'll look further if there is something available

Copy link
Member

Choose a reason for hiding this comment

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

@vbatts Do you know of anything?

Copy link
Member

Choose a reason for hiding this comment

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

@giuseppe @rhatdan You could use github.com/opencontainers/runc/libcontainer/user as we do in psgo (https://github.com/containers/psgo/blob/master/internal/process/process.go#L55).

Copy link
Member Author

Choose a reason for hiding this comment

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

@vrothberg looking at the code, it seems it also just parses /etc/passwd

Copy link
Member

Choose a reason for hiding this comment

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

RIght I think the libcontainer/user code is just for parsing the username inside of the container.

We need code to call into the gcc getpw library so that it will work with ldap and other providers of identity like freeipa.

@giuseppe giuseppe force-pushed the rootless-fix-lookup-when-USER-is-not-defined branch 2 times, most recently from 6276761 to 309047a Compare August 6, 2018 06:52
@@ -8,6 +8,7 @@ import (
"os"
"os/exec"
gosignal "os/signal"
"os/user"
Copy link
Member

Choose a reason for hiding this comment

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

Add line after

Copy link
Member Author

Choose a reason for hiding this comment

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

why do we need an extra line here? I thought we use it only to divide between std library and other packages

Copy link
Member

Choose a reason for hiding this comment

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

Your right, my mistake, I thought the test was complaining about it.

Lookup the current username by UID if the USER env variable is not
set.

Reported in: containers#1092

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the rootless-fix-lookup-when-USER-is-not-defined branch from 309047a to 4c814af Compare August 6, 2018 10:57
@rhatdan
Copy link
Member

rhatdan commented Aug 6, 2018

bot, retest this please

2 similar comments
@mheon
Copy link
Member

mheon commented Aug 6, 2018

bot, retest this please

@rhatdan
Copy link
Member

rhatdan commented Aug 6, 2018

bot, retest this please

@rhatdan
Copy link
Member

rhatdan commented Aug 7, 2018

LGTM

@rhatdan
Copy link
Member

rhatdan commented Aug 8, 2018

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 4c814af has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 4c814af with merge 9e06478...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr
Approved by: rhatdan
Pushing 9e06478 to master...

@giuseppe giuseppe deleted the rootless-fix-lookup-when-USER-is-not-defined branch February 26, 2019 10:31
@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 Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants