Commit 68d27bad authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'mptcp-better-validation-of-mptcpopt_mp_join-option'

Eric Dumazet says:

====================
mptcp: better validation of MPTCPOPT_MP_JOIN option

Based on a syzbot report (see 4th patch in the series).

We need to be more explicit about which one of the
following flag is set by mptcp_parse_option():

- OPTION_MPTCP_MPJ_SYN
- OPTION_MPTCP_MPJ_SYNACK
- OPTION_MPTCP_MPJ_ACK

Then select the appropriate values instead of OPTIONS_MPTCP_MPJ

Paolo suggested to do the same for OPTIONS_MPTCP_MPC (5th patch)
====================

Link: https://lore.kernel.org/r/20240111194917.4044654-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents cbdd50ec 724b00c1
...@@ -123,8 +123,8 @@ static void mptcp_parse_option(const struct sk_buff *skb, ...@@ -123,8 +123,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
break; break;
case MPTCPOPT_MP_JOIN: case MPTCPOPT_MP_JOIN:
mp_opt->suboptions |= OPTIONS_MPTCP_MPJ;
if (opsize == TCPOLEN_MPTCP_MPJ_SYN) { if (opsize == TCPOLEN_MPTCP_MPJ_SYN) {
mp_opt->suboptions |= OPTION_MPTCP_MPJ_SYN;
mp_opt->backup = *ptr++ & MPTCPOPT_BACKUP; mp_opt->backup = *ptr++ & MPTCPOPT_BACKUP;
mp_opt->join_id = *ptr++; mp_opt->join_id = *ptr++;
mp_opt->token = get_unaligned_be32(ptr); mp_opt->token = get_unaligned_be32(ptr);
...@@ -135,6 +135,7 @@ static void mptcp_parse_option(const struct sk_buff *skb, ...@@ -135,6 +135,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
mp_opt->backup, mp_opt->join_id, mp_opt->backup, mp_opt->join_id,
mp_opt->token, mp_opt->nonce); mp_opt->token, mp_opt->nonce);
} else if (opsize == TCPOLEN_MPTCP_MPJ_SYNACK) { } else if (opsize == TCPOLEN_MPTCP_MPJ_SYNACK) {
mp_opt->suboptions |= OPTION_MPTCP_MPJ_SYNACK;
mp_opt->backup = *ptr++ & MPTCPOPT_BACKUP; mp_opt->backup = *ptr++ & MPTCPOPT_BACKUP;
mp_opt->join_id = *ptr++; mp_opt->join_id = *ptr++;
mp_opt->thmac = get_unaligned_be64(ptr); mp_opt->thmac = get_unaligned_be64(ptr);
...@@ -145,11 +146,10 @@ static void mptcp_parse_option(const struct sk_buff *skb, ...@@ -145,11 +146,10 @@ static void mptcp_parse_option(const struct sk_buff *skb,
mp_opt->backup, mp_opt->join_id, mp_opt->backup, mp_opt->join_id,
mp_opt->thmac, mp_opt->nonce); mp_opt->thmac, mp_opt->nonce);
} else if (opsize == TCPOLEN_MPTCP_MPJ_ACK) { } else if (opsize == TCPOLEN_MPTCP_MPJ_ACK) {
mp_opt->suboptions |= OPTION_MPTCP_MPJ_ACK;
ptr += 2; ptr += 2;
memcpy(mp_opt->hmac, ptr, MPTCPOPT_HMAC_LEN); memcpy(mp_opt->hmac, ptr, MPTCPOPT_HMAC_LEN);
pr_debug("MP_JOIN hmac"); pr_debug("MP_JOIN hmac");
} else {
mp_opt->suboptions &= ~OPTIONS_MPTCP_MPJ;
} }
break; break;
......
...@@ -157,8 +157,8 @@ static int subflow_check_req(struct request_sock *req, ...@@ -157,8 +157,8 @@ static int subflow_check_req(struct request_sock *req,
mptcp_get_options(skb, &mp_opt); mptcp_get_options(skb, &mp_opt);
opt_mp_capable = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPC); opt_mp_capable = !!(mp_opt.suboptions & OPTION_MPTCP_MPC_SYN);
opt_mp_join = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ); opt_mp_join = !!(mp_opt.suboptions & OPTION_MPTCP_MPJ_SYN);
if (opt_mp_capable) { if (opt_mp_capable) {
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MPCAPABLEPASSIVE); SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MPCAPABLEPASSIVE);
...@@ -254,8 +254,8 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req, ...@@ -254,8 +254,8 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
subflow_init_req(req, sk_listener); subflow_init_req(req, sk_listener);
mptcp_get_options(skb, &mp_opt); mptcp_get_options(skb, &mp_opt);
opt_mp_capable = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPC); opt_mp_capable = !!(mp_opt.suboptions & OPTION_MPTCP_MPC_ACK);
opt_mp_join = !!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ); opt_mp_join = !!(mp_opt.suboptions & OPTION_MPTCP_MPJ_ACK);
if (opt_mp_capable && opt_mp_join) if (opt_mp_capable && opt_mp_join)
return -EINVAL; return -EINVAL;
...@@ -486,7 +486,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb) ...@@ -486,7 +486,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
mptcp_get_options(skb, &mp_opt); mptcp_get_options(skb, &mp_opt);
if (subflow->request_mptcp) { if (subflow->request_mptcp) {
if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPC)) { if (!(mp_opt.suboptions & OPTION_MPTCP_MPC_SYNACK)) {
MPTCP_INC_STATS(sock_net(sk), MPTCP_INC_STATS(sock_net(sk),
MPTCP_MIB_MPCAPABLEACTIVEFALLBACK); MPTCP_MIB_MPCAPABLEACTIVEFALLBACK);
mptcp_do_fallback(sk); mptcp_do_fallback(sk);
...@@ -506,7 +506,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb) ...@@ -506,7 +506,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
} else if (subflow->request_join) { } else if (subflow->request_join) {
u8 hmac[SHA256_DIGEST_SIZE]; u8 hmac[SHA256_DIGEST_SIZE];
if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ)) { if (!(mp_opt.suboptions & OPTION_MPTCP_MPJ_SYNACK)) {
subflow->reset_reason = MPTCP_RST_EMPTCP; subflow->reset_reason = MPTCP_RST_EMPTCP;
goto do_reset; goto do_reset;
} }
...@@ -783,12 +783,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, ...@@ -783,12 +783,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
* options. * options.
*/ */
mptcp_get_options(skb, &mp_opt); mptcp_get_options(skb, &mp_opt);
if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPC)) if (!(mp_opt.suboptions & OPTION_MPTCP_MPC_ACK))
fallback = true; fallback = true;
} else if (subflow_req->mp_join) { } else if (subflow_req->mp_join) {
mptcp_get_options(skb, &mp_opt); mptcp_get_options(skb, &mp_opt);
if (!(mp_opt.suboptions & OPTIONS_MPTCP_MPJ) || if (!(mp_opt.suboptions & OPTION_MPTCP_MPJ_ACK) ||
!subflow_hmac_valid(req, &mp_opt) || !subflow_hmac_valid(req, &mp_opt) ||
!mptcp_can_accept_new_subflow(subflow_req->msk)) { !mptcp_can_accept_new_subflow(subflow_req->msk)) {
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKMAC); SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKMAC);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment