NTS client configuration support has landed

[Gary E. Miller]

Yo Eric!

On Fri, 1 Feb 2019 22:45:10 -0500
"Eric S. Raymond" <esr at thyrsus.com> wrote:

> > 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 scheme requires forward references to parse the sentence.  Using
'nts' does not.  Forward references are confusing.

> 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.

Yes.

> 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 

Yes.

> Here's a diagram:

Yes.

> 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.

Right, a bad thing.

> 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.

Right, a bad thing.

> 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.

And potentially nts.

> 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.

But with the new extra complication that some fields have new and
different meanings.  Complicating the "fancy copy".

> 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.

Which is now too late to do some of the new error checking required.

> 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.

Except that some keywords are now different.

> 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.

So, if refclock was easy, why is nts not easy?  Why did you not force
refclock into the server type?

> 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.

The alternative is now all these new keywords that are illegal, and
pass the parser, for the old types.

> 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.

I'd be happy t try.  We program once, the users suffer the consequences
forever.

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

What you mean "we" white man?

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

Except we can't just do that with all the new keywords, and old
keywords that now mean different things.

> 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

The how about:

server <hostname ask <hostname> port <port> ntpport <otherport> nts ca=/etc/ssl/certs ciphers="AA:BB:CC" ciphersuites="DD:EE"FF"

Now spotting the nts in a hurry is a nithgmare.  And the concept of hostname
is different than before.

> > > 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 keep trying.

> I believe you that nts needs 5 new semantically new options.

We are past 13 now, plus some that changed from the old definitions.
How do you explain that to a user?  When the hostname in "server <hostname>"
means something very different than in "server <hostname> xxx yyy zzz nts yyy"

> 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.

Been in the spec for weeks.  More coming all the time.  Look in nts.adoc.

> 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.)

Yeah, then you have to change the refclock, server and peer code to
make the new options errors, and to change the sense of hostname and
likely more.

> At that point, all four of the declaration types would grammatically
> have all the same options, *including the new ones*.

A nightmare for the user.  A lot of work to catch all the new illegal
things that pass the parser.

> 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.

Plus for the options that changed sense.

> 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*.

Well, maybe not you.  And maybe just add a new type.  If it is that
hard then the current structure should die.

> 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.

Uh, we don't care about giving usefull error message when the user
can't figure out the weird new syntax?  That will create a support
nightmare.

You program something once, you have to support it forever.

> I don't think we need to care that much.

"we" don't.  At least I do.

>  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!!"

We must be on different mailing lists...
 
> 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.

Short memory.

> > 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.

Which leaves, as you correctly point out, a disaster in checking for
errors and warning the user of same.

> 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.

I'd rather let old lies justify new ones.  Can't we start to do things
better?

> And all this has exactly *nothing* to do with whether "server
> <hostname> nts" or "nts <hostname>" is better design.

Right, because we already know that "nts <hostname" is a better design
for the user.  The user is what matters.  And the "hostname" for
nts is a very different thing than the "hostname" for a server.  That
needs to be highlighted to the user, not hidden to save a hour or
tow of programming.

>  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.

Well, if either are simple, then why fight the prior consensus?

> > > - 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?

If it is that bad, then maybe it should go.  But you just said it would
be easy: "Either can be implemented without any serious change to the
config framework."

So, which is it?  Easy not?

> > 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

Well, it sure confused us as we discussed.  It is good in extending
protocols to remember the hard learned concepts so you can make
it easier for the user.  That is exactly how the consensus for "nts
<hostname>" came about.

> 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.

Except they are not.  The "hostname" is a different thing than the
"hostname".

> > > 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.

Glad we finally got there.

> 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.

I think you are way overcomplicating this.  Bison is just not that
hard.

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

Well, now we are back to hard?  I don't beleive it.

> > > > 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.

Thank you.

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

13 in the nts.adoc now.  More coming.

> > 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.

Bison is just not that hard.

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

Except now it will get twisted in ways never seen before.  We never
had forward referrences before.

> And all of this has ABSOLUTELY NOTHING - nada, zilch, zero - to do
> with the actual design question, which we are just now getting to.

Whoa!  All those new options are REQUIRED.  And they need to be
easy for new users.  In this case we are ALL new users.

> 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.

Too late.  The options are explicit, and implicit, in the Proposed RFC.
I had nothing to do with that, would never have done anything like that,
I'm just trying to do what is in the spec.

> > > 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.

Then please look some more.  Forward references are totally new here.
lot's of old stuff is illegal, and lots of new stuff.

> 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.

To quote Richard Nixon.  "We could do that, but it would be wrong."

The continuing user experience is of far bigger importance than a
few extra hours of coding.

> 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.

Then read the nts.adoc again.  A lot more now than when you last looked, 
and more coming.

> > > 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.

I was trying to be diplomatic before.  Adding 'nts' to 'pool' is recklesss
endangerment.  Should be a felony.

> 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.

Once again: reckless endangerment.  Don't even think about it.

> 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.

Niec straw man.  Yours, not mine.

I'd be happy to start that discussion, but I suspect until the
NTS WG has something to say it is not remotely possible.

> 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.

OK, so now you have based your decision on a stupid and flawed new
feature?  There is no way we can ever do "pool XXX ntp" without a
while new RFC, which will be a LONG time coming.

> 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.

Sad.  You should listen first.

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

You already ahd a lot more before you sent this.  More coming.

> > I still can't find where you moved the nts stuff to.  
> 
> docs/includes/assoc-options.adoc.

Thanks.  I already see new problems there.  Gonna need a major reorg.
Way too soon to put in the public doc.  For another day...

> > > > 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?

"noval" in the nts.adoc since yesterday.  Please keep up with the
consensus before saying it is wrong.  It saves us a lot of duplication
of discussion.

We either need to discuss before putting in nts.adoc, or put in nts.adoc
before we discuss.  It was discussed on the list, consensus reached,
then put in nts.adoc, then gone.  Going back to the list, to rediscuss,
works better when the prior work product is not gone.

RGDS
GARY
---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703
	gem at rellim.com  Tel:+1 541 382 8588

	    Veritas liberabit vos. -- Quid est veritas?
    "If you can’t measure it, you can’t improve it." - Lord Kelvin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 851 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ntpsec.org/pipermail/devel/attachments/20190201/0e7d2049/attachment.bin>