[PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc

[PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc

[Xin Long]

sctp_addr_id2transport is a function for sockopt to look up assoc by
address. As the address is from userspace, it can be a v4-mapped v6
address. But in sctp protocol stack, it always handles a v4-mapped
v6 address as a v4 address. So it's necessary to convert it to a v4
address before looking up assoc by address.

This patch is to fix it by calling sctp_verify_addr in which it can do
this conversion before calling sctp_endpoint_lookup_assoc, just like
what sctp_sendmsg and __sctp_connect do for the address from users.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/socket.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 318c678..37eeab7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -235,8 +235,12 @@ static struct sctp_transport *sctp_addr_id2transport(struct sock *sk,
 					      sctp_assoc_t id)
 {
 	struct sctp_association *addr_asoc = NULL, *id_asoc = NULL;
-	struct sctp_transport *transport;
+	struct sctp_af *af = sctp_get_af_specific(addr->ss_family);
 	union sctp_addr *laddr = (union sctp_addr *)addr;
+	struct sctp_transport *transport;
+
+	if (sctp_verify_addr(sk, laddr, af->sockaddr_len))
+		return NULL;
 
 	addr_asoc = sctp_endpoint_lookup_assoc(sctp_sk(sk)->ep,
 					       laddr,
-- 
2.1.0

Re: [PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc

[Neil Horman]

On Tue, Jan 24, 2017 at 02:01:53PM +0800, Xin Long wrote:
> sctp_addr_id2transport is a function for sockopt to look up assoc by
> address. As the address is from userspace, it can be a v4-mapped v6
> address. But in sctp protocol stack, it always handles a v4-mapped
> v6 address as a v4 address. So it's necessary to convert it to a v4
> address before looking up assoc by address.
> 
> This patch is to fix it by calling sctp_verify_addr in which it can do
> this conversion before calling sctp_endpoint_lookup_assoc, just like
> what sctp_sendmsg and __sctp_connect do for the address from users.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/socket.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 318c678..37eeab7 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -235,8 +235,12 @@ static struct sctp_transport *sctp_addr_id2transport(struct sock *sk,
>  					      sctp_assoc_t id)
>  {
>  	struct sctp_association *addr_asoc = NULL, *id_asoc = NULL;
> -	struct sctp_transport *transport;
> +	struct sctp_af *af = sctp_get_af_specific(addr->ss_family);
>  	union sctp_addr *laddr = (union sctp_addr *)addr;
> +	struct sctp_transport *transport;
> +
> +	if (sctp_verify_addr(sk, laddr, af->sockaddr_len))
> +		return NULL;
>  
>  	addr_asoc = sctp_endpoint_lookup_assoc(sctp_sk(sk)->ep,
>  					       laddr,
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc

[Xin Long]

On Wed, Jan 25, 2017 at 11:27 PM, Vladislav Yasevich
<vyasevich@gmail.com> wrote:
> On Tue, Jan 24, 2017 at 1:01 AM, Xin Long <lucien.xin@gmail.com> wrote:
>>
>> sctp_addr_id2transport is a function for sockopt to look up assoc by
>> address. As the address is from userspace, it can be a v4-mapped v6
>> address. But in sctp protocol stack, it always handles a v4-mapped
>> v6 address as a v4 address. So it's necessary to convert it to a v4
>> address before looking up assoc by address.
>>
>> This patch is to fix it by calling sctp_verify_addr in which it can do
>> this conversion before calling sctp_endpoint_lookup_assoc, just like
>> what sctp_sendmsg and __sctp_connect do for the address from users.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>>  net/sctp/socket.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
>> index 318c678..37eeab7 100644
>> --- a/net/sctp/socket.c
>> +++ b/net/sctp/socket.c
>> @@ -235,8 +235,12 @@ static struct sctp_transport
>> *sctp_addr_id2transport(struct sock *sk,
>>                                               sctp_assoc_t id)
>>  {
>>         struct sctp_association *addr_asoc = NULL, *id_asoc = NULL;
>> -       struct sctp_transport *transport;
>> +       struct sctp_af *af = sctp_get_af_specific(addr->ss_family);
>>         union sctp_addr *laddr = (union sctp_addr *)addr;
>> +       struct sctp_transport *transport;
>> +
>> +       if (sctp_verify_addr(sk, laddr, af->sockaddr_len))
>> +               return NULL;
>>
>
> This causes a side-effect such that GET options will end up with ipv4
> address instead
> of a v4mapped address that was passed in.
not really

(more below)
>
> -vlad
>
>>
>>         addr_asoc = sctp_endpoint_lookup_assoc(sctp_sk(sk)->ep,
>>                                                laddr,
        sctp_get_pf_specific(sk->sk_family)->addr_to_user(sctp_sk(sk),
                                                (union sctp_addr *)addr);

here it will convert it back to v4mapped v6 address.

>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Re: [PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc

[Vladislav Yasevich]

On Wed, Jan 25, 2017 at 10:34 AM, Xin Long <lucien.xin@gmail.com> wrote:
>
> On Wed, Jan 25, 2017 at 11:27 PM, Vladislav Yasevich
> <vyasevich@gmail.com> wrote:
> > On Tue, Jan 24, 2017 at 1:01 AM, Xin Long <lucien.xin@gmail.com> wrote:
> >>
> >> sctp_addr_id2transport is a function for sockopt to look up assoc by
> >> address. As the address is from userspace, it can be a v4-mapped v6
> >> address. But in sctp protocol stack, it always handles a v4-mapped
> >> v6 address as a v4 address. So it's necessary to convert it to a v4
> >> address before looking up assoc by address.
> >>
> >> This patch is to fix it by calling sctp_verify_addr in which it can do
> >> this conversion before calling sctp_endpoint_lookup_assoc, just like
> >> what sctp_sendmsg and __sctp_connect do for the address from users.
> >>
> >> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> ---
> >>  net/sctp/socket.c | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> >> index 318c678..37eeab7 100644
> >> --- a/net/sctp/socket.c
> >> +++ b/net/sctp/socket.c
> >> @@ -235,8 +235,12 @@ static struct sctp_transport
> >> *sctp_addr_id2transport(struct sock *sk,
> >>                                               sctp_assoc_t id)
> >>  {
> >>         struct sctp_association *addr_asoc = NULL, *id_asoc = NULL;
> >> -       struct sctp_transport *transport;
> >> +       struct sctp_af *af = sctp_get_af_specific(addr->ss_family);
> >>         union sctp_addr *laddr = (union sctp_addr *)addr;
> >> +       struct sctp_transport *transport;
> >> +
> >> +       if (sctp_verify_addr(sk, laddr, af->sockaddr_len))
> >> +               return NULL;
> >>
> >
> > This causes a side-effect such that GET options will end up with ipv4
> > address instead
> > of a v4mapped address that was passed in.
> not really
>
> (more below)
> >
> > -vlad
> >
> >>
> >>         addr_asoc = sctp_endpoint_lookup_assoc(sctp_sk(sk)->ep,
> >>                                                laddr,
>         sctp_get_pf_specific(sk->sk_family)->addr_to_user(sctp_sk(sk),
>                                                 (union sctp_addr *)addr);
>
> here it will convert it back to v4mapped v6 address.
>

Yep, you are right.  Missed the fact that it was already there.

ACK

-vlad

> >> --
> >> 2.1.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >

Re: [PATCH net] sctp: sctp_addr_id2transport should verify the addr before looking up assoc

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 24 Jan 2017 14:01:53 +0800

> sctp_addr_id2transport is a function for sockopt to look up assoc by
> address. As the address is from userspace, it can be a v4-mapped v6
> address. But in sctp protocol stack, it always handles a v4-mapped
> v6 address as a v4 address. So it's necessary to convert it to a v4
> address before looking up assoc by address.
> 
> This patch is to fix it by calling sctp_verify_addr in which it can do
> this conversion before calling sctp_endpoint_lookup_assoc, just like
> what sctp_sendmsg and __sctp_connect do for the address from users.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied.