[ntp:security] NOEPEER patch

Martin Burnicki martin.burnicki at meinberg.de
Wed Aug 1 08:00:03 UTC 2018


Harlan Stenn wrote:
> Hi Martin,
> 
> On 7/31/2018 2:47 PM, Martin Burnicki wrote:
>> Please keep in mind that w32time just sends one type of unusual request.
>> ntpd in the role of a server should handle also different unsual
>> requests gracefully.
> 
> Yes, and I'm still trying to figure out how I can test ths with w32time
> directly, as one of the key issues is whether or not w32time requires a
> mode 1/2 response to its mode 1 request, or if it will be happy with a
> mode 3 response.  I still need to talk to Dave (and maybe Pearly) about
> this.

>From my experience with w32time I can tell that the detailed behavior of
w32time depends strongly on the w32time version, which in turn depends
on the Windows version and even on the patch level of that windows version.

So if you run a test with a w32time version that is happy with a
"server" response to a "symmetric active" request you still don't know
if other w32time versions accept this, too.

Simply sending a mode 2 response in reply to a mode 1 request has been
working for many years now, without any problem report from a w32time
user. Well, except that it stopped working after the nopeer patch,
simply because no reply is sent at all.

So why do you think this should be changed now and a "server" mode
packet be sent as response?

What would be the advantage of sending a "server" mode packet?

>>>> The original problem was that an ephemeral association was mobilized on
>>>> the local node if the remote node used a valid symmetric key, but the
>>>> remote node wasn't explicitly configured. This should be prevented by
>>>> the NOEPEER keyword.
>>>
>>> I believe we are in agreement on this point.
>>>
>>> In the past, nopeer allowed spinning up ephemeral passive peers IFF the
>>> request was authenticated.
>>>
>>> The problem was there was no way to prevent spinning up an ephemeral
>>> passive peer from an authenticated source.
>>>
>>> There are (or should be) 2 ways to prevent this.
>>>
>>> 1) noepeer
>>>
>>> 2) the 4th argument in the ntp.keys file
>>
>> Yes, but at least in case of "nopeer" the daemon with the latest patch
>> doesnÄ't reply at all, which is not good.
> 
> You mean "noepeer", right?

Yes, I meant "noepeer".

> I forget how "nopeer" responds to an
> unauthenticated mode 1 request, but it should respond just fine to a
> properly authenticated mode 1 request.
> 
>>> You folks previous reported a bug in noepeer where there were cases that
>>> *would* spin up an ephemeral passive peering session.  The bad news here
>>> is the cleanest fix for this prevents broken Windows clients from syncing.
>>>
>>> More below.
>>>
>>>> I've put some pseudo code together describing the program flow that I
>>>> think is appropriate:
>>>>
>>>>   // A symmetric Packet has been received.
>>>>
>>>
>>> Isn't that next line supposed to be: Packet has NO signature?
>>
>> Yes, of course. Sorry.
> 
> No worries.  I adjusted this, below.  I only mention this because I want
> to be sure we're in full agreement.
> 
>>>>   if ( !auth_used ) //packet has NO signature
>>>>   {
>>>>     // Should we eventually accept it
>>>>     // as ephemeral peer anyway?
>>>>     // Probably not.
>>>>     goto send_reply;
>>>>   }
>>>>
>>>>   // Packet has a signature.
>>>
>>> In the next case, the previous code looked like it did a
>>> fast_xmit(MODE_ACTIVE) if an invalid signature was received, and that
>>> makes no sense to me.
>>
>> Shouldn't this depend on whether the received packet was a "symmetric
>> active" or symmetric passive" packet?
> 
> I don't recall what sort of strangeness was going on when this was written.
> 
> OK, I looked.  As of 13 May 2008, comments were added to describe what's
> going on.  The code then was:
> 
>  if (!AUTH(sys_authenticate | (restruict_mask & RES_NOPEER),
> 	is_authentic) {
> 	fast_xmit(rbufp, MODE_PASSIVE, skeyid, NULL);
> 	return;
>  }
> 
> The MODE_PASSIVE choice was put in place on 1 Feb 2008.  Before that, we
> replied with hismode.

Hm, sending the same type of packet back sounds strange to me, so I
think it's OK to send a MODE_PASSIVE response to a MODE_ACTIVE request.

> The !AUTH(...) test dates back to 4 April 2005.
> 
> The MODE_ACTIVE alternative was added in July of 2009.
> 
> Dave did all of this, and I remember him grumbling about the things we
> had to do to accommodate w32time.
> 
>>>>   if ( !auth_ok )  // signature not valid
>>>>   {
>>>>     // We never accept it.
>>>>     send_crypto_nack();
>>>>     goto done;
>>>>   }
>>>>
>>>>   // Auth is used and OK.
>>>
>>> This next case will never happen in the current code - at the point in
>>> question the packet has already made it to the point where we know this
>>> is an incoming request to spin up a *new* passive association:
>>
>> But if the remote node has explicitly been specified as peer, shouldn't
>> an association for that peer already exist?
> 
> Yes, but then we would not be in the AM_NEWPASS section of the code.
> We'd be in the AM_PROCPKT section.

OK. But that seems to work as expected.

> The windows client checks only need to be in the AM_NEWPASS section.
> 
>>>>   if ( remote_peer_is_configured )
>>>>   {
>>>>     // This is a standard peer association.
>>>>     assoc_type = standard;
>>>>
>>>>     // No need to check NOEPEER flag.
>>>>     goto create_assoc;
>>>>   }
>>>>
>>>>   // This is an ephemeral peer.
>>>>   assoc_type = ephemeral;
>>>>
>>>>   // Check NOEPEER flag if we accept it.
>>>>   if ( noepeer_flag )
>>>>   {
>>>>     // We don't accept ephemeral peers.
>>>>     goto send_reply;
>>>>   }
>>>>
>>>> create_assoc:
>>>>   // We get here only if we accept a standard
>>>>   // or ephemeral peer association for this
>>>>   // remote node.
>>>>   create_assoc_if_required( assoc_type );
>>>>
>>>> send_reply:
>>>>   // Send a reply with or without signature,
>>>>   // depending on whether the request had
>>>>   // a signature. We don't get here if a
>>>>   // signature was used, but not valid.
>>>>   send_reply_msg();
>>>>
>>>> done:  // That's it!
>>>>
>>>>
>>>> So as a summary:
>>>>
>>>> - A reply (symmetric passive in case of a symmetric active request) is
>>>> sent in any case, except that we send a crypto NAK if the request had an
>>>> invalid signature.
>>>>
>>>> - A peer association is mobilized only if auth is used and OK, and
>>>> -- if the NOEPEER keyword has *not* been specified
>>>> -- or the remote peer has explicitly been specified as peer.
>>>>
>>>>
>>>> We have verified here that Windows clients that send unauthenticated
>>>> symmetric active requests are happy if they receive an unauthenticated
>>>> symmetric passive reply from the server.
>>>
>>> Thanks!
>>>
>>> Are they happy if they receive an unauthenticated mode 4 (SERVER) response?
>>>
>>> Dave and I have talked about whether or not it's OK to give a SERVER
>>> response to an unexpected ACTIVE (or PASSIVE) packet.  So far, we've
>>> avoided this case, and I don't know enough about this to make a proper
>>> evaluation.
>>
>> Without the nopeer keyword, or without the latest patch, the client
>> receives an symmetric passive reply if it sends a symmetric active
>> request, and that works great for these Windows clients.
> 
> You mean 'noepeer' right?

Aaargh, yes. ;-)

> And yes, I know it works if we send a mode 2 (PASSIVE) response.
> 
> I remain curious how w32time will behave if instead of a mode 2
> response, it gets a mode 4 (SERVER) response.

See my comments avove on w32time versions.

>>> But to a point I alluded to above...
>>>
>>> Dave is a fan of Occam's Razor, perhaps even more than I am.  He wants
>>> to remove un-needed complexity wherever he sees it.  This goes to writing:
>>>
>>>  if (a)
>>> 	something;
>>>
>>> instead of:
>>>
>>>  if (a) {
>>> 	something;
>>>  }
>>
>> I also prefer the shorter format. ;-)
> 
> I used to, and then I saw too many problem where somebody added a debug
> line or something else, and forgot to add the braces.  That caused
> additional delays.
> 
>>> The current patch for NOEPEER is very clean.
>>>
>>> I can see us spending a LOT of time making sure a more complicated code
>>> path will actually work, and will be the source for numerous upcoming
>>> bug reports.  I'd like to avoid this situation.
>>
>> I don't think it's complicated. It's just a little bit different than
>> the existing code, and with the ntptest tool you can easily test the
>> different cases to see if they all work as expected.
> 
> There have been 2 bugs in the past couple of years' time where I have
> spent literally WEEKS staring at the code and running tests.  These have
> been security-related issues, and I now have an even stronger desire to
> avoid these problems in the future.

Understood. However, if you forget w32time and just determine how ntpd
should reply to specific requests with or without authentication, and
with or without allowing ephemeral peers then things shouldn't be very
complicated.

> We *could* put a 2nd NOEPEER test to cover the "else" clause, but there
> might be other, untested cases where we'd still have a hole that would
> require another patch release.

You can easily test different cases with the ntptest program.

>>> An argument can easily be made that the Windows clients are *requesting
>>> an ephemeral peering session* as evidenced by their sending a peering
>>> request, and as such, if we're using noepeer we *should* be denying
>>> these packets.
>>
>> No, please. Let's just restore the mode of operation that DLM introduced
>> in 2002 and 2008, as I've pointed out in a different email.
>>
>>> If windows clients are coming in, then if the server in question must
>>> support broken windows clients then I wonder if doing both:
>>>
>>> - using 'nopeer' to prevent unauthenticated peering
>>> - use the 4th argument in the  ntp.keys file to prevent
>>>   authenticated peering from unwanted servers
>>>
>>> is the better way to go.
>>>
>>> For this last case, if *no* (ephemeral?) peering is desired, I believe:
>>>
>>> ntp-keys:
>>>  10 md5 foo 0.0.0.0
>>>
>>> will work.
>>>
>>> Might you be able to test this?
>>>
>>> If this works, is it acceptable?
>>>
>>> I'm happy to write up more documentation on this.
>>
>> As said earlier, please don't just refer to Windows clients. Let's talk
>> about how the server should behave when it receives a particular type of
>> packet, as I've tried to show with my pseudo code.
>>
>> What should be fixed, IMO, is that ntpd *does* accept ephemeral peer
>> associations at all by default.
> 
> This is the point.  We plan to make NOEPEER the default in 4.4.

That's good.

> Windows machines are specifically asking for a peering association,
> ephemeral or not.  They have the option to send a client request.
> 
> We are saying "We do not accept ephemeral peer requests".
> 
> This one is tough.  I can see your point, however.

That's good, too. ;-)

Specifically since this used to work in earlier versions of ntpd.

> I'll keep thinking about it, and I'm very concerned about delaying the
> p12 release if we update the patch.
> 
> If the LANtime (or whatever) in question is being a symmetric peer, then
> its known peer(s) SHOULD be listed on the appropriate keyID lines in the
> ntp.keys file.  If the box is not being a symmetric peer, then the keyID
> line(s) in the ntp.keys file should be set to a value that would prevent
> using that keyID for peering.
> 
> But we've each said all of this to each other before, so I wonder what
> I'm missing.
> 
> It may be as simple as "we don't want to break backward compatibility".
> 
> But we are already dealing with a bug fix, and I'm trying to appreciate
> the tradeoffs, costs, and benefits.
> 
> I plan to be writing a blog post or a whitepaper about this.


Martin
-- 
Martin Burnicki

Senior Software Engineer

MEINBERG Funkuhren GmbH & Co. KG
Email: martin.burnicki at meinberg.de
Phone: +49 5281 9309-414
Linkedin: https://www.linkedin.com/in/martinburnicki/

Lange Wand 9, 31812 Bad Pyrmont, Germany
Amtsgericht Hannover 17HRA 100322
Geschäftsführer/Managing Directors: Günter Meinberg, Werner Meinberg,
Andre Hartmann, Heiko Gerstung
Websites: https://www.meinberg.de  https://www.meinbergglobal.com
Training: https://www.meinberg.academy



More information about the security mailing list