key version number in fill_mem_keytab_from_secrets()

[Stefan Metzmacher]

Hi Pavel,

> 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)

This is intended, we could also use 0 for all of them,
the key is that we should not care about the kvno, as there is
no reliable way to get the kvno.

We the acceptor should just iterate overall keys (for the given
encryption type) in the whole keytab and try to decrypt the ticket.

If I remember correctly that's what MIT krb5 does and for
Heimdal it's a bit random if it iterates the keytab, I think
it iterates as long as the kvno didn't match, so it's better to use
numbers which are very unlikely to be used by the KDC and for AD
the kvno starts with 1 for the first password.
I also created a GSS_KRB5_CRED_ITERATE_ACCEPTOR_KEYTAB_X for heimdal
to ensure that it iterates the whole keytab, but it's not yet upstream,
see https://github.com/heimdal/heimdal/pull/656

Also note that Samba asks gss_accept_sec_context() to ignore the
SPN values in the keytab, as it's also not possible to know all
possible names, which are available on the DC.

> 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.

No, info->next_change->password might be in progress for 4 reasons:
1. An error that happens before the password change reached the dc
2. The DC rejected the password change
3. The DC changed the password, but the success response didn't reach us
4. We we're not able to store the success (e.g. a ctdb failure in the cluster)

But we can't really know/should not care about the difference between these
reasons. And for 1. and 3. it's not possible to find the difference.
So we must be prepared to accept tickets encrypted with the in progress
password. Naturally we also need to accept all tickets with all other passwords
we know about, as ticket lifetimes are typically 10h and we have to deal with
replication latency.

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

The acceptor should ignore the kvno, I'm not how
gss_accept_sec_context works in other applications,
but for samba's usage I guess the iteration works
(apart from heimdal without GSS_KRB5_CRED_ITERATE_ACCEPTOR_KEYTAB_X).

> --------------------------------------------------
> 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.

I guess '< kvno -1' comes from times before we stored secrets_domain_infoB
in secrets.tdb. Today would be '< kvno -3'...

> 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)?

I think we want the kvno values to stay below 1 in order to have
heimdal keep iterating the keytab.

I'd do this (with kvno = 0):
- remove all old values with kvno - 3
- add all new values (if any) for kvno - 3
- remove all old values with kvno - 2
- add all new values (if any) for kvno - 2
- remove all old values for kvno - 1
- add all new values for kvno - 1
- remove all old values for kvno
- add all new values for kvno

The worst thing that can happen is that some keys are stored under two kvno values,
but we're sure at any given time the old values of kvno, kvno-1 and kvno-2 are still
in the keytab and we're ready to decrypt incoming tickets.


I think 'net ads keytab create' asks a DC about the msDS-KeyVersionNumber, which
is typically (but not reliable and always) the kvno. We may asks a DC that doesn't have
the current password yet, or it already has the in progress password already or the DC always returns 1,
see [MS-ADTS] 3.1.1.4.5.16 msDS-KeyVersionNumber:
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-adts/cb85ccdf-6469-42d5-a61c-ebae09b72e9d

Maybe we need an option to specify if the kvno should be fetched from the DC
for 'net ads keytab ' commands.

The kvno (as well as SPNs) might be important for some applications so
we better keep the default behavior for the 'net ads keytab' commands.

For legacy reasons this is all really complex in order to be compatible
with all sorts of situations and setups.

I hope I had some useful comments for you...

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20210630/a050f821/OpenPGP_signature.sig>