Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc ccache fixes #1143

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Misc ccache fixes #1143

wants to merge 30 commits into from

Conversation

nicowilliams
Copy link
Contributor

@nicowilliams nicowilliams commented May 29, 2023

[@riastradh here's a work-in-progress to fix some of the issues you turned up regarding KRB5CCNAME.]

083c994019e I'll mostly want to keep even if we switch to renaming the SQLite3 symbols.

This PR has a bunch of things:

  • finally makes it possible to configure different defaults for all the otherwise-hard-coded default cache names
    • this is done via adding a [libdefaults] default_ccache_name_by_type parameter below which are per-cache type alternative default cache names
    • this makes it possible to -finally!- ensure that the tests do not write to the user's credentials caches
  • completes previously half-baked support for non-default cache collections
    • adds krb5_cc_cache_match2() and krb5_cccol_cursor_new2() which take an optional collection name argument (<TYPE>:<name>)
    • enhances krb5_cc_new_unique() so that either its type or hint argument can be cache names and will be used to create the new [unique] cache in the named collection
    • extends krb5_cc_ops with several new methods to support all of this
    • repurposes the get_default_cache() method of krb5_cc_ops to have it return the token-expanded hard-coded default cache for that type, and adds a new get_primary_cache() method to return the current "switched" primary cache for a given collection (this is WIP not in this PR yet)
    • changes uses of krb5_cc_new_unique(), krb5_cc_cache_match in-tree to do use this new feature
  • klist now knows that X-RMED-CONF: is the realm of cc config entries that have been removed and no longer shows those as >>>Expired<<<
  • fixes FILE, DIR, and SCC bugs galore
    • klist -l output for DIR is nice now
  • significantly enhances the SCC cache type
    • it is now indexed, which should support huge credentials caches better
    • it uses WAL mode
    • it cleans out expired non-cc-config entries sooner
    • uses FOREIGN KEYs
  • documents credentials cache syntax, which is <TYPE>:[collection][:subsidiary] for all in-tree cache types except MEMORY (which is just MEMORY:<name> or MEMORY:anonymous) and FILE (which is FILE:[collection][+subsidiary])
    • as implied it is possible to use a bare cache type as a KRB5CCNAME value or -c option argument (e.g., FILE, DIR, ...) and the expected default will be used
    • ditto FILE:, DIR:, etc.
  • improves tests/kdc/check-cc.in and lib/krb5/test_cc.c
  • updates SQLite3 in-tree
  • versions SQLite3 symbols in-tree (though we'll probably want to rename all the sqlite3 symbols anyways)

TODO:

Anything else will go into a separate PR:

  • fix collection cache iteration start method to take a collection name so that when iterating a non-default collection we don't end up iterating the default one instead
  • rethink resolve_2() method and splitting of subsidiary cache names upstairs
  • rethink escaping of filesystem-related characters in subsidiary names upstairs (rethinking done: we'll move this into the DIR and FILE cache types, thus allowing SCC, KCM, and API to have arbitrary subsidiary names w/o aliasing concerns)

@riastradh
Copy link

Thanks, I'll give it a whirl when I have a chance.

FYI (not sure where best to discuss this so just dropping it here), the main reason I was looking into this credential cache syntax -- other than to see whether the sqlite3 ccache was worth keeping or could be disabled to sidestep the symbol collision issues of https://gnats.NetBSD.org/57406 altogether -- was to see whether it would be reasonable to pick DIR or SCC as the default credential cache type instead of FILE so kinit could just work out of the box to log into multiple realms at the same time.

Right now I do KRB5_CONFIG=/home/riastradh/foo/krb5.conf KRB5CCNAME=/tmp/riastradh/krb5cc.foo kinit riastradh@FOO.COM and KRB5_CONFIG=/home/riastradh/bar/krb5.conf KRB5CCNAME=/tmp/riastradh/krb5cc.bar kinit riastradh@BAR.COM, but that's a mouthful to maintain for every kinit and every kerberized client, and I'd like it to work out of the box with none of that effoort.

So, before spending effort on tweaking the compile-time defaults, I thought I'd test the usability of DIR and SCC caches, with whatever is the default location for them, by setting KRB5CCNAME=DIR: or KRB5CCNAME=SCC:. Which, as you now know, didn't work out very well!

@riastradh
Copy link

I skimmed through the changes, haven't processed anything yet -- but I think the most important thing here is to have extensive automatic tests for all the kinds of settings that one might find in the environment and/or krb5.conf.

For the tests that are already in here, one appears to be failing:

FAIL: test_cc
=============

test_cc: krb5_cc_gen_new: MEMORY: Refusing to create a new unique cache in the default collection for cache type MEMORY because it is not the type of the default cache
FAIL test_cc (exit status: 1)

@nicowilliams
Copy link
Contributor Author

Thanks, I'll give it a whirl when I have a chance.

Oh, it's not ready...

FYI (not sure where best to discuss this so just dropping it here), the main reason I was looking into this credential cache syntax -- other than to see whether the sqlite3 ccache was worth keeping or could be disabled to sidestep the symbol collision issues of https://gnats.netbsd.org/57406 altogether -- was to see whether it would be reasonable to pick DIR or SCC as the default credential cache type instead of FILE so kinit could just work out of the box to log into multiple realms at the same time.

Ah, well, in Heimdal the FILE cache type supports collections!

Try kinit --default-for-principal "$principal" then klist -l with the FILE ccache and with enable_file_cache_iteration = yes in [libdefaults], you'll see!

The context here is that the whole ccache collection concept that was grafted onto the krb5 API has been... somewhat unfinished this whole time, and this work is part of finishing it. Among the things that needed to be done to finish it was to teach the FILE cache type to be a collection cache type, and also to add a convention for naming sub-caches after the principals whose credentials they are intended to hold.

@nicowilliams
Copy link
Contributor Author

[...] but I think the most important thing here is to have extensive automatic tests for all the kinds of settings that one might find in the environment and/or krb5.conf.

I've not gotten to that yet. I don't have a lot of time or energy right now to write extensive tests, but the two tests we have of ccaches have a good enough framework for adding more.

@riastradh
Copy link

Ah, well, in Heimdal the FILE cache type supports collections!

Try kinit --default-for-principal "$principal" then klist -l with the FILE ccache and with enable_file_cache_iteration = yes in [libdefaults], you'll see!

I see. My hope is that in NetBSD 11 (or maybe even 10) a user can just write kinit me@FOO.COM and kinit me@BAR.COM and run klist and it will all be there, and any kerberized/GSSAPI application like Firefox will seamlessly work for either realm, with no fiddling of knobs or extra arguments. How difficult do you imagine is the path to that future?

For now, I tried the configuration and command-line option, and this is what happened:

$ cat /tmp/krb5.conf
[libdefaults]
	enable_file_cache_iteration = yes
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/kinit --default-for-principal riastradh@A.EXAMPLE.COM
riastradh@A.EXAMPLE.COM's Password:
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/kinit --default-for-principal riastradh@B.EXAMPLE.COM
riastradh@B.EXAMPLE.COM's Password:
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist
klist: No ticket file: /tmp/krb5cc_1000
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist -l
   Name     Cache name     Expires
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist -A
$ 

If I set KRB5CCTYPE=DIR it seems to work a little better, although plain klist gives no indication that there may be multiple principals available and there's still weird expired krb5_ccache_conf_data tickets I mentioned in #1109:

$ KRB5_CONFIG=/tmp/krb5.conf KRB5CCTYPE=DIR prefix/bin/kinit --default-for-principal riastradh@A.EXAMPLE.COM
riastradh@A.EXAMPLE.COM's Password: 
$ KRB5_CONFIG=/tmp/krb5.conf KRB5CCTYPE=DIR prefix/bin/kinit --default-for-principal riastradh@B.EXAMPLE.COM
riastradh@B.EXAMPLE.COM's Password: 
$ KRB5_CONFIG=/tmp/krb5.conf KRB5CCTYPE=DIR prefix/bin/klist
Credentials cache: DIR::/tmp/krb5cc_1000_dir/tkt.riastradh@A.EXAMPLE.COM
        Principal: riastradh@A.EXAMPLE.COM

  Issued                Expires               Principal
May 29 23:11:09 2023  May 30 23:11:09 2023  krbtgt/A.EXAMPLE.COM@A.EXAMPLE.COM
May 29 23:11:09 2023  >>>Expired<<<         krb5_ccache_conf_data/start_realm@X-RMED-CONF:
$ KRB5_CONFIG=/tmp/krb5.conf KRB5CCTYPE=DIR prefix/bin/klist -l
   Name                            Cache name                                                    Expires
 riastradh@A.EXAMPLE.COM         FILE:/tmp/krb5cc_1000_dir/tkt.riastradh@A.EXAMPLE.COM         May 30 23:11:09 2023
 riastradh@B.EXAMPLE.COM         FILE:/tmp/krb5cc_1000_dir/tkt.riastradh@B.EXAMPLE.COM         May 30 23:11:22 2023
$ KRB5_CONFIG=/tmp/krb5.conf KRB5CCTYPE=DIR prefix/bin/klist -A
Credentials cache: FILE:/tmp/krb5cc_1000_dir/tkt.riastradh@A.EXAMPLE.COM
        Principal: riastradh@A.EXAMPLE.COM

  Issued                Expires               Principal
May 29 23:11:09 2023  May 30 23:11:09 2023  krbtgt/A.EXAMPLE.COM@A.EXAMPLE.COM
May 29 23:11:09 2023  >>>Expired<<<         krb5_ccache_conf_data/start_realm@X-RMED-CONF:
Credentials cache: FILE:/tmp/krb5cc_1000_dir/tkt.riastradh@B.EXAMPLE.COM
        Principal: riastradh@B.EXAMPLE.COM

  Issued                Expires               Principal
May 29 23:11:22 2023  May 30 23:11:22 2023  krbtgt/B.EXAMPLE.COM@B.EXAMPLE.COM
May 29 23:11:22 2023  >>>Expired<<<         krb5_ccache_conf_data/start_realm@X-RMED-CONF:

I suspect that KRB5CCTYPE=DIR will have advantages over KRB5CCTYPE=FILE, by narrowing any iteration to a single directory that the user owns.

@riastradh
Copy link

[...] but I think the most important thing here is to have extensive automatic tests for all the kinds of settings that one might find in the environment and/or krb5.conf.

I've not gotten to that yet. I don't have a lot of time or energy right now to write extensive tests, but the two tests we have of ccaches have a good enough framework for adding more.

Understood, didn't mean to direct your time! If I find time perhaps I can contribute to that.

@nicowilliams
Copy link
Contributor Author

nicowilliams commented May 30, 2023

I see. My hope is that in NetBSD 11 (or maybe even 10) a user can just write kinit me@FOO.COM and kinit me@BAR.COM and run klist and it will all be there, [...]

That's there now even for FILE! But you do have to klist -l. I'm not sure how much we can change the default klist output... You know how it goes: people script around these tools even though they shouldn't because we didn't give them good porcelain to begin with (there's now decent JSON output support in klist).

[...] and any kerberized/GSSAPI application like Firefox will seamlessly work for either realm, with no fiddling of knobs or extra arguments. How difficult do you imagine is the path to that future?

Ah, well, that is a bit of another story. This is known as the "credential selection problem". If one knows the target service's realm a priori (either because it was furnished by the application or because we have a local [domain_realm] mapping for it) and that realm matches one of the client principals', then it's obvious what to do: use the matching credential. For all other cases it's much less obvious what to do.

Suppose you have no relevant [domain_realm] entries and that you have credentials for

  • joeuser@A
  • joeuser/admin@A
  • joeuser@B
  • joeuser@C

and you want to authenticate to host@somehost, now what?

Well, we could try all the credentials in... some order. How do we make that order deterministic? And what if you don't want to use admin credentials to authenticate to that service?

Keep in mind that the modern approach to determining a service's realm is to chase referrals from the client's "start realm" (i.e., the realm for which the client has a root krbtgt, where "root krbtgt" means that it's for and issued by the same realm). So we get to avoid an insecure DNS lookup for the host's realm, but then we also don't get to find the service's realm before trying to use a client credential, which means we have to try [a subset of] all the credentials we have in some canonical or configured order.

So it's complicated. The best we could do besides trying them all in C collation order is to have preferences recorded somewhere. MIT and Heimdal have been punting on this for decades, though MIT has done more work in this space than Heimdal.

It's not just Kerberos that has this problem -- PKI, JWT, you name it, they all have this problem.

And we also have a desire to support the most minimal local configuration possible -- zeroconf even. There's too many conflicts and too much mind reading to do here.

@nicowilliams
Copy link
Contributor Author

Understood, didn't mean to direct your time! If I find time perhaps I can contribute to that.

Help would be appreciated, and we really appreciate your contributions so far! If I'm devoting some energy to this right now it's because a) I happen to need to add some functionality to Heimdal for $WORK, b) that coincides in time with your contributions, c) I don't want to discourage you.

@nicowilliams
Copy link
Contributor Author

Ugh. There's several problems with the SCC cache type. For example, when we resolve a cache we recall it's rowid in the caches table, but then maybe we'll destroy or re-initialize it, and then that rowid will no longer be valid but we use it in the process of initializing which... is therefore racy and unsafe.

@nicowilliams
Copy link
Contributor Author

nicowilliams commented May 30, 2023

This is still a work in progress. This now includes a significant set of [untested!] fixes to the SCC cache type:

  • stop using row IDs -- that was causing race conditions
  • use SQL to create random() (SQL function, not C) "new unique" caches, and use RETURNING
  • stop using sqlite3_changes() and sqlite3_last_insert_rowid() now that we can use RETURNING
  • add a server column to the credentials table so we can optimize scc_remove_cred() and implement a fast scc_retrieve()
  • use FOREIGN KEYs to avoid the need for triggers and to simplify ccache detroy/initialize functions

In the process I'm upgrading SQLite3 in-tree to 3.42.0.

TODO:

  • add an index on credentials (service)! (DONE)
  • finish the rototill of scc_remove_cred() (needs testing)
  • implement an scc_retrieve()! (now that we have a server name in the credentials table...) (needs testing)
  • make lib/krb5/test_cc pass
  • make tests/kdc/check-cc pass
  • change configure.ac so that use of the in-tree SQLite3 is now mandatory
  • maybe rename all the SQLite3 symbols in lib/sqlite to avoid conflicts
  • fix up lib/sqlite/version-script.map to account for the upgrade of SQLite3
  • add error checking of sqlite3_bind_text() calls
  • audit the code for more bugs and design problems

@nicowilliams
Copy link
Contributor Author

@riastradh it might be really nice to have a set of simple rules one could express for credential selection, rules of the form [service], hostname_fqdn_suffix, preferred_client_realm, preferred_number_of_client_principal_components, then one could write rules for the above joeuser example as follows:

  • {"*", ".c.test.h5l.se", "B", 1}
  • {"*", ".b.test.h5l.se", "C", 1}
  • {"kadmin", "primary-kdc.test.h5l.se", "A", "2"}
  • {"", "A", 1}

I think something like that could have value, at least as a starting point. I decidedly do not have the energy or mandate to work on this.

@nicowilliams nicowilliams force-pushed the misc-ccache-fixes branch 3 times, most recently from 416d721 to ea8a8eb Compare May 30, 2023 21:31
@riastradh
Copy link

I see. My hope is that in NetBSD 11 (or maybe even 10) a user can just write kinit me@FOO.COM and kinit me@BAR.COM and run klist and it will all be there, [...]

That's there now even for FILE! But you do have to klist -l.

Are you sure?

$ cat /tmp/krb5.conf
[libdefaults]
	enable_file_cache_iteration = yes
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/kinit riastradh@A.EXAMPLE.COM
riastradh@A.EXAMPLE.COM's Password:
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist
Credentials cache: FILE:/tmp/krb5cc_1000
        Principal: riastradh@A.EXAMPLE.COM

  Issued                Expires               Principal
May 30 16:46:32 2023  May 31 16:46:32 2023  krbtgt/A.EXAMPLE.COM@A.EXAMPLE.COM
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist -l
   Name     Cache name     Expires
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist -A
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/kinit riastradh@B.EXAMPLE.COM
riastradh@B.EXAMPLE.COM's Password:
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist
Credentials cache: FILE:/tmp/krb5cc_1000
        Principal: riastradh@B.EXAMPLE.COM

  Issued                Expires               Principal
May 30 16:46:59 2023  May 31 16:46:59 2023  krbtgt/B.EXAMPLE.COM@B.EXAMPLE.COM
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist -l
   Name     Cache name     Expires
$ KRB5_CONFIG=/tmp/krb5.conf prefix/bin/klist -A
$ 

I'm not sure how much we can change the default klist output... You know how it goes: people script around these tools even though they shouldn't because we didn't give them good porcelain to begin with (there's now decent JSON output support in klist).

Fair enough. I'll settle for working klist -l or klist -A or whatever it is! (awk-friendly output would be nice too, if I could have a pony!)

[...] and any kerberized/GSSAPI application like Firefox will seamlessly work for either realm, with no fiddling of knobs or extra arguments. How difficult do you imagine is the path to that future?

Ah, well, that is a bit of another story. This is known as the "credential selection problem". If one knows the target service's realm a priori (either because it was furnished by the application or because we have a local [domain_realm] mapping for it) and that realm matches one of the client principals', then it's obvious what to do: use the matching credential. For all other cases it's much less obvious what to do.

Suppose you have no relevant [domain_realm] entries and that you have credentials for

  • joeuser@A

  • joeuser/admin@A

  • joeuser@B

  • joeuser@C

and you want to authenticate to host@somehost, now what?

Well, we could try all the credentials in... some order. How do we make that order deterministic? And what if you don't want to use admin credentials to authenticate to that service?

My expectation, from blind intuition without consulting any code or standards or documentation or past heated debates from mid-1990s mailing list archives:

  1. Find somehost's realm -- either by domain-to-realm mapping in local configuration, or by DNS domain-to-realm mapping (unless it's disabled in the config). If no realm, fail.
  2. If there's exactly one principal in that realm with an unexpired ticket, use that.
  3. If there's more than one principal in that realm, whatever was most recently kinited and has yet to expire or something.

In case (3), I'm not sure the fine details of automagic default selection matter so much as providing a way for the user to specify a principal as the default for that realm, like kswitch jruser/admin@EXAMPLE.COM -- just like you have to type su or sudo for local privileged operations.

I'm sure I've missed lots of issues in more complex scenarios like cross-realm setups, but presumably whatever more complex rules handle these more complex scenarios could reduce, in the simpler scenario I sketched, to easy predictable rules like the ones above.

Of course, there's also a good chance that I missed some problems that are obvious to people who have been steeped in Kerberos development for ages, but I hope that if so, there's a good reason for such problems to get in the way of relatively simple scenarios, rather than simply that the rules above don't give a unique solution for more complex scenarios.

(I'm somewhat deliberately being naive/obtuse here because my goal is to make easy things work predictably, rather than setting all the knobs and options to do exactly what I want each time like I used to do for many years, so I'm trying to do the obvious thing and then seeing what actually falls apart when I try it. If nothing else, I hope this results in having any difficulties with the obvious thing get documented!)

Keep in mind that the modern approach to determining a service's realm is to chase referrals from the client's "start realm" (i.e., the realm for which the client has a root krbtgt, where "root krbtgt" means that it's for and issued by the same realm). So we get to avoid an insecure DNS lookup for the host's realm, but then we also don't get to find the service's realm before trying to use a client credential, which means we have to try [a subset of] all the credentials we have in some canonical or configured order.

What's the harm of a DNS lookup here? I see three possible negative outcomes from it:

  1. Forgery leading to denial of service. Adversary convinces client to try a service ticket that somehost won't actually accept. But an adversary could have denied service by returning NXDOMAIN for the A record too, so I don't think this is worse.
  2. Forgery leading to confusion. Adversary convinces client to try a service ticket that somehost accepts, so client is legitimately authenticated, but as a principal other than what they expected. It is possible to imagine contrived scenarios where this is a problem, but you could get into the same problem by, e.g., browsing just before and just after expiry of your tickets for one principal. I'm having a hard time imagining where this could get you into trouble that you weren't asking for by having all the tickets around in the first place.
  3. Privacy leaks. Not actually sure offhand whether anything could leak here other than the fact that the user has a ticket, but let's suppose something does leak. In general I think preventing this requires disabling the use of DNS altogether, just like Tor-based browsers do, and I think for privacy it is reasonable to require setting an appropriate domain-to-realm mapping in the local configuration for the sites you do want to leak anything to (i.e., authenticate to).

And we also have a desire to support the most minimal local configuration possible -- zeroconf even. There's too many conflicts and too much mind reading to do here.

I certainly appreciate the desire for minimal or zero configuration, and that's what I'm aiming for too -- perhaps to ship an empty /etc/krb5.conf in NetBSD, or to safely obviate the need for having an /etc/krb5.conf at all. But I also expect that for certain purposes -- privacy of user's identity when browsing to non-kerberized sites, privacy of user's location when browsing to kerberized sites approved by the user -- some local configuration is needed. And I expect that for users juggling multiple identities within a realm, some extra interaction like kswitch jruser/admin@EXAMPLE.COM is reasonable.

Random thought: /etc/krb5.conf could start with a line saying what version it's based on and take all reasonable defaults from that version. New installations with /etc/krb5.conf created afresh with the new version line would get reasonable secure defaults without forcing users and admins to find all the obscure knobs to twiddle the right way; upgraded installations from old versions that lack a version line or still refer to an old version line wouldn't be impacted, but could be secured by updating the version line.

@nicowilliams
Copy link
Contributor Author

Are you sure?

Ugh, it used to work, and I'll be fixing it.

@nicowilliams
Copy link
Contributor Author

diff --git a/lib/krb5/fcache.c b/lib/krb5/fcache.c
index d94b0f48460..e147a4adfb7 100644
--- a/lib/krb5/fcache.c
+++ b/lib/krb5/fcache.c
@@ -1377,7 +1377,7 @@ fcc_get_cache_first(krb5_context context, krb5_cc_cursor *cursor)
         goto out;
     }

-    if (is_def_coll) {
+    if (is_def_coll && def_locs) {
         /* Since def_ccname is in the `def_locs', we'll include those */
         iter->locations = def_locs;
         free(iter->def_ccname);

fixes it, though I'll do better in a bit.

@nicowilliams
Copy link
Contributor Author

My expectation, from blind intuition without consulting any code or standards or documentation or past heated debates from mid-1990s mailing list archives:

1. Find somehost's realm -- either by domain-to-realm mapping in local configuration, or by DNS domain-to-realm mapping (unless it's disabled in the config).  If no realm, fail.

The new hotness is to chase referrals and never bother doing DNS domain-to-realm mapping. This means relying on KDCs' [domain_realm] rules and not on DNS. It's a good thing, except that you lose the ability to make client credential selection decisions based on the host's realm.

In principle you can disable referrals chasing by using [libdefaults] name_canon_rules with the no_referrals option, but looking at the code I think that must be broken because it merely causes the client to not set the canonicalize flag on the TGS-REQ instead of going through the get_cred_kdc_capath() code path... Another thing to fix. Still, I much prefer using referrals to not using referrals.

2. If there's exactly one principal in that realm with an unexpired ticket, use that.

3. If there's _more than one_ principal in that realm, whatever was most recently kinited and has yet to expire or something.

The "one with non-expired tickets" thing makes for surprises.

In case (3), I'm not sure the fine details of automagic default selection matter so much as providing a way for the user to specify a principal as the default for that realm, like kswitch jruser/admin@EXAMPLE.COM -- just like you have to type su or sudo for local privileged operations.

kswitch is terrible because it's so global.

The point of creating a convention for naming caches in a collection after the client principal whose credentials they contain is to make it possible to just type KRB5CCNAME=...:${USER}@${OTHER_REALM} ..., or to cut-n-paste from klist -l output and have it be obvious when you look at the comment which cache that is -- compare to the new_unique convention that yields randomized ccache names!

@nicowilliams
Copy link
Contributor Author

FILE and SCC as a collection should be working again in this PR, though I'm only testing by hand right now.

Next up: fix DIR, which too is broken.

@nicowilliams
Copy link
Contributor Author

OK, DIR should be working to in this PR now, but still I've only done manual testing.

@nicowilliams
Copy link
Contributor Author

lib/krb5/test_cc now passes.

When KRB5CCNAME was set (or a default ccache was set via
krb5_cc_set_default_name()) don't search any other collections in
krb5_cc_cache_match() or krb5_cccol_last_change_time().

Otherwise one could `kinit -c $SOME_NON_DEFAULT_CCACHE ...` and have
kinit look for and find a suitable ccache in some other collection.

The point of searching all collections is that if we know they're all
relevant in the current context, then we can save the user (or login
apps) having to set and coordinate KRB5CCNAME, which would be a
significant UI/UX improvement.  But we can only do that when we know
that a ccache [collection] was not named either via KRB5CCNAME nor more
explicitly.

Well, _if_ the ccache named via KRB5CCNAME or otherwise happens to be
the same as the default ccache for that cache type, then perhaps we
should still search the other cache types' default collections too, but
that will require more work.
Calling krb5_cc_new_unique() with a non-NULL `type` argument that is not the
same as the type of the default ccache (when that ccache is set via
`KRB5CCNAME` or `krb5_cc_set_default_name()`) is currently surprising: it will
create a ccache in the _default_ collection for the requested type, but if the
user set `KRB5CCNAME` to a non-default ccache then this will be clearly not
desired.

To fix this we'll:

a) let `krb5_cc_new_unique()` extract a collection name from its `type` argument,
   if present
b) let `krb5_cc_new_unique()` use its `hint` argument as a collection name for
   the given collection `type`
c) add a new `gen_new_2()` method to krb5_cc_ops that takes a collection name
   argument
d) change all in-tree uses of `krb5_cc_new_unique()` to pas a collection name
   where possible

TODO:

 - add the new `gen_new_2()` method to all in-tree cache types
 - change all in-tree uses of krb5_cc_new_unique() as described above
 - write a test
This function should be used in-tree and in ccache plugins to eliminate
duplicate code and make cache name parsing more consistent so that we
can have `KRB5CCNAME=<TYPE>` and `KRB5CCNAME=<TYPE>:` work consistently
for all cache types and denote "use the default collection name for
<TYPE>.

All cache types in-tree support collections nowadays, even FILE.
External cache plugins may not and need not.  To support collections a
cache must support the `set_default()` method of `krb5_cc_ops` -- see
`krb5_cc_support_switch()`.
 - Use WAL mode
 - Disable sync writes
 - Revamp the SQL schema
 - Replace triggers with foreign keys
 - Revamp scc_remove_cred()
 - Add scc_retrieve_cred()
 - When there is no default ccache, make the next one written to the default
   (maybe this is not a good idea?)
…one is (WIP)

I've noticed that `klist -l` doesn't show which ccache is the primary
one.  This is fundamentally because we don't have a method for getting
the primary ccache.  This commit prepares for adding that.
@nicowilliams
Copy link
Contributor Author

Using krb5_cc_parse_name() does simplify FILE a wee bit, and... maybe I can simplify DIR, we'll see.

@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jun 22, 2023

Using krb5_cc_parse_name() in the cache resolve methods really does not simplify things enough to be worthwhile. Especially not DIR, not as long as we support DIR::path/tktNAME syntax, and since MIT does, we must too.

netbsd-srcmastr pushed a commit to NetBSD/src that referenced this pull request Oct 11, 2023
SCC is not usable in Heimdal 7.8.0, and this brings a dependency on
libsqlite3 into libkrb5 and therefore libgssapi, which is problematic
downstream applications that have sqlite3 from pkgsrc or statically
built in.

SCC will undergo substantial revision in the next Heimdal version
(heimdal/heimdal#1143).  We can revisit later
how to deal with this -- perhaps by symbol-renaming a copy of sqlite3
in Heimdal as it looks like upstream intends to do.

PR lib/57406

XXX pullup-10
netbsd-srcmastr pushed a commit to NetBSD/src that referenced this pull request Oct 14, 2023
	share/mk/bsd.prog.mk: revision 1.346
	crypto/external/bsd/heimdal/sbin/kstash/Makefile: revision 1.6
	share/mk/bsd.prog.mk: revision 1.347
	crypto/external/bsd/heimdal/sbin/hprop/Makefile: revision 1.6
	crypto/external/bsd/heimdal/sbin/kdc/Makefile: revision 1.6
	crypto/external/bsd/heimdal/sbin/iprop-log/Makefile: revision 1.6
	crypto/external/bsd/heimdal/lib/libkrb5/Makefile: revision 1.16
	crypto/external/bsd/heimdal/libexec/digest-service/Makefile: revision 1.6
	crypto/external/bsd/heimdal/Makefile.inc: revision 1.10
	crypto/external/bsd/heimdal/Makefile.inc: revision 1.11
	crypto/external/bsd/heimdal/Makefile.inc: revision 1.9
	crypto/external/bsd/heimdal/libexec/kadmind/Makefile: revision 1.8
	crypto/external/bsd/heimdal/lib/libhdb/Makefile: revision 1.6
	crypto/external/bsd/heimdal/include/config.h: revision 1.12
	crypto/external/bsd/heimdal/libexec/hpropd/Makefile: revision 1.6
	crypto/external/bsd/heimdal/libexec/ipropd-slave/Makefile: revision 1.6
	crypto/external/bsd/heimdal/libexec/ipropd-master/Makefile: revision 1.6
	crypto/external/bsd/heimdal/libexec/kpasswdd/Makefile: revision 1.6
	crypto/external/bsd/heimdal/sbin/kadmin/Makefile: revision 1.7

heimdal: Disable sqlite3 credential cache (SCC).
SCC is not usable in Heimdal 7.8.0, and this brings a dependency on
libsqlite3 into libkrb5 and therefore libgssapi, which is problematic
downstream applications that have sqlite3 from pkgsrc or statically
built in.
SCC will undergo substantial revision in the next Heimdal version
heimdal/heimdal#1143.  We can revisit later
how to deal with this -- perhaps by symbol-renaming a copy of sqlite3
in Heimdal as it looks like upstream intends to do.
PR lib/57406

bsd.prog.mk: krb5 stuff no longer needs to link against sqlite3.
(Why is this here?  Seems like it should be a .mk fragment under
crypto/external/bsd/heimdal -- that way I would have found it for the
previous commit.)
PR lib/57406

heimdal: No need for -lm, was only added for -lsqlite3.
PR lib/57406

heimdal: Make sure whatever uses libhdb also gets libsqlite3 & libm.
Loose ends for static builds in the fix for PR lib/57406.
rokuyama pushed a commit to IIJ-NetBSD/netbsd-src that referenced this pull request Oct 26, 2023
SCC is not usable in Heimdal 7.8.0, and this brings a dependency on
libsqlite3 into libkrb5 and therefore libgssapi, which is problematic
downstream applications that have sqlite3 from pkgsrc or statically
built in.

SCC will undergo substantial revision in the next Heimdal version
(heimdal/heimdal#1143).  We can revisit later
how to deal with this -- perhaps by symbol-renaming a copy of sqlite3
in Heimdal as it looks like upstream intends to do.

PR lib/57406

XXX pullup-10
rokuyama pushed a commit to IIJ-NetBSD/netbsd-src that referenced this pull request Oct 26, 2023
	share/mk/bsd.prog.mk: revision 1.346
	crypto/external/bsd/heimdal/sbin/kstash/Makefile: revision 1.6
	share/mk/bsd.prog.mk: revision 1.347
	crypto/external/bsd/heimdal/sbin/hprop/Makefile: revision 1.6
	crypto/external/bsd/heimdal/sbin/kdc/Makefile: revision 1.6
	crypto/external/bsd/heimdal/sbin/iprop-log/Makefile: revision 1.6
	crypto/external/bsd/heimdal/lib/libkrb5/Makefile: revision 1.16
	crypto/external/bsd/heimdal/libexec/digest-service/Makefile: revision 1.6
	crypto/external/bsd/heimdal/Makefile.inc: revision 1.10
	crypto/external/bsd/heimdal/Makefile.inc: revision 1.11
	crypto/external/bsd/heimdal/Makefile.inc: revision 1.9
	crypto/external/bsd/heimdal/libexec/kadmind/Makefile: revision 1.8
	crypto/external/bsd/heimdal/lib/libhdb/Makefile: revision 1.6
	crypto/external/bsd/heimdal/include/config.h: revision 1.12
	crypto/external/bsd/heimdal/libexec/hpropd/Makefile: revision 1.6
	crypto/external/bsd/heimdal/libexec/ipropd-slave/Makefile: revision 1.6
	crypto/external/bsd/heimdal/libexec/ipropd-master/Makefile: revision 1.6
	crypto/external/bsd/heimdal/libexec/kpasswdd/Makefile: revision 1.6
	crypto/external/bsd/heimdal/sbin/kadmin/Makefile: revision 1.7

heimdal: Disable sqlite3 credential cache (SCC).
SCC is not usable in Heimdal 7.8.0, and this brings a dependency on
libsqlite3 into libkrb5 and therefore libgssapi, which is problematic
downstream applications that have sqlite3 from pkgsrc or statically
built in.
SCC will undergo substantial revision in the next Heimdal version
heimdal/heimdal#1143.  We can revisit later
how to deal with this -- perhaps by symbol-renaming a copy of sqlite3
in Heimdal as it looks like upstream intends to do.
PR lib/57406

bsd.prog.mk: krb5 stuff no longer needs to link against sqlite3.
(Why is this here?  Seems like it should be a .mk fragment under
crypto/external/bsd/heimdal -- that way I would have found it for the
previous commit.)
PR lib/57406

heimdal: No need for -lm, was only added for -lsqlite3.
PR lib/57406

heimdal: Make sure whatever uses libhdb also gets libsqlite3 & libm.
Loose ends for static builds in the fix for PR lib/57406.
@riastradh
Copy link

What can be done to unwedge this branch? Some other changes are blocked on this, like #1167. I forgot what the issues were holding this one up (other than circular tuit shortage).

@nicowilliams
Copy link
Contributor Author

@riastradh er, well, I guess I need a code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache collections APIs and commands only support default collections
2 participants