| Message ID | 74f12b1cf1e60fbfba1b737b8949526346afb983.1615584629.git.dcaratti@redhat.com |
|---|---|
| State | Accepted, archived |
| Commit | 442a009d990c35dd7a58f86d43aebfa73430fe34 |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [net] mptcp: fix ADD_ADDR HMAC in case port is specified | expand |
On Fri, 12 Mar 2021, Davide Caratti wrote: > Currently, Linux computes the HMAC contained in ADD_ADDR sub-option using > the Address Id and the IP Address, and hardcodes a destination port equal > to zero. This is not ok for ADD_ADDR with port: ensure to account for the > endpoint port when computing the HMAC, in compliance with RFC8684 §3.4.1. > > Fixes: 22fb85ffaefb ("mptcp: add port support for ADD_ADDR suboption writing") > Signed-off-by: Davide Caratti <dcaratti@redhat.com> > --- > net/mptcp/options.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > Hi Davide - Thanks for catching this. Fix looks good to me. Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> -- Mat Martineau Intel
Hi Davide, Mat, Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年3月13日周六 上午8:35写道： > > On Fri, 12 Mar 2021, Davide Caratti wrote: > > > Currently, Linux computes the HMAC contained in ADD_ADDR sub-option using > > the Address Id and the IP Address, and hardcodes a destination port equal > > to zero. This is not ok for ADD_ADDR with port: ensure to account for the > > endpoint port when computing the HMAC, in compliance with RFC8684 §3.4.1. > > > > Fixes: 22fb85ffaefb ("mptcp: add port support for ADD_ADDR suboption writing") > > Signed-off-by: Davide Caratti <dcaratti@redhat.com> > > --- > > net/mptcp/options.c | 24 ++++++++++++++---------- > > 1 file changed, 14 insertions(+), 10 deletions(-) > > > > Hi Davide - > > Thanks for catching this. Fix looks good to me. > > Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Acked-by: Geliang Tang <geliangtang@gmail.com> -Geliang > > -- > Mat Martineau > Intel
Hi Davide, Geliang, Mat, On 12/03/2021 22:32, Davide Caratti wrote: > Currently, Linux computes the HMAC contained in ADD_ADDR sub-option using > the Address Id and the IP Address, and hardcodes a destination port equal > to zero. This is not ok for ADD_ADDR with port: ensure to account for the > endpoint port when computing the HMAC, in compliance with RFC8684 §3.4.1. Thank you for the patch and the reviews! Now in our tree with Mat's RvB and Geliang's ACK: - 442a009d990c: mptcp: fix ADD_ADDR HMAC in case port is specified - Results: f1ac0c54f494..0f8f3b7b5e58 Tests + export have been queued! Cheers, Matt
diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 444a38681e93..89a4225ed321 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -567,15 +567,15 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, } static u64 add_addr_generate_hmac(u64 key1, u64 key2, u8 addr_id, - struct in_addr *addr) + struct in_addr *addr, u16 port) { u8 hmac[SHA256_DIGEST_SIZE]; u8 msg[7]; msg[0] = addr_id; memcpy(&msg[1], &addr->s_addr, 4); - msg[5] = 0; - msg[6] = 0; + msg[5] = port >> 8; + msg[6] = port & 0xFF; mptcp_crypto_hmac_sha(key1, key2, msg, 7, hmac); @@ -584,15 +584,15 @@ static u64 add_addr_generate_hmac(u64 key1, u64 key2, u8 addr_id, #if IS_ENABLED(CONFIG_MPTCP_IPV6) static u64 add_addr6_generate_hmac(u64 key1, u64 key2, u8 addr_id, - struct in6_addr *addr) + struct in6_addr *addr, u16 port) { u8 hmac[SHA256_DIGEST_SIZE]; u8 msg[19]; msg[0] = addr_id; memcpy(&msg[1], &addr->s6_addr, 16); - msg[17] = 0; - msg[18] = 0; + msg[17] = port >> 8; + msg[18] = port & 0xFF; mptcp_crypto_hmac_sha(key1, key2, msg, 19, hmac); @@ -646,7 +646,8 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * opts->ahmac = add_addr_generate_hmac(msk->local_key, msk->remote_key, opts->addr_id, - &opts->addr); + &opts->addr, + opts->port); } } #if IS_ENABLED(CONFIG_MPTCP_IPV6) @@ -657,7 +658,8 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * opts->ahmac = add_addr6_generate_hmac(msk->local_key, msk->remote_key, opts->addr_id, - &opts->addr6); + &opts->addr6, + opts->port); } } #endif @@ -962,12 +964,14 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk, if (mp_opt->family == MPTCP_ADDR_IPVERSION_4) hmac = add_addr_generate_hmac(msk->remote_key, msk->local_key, - mp_opt->addr_id, &mp_opt->addr); + mp_opt->addr_id, &mp_opt->addr, + mp_opt->port); #if IS_ENABLED(CONFIG_MPTCP_IPV6) else hmac = add_addr6_generate_hmac(msk->remote_key, msk->local_key, - mp_opt->addr_id, &mp_opt->addr6); + mp_opt->addr_id, &mp_opt->addr6, + mp_opt->port); #endif pr_debug("msk=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
Currently, Linux computes the HMAC contained in ADD_ADDR sub-option using the Address Id and the IP Address, and hardcodes a destination port equal to zero. This is not ok for ADD_ADDR with port: ensure to account for the endpoint port when computing the HMAC, in compliance with RFC8684 §3.4.1. Fixes: 22fb85ffaefb ("mptcp: add port support for ADD_ADDR suboption writing") Signed-off-by: Davide Caratti <dcaratti@redhat.com> --- net/mptcp/options.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)