[ntp:security] NOEPEER patch

Martin Burnicki martin.burnicki at meinberg.de
Fri Aug 3 15:28:32 UTC 2018


Harlan,

Harlan Stenn wrote:
> Before Pearly's change, the code was:
> 
>   if (   peer
>       && (peer->keyid > 0 || peer->flags & FLAG_SKEY))
> 	return VALIDNAK;
> 
> and now it is:
> 
>   if (!peer || !peer->keyid || !(peer->flags & FLAG_SKEY)
> 	return INVALIDNAK;
> 
> and I think we want:
> 
>   if (!peer || (!peer->keyid && !(peer->flags & FLAG_SKEY))
> 	return INVALIDNAK;
> 
> or:
> 
>   if (!peer || !(peer->keyid || (peer->flags & FLAG_SKEY))
> 	return INVALIDNAK;

This makes sense to me. Pearly's change has also changed the behavior.

I've now changed this code section to

	/*
	 * During the first few packets of the autokey dance there may
	 * not (yet) be a keyid, but in this case FLAG_SKEY is set.
	 * So the NAK is invalid if either there's no peer, or
	 * if the keyid is 0 and FLAG_SKEY is not set.
	 */
	if (!peer || (!peer->keyid && !(peer->flags & FLAG_SKEY))) {
		return INVALIDNAK;
	}

as you proposed, which restores the behavior of the original code.

I didn't find the time today to test all variations, but with this patch

- Peering works with a valid symmetric key.

- If an unknown key is used then "ntpq -c as" reports "auth bad" as
expected, but there are no nasty "Invalid-NAK" messages that were
previously sent to the syslog after each poll.

So I think this is a good solution. More tests to come.


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