mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.
Date Fri, 12 May 2017 01:18:38 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58224/#review174754
-----------------------------------------------------------



Currently we require getting the peer each time we receive some requests from the socket.
This patch can use the `Request->client` directly, without needing to introduce the session
change, without making the performance worse.

Since the performance isn't being made worse, how about we look into the peer caching optimization
after adding this feature?


3rdparty/libprocess/src/process.cpp
Lines 217-221 (patched)
<https://reviews.apache.org/r/58224/#comment247974>

    Can we exactly match the description you added to docs/configuration.md in the subsequent
patch? Seems nice to have them in sync, especially since it's not that long.



3rdparty/libprocess/src/process.cpp
Lines 586 (patched)
<https://reviews.apache.org/r/58224/#comment247975>

    Can you make this a pointer? We disallow static non-POD variables.



3rdparty/libprocess/src/process.cpp
Lines 2886-2891 (patched)
<https://reviews.apache.org/r/58224/#comment247976>

    At this point, you can look at `request->client` rather than `session->peer`. This
would avoid the need for introducing session for now, and doesn't make the performance worse.
    
    Separately from this feature, we could figure out how to cache the peer to avoid getting
each each time requests are decoded, in order to improve the performance.


- Benjamin Mahler


On May 10, 2017, 6:06 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58224/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
>     https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In general, libprocess is unable to validate that a peer
> is a legitimate owner of the UPID it claims in a libprocess
> message. This change adds a check that the IP address in the
> UPID matches the peer address. This makes spoofing the UPID
> harder (eg. to send authenticated messages), but also breaks
> some legitimate configurations, particularly on multihomed
> hosts.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
> 
> 
> Diff: https://reviews.apache.org/r/58224/diff/9/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25). Light manual testing.
> 
> With LIBPROCESS_require_peer_address_ip_match=true, all Mesos tests pass except ``ExamplesTest.DiskFullFramework``,
however enabling this will definitely break some libprocess APIs (though not in the way that
Mesos uses them) and legitimate multi-homed configurations. Note that setting LIBPROCESS_ip=127.0.0.1
makes you multihomed for this purpose, which is why ``ExamplesTest.DiskFullFramework`` breaks.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message