Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: Add an option to exclude sensitive attributes from the retro changelog DB #4701

Closed
jchapma opened this issue Mar 26, 2021 · 8 comments
Closed
Assignees
Labels
priority_high need urgent fix / highly valuable / easy to fix
Milestone

Comments

@jchapma
Copy link
Contributor

jchapma commented Mar 26, 2021

Description.
A user has requested functionality that will allow an admin exclude certain attributes from the retro changelog suffix.

Solution
Add an option that allows an admin exclude attributes from the retro changelog db:

sudo dsconf -D "cn=dm" inst01 plugin retro-changelog set --exclude-attrs "hideme,hidemetoo"

These attributes will be added to the cn=Retro Changelog Plugin,cn=plugins,cn=config dn:

dn: cn=Retro Changelog Plugin,cn=plugins,cn=config
objectClass: top
objectClass: nsSlapdPlugin
objectClass: extensibleObject
...
nsslapd-exclude-attrs: hideme,hidemetoo

When the post op plugin is called the retrocl plugin will query the server for excluded attributes and exclude them from the retro changelog db. Similar to the exclude suffix functionality.

Additional context
https://bugzilla.redhat.com/show_bug.cgi?id=1850664

@jchapma jchapma added the needs triage The issue will be triaged during scrum label Mar 26, 2021
@jchapma jchapma changed the title Add an option to exclude sensitive attributes from the retro changelog DB RFE: Add an option to exclude sensitive attributes from the retro changelog DB Mar 26, 2021
@jchapma jchapma self-assigned this Mar 26, 2021
@jchapma
Copy link
Contributor Author

jchapma commented Apr 9, 2021

This RFE will allow a user exclude attributes from the retro changelog db.

After an add/delete/modify/modrdn operation an entry is added to the "cn=changelog" suffix, reflecting the operation.

The changes for this issue will exclude specified attributes from the changelog entry, also the operation changestring will be modified to "hide" the excluded atributes.

Here are two examples of a modify operation before and after this change:

Before

dn: changenumber=32,cn=changelog
objectClass: top
objectClass: changelogentry
objectClass: extensibleObject
targetuniqueid: 86811681-991211eb-81fecedb-88ed0954
car: 131D16647
phone: 11111111
changeNumber: 32
targetDn: uid=ISauder99,ou=product development,dc=test02,dc=com
changeTime: 20210409090433Z
changeType: add
changes:: dWlkOiBJU2F1ZGVyOTkKZ2l2ZW5OYW1lOiBnaXZlbl9uYW1lCm9iamVjdENsYXNzOiB0
 b3AKb2JqZWN0Q2xhc3M6IHBlcnNvbgpvYmplY3RDbGFzczogb3JnYW5pemF0aW9uYWxQZXJzb24Kb
 2JqZWN0Q2xhc3M6IGluZXRvcmdwZXJzb24Kc246IHN1cm5hbWUKY246IHVzZXIKY2FyTGljZW5zZT
 ogMDR3eDExMDM4CmhvbWVQaG9uZTogMDg3ODI3MTQwNApjcmVhdG9yc05hbWU6IGNuPWRtCm1vZGl
 maWVyc05hbWU6IGNuPWRtCmNyZWF0ZVRpbWVzdGFtcDogMjAyMTA0MDkwOTA0MzNaCm1vZGlmeVRp
 bWVzdGFtcDogMjAyMTA0MDkwOTA0MzNaCnBhcmVudGlkOiAzCmVudHJ5aWQ6IDIzCg==

Decoded changestring

replace: homePhone
homePhone: 11111111
-
replace: carLicense
carLicense: 131D16647
-
replace: modifiersname
modifiersname: cn=dm
-
replace: modifytimestamp
modifytimestamp: 20210409090234Z

After (With homePhone attribute excluded)

dn: changenumber=32,cn=changelog
objectClass: top
objectClass: changelogentry
objectClass: extensibleObject
targetuniqueid: 86811681-991211eb-81fecedb-88ed0954
car: 131D16647
changeNumber: 32
targetDn: uid=ISauder99,ou=product development,dc=test02,dc=com
changeTime: 20210409090433Z
changeType: add
changes:: cmVwbGFjZTogaG9tZVBob25lCmhvbWVQaG9uZTogWFhYWFgKLQpyZXBsYWNlOiBjYXJMaWNlbnNl
CmNhckxpY2Vuc2U6IFhYWFhYCi0KcmVwbGFjZTogbW9kaWZpZXJzbmFtZQptb2RpZmllcnNuYW1l
OiBjbj1kbQotCnJlcGxhY2U6IG1vZGlmeXRpbWVzdGFtcAptb2RpZnl0aW1lc3RhbXA6IDIwMjEw
NDA5MDkwMjM0Wgo=

Decoded changestring

replace: homePhone
homePhone: XXXXX
-
replace: carLicense
carLicense: 131D16647
-
replace: modifiersname
modifiersname: cn=dm
-
replace: modifytimestamp
modifytimestamp: 20210409090234Z

I'm currently looking into how these changes will impact syncrepl, I would appreciate any help with answering these questions:

  1. Is it better to remove the excluded attribute from the changelog entry changestring or obfuscate the attribute value as shown above.
  2. If this change break syncrepl, is there a better way to do it.

NOTE:
Using an ACI to restrict access to certain changelog attributes didn't suit the customer's usage model.

@tbordaz
Copy link
Contributor

tbordaz commented Apr 12, 2021

@jchapma , sync_repl uses the retrochangelog to detect any change on an entry and send it back to the sync_repl client if the entry matches the sync_repl filter.
I have a concern this RFE can possibly "hide" updates to sync_repl. For example, if 'givenName', 'modifiersname' and 'modifytimestamp' are skipped from retroCL and an update changes only 'givenName', there is a risk that the update will be a NOOP at retroCL level and will not be recorded. To be tested.
If it happens then it should simply be documented so that the administrator knows such tuning may impact sync_repl

The RFE mentioned the ability to exclude attributes and it looks enough for me. obfuscating values can be more complex because you will need to set a value matching the attribute syntax.

@jchapma
Copy link
Contributor Author

jchapma commented Apr 13, 2021

@tbordaz
Thanks for your feedback, so the plan is to exclude the specified attribute from the retrocl entry and document that this feature will have an impact on syncrepl.

jchapma added a commit to jchapma/389-ds-base that referenced this issue Apr 22, 2021
Description: When the retro changelog plugin is enabled it writes the
             added/modified values to the "cn-changelog" suffix. In
             some cases an entries attribute values can be of a
             sensitive nature and should be excluded. This RFE adds
             functionality that will allow an admin exclude certain
             attributes from the retro changelog DB.

Relates: 389ds#4701

Reviewed by: mreynolds389, droideck (Thanks folks)
jchapma added a commit that referenced this issue Apr 27, 2021
Description: When the retro changelog plugin is enabled it writes the
             added/modified values to the "cn-changelog" suffix. In
             some cases an entries attribute values can be of a
             sensitive nature and should be excluded. This RFE adds
             functionality that will allow an admin exclude certain
             attributes from the retro changelog DB.

Relates: #4701

Reviewed by: mreynolds389, droideck (Thanks folks)
@tbordaz
Copy link
Contributor

tbordaz commented Apr 29, 2021

@jchapma is it okay to close it ?
Remember to copy/paste the git commit (master and branches)

@tbordaz tbordaz added priority_high need urgent fix / highly valuable / easy to fix and removed needs triage The issue will be triaged during scrum labels Apr 29, 2021
@tbordaz tbordaz added this to the 1.4.3 milestone Apr 29, 2021
@mreynolds389
Copy link
Contributor

@jchapma This needs to be cherry picked down to 1.4.3 please

jchapma added a commit to jchapma/389-ds-base that referenced this issue Apr 30, 2021
Description: When the retro changelog plugin is enabled it writes the
             added/modified values to the "cn-changelog" suffix. In
             some cases an entries attribute values can be of a
             sensitive nature and should be excluded. This RFE adds
             functionality that will allow an admin exclude certain
             attributes from the retro changelog DB.

Relates: 389ds#4701

Reviewed by: mreynolds389, droideck (Thanks folks)
@jchapma
Copy link
Contributor Author

jchapma commented Apr 30, 2021

e501b83 -> master
f0861d8 -> 389-ds-base-1.4.3
4ba0f07..e0cf8a9 389-ds-base-1.4.4 -> 389-ds-base-1.4.4

@mreynolds389
Copy link
Contributor

e501b83 -> master
f0861d8 -> 389-ds-base-1.4.3

And 389-ds-base-1.4.4 please

jchapma added a commit that referenced this issue Apr 30, 2021
…4746)

Description: When the retro changelog plugin is enabled it writes the
             added/modified values to the "cn-changelog" suffix. In
             some cases an entries attribute values can be of a
             sensitive nature and should be excluded. This RFE adds
             functionality that will allow an admin exclude certain
             attributes from the retro changelog DB.

Relates: #4701

Reviewed by: mreynolds389, droideck (Thanks folks)
jchapma added a commit that referenced this issue Apr 30, 2021
Description: When the retro changelog plugin is enabled it writes the
             added/modified values to the "cn-changelog" suffix. In
             some cases an entries attribute values can be of a
             sensitive nature and should be excluded. This RFE adds
             functionality that will allow an admin exclude certain
             attributes from the retro changelog DB.

Relates: #4701

Reviewed by: mreynolds389, droideck (Thanks folks)
@mreynolds389
Copy link
Contributor

This introduced a complier warning:

../389-ds-base/ldap/servers/plugins/retrocl/retrocl.c: In function ‘retrocl_start’:
../389-ds-base/ldap/servers/plugins/retrocl/retrocl.c:402:20: warning: unused variable ‘length’ [-Wunused-variable]
  402 |             size_t length = strlen(value);
      | 

please fix...

jchapma added a commit to jchapma/389-ds-base that referenced this issue May 4, 2021
Description: An unused variable generates a compiler warning.

Fix description: Remove unused variable.
		 Modify CI test to restart the test instance instead
		 of using dynamic plugins.

Fixes: 389ds#4750

Relates: 389ds#4701

Reviewed by: jchapma (One line commit rule)
jchapma added a commit that referenced this issue May 4, 2021
Description: An unused variable generates a compiler warning.

Fix description: Remove unused variable. Modify CI test to restart the test instance instead
		         of using dynamic plugins.

Fixes: #4750

Relates: #4701

Reviewed by: jchapma (One line commit rule)
@jchapma jchapma closed this as completed May 6, 2021
jchapma added a commit that referenced this issue Jun 3, 2021
Description: An unused variable generates a compiler warning.

Fix description: Remove unused variable. Modify CI test to restart the test instance instead
		         of using dynamic plugins.

Fixes: #4750

Relates: #4701

Reviewed by: jchapma (One line commit rule)
jchapma added a commit that referenced this issue Jun 3, 2021
Description: An unused variable generates a compiler warning.

Fix description: Remove unused variable. Modify CI test to restart the test instance instead
		         of using dynamic plugins.

Fixes: #4750

Relates: #4701

Reviewed by: jchapma (One line commit rule)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority_high need urgent fix / highly valuable / easy to fix
Projects
None yet
Development

No branches or pull requests

5 participants