ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-6721

HTML code of "authentication.php" contains a ldap_bind_password in clear text if LDAP auth is enabled

    Details

      Description

      This security problem is actual only for zabbix-super-administrator user accounts.

      When this is considered as a problem:
      for example I have several zabbix-super-admins but they should not know the LDAP bind pass.

      Goal:
      any zabbix-super-admins which doesn't own the password - should not be able to know it (we suppose that they don't have direct shell access to Apache/DB server)

      Possible solution:
      For example you typed new "bind password" and pressed the Save button. The new password will be send to Apache and if it's correct it will be stored in the database (as it is currently).
      Reloaded page will not contain any value in the "bind password" box and source HTML code.

      I'm not sure, but maybe it would worth to show some grayed default text in the box, like "Password stored into DB, type new password if required." if the password is not empty in the DB.
      This default text will help a bit after a user has enabled the LDAP auth.
      If locate a mouse cursor into the box then the default text will disappear (we have already such approach in some places in zabbix frontend).

      Somehow related issue ZBX-6410

        Issue Links

          Activity

          Show
          Volker Fröhlich added a comment - Might be http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5572
          Hide
          Eduards Samersovs added a comment -

          Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-6721

          Show
          Eduards Samersovs added a comment - Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-6721
          Hide
          Volker Fröhlich added a comment -

          Is this issue what the above CVE is about?

          Show
          Volker Fröhlich added a comment - Is this issue what the above CVE is about?
          Hide
          Oleksiy Zagorskyi added a comment - - edited

          dev branch tested.

          (1) if we first time try to enable LDAP auth then we see there "Change password" button which ... I have to say misleads.
          Could we, in case if "ldap_bind_password" column is empty, consider this like a first attempt and show an empty input box instead the button ?

          Eduards Samersovs RESOVLED r.41692

          Oleg Egorov CLOSED

          Show
          Oleksiy Zagorskyi added a comment - - edited dev branch tested. (1) if we first time try to enable LDAP auth then we see there "Change password" button which ... I have to say misleads. Could we, in case if "ldap_bind_password" column is empty, consider this like a first attempt and show an empty input box instead the button ? Eduards Samersovs RESOVLED r.41692 Oleg Egorov CLOSED
          Hide
          Oleksiy Zagorskyi added a comment - - edited

          (2) after I click to the "Change password" button, specified an incorrect Bind passwords, tried Test or Save and in both cases I'm getting expected errors.
          But both passwords fields returned empty in this case, which I believe is wrong.
          In current 2.2 versions both password fields being returned filled by values (which can be edited).
          In other words it's BAD that after a Test (success or not) an user has to again type both passwords to then try Save.
          I believe that before user successfully Saved the auth config we should preserve the passwords in frontend.

          Eduards Samersovs Due to the safety, we can't show password for future editing. I understand, this is not user friendly, but we can't do that.

          Oleg Egorov CLOSED

          <richlv> this seriously devalues testing - we test, it works - then we have to re-enter all detail - possibly making some mistake. we can not test the exact same config we would be saving. what is the security concern with keeping credentials in the current session ? -> REOPENED

          Eduards Samersovs We cannot use session to store form data due of multitab issue.

          Oleksiy Zagorskyi Hmmm, cannot get it. Then why it was possible before current development ?
          REOPENED

          Eduards Samersovs Before was bug! ))

          Oleg Egorov CLOSED AS DISCUSSED

          <richlv> please add the conclusions of the discussion here so we don't have to revisit this later

          Show
          Oleksiy Zagorskyi added a comment - - edited (2) after I click to the "Change password" button, specified an incorrect Bind passwords, tried Test or Save and in both cases I'm getting expected errors. But both passwords fields returned empty in this case, which I believe is wrong. In current 2.2 versions both password fields being returned filled by values (which can be edited). In other words it's BAD that after a Test (success or not) an user has to again type both passwords to then try Save. I believe that before user successfully Saved the auth config we should preserve the passwords in frontend. Eduards Samersovs Due to the safety, we can't show password for future editing. I understand, this is not user friendly, but we can't do that. Oleg Egorov CLOSED <richlv> this seriously devalues testing - we test, it works - then we have to re-enter all detail - possibly making some mistake. we can not test the exact same config we would be saving. what is the security concern with keeping credentials in the current session ? -> REOPENED Eduards Samersovs We cannot use session to store form data due of multitab issue. Oleksiy Zagorskyi Hmmm, cannot get it. Then why it was possible before current development ? REOPENED Eduards Samersovs Before was bug! )) Oleg Egorov CLOSED AS DISCUSSED <richlv> please add the conclusions of the discussion here so we don't have to revisit this later
          Hide
          Oleksiy Zagorskyi added a comment -

          Volker, as I see we should resolve the CVE report.

          Show
          Oleksiy Zagorskyi added a comment - Volker, as I see we should resolve the CVE report.
          Hide
          Volker Fröhlich added a comment -

          Very good! Can you please provide a 1.8 backport too?

          Show
          Volker Fröhlich added a comment - Very good! Can you please provide a 1.8 backport too?
          Hide
          Oleksiy Zagorskyi added a comment -

          Only if devs will be so kind to do it
          But I'm afraid it will not worth to do.
          I know for sure that there is big difference in the related code between 1.8. And current issue is not so critical.

          Show
          Oleksiy Zagorskyi added a comment - Only if devs will be so kind to do it But I'm afraid it will not worth to do. I know for sure that there is big difference in the related code between 1.8. And current issue is not so critical.
          Hide
          Oleg Egorov added a comment - - edited

          (3) Please fix coding style, div long rows.

          Eduards Samersovs RESOLVED r.41692

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (3) Please fix coding style, div long rows. Eduards Samersovs RESOLVED r.41692 Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (5) "Authentication method changed to LDAP", and if re-save form we see again "Authentication method changed to LDAP"

          Eduards Samersovs RESOLVED r.41692

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (5) "Authentication method changed to LDAP", and if re-save form we see again "Authentication method changed to LDAP" Eduards Samersovs RESOLVED r.41692 Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          Problems (4), (5) and (7) exist in 2.0.11 too...

          Eduards Samersovs This dev branch is made from 2.0.11rc1 and will be integrated in all versions

          Show
          Oleg Egorov added a comment - - edited Problems (4), (5) and (7) exist in 2.0.11 too... Eduards Samersovs This dev branch is made from 2.0.11rc1 and will be integrated in all versions
          Hide
          Oleg Egorov added a comment - - edited

          (6) After clicking on "Change password" field "Bind passord" is not empty...

          Oleksiy Zagorskyi are you sure?
          I don't see such behavior.

          Oleg Egorov CLOSED, browser cache

          Show
          Oleg Egorov added a comment - - edited (6) After clicking on "Change password" field "Bind passord" is not empty... Oleksiy Zagorskyi are you sure? I don't see such behavior. Oleg Egorov CLOSED, browser cache
          Hide
          Oleg Egorov added a comment - - edited

          (7) Open LDAP configuration, remove field value, for example from "Search attribute", click save
          Warning. Incorrect value for field "Search attribute": cannot be empty.

          Field not empty.

          Eduards Samersovs RESOLVED r.41692

          Oleg Egorov Correct and saved LDAP authentication, remove "Search attribute", then save.
          Page refreshing

          Warning. Incorrect value for field "Search attribute": cannot be empty.

          But field "Search attribute" now isn't empty, there is data, which was before removing

          Other problem, if in form exist problems, after saving reset values

          REOPEN

          Eduards Samersovs RESOLVED r.41737

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (7) Open LDAP configuration, remove field value, for example from "Search attribute", click save Warning. Incorrect value for field "Search attribute": cannot be empty. Field not empty. Eduards Samersovs RESOLVED r.41692 Oleg Egorov Correct and saved LDAP authentication, remove "Search attribute", then save. Page refreshing Warning. Incorrect value for field "Search attribute": cannot be empty. But field "Search attribute" now isn't empty, there is data, which was before removing Other problem, if in form exist problems, after saving reset values REOPEN Eduards Samersovs RESOLVED r.41737 Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (8) Default authentication: LDAP, change to Internal and again to LDAP...
          Warning. Field "user" is mandatory.
          Warning. Field "User password" is mandatory.

          Eduards Samersovs RESOLVED r.41703

          Interesting moment, fields Login (user) is not empty, there I see "Admin"

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (8) Default authentication: LDAP, change to Internal and again to LDAP... Warning. Field "user" is mandatory. Warning. Field "User password" is mandatory. Eduards Samersovs RESOLVED r.41703 Interesting moment, fields Login (user) is not empty, there I see "Admin" Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (9) Why after re-saving I should enter User password again and again...

          Oleksiy Zagorskyi I suppose your question duplicates mine (2)

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (9) Why after re-saving I should enter User password again and again... Oleksiy Zagorskyi I suppose your question duplicates mine (2) Oleg Egorov CLOSED
          Hide
          Oleksiy Zagorskyi added a comment - - edited

          (10) After changes do you test it at all?
          on latest rev 41703 I try to change Internal->LDAP (correct Bind password is already in DB).

          I type correct User password and to to Test and I get:

          Undefined variable: dn [include/classes/class.cldap.php:136]
          Undefined variable: ldapAccountOk [authentication.php:154]
          

          if I try to Save (with correct User passwprd) then I see green success header and this error bottom of it:

          Undefined variable: dn [include/classes/class.cldap.php:136]
          

          If I try to test with incorrect User password, then:

          Undefined variable: dn [include/classes/class.cldap.php:136]
          ldap_bind(): Unable to bind to server: Invalid credentials [include/classes/class.cldap.php:158]
          Login name or password is incorrect!
          Undefined variable: ldapAccountOk [authentication.php:154]
          

          Eduards Samersovs RESOLVED r.41737 Sorry my mistake!

          Oleg Egorov CLOSED

          Show
          Oleksiy Zagorskyi added a comment - - edited (10) After changes do you test it at all? on latest rev 41703 I try to change Internal->LDAP (correct Bind password is already in DB). I type correct User password and to to Test and I get: Undefined variable: dn [include/classes/class.cldap.php:136] Undefined variable: ldapAccountOk [authentication.php:154] if I try to Save (with correct User passwprd) then I see green success header and this error bottom of it: Undefined variable: dn [include/classes/class.cldap.php:136] If I try to test with incorrect User password, then: Undefined variable: dn [include/classes/class.cldap.php:136] ldap_bind(): Unable to bind to server: Invalid credentials [include/classes/class.cldap.php:158] Login name or password is incorrect! Undefined variable: ldapAccountOk [authentication.php:154] Eduards Samersovs RESOLVED r.41737 Sorry my mistake! Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (11) class.cldap.php
          $dn = null; Defined two times.

          if () {
           $dn = null;
          }
          elseif () {
           $dn = 1;
          }
          elseif () {
           $dn = 2;
          }
          else {
           $dn = null;
          }
          

          But should be:

          $dn = null;
          if () {
          }
          elseif () {
           $dn = 1;
          }
          elseif () {
           $dn = 2;
          }
          else {
          }
          

          Eduards Samersovs RESOLVED r.41775

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (11) class.cldap.php $dn = null; Defined two times. if () { $dn = null ; } elseif () { $dn = 1; } elseif () { $dn = 2; } else { $dn = null ; } But should be: $dn = null ; if () { } elseif () { $dn = 1; } elseif () { $dn = 2; } else { } Eduards Samersovs RESOLVED r.41775 Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment -

          TESTED

          Show
          Oleg Egorov added a comment - TESTED
          Show
          Oleksiy Zagorskyi added a comment - (11) Need to reflect new logic in documentation. https://www.zabbix.com/documentation/2.2/manual/web_interface/frontend_sections/administration/authentication https://www.zabbix.com/documentation/2.0/manual/web_interface/frontend_sections/administration/authentication
          Hide
          Eduards Samersovs added a comment -

          Fixed in versions 2.3.0 (trunk) r.41784, 2.2.2rc1 r.41783, 2.0.11.rc1 r.41781

          Show
          Eduards Samersovs added a comment - Fixed in versions 2.3.0 (trunk) r.41784, 2.2.2rc1 r.41783, 2.0.11.rc1 r.41781
          Hide
          richlv added a comment - - edited

          -------------------------
          Vulnerability description
          -------------------------

          Zabbix frontend may expose LDAP authentication password to users with Superadmin privileges.

          Please use CVE-2013-5572 to refer to this vulnerability.

          -------
          Details
          -------

          Zabbix stores password for binding to LDAP. This password was transmitted to any user of Superadmin privileges who would open authentication configuration page. Zabbix user or admin level privileges would not allow access to this password.

          -----------------
          Affected versions
          -----------------

          All of the Zabbix versions are vulnerable to this problem.

          --------------
          Fixed versions
          --------------

          This vulnerability has been fixed in the latest releases of Zabbix.

          The fix is available in the following Zabbix releases:
          2.2.2
          2.0.11
          1.8.20

          Show
          richlv added a comment - - edited ------------------------- Vulnerability description ------------------------- Zabbix frontend may expose LDAP authentication password to users with Superadmin privileges. Please use CVE-2013-5572 to refer to this vulnerability. ------- Details ------- Zabbix stores password for binding to LDAP. This password was transmitted to any user of Superadmin privileges who would open authentication configuration page. Zabbix user or admin level privileges would not allow access to this password. ----------------- Affected versions ----------------- All of the Zabbix versions are vulnerable to this problem. -------------- Fixed versions -------------- This vulnerability has been fixed in the latest releases of Zabbix. The fix is available in the following Zabbix releases: 2.2.2 2.0.11 1.8.20
          Hide
          Eduards Samersovs added a comment -

          Fixed 1.8 in development branch svn://svn.zabbix.com/branches/dev/ZBX-6721-18

          Show
          Eduards Samersovs added a comment - Fixed 1.8 in development branch svn://svn.zabbix.com/branches/dev/ZBX-6721-18
          Hide
          Volker Fröhlich added a comment -

          Thanks a lot, Eduards!

          Show
          Volker Fröhlich added a comment - Thanks a lot, Eduards!
          Hide
          Pavels Jelisejevs added a comment - - edited

          (12) I've made a minor fix in r42343, please review.

          Eduards Samersovs CLOSED

          Show
          Pavels Jelisejevs added a comment - - edited (12) I've made a minor fix in r42343, please review. Eduards Samersovs CLOSED
          Hide
          Pavels Jelisejevs added a comment -

          TESTED.

          If (12) is OK - you can merge.

          Show
          Pavels Jelisejevs added a comment - TESTED. If (12) is OK - you can merge.
          Hide
          Eduards Samersovs added a comment -

          Fixed in versions 1.8.20rc2 r.42345

          Show
          Eduards Samersovs added a comment - Fixed in versions 1.8.20rc2 r.42345
          Hide
          richlv added a comment - - edited

          (13) changelog entry is "fixed LDAP authentication" - that is way, way too vague. please, use something like "removed LDAP bind password from authentication page source" or similar

          Fixed in versions 2.3.0 (trunk) r.44462, 2.2.2rc1 r.44461, 2.0.11.rc1 r.44460, 1.8.20rc2 r.44459

          Show
          richlv added a comment - - edited (13) changelog entry is "fixed LDAP authentication" - that is way, way too vague. please, use something like "removed LDAP bind password from authentication page source" or similar Fixed in versions 2.3.0 (trunk) r.44462, 2.2.2rc1 r.44461, 2.0.11.rc1 r.44460, 1.8.20rc2 r.44459

            People

            • Assignee:
              Unassigned
              Reporter:
              Oleksiy Zagorskyi
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved: