From owner-freebsd-net@FreeBSD.ORG Thu Jan 5 13:44:35 2012 Return-Path: Delivered-To: freebsd-net@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3F45C1065677 for ; Thu, 5 Jan 2012 13:44:35 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.64.117]) by mx1.freebsd.org (Postfix) with ESMTP id C048E8FC13 for ; Thu, 5 Jan 2012 13:44:34 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.5/8.14.5) with ESMTP id q05DiXUv057953; Thu, 5 Jan 2012 17:44:33 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.5/8.14.5/Submit) id q05DiX1V057952; Thu, 5 Jan 2012 17:44:33 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Thu, 5 Jan 2012 17:44:33 +0400 From: Gleb Smirnoff To: Sami Halabi Message-ID: <20120105134433.GO34721@glebius.int.ru> References: <20111227044754.GK8035@FreeBSD.org> <20111227083503.GP8035@glebius.int.ru> <20120105095855.GI34721@glebius.int.ru> <20120105110116.GK34721@glebius.int.ru> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="FsscpQKzF/jJk6ya" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-net@FreeBSD.org Subject: Re: ng_mppc_decompress: too many (4094) packets dropped, disabling node X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jan 2012 13:44:35 -0000 --FsscpQKzF/jJk6ya Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Sami, I am running not with the exact patch that I've sent to you, but with additional debugging printf, see attach. I'd like to make sure that after such a large rekeying event the PPP link is still valid. Since I can't cook this reordering case by hand, can you please eventually patch your ng_mppc with attached patch and reload it. The functionality is the same as the previous patch, but a printf is added. And we need to ping peers IP address as soon as we see the printf in logs. If the event happens very rarely, then some script needs to be written, that monitors system log and once printf fires, it should find interface name via ng_mppc node ID, and then take peers IP address and send a ping probe. If ping is replied, then MPPE survives this large rekeying and code can be considered functional. -- Totus tuus, Glebius. --FsscpQKzF/jJk6ya Content-Type: text/x-diff; charset=koi8-r Content-Disposition: attachment; filename="ng_mppc.c.diff" Index: ng_mppc.c =================================================================== --- ng_mppc.c (revision 229571) +++ ng_mppc.c (working copy) @@ -98,15 +98,6 @@ /* Key length */ #define KEYLEN(b) (((b) & MPPE_128) ? 16 : 8) -/* - * When packets are lost with MPPE, we may have to re-key arbitrarily - * many times to 'catch up' to the new jumped-ahead sequence number. - * Since this can be expensive, we pose a limit on how many re-keyings - * we will do at one time to avoid a possible D.O.S. vulnerability. - * This should instead be a configurable parameter. - */ -#define MPPE_MAX_REKEY 1000 - /* MPPC packet header bits */ #define MPPC_FLAG_FLUSHED 0x8000 /* xmitter reset state */ #define MPPC_FLAG_RESTART 0x4000 /* compress history restart */ @@ -641,20 +632,22 @@ #endif #ifdef NETGRAPH_MPPC_ENCRYPTION if ((d->cfg.bits & MPPE_BITS) != 0) { - u_int rekey; + u_int rekey; + + /* How many times are we going to have to re-key? */ + rekey = ((d->cfg.bits & MPPE_STATELESS) != 0) ? + numLost : (numLost / (MPPE_UPDATE_MASK + 1)); + if (rekey > 1000) + log(LOG_ERR, "%s: %d packets dropped, " + "node [%x]\n", __func__, numLost, + node->nd_ID); - /* How many times are we going to have to re-key? */ - rekey = ((d->cfg.bits & MPPE_STATELESS) != 0) ? - numLost : (numLost / (MPPE_UPDATE_MASK + 1)); - if (rekey > MPPE_MAX_REKEY) { - log(LOG_ERR, "%s: too many (%d) packets" - " dropped, disabling node %p!", - __func__, numLost, node); - priv->recv.cfg.enable = 0; - goto failed; - } - - /* Re-key as necessary to catch up to peer */ + /* + * When packets are lost or re-ordered with MPPE, + * we may have to re-key up to 0xfff times to 'catch + * up' to the new jumped-ahead sequence number. Yep, + * this is heavy, but what else can we do? + */ while (d->cc != cc) { if ((d->cfg.bits & MPPE_STATELESS) != 0 || (d->cc & MPPE_UPDATE_MASK) --FsscpQKzF/jJk6ya--