dscreate and dsconf print DM's password in verbose mode

[389-ds-bot]

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/50251

• Created at 2019-02-28 16:32:38 by mhonek (@kenoh)
• Closed at 2019-05-16 19:45:01 as fixed
• Assigned to mreynolds (@mreynolds389)
• Associated bugzillas
  • https://bugzilla.redhat.com/show_bug.cgi?id=1654059

---

Issue Description

When dscreate is executed in verbose mode, it prints Directory Manager's password to stderr. The same happens with dsconf when I change the password.

Package Version and Platform

389-ds-base-1.4.0.19-2.

Steps to reproduce

1. dscreate -v interactive
2. dsconf -v localhost directory_manager password_change

Actual results

$ dscreate -v interactive
...
DEBUG: cn=config set REPLACE: ('nsslapd-rootpw', 'Directory_Manager_Password')


$ dsconf -v localhost directory_manager password_change
...
Enter new directory manager password : 
CONFIRM - Enter new directory manager password : 
DEBUG: cn=config set REPLACE: ('nsslapd-rootpw', 'new_password')

Expected results

Actual value should not be printed in the debug logs. Python logging module supports filters that should be used to redact sensitive information from the logs

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-02-28 16:35:26

Metadata Update from @kenoh:

• Custom field origin adjusted to QE
• Custom field reviewstatus adjusted to None
• Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1654059

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-03-01 02:56:07

So I think a way to fix this could be in DSLdapObject inside the set function, we can say "if field in " then we display ***** instead - but in DEBUGGING=True maybe we still display it?

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-03-01 10:56:12

I'm thinking of having a class variable DSLdapObject._sensitive_attributes: List[str] and using it wherever required (for now in DSLdapObject.set()). And yes, DEBUGGING=True should override this.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-03-04 02:10:11

@kenoh I think this seems like a good approach. I would have done the same, have a class variable of that signature, and then to use it to filter or replace the content with **** or similar.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-05-13 17:49:53

Metadata Update from @mreynolds389:

• Issue assigned to mreynolds389 (was: kenoh)

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-05-13 18:05:35

#3425

Please review...

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-05-13 18:05:36

Metadata Update from @mreynolds389:

• Custom field reviewstatus adjusted to review (was: None)

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-05-14 12:49:48

One thing I thought I put down but seemingly haven't, was that in the simplest implementation case, we should have a method, e.g. DSLdapObject.display_attr_value(attr, value, hide_sensitive=True) that would handle the displaying and would be used where necessary; that in order to have consistent behaviour across our library. Something like...

def display_attr_value(self, attr, value, hide_sesitive=True):
    if DEBUGGING or hide_sensitive:
        return value
    else:
        if attr.lower() in self._sensitive_attributes:
            return '********'
        else:
            return value

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-05-15 02:09:10

@kenoh Another option is to wrap / over-load the str/unicode method on the str we return. But that could be complex ....

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-05-16 19:45:02

commit 632ecb9

64a784f..1bb8882 389-ds-base-1.4.0 -> 389-ds-base-1.4.0

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-05-16 19:45:02

Metadata Update from @mreynolds389:

• Issue close_status updated to: fixed
• Issue status updated to: Closed (was: Open)