[SSSD] [PATCH] Two enhancements for PAM client

Stephen Gallagher sgallagh at redhat.com
Thu Apr 22 13:03:25 UTC 2010


On 04/22/2010 06:08 AM, Sumit Bose wrote:
> Hi,
>
> the two patches attached should fix #446 and #417 respectively.
>
> For #417 a different solution, where the message is generated by SSSD
> and send to the client, would be possilbe. But I decided against it,
> because with the attached patch it is possilbe to support localized
> messages.


Patch 0001: Ack.

Patch 0002: Nack. You would be introducing a security hole here in the 
form of a stat-open race condition. Please open the file first and use 
fstat() to determine its size. It probably wouldn't be a bad idea to 
verify that the file is owned by root and not writable by anyone but 
root. I don't think it's important to verify whether the file is a 
symlink or a regular file.

Even making the above check, it wouldn't be a bad idea to verify that 
total_len == stat_buf.st_size after the read loop.

The DEBUG message in the read loop is wrong. It should be "read failed", 
not "open failed".

Also, if you're going to allow custom messages here, we probably need to 
make this message file be per-domain. Since it may be translatable, I 
suggest we add a new directory structure under /etc/sssd that looks like 
this (assuming the domain name is LDAP):

/etc/sssd/customize
/etc/sssd/customize/LDAP/
/etc/sssd/customize/LDAP/root_pw_reset.txt
/etc/sssd/customize/LDAP/root_pw_reset.zh_TW
...


-- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/



More information about the sssd-devel mailing list