Issue 6550 - Patch for smbk5pwd slapd overlay to include shadowLastChange
Summary: Patch for smbk5pwd slapd overlay to include shadowLastChange
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-14 02:37 UTC by online@mark.ziesemer.com
Modified: 2010-06-01 18:35 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description online@mark.ziesemer.com 2010-05-14 02:37:09 UTC
Full_Name: Mark A. Ziesemer
Version: 2.4.21 / HEAD
OS: Ubuntu Linux
URL: ftp://ftp.openldap.org/incoming/smbk5pwd-shadow-b.patch
Submission from: (NULL) (2001:470:1f11:3ae:dc54:73ba:be16:148)


Using the PasswordModify Extended Operation (exop) along with the smbk5pwd slapd
overlay provides several benefits, but does not currently include the
shadowLastChange attribute of the shadowAccount class.  This means the
shadowLastChange is missed from update, unless specially done along with a
PasswordModify.

This patch adds support for updating shadowLastChange into the smbk5pwd overlay
for slapd.

An added benefit is that once the updated overlay is in effect, write access to
the shadowLastChange attribute can optionally be restricted by configuration,
preventing users from updating shadowLastChange without actually updating their
password.

The SHA-1 hash of the provided patch (smbk5pwd-shadow-b.patch) is
c29ff518ea4fe03a4c5ee87d07a3af0082256950 .  (Please discard
"smbk5pwd-shadow.patch".)

Patch was generated against HEAD just now, but also applies cleanly to 2.4.21.

I am currently using the patched overlay in my current environment without
noticeable issue.  However, C is not current primary language, so please give
appropriate attention to review.

This patch file is derived from OpenLDAP Software. All of the modifications to
OpenLDAP Software represented in the following patch were developed by Mark A.
Ziesemer <online@mark.ziesemer.com>. I have not assigned rights and/or interest
in this work to any party. 

I, Mark A. Ziesemer, hereby place the following modifications to OpenLDAP
Software (and only these modifications) into the public domain. Hence, these
modifications may be freely used and/or redistributed for any purpose with or
without attribution and/or other notice.
Comment 1 Michael Ströder 2010-05-14 10:35:39 UTC
online@mark.ziesemer.com wrote:
> Full_Name: Mark A. Ziesemer
> Version: 2.4.21 / HEAD
> OS: Ubuntu Linux
> URL: ftp://ftp.openldap.org/incoming/smbk5pwd-shadow-b.patch
> Submission from: (NULL) (2001:470:1f11:3ae:dc54:73ba:be16:148)
> 
> Using the PasswordModify Extended Operation (exop) along with the smbk5pwd slapd
> overlay provides several benefits, but does not currently include the
> shadowLastChange attribute of the shadowAccount class.  This means the
> shadowLastChange is missed from update, unless specially done along with a
> PasswordModify.

While I agree that this could be useful in general I'd rather argue that for
Samba 3 'sambaPwdLastSet' should be set.

'shadowLastChange' is rather a POSIX account attribute which from my
understanding is out-of-scope for slapo-smbk5pwd. Well, the scope could be
extended...

Ciao, Michael.

Comment 2 Michael Ströder 2010-05-14 11:25:16 UTC
michael@stroeder.com wrote:
> I'd rather argue that for
> Samba 3 'sambaPwdLastSet' should be set.

Uumpf! This is already set. Sorry for the noise.

> 'shadowLastChange' is rather a POSIX account attribute which from my
> understanding is out-of-scope for slapo-smbk5pwd. Well, the scope could be
> extended...

But still it's the question whether we want to have this functionality for
various password-related attribute all in on overlay or whether there should
be distinct overlays for each account type (posixAccount/shadowAccount,
sambaSAMAccount, Kerberos user).

Personally I'd like to see this overlay moved from contrib/ into the standard
build. But for Kerberos-related attributes the build and schema dependencies
are an obstacle. => separate overlays at least for KDC/LDAP and Samba-Posix/LDAP.

Ciao, Michael.

Comment 3 Howard Chu 2010-05-14 13:00:53 UTC
michael@stroeder.com wrote:
> michael@stroeder.com wrote:
>> I'd rather argue that for
>> Samba 3 'sambaPwdLastSet' should be set.
>
> Uumpf! This is already set. Sorry for the noise.
>
>> 'shadowLastChange' is rather a POSIX account attribute which from my
>> understanding is out-of-scope for slapo-smbk5pwd. Well, the scope could be
>> extended...
>
> But still it's the question whether we want to have this functionality for
> various password-related attribute all in on overlay or whether there should
> be distinct overlays for each account type (posixAccount/shadowAccount,
> sambaSAMAccount, Kerberos user).

shadowAccount is deprecated. LDAP ppolicy already provides a pwdChangedTime 
attribute.

> Personally I'd like to see this overlay moved from contrib/ into the standard
> build. But for Kerberos-related attributes the build and schema dependencies
> are an obstacle. =>  separate overlays at least for KDC/LDAP and Samba-Posix/LDAP.

Ultimately both Kerberos and Samba will just be using LDAP ppolicy. But yes, 
the build dependencies are still annoying.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 4 online@mark.ziesemer.com 2010-05-14 13:29:24 UTC
2010/5/14 Michael Ströder <michael@stroeder.com>

> online@mark.ziesemer.com wrote:
> > Full_Name: Mark A. Ziesemer
> > Version: 2.4.21 / HEAD
> > OS: Ubuntu Linux
> > URL: ftp://ftp.openldap.org/incoming/smbk5pwd-shadow-b.patch
> > Submission from: (NULL) (2001:470:1f11:3ae:dc54:73ba:be16:148)
> >
> > Using the PasswordModify Extended Operation (exop) along with the
> smbk5pwd slapd
> > overlay provides several benefits, but does not currently include the
> > shadowLastChange attribute of the shadowAccount class.  This means the
> > shadowLastChange is missed from update, unless specially done along with
> a
> > PasswordModify.
>
> While I agree that this could be useful in general I'd rather argue that
> for
> Samba 3 'sambaPwdLastSet' should be set.
>

sambaPwdLastSet is already handled by the "samba" portion of this overlay.

'shadowLastChange' is rather a POSIX account attribute which from my
> understanding is out-of-scope for slapo-smbk5pwd. Well, the scope could be
> extended...
>

I guess I wouldn't have any objections if all the references to "shadow"
were renamed to "posix".  However, the shadowLastChange attribute is part of
the shadowAccount objectClass - with neither of these names referring to
POSIX.

I had considered a separate overlay.  However, in terms of purpose, shared
code, functionality, and performance, it seems to make the most sense to
include this addition into the smbk5pwd overlay.

Both pam_ldap and the Samba client support use of exop password changes.
Additionally, pam_ldap doesn't appear to support hashing to SSHA (only MD5,
which is also the default) - so setting to "exop" also allows for a stronger
hash of the password to be stored.

With the unpatched overlay, doing an exop password change updates
userPassword (used by POSIX), as well as all the Samba attributes:
sambaLMPassword, sambaNTPassword, and sambaPwdLastSet .  This allows Samba
clients to use the updated password as well as seeing when the password was
last set, but POSIX clients do not see an updated shadowLastChange.  This
patch adds support for the otherwise missing shadowLastChange, keeping
everything consistent.

There are many issues posted online with all the password attributes except
shadowLastChange getting updated.  This patch should provide a solution for
many of these cases.


> Ciao, Michael.
>

--
Mark A. Ziesemer
www.ziesemer.com
Comment 5 Michael Ströder 2010-05-14 14:03:45 UTC
Mark A. Ziesemer wrote:
> 2010/5/14 Michael Ströder <michael@stroeder.com
> <mailto:michael@stroeder.com>>
> 'shadowLastChange' is rather a POSIX account attribute which from my 
> understanding is out-of-scope for slapo-smbk5pwd. Well, the scope could be 
> extended...
> 
> I guess I wouldn't have any objections if all the references to "shadow"
> were renamed to "posix".  However, the shadowLastChange attribute is
> part of the shadowAccount objectClass - with neither of these names
> referring to POSIX.

I didn't consider to change the name of the attribute. With POSIX account data
I rather wanted to point to RFC 2307 where posixAccount and shadowAccount
object classes and the accompanying attributes are defined.

Don't get me wrong. I support the idea of setting shadowLastChange even if
Howard considers it to be deprecated. And I have no objections to a
one-sets-all-of-these overlay.

But I'd even like to see this overlay available as standard feature. Since in
the current state it has build dependencies to Kerberos libs this is not easy.
Only building the Samba support is possible and needs some tweaking of the
Makefile.

> There are many issues posted online with all the password attributes
> except shadowLastChange getting updated.  This patch should provide a
> solution for many of these cases.

Yupp. I already thought these problems long ago when implementing the
different password change use-cases in web2ldap.

Ciao, Michael.

Comment 6 Michael Ströder 2010-05-14 14:06:07 UTC
Howard Chu wrote:
> michael@stroeder.com wrote:
>> michael@stroeder.com wrote:
>>> I'd rather argue that for
>>> Samba 3 'sambaPwdLastSet' should be set.
>>
>> Uumpf! This is already set. Sorry for the noise.
>>
>>> 'shadowLastChange' is rather a POSIX account attribute which from my
>>> understanding is out-of-scope for slapo-smbk5pwd. Well, the scope
>>> could be
>>> extended...
>>
>> But still it's the question whether we want to have this functionality
>> for
>> various password-related attribute all in on overlay or whether there
>> should
>> be distinct overlays for each account type (posixAccount/shadowAccount,
>> sambaSAMAccount, Kerberos user).
> 
> shadowAccount is deprecated. LDAP ppolicy already provides a
> pwdChangedTime attribute.

While I agree that slapo-ppolicy is the better solution in the long run I see
no reason why to not set both attributes at the server's side to make older
LDAP clients happy.

> Ultimately both Kerberos and Samba will just be using LDAP ppolicy.

Yes. But there is indeed a real need for a solution in the meantime...

Ciao, Michael.

Comment 7 Howard Chu 2010-05-14 14:34:44 UTC
Michael Ströder wrote:
> Howard Chu wrote:
>> michael@stroeder.com wrote:
>>> michael@stroeder.com wrote:
>>>> I'd rather argue that for
>>>> Samba 3 'sambaPwdLastSet' should be set.
>>>
>>> Uumpf! This is already set. Sorry for the noise.
>>>
>>>> 'shadowLastChange' is rather a POSIX account attribute which from my
>>>> understanding is out-of-scope for slapo-smbk5pwd. Well, the scope
>>>> could be
>>>> extended...
>>>
>>> But still it's the question whether we want to have this functionality
>>> for
>>> various password-related attribute all in on overlay or whether there
>>> should
>>> be distinct overlays for each account type (posixAccount/shadowAccount,
>>> sambaSAMAccount, Kerberos user).
>>
>> shadowAccount is deprecated. LDAP ppolicy already provides a
>> pwdChangedTime attribute.
>
> While I agree that slapo-ppolicy is the better solution in the long run I see
> no reason why to not set both attributes at the server's side to make older
> LDAP clients happy.

This is not a realistic use case. smbk5pwd was written starting in 2004; 
pam_ldap started supporting LDAP password policy long before then. Anyone 
running LDAP clients (pam_ldap, nss_ldap) older than that has far worse 
problems to worry about.

>> Ultimately both Kerberos and Samba will just be using LDAP ppolicy.
>
> Yes. But there is indeed a real need for a solution in the meantime...

Yes, in the meantime both Heimdal and Samba use the smbPwdLastSet attribute 
which is already taken care of.

This ITS will be closed.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 8 Michael Ströder 2010-05-15 13:49:37 UTC
Howard Chu wrote:
> Michael Ströder wrote:
>> While I agree that slapo-ppolicy is the better solution in the long run I
>> see no reason why to not set both attributes at the server's side to
>> make older LDAP clients happy.
> 
> This is not a realistic use case. smbk5pwd was written starting in 2004;
> pam_ldap started supporting LDAP password policy long before then.

Yes, pam_ldap supports enforcing the password policy probably by correcty
handling the response controls. Grepping through the source of recent versions
it seems to me it does not read attribute pwdChangedTime nor does nss_ldap.

> Anyone running LDAP clients (pam_ldap, nss_ldap) older than that has far
> worse problems to worry about.

AFAICS nss_ldap cannot deliver the correct value for 'shadowLastChange' when
someone or something invokes a call like this

getent shadow michael

'pwdChangedTime' is of syntax Generalized Time whereas 'shadowLastChange' is
Integer with seconds since epoch. In theory nss_ldap could convert it. But
AFAICs it doesn't. Also if an older client would search for
(shadowLastChange<=<value>) this wouldn't work either.

> This ITS will be closed.

Well, you're the OpenLDAP boss and free to refuse anything you want. But
personally I don't understand your strong objections.

Ciao, Michael.

Comment 9 Howard Chu 2010-05-15 18:19:47 UTC
Michael Ströder wrote:
> Howard Chu wrote:
>> Michael Ströder wrote:
>>> While I agree that slapo-ppolicy is the better solution in the long run I
>>> see no reason why to not set both attributes at the server's side to
>>> make older LDAP clients happy.
>>
>> This is not a realistic use case. smbk5pwd was written starting in 2004;
>> pam_ldap started supporting LDAP password policy long before then.
>
> Yes, pam_ldap supports enforcing the password policy probably by correcty
> handling the response controls. Grepping through the source of recent versions
> it seems to me it does not read attribute pwdChangedTime nor does nss_ldap.

Because clients don't need to read the value. Since password modification is 
all managed on the server, it's an irrelevant detail on the client.

>> Anyone running LDAP clients (pam_ldap, nss_ldap) older than that has far
>> worse problems to worry about.
>
> AFAICS nss_ldap cannot deliver the correct value for 'shadowLastChange' when
> someone or something invokes a call like this
>
> getent shadow michael

Nobody does that. Normal users don't even have read permission to do that.

> 'pwdChangedTime' is of syntax Generalized Time whereas 'shadowLastChange' is
> Integer with seconds since epoch. In theory nss_ldap could convert it. But
> AFAICs it doesn't. Also if an older client would search for
> (shadowLastChange<=<value>) this wouldn't work either.

You've just proven the point why shadowLastChange is problematic. The encoded 
value is in *minutes* since the epoch. All of the shadow values were poorly 
defined to begin with (talking about /etc/shadow, not just RFC2307), 
inconsistent with common Unix practice, and most people don't understand them 
anyway. They have no role in an LDAP-enabled environment, all they do is 
perpetuate confusion.

>> This ITS will be closed.
>
> Well, you're the OpenLDAP boss and free to refuse anything you want. But
> personally I don't understand your strong objections.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 10 Howard Chu 2010-06-01 18:35:39 UTC
changed state Open to Closed