Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Bug#989959: unbound: Corrupt/empty trust anchor file is not healed upon start

64 views
Skip to first unread message

Dennis Filder

unread,
Jun 16, 2021, 1:10:03 PM6/16/21
to
Package: unbound
Version: 1.13.1-1
Severity: normal
Tags: patch

I ran out of space on /var and unbound still tried to update the root
trust anchor file which ended up empty. Then later after reboot the
package-helper failed to detect and recover from that, and
unbound.service failed to start.

With the attached patch (which adds a rudimentary sanity check) and
freshly freed disk space unbound started normally. However, a better
solution might be to test more carefully for sufficient disk space
when making changes to the file or using 2 oversized files in rotation
and never truncating them.

Regards,
Dennis

P.S.: I also noticed that unbound.service under [Service] defines no
StateDirectory=/var/lib/unbound to ensure that it is mounted on start.
package-helper_heal_trust_file.diff

Michael Tokarev

unread,
Apr 18, 2022, 1:50:03 AM4/18/22
to
On Wed, 16 Jun 2021 18:57:26 +0200 Dennis Filder <d.fi...@web.de> wrote:
> Package: unbound
> Version: 1.13.1-1
> Severity: normal
> Tags: patch
>
> I ran out of space on /var and unbound still tried to update the root
> trust anchor file which ended up empty. Then later after reboot the
> package-helper failed to detect and recover from that, and
> unbound.service failed to start.
>
> With the attached patch (which adds a rudimentary sanity check) and
> freshly freed disk space unbound started normally. However, a better
> solution might be to test more carefully for sufficient disk space
> when making changes to the file or using 2 oversized files in rotation
> and never truncating them.

I wonder if we should address the root problem here instead of the
symptoms.

Maybe we can always update this (very small) file as a part of the
daemon startup procedure.

And/or, when doing this (it is just being copied to the chroot from
/usr/share/dns/root.key), unbound can do it atomically - copying it
to a temporary file and renaming it to the right place when done.
Unlike install(1), this will never result in having an incomplete
file in place (provided the filesystem is doing the right thing).

Yet another option is to compare the two files and copy over if the
files are different.

Neither of that requires verification of the validity of the file.
But it will surely change behavior if people used to keep their own
file in /var/lib/unbound/root.key - can this *ever* happen at all?
But this is not a conffile to begin with, if one wants to have their
own root.key, they sure can set ROOT_TRUST_ANCHOR_UPDATE=no in
/etc/default/unbound.

What do you think?

Thank you for the patch!

/mjt

> P.S.: I also noticed that unbound.service under [Service] defines no
> StateDirectory=/var/lib/unbound to ensure that it is mounted on start.

The chroot setup procedure has its own load of issues.

Michael Tokarev

unread,
Apr 19, 2022, 6:50:03 AM4/19/22
to
On Mon, 18 Apr 2022 08:43:24 +0300 Michael Tokarev <m...@tls.msk.ru> wrote:
..
> Maybe we can always update this (very small) file as a part of the
> daemon startup procedure.

It looks like I completely misunderstood this file purpose.
Am I right this is just the initial key and unbound updates
this key automatically by its own?

auto-trust-anchor-file: <filename>
File with trust anchor for one zone, which is tracked with RFC5011 probes.

Okay, it smells like it is, and it definitely it should not
be copied from /usr/share/dns/root.key..

I'll re-do my changes there (which I already comitted).

But now I've a question: how the initial problem happened?

The script (/usr/lib/unbound/package-helper) use install(1)
to update the file and to chown it (this also smells unsafe
from the security PoV). And install unlinks the destination
file first, creates destination file, writes contents to
it, and closes it. It looks like we should not use install(1)
here, or maybe install it to .tmp and mv it atomically, -
and from _there_, the problem will just go away.

/mjt

Dennis Filder

unread,
Apr 19, 2022, 4:50:03 PM4/19/22
to
I wasn't CC'ed, thus the late reply.

> But now I've a question: how the initial problem happened?
>
> Okay, it smells like it is, and it definitely it should not be
> copied from /usr/share/dns/root.key..

What can I say? My /var partition became full (too many .deb files
IIRC) and unbound.service wouldn't start because of an unusable trust
anchor file (which was empty). How exactly that happened I have no
idea. It may have been from unbound trying to replace root.key with
its own changes and failing, but looking at the code
(validator/autotrust.c:autr_write_file()) that should have left the
old version in place upon failure. So maybe it was from
package-helper trying to copy over the existing file for some other
reason. Maybe due to the missing StateDirectory=/var/lib/unbound
directive /var was not yet mounted when [ ! -e
"$ROOT_TRUST_ANCHOR_FILE" ] ran fooling the script into thinking it
should update that "missing" file, but became mounted before
/usr/bin/install was invoked which then failed due to missing disk
space leaving behind an empty file. The "-" in
"ExecStartPre=-/usr/lib/unbound/package-helper root_trust_anchor_update"
then resulted in this error being ignored which would have made it
impossible for me to learn what exactly went wrong. Again, I have no
way of reconstructing whether it happened like this, I'm just guessing
here.

As far as I see it a problem also was that (in the old version) after
the reboot the package-helper script only tried to overwrite
/var/lib/unbound/root.key when it was older than
/usr/share/dns/root.key or missing, but not when it was empty or
corrupted. Having package-helper automatically detect and try to
auto-repair that would be a nice convenience to have as then simply
restarting the unit would fix the problem.

> The script (/usr/lib/unbound/package-helper) use install(1) to
> update the file and to chown it (this also smells unsafe from the
> security PoV). And install unlinks the destination file first,
> creates destination file, writes contents to it, and closes it. It
> looks like we should not use install(1) here, or maybe install it to
> .tmp and mv it atomically, - and from _there_, the problem will just
> go away.
> ...
> Am I right this is just the initial key and unbound updates this key
> automatically by its own?

I think so, but I'm not sure. My knowledge of both unbound and the
details of DNSSEC is rather spotty.

> > P.S.: I also noticed that unbound.service under [Service] defines no
> > StateDirectory=/var/lib/unbound to ensure that it is mounted on start.
>
> The chroot setup procedure has its own load of issues.

I see. Maybe adding just

After=var.mount var-lib.mount var-lib-unbound.mount

would still be worth considering. It would cause unbound.service to
wait only if these mount units exist. And since /var/lib/unbound gets
installed as part of the package this should not cause any problems
even if unbound is run in chroot-mode.

Regards.

Michael Tokarev

unread,
Apr 19, 2022, 5:30:03 PM4/19/22
to
19.04.2022 23:38, Dennis Filder wrote:
> I wasn't CC'ed, thus the late reply.

I didn't know either, Dennis. I just replied to the bug :)

>> But now I've a question: how the initial problem happened?
>>
>> Okay, it smells like it is, and it definitely it should not be
>> copied from /usr/share/dns/root.key..
>
> What can I say? My /var partition became full (too many .deb files
> IIRC) and unbound.service wouldn't start because of an unusable trust

Yeah, I see it quite well now how exactly things went wrong, and
you confirm my thoughts.

And the root cause was using install/cp to create the file, --
neither of which do it atomically. So any error during the
copy and we'll have some bad file out there. Even for a small
file like this - today we have filesystems with inline data,
when small files are written directly into the directory,
and only for larger files a real extent is allocated, -
this root.key is enough to flip from inline to extent (which
we might not have).

> anchor file (which was empty). How exactly that happened I have no
> idea. It may have been from unbound trying to replace root.key with
> its own changes and failing, but looking at the code
> (validator/autotrust.c:autr_write_file()) that should have left the
> old version in place upon failure. So maybe it was from
> package-helper trying to copy over the existing file for some other

It definitely is. Because install/cp which was used.
...
> As far as I see it a problem also was that (in the old version) after
> the reboot the package-helper script only tried to overwrite
> /var/lib/unbound/root.key when it was older than

Because no one thought about possible partial file in there.
It should never be partial. And this update ensures just that.
Well, there still *is* a possibility here, but I don't know if
any real modern filesystem allows this: a file should be comitted
to disk before the final rename completes.

...
>> Am I right this is just the initial key and unbound updates this key
>> automatically by its own?
>
> I think so, but I'm not sure. My knowledge of both unbound and the
> details of DNSSEC is rather spotty.

It is the initial value and unbound will keep it updated.

>>> P.S.: I also noticed that unbound.service under [Service] defines no
>>> StateDirectory=/var/lib/unbound to ensure that it is mounted on start.

Hmm this is a good idea to add this.

>> The chroot setup procedure has its own load of issues.
>
> I see. Maybe adding just
>
> After=var.mount var-lib.mount var-lib-unbound.mount

I think StateDirectory helps here.

Thank you!

/mjt
0 new messages