NTS client configuration support has landed

[Eric S. Raymond]

Gary E. Miller via devel <devel at ntpsec.org>:
> Yo Eric!
> 
> On Fri, 1 Feb 2019 16:53:37 -0500
> "Eric S. Raymond" <esr at thyrsus.com> wrote:
> 
> > Gary E. Miller via devel <devel at ntpsec.org>:
> > > > If there's a good semantic reason to have a separate nts
> > > > statement, I can do that.  
> > > 
> > > The options for the 'nts' and 'server' statements in the config file
> > > will be different.  Trying to merge them will confuse everyone.  
> > 
> > Heh.  The "confuse" ship has long since sailed, Gary.  The server,
> > pool, and refclock declarations are handled by the same portion of the
> > grammar - the only reason they look distinct is that I created
> > refclock as a synonym for "server" to make the mess a bit
> > more readable.  
> 
> Right, and if we use the 'nts' keyword, that is no longer the case.

Incorrect.  A new declaration head keyword by itself cannot fix any
problem at all. Not the problem you think you have, and no other
problem either.

You have misunderstood the constraints.  No blame attaches,
it's not an easy thing to wrap your head around unless you're very
accustomed to using and abusing parser technology, and understand the
slightly odd way that the NTP parser maps grammar to configuration.

So I'm going to explain it in detail now.

There are two stages of processing that translate from ntp.conf syntax
to configuration state that the code actually uses.  One level grovels
through the syntax using a yacc-generared parser and collects stuff
from it into intermediate configuration blocks - these are just C
structures.  Another layer does some simple operations on the
intermediate configuration blocks to initialize things like the peer
structures.

For our purposes in this discussion, the only part of the grammar
we care about is the server, refclock, pool, and (in one future) nts
declarations.  The only structure we care about is struct peer, which
is the protocol machine's state 

Here's a diagram:

    +-----------------+
    | ntp.conf syntax |
    +-----------------+
             |
             | <----------------- Yacc-generated parser
	     |
    +--------------------+
    |configuration blocks|
    +--------------------+
             |
	     |<------------------- Fancy copy operation.
	     |
   +---------------------+
   |struct peer instances|
   +---------------------+


At present, server/pool/refclock all use the same branch of the yacc
grammar; thus they take a set of options that is grammatically identical.
You can, for example, specify a 'refid' string for a server, or an outgoing
'version' for a refclock, even though doing either of those things
doesn't make any semantic sense.  The parser doesn't care.

This is slightly odd because normally configuration languages try to
be more restrictive at the grammar level in order to generate better
error messages.  But the NTP parser won't complain when you specify that
'refid' string for a server.

The documentation *pretends* that the different declaration types have
different option subgrammars, but it's lying by omission. Like I said,
the parser doesn't care.  It just takes any option values you feed it
and stuffs them into a configuration block associated with the
hostname in the declaration.  The configuration block has slots in it
for every possible option in all three declaration types.  The
declaration type (server/refclock/pool) is just another slot in that
block.

A later stage of configuration walks through the configuration blocks
and massages them ("Fancy copy operation") into instances of the
internal peer structure that NTP uses.

Yes, these declaration types are *semantically* different.  All three
use different subsets of the universal option set in the config block.
But nothing in the code actually cares or knows which subsets those
are until the protocol machine starts picking parameters out of the
peer structures, well after config time.

Adding an option is, in this context, generally something you do by
adding a token to an alternatives list.  That *is* a one-keyword
change. Trivial.  Turns into a new slot in the config block, or maybe
a new bit in a flag mask.

Adding a new declaration head *can* be a "just add a keyword" change,
but only if you're willing to re-use the source-declaration branch of
the grammar.  I did that when I added "refclock" - it was added to the
alternative list that previously contained just server and pool. It's
a head-fake, really - doesn't do anything different from "server", just
there to make the config language more readable.

If we decide to create an entire new branch of the grammar off of nts
as a declaration head, yes, that could be done. In that case it could
have an option set grammatically (not just semantically) different
from the server/pool/refclock branch.

But that's a considerably bigger deal than adding a keyword. And
by the time I finish this explanation I hope you will understand
that it would be a fairly pointless thing to do.

> Major simplification.

Nope. Not at all. Not on any of the three levels we can consider.

Implementation: Adding a new branch to the grammar would add
significant implementation complexity. *Re-using* the existing
declaration grammar adds almost none.

Documentation: Remember, the visible difference is "nts <hostname>" vs.
"server <hostname> nts". You're not going to persuade me the first is
major simplification over the second; it isn't

Kind semantics: I'll get back to that.

> > In fact grammatically they all take the same options
> 
> No.  There are at least 5 new options for the nts.  

You're not speaking the same language I am.  Let's get synchronized.

I believe you that nts needs 5 new semantically new options.
I've only seen specs for two of them ("ask" and "require")  and
a hint of a third ("noval"), I'll readily take it on faith there will
be two more.

But I were to add the "nts" declaration type you want, the natural way
to do it would be to add it to the head alternates list, then add your
five new options to the one declaration grammar branch that uses those
alternates.  (In fact I've already added ask and require.)

At that point, all four of the declaration types would grammatically
have all the same options, *including the new ones*.  They would just
turn into 5 more peer-structure slots for the protocol machine to ignore
when it's processing for anything but an nts-enabled server.

The only way to avoid this would be for me to go out of my way to create
distinct grammar branches for *each declaration type*.

That's not a crazy idea.  It's something I'd do if we decided we cared a
lot about giving useful error messages when a user does something like
declaring a refid for a peer or a version for a refclock.  It would add
complexity, though.

I don't think we need to care that much.  There's zero evidence in all
the NTP issues I've looked at that any user has ever said "I mistakenly
put a refid option on a server declaration and ntpd didn't complain 
and ZOMG THAT!S A BUG!!"

Considering that we're talking a quarter century of road miles...well,
I'm going to want to actually *see* a bug report like that before
I incur the complexity cost to prevent it.

> Worse, some of the options mean different things for server and nts.

Yes, just as some of the existing options mean different things for server
vs. refclock.

The grammar is honey badger, it don't care.  The fancy copy just
dumps the bits into peer structures and lets the protocol machine
associate any semantics with them it wants.

The documentation already lies about which declaration types
grammatically accept which options - it's actually telling you subset
and semantics while pretending to describe syntax.  More lies of that
kind seem unlikely to do more harm than the existing lies have, which
is zero.

And all this has exactly *nothing* to do with whether "server
<hostname> nts" or "nts <hostname>" is better design.  Either can be
implemented without any serious change to the config framework.  I
explained all that to you only so you can stop believing things that
are (a) not true, and (b) would be irrelevant to the design question
even if they were true.

> > - it's just that
> > the config builder selectively ignores various option subsets
> > depending on the entry type.  It's not really a very well constructed
> > grammar; makes for poor error messages.
> 
> Exactly.  So don't do that!

Too late.  Code has already been that way for *decades*.  Simpler to
go with the flow. Or do *you* want to rewrite the whole freaking grammar
without any evidence that there is an actual problem here?

> Worse, confusing to the parser means confusing to the user.

Normally, in a grammar with tighter well-formedness checking at the
syntactic level, you'd be right to think so.  Not in this case

In this case, because of the way it's implemented, those issues are
completely orthogonal to each other. They're kept orthogonal by the
amount of lying by insinuation that the config documentation does.

> > To "deconfuse" the situation I'd have to tear down the grammar and
> > redesign it in a way that wouldn't be backwards-compatible. Crash
> > landing.
> 
> The new 'nts' keyword does not require any teardown.  Just the opposite.

You are right, but not in the way you think you are.

If I implement it a la "refclock", just plugging it into the head alternates
list, it's trivial.  No teardown.  On the other hand, if I try to build
a separate grammar branch?  I can do that, but there would be stuffing
flying around and risk of breakage before I'm done.

We don't have time for that.  We have a first ship to Cisco to get
out the door.

> > > Right now, server looks like this:
> > > 
> > > server address [key key] [burst] [iburst] [version version]
> > >        [minpoll minpoll] [maxpoll maxpoll]
> > > 
> > > nts looks like this (not well decided or documented yet):
> > > 
> > > nts address [[ask address]|[require address]] [noval]] [burst]
> > > [iburst] [prefer] [minpoll minpoll] [maxpoll maxpoll]  
> > 
> > OK, it wasn't clear before that the new "nts" declaration was
> > supposed to *entirely replace* a server declaration rather than
> > attaching nts options to an existing entry.  Next time do a better
> > job of spec-writing, please!
> 
> How about asking before just removing next time?

OK, you're quite right, this one was partly my fault.  I should have
remembered how terrible most programmers, including you, are at
documenting/specifying and not assumed that spec was complete.  I
should in fact have asked.

> And, expect several more options to the 'nts' keyword.

I'm ready. 

> And if it is anywhere in the server list it makes parsing much more
> difficult as you have to parse the entire line before you can
> decide what a particular keyward argument means, or is even valid.

Too late.  We're already there, and we can't get out of there without
a big and in my opinion entirely pointless rewrite.  We'd *still be
there even if I added your nts declaration.*  It's inherent in the
way the combination of parser and config block works.

Fortunately, the "much more complicated parsing" doesn't just work,
it's had that decades of road-testing.

And all of this has ABSOLUTELY NOTHING - nada, zilch, zero - to do
with the actual design question, which we are just now getting to.
But if you're going to spec more options you need to have a better
notion of what their complexity costs are, so this was no waste.

> > The remaining design question is a subtle one: in what natural kind
> > are "server", "pool", and "refclock" alternatives?  Is "nts" another
> > alternative in that natural kind, or is it a property of instances of
> > one of the existing request alternatives?
> 
> 'nts' is very much different from the others.  Totally different
> protocol.  Different options.

Doesn't look that way from here.

Of course you're right that the NTS wire protocol is quite different
from the NTP one.  But refclock communication was also quite different
from peer traffic, and yet there's a lot of overlap in the option sets
for those declarations - enough so that in Classic they used the same
head token "server".  And your own example shows nts sharing burst,
iburst, minpoll, and maxpoll with server.

What we need to compare here is the ontology of the *request parameters*
for an NTS-enabled and non-NTS-enabled server. And those, as your example
shows, are not so different. Less different than either are from
refclocks.

> > What is the natural kind in which "nts" is an alternative to "server",
> > "pool", and "refclock"?  If there is such a kind, how does that square
> > with the possibility that "nts" may become a property of pool request
> > instances?
> 
> Since the discussion has not figured out how that can even work, premature
> to make that a given.  My gut feel is that 'nts' can not be part of
> the 'pool' as then a casual attacker can break the system.

You might be right, but I'm not going to design on the assumption that
you are because the payoff matrix is too asymmetrical.

If we choose nts-as-option we are covered against the possibility that
someday we want to have an nts flag on pool connections. Easy to
imagine how that would work; it would set a request bit that says
"only give me back servers that advertise NTS service".  This is
similar enough to what the nts option means for individual servers not
to cause confusion.

On the other hand, if we make "nts" the head token of a new
declaration, *then* find we want to add an nts pool flag, every admin
until the end of time will find it ugly and wonder why we did the
weird non-orthogonal thing that we did.

Aaaaand...now that I think about it *that* way...

...that's it. That payoff asymmetry is a show-stopper, we don't have
to wrangle about natural kinds, and we're done with *this* argument.
It's my responsibility to notice potential design errors like that,
and when I do to put my foot down and prevent them. I'm doing so now.

On the other hand, I still want a spec for the other nts options.

> I still can't find where you moved the nts stuff to.

docs/includes/assoc-options.adoc.

> > > I suspect some more options will be needed for the nts. 

OK. You gave minpoll/maxpoll/burst/iburst, but those are just NTP options.
What is 'noval'?  And what else do you think we are going to need?
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

My work is funded by the Internet Civil Engineering Institute: https://icei.org
Please visit their site and donate: the civilization you save might be your own.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ntpsec.org/pipermail/devel/attachments/20190201/0acd8c0e/attachment.bin>