[389-ds-base/el5] fix modrdn aci security problem
Richard Allen Megginson
rmeggins at fedoraproject.org
Tue Oct 16 16:15:37 UTC 2012
commit da7853ec028ce8caa1e4ef0cb84238b80fb51c5c
Author: Rich Megginson <rmeggins at redhat.com>
Date: Tue Oct 16 10:13:51 2012 -0600
fix modrdn aci security problem
Trac Ticket #340 - Change on SLAPI_MODRDN_NEWSUPERIOR is not evaluated in acl
...-340-Change-on-SLAPI_MODRDN_NEWSUPERIOR-i.patch | 380 ++++++++++++++++++++
389-ds-base.spec | 7 +-
2 files changed, 386 insertions(+), 1 deletions(-)
---
diff --git a/0002-Trac-Ticket-340-Change-on-SLAPI_MODRDN_NEWSUPERIOR-i.patch b/0002-Trac-Ticket-340-Change-on-SLAPI_MODRDN_NEWSUPERIOR-i.patch
new file mode 100644
index 0000000..377eeab
--- /dev/null
+++ b/0002-Trac-Ticket-340-Change-on-SLAPI_MODRDN_NEWSUPERIOR-i.patch
@@ -0,0 +1,380 @@
+From 77b673974892eaa1f026687f6cf683372cc45c8c Mon Sep 17 00:00:00 2001
+From: Noriko Hosoi <nhosoi at totoro.usersys.redhat.com>
+Date: Fri, 21 Sep 2012 12:35:18 -0700
+Subject: [PATCH 2/3] Trac Ticket #340 - Change on SLAPI_MODRDN_NEWSUPERIOR is not
+ evaluated in acl
+
+https://fedorahosted.org/389/ticket/340
+
+Bug Description: When modrdn operation was executed, only newrdn
+change was passed to the acl plugin. Also, the change was used
+only for the acl search, but not for the acl target in the items
+in the acl cache.
+
+Fix Description: This patch also passes the newsuperior update
+to the acl plugin. And the modrdn updates are applied to the
+acl target in the acl cache.
+(cherry picked from commit 5beb93d42efb807838c09c5fab898876876f8d09)
+1.2.10 - cherry-picked from 7399cbd53d6289df592d3414a84972eacb4dc97d
+- had to fix a couple of conflicts in the acl code
+---
+ ldap/servers/plugins/acl/acl.c | 77 ++++++++++++++++++++++------------
+ ldap/servers/plugins/acl/acl.h | 5 +-
+ ldap/servers/plugins/acl/aclgroup.c | 2 +-
+ ldap/servers/plugins/acl/acllist.c | 48 +++++++++++++---------
+ ldap/servers/slapd/dn.c | 2 +-
+ ldap/servers/slapd/plugin_acl.c | 35 ++++++++++++----
+ 6 files changed, 110 insertions(+), 59 deletions(-)
+
+diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c
+index 5a7b446..e17a827 100644
+--- a/ldap/servers/plugins/acl/acl.c
++++ b/ldap/servers/plugins/acl/acl.c
+@@ -170,9 +170,9 @@ acl_access_allowed_modrdn(
+ * Test if have access to make the first rdn of dn in entry e.
+ */
+
+-static int check_rdn_access( Slapi_PBlock *pb, Slapi_Entry *e, const char *dn,
+- int access) {
+-
++static int
++check_rdn_access( Slapi_PBlock *pb, Slapi_Entry *e, const char *dn, int access)
++{
+ char **dns;
+ char **rdns;
+ int retCode = LDAP_INSUFFICIENT_ACCESS;
+@@ -659,7 +659,8 @@ cleanup_and_ret:
+
+ }
+
+-static void print_access_control_summary( char *source, int ret_val, char *clientDn,
++static void
++print_access_control_summary( char *source, int ret_val, char *clientDn,
+ struct acl_pblock *aclpb,
+ char *right,
+ char *attr,
+@@ -1533,11 +1534,12 @@ acl_check_mods(
+ *
+ **************************************************************************/
+ extern void
+-acl_modified (Slapi_PBlock *pb, int optype, char *n_dn, void *change)
++acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change)
+ {
+ struct berval **bvalue;
+ char **value;
+ int rv=0; /* returned value */
++ const char* n_dn;
+ char* new_RDN;
+ char* parent_DN;
+ char* new_DN;
+@@ -1547,10 +1549,12 @@ acl_modified (Slapi_PBlock *pb, int optype, char *n_dn, void *change)
+ Slapi_Attr *attr = NULL;
+ Slapi_Entry *e = NULL;
+ char ebuf [ BUFSIZ];
+- Slapi_DN *e_sdn;
+ aclUserGroup *ugroup = NULL;
+
+- e_sdn = slapi_sdn_new_normdn_byval ( n_dn );
++ if (NULL == e_sdn) {
++ return;
++ }
++ n_dn = slapi_sdn_get_dn(e_sdn);
+ /* Before we proceed, Let's first check if we are changing any groups.
+ ** If we are, then we need to change the signature
+ */
+@@ -1778,45 +1782,64 @@ acl_modified (Slapi_PBlock *pb, int optype, char *n_dn, void *change)
+ }
+
+ break;
+- }/* case op is modify*/
++ }/* case op is modify*/
+
+- case SLAPI_OPERATION_MODRDN:
+-
+- new_RDN = (char*) change;
+- slapi_log_error (SLAPI_LOG_ACL, plugin_name,
+- "acl_modified (MODRDN %s => \"%s\"\n",
+- ACL_ESCAPE_STRING_WITH_PUNCTUATION (n_dn, ebuf), new_RDN);
++ case SLAPI_OPERATION_MODRDN:
++ {
++ char **rdn_parent;
++ rdn_parent = (char **)change;
++ new_RDN = rdn_parent[0];
++ parent_DN = rdn_parent[1];
+
+ /* compute new_DN: */
+- parent_DN = slapi_dn_parent (n_dn);
+- if (parent_DN == NULL) {
+- new_DN = new_RDN;
++ if (NULL == parent_DN) {
++ parent_DN = slapi_dn_parent(n_dn);
++ }
++ if (NULL == parent_DN) {
++ if (NULL == new_RDN) {
++ slapi_log_error (SLAPI_LOG_ACL, plugin_name,
++ "acl_modified (MODRDN %s => \"no change\"\n",
++ n_dn);
++ break;
++ } else {
++ new_DN = new_RDN;
++ }
+ } else {
+- new_DN = slapi_create_dn_string("%s,%s", new_RDN, parent_DN);
++ if (NULL == new_RDN) {
++ Slapi_RDN *rdn= slapi_rdn_new();
++ slapi_sdn_get_rdn(e_sdn, rdn);
++ new_DN = slapi_create_dn_string("%s,%s", slapi_rdn_get_rdn(rdn),
++ parent_DN);
++ slapi_rdn_free(&rdn);
++ } else {
++ new_DN = slapi_create_dn_string("%s,%s", new_RDN, parent_DN);
++ }
+ }
++ slapi_log_error (SLAPI_LOG_ACL, plugin_name,
++ "acl_modified (MODRDN %s => \"%s\"\n", n_dn, new_RDN);
+
+ /* Change the acls */
+- acllist_acicache_WRITE_LOCK();
++ acllist_acicache_WRITE_LOCK();
+ /* acllist_moddn_aci_needsLock expects normalized new_DN,
+ * which is no need to be case-ignored */
+ acllist_moddn_aci_needsLock ( e_sdn, new_DN );
+ acllist_acicache_WRITE_UNLOCK();
+
+ /* deallocat the parent_DN */
+- if (parent_DN != NULL) {
+- slapi_ch_free ( (void **) &new_DN );
+- slapi_ch_free ( (void **) &parent_DN );
++ if (parent_DN != NULL) {
++ slapi_ch_free_string(&new_DN);
++ if (parent_DN != rdn_parent[1]) {
++ slapi_ch_free_string(&parent_DN);
++ }
+ }
+ break;
+-
+- default:
++ } /* case op is modrdn */
++ default:
+ /* print ERROR */
+ break;
+ } /*optype switch */
+-
+- slapi_sdn_free ( &e_sdn );
+-
+ }
++
+ /***************************************************************************
+ *
+ * acl__scan_for_acis
+diff --git a/ldap/servers/plugins/acl/acl.h b/ldap/servers/plugins/acl/acl.h
+index 4fa3e3f..28c38e7 100644
+--- a/ldap/servers/plugins/acl/acl.h
++++ b/ldap/servers/plugins/acl/acl.h
+@@ -796,7 +796,8 @@ int acl_read_access_allowed_on_attr ( Slapi_PBlock *pb, Slapi_Entry *e, char
+ struct berval *val, int access);
+ void acl_set_acllist (Slapi_PBlock *pb, int scope, char *base);
+ void acl_gen_err_msg(int access, char *edn, char *attr, char **errbuf);
+-void acl_modified ( Slapi_PBlock *pb, int optype, char *dn, void *change);
++void acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change);
++
+ int acl_access_allowed_disjoint_resource( Slapi_PBlock *pb, Slapi_Entry *e,
+ char *attr, struct berval *val, int access );
+ int acl_access_allowed_main ( Slapi_PBlock *pb, Slapi_Entry *e, char **attrs,
+@@ -866,7 +867,7 @@ void acllist_print_tree ( Avlnode *root, int *depth, char *start, char *side);
+ AciContainer *acllist_get_aciContainer_new ( );
+ void acllist_done_aciContainer ( AciContainer *);
+
+-aclUserGroup* aclg_find_userGroup (char *n_dn);
++aclUserGroup* aclg_find_userGroup (const char *n_dn);
+ void aclg_regen_ugroup_signature( aclUserGroup *ugroup);
+ void aclg_markUgroupForRemoval ( aclUserGroup *u_group );
+ void aclg_reader_incr_ugroup_refcnt(aclUserGroup* u_group);
+diff --git a/ldap/servers/plugins/acl/aclgroup.c b/ldap/servers/plugins/acl/aclgroup.c
+index c694293..2231304 100644
+--- a/ldap/servers/plugins/acl/aclgroup.c
++++ b/ldap/servers/plugins/acl/aclgroup.c
+@@ -213,7 +213,7 @@ aclg_reset_userGroup ( struct acl_pblock *aclpb )
+ */
+
+ aclUserGroup*
+-aclg_find_userGroup(char *n_dn)
++aclg_find_userGroup(const char *n_dn)
+ {
+ aclUserGroup *u_group = NULL;
+ int i;
+diff --git a/ldap/servers/plugins/acl/acllist.c b/ldap/servers/plugins/acl/acllist.c
+index 9b5363a..e8198af 100644
+--- a/ldap/servers/plugins/acl/acllist.c
++++ b/ldap/servers/plugins/acl/acllist.c
+@@ -600,7 +600,6 @@ void
+ acllist_init_scan (Slapi_PBlock *pb, int scope, const char *base)
+ {
+ Acl_PBlock *aclpb;
+- int i;
+ AciContainer *root;
+ char *basedn = NULL;
+ int index;
+@@ -671,11 +670,6 @@ acllist_init_scan (Slapi_PBlock *pb, int scope, const char *base)
+ aclpb->aclpb_state &= ~ACLPB_SEARCH_BASED_ON_LIST ;
+
+ acllist_acicache_READ_UNLOCK();
+-
+- i = 0;
+- while ( i < aclpb_max_selected_acls && aclpb->aclpb_base_handles_index[i] != -1 ) {
+- i++;
+- }
+ }
+
+ /*
+@@ -893,34 +887,50 @@ acllist_acicache_WRITE_LOCK( )
+ int
+ acllist_moddn_aci_needsLock ( Slapi_DN *oldsdn, char *newdn )
+ {
+-
+-
+ AciContainer *aciListHead;
+ AciContainer *head;
++ aci_t *acip;
++ const char *oldndn;
+
+ /* first get the container */
+
+ aciListHead = acllist_get_aciContainer_new ( );
+ slapi_sdn_free(&aciListHead->acic_sdn);
+- aciListHead->acic_sdn = oldsdn;
+-
++ aciListHead->acic_sdn = oldsdn;
+
+ if ( NULL == (head = (AciContainer *) avl_find( acllistRoot, aciListHead,
+- (IFP) __acllist_aciContainer_node_cmp ) ) ) {
++ (IFP) __acllist_aciContainer_node_cmp ) ) ) {
+
+ slapi_log_error ( SLAPI_PLUGIN_ACL, plugin_name,
+- "Can't find the acl in the tree for moddn operation:olddn%s\n",
+- slapi_sdn_get_ndn ( oldsdn ));
++ "Can't find the acl in the tree for moddn operation:olddn%s\n",
++ slapi_sdn_get_ndn ( oldsdn ));
+ aciListHead->acic_sdn = NULL;
+ __acllist_free_aciContainer ( &aciListHead );
+- return 1;
++ return 1;
+ }
+
+-
+- /* Now set the new DN */
+- slapi_sdn_done ( head->acic_sdn );
+- slapi_sdn_set_normdn_byval ( head->acic_sdn, newdn );
+-
++ /* Now set the new DN */
++ slapi_sdn_set_normdn_byval(head->acic_sdn, newdn);
++
++ /* If necessary, reset the target DNs, as well. */
++ oldndn = slapi_sdn_get_ndn(oldsdn);
++ for (acip = head->acic_list; acip; acip = acip->aci_next) {
++ const char *ndn = slapi_sdn_get_ndn(acip->aci_sdn);
++ char *p = PL_strstr(ndn, oldndn);
++ if (p) {
++ if (p == ndn) {
++ /* target dn is identical, replace it with new DN*/
++ slapi_sdn_set_normdn_byval(acip->aci_sdn, newdn);
++ } else {
++ /* target dn is a descendent of olddn, merge it with new DN*/
++ char *mynewdn;
++ *p = '\0';
++ mynewdn = slapi_ch_smprintf("%s%s", ndn, newdn);
++ slapi_sdn_set_normdn_passin(acip->aci_sdn, mynewdn);
++ }
++ }
++ }
++
+ aciListHead->acic_sdn = NULL;
+ __acllist_free_aciContainer ( &aciListHead );
+
+diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c
+index 568871f..35c0700 100644
+--- a/ldap/servers/slapd/dn.c
++++ b/ldap/servers/slapd/dn.c
+@@ -2037,7 +2037,7 @@ slapi_sdn_set_normdn_byval(Slapi_DN *sdn, const char *normdn)
+ slapi_sdn_done(sdn);
+ sdn->flag = slapi_setbit_uchar(sdn->flag, FLAG_DN);
+ if(normdn == NULL) {
+- sdn->dn = slapi_ch_strdup(normdn);
++ sdn->dn = NULL;
+ sdn->ndn_len = 0;
+ } else {
+ sdn->dn = slapi_ch_strdup(normdn);
+diff --git a/ldap/servers/slapd/plugin_acl.c b/ldap/servers/slapd/plugin_acl.c
+index 24dcc76..3bc3f21 100644
+--- a/ldap/servers/slapd/plugin_acl.c
++++ b/ldap/servers/slapd/plugin_acl.c
+@@ -134,11 +134,10 @@ int
+ plugin_call_acl_mods_update ( Slapi_PBlock *pb, int optype )
+ {
+ struct slapdplugin *p;
+- char *dn;
+ int rc = 0;
+- void *change = NULL;
+- Slapi_Entry *te = NULL;
+- Slapi_DN *sdn = NULL;
++ void *change = NULL;
++ Slapi_Entry *te = NULL;
++ Slapi_DN *sdn = NULL;
+ Operation *operation;
+
+ slapi_pblock_get (pb, SLAPI_OPERATION, &operation);
+@@ -146,7 +145,7 @@ plugin_call_acl_mods_update ( Slapi_PBlock *pb, int optype )
+ (void)slapi_pblock_get( pb, SLAPI_TARGET_SDN, &sdn );
+
+ switch ( optype ) {
+- case SLAPI_OPERATION_MODIFY:
++ case SLAPI_OPERATION_MODIFY:
+ (void)slapi_pblock_get( pb, SLAPI_MODIFY_MODS, &change );
+ break;
+ case SLAPI_OPERATION_ADD:
+@@ -157,9 +156,28 @@ plugin_call_acl_mods_update ( Slapi_PBlock *pb, int optype )
+ sdn = slapi_entry_get_sdn(te);
+ }
+ break;
+- case SLAPI_OPERATION_MODRDN:
+- (void)slapi_pblock_get( pb, SLAPI_MODRDN_NEWRDN, &change );
++ case SLAPI_OPERATION_MODRDN:
++ {
++ void *mychange[2];
++ char *newrdn = NULL;
++ Slapi_DN *psdn = NULL;
++ char *pdn = NULL;
++
++ /* newrdn: "change" is normalized but not case-ignored */
++ /* The acl plugin expects normalized newrdn, but no need to be case-
++ * ignored. */
++ (void)slapi_pblock_get( pb, SLAPI_MODRDN_NEWRDN, &newrdn );
++ (void)slapi_pblock_get( pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &psdn );
++ if (psdn) {
++ pdn = (char *)slapi_sdn_get_dn(psdn);
++ } else {
++ (void)slapi_pblock_get( pb, SLAPI_MODRDN_NEWSUPERIOR, &pdn );
++ }
++ mychange[0] = newrdn;
++ mychange[1] = pdn;
++ change = mychange;
+ break;
++ }
+ }
+
+ if (NULL == sdn) {
+@@ -169,10 +187,9 @@ plugin_call_acl_mods_update ( Slapi_PBlock *pb, int optype )
+ }
+
+ /* call the global plugins first and then the backend specific */
+- dn = (char*)slapi_sdn_get_ndn(sdn); /* jcm - Had to cast away const */
+ for ( p = get_plugin_list(PLUGIN_LIST_ACL); p != NULL; p = p->plg_next ) {
+ if (plugin_invoke_plugin_sdn(p, SLAPI_PLUGIN_ACL_MODS_UPDATE, pb, sdn)){
+- rc = (*p->plg_acl_mods_update)(pb, optype, dn, change );
++ rc = (*p->plg_acl_mods_update)(pb, optype, sdn, change );
+ if ( rc != LDAP_SUCCESS ) break;
+ }
+ }
+--
+1.7.1
+
diff --git a/389-ds-base.spec b/389-ds-base.spec
index da5e57d..44c64bb 100644
--- a/389-ds-base.spec
+++ b/389-ds-base.spec
@@ -18,7 +18,7 @@
Summary: 389 Directory Server (base)
Name: 389-ds-base
Version: 1.2.10.14
-Release: %{?relprefix}1%{?prerel}%{?dist}
+Release: %{?relprefix}2%{?prerel}%{?dist}
License: GPLv2 with exceptions
URL: http://port389.org/
Group: System Environment/Daemons
@@ -104,6 +104,7 @@ Source0: http://port389.org/sources/%{name}-%{version}%{?prerel}.tar.bz
Source1: %{name}-git.sh
Source2: %{name}-devel.README
Patch0: 0001-Ticket-331-transaction-errors-with-db-4.3-and-db-4.2.patch
+Patch1: 0002-Trac-Ticket-340-Change-on-SLAPI_MODRDN_NEWSUPERIOR-i.patch
%description
389 Directory Server is an LDAPv3 compliant server. The base package includes
@@ -161,6 +162,7 @@ SELinux policy interface for the 389 Directory Server base package.
%setup -q -n %{name}-%{version}%{?prerel}
cp %{SOURCE2} README.devel
%patch0 -p1
+%patch1 -p1
%build
%if %{use_openldap}
@@ -350,6 +352,9 @@ exit 0
%{_libdir}/%{pkgname}/libslapd.so.*
%changelog
+* Tue Oct 16 2012 Rich Megginson <rmeggins at redhat.com> - 1.2.10.14-2
+- Trac Ticket #340 - Change on SLAPI_MODRDN_NEWSUPERIOR is not evaluated in acl
+
* Wed Jul 18 2012 Rich Megginson <rmeggins at redhat.com> - 1.2.10.14-1
- Ticket #410 - Referential integrity plug-in does not work when update interval is not zero
More information about the scm-commits
mailing list