svn commit: r368178 - stable/12/sys/netinet

Michael Tuexen tuexen at FreeBSD.org
Mon Nov 30 09:18:14 UTC 2020


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);


More information about the svn-src-all mailing list