From owner-svn-src-stable-12@freebsd.org Mon Nov 30 09:18:14 2020 Return-Path: Delivered-To: svn-src-stable-12@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 702FA4745F8; Mon, 30 Nov 2020 09:18:14 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Cl05L2lxCz4njt; Mon, 30 Nov 2020 09:18:14 +0000 (UTC) (envelope-from tuexen@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 mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5136F21111; Mon, 30 Nov 2020 09:18:14 +0000 (UTC) (envelope-from tuexen@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0AU9IEq1090256; Mon, 30 Nov 2020 09:18:14 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0AU9IDcJ090251; Mon, 30 Nov 2020 09:18:13 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <202011300918.0AU9IDcJ090251@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Mon, 30 Nov 2020 09:18:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r368178 - stable/12/sys/netinet X-SVN-Group: stable-12 X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: stable/12/sys/netinet X-SVN-Commit-Revision: 368178 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-12@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for only the 12-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Nov 2020 09:18:14 -0000 Author: tuexen Date: Mon Nov 30 09:18:13 2020 New Revision: 368178 URL: https://svnweb.freebsd.org/changeset/base/368178 Log: MFC r366750: Improve the handling of cookie life times. The staleness reported in an error cause is in us, not ms. Enforce limits on the life time via sysct; and socket options consistently. Update the description of the sysctl variable to use the right unit. Also do some minor cleanups. This also fixes an interger overflow issue if the peer can modify the cookie. This was reported by Felix Weinrank by fuzz testing the userland stack and in https://oss-fuzz.com/testcase-detail/4800394024452096 Modified: stable/12/sys/netinet/sctp.h stable/12/sys/netinet/sctp_input.c stable/12/sys/netinet/sctp_output.c stable/12/sys/netinet/sctp_sysctl.h stable/12/sys/netinet/sctp_usrreq.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/netinet/sctp.h ============================================================================== --- stable/12/sys/netinet/sctp.h Mon Nov 30 09:16:51 2020 (r368177) +++ stable/12/sys/netinet/sctp.h Mon Nov 30 09:18:13 2020 (r368178) @@ -605,6 +605,7 @@ struct sctp_error_auth_invalid_hmac { */ #define SCTP_MAX_SACK_DELAY 500 /* per RFC4960 */ #define SCTP_MAX_HB_INTERVAL 14400000 /* 4 hours in ms */ +#define SCTP_MIN_COOKIE_LIFE 1000 /* 1 second in ms */ #define SCTP_MAX_COOKIE_LIFE 3600000 /* 1 hour in ms */ Modified: stable/12/sys/netinet/sctp_input.c ============================================================================== --- stable/12/sys/netinet/sctp_input.c Mon Nov 30 09:16:51 2020 (r368177) +++ stable/12/sys/netinet/sctp_input.c Mon Nov 30 09:18:13 2020 (r368178) @@ -1168,13 +1168,10 @@ sctp_handle_error(struct sctp_chunkhdr *ch, struct sctp_error_stale_cookie *stale_cookie; stale_cookie = (struct sctp_error_stale_cookie *)cause; - asoc->cookie_preserve_req = ntohl(stale_cookie->stale_time); - /* Double it to be more robust on RTX */ - if (asoc->cookie_preserve_req <= UINT32_MAX / 2) { - asoc->cookie_preserve_req *= 2; - } else { - asoc->cookie_preserve_req = UINT32_MAX; - } + /* stable_time is in usec, convert to msec. */ + asoc->cookie_preserve_req = ntohl(stale_cookie->stale_time) / 1000; + /* Double it to be more robust on RTX. */ + asoc->cookie_preserve_req *= 2; asoc->stale_cookie_count++; if (asoc->stale_cookie_count > asoc->max_init_times) { @@ -2263,7 +2260,7 @@ sctp_handle_cookie_echo(struct mbuf *m, int iphlen, in unsigned int sig_offset, cookie_offset; unsigned int cookie_len; struct timeval now; - struct timeval time_expires; + struct timeval time_entered, time_expires; int notification = 0; struct sctp_nets *netl; int had_a_existing_tcb = 0; @@ -2391,13 +2388,30 @@ sctp_handle_cookie_echo(struct mbuf *m, int iphlen, in return (NULL); } + if (sctp_ticks_to_msecs(cookie->cookie_life) > SCTP_MAX_COOKIE_LIFE) { + SCTPDBG(SCTP_DEBUG_INPUT2, "handle_cookie_echo: Invalid cookie lifetime\n"); + return (NULL); + } + time_entered.tv_sec = cookie->time_entered.tv_sec; + time_entered.tv_usec = cookie->time_entered.tv_usec; + if ((time_entered.tv_sec < 0) || + (time_entered.tv_usec < 0) || + (time_entered.tv_usec >= 1000000)) { + /* Invalid time stamp. Cookie must have been modified. */ + SCTPDBG(SCTP_DEBUG_INPUT2, "handle_cookie_echo: Invalid time stamp\n"); + return (NULL); + } + (void)SCTP_GETTIME_TIMEVAL(&now); + if (timevalcmp(&now, &time_entered, <)) { + SCTPDBG(SCTP_DEBUG_INPUT2, "handle_cookie_echo: cookie generated in the future!\n"); + return (NULL); + } /* - * check the cookie timestamps to be sure it's not stale + * Check the cookie timestamps to be sure it's not stale. + * cookie_life is in ticks, so we convert to seconds. */ - (void)SCTP_GETTIME_TIMEVAL(&now); - /* Expire time is in Ticks, so we convert to seconds */ - time_expires.tv_sec = cookie->time_entered.tv_sec + sctp_ticks_to_secs(cookie->cookie_life); - time_expires.tv_usec = cookie->time_entered.tv_usec; + time_expires.tv_sec = time_entered.tv_sec + sctp_ticks_to_secs(cookie->cookie_life); + time_expires.tv_usec = time_entered.tv_usec; if (timevalcmp(&now, &time_expires, >)) { /* cookie is stale! */ struct mbuf *op_err; @@ -2415,8 +2429,7 @@ sctp_handle_cookie_echo(struct mbuf *m, int iphlen, in SCTP_BUF_LEN(op_err) = sizeof(struct sctp_error_stale_cookie); cause = mtod(op_err, struct sctp_error_stale_cookie *); cause->cause.code = htons(SCTP_CAUSE_STALE_COOKIE); - cause->cause.length = htons((sizeof(struct sctp_paramhdr) + - (sizeof(uint32_t)))); + cause->cause.length = htons(sizeof(struct sctp_error_stale_cookie)); diff = now; timevalsub(&diff, &time_expires); if ((uint32_t)diff.tv_sec > UINT32_MAX / 1000000) { Modified: stable/12/sys/netinet/sctp_output.c ============================================================================== --- stable/12/sys/netinet/sctp_output.c Mon Nov 30 09:16:51 2020 (r368177) +++ stable/12/sys/netinet/sctp_output.c Mon Nov 30 09:18:13 2020 (r368178) @@ -4833,7 +4833,7 @@ sctp_send_initiate(struct sctp_inpcb *inp, struct sctp } /* now any cookie time extensions */ - if (stcb->asoc.cookie_preserve_req) { + if (stcb->asoc.cookie_preserve_req > 0) { struct sctp_cookie_perserve_param *cookie_preserve; if (padding_len > 0) { Modified: stable/12/sys/netinet/sctp_sysctl.h ============================================================================== --- stable/12/sys/netinet/sctp_sysctl.h Mon Nov 30 09:16:51 2020 (r368177) +++ stable/12/sys/netinet/sctp_sysctl.h Mon Nov 30 09:18:13 2020 (r368178) @@ -318,10 +318,10 @@ struct sctp_sysctl { #define SCTPCTL_INIT_RTO_MAX_MAX 0xFFFFFFFF #define SCTPCTL_INIT_RTO_MAX_DEFAULT SCTP_RTO_UPPER_BOUND -/* valid_cookie_life: Default cookie lifetime in sec */ -#define SCTPCTL_VALID_COOKIE_LIFE_DESC "Default cookie lifetime in seconds" -#define SCTPCTL_VALID_COOKIE_LIFE_MIN 0 -#define SCTPCTL_VALID_COOKIE_LIFE_MAX 0xFFFFFFFF +/* valid_cookie_life: Default cookie lifetime in ms */ +#define SCTPCTL_VALID_COOKIE_LIFE_DESC "Default cookie lifetime in ms" +#define SCTPCTL_VALID_COOKIE_LIFE_MIN SCTP_MIN_COOKIE_LIFE +#define SCTPCTL_VALID_COOKIE_LIFE_MAX SCTP_MAX_COOKIE_LIFE #define SCTPCTL_VALID_COOKIE_LIFE_DEFAULT SCTP_DEFAULT_COOKIE_LIFE /* init_rtx_max: Default maximum number of retransmission for INIT chunks */ Modified: stable/12/sys/netinet/sctp_usrreq.c ============================================================================== --- stable/12/sys/netinet/sctp_usrreq.c Mon Nov 30 09:16:51 2020 (r368177) +++ stable/12/sys/netinet/sctp_usrreq.c Mon Nov 30 09:18:13 2020 (r368178) @@ -5691,18 +5691,20 @@ sctp_setopt(struct socket *so, int optname, void *optv SCTP_CHECK_AND_CAST(sasoc, optval, struct sctp_assocparams, optsize); SCTP_FIND_STCB(inp, stcb, sasoc->sasoc_assoc_id); - if (sasoc->sasoc_cookie_life) { + if (sasoc->sasoc_cookie_life > 0) { /* boundary check the cookie life */ - if (sasoc->sasoc_cookie_life < 1000) - sasoc->sasoc_cookie_life = 1000; + if (sasoc->sasoc_cookie_life < SCTP_MIN_COOKIE_LIFE) { + sasoc->sasoc_cookie_life = SCTP_MIN_COOKIE_LIFE; + } if (sasoc->sasoc_cookie_life > SCTP_MAX_COOKIE_LIFE) { sasoc->sasoc_cookie_life = SCTP_MAX_COOKIE_LIFE; } } if (stcb) { - if (sasoc->sasoc_asocmaxrxt) + if (sasoc->sasoc_asocmaxrxt > 0) { stcb->asoc.max_send_times = sasoc->sasoc_asocmaxrxt; - if (sasoc->sasoc_cookie_life) { + } + if (sasoc->sasoc_cookie_life > 0) { stcb->asoc.cookie_life = sctp_msecs_to_ticks(sasoc->sasoc_cookie_life); } SCTP_TCB_UNLOCK(stcb); @@ -5712,9 +5714,10 @@ sctp_setopt(struct socket *so, int optname, void *optv ((inp->sctp_flags & SCTP_PCB_FLAGS_UDPTYPE) && (sasoc->sasoc_assoc_id == SCTP_FUTURE_ASSOC))) { SCTP_INP_WLOCK(inp); - if (sasoc->sasoc_asocmaxrxt) + if (sasoc->sasoc_asocmaxrxt > 0) { inp->sctp_ep.max_send_times = sasoc->sasoc_asocmaxrxt; - if (sasoc->sasoc_cookie_life) { + } + if (sasoc->sasoc_cookie_life > 0) { inp->sctp_ep.def_cookie_life = sctp_msecs_to_ticks(sasoc->sasoc_cookie_life); } SCTP_INP_WUNLOCK(inp);