Ticket #626 - Possible to add nonexistent target to ACI

Fix description: This patch checks if the acl target exists and logs
a warning in the server log if the target does not exist. It does
not, however, validate targets containing wildcards or macros

https://fedorahosted.org/389/ticket/626

Reviewed by rmeggins and nhosoi.

ldap/servers/plugins/acl/acl.c

@@ -1494,16 +1494,16 @@ acl_check_mods(
 				** syntax
 				*/
 				if (strcmp(mod->mod_type, 
-					   aci_attr_type) == 0) {
-					if ( 0 != (rv = acl_verify_syntax( e_sdn,
-						                        mod->mod_bvalues[i], errbuf))) {
+					aci_attr_type) == 0) {
+					if ( 0 != (rv = acl_verify_syntax(pb, e_sdn,
+						mod->mod_bvalues[i], errbuf))) {
 						aclutil_print_err(rv, e_sdn, 
 							mod->mod_bvalues[i],
-							errbuf);
+								errbuf);
 						/* Cleanup */
 						slapi_mods_done(&smods);
 						return LDAP_INVALID_SYNTAX;
-				   	}
+					}
 				}
 			} /* for */
 		}
@@ -1680,8 +1680,8 @@ acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change)
 			acllist_acicache_WRITE_LOCK();
 			i= slapi_attr_first_value ( attr,&sval );
 			while ( i != -1 ) {
-			        attrVal = slapi_value_get_berval(sval);
-				rv= acllist_insert_aci_needsLock(e_sdn, attrVal );
+				attrVal = slapi_value_get_berval(sval);
+				rv= acllist_insert_aci_needsLock_ext(pb, e_sdn, attrVal );
 				if (rv <= ACL_ERR) 
 					aclutil_print_err(rv, e_sdn, attrVal, NULL);
 				/* Print the aci list */
@@ -1722,10 +1722,10 @@ acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change)
 					if (bvalue == NULL)
 						break;
 					for (; *bvalue != NULL; ++bvalue) {
-						rv=acllist_insert_aci_needsLock( e_sdn, *bvalue);
+						rv=acllist_insert_aci_needsLock_ext(pb, e_sdn, *bvalue);
 						if (rv <= ACL_ERR) { 
-						    aclutil_print_err(rv, e_sdn,
-								   *bvalue, NULL);
+							aclutil_print_err(rv, e_sdn,
+								*bvalue, NULL);
 						}
 					}
 				} else {
@@ -1735,10 +1735,10 @@ acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change)
 					for (; *value != NULL; ++value) {
 						b.bv_len = strlen (*value);
 						b.bv_val = *value;
-						rv=acllist_insert_aci_needsLock( e_sdn, &b);
+						rv=acllist_insert_aci_needsLock_ext(pb, e_sdn, &b);
 						if (rv <= ACL_ERR) {
-						    aclutil_print_err(rv, e_sdn,
-								   &b, NULL);
+							aclutil_print_err(rv, e_sdn,
+								&b, NULL);
 						}
 					}
 				}

ldap/servers/plugins/acl/acl.h

@@ -808,14 +808,17 @@ int 		acl_access_allowed_main ( Slapi_PBlock *pb, Slapi_Entry *e, char **attrs,
                           		  struct berval *val, int access , int flags, char **errbuf);
 int 		acl_access_allowed( Slapi_PBlock *pb, Slapi_Entry *e, char *attr,
 				        	struct berval *val, int access );
-int 		acl_verify_syntax(const Slapi_DN *e_sdn, const struct berval *bval, char **errbuf);
 aclUserGroup * acl_get_usersGroup ( struct acl_pblock *aclpb , char *n_dn);
 void		acl_print_acllib_err (NSErr_t *errp , char * str);	
 int 		acl_check_mods( Slapi_PBlock *pb, Slapi_Entry *e, LDAPMod **mods, char **errbuf );
-int 		acl_verify_aci_syntax (Slapi_Entry *e, char **errbuf);
 char * 		acl__access2str(int access);
 void		acl_strcpy_special (char *d, char *s);
-int			acl_parse(char *str, aci_t *aci_item, char **errbuf);
+int		acl_parse(Slapi_PBlock *pb, char * str, aci_t *aci_item, char **errbuf);
+int		acl_verify_aci_syntax (Slapi_PBlock *pb, Slapi_Entry *e, char **errbuf);
+int		acl_verify_syntax(Slapi_PBlock *pb, const Slapi_DN *e_sdn,
+			const struct berval *bval, char **errbuf);
+int		acllist_insert_aci_needsLock_ext( Slapi_PBlock *pb, const Slapi_DN *e_sdn,
+			const struct berval* aci_attr);
 char *		acl_access2str ( int access );
 int 		acl_init_ext ();
 void * 		acl_get_ext (ext_type type, void *object);
@@ -864,7 +867,7 @@ void		acllist_acicache_WRITE_UNLOCK(void);
 void		acllist_acicache_WRITE_LOCK(void);
 void		acllist_aciscan_update_scan ( Acl_PBlock *aclpb, char *edn );
 int 		acllist_remove_aci_needsLock( const Slapi_DN *sdn,  const struct berval *attr );
-int 		acllist_insert_aci_needsLock( const Slapi_DN *e_sdn, const struct berval* aci_attr);
+int		acllist_insert_aci_needsLock( const Slapi_DN *e_sdn, const struct berval* aci_attr);
 int 		acllist_init ();
 int			acllist_moddn_aci_needsLock ( Slapi_DN *oldsdn, char *newdn );
 void		acllist_print_tree ( Avlnode *root, int *depth, char *start, char *side);

ldap/servers/plugins/acl/acllist.c

@@ -97,7 +97,6 @@ static void 	__acllist_free_aciContainer (  AciContainer **container);
 
 void my_print( Avlnode	*root );
 
-
 int
 acllist_init ()
 {
@@ -195,6 +194,12 @@ void acl_be_state_change_fnc ( void *handle, char *be_name, int old_state,
 /* This routine must be called with the acicache write lock taken */
 int
 acllist_insert_aci_needsLock( const Slapi_DN *e_sdn, const struct berval* aci_attr)
+{
+	return(acllist_insert_aci_needsLock_ext(NULL, e_sdn, aci_attr));
+}
+
+int
+acllist_insert_aci_needsLock_ext( Slapi_PBlock *pb, const Slapi_DN *e_sdn, const struct berval* aci_attr)
 {
 
 	aci_t			*aci;
@@ -209,7 +214,7 @@ acllist_insert_aci_needsLock( const Slapi_DN *e_sdn, const struct berval* aci_at
 
 	acl_str = slapi_ch_strdup(aci_attr->bv_val);
 	/* Parse the ACL TEXT */
-	if (  0 != (rv = acl_parse ( acl_str, aci, NULL )) ) {
+	if (  0 != (rv = acl_parse ( pb, acl_str, aci, NULL )) ) {
 		slapi_log_error (SLAPI_LOG_FATAL, plugin_name,
 				"ACL PARSE ERR(rv=%d): %s\n", rv, acl_str );
 		slapi_ch_free ( (void **) &acl_str );

ldap/servers/plugins/acl/aclparse.c

@@ -65,7 +65,6 @@ static int acl_verify_exactly_one_attribute( char *attr_name, Slapi_Filter *f);
 static int type_compare( Slapi_Filter *f, void *arg);
 static int acl_check_for_target_macro( aci_t *aci_item, char *value);
 static int get_acl_rights_as_int( char * strValue);
-
 /***************************************************************************
 *
 * acl_parse
@@ -75,6 +74,7 @@ static int get_acl_rights_as_int( char * strValue);
 *
 *
 * Input:
+* 	Slapi_PBlock	*pb	- Parameter block
 *	char	*str		- Input string which has the ACL
 *				  This is a duped copy, so here we have
 *				  the right to stich '\0' characters into str for
@@ -93,7 +93,7 @@ static int get_acl_rights_as_int( char * strValue);
 *
 **************************************************************************/
 int
-acl_parse(char * str, aci_t *aci_item, char **errbuf)
+acl_parse(Slapi_PBlock *pb, char * str, aci_t *aci_item, char **errbuf)
 {
 
 	int  		rv=0;
@@ -110,10 +110,10 @@ acl_parse(char * str, aci_t *aci_item, char **errbuf)
 			}
 		} else if (!next) {
 			/* the statement does not start with a parenthesis */
-                  	return(ACL_SYNTAX_ERR);
-                } else {
+			return(ACL_SYNTAX_ERR);
+		} else {
 			/* then we have done all the processing */
-		  	return  0;
+			return  0;
 		}
 		LDAP_UTF8INC(str);	/* skip the "(" */
 		save = next;
@@ -146,14 +146,50 @@ acl_parse(char * str, aci_t *aci_item, char **errbuf)
 		if (aci_item->aci_type & ACI_TARGET_DN) {
 			char           *avaType;
 			struct berval   *avaValue;
-			const char      *dn;
 
-			dn = slapi_sdn_get_ndn(aci_item->aci_sdn);
-			slapi_filter_get_ava(f, &avaType, &avaValue);
+			Slapi_DN *targdn = slapi_sdn_new();
+			slapi_filter_get_ava ( f, &avaType, &avaValue );
+			slapi_sdn_init_dn_byref(targdn, avaValue->bv_val);
 
-			if (!slapi_dn_issuffix(avaValue->bv_val, dn)) {
+			if (!slapi_sdn_get_dn(targdn)) {
+				/* not a valid DN */
+				slapi_sdn_free(&targdn);
 				return ACL_INVALID_TARGET;
 			}
+
+			if (!slapi_sdn_issuffix(targdn, aci_item->aci_sdn)) {
+				slapi_sdn_free(&targdn);
+				return ACL_INVALID_TARGET;
+			}
+
+			if (slapi_sdn_compare(targdn, aci_item->aci_sdn)) {
+				int target_check = 0;
+				if (pb) {
+					slapi_pblock_get(pb, SLAPI_ACI_TARGET_CHECK, &target_check);
+				}
+				if (target_check != 1) {
+					/* Make sure that the target exists */
+					int rc = 0;
+					Slapi_PBlock *temppb = slapi_pblock_new();
+					slapi_search_internal_set_pb_ext(temppb, targdn,
+						LDAP_SCOPE_BASE, "(objectclass=*)", NULL, 1, NULL, NULL,
+							(void *)plugin_get_default_component_id(), 0);
+					slapi_search_internal_pb(temppb);
+					slapi_pblock_get(temppb, SLAPI_PLUGIN_INTOP_RESULT, &rc);
+					if (rc != LDAP_SUCCESS) {
+						slapi_log_error(SLAPI_LOG_FATAL, plugin_name,
+							"The ACL target %s does not exist\n", slapi_sdn_get_dn(targdn));
+					}
+	
+					slapi_free_search_results_internal(temppb);
+					slapi_pblock_destroy(temppb);
+					if (pb) {
+						target_check = 1;
+						slapi_pblock_set(pb, SLAPI_ACI_TARGET_CHECK, &target_check);
+					}
+				}
+			}
+			slapi_sdn_free(&targdn);
 		}
 	}
 
@@ -164,8 +200,8 @@ acl_parse(char * str, aci_t *aci_item, char **errbuf)
 	** 
 	*/
 	if ((aci_item->aci_elevel != ACI_ELEVEL_USERDN_ANYONE) &&
-	    !(aci_item->aci_type & ACI_TARGET_MACRO_DN)) {
-		slapi_ch_free((void **)&aci_item->targetFilterStr);
+		!(aci_item->aci_type & ACI_TARGET_MACRO_DN)) {
+			slapi_ch_free((void **)&aci_item->targetFilterStr);
 	}
 
 	/*
@@ -1550,6 +1586,7 @@ acl_strcpy_special (char *d, char *s)
 *    verify if the aci's being added for the entry has a valid syntax or not.
 *
 * Input:
+*	Slapi_PBlock	*pb	- Parameter block
 *	Slapi_Entry	*e		- The Slapi_Entry itself
 *	char	**errbuf;	-- error message
 *
@@ -1562,7 +1599,7 @@ acl_strcpy_special (char *d, char *s)
 *
 **************************************************************************/
 int
-acl_verify_aci_syntax (Slapi_Entry *e, char **errbuf)
+acl_verify_aci_syntax (Slapi_PBlock *pb, Slapi_Entry *e, char **errbuf)
 {
 
 	if (e != NULL) {
@@ -1580,8 +1617,8 @@ acl_verify_aci_syntax (Slapi_Entry *e, char **errbuf)
 
 		i= slapi_attr_first_value ( attr,&sval );
 		while ( i != -1 ) {
-		        attrVal = slapi_value_get_berval ( sval );
-		        rv = acl_verify_syntax( e_sdn, attrVal, errbuf );
+			attrVal = slapi_value_get_berval ( sval );
+			rv = acl_verify_syntax( pb, e_sdn, attrVal, errbuf );
 			if ( 0 != rv ) {
 				aclutil_print_err(rv, e_sdn, attrVal, errbuf);
 				return ACL_ERR;
@@ -1598,6 +1635,7 @@ acl_verify_aci_syntax (Slapi_Entry *e, char **errbuf)
 *	added/replaced has the right syntax or not.
 *
 * Input:
+*	Slapi_PBlock	*pb	- Parameter block
 *	Slapi_DN	*e_sdn	- sdn of the entry
 *	berval	 *bval		- The berval containg the aci value
 *
@@ -1608,19 +1646,20 @@ acl_verify_aci_syntax (Slapi_Entry *e, char **errbuf)
 *	None.
 *
 **************************************************************************/
+
 int
-acl_verify_syntax(const Slapi_DN *e_sdn, 
-                  const struct berval *bval, char **errbuf)
+acl_verify_syntax(Slapi_PBlock *pb, const Slapi_DN *e_sdn,
+	const struct berval *bval, char **errbuf)
 {
 	aci_t			*aci_item;
-	int				rv = 0;
+	int			rv = 0;
 	char			*str;
 	aci_item = acllist_get_aci_new ();
 	slapi_sdn_set_ndn_byval ( aci_item->aci_sdn, slapi_sdn_get_ndn ( e_sdn ) );
 
 	/* make a copy the the string */
-	str =  slapi_ch_strdup(bval->bv_val);
-	rv = acl_parse(str, aci_item, errbuf);
+	str = slapi_ch_strdup(bval->bv_val);
+	rv = acl_parse(pb, str, aci_item, errbuf);
 
 	/* cleanup before you leave ... */
 	acllist_free_aci (aci_item);

ldap/servers/slapd/pblock.c

@@ -1953,6 +1953,12 @@ slapi_pblock_get( Slapi_PBlock *pblock, int arg, void *value )
 			(*(int *)value) = -1;
 		}
 		break;
+
+	/* ACI Target Check */	
+	case SLAPI_ACI_TARGET_CHECK:
+		(*(int *)value) = pblock->pb_aci_target_check;
+		break;
+
 	default:
 		LDAPDebug( LDAP_DEBUG_ANY,
 		    "Unknown parameter block argument %d\n", arg, 0, 0 );
@@ -3526,6 +3532,11 @@ slapi_pblock_set( Slapi_PBlock *pblock, int arg, void *value )
 		pblock->pb_paged_results_index = *(int *)value;
 		break;
 
+	/* ACI Target Check */
+	case SLAPI_ACI_TARGET_CHECK:
+		pblock->pb_aci_target_check = *((int *) value);
+		break;
+
 	default:
 		LDAPDebug( LDAP_DEBUG_ANY,
 		    "Unknown parameter block argument %d\n", arg, 0, 0 );

ldap/servers/slapd/plugin_acl.c

@@ -217,7 +217,7 @@ plugin_call_acl_verify_syntax ( Slapi_PBlock *pb, Slapi_Entry *e, char **errbuf
 		if (plugin_invoke_plugin_sdn (p, SLAPI_PLUGIN_ACL_SYNTAX_CHECK, pb, 
 									  (Slapi_DN*)slapi_entry_get_sdn_const (e))){
 			plugin_called = 1;
-			rc = (*p->plg_acl_syntax_check)( e, errbuf );
+			rc = (*p->plg_acl_syntax_check)( pb, e, errbuf );
 			if ( rc != LDAP_SUCCESS ) break;
 		}
 	}

ldap/servers/slapd/slap.h

@@ -1745,6 +1745,10 @@ typedef struct slapi_pblock {
 	int	pb_paged_results_index;    /* stash SLAPI_PAGED_RESULTS_INDEX */
 	passwdPolicy *pwdpolicy;
 	void *op_stack_elem;
+
+	/* For ACI Target Check */
+	int pb_aci_target_check; /* this flag prevents duplicate checking of ACI's target existence */
+
 } slapi_pblock;
 
 /* index if substrlens */

ldap/servers/slapd/slapi-plugin.h

@@ -6989,6 +6989,9 @@ typedef struct slapi_plugindesc {
 /* Simple paged results index */
 #define SLAPI_PAGED_RESULTS_INDEX   1945
 
+/* ACI Target Check */
+#define SLAPI_ACI_TARGET_CHECK      1946
+
 /* convenience macros for checking modify operation types */
 #define SLAPI_IS_MOD_ADD(x) (((x) & ~LDAP_MOD_BVALUES) == LDAP_MOD_ADD)
 #define SLAPI_IS_MOD_DELETE(x) (((x) & ~LDAP_MOD_BVALUES) == LDAP_MOD_DELETE)