> On Apr 25, 2017, at 5:45 PM, Benjamin Mahler <bmah...@apache.org> wrote: > > Thanks, this sounds good, just want to clarify a few things: > > (1) When you say "bind" the UPID IP address to the address of the message > sender, you specifically just mean disallowing messages where they do not > match?
Correct. > > (2) Why IP and not port? Or do you think port enforcement would be > optionally useful on top of IP enforcement? The UPID contains the listener port, not the sender port, so they are guaranteed not to match. > > (3) What will libprocess do when it finds the IP doesn't match? Drop and > log? It responds with an InternalServerError. > > On Thu, Apr 20, 2017 at 9:16 AM, James Peach <jor...@gmail.com> wrote: > >> >>> On Apr 19, 2017, at 5:24 PM, Benjamin Mahler <bmah...@apache.org> wrote: >>> >>> It's not obvious to me what all of the implications are. I feel we need >> a more comprehensive proposal here, can you do a small write up that shows >> you've thought through all the implications? >> >> Ok, let me summarize ... >> >> In libprocess, actors (processes) are globally identified by their UPID. >> The UPID is a string of the format <ID>@<IP address>:port. When libprocess >> actors communicate, they listen on a single IP address and port and claim >> this endpoint in their UPID. The expectation of other actors is that they >> can send messages to this actor by connecting to the endpoint claimed in >> the UPID. Note that the peer address used to send messages is not the UPID, >> but whichever source address:port that the sender happened to bind. >> >> Due to various network configurations (eg. NAT, multihoming), situations >> arise where a receiver is not able to easily predict which IP address it >> needs to claim in its UPID such that senders will be able to connect to it. >> This leads to the need for configuration options like LIBPROCESS_IP >> (specify the address to listen on) and LIBPROCESS_ADVERTISE_IP (specify the >> address in the UPID). >> >> The fundamental problem this proposal attempts to mitigate is the >> libprocess has no mechanism that can authenticate a UPID. This means that >> unless the network fabric is completely trusted, anyone with access to the >> network can inject arbitrary messages into any libprocess actor, >> impersonating any other actor. >> >> This proposal adds an optional libprocess setting to bind the IP address >> claimed in the UPID to the IP address of the message sender. This is a >> receiver option, where the receiver compared the result of getpeername(2) >> with the UPID from the libprocess message. The IP address is required to >> match, but the port and ID are allowed to differ. In practice, this means >> that libprocess actors are required to send from the same IP address they >> are listening on. So to impersonate a specific UPID you have to be able to >> send the message from the same IP address the impersonatee is using (ie. be >> running on the same host), which increases the difficulty of impersonation. >> >> In our deployments, all the cluster members are single-homed and remote >> access to the system is restricted. We know in advance that neither >> LIBPROCESS_IP nor LIBPROCESS_ADVERTISE_IP options are required. We have >> some confidence that we control the services running on cluster hosts. If >> we bind the UPID address to the socket peer address, then UPID >> impersonation requires that malicious code is already running on a cluster >> host, at which point we probably have bigger problems. >> >> In terms of testing, all the test suite passes except for >> ExamplesTest.DiskFullFramework. >> This test uses the LIBPROCESS_IP option to bind to the lo0 interface, which >> can result in the UPID claiming 127.0.0.1 but getpeername(2) returning one >> of the hosts external IP addresses. This is really just one type of a >> multi-homed host configuration, so it demonstrates that this configuration >> will break. >> >>> >>> E.g. how does this affect the use of proxies, multihomed hosts, our >> testing facilities for message spoofing, etc. >>> >>> On Tue, Apr 18, 2017 at 2:07 PM, James Peach <jor...@gmail.com> wrote: >>> >>>> On Apr 7, 2017, at 8:47 AM, Jie Yu <yujie....@gmail.com> wrote: >>>> >>>> + BenM >>>> >>>> James, I don't have immediate context on this issue. BenM will be back >> next >>>> week and he should be able to give you more accurate feedback. >>> >>> >>> I updated the review chain (from https://reviews.apache.org/r/58517/). >> Is anyone able to shepherd this? >>> >>> >>>> >>>> - Jie >>>> >>>> On Fri, Apr 7, 2017 at 8:37 AM, James Peach <jor...@gmail.com> wrote: >>>> >>>>> >>>>>> On Apr 5, 2017, at 5:42 PM, Jie Yu <yujie....@gmail.com> wrote: >>>>>> >>>>>> One comment here is that: >>>>>> >>>>>> We plan to support libprocess communication using domain socket. In >> other >>>>>> words, we plan to make UPID a socket addr. Can we make sure this >> approach >>>>>> also works for the case where UPID is a unix address in the future? >> For >>>>>> instance, what will `socket->peer();` returns for domain socket? >>> >>> This would probably work, but it depends on the lib process >> implementation. Since this is (default false) option, I this it is OK for >> now. I'll be happy to revisit when libprocess messaging supports domain >> sockets. >>> >>>>> >>>>> I can look into that. >>>>> >>>>> So you would consider this approach a reasonable mitigation? >>>>> >>>>>> >>>>>> - Jie >>>>>> >>>>>> On Wed, Apr 5, 2017 at 3:27 PM, James Peach <jor...@gmail.com> >> wrote: >>>>>> >>>>>>> Hi all, >>>>>>> >>>>>>> Currently, libprocess messages contain a UPID, which is send by the >> peer >>>>>>> in the HTTP message header. There's no validation of this, so >> generally >>>>>>> messages are trusted to be from the UPID they claim to be. >>>>>>> >>>>>>> As an RFC, I've pushed https://reviews.apache.org/r/58224/. This >> patch >>>>>>> constrains the UPID to not change for the lifetime of the socket >>> >>> I dropped this constraint. There are DiskQuotaTest.SlaveRecovery depends >> on the UPID being able to change. More accurately, libprocess only matches >> on the address portion of the UPID when finding a socket to use, and for >> now I don't think this change is beneficial enough to break that assumption. >>> >>>>>>> , and >>>>> also >>>>>>> enforces that the the IP address portion of the UPID matches the >> peer >>>>>>> socket address. This makes UPIDs more reliable, but the latter check >>>>> would >>>>>>> break existing configurations. I'd appreciate any feedback on >> whether >>>>> this >>>>>>> is worth pursuing at the lib process level and whether people feel >> that >>>>>>> this specific mitigation is worthwhile. >>>>>>> >>>>>>> thanks, >>>>>>> James >>>>> >>>>> >>> >>> >> >>
