d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve the handling of cookie life times.

Browse Source 
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
This commit is contained in:
Michael Tuexen 2020-10-16 10:44:48 +00:00
parent 139c09788b
commit a92d501617
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=366750
5 changed files with 45 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/netinet/sctp.h
View File

@ -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

45
sys/netinet/sctp_input.c
View File

@ -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, int offset,
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, int offset,
return (NULL);
}
/*
* check the cookie timestamps to be sure it's not stale
*/
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);
/* 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;
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.
* cookie_life is in ticks, so we convert to seconds.
*/
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, int offset,
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) {

2
sys/netinet/sctp_output.c
View File

@ -4814,7 +4814,7 @@ sctp_send_initiate(struct sctp_inpcb *inp, struct sctp_tcb *stcb, int so_locked)
}
/* 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) {

8
sys/netinet/sctp_sysctl.h
View File

@ -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 */

17
sys/netinet/sctp_usrreq.c
View File

@ -5699,18 +5699,20 @@ sctp_setopt(struct socket *so, int optname, void *optval, size_t optsize,
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 *optval, size_t optsize,
((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);