After setting e.g. pdbedit -P "password history" -C 3 a subsequent run of pdbedit -Lv fails with pdb_set_pw_history: data_blob_talloc() failed! for every user in the SAM database (in a classic Samba NT PDC setup. An ADDS setup seems to be unaffected). It looks like the patch https://lists.samba.org/archive/samba-technical/2014-September/102319.html caused this change in behaviour. The issue is that data_blob_talloc returns by value, so after the call we still have old_nt_pw_his == &(sampass->nt_pw_his) and the subsequent call to data_blob_free(old_nt_pw_his) then sets sampass->nt_pw_his.length to 0 and frees the data. pdbedit worked again after recompiling with the patch reverted.
This bug prevented me from connecting to a share, on server installed with samba as PDC. I think it merits a higher priority.
Created attachment 10565 [details] Level 10 debug log of failed smbclient connection
Created attachment 10566 [details] Level 10 debug log of succesful smbclient connection after patch reverted
I've added two level 10 debug logs of Samba 4.2.0rc3, made during an smbclient connection to localhost. The command I ran was: smbclient //localhost/intranet -U ad%hallo123 -d 10 The first log is a log with plain Samba 4.2.0rc3, where the connection fails. The second log is a log of Samba 4.2.0rc3 with the patch that Arvid mentions reverted, where the connection succeeds.
(In reply to Roel van Meer from comment #4) Can you try the following fix from master? https://git.samba.org/?p=samba.git;a=commitdiff;h=966192ee16d6802da5c2b046d24 If guess we should backport that fix...
(In reply to Stefan (metze) Metzmacher from comment #5) It seems that fix is already present in 4.2.0rc3.
(In reply to Stefan (metze) Metzmacher from comment #5) > Can you try the following fix from master? > https://git.samba.org/?p=samba.git;a=commitdiff;h=966192ee16d6802da5c2b046d24 > If guess we should backport that fix... Ok, it won't fix it. Something like this should fix it: diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c index 1b716f4..5e162db 100644 --- a/source3/passdb/pdb_get_set.c +++ b/source3/passdb/pdb_get_set.c @@ -872,19 +872,20 @@ bool pdb_set_lanman_passwd(struct samu *sampass, const uint8 pwd[LM_HASH_LEN], e bool pdb_set_pw_history(struct samu *sampass, const uint8 *pwd, uint32_t historyLen, enum pdb_value_state flag) { + DATA_BLOB new_nt_pw_his = {}; + if (historyLen && pwd){ - DATA_BLOB *old_nt_pw_his = &(sampass->nt_pw_his); - sampass->nt_pw_his = data_blob_talloc(sampass, - pwd, historyLen*PW_HISTORY_ENTRY_LEN); - data_blob_free(old_nt_pw_his); - if (!sampass->nt_pw_his.length) { + new_nt_pw_his = data_blob_talloc(sampass, + pwd, historyLen*PW_HISTORY_ENTRY_LEN); + if (new_nt_pw_his.length == 0) { DEBUG(0, ("pdb_set_pw_history: data_blob_talloc() failed!\n")); return False; } - } else { - sampass->nt_pw_his = data_blob_talloc(sampass, NULL, 0); } + data_blob_free(&sampass->nt_pw_his); + sampass->nt_pw_his = new_nt_pw_his; + return pdb_set_init_flags(sampass, PDB_PWHISTORY, flag); }
(In reply to Stefan (metze) Metzmacher from comment #7) Tested, and I can confirm both problems are fixed. Thanks!
Created attachment 10576 [details] Patch for v4-2-test
Pushed to autobuild-v4-2-test.
Pushed to v4-2-test. Closing out bug report. Thanks!