netdev - Re: [PATCH net-next 2/3] rds: Enable RDS IPv6 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <25e1afda-7497-7f08-815a-286cf775bc09@oracle.com>
Date:   Tue, 26 Jun 2018 01:43:27 +0800
From:   Ka-Cheong Poon <ka-cheong.poon@...cle.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     netdev@...r.kernel.org, santosh.shilimkar@...cle.com,
        davem@...emloft.net, rds-devel@....oracle.com
Subject: Re: [PATCH net-next 2/3] rds: Enable RDS IPv6 support

On 06/26/2018 01:03 AM, Sowmini Varadhan wrote:
> On (06/25/18 03:38), Ka-Cheong Poon wrote:
>> @@ -1105,8 +1105,27 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
>>   			break;
>>   
>>   		case sizeof(*sin6): {
>> -			ret = -EPROTONOSUPPORT;
>> -			goto out;
>> +			int addr_type;
>                           :
>                           :
>> +			daddr = sin6->sin6_addr;
>> +			dport = sin6->sin6_port;
>> +			scope_id = sin6->sin6_scope_id;
>> +			break;
>>   		}
> 
> In rds_sendmsg, the scopeid passed to rds_conn_create_outgoing
> may come from the msg_name (if msg_name is a link-local) or
> may come from the rs_bound_scope_id (for connected socket, change
> made in Patch 1 of the series).
> 
> This sounds inconsistent.
> 
> If I bind to scopeid if1 and then send to fe80::1%if2 (without connect()),
> we'd create an rds_connection with dev_if set to if2.
> (first off, its a bit unexpected to be sending to fe80::1%if2 when you
> are bound to a link-local on if1!)
> 
> But then, if we got back a response from fe80::1%if2, I think we would
> not find a matching conn in rds_recv_incoming?


Yes, I think if the socket is bound, it should check the scope_id
in msg_name (if not NULL) to make sure that they match.  A bound
RDS socket can send to multiple peers.  But if the bound local
address is link local, it should only be allowed to send to peers
on the same link.


> And this is even more confusing because the fastpath in rds_sendmsg
> does not take the bound_scope_id into consideration at all:
> 1213         if (rs->rs_conn && ipv6_addr_equal(&rs->rs_conn->c_faddr, &daddr))
> 1214                 conn = rs->rs_conn;
> 1215         else {
> 1216                 conn = rds_conn_create_outgoing( /* .. */, scope_id)
> so if I erroneously passed a msg_name on a connected rds socket, what
> would happen? (see also question about rds_connect() itself, below)


The check added above takes care of this.  The scope_id should
match.


> Should we always use rs_bound_scope_id for creating the outgoing
> rds_connection? (you may need something deterministic for this,
> like "if bound addr is linklocal, return error if daddr has a different
> scopeid, else use the bound addr's scopeid", plus, "if bound addr is
> not global, and daddr is link-local, we need a conn with the daddr's
> scopeid")


If a socket is bound, I guess the scope_id should be used.  So
if a socket is not bound to a link local address and the socket
is used to sent to a link local peer, it should fail.


> Also, why is there no IPv6 support in rds_connect?


Oops, I missed this when I ported the internal version to the
net-next version.  Will add it back.



-- 
K. Poon
ka-cheong.poon@...cle.com