Uploaded image for project: 'Spark'
  1. Spark
  2. SPARK-13331

AES support for over-the-wire encryption

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.2.0
    • Component/s: Deploy
    • Labels:
      None

      Description

      In network/common, SASL with DIGEST­-MD5 authentication is used for negotiating a secure communication channel. When SASL operation mode is "auth­-conf", the data transferred on the network is encrypted. DIGEST-MD5 mechanism supports following encryption: 3DES, DES, and RC4. The negotiation procedure will select one of them to encrypt / decrypt the data on the channel.

      However, 3des and rc4 are slow relatively. We could add code in the negotiation to make it support AES for more secure and performance.

      The proposed solution is:
      When "auth-conf" is enabled, at the end of original negotiation, the authentication succeeds and a secure channel is built. We could add one more negotiation step: Client and server negotiate whether they both support AES. If yes, the Key and IV used by AES will be generated by server and sent to client through the already secure channel. Then update the encryption / decryption handler to AES at both client and server side. Following data transfer will use AES instead of original encryption algorithm.

        Issue Links

          Activity

          Hide
          srowen Sean Owen added a comment -

          This sounds like a duplicate of SPARK-10771. You should provide some detail on what you think would implement this change.

          Show
          srowen Sean Owen added a comment - This sounds like a duplicate of SPARK-10771 . You should provide some detail on what you think would implement this change.
          Hide
          dongc Dong Chen added a comment -

          Sure, updated the description with some details.

          Show
          dongc Dong Chen added a comment - Sure, updated the description with some details.
          Hide
          srowen Sean Owen added a comment -

          I meant, add details about what change this would entail in Spark, not the protocol

          Show
          srowen Sean Owen added a comment - I meant, add details about what change this would entail in Spark, not the protocol
          Hide
          dongc Dong Chen added a comment - - edited

          Sorry for the confusion, below is the intention and the change would entail in Spark. Please let me know if any confusions.

          SPARK-6229 add SASL encryption to network library. The encryption support 3DES, DES, and RC4. This JIRA intends to make the encryption support AES for better performance.

          The change in Spark would involve:

          • add code in SaslClientBootstrap.doBootstrap() and SaslRpcHandler.receive() to negotiate AES encryption.
          • Then update SparkSaslClient and SparkSaslServer to wrap / unwrap message with AES.

          SPARK-10771 and this JIRA has same point that they both use JCE or a library to implement AES. But have different focus in Spark. SPARK-10771 is for data encryption when writing / reading shuffle data to disk, and this JIRA is for data encryption when transfering data on wire.

          Show
          dongc Dong Chen added a comment - - edited Sorry for the confusion, below is the intention and the change would entail in Spark. Please let me know if any confusions. SPARK-6229 add SASL encryption to network library. The encryption support 3DES, DES, and RC4. This JIRA intends to make the encryption support AES for better performance. The change in Spark would involve: add code in SaslClientBootstrap.doBootstrap() and SaslRpcHandler.receive() to negotiate AES encryption. Then update SparkSaslClient and SparkSaslServer to wrap / unwrap message with AES. SPARK-10771 and this JIRA has same point that they both use JCE or a library to implement AES. But have different focus in Spark. SPARK-10771 is for data encryption when writing / reading shuffle data to disk, and this JIRA is for data encryption when transfering data on wire.
          Hide
          dongc Dong Chen added a comment -

          Hi, Sean Owen, what do you think about above comment? Thanks.

          Show
          dongc Dong Chen added a comment - Hi, Sean Owen , what do you think about above comment? Thanks.
          Hide
          apachespark Apache Spark added a comment -

          User 'cjjnjust' has created a pull request for this issue:
          https://github.com/apache/spark/pull/15172

          Show
          apachespark Apache Spark added a comment - User 'cjjnjust' has created a pull request for this issue: https://github.com/apache/spark/pull/15172
          Hide
          Lifeng Wang Lifeng Wang added a comment -

          We use TPCx-BB[1] to evaluate this performance feature. We conduct TPCx-BB test on 3TB dataset on Intel HSW-EP platform. The score shows AES encryption brings 12.5% overall performance improvement and as high as 56% individual query improvement compared to default 3DES encryption.

          [1]http://www.tpc.org/tpcx-bb/default.asp

          Show
          Lifeng Wang Lifeng Wang added a comment - We use TPCx-BB[1] to evaluate this performance feature. We conduct TPCx-BB test on 3TB dataset on Intel HSW-EP platform. The score shows AES encryption brings 12.5% overall performance improvement and as high as 56% individual query improvement compared to default 3DES encryption. [1] http://www.tpc.org/tpcx-bb/default.asp
          Hide
          junjie Junjie Chen added a comment -

          Hi Marcelo Vanzin

          Could you help to review the code?

          Show
          junjie Junjie Chen added a comment - Hi Marcelo Vanzin Could you help to review the code?
          Hide
          junjie Junjie Chen added a comment -

          Hi Marcelo Vanzin
          I have updated the latest patch, Could you please help to review it?

          Due to an issue (CRYPTO-125) in Common Crypto, the patch has to use two helper channels. Once it be fixed and released, I will remove these channels.

          Show
          junjie Junjie Chen added a comment - Hi Marcelo Vanzin I have updated the latest patch, Could you please help to review it? Due to an issue ( CRYPTO-125 ) in Common Crypto, the patch has to use two helper channels. Once it be fixed and released, I will remove these channels.
          Hide
          junjie Junjie Chen added a comment -

          Hi Marcelo Vanzin

          The updated patch was committed according to your comments.

          I tried to change the negotiation by sending configuration to server without waiting for response, but server end does not get data as expected. From the description of TransportClient.send, it doesn't guarantee delivery.

          I think it should wait for response from server to do a handshake here, otherwise client will send encrypted data out and server may still not ready to accept encrypted data. isn't it?

          Show
          junjie Junjie Chen added a comment - Hi Marcelo Vanzin The updated patch was committed according to your comments. I tried to change the negotiation by sending configuration to server without waiting for response, but server end does not get data as expected. From the description of TransportClient.send, it doesn't guarantee delivery. I think it should wait for response from server to do a handshake here, otherwise client will send encrypted data out and server may still not ready to accept encrypted data. isn't it?
          Hide
          junjie Junjie Chen added a comment -

          Hi Marcelo Vanzin

          The patch was updated according to your comments.

          Show
          junjie Junjie Chen added a comment - Hi Marcelo Vanzin The patch was updated according to your comments.
          Hide
          junjie Junjie Chen added a comment -

          Hi Marcelo Vanzin

          Do we need more review?

          Show
          junjie Junjie Chen added a comment - Hi Marcelo Vanzin Do we need more review?
          Hide
          vanzin Marcelo Vanzin added a comment -

          You need to be a little patient. People have things to do outside of reviewing your code.

          Show
          vanzin Marcelo Vanzin added a comment - You need to be a little patient. People have things to do outside of reviewing your code.
          Hide
          jlewandowski Jacek Lewandowski added a comment -

          I wonder where those changes is going to end up. AES is a protocol used by TLS, standard protocol for encryption. Instead, deprecated SASL mechanism, which uses insecure MD5 is manually fixed. Now we have MD5+AES - pretty crazy, isn't it?

          Show
          jlewandowski Jacek Lewandowski added a comment - I wonder where those changes is going to end up. AES is a protocol used by TLS, standard protocol for encryption. Instead, deprecated SASL mechanism, which uses insecure MD5 is manually fixed. Now we have MD5+AES - pretty crazy, isn't it?
          Hide
          Ferd Ferdinand Xu added a comment -

          Hi Jacek Lewandowski, this feature provides an option to support AES and doesn't have impacts on the existing SASL part which means user can either use SASL or AES to encrypt network.

          Now we have MD5+AES - pretty crazy, isn't it?

          Does this mean you suggest deprecating SASL mechanism and only provide the AES part?

          Show
          Ferd Ferdinand Xu added a comment - Hi Jacek Lewandowski , this feature provides an option to support AES and doesn't have impacts on the existing SASL part which means user can either use SASL or AES to encrypt network. Now we have MD5+AES - pretty crazy, isn't it? Does this mean you suggest deprecating SASL mechanism and only provide the AES part?

            People

            • Assignee:
              junjie Junjie Chen
              Reporter:
              dongc Dong Chen
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development