Requesting code review on possible fix for nopeer/pool conflict

[Eric S. Raymond]

Mark: Heads up! Policy issue.

Hal Murray <hmurray at megapathdsl.net>:
> dfoxfranke at gmail.com said:
> > One more reason I need to get my ACL language implemented and restrict needs
> > to die. 
> 
> If you kill restrict, we are taking a major step toward making ntp.conf file 
> no longer compatible.

We are aware of this.  Daniel's argument for that step is that the
present language (a) has wrong (insecure) defaults, and (b) is so
badly designed and easy to mis-code that the standard default restrict
everybody ships is actually broken.

While the former is partly a policy question, the latter is certainly
true.  The confusion about the pool command and the related bug I
found is evidence for that.

This means that we can wave the big SECURITY stick at our potential
userbase and get them to more or less cheerfully accept a degree of
incompatibility that would otherwise provoke howls.  But see below; we
may not have to strain their patience very far, after all.

> Would it be possible to for your new code to support the old restrict stuff?  
> (Similar to the way Eric's new refclock stuff still works with the old stuff.)

Speaking as the person who knows how to write the Bison productions
and hack the scanner, there would be no great difficulty in
supporting both subgrammars simultaneously.

The real sticking point is that the null specification means
different things in the old and new grammars.  In the old grammar,
no restrictions.  In the new, a set of restrictions roughly
equivalent to:

restrict default kod limited nomodify notrap nopeer noquery  
restrict -6 default kod limited nomodify notrap nopeer noquery
restrict 127.0.0.1  
restrict -6 ::1

I say "roughly" because this spec has at least one bug I know about
(the nopeer/pool conflict) and Daniel has mumbled of others.

The actual policy decision we need to make, then, is when to change the
meaning of the null specification.

I think there's a strong case for being aggressive about this. The
case is founded on the fact that the equivalent of the new empty spec
is what all distributions ship as a stock config and 99.9% of users
never modify.  So while theoretically it's a major incompatibility,
in practice only a tiny minority will see breakage - and those will
be precisely the ones best equipped to cope.

Also, I have in my head a clever design for a migration script...

> > I think this working as designed. 'restrict nopeer' means "Don't establish
> > unauthenticated ephemeral associations with this IP address", which is
> > exactly what pool does. I agree this is stupid design but I don't think it's
> > a bug.
> 
> I think the current setup is buggy, but maybe that whole area is more 
> complicated than I currently understand.
> 
> Maybe restrict needs a nopool tag so we don't get confused by peer vs pool.

No, on this I disagree with Daniel.  I think it is reasonable for a pool
declaration to poke a hole through nopeer, and the logic change is easy.
I'm not pushing it only because I expect the new rule language to obviate
the issue.

> There is a fundamental problem here.  What should happen if server/pool and 
> restrict lines conflict?  Is server DNS different from server IP Address?  My 
> straw man is that a restrict line with explicit IP Address(es) should block 
> server/pool addresses but the default restrict should not.

I agree.

> I'd also vote for conflicts to generate error messages at startup time and/or 
> at DNS lookup time.  The DNS lookup could try the next address and/or try 
> again later (after TTL).

That seems doable.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>