Bug 5432 - password changing failed via pam_winbind on Solaris
Summary: password changing failed via pam_winbind on Solaris
Status: NEW
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Winbind (show other bugs)
Version: unspecified
Hardware: Other Solaris
: P3 critical (vote)
Target Milestone: ---
Assignee: Ira Cooper
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-30 04:59 UTC by SATOH Fumiyasu
Modified: 2016-12-08 17:18 UTC (History)
4 users (show)

See Also:


Attachments
Proposed patch: Remove redundant pam_set_item() call (617 bytes, patch)
2008-04-30 05:03 UTC, SATOH Fumiyasu
no flags Details
Updated patch for 3.2.8 (v3-2-test) (1.32 KB, patch)
2009-02-05 07:47 UTC, SATOH Fumiyasu
no flags Details
Patch for Samba 3.6 (v3-6-test) (1.06 KB, patch)
2012-04-19 10:38 UTC, SATOH Fumiyasu
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description SATOH Fumiyasu 2008-04-30 04:59:11 UTC
When user tries to change his/her password via pam_winbind.so,
pam_set_item(3PAM) is called twice (at _winbind_read_password() and
pam_sm_chauthtok() in nsswitch/pam_winbind.c) with PAM_OLDAUTHTOK
and the old password. The second pam_set_item() call (at pam_sm_chauthtok())
is redundantly because the old password for pam_set_item() has same
value (pointer to char*) before (at _winbind_read_password()).

_winbind_read_password(..., const char **pass) {
  ...
  authtok_flag = on(WINBIND__OLD_PASSWORD, ctrl) ? PAM_OLDAUTHTOK : PAM_AUTHTOK;
  ...
  retval = pam_set_item(pamh, authtok_flag, token);
  _pam_delete(token);     /* clean it up */
  if (retval != PAM_SUCCESS || 
      (retval = _pam_get_item(pamh, authtok_flag, &item)) != PAM_SUCCESS) {
	  _pam_log(pamh, ctrl, LOG_CRIT, "error manipulating password");
	  return retval;
  }
  *pass = item;
  ...
}

pam_sm_chauthtok() {
  ...
  if (flags & PAM_PRELIM_CHECK) {
    ret = _winbind_read_password(..., (const char **) &pass_old);
    ...
    ret = pam_set_item(pamh, PAM_OLDAUTHTOK, (const void *) pass_old);
    ...
  } else if (flags & PAM_UPDATE_AUTHTOK) {
    ...
  }
  ...
}

On the other hand, on Solaris, the pam_set_item() call clears
the previous set password (by pam_set_item()) in PAM informations
(maybe security reason). For example:

  const char *item;
  ## Get PAM_OLDAUTHTOK data from PAM
  pam_get_item(pamh, PAM_OLDAUTHTOK, &item);
  ## Put item to PAM_OLDAUTHTOK data in PAM
  ##   1. Clear previous PAM_OLDAUTHTOK data (in *item) in PAM
  ##   2. Set new data (in *item) as PAM_OLDAUTHTOK  in PAM
  pam_set_item(pamh, PAM_OLDAUTHTOK, &item);
  ## Get PAM_OLDAUTHTOK data from PAM again
  pam_get_item(pamh, PAM_OLDAUTHTOK, &item);
  ## Show PAM_OLDAUTHTOK (it is broken on Solaris)
  puts(item);

That is why, password changing failed via pam_winbind.so on Solaris.
Comment 1 SATOH Fumiyasu 2008-04-30 05:03:08 UTC
Created attachment 3271 [details]
Proposed patch: Remove redundant pam_set_item() call
Comment 2 SATOH Fumiyasu 2009-02-05 07:47:13 UTC
Created attachment 3922 [details]
Updated patch for 3.2.8 (v3-2-test)

Remove a redundant pam_set_item() call for a PAM
item "PAM_OLDAUTHTOK" because it was already stored
in _winbind_read_password() or a previous stacked
PAM module.
Comment 3 Karolin Seeger 2009-07-28 02:02:06 UTC
Raising version number as requested by the reporter.
Comment 4 Karolin Seeger 2009-09-29 03:12:53 UTC
Any volunteer to review the patches?
Would be nice to include the patches in 3.3.8.
Comment 5 SATOH Fumiyasu 2012-04-19 10:38:28 UTC
Created attachment 7469 [details]
Patch for Samba 3.6 (v3-6-test)
Comment 6 Björn Jacke 2016-12-08 17:18:41 UTC
Ira, Slow, Jiri: can you review/test this? I would be great to get this ancient bug with attached patch fixed finally...