Password history wasn't working because the qsort comparison function was comparing pointers, not data. This resulted in a random element being removed from the history on overflow rather than the oldest.

We sort in reverse so we don't have to move elements inside the list when removing to make more room. We just pop off the top then shove on the new password. The history includes a time to make comparisons straightforward (and LDAP doesn't guarantee order).

I've attached a test script to exercise things. I don't see a way to easily include this into our current framework at the moment. We'd need a way to switch users in the middle of a test.

rob
>From 8f2dc99598dd30cac92fa3f6b86bae0148242587 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 9 Apr 2012 23:42:41 -0400
Subject: [PATCH] Dereference pointer when comparing password history in qsort
 compare.

The man page for qsort(3) says that the comparison function is called
with pointers to pointers to char but memcmp(3) wants a pointer to void
so we need to cast and dereference.

Without this the qsort() call wasn't properly sorting the elements so
a random password was being removed rather than the oldest when the
list overflowed.

https://fedorahosted.org/freeipa/ticket/2613
---
 util/ipa_pwd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c
index b6ed929b3761fbce8f0ce90e6555123848d0c88d..92fb3b0298418592881d100fb7a9ccfac99fd665 100644
--- a/util/ipa_pwd.c
+++ b/util/ipa_pwd.c
@@ -152,7 +152,7 @@ static int ipapwd_gentime_cmp(const void *p1, const void *p2)
      * a higher letter or number */
 
     /* return youngest first by inverting terms */
-    return memcmp(p2, p1, GENERALIZED_TIME_LENGTH);
+    return memcmp(*(void * const *)p2, *(void * const *)p1, GENERALIZED_TIME_LENGTH);
 }
 
 #define SHA_SALT_LENGTH 8
-- 
1.7.6

#!/bin/sh

SLEEP=1

echo password | kinit admin
ipa user-del tuser1

echo "Set password policy"
ipa pwpolicy-mod --history=3 --minlife=0

echo "Create user"
echo password | ipa user-add --first=tim --last=user tuser1 --password
sleep $SLEEP

echo "Password 1"
echo -e 'password\nredhat001\nredhat001\n' | kinit tuser1
sleep $SLEEP

echo "Password 2"
echo -e 'redhat001\nredhat002\nredhat002' | ipa passwd
sleep $SLEEP

echo "Password 3"
echo -e 'redhat002\nredhat003\nredhat003' | ipa passwd
sleep $SLEEP

echo "Try resetting to password 1: it should fail"
echo -e 'redhat003\nredhat001\nredhat001' | ipa passwd
sleep $SLEEP

echo "Password 4"
echo -e 'redhat003\nredhat004\nredhat004' | ipa passwd
sleep $SLEEP

echo "Try resetting to password 1: it should succeed"
echo -e 'redhat004\nredhat001\nredhat001' | ipa passwd
sleep $SLEEP

echo "Try resetting to password 3: it should fail"
echo -e 'redhat001\nredhat003\nredhat003' | ipa passwd
sleep $SLEEP

echo "Try resetting to password 2: it should succeed"
echo -e 'redhat001\nredhat002\nredhat002' | ipa passwd
sleep $SLEEP

echo "Try resetting to password 4: it should fail"
echo -e 'redhat002\nredhat004\nredhat004' | ipa passwd
sleep $SLEEP

echo "Try resetting to password 3: it should succeed"
echo -e 'redhat002\nredhat003\nredhat003' | ipa passwd
sleep $SLEEP
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to