-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow UID to be returned by the mapper script. #3795
Conversation
…in multi-domain situations where "username" may be different people in different domains.
I'll have to think about this for a bit and the ramifications. It seems OK on the surface, but I just need a second to consider what side affects it could have. Also - the CI is failing, but not because of this change. I'm fixing the CI in another PR. |
nginx_stage/lib/nginx_stage/user.rb
Outdated
@@ -33,11 +33,12 @@ class User | |||
# @param user [String] the user name defining this object | |||
# @raise [ArgumentError] if user or primary group doesn't exist on local system | |||
def initialize(user) | |||
@passwd = Etc.getpwnam user.to_s | |||
user_is_uid = (user[0].to_i and user.to_i > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why check the first character here - user[0]
? "123jeff".to_i
returns 123
and "jeff123".to_i
returns 0.
Maybe we need a regex to account for the case when user
is a proper username that just happens to start with a number. Maybe user.to_s.chomp.match?(/\A\d+\Z/)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usernames are illegal to start with numbers per the POSIX standard (^[a-z][-a-z0-9]*$").
I was of the impression there were cases that certain strings could return numbers (I am not a Ruby expert), but you are correct, just user.to_i > 0 is shorter. I am away from the desk, but I will change that soon. I didn't want to do a regexp because of the overhead of parsing regexp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Usernames are illegal to start with numbers per the POSIX standard (^[a-z][-a-z0-9]*$").
If only this were true! Or at least, if only all Linux distros adhered to it. Here I just hopped into a rocky 9 container and made the 123jeff
user without any issue.
And just for kicks - there's the --badname
flag (lol)
I just went through this with #3753 having to account for @
in usernames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put a new version up that works in various test I did and this will 'fall through' if someone out there has all-number login as their user name.
I also found a bug in the group conversion. Eg "domain users" is a group name that SSSD can return. So again, I used number id and then map them to an array of strings using Etc.
@johrstrom Thanks: I understand there may be other parts, I have tested it in my environment and so far have not seen any side effects. As far as the ramifications, in our environment this is a security "hole" basically simply mapping the OIDC natively to a username maps occasionally to the "wrong" user. So let's say we get back [email protected] from Globus, simply chopping off the domain is not enough, because there may be a [email protected]. SSSD will resolve johrstom to say 12345 whereas [email protected] should resolve to 54321 thereby launching OOD NGINX sudo'ed with permissions to someone else's home directory. We are in a situation where multiple domains and an LDAP have merged over time, so you have your "LDAP username", your "AD1 username" and "AD2 username". For new accounts those are coordinated, for some people for historical reasons, they are not and jdoe@AD1 is not jdoe@AD2 or jdoe@LDAP and although AD1 and AD2 are in the same forest whichever primary domain you resolve to will return with "an" uid. I have built a Python script that thus queries both domains for the attributes we get from Globus, but I ran into the issue that just returning a username would occassionally resolve into the "wrong" user (ID). Oh, and it is completely legal from SSSD's perspective to have id -un uid1 and id -un uid2 resolve to the same username, SSSD keeps track of it with the domain in the background and it's just printing the same user, behind the scenes all the processes uses pure UID. |
Sorry for the delay on this. This week just got super busy for me, so I'll try to carve out time later in the week. |
Make sure we only rescue from Etc failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution (and the bugfix with get_groups
)!
In multi-domain situations where the same "username" may be different people in different domains, you need to be able to map a user to a uid number.
This fixes the Ruby and Lua script to be able to handle both strings and user id's.
Ideally, OOD would not rely on usernames as the recommended SSSD 'fix' (full_name_format = %1$s) allows for users returning the same username with a simple getpwnam(str).