Please add /etc/krb5.conf.d directory to the package and an include
directive in krb5.conf so that other packages can provide snippets under
the directory. For instance next FreeIPA version expects this directory
to exist.

control: -1 severity wishlist

    Timo> Please add /etc/krb5.conf.d directory to the package and an
    Timo> include directive in krb5.conf so that other packages can
    Timo> provide snippets under the directory.

Hi Sam,

Per our discussion today, unfortunately, it looks like include and
includedir support was merged into Heimdal, but hasn't yet made it into a
release.  It's on the master branch, but is not in 7.4.0.  But maybe soon?

Once that code is released, it looks like the way to include a directory
of fragments in Heimdal is:

    includedir /etc/krb5.conf.d

include also works, for single files.

There are some name restrictions, namely:

    Only files whose names consist only of alphanumeric characters,
    hyphen, and underscore, will be parsed, though files ending in ".conf"
    will also be parsed.

In hopes of honoring this request, I just looked at the heimdal sources
in debian.  I cannot find evidence of the includedir or include
krb5.conf directives there even in 2022.
Unless I'm missing something I still don't think it makes sense to add
this to Debian without heimdal support.

Sam Hartman <hartmans@debian.org> writes:

Yes, agreed.  Heimdal upstream seems to have stalled out entirely on
issuing new releases, although there's still active development going into
their main branch.

includedir is supported in current Heimdal master (added in 2017), but so
far as I can tell it's never made it into a release.

I'm also starting to feel the need to add this includedir directive to
/etc/krb5.conf by default. sssd is expecting this to work[1], as it places
a config snippet in /etc/krb5.conf.d.

I looked at the heimdal packages (which are in sync between ubuntu and
debian) and the includedir support is not there yet, indeed. But I found
this commit[2], and it looks like it's self contained. It applies with just
some offsets, and I'm testing a build. Would debian consider including that
as a patch? I think we would still need to consider handling of backupd
files (*~), renamed conf files by dpkg, etc, which I think also mit
kerberos doesn't exclude.


1. https://bugs.launchpad.net/bugs/2037321
2.,
https://github.com/heimdal/heimdal/commit/fe43be85587f834266623adb0ecf2793d212a7ca

I opened #1074775[1] to backport the heimdal patches that add include
and includedir support, filed a couple of salsa PRs[2][3] with tests,
and they were merged. Once there is a new upload of heimdal, we can
consider making this change in kerberos-configs then. What do you
think?


1. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1074775
2. https://salsa.debian.org/debian/heimdal/-/merge_requests/3
3. https://salsa.debian.org/debian/heimdal/-/merge_requests/4

Andreas Hasenack <andreas@canonical.com> writes:

I am in favor of making this change.  Thank you very much for clearing the
blocker in Heimdal.  This will, among other things, let me finally address
#756880[1].

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=756880

The change is not entirely trivial, however.  Here are some things that
come to mind that we probably need a plan for how to handle:

1. For already-configured systems, should we add the include directive to
   the existing krb5.conf file?  Presumably the answer is yes, or the
   migration is going to be rather difficult.  Is there a correct place in
   the krb5.conf file to add the include so that we get the correct
   semantics for whether fragments override the main file or vice versa?
   Are we going to break anyone's system by suddenly including the
   fragments?  We'll at least need a NEWS.Debian entry; maybe we also need
   a debconf warning in some situations?

2. With the current logic, it's not possible to guarantee that the include
   directive has been added, since krb5-config by design doesn't touch a
   krb5.conf file that's a symlink.  That means it's possible to have the
   latest version of everything installed and still not respect the
   configuration fragments.  Do we just live with this?  I'm nervous about
   moving critical configuration into a fragment when we can't guarantee
   that the fragment is loaded.  In the libpam-krb5 case, this can lead to
   a security vulnerability.

3. How do dependencies work?  This change to krb5-config will require a
   particular version of Heimdal, since earlier versions don't support
   include (and will this break Kerberos entirely if the include is
   present?).  But krb5-config can't depend on any specific Kerberos
   implementation, so I don't know how to represent this as a dependency.
   And what dependency should a package that wants to use included
   fragments have to ensure that those included fragments are loaded?

Hi,
(...)

Presumably yes, but we have to indeed think about it. Normal dpkg conf
prompts will apply here, unless we do something (smart?) in postinst.
update: just saw the krb5-config postinst, it indeed tries to handle
many cases, and this would be another one.

The included file needs to have the section specified, and the
includedir directive lives at the top without a section of its own,
that's how I have seen it being used so far (in MIT kerberos).
a) It's quite possible some users already have a
/etc/krb5.conf.d/foo.conf file that has been ignored so far, and will
now be included. That could lead to unexpected behavior, yes.

b) if old heimdal is installed, and confronted with a krb5.conf that
has a includedir line outside of a section (like, first line), it will
fail to parse the file and break ("binding before section" error).
That's what I've seen in my testing. I haven't tried inclusions from
other parts of krb5.conf.

We could grep for include/includedir in krb5.conf, be it a symlink or
not? What is the scenario where /etc/krb5.conf is a symlink, are some
sites doing that?

I see, so for example you will want to create a configuration snippet
to address #756880, but aren't sure if that file will even be included
because krb5.conf might not have the includedir directive.

Note we can now also include specific files, without it having to be a
whole directory, if this helps.

I was thinking about a breaks, as in, new krb5-config would break old heimdal.

Some virtual provides perhaps? Too much?

    Russ> Andreas Hasenack <andreas@canonical.com> writes:
    >> I opened #1074775[1] to backport the heimdal patches that add
    >> include and includedir support, filed a couple of salsa PRs[2][3]
    >> with tests, and they were merged. Once there is a new upload of
    >> heimdal, we can consider making this change in kerberos-configs
    >> then. What do you think?

    Russ> I am in favor of making this change.  Thank you very much for
    Russ> clearing the blocker in Heimdal.  This will, among other
    Russ> things, let me finally address #756880[1].

I am also in favor of this change.
Please let me know when you want me to look at kerberos-configs.
I believe I can prepare a patch there once Heimdal is ready.

    >> And what dependency should a package that wants to use included
    >> fragments have to ensure that those included fragments are
    >> loaded?

I don't think you can.
An administrator might remove the includedir.
krb5.conf might be a symlink.
I think the strongest hint you can make is breaks krb5-config <<
version.

At the moment, heimdal's verify_krb5_conf is already not happy with
krb5.conf shipped by bin:krb5-config:

# verify_krb5_conf
verify_krb5_conf: krb5_config_parse_file: open /root/.krb5/config: No
such file or directory
verify_krb5_conf: /libdefaults/ccache_type: unknown entry
verify_krb5_conf: /libdefaults/rdns: unknown entry
verify_krb5_conf: /realms/1TS.ORG/kdc: Temporary failure in name
resolution (kerberos.1ts.org)
verify_krb5_conf: /realms/1TS.ORG/admin_server: Temporary failure in
name resolution (kerberos.1ts.org)
^C
(remove 1TS.ORG and try again)

# verify_krb5_conf
verify_krb5_conf: krb5_config_parse_file: open /root/.krb5/config: No
such file or directory
verify_krb5_conf: /libdefaults/ccache_type: unknown entry
verify_krb5_conf: /libdefaults/rdns: unknown entry
root@o-heimdal:~# echo $?
1

I can grab a ticket, though:
root@o-heimdal:~# kinit andreas
andreas@LOWTECH's Password:
root@o-heimdal:~#

But if I add the includedir line, without support in heimdal, it
breaks immediately:
root@o-heimdal:~# mkdir /etc/krb5.conf.d
root@o-heimdal:~# sed -r -i '1 i\includedir /etc/krb5.conf.d\n' /etc/krb5.conf
root@o-heimdal:~# kinit andreas
kinit: krb5_parse_name_flags: unable to find realm of host o-heimdal
root@o-heimdal:~#

root@o-heimdal:~# head /etc/krb5.conf  -n 4
includedir /etc/krb5.conf.d

[libdefaults]
        default_realm = LOWTECH

Oddly enough, verify_krb5_conf is now happier:
root@o-heimdal:~# verify_krb5_conf
verify_krb5_conf: krb5_config_parse_file: open /root/.krb5/config: No
such file or directory
verify_krb5_conf: krb5_config_parse_file: /etc/krb5.conf:1: binding
before section
root@o-heimdal:~# echo $?
0

This was all with the normal heimdal package, not the patched one.

And, to reply to another question from before, if I put the includedir
directive inside a section, for example, inside [libdefaults], that's
invalid because it expects all entries in sections to be key=pair
values:
root@o-heimdal:~# head /etc/krb5.conf  -n 4

[libdefaults]
includedir /etc/krb5.conf.d
        default_realm = LOWTECH

root@o-heimdal:~# verify_krb5_conf
verify_krb5_conf: krb5_config_parse_file: open /root/.krb5/config: No
such file or directory
verify_krb5_conf: krb5_config_parse_file: /etc/krb5.conf:3: missing =

it allows that includedir inside a section. And indeed there are
questions of ordering and overrides then, to be better understood.

This config snippet:
[libdefaults]
default_realm = LXD

If I include it via this krb5.conf:
[libdefaults]
includedir /etc/krb5.conf.d
default_realm = LOWTECH

default realm is LXD.

If I include it like this:
[libdefaults]
default_realm = LOWTECH
includedir /etc/krb5.conf.d

Then default realm is LOWTECH.

I think it's best to have the includedir at the very top, outside any
section. Seems to be the least surprising.

Andreas Hasenack <andreas@canonical.com> writes:

Do both MIT and Heimdal use the same order (first seen wins)?  I hope so,
otherwise this is going to be tricky.

I think that's right.  That means that fragments will override anything in
the base /etc/krb5.conf, which feels correct to me.  We should add a
prominent comment to the top of the default /etc/krb5.conf that explains
this, as well as a NEWS.Debian entry.

Do both MIT and Heimdal sort the fragments alphabetically before including
them, so that there's some predictable order for which fragments override
each other?  We'll want to document the ordering.

Sam Hartman <hartmans@debian.org> writes:

I'm not sure what that means for https://bugs.debian.org/756880 then.  I
want to move the minimum_uid setting into a configuration fragment so that
people can easily change it as a conffile instead of having to stop using
pam-auth-update if they need to change it.  But if that setting is not
applied somehow, this may allow local root compromise in pathological
cases of Kerberos realms where people have unexpected principal names.

I guess I can yell really loud in NEWS.Debian and call that good enough.
Or add some postinst probe to make sure that the include is present,
although I hate doing that because it means debconf, translations, and all
of that machinery.

Of course, this work is not required to solve my problem in a separate
package.  :)  I was just hoping that it would.

Andreas Hasenack <andreas@canonical.com> writes:

Yeah, krb5-config is quite complicated and doesn't treat /etc/krb5.conf as
a typical Debian configuration file.  It would be really nice to find a
way to get more typical configuration semantics for /etc/krb5.conf, since
the level of complexity makes krb5-config changes fragile and hard to
test, but I don't have any great ideas for how to do that.

I think this we can probably say is okay to handle via NEWS.Debian.

I suspect there's at least one site somewhere that makes /etc/krb5.conf a
symlink to some file in AFS, but I have no idea how common this sort of
thing is.  Given that the local administrator can just replace the file
with one that doesn't have the includedir, I suppose that at some level
it's just another case of "the local administrator can break their own
system" and we mostly need to try to make people aware of it during
upgrades.

Yes, exactly.

I don't think that it does.  The file may or may not be there (lots of
people use Kerberos without using libpam-krb5), and there's still the
basic problem that we can't guarantee that the include directive is
present.

Ah, yes, that makes sense.  And then packages that need configuration
snippets can depend on the new version of krb5-config and that probably
makes all the right things happen.

Hi,

A quick test exactly like above with MIT kerberos 1.20.1 showed the
same result, i.e., first one wins.

Still on this topic, the unit tests added to heimdal also use the
include statement at the top, outside any section.

MIT:
The krb5.conf file can include other files using either of the
following directives at the beginning of a line:

          include FILENAME
          includedir DIRNAME

       FILENAME or DIRNAME should be an absolute path. The named file
or directory must exist and be readable.  Including a directory
includes all files within the directory
       whose names consist solely of alphanumeric characters, dashes,
or underscores.  Starting in release 1.15, files with names ending in
".conf" are also included, unless
       the name begins with ".".  Included profile files are
syntactically independent of their parents, so each included file must
begin with a section header.  Starting in
       release 1.17, files are read in alphanumeric order; in previous
releases, they may be read in any order.


Heimdal's ktb5.conf manpage (with the patches applied):

       Files and directories may be included by absolute path.
Including a directory causes all files in the directory to be included
as if each file had been included sep‐
       arately, but only files whose names consist of alphanumeric,
hyphen, and underscore are included, though they may also end in
'.conf'.

Heimdal doesn't mention ordering, so it's readdir() ordering, whatever that is:

+    if ((d = opendir(dname)) == NULL)
+        return errno;
+
+    while ((entry = readdir(d)) != NULL) {
(...)

MIT also does opendir/readdir, but at the end calls qsort to order the list:

https://github.com/krb5/krb5/blob/47371f6db423e7dba29afe696282ddfc0b92a81c/src/util/support/dir_filenames.c#L130

Andreas Hasenack <andreas@canonical.com> writes:

readdir ordering is probably bad.  I think that's essentially random on a
lot of file systems, and I'm not sure it's even guaranteed to be stable.
Is there any chance we could get that fixed in Heimdal before we start to
rely on it?

Hi,

I filed https://github.com/heimdal/heimdal/issues/1252 showing the problem

In heimdal, listing the directory and processing each file is done in
one function, there is no prior list of files to be ordered. MIT
kerberos creates a list first (maybe it doesn't support nesting, so
it's easier to create a list first).

Hello,

I suppose the handling of .d directories in the archive varies a lot.
Some might have explicit ordering, others (like heimdal) not.

The "normal" way we would be using this include mechanism is to add a
new section, add something to an existing section, or perhaps override
something. Always thinking about what's in the main config file. This
seems like it would work with the current patches, without surprises.

Having multiple snippets trying to override each other seems like a
bad idea in the first place. But it could happen, and that's where the
readdir() ordering becomes problematic.

We could consdier this ordering an open bug, and move forward, with
the expectation that it would be fixed someday. But we might end up in
the same place as this bug here, which was filed in 2017 and we still
don't have an upstream release with a fix 7 years later.

Hello,

reviving this old thread/bug. I tried reading through it all again.
We will go ahead with this change in Ubuntu. There is a PR up at [1].
- default installs ship with the includedir statement in krb5.conf
- upgrades will get the includedir statement added if and only if it wasn't
there before, commented out or not



2. With the current logic, it's not possible to guarantee that the include

I saw an answer to my question of "what is the scenario where
/etc/krb5.conf is a symlink?". Let me paste it below for context, from Russ:

So yeah, I agree it seems far fetched, and falls into the category "you
made local changes". But on the other hand, why do we care if
/etc/krb5.conf is a symlink or not? The includedir path is absolute, you
mean that in the symlink scenario it could be wrong to have snippets in
/etc/krb5.conf.d because we don't know where krb5.conf actually lives?
The PR adds a Breaks for the versions of heimdal and mit kerberos that do
not have includedir support. I'm unsure on which mit krb5 package to break,
should it be the library libkrb5-3?

Regarding the include ordering, heimdal has acked the bug and thinks it's
worth fixing, but need a PR. We will treat this lack of ordering as a bug
and process it, but won't block on it for this first change as we want to
meet the 26.04 LTS feature freeze date which is next week.

Many things were discussed in this bug, and we want to address them all as
much as possible, but I think it's best to make some progress here at
least. We will definitely miss something in these first iterations, but I
think it's a move in the right direction.

That being said, if we missed something catastrophic and obvious, or
anything else really, please shout :)


1.
https://code.launchpad.net/~bamf0/ubuntu/+source/kerberos-configs/+git/kerberos-configs/+merge/500077

One more thing that I forgot to add. We definitely want to work with debian
on this, as I think we all agreed that it benefits us both. Happy to create
PRs about it in salsa.

Andreas Hasenack <andreas.hasenack@canonical.com> writes:

I would love to get the same change into Debian, although Sam has been the
active maintainer for some time now and I would defer to him.

This sounds fine. It does mean that any files in that directory will
suddenly be processed when they weren't before, but that seems acceptable
(and indeed the whole point).

The reason why the symlink case is interesting is that it's not safe to
modify the file if it's a symlink. (It very well may not be possible,
which is the most likely case if it's a symlink into AFS.) In the current
implementation, that means that this change just won't be applied, which
probably warrants some sort of loud warning.

The other option would be to fail the upgrade of krb5-config if krb5.conf
is a symlink and thus stop supporting symlinks. I'm not sure if that would
break anyone's use case, but it would make the addition of the new
includedir more reliable.

I think that's correct.

Hi,

We can most definitely propose these changes to debian. I sent a follow-up
email after I noticed I missed making this important statement :)

Will add a note about this scenario as well.

Will add a note to NEWS about the symlink case as well.

I think failing the upgrade in this case would be harsh. But I don't yet
fully understand these scenarios. It's been a while since I last used AFS ;)
Thanks

Hi there!

I have created a merge request for this bug on Salsa, as this was recently
merged on Ubuntu. Feel free to let me know your thoughts:

https://salsa.debian.org/debian/kerberos-configs/-/merge_requests/4

Simon