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?
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 > >> > >> > >
