Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Not a Problem
    • Fix Version/s: 1.0.3
    • Component/s: Core
    • Environment:

      All.

      Description

      A prerequisite for preventing malicious nodes from joining a cluster (parent issue https://issues.apache.org/jira/browse/CASSANDRA-2274) is that we can determine the IP of the sender (setting aside the fact that this may be spoofed by a determined attacker).

      Currently we deserialize the "from" IP address from the incoming message header, using Header.deserialize() and CompactEndpointSerializationHelper.deserialize() i.e. we trust the sender to supply a true IP address.

      We could stop storing the IP address in the message Header at all (saving a small amount of space) and set the 'true' sender IP upon receipt of the message, in org.apache.cassandra.net.IncomingTcpConnection, using socket.getInetAddress().

      1. Cassandra-3462-v2.patch
        2 kB
        David Allsopp
      2. Cassandra-3462.patch
        1.0 kB
        David Allsopp

        Activity

        Hide
        David Allsopp added a comment - - edited

        This small patch corrects the Header if the IP addresses don't match. Unfortunately it breaks org.apache.cassandra.service.RemoveTest, which creates a ring of nodes with fake IP addresses (127.0.0.2 etc), so not really sure how to proceed... Probably need a static flag that the tests can unset to bypass the IP check, which seems a little ugly.

        Show
        David Allsopp added a comment - - edited This small patch corrects the Header if the IP addresses don't match. Unfortunately it breaks org.apache.cassandra.service.RemoveTest, which creates a ring of nodes with fake IP addresses (127.0.0.2 etc), so not really sure how to proceed... Probably need a static flag that the tests can unset to bypass the IP check, which seems a little ugly.
        Hide
        David Allsopp added a comment -

        Patch v2 adds a static flag to enable unit tests to disable the IP validation.

        Show
        David Allsopp added a comment - Patch v2 adds a static flag to enable unit tests to disable the IP validation.
        Hide
        David Allsopp added a comment -

        The patch above logs a warning and 'repairs' the incorrect IP - but should we actually just drop any messages with incorrect IPs on the assumption that they come from a hostile (or badly broken) node?

        Show
        David Allsopp added a comment - The patch above logs a warning and 'repairs' the incorrect IP - but should we actually just drop any messages with incorrect IPs on the assumption that they come from a hostile (or badly broken) node?
        Hide
        Jonathan Ellis added a comment -

        A worthy attempt, but it would break multi-DC replication. See the additional comments I've added to Header.java

        Show
        Jonathan Ellis added a comment - A worthy attempt, but it would break multi-DC replication. See the additional comments I've added to Header.java
        Hide
        David Allsopp added a comment -

        Ah, yes . Given that the parent issue has been closed there's probably no point pursuing this.

        Validating forwarded messages end-to-end is possible using a 'chain of trust' where each forwarding node vouches for the originator, but it seems the inter-node encryption achieves the same goal and more!

        Show
        David Allsopp added a comment - Ah, yes . Given that the parent issue has been closed there's probably no point pursuing this. Validating forwarded messages end-to-end is possible using a 'chain of trust' where each forwarding node vouches for the originator, but it seems the inter-node encryption achieves the same goal and more!
        Hide
        David Allsopp added a comment -

        Closed to reflect the status of the parent issue (CASSANDRA-2274).

        Show
        David Allsopp added a comment - Closed to reflect the status of the parent issue ( CASSANDRA-2274 ).

          People

          • Assignee:
            Unassigned
            Reporter:
            David Allsopp
            Reviewer:
            Brandon Williams
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development