Uploaded image for project: 'Mesos'
  1. Mesos
  2. MESOS-7753

`log.LearnedMessage` could be rejected due to being sent from '@0.0.0.0:0'

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.4.0
    • Fix Version/s: 1.4.0
    • Component/s: replicated log
    • Labels:
      None

      Description

      This is due to the use of https://github.com/apache/mesos/blob/ced7d69767142c912db0c2e01b95c0f5de791bd9/src/log/network.hpp#L247 which sets the message's from field to an empty UPID.

      The rationale for using this form is that there's no intended response for this message. However with MESOS-7401, this message could be rejected.

        Issue Links

          Activity

          Hide
          xujyan Yan Xu added a comment -

          A few things in consideration:

          • We cannot simply whitelist @0.0.0.0:0 in MESOS-7401 otherwise the protection mechanism is rendered useless.
          • Even though some messages are not supposed to have a "return address", it makes sense to always require a non-empty from address for actor to actor communication.
          • For non-testing libprocess communication in Mesos it makes sense to assume all messages are sent by libprocess actors.

          So for this the easiest fix is probably just give the LearnedMessage a sender: define a simple RequestProcess, in contrast with ReqResProcess, which is used for other replicated log messages.

          Of course no attempt has been made in the replicated log to really verify that the learned message is sent by a coordinator so this is simply to make the replicated log compliant with MESOS-7401.

          /cc James Peach Jie Yu

          Show
          xujyan Yan Xu added a comment - A few things in consideration: We cannot simply whitelist @0.0.0.0:0 in MESOS-7401 otherwise the protection mechanism is rendered useless. Even though some messages are not supposed to have a "return address", it makes sense to always require a non-empty from address for actor to actor communication. For non-testing libprocess communication in Mesos it makes sense to assume all messages are sent by libprocess actors. So for this the easiest fix is probably just give the LearnedMessage a sender: define a simple RequestProcess , in contrast with ReqResProcess , which is used for other replicated log messages. Of course no attempt has been made in the replicated log to really verify that the learned message is sent by a coordinator so this is simply to make the replicated log compliant with MESOS-7401 . /cc James Peach Jie Yu
          Hide
          jamespeach James Peach added a comment -

          I agree that every message actually does have a sender. Even a broadcast is sent from a specific Process. In this case, there is something that is a member of the Network that is performing the broadcast, so it seems really wonky to not have a source UPID (since membership in the network is defined by a UPID). In the case it looks like the broadcast is sent by the NetworkProcess so it should use the to/from version of process::post (maybe we we should deprecate the to-only version?).

          Show
          jamespeach James Peach added a comment - I agree that every message actually does have a sender. Even a broadcast is sent from a specific Process . In this case, there is something that is a member of the Network that is performing the broadcast, so it seems really wonky to not have a source UPID (since membership in the network is defined by a UPID ). In the case it looks like the broadcast is sent by the NetworkProcess so it should use the to/from version of process::post (maybe we we should deprecate the to-only version?).
          Hide
          xujyan Yan Xu added a comment - - edited

          This version of post is used mostly by tests but pretty convenient. I guess we can also have an untyped ProcessBase as a sender that's implicitly created?

          Show
          xujyan Yan Xu added a comment - - edited This version of post is used mostly by tests but pretty convenient. I guess we can also have an untyped ProcessBase as a sender that's implicitly created?
          Hide
          xujyan Yan Xu added a comment -

          The libprocess change is still separate though as I feel that in production logs for replicated log, a "real" process with a identifiable name as least looks better?

          Show
          xujyan Yan Xu added a comment - The libprocess change is still separate though as I feel that in production logs for replicated log, a "real" process with a identifiable name as least looks better?
          Hide
          xujyan Yan Xu added a comment -

          Eventually decided to address this ticket separately from MESOS-7786. Will just use the pid of the NetworkProcess as the sender.

          Show
          xujyan Yan Xu added a comment - Eventually decided to address this ticket separately from MESOS-7786 . Will just use the pid of the NetworkProcess as the sender.
          Show
          xujyan Yan Xu added a comment - https://reviews.apache.org/r/60917/
          Hide
          xujyan Yan Xu added a comment -
          commit d5b85d2b9063b5fb4a40108ec4801cc5ed2f5155
          Author: Jiang Yan Xu <xujyan@apple.com>
          Date:   Mon Jul 17 11:26:21 2017 -0700
          
              Used a real UPID to send `LearnedMessage`.
              
              Previously an empty UPID `@0.0.0.0:0` was used which could result in
              the message being dropped due to certain security check (MESOS-7401).
              
              Review: https://reviews.apache.org/r/60917
          
          Show
          xujyan Yan Xu added a comment - commit d5b85d2b9063b5fb4a40108ec4801cc5ed2f5155 Author: Jiang Yan Xu <xujyan@apple.com> Date: Mon Jul 17 11:26:21 2017 -0700 Used a real UPID to send `LearnedMessage`. Previously an empty UPID `@0.0.0.0:0` was used which could result in the message being dropped due to certain security check (MESOS-7401). Review: https://reviews.apache.org/r/60917

            People

            • Assignee:
              xujyan Yan Xu
              Reporter:
              xujyan Yan Xu
              Shepherd:
              Joseph Wu
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development