allow-update in global options (was Re: bind and certbot with dns-challenge)

[Timothe Litt]

Data points:

I saw another report of this issue on gitlab - #913 just after my
previous note.  It indicated that a distributions initial configuration
breaks with the change.  I see that it has been updated by Alan since.

I checked my configuration files.

I use allow-update-forwarding at the options level.

I use update-policy at the zone level.

I don't currently use either at the view level.

So my configurations would break.  (I haven't had the cycles to run
9.13, unfortunately for you - apparently, fortunately for me :-)

I don't see the serious harm in allowing these options to be inherited -
there are certainly other options that, if incorrectly/accidentally
inherited, could be dangerous.  Allow-transfer; allow-query,
deny-answer-* I could go on alphabetically, but I'm pretty sure a case
could be made for the majority of options causing mischief if
inadvertently inherited.

I'm curious about why these particular options were singled out -- yes,
they update persistent zone data.  But denial of service, information
leaks, and using the wrong directories can also be serious.

In any case, where a change is made that invalidates existing
configurations, I strongly prefer a deprecation warning at least one
(non-development) release prior.  With documentation.

Given that these prerequisites didn't happen in this case, I believe
that regardless of the merits, the previous behavior should be reinstated.

If there is a determination that the benefits of the change outweigh the
costs, then add a deprecation warning a stable release prior (perhaps
now?) and update the documentation -- including the ARM & release notes.

Also, the same arguments should be applied to all the other inheritable
options -- if there is justification for other changes, it's much better
to force operators to make a bundled set of changes than to dribble them
out piecemeal.

FWIW: In general, I choose to place configuration statements at the
level resulting in the shortest configuration.  (Not for performance,
but for clarity/ease of maintenance.)  So that's sometimes "global
enable, exception disable", and sometimes the inverse.  (This can be
frustrated when there's no obvious inverse to a directive, but that's
for another day.)

Finally, I looked at the 9.13 ARM for a list of which options are
allowed in the view statement.  The view Statement Grammar lists
[view_option; ...] - 'view_option' appears nowhere else in the ARM.  The
definition and usage section (in chapter 5) says only: "Many of the
options given in the *options* statement can also be used within
a *view* statement,".  To find an explicit list, one has to go to the
VIEW section of chapter 8 (the man page for named.conf) - which isn't
tagged with 'view_option'.  This frustrates searchers and people
unfamiliar with the ARM structure.  Note that allow-update and
allow-update-forwarding both appear as valid in the view syntax there,
although in chapter 5 the descriptions on p.97 says "only zone, not
options or view".

My 3.5¢ (USD, but your local currency will do :-)

Timothe Litt
ACM Distinguished Engineer
--------------------------
This communication may not represent the ACM or my employer's views,
if any, on the matters discussed. 

On 17-Mar-19 16:37, Alan Clegg wrote:
> On 3/17/19 2:51 PM, Alan Clegg wrote:
>> On 3/17/19 7:13 AM, Stephan von Krawczynski wrote:
>>> Hello all,
>>>
>>> I am using "BIND 9.13.7 (Development Release) <id:6491691>" on arch linux. Up
>>> to few days ago everything was fine using "certbot renew". I had
>>> "allow-update" in nameds' global section, everything worked well. Updating to
>>> the above version threw a config error that "allow-update" has no global scope
>>> and is to be used in every single zone definition.
>> And you may have found a bug.  I'm checking internally at this time.
> So, after a discussion with one of the BIND engineers this afternoon,
> this turned out to be quite an interesting and deep-rooted issue.
>
> During a cleanup of other code (specifically named-checkconf), code was
> changed that enforced what was believed to have been the default
> previously: specifically, allow-update was only allowed in zone stanzas.
>  The chain of changes follows:
>
> 5136.   [cleanup]       Check in named-checkconf that allow-update and
>                         allow-update-forwarding are not set at the
>                         view/options level; fix documentation. [GL #512]
>
> This, if the change remains, will be updated to [func] and additional
> documentation will be added to the release notes.
>
> The other changes down this long and twisting passage are:
>
> 4836.   [bug]           Zones created using "rndc addzone" could
>                         temporarily fail to inherit an "allow-transfer"
>                         ACL that had been configured in the options
>                         statement. [RT #46603]
>
> and
>
> 2373.  [bug]           Default values of zone ACLs were re-parsed each
>                        time a new zone was configured, causing an
>                        overconsumption of memory. [RT #18092]
>
> It turns out that this series of changes, taken as a whole, removed
> allow-update as a global option.
>
> The question now becomes:  Is there a need for the ability to apply
> allow-update across all zones in your configuration?
>
> Smaller operators should be able to add the allow-update per zone very
> easily, and large operators should (probably) be doing things at a much
> more granular level.
>
> I'm sure that there will be internal discussion here at ISC regarding
> this topic over the next week.
>
> We are hoping to release 9.14.0 soon and this would be an "allowed"
> change (at a .0 release).  If we don't make this change in 9.14.0, it
> won't be allowed in an official production release until 9.16.0 due to
> the "no changes to the configuration file except at .0 releases" rule.
>
> At this moment, I (personally) believe that the change is fixing a bug,
> as "allow-update" was not planned and is not a good idea at the global
> option level.  I believe that it should be allowed only in zone stanzas.
>
> If you have thoughts/opinions, please let us know!
>
> Alan Clegg, ISC
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.isc.org/pipermail/bind-users/attachments/20190317/76afb178/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.isc.org/pipermail/bind-users/attachments/20190317/76afb178/attachment.bin>