Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-5910

Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.0
    • Fix Version/s: 2.4.0
    • Component/s: security
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      It is possible to enable encryption of DataTransferProtocol.
      In some use cases, it is required to encrypt data transfer with some clients , but communicate in plain text with some other clients and data nodes.

      A sample use case will be that any data transfer inside a firewall can be in plain text whereas any data transfer from clients outside the firewall needs to be encrypted.

      1. HDFS-5910.patch
        21 kB
        Benoy Antony
      2. HDFS-5910.patch
        21 kB
        Benoy Antony
      3. HDFS-5910.patch
        21 kB
        Benoy Antony
      4. HDFS-5910.patch
        20 kB
        Benoy Antony
      5. HDFS-5910.patch
        6 kB
        Benoy Antony

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1738 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1738/)
          HDFS-5910. Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1738 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1738/ ) HDFS-5910 . Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #1713 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1713/)
          HDFS-5910. Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1713 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1713/ ) HDFS-5910 . Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #521 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/521/)
          HDFS-5910. Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #521 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/521/ ) HDFS-5910 . Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #5405 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5405/)
          HDFS-5910. Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5405 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5405/ ) HDFS-5910 . Enhance DataTransferProtocol to allow per-connection choice of encryption/plain-text. (Contributed by Benoy Antony) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1581688 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/TrustedChannelResolver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataXceiver.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptedTransfer.java
          Hide
          Arpit Agarwal added a comment -

          I committed this to trunk, branch-2 and branch-2.4.

          Thanks for the contribution Benoy Antony!

          Show
          Arpit Agarwal added a comment - I committed this to trunk, branch-2 and branch-2.4. Thanks for the contribution Benoy Antony !
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12636795/HDFS-5910.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6503//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6503//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636795/HDFS-5910.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6503//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6503//console This message is automatically generated.
          Hide
          Arpit Agarwal added a comment -

          +1 for the latest patch, pending Jenkins.

          Show
          Arpit Agarwal added a comment - +1 for the latest patch, pending Jenkins.
          Hide
          Benoy Antony added a comment -

          Attaching the patch after fixing compilation issue and rest of the comments.

          Show
          Benoy Antony added a comment - Attaching the patch after fixing compilation issue and rest of the comments.
          Hide
          Arpit Agarwal added a comment -

          Thanks Benoy!

          One build issue and a few minor comments:

          1. NameNodeConnector.java:162 - isTrustedOnClient should be isTrusted
          2. DataXceiver.java:273 - getClientAddress no longer throws UnknownHostException.
          3. hdfs-default.xml:1341 - Doc typo It is possible override --> It is possible to override
          4. hdfs-default.xml:1360 - Doc typo TrustedChannelResolver used --> TrustedChannelResolver is used

          +1 with these changes.

          Show
          Arpit Agarwal added a comment - Thanks Benoy! One build issue and a few minor comments: NameNodeConnector.java:162 - isTrustedOnClient should be isTrusted DataXceiver.java:273 - getClientAddress no longer throws UnknownHostException . hdfs-default.xml:1341 - Doc typo It is possible override --> It is possible to override hdfs-default.xml:1360 - Doc typo TrustedChannelResolver used --> TrustedChannelResolver is used +1 with these changes.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12636755/HDFS-5910.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6498//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636755/HDFS-5910.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6498//console This message is automatically generated.
          Hide
          Benoy Antony added a comment -

          BTW, I'll open a new jira to refactor the code so that for getDataEncryptionKey() is invoked only after determining that channel with peer cannot be trusted.

          Show
          Benoy Antony added a comment - BTW, I'll open a new jira to refactor the code so that for getDataEncryptionKey() is invoked only after determining that channel with peer cannot be trusted.
          Hide
          Benoy Antony added a comment -

          Thanks Arpit Agarwal for the review and comments.
          Attaching a new patch with the following changes.

          • Wherever peer address is available, I have used isTrusted(InetAddress)
          • uses InetAddresses#forString from Guava
          Show
          Benoy Antony added a comment - Thanks Arpit Agarwal for the review and comments. Attaching a new patch with the following changes. Wherever peer address is available, I have used isTrusted(InetAddress) uses InetAddresses#forString from Guava
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12636464/HDFS-5910.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6482//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6482//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636464/HDFS-5910.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6482//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6482//console This message is automatically generated.
          Hide
          Arpit Agarwal added a comment -

          Thanks for making the changes Benoy.

          The ip address of namenode or datanode is not available at some of the client invocations. Please let me know if there is a way to get an ip address..

          Just for my understanding - lacking the peer's IP address is it your intention to use configuration to decide the client's behavior?

          I looked through the usages of isTrusted and some of them already have the connected socket available, so it is fairly easy to query the remote's socket address and pass it to isTrusted.

          For the usage in getDataEncryptionKey(), we can refactor to pass a functor as the encryption key to e.g. getFileChecksum. However I am okay with doing the refactoring in a separate change. We can leave the parameter-less overload of isTrusted for now and just use it from getEcnryptionKey and file a separate Jira to fix it.

          I wanted to use InetAddress as the argument to TrustedChannelResolver than a string-ip-address to maintain parity with SaslPropertiesResolver. To convert a string ip, I use InetAddress.getByName

          Thanks for the explanation. Will InetAddresses#forString from Guava work for you? I just checked and it's available in our build.

          Show
          Arpit Agarwal added a comment - Thanks for making the changes Benoy. The ip address of namenode or datanode is not available at some of the client invocations. Please let me know if there is a way to get an ip address.. Just for my understanding - lacking the peer's IP address is it your intention to use configuration to decide the client's behavior? I looked through the usages of isTrusted and some of them already have the connected socket available, so it is fairly easy to query the remote's socket address and pass it to isTrusted . For the usage in getDataEncryptionKey(), we can refactor to pass a functor as the encryption key to e.g. getFileChecksum . However I am okay with doing the refactoring in a separate change. We can leave the parameter-less overload of isTrusted for now and just use it from getEcnryptionKey and file a separate Jira to fix it. I wanted to use InetAddress as the argument to TrustedChannelResolver than a string-ip-address to maintain parity with SaslPropertiesResolver. To convert a string ip, I use InetAddress.getByName Thanks for the explanation. Will InetAddresses#forString from Guava work for you? I just checked and it's available in our build.
          Hide
          Benoy Antony added a comment -

          Thanks Arpit Agarwal for the comments.

          I am attaching a new based on some of your comments. I request guidance on #1 and #4.

          1. isSecureOnClient may also want to use the peer's address to make a decision. e.g. intra-cluster transfer vs. distcp to remote cluster.

          The ip address of namenode or datanode is not available at some of the client invocations. Please let me know if there is a way to get an ip address..

          2. Related to #1, isSecureOnClient and isSecureOnServer look awkward. How about replacing both with isTrustedChannel that takes the peer's IP address? We should probably avoid overloading the term secure in this context since there is a related concept ofPeer#hasSecureChannel().

          I have renamed the class to TrustedChannelResolver and function to isTrusted() .

          3. Could you please update the documentation

          done

          4. Is the InetAddress.getByName call in DataXceiver#getClientAddress necessary? If it were necessary it would have been a security hole since DNS resolution may yield a different IP address than the one used by the client. It turns out for the kinds of Peers we are interested in this will be an IP address, so let's just remove the call.

          I wanted to use InetAddress as the argument to TrustedChannelResolver than a string-ip-address to maintain parity with SaslPropertiesResolver. To convert a string ip, I use InetAddress.getByName

          From the documentation of InetAddress.getByName(String host):
          The host name can either be a machine name, such as "java.sun.com", or a textual representation of its IP address. If a literal IP address is supplied, only the validity of the address format is checked.
          So basically , if the argument is ip address, getByName doesn't do a DNS check.
          If there is a different way to get the InetAddress , we can definitely use that.

          Other option is to not care about the parity with SaslPropertiesResolver and pass the string ip address.
          Yet another option will be to pass the Peer itself to TrustedChannelResolver so that the custom implementation can take care of getting the ip address etc. Will be great to get your opinion on this.

          Show
          Benoy Antony added a comment - Thanks Arpit Agarwal for the comments. I am attaching a new based on some of your comments. I request guidance on #1 and #4. 1. isSecureOnClient may also want to use the peer's address to make a decision. e.g. intra-cluster transfer vs. distcp to remote cluster. The ip address of namenode or datanode is not available at some of the client invocations. Please let me know if there is a way to get an ip address.. 2. Related to #1, isSecureOnClient and isSecureOnServer look awkward. How about replacing both with isTrustedChannel that takes the peer's IP address? We should probably avoid overloading the term secure in this context since there is a related concept ofPeer#hasSecureChannel(). I have renamed the class to TrustedChannelResolver and function to isTrusted() . 3. Could you please update the documentation done 4. Is the InetAddress.getByName call in DataXceiver#getClientAddress necessary? If it were necessary it would have been a security hole since DNS resolution may yield a different IP address than the one used by the client. It turns out for the kinds of Peers we are interested in this will be an IP address, so let's just remove the call. I wanted to use InetAddress as the argument to TrustedChannelResolver than a string-ip-address to maintain parity with SaslPropertiesResolver . To convert a string ip, I use InetAddress.getByName From the documentation of InetAddress.getByName(String host): The host name can either be a machine name, such as "java.sun.com", or a textual representation of its IP address. If a literal IP address is supplied, only the validity of the address format is checked. So basically , if the argument is ip address, getByName doesn't do a DNS check. If there is a different way to get the InetAddress , we can definitely use that. Other option is to not care about the parity with SaslPropertiesResolver and pass the string ip address. Yet another option will be to pass the Peer itself to TrustedChannelResolver so that the custom implementation can take care of getting the ip address etc. Will be great to get your opinion on this.
          Hide
          Suresh Srinivas added a comment -

          Benoy Antony, any updates on this?

          Show
          Suresh Srinivas added a comment - Benoy Antony , any updates on this?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12635876/HDFS-5910.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6456//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6456//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12635876/HDFS-5910.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6456//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6456//console This message is automatically generated.
          Hide
          Arpit Agarwal added a comment -

          Thanks for the clarifications Benoy Antony.

          Few comments on the patch:

          1. isSecureOnClient may also want to use the peer's address to make a decision. e.g. intra-cluster transfer vs. distcp to remote cluster.
          2. Related to #1, isSecureOnClient and isSecureOnServer look awkward. How about replacing both with isTrustedChannel that takes the peer's IP address? We should probably avoid overloading the term secure in this context since there is a related concept of Peer#hasSecureChannel().
          3. Could you please update the documentation for dfs.encrypt.data.transfer to state that per-connection override is possible via a custom resolver which can be configured with dfs.securechannel.resolver.class. Also move the two settings to be next to each other in the docs?
          4. Is the InetAddress.getByName call in DataXceiver#getClientAddress necessary? If it were necessary it would have been a security hole since DNS resolution may yield a different IP address than the one used by the client. It turns out for the kinds of Peers we are interested in this will be an IP address, so let's just remove the call.

          The patch looks fine otherwise.

          Show
          Arpit Agarwal added a comment - Thanks for the clarifications Benoy Antony . Few comments on the patch: isSecureOnClient may also want to use the peer's address to make a decision. e.g. intra-cluster transfer vs. distcp to remote cluster. Related to #1, isSecureOnClient and isSecureOnServer look awkward. How about replacing both with isTrustedChannel that takes the peer's IP address? We should probably avoid overloading the term secure in this context since there is a related concept of Peer#hasSecureChannel() . Could you please update the documentation for dfs.encrypt.data.transfer to state that per-connection override is possible via a custom resolver which can be configured with dfs.securechannel.resolver.class . Also move the two settings to be next to each other in the docs? Is the InetAddress.getByName call in DataXceiver#getClientAddress necessary? If it were necessary it would have been a security hole since DNS resolution may yield a different IP address than the one used by the client. It turns out for the kinds of Peers we are interested in this will be an IP address, so let's just remove the call. The patch looks fine otherwise.
          Hide
          Benoy Antony added a comment -

          BTW, this is in similar lines of HADOOP-10221 where custom logic can be used to determine the QOP for a Sasl connection.

          Show
          Benoy Antony added a comment - BTW, this is in similar lines of HADOOP-10221 where custom logic can be used to determine the QOP for a Sasl connection.
          Hide
          Benoy Antony added a comment -

          Arpit Agarwal I have added the sample use case as part of the description. The title looks accurate. Thanks.

          Show
          Benoy Antony added a comment - Arpit Agarwal I have added the sample use case as part of the description. The title looks accurate. Thanks.
          Hide
          Arpit Agarwal added a comment -

          I updated the Jira title to reflect the content of the change more accurately. Let me know if it's inaccurate.

          Show
          Arpit Agarwal added a comment - I updated the Jira title to reflect the content of the change more accurately. Let me know if it's inaccurate.
          Hide
          Arpit Agarwal added a comment -

          Hi Benoy, do you have an example use case for when we might want to let the SecureChannelResolver override the encryption setting?

          Show
          Arpit Agarwal added a comment - Hi Benoy, do you have an example use case for when we might want to let the SecureChannelResolver override the encryption setting?
          Hide
          Benoy Antony added a comment -

          Attaching a new patch .

          The approach taken is different from the approach mentioned in the previous comment. The DataTransfer does not support the full SASL QOP values. It supports encryption and plainText.
          To achieve the goal of determining whether to encrypt based on each connection, SecureChannelResolver (new interface) is defined. The interface can be plugged in via configuration.

          The server side (DataXceiver) consults SecureChannelResolver instance to determine whether the channel is secure or not. If the SecureChannelResolver returns true (channel is secure) , then encryption is not done.
          Similar process is done at the client side also while attempting the data transfer.

          The default implementation of SecureChannelResolver returns false signaling that the channel is not secure and encryption is required.
          The SecureChannelResolver is consulted only if dfs.encrypt.data.transfer is set to true.

          Show
          Benoy Antony added a comment - Attaching a new patch . The approach taken is different from the approach mentioned in the previous comment. The DataTransfer does not support the full SASL QOP values. It supports encryption and plainText. To achieve the goal of determining whether to encrypt based on each connection, SecureChannelResolver (new interface) is defined. The interface can be plugged in via configuration. The server side ( DataXceiver ) consults SecureChannelResolver instance to determine whether the channel is secure or not. If the SecureChannelResolver returns true (channel is secure) , then encryption is not done. Similar process is done at the client side also while attempting the data transfer. The default implementation of SecureChannelResolver returns false signaling that the channel is not secure and encryption is required. The SecureChannelResolver is consulted only if dfs.encrypt.data.transfer is set to true.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12627696/HDFS-5910.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6450//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12627696/HDFS-5910.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6450//console This message is automatically generated.
          Hide
          Arpit Agarwal added a comment -

          Benoy Antony is this ready for review? I noticed you hadn't submitted the patch.

          Show
          Arpit Agarwal added a comment - Benoy Antony is this ready for review? I noticed you hadn't submitted the patch.
          Hide
          Benoy Antony added a comment -

          An enhancement is done on dataTransferProtocol to decide this based on pluggable Resolver. This is the same resolver introduced in HADOP-10221

          The DFSClient is modified to look into its local config in addition to the encryption requirement indicated by Namenode to decide whether to encrypt the data transfer.
          The NN is modified to return encryption key to the clients even if encryption is not turned on.
          The datanode is modified to consult with the resolver in addition to dfs.encrypt.data.transfer value

          Show
          Benoy Antony added a comment - An enhancement is done on dataTransferProtocol to decide this based on pluggable Resolver. This is the same resolver introduced in HADOP-10221 The DFSClient is modified to look into its local config in addition to the encryption requirement indicated by Namenode to decide whether to encrypt the data transfer. The NN is modified to return encryption key to the clients even if encryption is not turned on. The datanode is modified to consult with the resolver in addition to dfs.encrypt.data.transfer value

            People

            • Assignee:
              Benoy Antony
              Reporter:
              Benoy Antony
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development