key version number in fill_mem_keytab_from_secrets()

[Pavel Filipensky]

Hi Stefan,

can you please help me and answer my questions related to feature
'Implement 'update keytab' for winbind and tools' (
https://gitlab.com/samba-team/samba/-/merge_requests/1999).

You commented in MR 1999:
*I also fail to see where we add the keys for the old(er) passwords.
Shouldn't we just dump the keys from in memory keytab (which is filled from
the precalculated hashed of secrets.tdb) into the file keytab.*
--------------------------------------------------
A) fill_mem_keytab_from_secrets()
--------------------------------------------------
I have checked the code in fill_mem_keytab_from_secrets() and what it does
is (code is simplified):

krb5_kvno kvno = 0; /* FIXME: fetch current vno from KDC ? */
fill_keytab_from_password(krbctx, *keytab, princ, kvno, info->password);
fill_keytab_from_password(krbctx, *keytab, princ, kvno - 1,
info->old_password);
fill_keytab_from_password(krbctx, *keytab, princ, kvno - 2,
info->older_password);
fill_keytab_from_password(krbctx, *keytab, princ, kvno - 3,
info->next_change->password);

I see two issues here:
1) kvno is incorrectly initialized and keys are added with unreal values:
vno=0
vno=4294967295  (-1)
vno=4294967294  (-2)
vno=4294967293  (-3)   ###  btw. the semantics of next_change is more (kvno
+ 1) than (kvno - 3)

2) info->next_change->password should not be passed to
fill_keytab_from_password().
next_change, if exists, refers to a password change in progress, and should
not be used before it is accepted and finalized, then it will become
info->password and only then it will be added to keytab.

Q: How is it possible that it works with wrong vnos?

--------------------------------------------------
B) ads_keytab_create_default()
--------------------------------------------------
this is the code I use to implement keytab update. It does:

* ads_get_service_principal_names() and adds keys for all SPNs via
ads_keytab_add_entry()
* adds keys for sam_account_name and UPN via ads_keytab_add_entry()
* It stores SPNs from keys with (kt_entry.vno != kvno) into array
oldEntries and for whole array ads_keytab_add_entry(ads, res,
oldEntries[i], false) is called

Every call of ads_keytab_add_entry() also calls
smb_krb5_kt_seek_and_delete_old_entries(), which deletes all keys with vno
< vno -1 (i.e. vno -2, vno -3, ....) for given principal.
So it should also ensure that old (but not older password and it's keys)
are in keytab.

Q: Do we also need the older password and why? If needed, isn't it better
just to avoid its removal from keytab, instead of filling it from
secrets.tdb (or from in memory keytab)?


Cheers,
Pavel