From owner-svn-src-all@freebsd.org Fri Oct 16 10:44:50 2020 Return-Path: Delivered-To: svn-src-all@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 7F783430583; Fri, 16 Oct 2020 10:44:50 +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 4CCN822vFzz4l6y; Fri, 16 Oct 2020 10:44:50 +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 448C0F04A; Fri, 16 Oct 2020 10:44:50 +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 09GAior4048947; Fri, 16 Oct 2020 10:44:50 GMT (envelope-from tuexen@FreeBSD.org) Received: (from tuexen@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 09GAinwx048942; Fri, 16 Oct 2020 10:44:49 GMT (envelope-from tuexen@FreeBSD.org) Message-Id: <202010161044.09GAinwx048942@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: tuexen set sender to tuexen@FreeBSD.org using -f From: Michael Tuexen Date: Fri, 16 Oct 2020 10:44:49 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r366750 - head/sys/netinet X-SVN-Group: head X-SVN-Commit-Author: tuexen X-SVN-Commit-Paths: head/sys/netinet X-SVN-Commit-Revision: 366750 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Oct 2020 10:44:50 -0000 Author: tuexen Date: Fri Oct 16 10:44:48 2020 New Revision: 366750 URL: https://svnweb.freebsd.org/changeset/base/366750 Log: 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 MFC after: 3 days Modified: head/sys/netinet/sctp.h head/sys/netinet/sctp_input.c head/sys/netinet/sctp_output.c head/sys/netinet/sctp_sysctl.h head/sys/netinet/sctp_usrreq.c Modified: head/sys/netinet/sctp.h ============================================================================== --- head/sys/netinet/sctp.h Fri Oct 16 10:10:09 2020 (r366749) +++ head/sys/netinet/sctp.h Fri Oct 16 10:44:48 2020 (r366750) @@ -599,6 +599,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 */ /* Types of logging/KTR tracing that can be enabled via the Modified: head/sys/netinet/sctp_input.c ============================================================================== --- head/sys/netinet/sctp_input.c Fri Oct 16 10:10:09 2020 (r366749) +++ head/sys/netinet/sctp_input.c Fri Oct 16 10:44:48 2020 (r366750) @@ -1164,13 +1164,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) { @@ -2254,7 +2251,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; @@ -2382,13 +2379,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; @@ -2406,8 +2420,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: head/sys/netinet/sctp_output.c ============================================================================== --- head/sys/netinet/sctp_output.c Fri Oct 16 10:10:09 2020 (r366749) +++ head/sys/netinet/sctp_output.c Fri Oct 16 10:44:48 2020 (r366750) @@ -4814,7 +4814,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: head/sys/netinet/sctp_sysctl.h ============================================================================== --- head/sys/netinet/sctp_sysctl.h Fri Oct 16 10:10:09 2020 (r366749) +++ head/sys/netinet/sctp_sysctl.h Fri Oct 16 10:44:48 2020 (r366750) @@ -317,10 +317,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: head/sys/netinet/sctp_usrreq.c ============================================================================== --- head/sys/netinet/sctp_usrreq.c Fri Oct 16 10:10:09 2020 (r366749) +++ head/sys/netinet/sctp_usrreq.c Fri Oct 16 10:44:48 2020 (r366750) @@ -5699,18 +5699,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); @@ -5720,9 +5722,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);