[ntp:security] NOEPEER patch

Harlan Stenn stenn at nwtime.org
Wed Aug 1 03:35:44 UTC 2018


Hi Martin,

On 7/31/2018 2:47 PM, Martin Burnicki wrote:
> Harlan,
> 
> Harlan Stenn wrote:
>> Hi Martin,
>>
>> First, might I trouble you to tell me how I can repeat this case?
> 
> I've a test program that can send different types of NTP packets to a
> server, and displays details of the request and reply packet.
> 
> Today I've extended the program to support MD5 symmetric key
> authentication (I already had that in a different test program), and
> made a few tiny changes so that you can now build it under Linux or
> FreeBSD. Here's the download link:
> https://www.meinberg.de/download/source/ntptest-1.10.tar.gz
> 
> cd into the unix/ subdirectory and type "make" to build the binary.
> 
> The simplest way to use it is to run
> 
>   ./ntptest <host>
> 
> which sends an NTPv4 client request to server <host>.
> 
> With option "-M 1" it sends a symmetric active packet:
> 
>   ./ntptest -M 1 <host>
> 
> With option "-V 3" you can set the protocol version to 3.
> 
> If the server has been configured to support symmetric keys you can use
> the "-m" and "-k" options to specify a key ID ("-m") and the keyphrase
> ("-k"), for example:
> 
>   ./ntptest -m 1 -k example <host>
> 
> will succeed if the server has a trusted MD5 key with key ID 1 and
> passphrase "example".
> 
> Of course you can mix these options to get send the required request
> packet format. Type
> 
>   ./ntptest -?
> 
> for a list of supported options.

Thanks!

>> That would be (un)authenticated Windows clients sending these (broken)
>> requests to an NTP server.
> 
> With ntptest you can send requests with or without authentication, and
> with valid or invalid keys, and see if and how the server replies.
> 
>> On 7/30/2018 5:52 AM, Martin Burnicki wrote:
>>> Harlan Stenn wrote:
> [...]
>>>> Does the LANtime create ephemeral associations from Windows clients?
>>>
>>> No, actually it doesn't, even if the NOEPEER keyword isn't specified.
>>
>> That's good, and expected, because of the lengths we've taken to deal
>> with the broken Windows clients in the past.
> 
> 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.

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

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.

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?

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.

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

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.

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

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.

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.

-- 
Harlan Stenn, Network Time Foundation
http://nwtime.org - be a Member!


More information about the security mailing list