svn commit: r312070 - projects/ipsec/sys/netipsec

[Andrey V. Elsukov]

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;