goakerberosidentity: Fix automatic reinitialization

See merge request !117

At the moment, the identity service doesn't recognize when a credentials
cache gets kdestroy'd explicitly by the user. It knows when a principal
is purged from all credential caches, but it doesn't know when a
specific cache is removed.

This means it doesn't fall back properly to an older credential crash if
the active cache gets destroyed.

This commit addresses that problem by reachitecting things a bit.

Previously, cache updates were processed by creating a new transient
GoaKerberosIdentity and then merging it into an existing identity using
goa_kerberos_identity_update. Using a full blown GoaKerberosIdentity
object as a wrapper around a lone credentials cache is kind of a weird
pattern and doesn't facillate processing cache removal, since we can't
create a transient identity for what's not there.

This commit exports goa_kerberos_identity_add_credentials_cache as
public api as an alternative to the merging transi...

The identity service has the ability to automatically fetch a new ticket
granting ticket from the KDC when the existing one expires, provided the
user keeps their kerberos password in GNOME keyring.

Unfortunately, commit aca400799c225a84e5d0fc90efb206c8f1d48bc3
inadvertently broke this feature in some cases.

When deciding whether or not to make a new credentials cache for a
principal the active one it looks at various characteristics of the
competing credentials to decide which cache is better.

For instance, if one credentials cache has a ticket that's valid and
signed in, but the other credentials cache only has an expired ticket,
then obviously the one that's valid and signed in gets picked to be
active.

Likewise, if one is expiring in 10 minutes and one is expiring in
24 hours, the one that expires in 24 hours will be treated as better.

This comparison, only makes sense, though when looking at two different
credentials caches.  If we're updating a preexisting credentials cache,
then we're actually just comparing up to date data with out of date
data.  In that case, we need to proceed even if new newer view of the
credentials look worse than the older view of those credentials.
Unfortunately, the buggy commit neglected to account for that.

This commit fixes that problem and related problems, by more
thoroughly and systematically checking all the permutations of
credentials in the old credentials cache for the identity, the
new credentials cache for the identity, and the default credentials
cache. It also adds a lot more logging for clarity.

Carmina16 authored Dec 27, 2022 and Administrator committed Dec 27, 2022

(cherry picked from commit 4a126ea5)

Explicitly switch to credentials cache when needed

See merge request !116

If we're updating a credentials cache and decide
it should be the new default for an identity, and
the old credentials cache was the default cache
for the cache collection then we should make the
new credential cache the default cache for the
collection, too.

This commit adds that. It also makes the new
credentials cache the default if there wasn't a
valid default set already. This brings consistency
to differences in behavior from different kerberos
ccache types.

Right now when erasing an identity we erase the
active credentials first and then the inactive
ones.

We neglect to take the active one out of the hash
table, though, so it gets destroyed twice.

This commit fixes that.

kerberos-identity: Unbreak handling of fresh caches

See merge request !115

The update_identity function is supposed to transfer the identity
form one object to another.

In practice, this is currently always a noop because only objects
with the same identities get copied to each other.

Nevertheless, there is a bug in the function. It grabs the identity
from the target object instead of from the source object.

This commit fixes that.

commit 4acfcc32 attempted to
avoid an error variable getting stomped all over by returning
FALSE when encountering the error.

Unfortunately, it's actual legitimate for an error to happen
in that path and we should proceed anyway.

That can happen when a credential cache is new and not yet
initialized, so it won't have a principal associated with it
yet.

This commit changes the problematic code to just pass NULL
for the error variable, since we don't need it.

kerberos-identity: Drop the weird ampersand

See merge request !114

This commit drops an unnecessary and non-idiomatic ampersand.

kerberos-identity: Add missing locking

See merge request !113

commit c492cbfd introduced some
code to goa_kerberos_identity_update that's not protected by the
identity lock.

It really should be.

This commit fixes that.

kerberos-identity: Attempt to cope with multiple credential caches per identity

Closes #79

See merge request !112

When the identity service does a refresh, it creates a new temporary
identity object to check the credentials, then it merges that
temporary identity into the preexisting identity object (so the
pointers don't change).

This has the unfortunate side-effect of arming expiration alarms in
the temporary object, that can then fire immediately before the object
is thrown out.

This commit disarms those alarms so they don't fire needlessly.

At the moment the identity service assumes there will just be one
credential cache collection for any given prinicipal.

This isn't necessarily true though, and the service gets quite
confused when that assumption doesn't hold up.

This commit attempts to make it cope with the situation better, by
maintaining a hash table of collections per identity.  It deems
one of the collections the "active" one and relegates the rest to
be backup if the active one expires and can't be renewed.

Closes: #79

kerberos-identity: Ensure idles queued to main thread are property synchronized

Closes #160

See merge request !109

identity: Don't add temporary accounts for expired credentials

Closes #32

See merge request !106

The identity service creates a "temporary" kerberos account when
a user manually does kinit, to handle automatic renewal, etc.

Unfortunately, it also picks up expired cruft that builds up in
KCM based credential caches, and creates temporary accounts for
that as well.

This commit tries to avoid that.

Closes #32

Resolve "Change ownCloud/Nextcloud base path to /remote.php/dav/"

Closes #67

See merge request !110

The new endpoints are "/dav/" for both. Adapt to it, as the "/caldav/" and
"/carddav/" endpoints do not offer the same features and they might be
possibly dropped in the future.

Closes #67

Kerberos identities are refreshed on a helper thread, and the state of
those identities are exported over the user bus on the main thread.

Since the main consumer of an identity's properties is the bus service
running on the main thread, to simplify things, property notifications
are dispatched from the main thread as well (even though the underlying
state is changed on a worker thread).

The mechanism to dispatch property notifies to the main thread is an
idle handler. The logic for doing the dispatch has a concurrency
bug however. In order to coaelsce multiple notifies that happen in
quick succession, the dispatch code checks for a preexisting idle id
associated with the given property. That idle id is set from the worker
thread when the idle is queued, and it's cleared from the main thread
when the idle is dispatched. The bug is that the main thread could in
theory clear the idle id immediately after the worker thread decided
there was already a notify queued, leading to a notify getting
completely dropped.

This commit addresses the bug by adding appropriate locking.

Closes #160

This reverts commit e30df2e4

kerberos-identity: Ensure idles queued to main thread are property synchronized

Closes #160

See merge request !108

Kerberos identities are refreshed on a helper thread, and the state of
those identities are exported over the user bus on the main thread.

Since the main consumer of an identity's properties is the bus service
running on the main thread, to simplify things, property notifications
are dispatched from the main thread as well (even though the underlying
state is changed on a worker thread).

The mechanism to dispatch property notifies to the main thread is an
idle handler. The logic for doing the dispatch has a concurrency
bug however. In order to coaelsce multiple notifies that happen in
quick succession, the dispatch code checks for a preexisting idle id
associated with the given property. That idle id is set from the worker
thread when the idle is queued, and it's cleared from the main thread
when the idle is dispatched. The bug is that the main thread could in
theory clear the idle id immediately after the worker thread decided
there was already a notify queued, leading to a notify getting
completely dropped.

This commit addresses the bug by adding appropriate locking.

Closes #160

kerberos-identity: Fail initialization if an identifier can't be found

See merge request !107

The inability to get an identifier already leads to an error.
Continuing beyond that point can lead to the verification_error trying
to clobber it.

!107

kerberos-identity: Clarify and remove redundancy from the renewal errors

See merge request !105

When krb5_get_renewed_creds fails to talk to the Kerberos Key
Distribution Centre (or KDC) because of network problems,
krb5_get_error_message translates the krb5_error_code as:
  Resource temporarily unavailable

This ends up in the debug logs as:
  GoaKerberosIdentityManager: could not renew identity: Could not get
    new credentials to renew identity FOO@BAR.ORG: Resource temporarily
    unavailable
  GoaIdentityService: could not renew identity: Could not get new
    credentials to renew identity FOO@BAR.ORG: Resource temporarily
    unavailable

It's not immediately clear that the 'resource' that's 'temporarily
unavailable' is actually the KDC, which would make it easier to
understand that there is a network problem.

Therefore, mention KDC in the error string from krb5_get_renewed_creds
and make it match the documentation [1] of that function because it
makes the code self-documenting.

[1] https://web.mit.edu/kerberos/krb5-devel/doc/appdev/refs/api/krb5_get_renewed_creds.html

#160

The "Could not renew identity" segment in the error strings from
goa_kerberos_identity_renew is redundant because the caller already
knows that this function is about renewing an identity and any failure
means just that.  In fact, it's callers in GoaIdentityService and
GoaKerberosIdentityManager already prepend a similar segment when using
the error strings.

Debug logs are already verbose enough.  It's better not to make it even
more difficult to follow them through needless redundancy.

When reporting errors from krb5_cc_get_principal, it's good to make the
error string match the documentation [1] of that function because it
makes the code self-documenting.

[1] https://web.mit.edu/kerberos/krb5-devel/doc/appdev/refs/api/krb5_cc_get_principal.html

#160