tsig question (and documentation bug)

[raf]

On Fri, Aug 20, 2021 at 09:46:46PM +1000, raf via bind-users <bind-users at lists.isc.org> wrote:

> On Fri, Aug 20, 2021 at 09:33:01PM +1000, raf via bind-users <bind-users at lists.isc.org> wrote:
> 
> > Hi,
> > 
> > I want to use TSIG for zone transfers,
> > only allowing zone transfers to
> > particular IP addresses if they
> > possess the TSIG shared secret.
> > 
> > The documentation at:
> > 
> >   https://bind9.readthedocs.io/en/latest/advanced.html
> > 
> > has this section:
> > 
> >   5.5.4. TSIG-Based Access Control
> > 
> > which gives this relevant but non-obvious example:
> > 
> >   allow-update { !{ !localnets; any; }; key host1-host2. ;};
> > 
> > which somehow means localnets *and* possesses the shared secret.
> > 
> > I've found old tutorials online that recommend:
> > 
> >   allow-update { key "KEYNAME"; };
> > 
> > Because (they say) including the IP address (no mention
> > of nested negative boolean logic) allows the transfer
> > if *either* the address matches *or* the key is known.
> > 
> > To do what I want, do I need to have this:
> > 
> >   allow-transfer { !{ !IPADDR; any; }; key KEYNAME; };
> > 
> > where IPADDR is the address(es) of the secondary
> > (or the name of an acl containing the address(es)
> > of the secondary)?
> > 
> > And if so, do I really want to? I'd like to, but
> > that syntax is a bit gross. Maybe I'm being silly.
> > Maybe I should just rely on the possession of the key.
> > Thoughts?
> 
> Does this mean what I want?
> 
>   allow-transfer { !{ !IPADDR; !key KEYNAME; }; };
> 
>   meaning: not (not ipaddr or not possesses shared secret)
>   meaning: ipaddr and possess shared secret

That was wishful thinking. :-)

I've found a good explanation of the short-circuiting
behaviour of negation.

So I think this is what I need to restrict zone transfers
to a set of IP addresses that possess the TSIG key:

  acl secondaries { IPADDR1; IPADDR2; }; # Secondary servers
  acl transfer { IPADDR3; }; # Separate transfer-only server
  acl peers { secondaries; transfer; }; # The above combined
  acl strangers { !peers; any; }; # Everywhere else
  key tsigkey { algorithm hmac-sha512; secret "XXX"; };
  zone ZONE
  {
    ...
    allow-transfer { !strangers; key tsigkey; };
    also-notify { IPADDR3 key tsigkey; }; # IPADDR3 from acl transfer
  };

I'm expecting that this will work, even with the nested
acls, because named-checkconf doesn't complain.

However, I'd also like to replace the explicit IPADDR3
in the also-notify statement with a name to represent
the IPADDR3 in the acl "transfer", but named-checkconf
doesn't like anything I try. But, according to my
(perhaps incorrect) understanding of the documentation,
I should be able to do this.

Even though I knew it didn't make sense, I first tried
the acl name "transfer" but the parser thought that it
was a primaries list name ("unable to find primaries
list 'transfer'"), and it saw the key's name as an
unexpected token (more on that below).

Even though I didn't understand why you would notify
primaries (not important), I tried defining a primaries
list named "transfer", but the key's name was still
rejected as an "unexpected token".

Then I looked at the documentation. In reference.html
section 4.2.16.7. Zone Transfers, it says also-notify
defines a list of IP addresses, with an optional port
each, and an optional TSIG key each. It then says "In
place of explicit addresses, one or more named
primaries lists can be used". It doesn't explicitly say
whether or not a TSIG key can be used with each
primaries list. Perhaps it is trying to imply that it
can't, but it was ambiguous.

I thought the grammar would make it explicit:

  zone <string> [ <class> ] {
    ...
    also-notify [ port <integer> ] [ dscp <integer> ] {
      (
        <remote-servers> |
        <ipv4_address> [ port <integer> ] |
        <ipv6_address> [ port <integer> ]
      )
      [ key <string> ] [ tls <string> ];
      ...
    };
    ...
  }

I couldn't find a definition of <remote-servers>. At
first, I had assumed it meant an FQDN, but it doesn't.
I realised that it must refer to the name of a defined
primaries list.

Suggestion: Perhaps the grammar should make it clear
that the <remote-servers> is a primaries list name.

But more importantly, the grammar seems to say
(if <remote-servers> does mean a primaries list),
that a TSIG key can follow a primaries list.

But either named-checkconf doesn't agree with the
documented grammar, or I did something wrong. I had:

  primaries transfer { IPADDR3; };
  key tsigkey { algorithm hmac-sha512; secret "XXX"; };
  zone ZONE { ...; also-notify { transfer key tsigkey; }; };

Does that agree with the documented grammar?
Or did I do it wrong?

If it is true that a TSIG key can't follow a primaries list
name, I would expect the parser to complain about the "key"
token. But it complains about the key's name being an
unexpected token. So maybe it is me just not understanding
something about the notation used for the grammar, but it
seems clear enough.

Suggestion: (Unless I misunderstood the documentation)
change the documentation (Section 4.2.16.7 and the
also-notify grammar) to unambiguously match the
parser's behaviour, or better still, change the parser
to match the documented grammar and allow TSIG keys to
be used with primaries lists in also-notify statements.

So is it the documentation, or is it just me?

I've just worked out that the TSIG has to be in the
primaries list definition itself, which makes sense,
so this seems to keep named-checkconf happy:

  primaries notified { IPADDR3 key tsigkey; };
  key tsigkey { algorithm hmac-sha512; secret "XXX"; };
  zone ZONE { ...; also-notify { notified; }; };

I'd prefer it if I only needed one primaries list per
IPADDR, rather than one per IPADDR+TSIGKEY, and could
apply different TSIG keys to that primaries list,
because I use multiple keys for the same IPADDR3, but
that's OK.

But I think the also-notify grammar should be changed to:

  zone <string> [ <class> ] {
    ...
    also-notify [ port <integer> ] [ dscp <integer> ] {
      (
        <remote-servers> |
        <ipv4_address> [ port <integer> ] [ key <string> ] [ tls <string> ] |
        <ipv6_address> [ port <integer> ] [ key <string> ] [ tls <string> ]
      );
      ...
    };
    ...
  }

And maybe <remote-servers> could be changed to <primaries-list>
or similar.

cheers,
raf