From: Juergen Perlinger Date: Tue, 13 Sep 2016 05:26:06 +0000 (+0200) Subject: [Bug 3072] Attack on interface selection X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4d504bd5a98dccdec11fb93ad588bbc5a33ea312;p=thirdparty%2Fntp.git [Bug 3072] Attack on interface selection bk: 57d78deeyfknMUHpF4CWmP8gRRf6qg --- diff --git a/ChangeLog b/ChangeLog index 0805467dc..0ae8a6787 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +--- +* [Bug 3072] Attack on interface selection + - implemented Miroslav Lichvars suggestion + to skip interface updates based on incoming packets + --- (4.2.8p8) 2016/06/02 Released by Harlan Stenn @@ -19,7 +24,7 @@ * Fix typo in ntp-wait and plot_summary. HStenn. * Make sure we have an "author" file for git imports. HStenn. * Update the sntp problem tests for MacOS. HStenn. - + --- (4.2.8p7) 2016/04/26 Released by Harlan Stenn diff --git a/ntpd/ntp_peer.c b/ntpd/ntp_peer.c index cc23a3823..b60188266 100644 --- a/ntpd/ntp_peer.c +++ b/ntpd/ntp_peer.c @@ -273,6 +273,22 @@ findexistingpeer( /* * findpeer - find and return a peer match for a received datagram in * the peer_hash table. + * + * [Bug 3072] To faciliate a faster reorganisation after routing changes + * the original code re-assigned the peer address to be the destination + * of the received packet and initiated another round on a mismatch. + * Unfortunately this leaves us wide open for a DoS attack where the + * attacker directs a packet with forged destination address to us -- + * this results in a wrong interface assignment, actually creating a DoS + * situation. + * + * This condition would persist until the next update of the interface + * list, but a continued attack would put us out of business again soon + * enough. Authentication alone does not help here, since it does not + * protect the UDP layer and leaves us open for a replay attack. + * + * So we do not update the adresses and wait until the next interface + * list update does the right thing for us. */ struct peer * findpeer( @@ -291,61 +307,50 @@ findpeer( srcadr = &rbufp->recv_srcadr; hash = NTP_HASH_ADDR(srcadr); for (p = peer_hash[hash]; p != NULL; p = p->adr_link) { - if (ADDR_PORT_EQ(srcadr, &p->srcadr)) { - - /* - * if the association matching rules determine - * that this is not a valid combination, then - * look for the next valid peer association. - */ - *action = MATCH_ASSOC(p->hmode, pkt_mode); - /* - * A response to our manycastclient solicitation - * might be misassociated with an ephemeral peer - * already spun for the server. If the packet's - * org timestamp doesn't match the peer's, check - * if it matches the ACST prototype peer's. If - * so it is a redundant solicitation response, - * return AM_ERR to discard it. [Bug 1762] - */ - if (MODE_SERVER == pkt_mode && - AM_PROCPKT == *action) { - pkt = &rbufp->recv_pkt; - NTOHL_FP(&pkt->org, &pkt_org); - if (!L_ISEQU(&p->aorg, &pkt_org) && - findmanycastpeer(rbufp)) - *action = AM_ERR; - } + /* [Bug 3072] ensure interface of peer matches */ + if (p->dstadr != rbufp->dstadr) + continue; + + /* ensure peer source address matches */ + if ( ! ADDR_PORT_EQ(srcadr, &p->srcadr)) + continue; + + /* If the association matching rules determine that this + * is not a valid combination, then look for the next + * valid peer association. + */ + *action = MATCH_ASSOC(p->hmode, pkt_mode); + + /* A response to our manycastclient solicitation might + * be misassociated with an ephemeral peer already spun + * for the server. If the packet's org timestamp + * doesn't match the peer's, check if it matches the + * ACST prototype peer's. If so it is a redundant + * solicitation response, return AM_ERR to discard it. + * [Bug 1762] + */ + if (MODE_SERVER == pkt_mode && AM_PROCPKT == *action) { + pkt = &rbufp->recv_pkt; + NTOHL_FP(&pkt->org, &pkt_org); + if (!L_ISEQU(&p->aorg, &pkt_org) && + findmanycastpeer(rbufp)) + *action = AM_ERR; + } - /* - * if an error was returned, exit back right - * here. - */ - if (*action == AM_ERR) - return NULL; + /* if an error was returned, exit back right here. */ + if (*action == AM_ERR) + return NULL; - /* - * if a match is found, we stop our search. - */ - if (*action != AM_NOMATCH) - break; - } + /* if a match is found, we stop our search. */ + if (*action != AM_NOMATCH) + break; } - /* - * If no matching association is found - */ - if (NULL == p) { + /* If no matching association is found... */ + if (NULL == p) *action = MATCH_ASSOC(NO_PEER, pkt_mode); - } else if (p->dstadr != rbufp->dstadr) { - set_peerdstadr(p, rbufp->dstadr); - if (p->dstadr == rbufp->dstadr) { - DPRINTF(1, ("Changed %s local address to match response\n", - stoa(&p->srcadr))); - return findpeer(rbufp, pkt_mode, action); - } - } + return p; } @@ -621,7 +626,8 @@ set_peerdstadr( { struct peer * unlinked; - if (p->dstadr == dstadr) + /* check for impossible or identical assignment */ + if (p == NULL || p->dstadr == dstadr) return; /* @@ -632,6 +638,8 @@ set_peerdstadr( (INT_MCASTIF & dstadr->flags) && MODE_CLIENT == p->hmode) { return; } + + /* unlink from list if we have an address prior to assignment */ if (p->dstadr != NULL) { p->dstadr->peercnt--; UNLINK_SLIST(unlinked, p->dstadr->peers, p, ilink, @@ -640,8 +648,11 @@ set_peerdstadr( stoa(&p->srcadr), latoa(p->dstadr), latoa(dstadr)); } + p->dstadr = dstadr; - if (dstadr != NULL) { + + /* link to list if we have an address after assignment */ + if (p->dstadr != NULL) { LINK_SLIST(dstadr->peers, p, ilink); dstadr->peercnt++; }