From owner-svn-src-projects@freebsd.org Fri Jan 13 12:59:30 2017 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7829BCAEE76 for ; Fri, 13 Jan 2017 12:59:30 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 293BA12DD; Fri, 13 Jan 2017 12:59:30 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v0DCxT7E060007; Fri, 13 Jan 2017 12:59:29 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v0DCxT5O060006; Fri, 13 Jan 2017 12:59:29 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201701131259.v0DCxT5O060006@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Fri, 13 Jan 2017 12:59:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r312070 - projects/ipsec/sys/netipsec X-SVN-Group: projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Jan 2017 12:59:30 -0000 Author: ae Date: Fri Jan 13 12:59:29 2017 New Revision: 312070 URL: https://svnweb.freebsd.org/changeset/base/312070 Log: Require the presence of tunnel endpoints addresses for tunnel mode IPsec requets. When we are parsing SADB_X_SPDADD message, check that specified IPsec request contains tunnel endpoints addresses for tunnel mode requests. setkey(8) doesn't allow create policies with empty tunnel specification like 'esp/tunnel//require'. But it is possible to receive such policies from IKE. This leads to the presence of zero filled sockaddr structures in the IPsec request. Almost all the IPsec code assumes that sockaddr structures have correctly filled sa_len and sa_family fields. We use addresses from IPsec request for outbound SA lookup in ipsec[46]_allocsa() functions. Using these addresses key_allocsa_policy() chooses bucket from SAHADDRHASH table when does lookup for outbound SA. Also when check_policy_history is enabled ipsec_check_history() uses destination address to check that packet was handled by correct SA when we do inbound policy check. It appears that racoon sometimes can install such policies. Previously this was not an error due to several reasons. First of, we didn't have any hash tables and did the lookup among all SAs (no need to choose the bucket where lookup will be done). Second, since sa_family is zero, key_sockaddrcmp() has used bcmp() to compare full sockaddr structures, but since sa_len is zero too, it always matches addresses. In some cases this can be considered as feature, because we can use empty tunnel specification like an "any" selector. But for now I think it is a bug, because the lookup results are not predictable and any changes in SADB (new SA added or deleted) may change the lookup result. Note that linux's PF_KEY implementation also considers such policies as invalid. Reported by: Matthew Grooms Modified: projects/ipsec/sys/netipsec/key.c Modified: projects/ipsec/sys/netipsec/key.c ============================================================================== --- projects/ipsec/sys/netipsec/key.c Fri Jan 13 12:47:44 2017 (r312069) +++ projects/ipsec/sys/netipsec/key.c Fri Jan 13 12:59:29 2017 (r312070) @@ -1499,7 +1499,6 @@ key_msg2sp(struct sadb_x_policy *xpl0, s isr->level = xisr->sadb_x_ipsecrequest_level; /* set IP addresses if there */ - /* XXXAE: those are needed only for tunnel mode */ if (xisr->sadb_x_ipsecrequest_len > sizeof(*xisr)) { struct sockaddr *paddr; @@ -1539,6 +1538,18 @@ key_msg2sp(struct sadb_x_policy *xpl0, s return (NULL); } bcopy(paddr, &isr->saidx.dst, paddr->sa_len); + } else { + /* + * Addresses for TUNNEL mode requests are + * mandatory. + */ + if (isr->saidx.mode == IPSEC_MODE_TUNNEL) { + ipseclog((LOG_DEBUG, "%s: missing ", + "request addresses.\n", __func__)); + key_freesp(&newsp); + *error = EINVAL; + return (NULL); + } } tlen -= xisr->sadb_x_ipsecrequest_len;