Skip to content
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

ws: Accept upper-case login names for ssh/polkit agent challenges #13934

Merged

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Apr 21, 2020

Identity management domain users are usually case insensitive, in
particular the domain part. E. g. with FreeIPA, "user@domain" and
"user@DOMAIN" mean exactly the same. With AD, even the user name is case
insensitive, and in fact the canonical name starts with an upper case
("Administrator@DOMAIN").

Linux' canonical form (reverse resolution of uids, or $USER) is
lower-case, though. This led to a failed "original vs. challenge
subject" string comparison in authorize_check_user(), resulting in
sessions not getting root privileges.

To fix this, if a direct comparison fails, compare again against the
lower-case form of the credential's user name. This avoids having to
decode the subject's hex string and thus introducing more protocol
assumptions.

Note that native Linux user names are case sensitive, i. e. "user" and
"User" are both legitimate and different. This comparison is just a
plausibility check for not accidentally logging into a remote machine
that has a different user name and spilling the password. It's
acceptable to introduce the corner case for auto-logging into remote
machines if the remote user name only differs in case.

Revert the workaround for this bug from commit XXXXX, and explicitly
test a variation in case.

https://bugzilla.redhat.com/show_bug.cgi?id=1825749

@martinpitt martinpitt marked this pull request as ready for review April 21, 2020 20:04
Identity management domain users are usually case insensitive, in
particular the domain part. E. g. with FreeIPA, "user@domain" and
"user@DOMAIN" mean exactly the same. With AD, even the user name is case
insensitive, and in fact the canonical name starts with an upper case
("Administrator@DOMAIN").

Linux' canonical form (reverse resolution of uids, or `$USER`) is
lower-case, though. This led to a failed "original vs. challenge
subject" string comparison in authorize_check_user(), resulting in
sessions not getting root privileges.

To fix this, if a direct comparison fails, compare again against the
lower-case form of the credential's user name. This avoids having to
decode the subject's hex string and thus introducing more protocol
assumptions.

Note that native Linux user names are case sensitive, i. e. "user" and
"User" are both legitimate and different. This comparison is just a
plausibility check for not accidentally logging into a remote machine
that has a different user name and spilling the password. It's
acceptable to introduce the corner case for auto-logging into remote
machines if the remote user name only differs in case.

Revert the workaround for this bug from commit c5ee044, and explicitly
test a variation in case.

https://bugzilla.redhat.com/show_bug.cgi?id=1825749

Closes cockpit-project#13934
@martinpitt
Copy link
Member Author

Still a distropkg failure, fixed.

@martinpitt martinpitt added the release-blocker Targetted for next release label Apr 24, 2020
@martinpitt
Copy link
Member Author

There are still two flakes about realm join taking more than a minute, we need to bump the timeout there. But that's entirely unrelated to this PR, I'll send a follow-up.

@martinpitt martinpitt merged commit 1a5f618 into cockpit-project:master Apr 24, 2020
@martinpitt martinpitt deleted the domain-users-uppercase branch April 24, 2020 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Targetted for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants