[ntp:security] [Bug 1331] DoS with mode 7 packets (CVE-2009-3563)
davehart at gmail.com
Wed Oct 7 04:01:07 UTC 2009
On Wed, Oct 7, 2009 at 3:47 AM, Danny Mayer <mayer at ntp.org> wrote:
> Dave Hart via the NTP Bugzilla wrote:
>> Danny, I agree it is important to keep the patch simple and focused. If you
>> compare my proposed patch with yours I think you'll see mine is simpler. It
>> does not split the early sanity checks into two parts, it does not rearrange the
>> order of the tests, and thereby the meaning of the logged test numbers.
> Your changes are very different and do not differentiate between attack
> and error.
Very different from your patch, yes, but less different from the
original code, which is how I define simpler.
> The break is intentional and not accidental and is definitely simpler.
Making a judgement call about which ones to drop and never log vs
which to reply to and log without rate limits is subject to mistake,
such as your initial mistake where your patch responded with an error
to error responses. It is simpler and safer to drop all malformed
mode 7 packets and not try to be clever about which ones it's safe to
respond to. There is no value in the response that I see.
> You are adding additional complexity that is unnecessary using
> constructs that may not exist in earlier code. In fact they certainly
> don't in xntp 3 and I need to keep that in mind when issuing the fix.
The rate-limited msyslog approach is nothing new, and if someone wants
to backport the fix and finds NLOG() unavailable in their ntpd, they
can omit that line or omit the logging entirely.
>> What it does do is keep to the ntpd practice in mode 6 and mainline processing
>> of dropping malformed packets without a peep in responses, and add rate-limiting
>> code to ensure the msyslog triggered by the big if statement happens no more
>> than once per minute.
>> I stand by my proposed patch as a simpler, more focused fix and one that brings
>> mode 7 handling in line with other packet input code paths in ntpd.
> We can align code later but now is not the time.
We have plenty of time before any public announcement, assuming leaks
are avoided. There is no need to rush to judgement on which patch to
> No you are adding unnecessary complexity and I did look at your code.
> Adding a bunch of additional code for logging purposes only is not
> simpler. It begs the question of what is going to get logged and you
> have not mapped out all the scenarios involved. The only other thing you
> did was to drop ALL packets that fail the tests.
Why do you think it's important to reply to packets with the MORE bit
set but not to packets with the RESPONSE bit set? Neither one is
valid in a query. Mode 6 drops both, mode 7 should as well.
> You are overcomplicating the matter which involves a simple issue which
> I have fixed. Can we drop this please?
Again I say my patch is simpler and more focused on the vulnerability,
namely, responding to responses. The rate-limiting addition to the
msyslog is needed if we are to retain any logging of queries with the
response bit lit, due to the possibility there will be a flood of
them, particularly in the broadcast/multicast scenario.
More information about the security