#111 Plugins - ability to control behavior of modifyTimestamp/modifiersName
Closed: wontfix None Opened 12 years ago by mkosek.

https://bugzilla.redhat.com/show_bug.cgi?id=453756

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9)
Gecko/2008052906 Firefox/3.0

Description of problem:
It would be a nice feature (for example, some additional functions in the
plug-in API) that the plugins could execute internal modification operations
without changing the operational attributes (modifiersName, modifyTimestamp
etc).

It would greatly simplify the management of an LDAP infrastructure in case
there are many admins and unit managers - one could see right away who was the
last person to change the entry. Today in our production environment we have to
write and stock full audit logs to follow these changes.

The problem is that each time an internal plug-in modifies the entry (in
particular it concerns the referential integrity and memberOf plugins in our
production environment) the modifiersName is changed to the plug-in
configuration DN (and the attribute modifyTimestamp accordingly).

Version-Release number of selected component (if applicable):


How reproducible:
Always


Steps to Reproduce:
1. Make a modification that concerns a plug-in like memberOf or referential
integrity.


Actual Results:
Take a look at the attributes modifiersName and modifyTimestamp. Even if we
have not DIRECTLY changed the entry we will find these attributes changed.
Example :

nscpentryWSI: creatorsName: uid=andrey.ivanov,ou=person...
nscpentryWSI: modifiersName: cn=MemberOf,cn=plugins,cn=config
nscpentryWSI: createTimestamp: 20070803092138Z
nscpentryWSI: modifyTimestamp: 20080419154554Z


Expected Results:
An option should allow to avoid touching the modifiersName and modifyTimestamp
by internal plug-in modification operations.

Additional info:
This is not a bug, it is a feature request. It is linked in a certain way to
the bug 434914.

batch update to FUTURE milestone

If the concern is the audit trail for the modifies, then the modifiersName should be the DN from the original operation, not the DN of the plugin that performed some internal update operation.

Also, for auditability, wouldn't you still want to know when the entry was changed, even if you don't update the modifyTimestamp? For example, during a BIND operation, if password retries are checked, wouldn't you still want to know when the entry was updated due to a failed bind attempt?

in dna_load_plugin_config() and dna_update_config_event():
you should initialize char *dn = NULL;
if for some reason pb->pb_op is NULL, then dn will be unset, so you will be passing uninitialized memory to dna_update_config_event

Also, the dn returned points into pb->pb_op->o_sdn - so if the op is finished, or goes out of scope, the dn in dna_update_config_event() may be corrupted - to be safe, you should pass in a copy made with slapi_ch_strdup(), then free it in dna_update_config_event()

in linked_attrs_parse_config_entry():
{{{
"Distinguished Name syntax for linked attribute "
560 "pair \"%s\".\n", LINK_LINK_TYPE, entry->dn);
559 "Distinguished Name syntax for linked attribute"
560 "pair \"%s\" attribute \"%s\".\n", LINK_LINK_TYPE, entry->dn, value);
}}}

you should leave the space character after attribute - it should be ".. attribute "

in writeintegritylog()
should use slapi_pblock_get(pb, SLAPI_REQUESTOR_DN, &dn) to get the dn rather than accessing the pb structure directly. Only internal server operations should access the pb directly - everything else should use slapi_pblock_get/set.

in slapi_pblock_set():
case SLAPI_REQUESTOR_DN:
are you sure that value is always properly normalized? If not, you'll have to use slapi_sdn_set_dn_byref() instead.

You might also want to add a case for SLAPI_REQUESTOR_SDN where the value is a Slapi_DN*

Thanks Rich for the comments! I also forgot to set the new attributes' syntax to distinguished name. I had previously set it as Directory String.

Making changes...

in slapi_pblock_new_by_pb()
{{{
slapi_sdn_set_normdn_byval((&pb->pb_op->o_sdn),(&origpb->pb_op->o_sdn)->dn);
}}}
you can't guarantee that (&origpb->pb_op->o_sdn)->dn is set - it may be NULL if the sdn was originally set to an unnormalized DN - you have to use slapi_sdn_get_dn() which will normalize and set dn if necessary

otherwise, looks good

Thanks Rich!

[mareynol@localhost servers]$ git merge ticket111
Updating d664d54..f7b882a
Fast-forward
ldap/schema/01core389.ldif | 2 +
ldap/servers/plugins/automember/automember.c | 16 ++--
ldap/servers/plugins/dna/dna.c | 107 ++++++++++++-----------
ldap/servers/plugins/linkedattrs/fixup_task.c | 22 +++--
ldap/servers/plugins/linkedattrs/linked_attrs.c | 36 ++++----
ldap/servers/plugins/linkedattrs/linked_attrs.h | 1 +
ldap/servers/plugins/memberof/memberof.c | 51 +++++++----
ldap/servers/plugins/mep/mep.c | 24 +++---
ldap/servers/plugins/referint/referint.c | 41 ++++++---
ldap/servers/slapd/add.c | 41 ++++++++-
ldap/servers/slapd/config.c | 17 ++++-
ldap/servers/slapd/libglobs.c | 17 ++++
ldap/servers/slapd/modify.c | 20 ++++-
ldap/servers/slapd/modrdn.c | 12 ++-
ldap/servers/slapd/opshared.c | 23 +++++-
ldap/servers/slapd/pblock.c | 73 ++++++++++++++--
ldap/servers/slapd/plugin_internal_op.c | 17 +++-
ldap/servers/slapd/proto-slap.h | 2 +
ldap/servers/slapd/slap.h | 3 +
ldap/servers/slapd/slapi-plugin.h | 14 +++
20 files changed, 381 insertions(+), 158 deletions(-)

[mareynol@localhost servers]$ git push origin master
Counting objects: 65, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (33/33), done.
Writing objects: 100% (33/33), 7.88 KiB, done.
Total 33 (delta 26), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
d664d54..f7b882a master -> master

Here is how to use the feature:

To activate the feature you need to set this attribute under cn=config

nsslapd-plugin-binddn-tracking: on

This will set the modifiersname/creatorsname to the the bind DN that initiated the operation. We have also added two new operational attributes:

    internalModifiersname 
    internalCreatorsname

These will contain the plugin DN that modified/added the entry. So we are not losing any information with the feature turned on.

Note - if using replication you need to make sure the schema is in sync. These new attributes were added to 01core389.ldif, and they will need to present on all the replicas.

Mark

The new strategy will be to use Thread Local Storage to store the modifiersName so plugins won't have to clone pblocks, etc.

commit changeset:9c0c0aa/389-ds-base
Author: Rich Megginson rmeggins@redhat.com
Date: Thu Feb 16 12:51:23 2012 -0700
Revert "Ticket #111 - ability to control behavior of modifyTimestamp/modifie
rsName"
This reverts commit changeset:f7b882a/389-ds-base
This is a partial revert. We still want to keep the new config parameter,
we just don't want to have to change the plugins in any way. We will
come up with a new mechanism for keeping track of the original requestor
DN, most likely a scheme using Thread Local Storage (TLS).

originally targeted for 1.2.11.rc1, but actually in the 1.2.11.a1 release

Added initial screened field value.

This ticket has been superseded by tickets 302, and now 495

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.2.11.a1

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/111

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata