Date: Fri, 13 Jan 2017 12:59:29 +0000 (UTC) From: "Andrey V. Elsukov" <ae@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r312070 - projects/ipsec/sys/netipsec Message-ID: <201701131259.v0DCxT5O060006@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 <mgrooms shrew net> 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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201701131259.v0DCxT5O060006>