Bug 602456

Noriko Hosoi wrote:
Basically, config params are supposed not to allow to add and delete.  The check is done at the line 399 and 400.  Endi ran into this problem when he was trying to delete nsslapd-listenhost.  But he did not have a problem to add it on-line (I guess).  The reason why he could add it was LDAP_MOD_ADD was not "set".  Well, it cannot be set, actually.  The macro is defined to '0', which is true in the mozldap header and the openldap header...

Should the DS be strict not to allow to add config params, as well?  I'm afraid we may hear complaints from customers: "We could do this before, but suddenly it's rejected with LDAP_UNWILLING_TO_PERFORM!!!"...

Rich Megginson wrote:
We should use the macros that Nathan added for this - SLAPI_IS_MOD_ADD() - see slapi-plugin.h

This is tough - yes, we should not use MOD_ADD for replace, but like you say, someone (including us - the console) may expect to be able to do this.  So I think we're just going to have to allow it, and log it. 

373 int
374 modify_config_dse(Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e    , int *returncode, char *returntext, void *arg)
375 {
            [...]
399             if ((mods[i]->mod_op & LDAP_MOD_DELETE) ||
400                 (mods[i]->mod_op & LDAP_MOD_ADD)) {
401                 rc= LDAP_UNWILLING_TO_PERFORM;

/usr/include/ldap.h:#define LDAP_MOD_ADD        (0x0000)
/usr/include/ldap.h:#define LDAP_MOD_DELETE        (0x0001)
/usr/include/ldap.h:#define LDAP_MOD_REPLACE    (0x0002)
/usr/include/ldap.h:#define LDAP_MOD_INCREMENT    (0x0003) /* OpenLDAP extension */
/usr/include/ldap.h:#define LDAP_MOD_BVALUES    (0x0080)

/usr/include/mozldap/ldap-standard.h:#define LDAP_MOD_ADD            0x00
/usr/include/mozldap/ldap-standard.h:#define LDAP_MOD_DELETE         0x01
/usr/include/mozldap/ldap-standard.h:#define LDAP_MOD_REPLACE        0x02
/usr/include/mozldap/ldap-standard.h:#define LDAP_MOD_BVALUES        0x80

Created attachment 453046 [details]
git patch file (master)

Description:
1. Originally, configuration attributes are designed not to allow
adding or deleting, but just replacing.  Due to the defect in
checking the add operation, add (LDAP_MOD_ADD) is not rejected.
Instead of fixing the add operation check to disallow the add,
log the operation in the error log.
2. Deleting configuration attributes is rejected with
LDAP_UNWILLING_TO_PERFORM.  We have a request that some attributes
need to be allowed to delete.  This patch introduces a config
attribute nsslapd-allowed-to-delete-attrs, which value is
configuration attributes separated by a space ' '.  If an
attribute is in the list, the attribute is allowed to delete.

Files:
 ldap/servers/slapd/configdse.c
 ldap/servers/slapd/libglobs.c
 ldap/servers/slapd/proto-slap.h
 ldap/servers/slapd/slap.h

(In reply to comment #0)
> Certain attributes should be allowed to be deleted from cn=config.

Note: nsslapd-listenhost value is evaluated only at the server's startup time.  Even if the attribute is deleted, the server needs to be restarted to take effect.

Comment on attachment 453046 [details]
git patch file (master)

Found a memory leak.

Created attachment 453248 [details]
git patch file (master)

Description:
1. Originally, configuration attributes are designed not to allow
adding or deleting, but to allow just replacing.  Due to a defect
in checking the add operation, adding (LDAP_MOD_ADD) is not rejected.
Instead of fixing the add checking to disallow adding, this patch
logs the operation in the error log.
2. On the other hand, deleting configuration attributes is rejected
by LDAP_UNWILLING_TO_PERFORM.  We have a request that some attributes
need to allow to delete.  This patch introduces a config attribute
nsslapd-allowed-to-delete-attrs, which value is configuration
attributes separated by a space ' '.  If an attribute is in the list,
the attribute is allowed to delete.  The delete operation is also
logged in the error log.

Files:
 ldap/servers/slapd/configdse.c
 ldap/servers/slapd/libglobs.c
 ldap/servers/slapd/proto-slap.h
 ldap/servers/slapd/slap.h

Steps to verify:
* nsslapd-allowed-to-delete-attrs is not defined in the config file dse.ldif, by default.
* The default value of nsslapd-allowed-to-delete-attrs is "nsslapd-listenhost".
* add nsslapd-listenhost
  ldapmodify ...
  dn: cn=config
  changetype: modify
  add: nsslapd-listenhost
  nsslapd-listenhost: <your FQDN>
* restart the server
  error log:
  slapd started.  Listening on <you IP addr> port <your port> for LDAP requests
* delete nsslapd-listenhost
  ldapmodify ...
  dn: cn=config
  changetype: modify
  delete: nsslapd-listenhost
  <== This operation should be successful.
* delete other attr, e.g., nsslapd-rewrite-rfc1274
  ldapmodify ...
  dn: cn=config
  changetype: modify
  delete: nsslapd-rewrite-rfc1274
  <== This operation should fail
  ldap_modify: DSA is unwilling to perform
  ldap_modify: additional info: Deleting attributes is not allowed
* to add nsslapd-rewrite-rfc1274 to the delete-attrs list, replace the
  value of nsslapd-allowed-to-delete-attrs as follows:
  ldapmodify ...
  dn: cn=config
  changetype: modify
  replace: nsslapd-allowed-to-delete-attrs
  nsslapd-allowed-to-delete-attrs: nsslapd-listenhost nsslapd-rewrite-rfc1274
* delete nsslapd-rewrite-rfc1274
  ldapmodify ...
  dn: cn=config
  changetype: modify
  delete: nsslapd-rewrite-rfc1274
  <== This operation should be successful.
* add nsslapd-rewrite-rfc1274 back
  ldapmodify ...
  dn: cn=config
  changetype: modify
  add: nsslapd-rewrite-rfc1274
  nsslapd-rewrite-rfc1274: off
* check the error log:
   Warning: Adding configuration attribute "nsslapd-listenhost"
   Warning: Deleting configuration attribute "nsslapd-listenhost"
   Warning: Deleting configuration attribute "nsslapd-rewrite-rfc1274"
   Warning: Adding configuration attribute "nsslapd-rewrite-rfc1274"

Comment on attachment 453248 [details]
git patch file (master)

should add nsslapd-securelistenhost to the list of attributes allowed to be deleted

Also, MOD_ADD should be able to add multi-valued attributes.  I don't know if we have any?

(In reply to comment #8)
> Comment on attachment 453248 [details]
> git patch file (master)
> 
> should add nsslapd-securelistenhost to the list of attributes allowed to be
> deleted

A good catch!  I'm adding it to the default nsslapd-allowed-to-delete-attrs value.

> Also, MOD_ADD should be able to add multi-valued attributes.  I don't know if
> we have any?

In cn=config?  I see objectClass is, but nothing else...

(In reply to comment #9)
> (In reply to comment #8)
> > Comment on attachment 453248 [details] [details]
> > git patch file (master)
> > 
> > should add nsslapd-securelistenhost to the list of attributes allowed to be
> > deleted
> 
> A good catch!  I'm adding it to the default nsslapd-allowed-to-delete-attrs
> value.
> 
> > Also, MOD_ADD should be able to add multi-valued attributes.  I don't know if
> > we have any?
> 
> In cn=config?  I see objectClass is, but nothing else...

aci is - but I think that is not handled by the config_set() code.

just looked at the CONFIG_CHARRAY attrs in libglobs.c - the only ones listed are read-only, so we're safe there.

Created attachment 453261 [details]
git patch file (master)


Following the suggestion by Rich, adding "nsslapd-securelistenhost" to the default nsslapd-allowed-to-delete-attrs list.

diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index 6b58dde..a7cc1bc 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/ldap/servers/slapd/libglobs.c
@@ -1013,6 +1013,8 @@ FrontendConfig_init () {
   cfg->entryusn_global = LDAP_OFF; 
   slapi_ch_array_add(&(cfg->allowed_to_delete_attrs),
                      slapi_ch_strdup("nsslapd-listenhost"));
+  slapi_ch_array_add(&(cfg->allowed_to_delete_attrs),
+                     slapi_ch_strdup("nsslapd-securelistenhost"));
 
 #ifdef MEMPOOL_EXPERIMENTAL
   cfg->mempool_switch = LDAP_ON;

Reviewed and commented by Nathan and Rich (Thanks!!)

Pushed to master.

$ git merge work
Updating c5cd4dd..e6c0ce5
Fast-forward
 ldap/servers/slapd/configdse.c  |   50 ++++++++++++++++++++++++++++++++------
 ldap/servers/slapd/libglobs.c   |   39 +++++++++++++++++++++++++++++-
 ldap/servers/slapd/proto-slap.h |    2 +
 ldap/servers/slapd/slap.h       |    2 +
 4 files changed, 84 insertions(+), 9 deletions(-)

$ git push
Counting objects: 17, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 2.18 KiB, done.
Total 9 (delta 7), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   c5cd4dd..e6c0ce5  master -> master

Hey. What's the format of the value for nsslapd-allowed-to-delete-attrs? It looks like it is hashed:

[root@dell-gx240-03 slapd-dell-gx240-03]# /usr/lib/mozldap/ldapmodify -D "cn=directory manager" -w secret12
dn: cn=config
changetype: modify
replace: nsslapd-allowed-to-delete-attrs
nsslapd-allowed-to-delete-attrs: nsslapd-listenhost nsslapd-rewrite-rfc1274

modifying entry cn=config

[root@dell-gx240-03 slapd-dell-gx240-03]# /usr/lib/mozldap/ldapsearch -D "cn=directory manager" -w secret12 -s sub -b "cn=config" objectclass=* | grep nsslapd-allowed-to-delete-attrs
nsslapd-allowed-to-delete-attrs:: QOnAs7DxwLM=


=================

How is it possible for someone to check the list of server config attributes they're allowed to delete?

Thanks!

Yeah, you are right, Deon.  My ldapsearch command also returns a base64 encoded attr value.  Let me investigate it.

If you grep nsslapd-allowed-to-delete-attrs in your config file, it'd return the clear test, though...
 # egrep nsslapd-allowed-to-delete-attrs /etc/dirsrv/slapd-jiji/dse.ldif
 nsslapd-allowed-to-delete-attrs: nsslapd-listenhost nsslapd-rewrite-rfc1274

Created attachment 468988 [details]
git patch file (master)

Description:
commit e6c0ce5d97a78689722fe3c627f7a99cf81f6b77 included a
config param value type mismatch.  The value is a string,
not an array of strings.

To verify the fix, the following command line should return
the space separated config attribute list.
$ ldapsearch ... -b "cn=config" "(cn=*)" nsslapd-allowed-to-delete-attrs
nsslapd-allowed-to-delete-attrs: nsslapd-listenhost nsslapd-securelistenhost

Reviewed by Rich (Thanks!!)

Pushed to master.

$ git merge 602456
Updating 7aef407..ad544f2
Fast-forward
 ldap/servers/slapd/configdse.c  |   15 ++++++++++-----
 ldap/servers/slapd/libglobs.c   |   16 ++++++----------
 ldap/servers/slapd/proto-slap.h |    2 +-
 3 files changed, 17 insertions(+), 16 deletions(-)

$ git push
Counting objects: 15, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 1.26 KiB, done.
Total 8 (delta 6), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   7aef407..ad544f2  master -> master

[root@rhel61 export]# ldapsearch -x -h localhost -p 1389 -D "cn=Directory Manager" -w Secret123 -b "cn=config" "(cn=*)" | grep nsslapd-allowed-to-delete-attrs
nsslapd-allowed-to-delete-attrs: nsslapd-listenhost nsslapd-securelistenhost


[root@rhel61 export]# ldapmodify -x -h localhost -p 1389 -D "cn=Directory Manager" -w Secret123 << EOF
> dn: cn=config
> changetype: modify
> replace: nsslapd-listenhost
> nsslapd-listenhost: rhel6.pnq.redhat.com
> EOF
modifying entry "cn=config"

[root@rhel61 export]# tail -f /var/log/dirsrv/slapd-rhel61/errors
[08/Jul/2011:17:42:39 +051800] slapd_listenhost2addr - PR_GetAddrInfoByName(pnq.redhat.com) failed - Netscape Portable Runtime error 0 (unknown)
[08/Jul/2011:17:43:58 +051800] - 389-Directory/1.2.8.5 B2011.187.02 starting up
[08/Jul/2011:17:43:58 +051800] - slapd started.  Listening on 10.65.201.213 port 1389 for LDAP requests
[08/Jul/2011:17:44:04 +051800] - slapd shutting down - signaling operation threads
[08/Jul/2011:17:44:04 +051800] - slapd shutting down - closing down internal subsystems and plugins
[08/Jul/2011:17:44:04 +051800] - Waiting for 4 database threads to stop
[08/Jul/2011:17:44:04 +051800] - All database threads now stopped
[08/Jul/2011:17:44:04 +051800] - slapd stopped.
[08/Jul/2011:17:44:07 +051800] - 389-Directory/1.2.8.5 B2011.187.02 starting up
[08/Jul/2011:17:44:07 +051800] - slapd started.  Listening on 10.65.201.213 port 1389 for LDAP requests

But after this, I am not able to bind to my DS:
[root@rhel61 export]# service dirsrv status
dirsrv rhel61 (pid 21730) is running...
[root@rhel61 export]# ldapsearch -x -h localhost -p 1389 -D "cn=Directory Manager" -w Secret123 -b "cn=config" "(cn=*)" | grep nsslapd-listenhost
ldap_sasl_bind(SIMPLE): Can't contact LDAP server (-1)

Please guide here.....

ok, I used IP instead of localhost in the ldap commands then It is working fine..

[root@rhel61 export]# ldapmodify -x -h 10.65.201.213 -p 1389 -D "cn=Directory Manager" -w Secret123 << EOF
> dn: cn=config
> changetype: modify
> delete: nsslapd-listenhost
> EOF
modifying entry "cn=config


But is that fine/ as expected?

(In reply to comment #18)
> ok, I used IP instead of localhost in the ldap commands then It is working
> fine..
> 
> [root@rhel61 export]# ldapmodify -x -h 10.65.201.213 -p 1389 -D "cn=Directory
> Manager" -w Secret123 << EOF
> > dn: cn=config
> > changetype: modify
> > delete: nsslapd-listenhost
> > EOF
> modifying entry "cn=config
> 
> 
> But is that fine/ as expected?

Yes.

Based on comment#19 - marking the bug as VERIFIED.