Pipex did panic when the 0 length mppe is given by ioctl. Return

EINVAL instead of panic.  Also npppd called ioctl with the invalid
argument because of the bugs introduced by the config parser change
commit.  Fixed those bugs and make sure not to use 0 length keys for
MPPE.

reported by csszep at gmail and giovanni
ok giovanni

6 files changed, 59 insertions, 46 deletions

diff --git a/sys/net/pipex.c b/sys/net/pipex.c
index 3b700984e81..f3b1e22fe1d 100644
--- a/sys/net/pipex.c
+++ b/sys/net/pipex.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pipex.c,v 1.37 2012/12/14 01:19:26 dlg Exp $	*/
+/*	$OpenBSD: pipex.c,v 1.38 2013/02/13 22:10:38 yasuoka Exp $	*/
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -396,14 +396,24 @@ pipex_add_session(struct pipex_session_req *req,
 	}
 #endif
 #ifdef PIPEX_MPPE
-    	if ((req->pr_ppp_flags & PIPEX_PPP_MPPE_ACCEPTED) != 0)
+    	if ((req->pr_ppp_flags & PIPEX_PPP_MPPE_ACCEPTED) != 0) {
+		if (req->pr_mppe_recv.keylenbits <= 0) {
+			free(session, M_TEMP);
+			return (EINVAL);
+		}
 		pipex_session_init_mppe_recv(session,
 		    req->pr_mppe_recv.stateless, req->pr_mppe_recv.keylenbits,
 		    req->pr_mppe_recv.master_key);
-    	if ((req->pr_ppp_flags & PIPEX_PPP_MPPE_ENABLED) != 0)
+	}
+    	if ((req->pr_ppp_flags & PIPEX_PPP_MPPE_ENABLED) != 0) {
+		if (req->pr_mppe_send.keylenbits <= 0) {
+			free(session, M_TEMP);
+			return (EINVAL);
+		}
 		pipex_session_init_mppe_send(session,
 		    req->pr_mppe_send.stateless, req->pr_mppe_send.keylenbits,
 		    req->pr_mppe_send.master_key);
+	}
 
 	if (pipex_session_is_mppe_required(session)) {
 		if (!pipex_session_is_mppe_enabled(session) ||
diff --git a/usr.sbin/npppd/npppd/mppe.c b/usr.sbin/npppd/npppd/mppe.c
index 69033b34f0b..1e766d14c96 100644
--- a/usr.sbin/npppd/npppd/mppe.c
+++ b/usr.sbin/npppd/npppd/mppe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mppe.c,v 1.9 2012/12/19 09:23:54 sthen Exp $ */
+/*	$OpenBSD: mppe.c,v 1.10 2013/02/13 22:10:38 yasuoka Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-/* $Id: mppe.c,v 1.9 2012/12/19 09:23:54 sthen Exp $ */
+/* $Id: mppe.c,v 1.10 2013/02/13 22:10:38 yasuoka Exp $ */
 /**@file
  *
  * The implementation of MPPE(Microsoft Point-To-Point Encryption Protocol)
@@ -119,9 +119,6 @@ mppe_init(mppe *_this, npppd_ppp *ppp)
 
 	_this->required = conf->mppe_required;
 
-	if (_this->required == 0)
-		goto mppe_config_done;
-
 	if (conf->mppe_keystate == (NPPPD_MPPE_STATEFUL|NPPPD_MPPE_STATELESS)) {
 		/* no need to change from default. */
 	} else if (conf->mppe_keystate == NPPPD_MPPE_STATELESS) {
@@ -230,22 +227,22 @@ mppe_start(mppe *_this)
 		_this->recv.keybits = 128;
 	}
 
-	mppe_rc4_init(_this, &_this->send, 0);
-	mppe_rc4_init(_this, &_this->recv, _this->recv.stateless);
-
-	GetNewKeyFromSHA(_this->recv.master_key, _this->recv.master_key,
-	    _this->recv.keylen, _this->recv.session_key);
-	GetNewKeyFromSHA(_this->send.master_key, _this->send.master_key,
-	    _this->send.keylen, _this->send.session_key);
-
-	mppe_reduce_key(&_this->recv);
-	mppe_reduce_key(&_this->send);
-
-	mppe_rc4_setkey(_this, &_this->recv);
-	mppe_rc4_setkey(_this, &_this->send);
+	if (_this->send.keybits > 0) {
+		mppe_rc4_init(_this, &_this->send, 0);
+		GetNewKeyFromSHA(_this->send.master_key, _this->send.master_key,
+		    _this->send.keylen, _this->send.session_key);
+		mppe_reduce_key(&_this->send);
+		mppe_rc4_setkey(_this, &_this->send);
+	}
+	if (_this->recv.keybits > 0) {
+		mppe_rc4_init(_this, &_this->recv, _this->recv.stateless);
+		GetNewKeyFromSHA(_this->recv.master_key, _this->recv.master_key,
+		    _this->recv.keylen, _this->recv.session_key);
+		mppe_reduce_key(&_this->recv);
+		mppe_rc4_setkey(_this, &_this->recv);
+	}
 }
 
-
 /**
  * creating the mppe bits. In case of first proposal, it specifies the
  * peer_bits as 0 value. If it specifies the peer_bits, it returns the
diff --git a/usr.sbin/npppd/npppd/npppd.c b/usr.sbin/npppd/npppd/npppd.c
index aa64f9ea0b1..7023d0452ed 100644
--- a/usr.sbin/npppd/npppd/npppd.c
+++ b/usr.sbin/npppd/npppd/npppd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: npppd.c,v 1.26 2012/12/05 23:20:26 deraadt Exp $ */
+/*	$OpenBSD: npppd.c,v 1.27 2013/02/13 22:10:38 yasuoka Exp $ */
 
 /*-
  * Copyright (c) 2005-2008,2009 Internet Initiative Japan Inc.
@@ -29,7 +29,7 @@
  * Next pppd(nppd). This file provides a npppd daemon process and operations
  * for npppd instance.
  * @author	Yasuoka Masahiko
- * $Id: npppd.c,v 1.26 2012/12/05 23:20:26 deraadt Exp $
+ * $Id: npppd.c,v 1.27 2013/02/13 22:10:38 yasuoka Exp $
  */
 #include "version.h"
 #include <sys/types.h>
@@ -899,20 +899,22 @@ pipex_setup_common(npppd_ppp *ppp, struct pipex_session_req *req)
 
 #ifdef USE_NPPPD_MPPE
 	req->pr_ccp_id = ppp->ccp.fsm.id;
-	memcpy(req->pr_mppe_send.master_key,
-	    ppp->mppe.send.master_key, sizeof(req->pr_mppe_send.master_key));
-	req->pr_mppe_send.stateless = ppp->mppe.send.stateless;
-	req->pr_mppe_send.keylenbits = ppp->mppe.send.keybits;
-
-	memcpy(req->pr_mppe_recv.master_key,
-	    ppp->mppe.recv.master_key, sizeof(req->pr_mppe_recv.master_key));
-	req->pr_mppe_recv.stateless = ppp->mppe.recv.stateless;
-	req->pr_mppe_recv.keylenbits = ppp->mppe.recv.keybits;
-
-	if (ppp->mppe_started != 0) {
-		req->pr_ppp_flags |= PIPEX_PPP_MPPE_ACCEPTED;
+	if (ppp->mppe.send.keybits > 0) {
+		memcpy(req->pr_mppe_send.master_key,
+		    ppp->mppe.send.master_key,
+		    sizeof(req->pr_mppe_send.master_key));
+		req->pr_mppe_send.stateless = ppp->mppe.send.stateless;
+		req->pr_mppe_send.keylenbits = ppp->mppe.send.keybits;
 		req->pr_ppp_flags |= PIPEX_PPP_MPPE_ENABLED;
 	}
+	if (ppp->mppe.recv.keybits > 0) {
+		memcpy(req->pr_mppe_recv.master_key,
+		    ppp->mppe.recv.master_key,
+		    sizeof(req->pr_mppe_recv.master_key));
+		req->pr_mppe_recv.stateless = ppp->mppe.recv.stateless;
+		req->pr_mppe_recv.keylenbits = ppp->mppe.recv.keybits;
+		req->pr_ppp_flags |= PIPEX_PPP_MPPE_ACCEPTED;
+	}
 	if (ppp->mppe.required)
 		req->pr_ppp_flags |= PIPEX_PPP_MPPE_REQUIRED;
 #endif /* USE_NPPPD_MPPE */
diff --git a/usr.sbin/npppd/npppd/npppd_iface.c b/usr.sbin/npppd/npppd/npppd_iface.c
index fa19fbaaa59..b1ea8fb6ad3 100644
--- a/usr.sbin/npppd/npppd/npppd_iface.c
+++ b/usr.sbin/npppd/npppd/npppd_iface.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: npppd_iface.c,v 1.7 2012/09/18 13:14:08 yasuoka Exp $ */
+/*	$OpenBSD: npppd_iface.c,v 1.8 2013/02/13 22:10:38 yasuoka Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-/* $Id: npppd_iface.c,v 1.7 2012/09/18 13:14:08 yasuoka Exp $ */
+/* $Id: npppd_iface.c,v 1.8 2013/02/13 22:10:38 yasuoka Exp $ */
 /**@file
  * The interface of npppd and kernel.
  * This is an implementation to use tun(4) or pppx(4).
@@ -467,7 +467,7 @@ npppd_iface_network_input_delegate(struct radish *radish, void *args0)
 		if (ppp_iface(ppp) != args->_this)
 			return 0;
 #ifdef	USE_NPPPD_MPPE
-		if (MPPE_READY(ppp)) {
+		if (MPPE_SEND_READY(ppp)) {
 			/* output via MPPE if MPPE started */
 			mppe_pkt_output(&ppp->mppe, PPP_PROTO_IP, args->pktp,
 			    args->lpktp);
@@ -528,7 +528,7 @@ npppd_iface_network_input_ipv4(npppd_iface *_this, u_char *pktp, int lpktp)
 		ppp_reset_idle_timeout(ppp);
 
 #ifdef	USE_NPPPD_MPPE
-	if (MPPE_READY(ppp)) {
+	if (MPPE_SEND_READY(ppp)) {
 		/* output via MPPE if MPPE started */
 		mppe_pkt_output(&ppp->mppe, PPP_PROTO_IP, pktp, lpktp);
 		return;
diff --git a/usr.sbin/npppd/npppd/ppp.c b/usr.sbin/npppd/npppd/ppp.c
index 740ccf708c4..18c86b53601 100644
--- a/usr.sbin/npppd/npppd/ppp.c
+++ b/usr.sbin/npppd/npppd/ppp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ppp.c,v 1.17 2013/01/07 18:12:08 brad Exp $ */
+/*	$OpenBSD: ppp.c,v 1.18 2013/02/13 22:10:38 yasuoka Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-/* $Id: ppp.c,v 1.17 2013/01/07 18:12:08 brad Exp $ */
+/* $Id: ppp.c,v 1.18 2013/02/13 22:10:38 yasuoka Exp $ */
 /**@file
  * This file provides PPP(Point-to-Point Protocol, RFC 1661) and
  * {@link :: _npppd_ppp PPP instance} related functions.
@@ -869,7 +869,7 @@ ppp_recv_packet(npppd_ppp *_this, unsigned char *pkt, int lpkt, int flags)
 
 				return 1;
 			}
-			if (MPPE_READY(_this)) {
+			if (MPPE_RECV_READY(_this)) {
 				/* MPPE is opened but naked ip packet */
 				ppp_log(_this, LOG_WARNING,
 				    "mppe is available but received naked IP.");
@@ -879,7 +879,7 @@ ppp_recv_packet(npppd_ppp *_this, unsigned char *pkt, int lpkt, int flags)
 		break;
 	case PPP_PROTO_MPPE:
 #ifdef USE_NPPPD_MPPE
-		if (_this->mppe_started == 0)  {
+		if (!MPPE_RECV_READY(_this)) {
 #else
 		{
 #endif
diff --git a/usr.sbin/npppd/npppd/ppp.h b/usr.sbin/npppd/npppd/ppp.h
index e01c8260cda..b471be6d1f3 100644
--- a/usr.sbin/npppd/npppd/ppp.h
+++ b/usr.sbin/npppd/npppd/ppp.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ppp.h,v 1.14 2013/01/07 18:12:08 brad Exp $ */
+/*	$OpenBSD: ppp.h,v 1.15 2013/02/13 22:10:38 yasuoka Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -645,7 +645,11 @@ typedef struct _dialin_proxy_info {
 	(((ppp)->mppe.enabled != 0) && ((ppp)->mppe.required != 0))
 
 /** MPPE is ready to use */
-#define	MPPE_READY(ppp) 	((ppp)->mppe_started  != 0)
+#define	MPPE_SEND_READY(ppp)				\
+    ((ppp)->mppe_started != 0 && (ppp)->mppe.send.keybits > 0)
+#define	MPPE_RECV_READY(ppp) 				\
+    ((ppp)->mppe_started != 0 && (ppp)->mppe.recv.keybits > 0)
+
 
 /* Adapted from NetBSD:/usr/src/usr.sbin/pppd/pppd/pppd.h */
 /*