[ntp:security] NOEPEER patch

Harlan Stenn stenn at nwtime.org
Wed Aug 1 06:36:55 UTC 2018


Hi Martin,

Pearly and I just spent a bunch of time talking about this issue.

I'm going to see if I can re-enable the windows behavior and plug the
other hole.

I don't believe we really *like* this choice, but it is one of several
tolerable choices.

I'll see if I can get you a patch before I fall asleep.

Assuming this is OK and it works, should we delay the release until
Tuesday the 14th?

H

On 7/31/2018 8:35 PM, Harlan Stenn wrote:
> 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