SSSD pam module accepts usernames with leading spaces #2997

sssd-bot · 2020-05-02T12:52:29Z

Cloned from Pagure issue: https://pagure.io/SSSD/sssd/issue/1955

Created at 2013-06-03 12:34:58 by euj
Closed as Fixed
Assigned to lslebodn
Associated bugzillas
- https://bugzilla.redhat.com/show_bug.cgi?id=1065534

At login with

id_provider=ldap
access_provider=ldap
auth_provider=krb5

the SSSD pam module will authenticate usernames with leading spaces as if they did not have them. It will also write them in utmp and wtmp. This results in funny stuff like:

$ w
USER     TTY      FROM              LOGIN@   IDLE   JCPU   PCPU WHAT
root     pts/0    xxxxxxxx.xxx     10:16    3:07m  0.05s  0.05s /bin/bash
         tty1                      13:00   17days  0.03s  0.01s -bash


$ last
         tty1                          Mon Jun  3 12:52 - 13:00  (00:07)

No other pam module seems to let that through.

I have included an example sssd_pam.log.

Comments

Comment from euj at 2013-06-03 12:35:33

sssd_pam.log
sssd_pam.log

Comment from okos at 2013-06-03 14:29:29

I think we should follow the habits of other PAM modules, even though the first idea that crossed my mind was to trim the spaces before. The solution internally should be to take whole " ejarnfor" as username, which should trigger authentication error.

Comment from jhrozek at 2013-06-03 15:26:13

I don't like any form of trimming or other modification of user names, that sounds very dangerous to me.

We should consistently accept or fail, nothing in between.

Comment from dpal at 2013-06-06 15:35:15

Fields changed

keywords: => easyfix
milestone: NEEDS_TRIAGE => SSSD 1.13 beta

Comment from dpal at 2013-06-06 15:35:31

Fields changed

rhbz: => todo

Comment from jhrozek at 2013-06-06 16:21:29

Putting the ticket to 1.13 for now.

User names that start with spaces are quite a corner case, most UIs wouldn't even allow you to define such user. useradd/adduser on UNIX wouldn't and even in Active Directory the UI seems to be trimming the username not to include leading or trailing spaces.

I think we should check the leading spaces and just fail with such username, but I don't see this bug report as a priority for upstream right now.

Comment from euj at 2013-07-05 15:40:38

Mostly the point here was not to enable login for usernames with leading spaces (are those even possible/allowed/used anywhere?) but that users can hide their identity somewhat from other users by adding leading spaces to their username at login time. And that treating authentication-related strings verbatim is usually what one would expect (and what happens with other pieces of software performing the same task).

Comment from jhrozek at 2013-07-08 08:01:32

Sorry I still don't quite understand how can you hide your identity with leading spaces? Can you describe some particular use-case with other software? (nss_ldap, nss-pam-ldapd, ...)

Comment from euj at 2013-07-08 11:52:05

SSSD writes the non-trimmed strings into utmp and wtmp, which causes things that read those to display them incorrectly ("w" and "last" at least as pasted in the ticket, "who" only if you give 32+ spaces). The length of the utmp username record is (on our systems at least) 32 characters. If you login with 32 leading spaces, only the spaces will be written into wtmp and utmp. This is not a big issue for a sysadmin with access to logs etc, so it does not really mask the identity but what it does is make it harder for normal users to find out who else is using the system. Of course you can just say "don't do it" but I don't think this should be possible at all in the first place.

In my opinion, the usernames with leading spaces should either be treated verbatim so that the logins fail because the username was not correct (the sane behaviour and what everybody else does) or the trimmed version which is apparently being used internally should also be written into wtmp and utmp (and possibly someplace else too) (the semi-insane behaviour :) ).

Comment from euj at 2013-07-08 12:01:15

Ok SSSD probably doesn't write the utmp/wtmp directly now that I actually read what I wrote but by allowing the login with that kind of username, the usernames do eventually get written there.

Comment from mpesari at 2013-10-18 09:24:08

sanitize whitespace character in ldap filter
sanitize_whitespace.patch

Comment from mpesari at 2013-10-18 09:31:40

I added a patch that fixes this for me on 1.9.1. It adds the whitespace character to the list of characters to be sanitized (replaced with the hex representation) before sending to ldap/ldb (can't remember the details). This causes authentication of usernames with extra whitespace to fail.

Maybe someone with understanding of sssd internals could comment on the sanity of this fix?

Comment from jhrozek at 2013-10-18 12:09:55

Replying to [comment:10 mpesari]:

I added a patch that fixes this for me on 1.9.1. It adds the whitespace character to the list of characters to be sanitized (replaced with the hex representation) before sending to ldap/ldb (can't remember the details). This causes authentication of usernames with extra whitespace to fail.

Maybe someone with understanding of sssd internals could comment on the sanity of this fix?

Thanks for the patch, so far I haven't tested it, but can you send it to the sssd-devel list?

Also, have you checked if the patch doesn't break authentication of users with space in their username? (ssh client.example.com -l "some user")

Comment from jhrozek at 2014-02-20 10:49:32

Linked to Bugzilla bug: https://bugzilla.redhat.com/show_bug.cgi?id=1065534 (Red Hat Enterprise Linux 6)

rhbz: todo => [https://bugzilla.redhat.com/show_bug.cgi?id=1065534 1065534]

Comment from lslebodn at 2014-02-24 13:20:12

Fields changed

owner: somebody => lslebodn
patch: 0 => 1
status: new => assigned

Comment from jhrozek at 2014-02-24 19:00:03

This fix was requestes by a downstream, so I'm moving it to 1.11.5

milestone: SSSD 1.14 beta => SSSD 1.11.5
priority: major => critical

Comment from jhrozek at 2014-02-26 14:19:18