From 0fb86b794ac4d38cfe04f53619aca44968307d73 Mon Sep 17 00:00:00 2001 From: Quanah Gibson-Mount Date: Wed, 13 Feb 2008 02:58:56 +0000 Subject: [PATCH] ITS#5373 --- CHANGES | 1 + servers/slapd/overlays/ppolicy.c | 87 +++++++++++++++----------------- 2 files changed, 42 insertions(+), 46 deletions(-) diff --git a/CHANGES b/CHANGES index cb65735334..d2eb211e90 100644 --- a/CHANGES +++ b/CHANGES @@ -46,6 +46,7 @@ OpenLDAP 2.4.8 Engineering Added slapo-memberof global overlay support (ITS#5301) Fixed slapo-memberof leak (ITS#5302) Fixed slapo-ppolicy only password check with policy (ITS#5285) + Fixed slapo-ppolicy del/replace password without new one (ITS#5373) Fixed slapo-syncrepl hang on checkpoint (ITS#5261) Added slapo-translucent local searching (ITS#5283) Build Environment diff --git a/servers/slapd/overlays/ppolicy.c b/servers/slapd/overlays/ppolicy.c index c1f2eb387a..622fba7f9f 100644 --- a/servers/slapd/overlays/ppolicy.c +++ b/servers/slapd/overlays/ppolicy.c @@ -1550,35 +1550,35 @@ ppolicy_modify( Operation *op, SlapReply *rs ) delmod = ml; } - if ((deladd == 1) && ((ml->sml_op == LDAP_MOD_ADD) || - (ml->sml_op == LDAP_MOD_REPLACE))) - { - deladd = 2; - } - if ((ml->sml_op == LDAP_MOD_ADD) || (ml->sml_op == LDAP_MOD_REPLACE)) { - /* FIXME: there's no easy way to ensure - * that add does not cause multiple - * userPassword values; one way (that - * would be consistent with the single - * password constraint) would be to turn - * add into replace); another would be - * to disallow add. - * - * Let's check at least that a single value - * is being added - */ - assert( ml->sml_values != NULL ); - assert( !BER_BVISNULL( &ml->sml_values[ 0 ] ) ); - if ( addmod || !BER_BVISNULL( &ml->sml_values[ 1 ] ) ) { - rs->sr_err = LDAP_CONSTRAINT_VIOLATION; - rs->sr_text = "Password policy only allows one password value"; - goto return_results; + if ( ml->sml_values && !BER_BVISNULL( &ml->sml_values[0] )) { + if ( deladd == 1 ) + deladd = 2; + + /* FIXME: there's no easy way to ensure + * that add does not cause multiple + * userPassword values; one way (that + * would be consistent with the single + * password constraint) would be to turn + * add into replace); another would be + * to disallow add. + * + * Let's check at least that a single value + * is being added + */ + if ( addmod || !BER_BVISNULL( &ml->sml_values[ 1 ] ) ) { + rs->sr_err = LDAP_CONSTRAINT_VIOLATION; + rs->sr_text = "Password policy only allows one password value"; + goto return_results; + } + + addmod = ml; + } else { + /* replace can have no values, add cannot */ + assert( ml->sml_op == LDAP_MOD_REPLACE ); } - - addmod = ml; } } else if ( !is_at_operational( ml->sml_desc->ad_type ) ) { @@ -1622,30 +1622,18 @@ ppolicy_modify( Operation *op, SlapReply *rs ) * if we have a "safe password modify policy", then we need to check if we're doing * a delete (with the old password), followed by an add (with the new password). * - * If we don't have this, then we fail with an error. We also skip all the checks if + * If we got just a delete with nothing else, just let it go. We also skip all the checks if * the root user is bound. Root can do anything, including avoid the policies. */ if (!pwmod) goto do_modify; - /* - * Did we get a valid add mod? - */ - - if (!addmod) { - rs->sr_err = LDAP_OTHER; - rs->sr_text = "Internal Error"; - Debug( LDAP_DEBUG_TRACE, - "cannot locate modification supplying new password\n", 0, 0, 0 ); - goto return_results; - } - /* * Build the password history list in ascending time order * We need this, even if the user is root, in order to maintain * the pwdHistory operational attributes properly. */ - if (pp.pwdInHistory > 0 && (ha = attr_find( e->e_attrs, ad_pwdHistory ))) { + if (addmod && pp.pwdInHistory > 0 && (ha = attr_find( e->e_attrs, ad_pwdHistory ))) { struct berval oldpw; time_t oldtime; @@ -1667,6 +1655,20 @@ ppolicy_modify( Operation *op, SlapReply *rs ) if (be_isroot( op )) goto do_modify; + if (!pp.pwdAllowUserChange) { + rs->sr_err = LDAP_INSUFFICIENT_ACCESS; + rs->sr_text = "User alteration of password is not allowed"; + pErr = PP_passwordModNotAllowed; + goto return_results; + } + + /* Just deleting? */ + if (!addmod) { + /* skip everything else */ + pwmod = 0; + goto do_modify; + } + /* This is a pwdModify exop that provided the old pw. * We need to create a Delete mod for this old pw and * let the matching value get found later @@ -1697,13 +1699,6 @@ ppolicy_modify( Operation *op, SlapReply *rs ) goto return_results; } - if (!pp.pwdAllowUserChange) { - rs->sr_err = LDAP_INSUFFICIENT_ACCESS; - rs->sr_text = "User alteration of password is not allowed"; - pErr = PP_passwordModNotAllowed; - goto return_results; - } - /* Check age, but only if pwdReset is not TRUE */ pa = attr_find( e->e_attrs, ad_pwdReset ); if ((!pa || !bvmatch( &pa->a_nvals[0], &slap_true_bv )) && -- 2.39.5