[Git][NTPsec/ntpsec][master] 3 commits: Respond to symmetric active packets without creating new association

[Daniel Fox Franke]

Daniel Fox Franke pushed to branch master at NTPsec / ntpsec


Commits:
3c676958 by Daniel Fox Franke at 2016-10-27T18:36:16-04:00
Respond to symmetric active packets without creating new association

- - - - -
d880e76c by Daniel Fox Franke at 2016-10-27T18:36:25-04:00
Simplify i_require_authentication()

- - - - -
ccbe4a46 by Daniel Fox Franke at 2016-10-27T18:36:33-04:00
Remove logic for accepting symmetric-mode packets with bad origin timestamps

- - - - -


1 changed file:

- ntpd/ntp_proto.c


Changes:

=====================================
ntpd/ntp_proto.c
=====================================
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -378,27 +378,19 @@ parse_packet(
 }
 
 /* Returns true if we should not accept any unauthenticated packets from
-   this peer. There are four ways the user can configure this requirement:
+   this peer. There are three ways the user can configure this requirement:
 
    1. A 'restrict notrust' command applies to the peer's IP, or,
    2. We've configured the peer with a 'key' option,
    3. The packet wants to create a new associations, and a 'restrict
       nopeer' command applies to the peer's IP.
-   4. This is a symmetric mode packet, and sys_authenticate is set
-      (as it is by default).
       
    The 'peer' argument may be NULL to indicate that we have no current
    association.
 
-   These rules implement a couple changes in behavior from NTP Classic:
+   In contrast to NTP classic, We don't enforce 'restrict nopeer'
+   against pool-mode responses.
 
-   1. Unless sys_authenticate is disabled, we always require
-      authentication for symmetric mode. NTP Classic only requires
-      authentication for symmetric active mode packets from new peers.
-      However, without authentication, symmetric mode is vulnerable
-      to simple attacks even from off path, so we require it.
-
-   2. We don't enforce 'restrict nopeer' against pool-mode responses.
 */
 static bool
 i_require_authentication(
@@ -407,15 +399,17 @@ i_require_authentication(
 	u_short restrict_mask
 	)
 {
-	return (peer != NULL && peer->keyid != 0) ||
-	    (restrict_mask & RES_DONTTRUST) ||
-	    ((restrict_mask & RES_NOPEER) &&
-	     ((peer != NULL && (peer->cast_flags & MDF_ACAST)) ||
-	      (peer == NULL && PKT_MODE(pkt->li_vn_mode) == MODE_ACTIVE) ||
-	      (PKT_MODE(pkt->li_vn_mode) == MODE_BROADCAST))) ||
-	     (sys_authenticate &&
-	      (PKT_MODE(pkt->li_vn_mode) == MODE_ACTIVE ||
-	       PKT_MODE(pkt->li_vn_mode) == MODE_PASSIVE));
+        bool restrict_notrust = restrict_mask & RES_DONTTRUST;
+        bool peer_has_key = peer != NULL && peer->keyid != 0;
+        bool wants_association =
+            PKT_MODE(pkt->li_vn_mode) == MODE_BROADCAST ||
+            (peer == NULL && PKT_MODE(pkt->li_vn_mode == MODE_ACTIVE)) ||
+            (peer != NULL && peer->cast_flags & MDF_ACAST);
+        bool restrict_nopeer =
+            (restrict_mask & RES_NOPEER) &&
+            wants_association;
+
+        return restrict_notrust || peer_has_key || restrict_nopeer;
 }
 
 static bool
@@ -501,7 +495,9 @@ handle_fastxmit(
 		xkeyid = 0;
 	}
 
-	fast_xmit(rbufp, MODE_SERVER, xkeyid, restrict_mask);
+        int xmode =
+            PKT_MODE(pkt->li_vn_mode) == MODE_ACTIVE ? MODE_PASSIVE : MODE_SERVER;
+	fast_xmit(rbufp, xmode, xkeyid, restrict_mask);
 }
 
 static void
@@ -545,26 +541,6 @@ handle_procpkt(
 			peer->bogusorg++;
 			return;
 		}
-	} else if(PKT_MODE(pkt->li_vn_mode) == MODE_ACTIVE ||
-		  PKT_MODE(pkt->li_vn_mode) == MODE_PASSIVE) {
-		/* In symmetric modes, even if the origin timestamp is
-		   bogus we still need to be willing to update the xmt
-		   peer variable. Otherwise, a single droppped packet
-		   will result in permanent DoS as the peers continually
-		   reject each other as bogus. Also, to be tolerant of
-		   restarts, we can't enforce outcount-checking.
-		*/
-		if(pkt->org == 0) {
-			uint64_to_lfp(&peer->xmt, pkt->xmt);
-			peer->flash |= BOGON3;
-			peer->bogusorg++;
-			return;
-		} else if(pkt->org != lfp_to_uint64(&peer->org)) {
-			uint64_to_lfp(&peer->xmt, pkt->xmt);
-			peer->flash |= BOGON2;
-			peer->bogusorg++;
-			return;
-		}
 	} else {
 		/* This case should be unreachable. */
 		sys_declined++;
@@ -816,6 +792,7 @@ receive(
 
 	switch(match) {
 	    case AM_FXMIT:
+            case AM_NEWPASS:
 		handle_fastxmit(rbufp, restrict_mask, pkt, peer, authenticated);
 		break;
 	    case AM_PROCPKT:
@@ -825,9 +802,9 @@ receive(
 		handle_manycast(rbufp, restrict_mask, pkt, peer, authenticated);
 		break;
 	    default:
-		/* Everything else is for symmetric passive, broadcast,
-		   or multicast modes, which are a security nightmare.
-		   So they go to the bit bucket until this improves.
+		/* Everything else is for broadcast or multicast modes,
+		   which are a security nightmare.  So they go to the
+		   bit bucket until this improves.
 		*/
 		sys_declined++;
 		break;



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/ed688cdc8f104cffd1e93ad261af8baa9852f2e3...ccbe4a463b23a59ded9076067050c7c4c9a1e860
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ntpsec.org/pipermail/vc/attachments/20161028/4a798cd4/attachment.html>