From owner-svn-src-projects@freebsd.org Wed Nov 23 06:51:21 2016 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 0D120C4ECED for ; Wed, 23 Nov 2016 06:51:21 +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 BE998A77; Wed, 23 Nov 2016 06:51:20 +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 uAN6pJvs083231; Wed, 23 Nov 2016 06:51:19 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id uAN6pJlw083230; Wed, 23 Nov 2016 06:51:19 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201611230651.uAN6pJlw083230@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Wed, 23 Nov 2016 06:51:19 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r309032 - 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: Wed, 23 Nov 2016 06:51:21 -0000 Author: ae Date: Wed Nov 23 06:51:19 2016 New Revision: 309032 URL: https://svnweb.freebsd.org/changeset/base/309032 Log: Rework key_add(). Use SADB_CHECKHDR/SADB_CHECKLEN macros and key_checksockaddrs() to do sanity checks. Now key_newsav() will allocate and initialize all needed SA fields. Modified: projects/ipsec/sys/netipsec/key.c Modified: projects/ipsec/sys/netipsec/key.c ============================================================================== --- projects/ipsec/sys/netipsec/key.c Wed Nov 23 06:38:13 2016 (r309031) +++ projects/ipsec/sys/netipsec/key.c Wed Nov 23 06:51:19 2016 (r309032) @@ -4891,40 +4891,6 @@ key_update(struct socket *so, struct mbu } /* - * search SAD with sequence for a SA which state is SADB_SASTATE_LARVAL. - * only called by key_update(). - * OUT: - * NULL : not found - * others : found, pointer to a SA. - */ -#ifdef IPSEC_DOSEQCHECK -static struct secasvar * -key_getsavbyseq(struct secashead *sah, u_int32_t seq) -{ - struct secasvar *sav; - u_int state; - - state = SADB_SASTATE_LARVAL; - - /* search SAD with sequence number ? */ - LIST_FOREACH(sav, &sah->savtree[state], chain) { - - KEY_CHKSASTATE(state, sav->state, __func__); - - if (sav->seq == seq) { - sa_addref(sav); - KEYDEBUG(KEYDEBUG_IPSEC_STAMP, - printf("DP %s cause refcnt++:%d SA:%p\n", - __func__, sav->refcnt, sav)); - return sav; - } - } - - return NULL; -} -#endif - -/* * SADB_ADD processing * add an entry to SA database, when received * msg->sadb_msg_satype)) == 0) { ipseclog((LOG_DEBUG, "%s: invalid satype is passed.\n", - __func__)); + __func__)); return key_senderror(so, m, EINVAL); } - if (mhp->ext[SADB_EXT_SA] == NULL || - mhp->ext[SADB_EXT_ADDRESS_SRC] == NULL || - mhp->ext[SADB_EXT_ADDRESS_DST] == NULL || - (mhp->msg->sadb_msg_satype == SADB_SATYPE_ESP && - mhp->ext[SADB_EXT_KEY_ENCRYPT] == NULL) || - (mhp->msg->sadb_msg_satype == SADB_SATYPE_AH && - mhp->ext[SADB_EXT_KEY_AUTH] == NULL) || - (mhp->ext[SADB_EXT_LIFETIME_HARD] != NULL && - mhp->ext[SADB_EXT_LIFETIME_SOFT] == NULL) || - (mhp->ext[SADB_EXT_LIFETIME_HARD] == NULL && - mhp->ext[SADB_EXT_LIFETIME_SOFT] != NULL)) { - ipseclog((LOG_DEBUG, "%s: invalid message is passed.\n", - __func__)); + if (SADB_CHECKHDR(mhp, SADB_EXT_SA) || + SADB_CHECKHDR(mhp, SADB_EXT_ADDRESS_SRC) || + SADB_CHECKHDR(mhp, SADB_EXT_ADDRESS_DST) || + (mhp->msg->sadb_msg_satype == SADB_SATYPE_ESP && ( + SADB_CHECKHDR(mhp, SADB_EXT_KEY_ENCRYPT) || + SADB_CHECKLEN(mhp, SADB_EXT_KEY_ENCRYPT))) || + (mhp->msg->sadb_msg_satype == SADB_SATYPE_AH && ( + SADB_CHECKHDR(mhp, SADB_EXT_KEY_AUTH) || + SADB_CHECKLEN(mhp, SADB_EXT_KEY_AUTH))) || + (SADB_CHECKHDR(mhp, SADB_EXT_LIFETIME_HARD) && + !SADB_CHECKHDR(mhp, SADB_EXT_LIFETIME_SOFT)) || + (SADB_CHECKHDR(mhp, SADB_EXT_LIFETIME_SOFT) && + !SADB_CHECKHDR(mhp, SADB_EXT_LIFETIME_HARD))) { + ipseclog((LOG_DEBUG, + "%s: invalid message: missing required header.\n", + __func__)); return key_senderror(so, m, EINVAL); } - if (mhp->extlen[SADB_EXT_SA] < sizeof(struct sadb_sa) || - mhp->extlen[SADB_EXT_ADDRESS_SRC] < sizeof(struct sadb_address) || - mhp->extlen[SADB_EXT_ADDRESS_DST] < sizeof(struct sadb_address)) { - /* XXX need more */ - ipseclog((LOG_DEBUG, "%s: invalid message is passed.\n", - __func__)); + if (SADB_CHECKLEN(mhp, SADB_EXT_SA) || + SADB_CHECKLEN(mhp, SADB_EXT_ADDRESS_SRC) || + SADB_CHECKLEN(mhp, SADB_EXT_ADDRESS_DST)) { + ipseclog((LOG_DEBUG, + "%s: invalid message: wrong header size.\n", __func__)); return key_senderror(so, m, EINVAL); } - if (mhp->ext[SADB_X_EXT_SA2] != NULL) { - mode = ((struct sadb_x_sa2 *)mhp->ext[SADB_X_EXT_SA2])->sadb_x_sa2_mode; - reqid = ((struct sadb_x_sa2 *)mhp->ext[SADB_X_EXT_SA2])->sadb_x_sa2_reqid; - } else { + if (SADB_CHECKHDR(mhp, SADB_X_EXT_SA2)) { mode = IPSEC_MODE_ANY; reqid = 0; + } else { + if (SADB_CHECKLEN(mhp, SADB_X_EXT_SA2)) { + ipseclog((LOG_DEBUG, + "%s: invalid message: wrong header size.\n", + __func__)); + return key_senderror(so, m, EINVAL); + } + mode = ((struct sadb_x_sa2 *) + mhp->ext[SADB_X_EXT_SA2])->sadb_x_sa2_mode; + reqid = ((struct sadb_x_sa2 *) + mhp->ext[SADB_X_EXT_SA2])->sadb_x_sa2_reqid; } sa0 = (struct sadb_sa *)mhp->ext[SADB_EXT_SA]; src0 = (struct sadb_address *)mhp->ext[SADB_EXT_ADDRESS_SRC]; dst0 = (struct sadb_address *)mhp->ext[SADB_EXT_ADDRESS_DST]; - /* XXX boundary check against sa_len */ + /* + * Only SADB_SASTATE_MATURE SAs may be submitted in an + * SADB_ADD message. + */ + if (sa0->sadb_sa_state != SADB_SASTATE_MATURE) { + ipseclog((LOG_DEBUG, "%s: invalid state.\n", __func__)); +#ifdef PFKEY_STRICT_CHECKS + return key_senderror(so, m, EINVAL); +#endif + } + error = key_checksockaddrs((struct sockaddr *)(src0 + 1), + (struct sockaddr *)(dst0 + 1)); + if (error != 0) { + ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__)); + return key_senderror(so, m, error); + } KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx); - /* * Make sure the port numbers are zero. * In case of NAT-T we will update them later if needed. @@ -5014,127 +4997,32 @@ key_add(struct socket *so, struct mbuf * KEY_PORTTOSADDR(&saidx.src, 0); KEY_PORTTOSADDR(&saidx.dst, 0); -#ifdef IPSEC_NAT_T - /* - * Handle NAT-T info if present. - */ - if (mhp->ext[SADB_X_EXT_NAT_T_TYPE] != NULL && - mhp->ext[SADB_X_EXT_NAT_T_SPORT] != NULL && - mhp->ext[SADB_X_EXT_NAT_T_DPORT] != NULL) { - struct sadb_x_nat_t_port *sport, *dport; - - if (mhp->extlen[SADB_X_EXT_NAT_T_TYPE] < sizeof(*type) || - mhp->extlen[SADB_X_EXT_NAT_T_SPORT] < sizeof(*sport) || - mhp->extlen[SADB_X_EXT_NAT_T_DPORT] < sizeof(*dport)) { - ipseclog((LOG_DEBUG, "%s: invalid message.\n", - __func__)); - return key_senderror(so, m, EINVAL); - } - - type = (struct sadb_x_nat_t_type *) - mhp->ext[SADB_X_EXT_NAT_T_TYPE]; - sport = (struct sadb_x_nat_t_port *) - mhp->ext[SADB_X_EXT_NAT_T_SPORT]; - dport = (struct sadb_x_nat_t_port *) - mhp->ext[SADB_X_EXT_NAT_T_DPORT]; - - if (sport) - KEY_PORTTOSADDR(&saidx.src, - sport->sadb_x_nat_t_port_port); - if (dport) - KEY_PORTTOSADDR(&saidx.dst, - dport->sadb_x_nat_t_port_port); - } else { - type = NULL; - } - if (mhp->ext[SADB_X_EXT_NAT_T_OAI] != NULL && - mhp->ext[SADB_X_EXT_NAT_T_OAR] != NULL) { - if (mhp->extlen[SADB_X_EXT_NAT_T_OAI] < sizeof(*iaddr) || - mhp->extlen[SADB_X_EXT_NAT_T_OAR] < sizeof(*raddr)) { - ipseclog((LOG_DEBUG, "%s: invalid message\n", - __func__)); - return key_senderror(so, m, EINVAL); - } - iaddr = (struct sadb_address *)mhp->ext[SADB_X_EXT_NAT_T_OAI]; - raddr = (struct sadb_address *)mhp->ext[SADB_X_EXT_NAT_T_OAR]; - ipseclog((LOG_DEBUG, "%s: NAT-T OAi/r present\n", __func__)); - } else { - iaddr = raddr = NULL; - } - if (mhp->ext[SADB_X_EXT_NAT_T_FRAG] != NULL) { - if (mhp->extlen[SADB_X_EXT_NAT_T_FRAG] < sizeof(*frag)) { - ipseclog((LOG_DEBUG, "%s: invalid message\n", - __func__)); - return key_senderror(so, m, EINVAL); - } - frag = (struct sadb_x_nat_t_frag *) - mhp->ext[SADB_X_EXT_NAT_T_FRAG]; - } else { - frag = NULL; - } -#endif - - /* get a SA header */ - if ((newsah = key_getsah(&saidx)) == NULL) { - /* create a new SA header */ - if ((newsah = key_newsah(&saidx)) == NULL) { - ipseclog((LOG_DEBUG, "%s: No more memory.\n",__func__)); - return key_senderror(so, m, ENOBUFS); - } - } - - /* set spidx if there */ - /* XXX rewrite */ - error = key_setident(newsah, m, mhp); - if (error) { - return key_senderror(so, m, error); - } - - /* create new SA entry. */ - /* We can create new SA only if SPI is differenct. */ - SAHTREE_LOCK(); - newsav = key_getsavbyspi(newsah, sa0->sadb_sa_spi); - SAHTREE_UNLOCK(); - if (newsav != NULL) { + /* We can create new SA only if SPI is different. */ + sav = key_getsavbyspi(sa0->sadb_sa_spi); + if (sav != NULL) { + key_freesav(&sav); ipseclog((LOG_DEBUG, "%s: SA already exists.\n", __func__)); return key_senderror(so, m, EEXIST); } - newsav = KEY_NEWSAV(m, mhp, newsah, &error); - if (newsav == NULL) { - return key_senderror(so, m, error); - } -#ifdef IPSEC_NAT_T - /* - * Handle more NAT-T info if present, - * now that we have a sav to fill. - */ - if (type) - newsav->natt_type = type->sadb_x_nat_t_type_type; - -#if 0 - /* - * In case SADB_X_EXT_NAT_T_FRAG was not given, leave it at 0. - * We should actually check for a minimum MTU here, if we - * want to support it in ip_output. - */ - if (frag) - newsav->natt_esp_frag_len = frag->sadb_x_nat_t_frag_fraglen; -#endif -#endif - - /* check SA values to be mature. */ - if ((error = key_mature(newsav)) != 0) { - KEY_FREESAV(&newsav); + sav = key_newsav(mhp, &saidx, sa0->sadb_sa_spi, &error); + if (sav == NULL) return key_senderror(so, m, error); - } - + KEYDBG(KEY_STAMP, + printf("%s: return SA(%p)\n", __func__, sav)); + KEYDBG(KEY_DATA, kdebug_secasv(sav)); /* - * don't call key_freesav() here, as we would like to keep the SA - * in the database on success. + * If SADB_ADD was in response to SADB_ACQUIRE, we need to schedule + * ACQ for deletion. */ + if (sav->seq != 0) + key_acqdone(&saidx, sav->seq); { + /* + * Don't call key_freesav() on error here, as we would like to + * keep the SA in the database. + */ struct mbuf *n; /* set msg buf from mhp */