Project

General

Profile

Actions
Copy link

Bug #3384

closed

NTPd should deny service if the packet spacing violates the lower limits specified in the discard command (CVE-2013-5211)

Added by Jeroen Roovers almost 11 years ago. Updated almost 11 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
NTPD
Target version:
2.1.1
Start date:
01/04/2014
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
All
Affected Architecture:
All

Description

ntp.conf(5):

limited
       Deny service if the packet spacing violates the lower limits specified
       in the discard command. A history of clients is kept using the
       monitoring capability of ntpd(8).  Thus, monitoring is always active
       as long as there is a restriction entry with the limited flag.

Upstream bug report:
[[http://bugs.ntp.org/show_bug.cgi?id=1532]]

Various distributions are working around the issue in ntp.conf instead of upgrading to the development branch.

The solution would be to include "restrict.* limited" (and probably "kod" too) in /etc/inc/system.inc where it writes /var/etc/ntpd.conf

Files

etc-inc-system.inc.patch (651 Bytes) etc-inc-system.inc.patch Add "limited" to ntpd.conf Jeroen Roovers, 01/04/2014 08:13 AM
Actions
Copy link
 #2

Updated by Renato Botelho almost 11 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100
Actions
Copy link
 #4

Updated by Jim Pingle almost 11 years ago

  • Status changed from Feedback to New
  • Priority changed from High to Normal
  • Target version set to 2.1.1

Using limited as-is denies access to NTP clients, so this change is not viable. If you try to sync time against ntpd with limited present, it denies access. If you remove limited, it works. No errors are logged.

If an appropriate combination of discard parameters plus this statement can be formulated, we can try it, but we have to ensure we can work as an NTP server as well as a client.

We have several other changes for that CVE already committed that it may be mitigated sufficiently without this change.

Actions
Copy link
 #5

Updated by ky41083 - almost 11 years ago

The default values with the "limited" parameter specified only allow a client to NTP sync once every 5 seconds OR twice in a row. Seems like a lot to me, and yes, I've tested this without revision 2b8dfa4e.

Would something like this make more sense? If not, at least the comments for the values should be correct and easily tweakable. Possibly not in/on the correct line. Following values with both LAN's and ISP's in mind. Nothing should be NTP syncing on a LAN more than once every minute, but it's local so we allow some grace here. Nothing should really be NTP syncing with a public NTP server more than once every 15 minutes, but again, given the server load, we can give some grace here also.

Modified from NTP section of system.inc:

$ntpcfg .= "driftfile {$driftfile}\n";

[Access Control Commands - Start]
[Accept 2 packets in a row from same client IP]
[Accept 1 packet every 30 seconds from same client IP]
[100% Probability of discard for packets that overflow]
$ntpcfg .= "discard average 30 minimum 2 monitor 100\n";
[Access Control Commands - End]

$ntpcfg .= "restrict default kod limited nomodify notrap nopeer\n";
$ntpcfg .= "restrict -6 default kod limited nomodify notrap nopeer\n";

Or, the harder route, but also the more correct one. Add these as variables / settings to the NTP services page so we can tune our boxes as needed for the role(s) they are in.

Actions
Copy link
 #6

Updated by Doktor Notor almost 11 years ago

every 5 seconds OR twice in a row. Seems like a lot to me

Uhm, not really a lot when iburst is used. (8 packets are sent every 2 seconds for a server that does respond.) This is very widely used and even default configuration in many places.

Actions
Copy link
 #7

Updated by ky41083 - almost 11 years ago

Hey, that works. I was saying it seems like a lot from the standpoint of a public NTP server. Clearly if you have needs like that, you should setup your own NTP server that you can hammer with requests all you like :-)

I'll count that as a +1 for the "add these as variables / settings to the NTP services page" preference ;-)

Actions
Copy link
 #8

Updated by Doktor Notor almost 11 years ago

Actions
Copy link
 #9

Updated by ky41083 - almost 11 years ago

No no, I get what you're saying, and I don't disagree with it at all in the correct scenario.

But the person who opened this ticket, as well as the people responsible for the "limited" option being coded into NTPd, me, and a host of other network / data center operators clearly see a benefit in protecting pretty much any network service. Especially something as widely open as NTP (remember when SMTP relays used to be wide open too?) from accidental flooding, or even more a concern, outright DoS attacks. If it's a non-issue, why all the effort to make a solution for it?

So, if you want iburst to work, configure it that way. If you want to provide something more conservative to protect your hardware and your network for customers who don't flood your NTP servers, configure it that way.

My argument was and is, that it wasn't "completely broken", simply using limited's defaults and dropping some requests.

The second part of my argument also was and is, for easy custom settings, as opposed to a hard coded "one size fits all" preset, as was done when "limited" was added initially. Which clearly wasn't working for everyone, even though it was working just peachy for me, but in cases such as yours, was considered "completely unusable".

So, again, how about a check box to turn limited on or off, with some fill in box settings for rate control, all defaulted to off initially?

Actions
Copy link
 #10

Updated by Chris Buechler almost 11 years ago

  • Status changed from New to Feedback
  • Affected Version set to All

I believe we have adequate solutions in place here, and having discard enabled by default doesn't seem to be appropriate.

Actions
Copy link
 #11

Updated by ky41083 - almost 11 years ago

Having limited enabled by default is not appropriate, and it shouldn't be the case.

Having the option to turn it on and adjust its 2 (two) parameters in the GUI seems more appropriate than having to roll your own diff for the patch system just to enable and tune it.

The current system of control is "enable all NTP always" or "disable all NTP always" by IP / interface / etc...

If that system of control was adequate, why is "limited" even an option in the NTPd binary?

Actions
Copy link
 #12

Updated by Chris Buechler almost 11 years ago

  • Status changed from Feedback to Resolved

limited is not enabled by default (the commits associated with this ticket have been backed out). Everything here is adequately addressed. In 2.1.1, it's disabled. In 2.2, it's a configurable option.

Actions
Copy link
 #13

Updated by ky41083 - almost 11 years ago

I know it's not enabled by default, you said:

having discard enabled by default doesn't seem to be appropriate

inferring I thought it should be, so I was clarifying my train of thought for everyone you.

I'm glad? we'll see it in a year or two in 2.2. Hopefully its parameters will also be configurable as to not make it a useless option, twice.

Wouldn't it have been easier to just say:

Hey, it's in 2.2, we don't feel like backporting it. [Update Ticket Target Version]

Rather than

Stuff that's there is stupid, because I didn't write it, or put it there, so it must not make sense, clearly.

Btw, the ticket still lists 2.1.1 as a target. You might want to update it...

Cheers Mate

Actions
Copy link

Also available in: Atom PDF