Browse Source

Bug 829213 - unhashed#user#password visible after changing password https://bugzilla.redhat.com/show_bug.cgi?id=829213

Bug 830001 - unhashed#user#password visible after changing password [rhel-6.3]
https://bugzilla.redhat.com/show_bug.cgi?id=830001

Bug Description: unhashed password is stored in the entry in memory
when an entry/a password is added or the password is modified.
The password could be visible by the ordinary search if the type
"unhashed#user#password" is specified in the attribute list.

Fix Description:
1. Set "unhashed#user#password" to the forbidden attribute list,
   which is dropped from the search attribute list.
2. Get effective right does not return "unhashed#user#password"
3. In the modify operation, adding "unhashed#user#password" to or
   deleting "unhashed#user#password" from the entry never returns
   an error regardless of the attribute value.  Internally, the
   operation is ignored.
(cherry picked from commit 9df3c438ebd05bbaa5e7b2506fc5d5e9f3ff4a95)
(cherry picked from commit 8f0811a86a1b233cf9566349653ef7f184278144)
(Fixed conflicts in ldap/servers/slapd/{entry.c,entrywsi.c,slapi-private.h)
Noriko Hosoi 13 years ago
parent
commit
8f9e49e73e

+ 8 - 1
ldap/servers/slapd/attr.c

@@ -805,7 +805,14 @@ attr_add_valuearray(Slapi_Attr *a, Slapi_Value **vals, const char *dn)
         for ( i = 0; vals[i] != NULL; ++i ) {
             if ( slapi_attr_value_find( a, slapi_value_get_berval(vals[i]) ) == 0 ) {
                 duplicate_index = i;
-                rc = LDAP_TYPE_OR_VALUE_EXISTS;
+                if (is_type_forbidden(a->a_type)) {
+                    /* If the attr is in the forbidden list
+                     * (e.g., unhashed password),
+                     * we don't return any useful info to the clients. */
+                    rc = LDAP_OTHER;
+                } else {
+                    rc = LDAP_TYPE_OR_VALUE_EXISTS;
+                }
                 break;
             }
         }

+ 23 - 3
ldap/servers/slapd/entry.c

@@ -70,6 +70,9 @@ static char *protected_attrs_all [] = {PSEUDO_ATTR_UNHASHEDUSERPASSWORD,
                                        SLAPI_ATTR_ENTRYDN,
                                        NULL};
 
+static char *forbidden_attrs [] = {PSEUDO_ATTR_UNHASHEDUSERPASSWORD,
+                                   NULL};
+
 /*
  * An attribute name is of the form 'basename[;option]'.
  * The state informaion is encoded in options. For example:
@@ -1624,6 +1627,18 @@ is_type_protected(const char *type)
     return 0;
 }
 
+int
+is_type_forbidden(const char *type)
+{
+    char **paap = NULL;
+    for (paap = forbidden_attrs; paap && *paap; paap++) {
+        if (0 == strcasecmp(type, *paap)) {
+            return 1;
+        }
+    }
+    return 0;
+}
+
 static void
 entry2str_internal_put_attrlist( const Slapi_Attr *attrlist, int attr_state, int entry2str_ctrl, char **ecur, char **typebuf, size_t *typebuf_len)
 {
@@ -3408,7 +3423,7 @@ delete_values_sv_internal(
 	 * add/mod operation is done, while the retried entry from the db does not
 	 * contain the attribute.
 	 */
-	if (is_type_protected(type)) {
+	if (is_type_protected(type) || is_type_forbidden(type)) {
 		flags |= SLAPI_VALUE_FLAG_IGNOREERROR;
 	}
 
@@ -3419,7 +3434,6 @@ delete_values_sv_internal(
 		retVal = attrlist_delete( &e->e_attrs, type);
 		if (flags & SLAPI_VALUE_FLAG_IGNOREERROR) {
 			return LDAP_SUCCESS;
-		} else {
 		}
 		return(retVal ? LDAP_NO_SUCH_ATTRIBUTE : LDAP_SUCCESS);
 	}
@@ -3429,6 +3443,9 @@ delete_values_sv_internal(
 	if ( a == NULL ) {
 		LDAPDebug( LDAP_DEBUG_ARGS, "could not find attribute %s\n",
 		    type, 0, 0 );
+		if (flags & SLAPI_VALUE_FLAG_IGNOREERROR) {
+			return LDAP_SUCCESS;
+		}
 		return( LDAP_NO_SUCH_ATTRIBUTE );
 	}
 
@@ -3457,8 +3474,11 @@ delete_values_sv_internal(
 					"value for attribute type %s found in "
 					"entry %s\n", a->a_type, slapi_entry_get_dn_const(e), 0 );
 			}
+			if (flags & SLAPI_VALUE_FLAG_IGNOREERROR) {
+				retVal = LDAP_SUCCESS;
+			}
 		}
-	}	
+	}
 	
 	return( retVal );
 }

+ 10 - 2
ldap/servers/slapd/entrywsi.c

@@ -634,7 +634,13 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval
 	}
 	else if (attr_state==ATTRIBUTE_DELETED)
 	{
-		retVal= LDAP_NO_SUCH_ATTRIBUTE;
+		/* If the type is in the forbidden attr list (e.g., unhashed password),
+		 * we don't return the reason of the failure to the clients. */
+		if (is_type_forbidden(type)) {
+			retVal = LDAP_SUCCESS;
+		} else {
+			retVal= LDAP_NO_SUCH_ATTRIBUTE;
+		}
 	}
 	else if (attr_state==ATTRIBUTE_NOTFOUND)
 	{
@@ -643,8 +649,10 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval
 		 * failure, as the attribute could only exist in the entry in the 
 		 * memory when the add/mod operation is done, while the retried entry 
 		 * from the db does not contain the attribute.
+		 * So is in the forbidden_attrs list.  We don't return the reason
+		 * of the failure.
 		 */
-		if (is_type_protected(type)) {
+		if (is_type_protected(type) || is_type_forbidden(type)) {
 			retVal = LDAP_SUCCESS;
 		} else {
 			if (!urp) {

+ 16 - 0
ldap/servers/slapd/pblock.c

@@ -3060,6 +3060,22 @@ slapi_pblock_set( Slapi_PBlock *pblock, int arg, void *value )
 	case SLAPI_SEARCH_ATTRS:
 		if(pblock->pb_op!=NULL)
 		{
+			char **attrs;
+			for (attrs = (char **)value; attrs && *attrs; attrs++) {
+				/* Get rid of forbidden attr, e.g.,
+				 * PSEUDO_ATTR_UNHASHEDUSERPASSWORD,
+				 * which never be returned. */
+				if (is_type_forbidden(*attrs)) {
+					char **ptr;
+					for (ptr = attrs; ptr && *ptr; ptr++) {
+						if (ptr == attrs) {
+							slapi_ch_free_string(ptr); /* free unhashed type */
+						}
+						*ptr = *(ptr + 1); /* attrs is NULL terminated;
+						                      the NULL is copied here. */
+					}
+				}
+			}
 			pblock->pb_op->o_params.p.p_search.search_attrs = (char **) value;
 		}
 		break;

+ 22 - 5
ldap/servers/slapd/plugin_internal_op.c

@@ -291,6 +291,7 @@ slapi_search_internal_set_pb (Slapi_PBlock *pb, const char *base,
                               int operation_flags)
 {
 	Operation *op;
+	char **tmp_attrs = NULL;
 	if (pb == NULL || base == NULL)
 	{
 		slapi_log_error(SLAPI_LOG_FATAL, NULL, 
@@ -304,7 +305,9 @@ slapi_search_internal_set_pb (Slapi_PBlock *pb, const char *base,
 	slapi_pblock_set(pb, SLAPI_SEARCH_SCOPE, &scope);
 	slapi_pblock_set(pb, SLAPI_SEARCH_STRFILTER, (void*)filter);
 	slapi_pblock_set(pb, SLAPI_CONTROLS_ARG, controls);
-	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, attrs);
+	/* forbidden attrs could be removed in slapi_pblock_set. */
+	tmp_attrs = slapi_ch_array_dup(attrs);
+	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, tmp_attrs);
 	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly);
 	if (uniqueid)
 	{
@@ -322,6 +325,7 @@ slapi_search_internal_set_pb_ext (Slapi_PBlock *pb, Slapi_DN *sdn,
                                   int operation_flags)
 {
 	Operation *op;
+	char **tmp_attrs = NULL;
 	if (pb == NULL || sdn == NULL)
 	{
 		slapi_log_error(SLAPI_LOG_FATAL, NULL, 
@@ -337,7 +341,9 @@ slapi_search_internal_set_pb_ext (Slapi_PBlock *pb, Slapi_DN *sdn,
 	slapi_pblock_set(pb, SLAPI_SEARCH_SCOPE, &scope);
 	slapi_pblock_set(pb, SLAPI_SEARCH_STRFILTER, (void*)filter);
 	slapi_pblock_set(pb, SLAPI_CONTROLS_ARG, controls);
-	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, attrs);
+	/* forbidden attrs could be removed in slapi_pblock_set. */
+	tmp_attrs = slapi_ch_array_dup(attrs);
+	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, tmp_attrs);
 	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly);
 	if (uniqueid)
 	{
@@ -351,6 +357,7 @@ void slapi_seq_internal_set_pb(Slapi_PBlock *pb, char *base, int type, char *att
 							  Slapi_ComponentId *plugin_identity, int operation_flags)
 {
 	Operation *op;
+	char **tmp_attrs = NULL;
 	if (pb == NULL || base == NULL)
 	{
 		slapi_log_error(SLAPI_LOG_FATAL, NULL, 
@@ -364,8 +371,10 @@ void slapi_seq_internal_set_pb(Slapi_PBlock *pb, char *base, int type, char *att
 	slapi_pblock_set(pb, SLAPI_SEQ_TYPE, &type);
 	slapi_pblock_set(pb, SLAPI_SEQ_ATTRNAME, attrname);
 	slapi_pblock_set(pb, SLAPI_SEQ_VAL, val);
-    slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, attrs);
-    slapi_pblock_set(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly);        
+	/* forbidden attrs could be removed in slapi_pblock_set. */
+	tmp_attrs = slapi_ch_array_dup(attrs);
+	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, tmp_attrs);
+	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly);        
 	slapi_pblock_set(pb, SLAPI_CONTROLS_ARG, controls);
 	slapi_pblock_set(pb, SLAPI_PLUGIN_IDENTITY, plugin_identity);
 }
@@ -383,6 +392,7 @@ static int seq_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
 	char *base;
 	char *attrname, *val;
 	Slapi_DN *sdn = NULL;
+	char **tmp_attrs = NULL;
 
 	slapi_pblock_get(pb, SLAPI_ORIGINAL_TARGET_DN, (void *)&base );
 	slapi_pblock_get(pb, SLAPI_CONTROLS_ARG, &controls);
@@ -445,6 +455,9 @@ static int seq_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
 	slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &sdn);
 	slapi_sdn_free(&sdn);
 	slapi_pblock_set(pb, SLAPI_SEARCH_TARGET_SDN, NULL);
+	slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs);
+	slapi_ch_array_free(tmp_attrs);
+	slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, NULL);
 
 	return rc;
 }
@@ -731,6 +744,7 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
 	char					  *ifstr;
 	int						  opresult;
 	int						  rc = 0;
+	char **tmp_attrs = NULL;
 
 	PR_ASSERT (pb);
 
@@ -801,10 +815,13 @@ search_internal_callback_pb (Slapi_PBlock *pb, void *callback_data,
 
 done:
     slapi_ch_free((void **) & fstr);
-	if (filter != NULL) 
+    if (filter != NULL) 
     {
         slapi_filter_free(filter, 1 /* recurse */);
     }
+    slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &tmp_attrs);
+    slapi_ch_array_free(tmp_attrs);
+    slapi_pblock_set(pb, SLAPI_SEARCH_ATTRS, NULL);
 
     return(rc);
 }

+ 9 - 6
ldap/servers/slapd/schema.c

@@ -1381,15 +1381,18 @@ schema_list_attributes_callback(struct asyntaxinfo *asi, void *arg)
                 return ATTR_SYNTAX_ENUM_NEXT;
         }
         if (aew->flag && (asi->asi_flags & aew->flag)) {
-				charray_add(&aew->attrs, slapi_ch_strdup(asi->asi_name));
+           /* skip unhashed password */
+           if (!is_type_forbidden(asi->asi_name)) {
+                charray_add(&aew->attrs, slapi_ch_strdup(asi->asi_name));
                 if (NULL != asi->asi_aliases) {
-					int		i;
+                    int        i;
 
-					for ( i = 0; asi->asi_aliases[i] != NULL; ++i ) {
+                    for ( i = 0; asi->asi_aliases[i] != NULL; ++i ) {
                         charray_add(&aew->attrs,
-									slapi_ch_strdup(asi->asi_aliases[i]));
-					}
-				}
+                                    slapi_ch_strdup(asi->asi_aliases[i]));
+                    }
+                }
+            }
         }
         return ATTR_SYNTAX_ENUM_NEXT;
 }

+ 1 - 0
ldap/servers/slapd/slapi-private.h

@@ -331,6 +331,7 @@ int entry_next_deleted_attribute( const Slapi_Entry *e, Slapi_Attr **a);
 /* entry.c */
 int entry_apply_mods( Slapi_Entry *e, LDAPMod **mods );
 int is_type_protected(const char *type);
+int is_type_forbidden(const char *type);
 
 int slapi_entries_diff(Slapi_Entry **old_entries, Slapi_Entry **new_entries, int testall, const char *logging_prestr, const int force_update, void *plg_id);