From owner-svn-src-projects@freebsd.org Tue Nov 22 17:09:18 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 641D6C5076F for ; Tue, 22 Nov 2016 17:09:18 +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 3BA2898F; Tue, 22 Nov 2016 17:09:18 +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 uAMH9Hqh043245; Tue, 22 Nov 2016 17:09:17 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id uAMH9HWw043244; Tue, 22 Nov 2016 17:09:17 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201611221709.uAMH9HWw043244@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Tue, 22 Nov 2016 17:09:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r309012 - 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: Tue, 22 Nov 2016 17:09:18 -0000 Author: ae Date: Tue Nov 22 17:09:17 2016 New Revision: 309012 URL: https://svnweb.freebsd.org/changeset/base/309012 Log: Modify key_satype2proto() and key_proto2satype() to use uint8_t type. Modify key_getspi() to use SADB_CHECKHDR() and SADB_CHECKLEN() macros, also use key_checksockaddrs() to check addresses. Modified: projects/ipsec/sys/netipsec/key.c Modified: projects/ipsec/sys/netipsec/key.c ============================================================================== --- projects/ipsec/sys/netipsec/key.c Tue Nov 22 16:43:41 2016 (r309011) +++ projects/ipsec/sys/netipsec/key.c Tue Nov 22 17:09:17 2016 (r309012) @@ -4367,8 +4367,8 @@ key_randomfill(void *p, size_t l) * OUT: * 0: invalid satype. */ -static u_int16_t -key_satype2proto(u_int8_t satype) +static uint8_t +key_satype2proto(uint8_t satype) { switch (satype) { case SADB_SATYPE_UNSPEC: @@ -4392,8 +4392,8 @@ key_satype2proto(u_int8_t satype) * OUT: * 0: invalid protocol type. */ -static u_int8_t -key_proto2satype(u_int16_t proto) +static uint8_t +key_proto2satype(uint8_t proto) { switch (proto) { case IPPROTO_AH: @@ -4426,39 +4426,56 @@ key_proto2satype(u_int16_t proto) static int key_getspi(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) { - struct sadb_address *src0, *dst0; struct secasindex saidx; - struct secashead *newsah; - struct secasvar *newsav; - u_int8_t proto; - u_int32_t spi; - u_int8_t mode; - u_int32_t reqid; + struct sadb_address *src0, *dst0; + struct secasvar *sav; + uint32_t reqid, spi; int error; + uint8_t mode, proto; IPSEC_ASSERT(so != NULL, ("null socket")); IPSEC_ASSERT(m != NULL, ("null mbuf")); IPSEC_ASSERT(mhp != NULL, ("null msghdr")); IPSEC_ASSERT(mhp->msg != NULL, ("null msg")); - if (mhp->ext[SADB_EXT_ADDRESS_SRC] == NULL || - mhp->ext[SADB_EXT_ADDRESS_DST] == NULL) { - ipseclog((LOG_DEBUG, "%s: invalid message is passed.\n", - __func__)); - return key_senderror(so, m, EINVAL); + if (SADB_CHECKHDR(mhp, SADB_EXT_ADDRESS_SRC) || + SADB_CHECKHDR(mhp, SADB_EXT_ADDRESS_DST) +#ifdef PFKEY_STRICT_CHECKS + || SADB_CHECKHDR(mhp, SADB_EXT_SPIRANGE) +#endif + ) { + ipseclog((LOG_DEBUG, + "%s: invalid message: missing required header.\n", + __func__)); + error = EINVAL; + goto fail; } - if (mhp->extlen[SADB_EXT_ADDRESS_SRC] < sizeof(struct sadb_address) || - mhp->extlen[SADB_EXT_ADDRESS_DST] < sizeof(struct sadb_address)) { - ipseclog((LOG_DEBUG, "%s: invalid message is passed.\n", - __func__)); - return key_senderror(so, m, EINVAL); + if (SADB_CHECKLEN(mhp, SADB_EXT_ADDRESS_SRC) || + SADB_CHECKLEN(mhp, SADB_EXT_ADDRESS_DST) +#ifdef PFKEY_STRICT_CHECKS + || SADB_CHECKLEN(mhp, SADB_EXT_SPIRANGE) +#endif + ) { + ipseclog((LOG_DEBUG, + "%s: invalid message: wrong header size.\n", __func__)); + error = EINVAL; + goto fail; } - 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__)); + error = EINVAL; + goto fail; + } + 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; } src0 = (struct sadb_address *)(mhp->ext[SADB_EXT_ADDRESS_SRC]); @@ -4468,121 +4485,61 @@ key_getspi(struct socket *so, struct mbu if ((proto = key_satype2proto(mhp->msg->sadb_msg_satype)) == 0) { ipseclog((LOG_DEBUG, "%s: invalid satype is passed.\n", __func__)); - return key_senderror(so, m, EINVAL); - } - - /* - * Make sure the port numbers are zero. - * In case of NAT-T we will update them later if needed. - */ - switch (((struct sockaddr *)(src0 + 1))->sa_family) { - case AF_INET: - if (((struct sockaddr *)(src0 + 1))->sa_len != - sizeof(struct sockaddr_in)) - return key_senderror(so, m, EINVAL); - ((struct sockaddr_in *)(src0 + 1))->sin_port = 0; - break; - case AF_INET6: - if (((struct sockaddr *)(src0 + 1))->sa_len != - sizeof(struct sockaddr_in6)) - return key_senderror(so, m, EINVAL); - ((struct sockaddr_in6 *)(src0 + 1))->sin6_port = 0; - break; - default: - ; /*???*/ + error = EINVAL; + goto fail; } - switch (((struct sockaddr *)(dst0 + 1))->sa_family) { - case AF_INET: - if (((struct sockaddr *)(dst0 + 1))->sa_len != - sizeof(struct sockaddr_in)) - return key_senderror(so, m, EINVAL); - ((struct sockaddr_in *)(dst0 + 1))->sin_port = 0; - break; - case AF_INET6: - if (((struct sockaddr *)(dst0 + 1))->sa_len != - sizeof(struct sockaddr_in6)) - return key_senderror(so, m, EINVAL); - ((struct sockaddr_in6 *)(dst0 + 1))->sin6_port = 0; - break; - default: - ; /*???*/ + error = key_checksockaddrs((struct sockaddr *)(src0 + 1), + (struct sockaddr *)(dst0 + 1)); + if (error != 0) { + ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__)); + error = EINVAL; + goto fail; } - - /* XXX boundary check against sa_len */ KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx); - -#ifdef IPSEC_NAT_T /* - * Handle NAT-T info if present. - * We made sure the port numbers are zero above, so we do - * not have to worry in case we do not update them. + * Make sure the port numbers are zero. + * In case of NAT-T we will update them later if needed. */ - if (mhp->ext[SADB_X_EXT_NAT_T_OAI] != NULL) - ipseclog((LOG_DEBUG, "%s: NAT-T OAi present\n", __func__)); - if (mhp->ext[SADB_X_EXT_NAT_T_OAR] != NULL) - ipseclog((LOG_DEBUG, "%s: NAT-T OAr present\n", __func__)); - - 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_type *type; - 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 nat-t message " - "passed.\n", __func__)); - return key_senderror(so, m, EINVAL); - } - - 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); - } -#endif + KEY_PORTTOSADDR(&saidx.src, 0); + KEY_PORTTOSADDR(&saidx.dst, 0); /* SPI allocation */ - spi = key_do_getnewspi((struct sadb_spirange *)mhp->ext[SADB_EXT_SPIRANGE], - &saidx); - if (spi == 0) - return key_senderror(so, m, EINVAL); - - /* get a SA index */ - if ((newsah = key_getsah(&saidx)) == NULL) { - /* create a new SA index */ - if ((newsah = key_newsah(&saidx)) == NULL) { - ipseclog((LOG_DEBUG, "%s: No more memory.\n",__func__)); - return key_senderror(so, m, ENOBUFS); - } + spi = key_do_getnewspi( + (struct sadb_spirange *)mhp->ext[SADB_EXT_SPIRANGE], &saidx); + if (spi == 0) { + /* + * Requested SPI or SPI range is not available or + * already used. + */ + error = EEXIST; + goto fail; } + sav = key_newsav(mhp, &saidx, spi, &error); + if (sav == NULL) + goto fail; - /* get a new SA */ - /* XXX rewrite */ - newsav = KEY_NEWSAV(m, mhp, newsah, &error); - if (newsav == NULL) { - /* XXX don't free new SA index allocated in above. */ - return key_senderror(so, m, error); + if (sav->seq != 0) { + /* + * RFC2367: + * If the SADB_GETSPI message is in response to a + * kernel-generated SADB_ACQUIRE, the sadb_msg_seq + * MUST be the same as the SADB_ACQUIRE message. + * + * XXXAE: However it doesn't definethe behaviour how to + * check this and what to do if it doesn't match. + * Also what we should do if it matches? + * + * We can compare saidx used in SADB_ACQUIRE with saidx + * used in SADB_GETSPI, but this probably can break + * existing software. For now just warn if it doesn't match. + * + * XXXAE: anyway it looks useless. + */ + key_acqdone(&saidx, sav->seq); } - - /* set spi */ - newsav->spi = htonl(spi); - - /* delete the entry in acqtree */ - if (mhp->msg->sadb_msg_seq != 0) { - struct secacq *acq; - if ((acq = key_getacqbyseq(mhp->msg->sadb_msg_seq)) != NULL) { - /* reset counter in order to deletion by timehandler. */ - acq->created = time_second; - acq->count = 0; - } - } + KEYDBG(KEY_STAMP, + printf("%s: SA(%p)\n", __func__, sav)); + KEYDBG(KEY_DATA, kdebug_secasv(sav)); { struct mbuf *n, *nn; @@ -4601,8 +4558,10 @@ key_getspi(struct socket *so, struct mbu n = NULL; } } - if (!n) - return key_senderror(so, m, ENOBUFS); + if (!n) { + error = ENOBUFS; + goto fail; + } n->m_len = len; n->m_next = NULL; @@ -4614,7 +4573,7 @@ key_getspi(struct socket *so, struct mbu m_sa = (struct sadb_sa *)(mtod(n, caddr_t) + off); m_sa->sadb_sa_len = PFKEY_UNIT64(sizeof(struct sadb_sa)); m_sa->sadb_sa_exttype = SADB_EXT_SA; - m_sa->sadb_sa_spi = htonl(spi); + m_sa->sadb_sa_spi = spi; /* SPI is already in network byte order */ off += PFKEY_ALIGN8(sizeof(struct sadb_sa)); IPSEC_ASSERT(off == len, @@ -4624,7 +4583,8 @@ key_getspi(struct socket *so, struct mbu SADB_EXT_ADDRESS_DST); if (!n->m_next) { m_freem(n); - return key_senderror(so, m, ENOBUFS); + error = ENOBUFS; + goto fail; } if (n->m_len < sizeof(struct sadb_msg)) { @@ -4638,13 +4598,16 @@ key_getspi(struct socket *so, struct mbu n->m_pkthdr.len += nn->m_len; newmsg = mtod(n, struct sadb_msg *); - newmsg->sadb_msg_seq = newsav->seq; + newmsg->sadb_msg_seq = sav->seq; newmsg->sadb_msg_errno = 0; newmsg->sadb_msg_len = PFKEY_UNIT64(n->m_pkthdr.len); m_freem(m); return key_sendup_mbuf(so, n, KEY_SENDUP_ONE); } + +fail: + return (key_senderror(so, m, error)); } /*