Flume
  1. Flume
  2. FLUME-997

Support secure transport mechanism

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: v1.0.0
    • Fix Version/s: v1.4.0
    • Component/s: None
    • Labels:

      Description

      Flume needs support for a secure network transport protocol. See AVRO-898 for a patch that made it into Avro 1.6.0 that allows us to do this relatively easily.

      1. FLUME-997-3.patch
        40 kB
        Joey Echeverria
      2. FLUME-997-2.patch
        32 kB
        Joey Echeverria
      3. FLUME-997-1.patch
        28 kB
        Joey Echeverria

        Issue Links

          Activity

          Hide
          Mike Percy added a comment -

          Creating new issue since FLUME-13 seems to be focused on the 0.9.x code line

          Show
          Mike Percy added a comment - Creating new issue since FLUME-13 seems to be focused on the 0.9.x code line
          Hide
          Joey Echeverria added a comment -

          I created an initial implementation based on the Avro SSL tests added in AVRO-1119. The changes are implemented by adding additional settings on AvroSource and AvroSink to enable SSL. The current patch passes all of the existing tests as well as new tests with SSL enabled. It's still needs more testing to cover the FailoverRpcClient and LoadBalancingRpcClient as well as documentation of the new settings.

          Show
          Joey Echeverria added a comment - I created an initial implementation based on the Avro SSL tests added in AVRO-1119 . The changes are implemented by adding additional settings on AvroSource and AvroSink to enable SSL. The current patch passes all of the existing tests as well as new tests with SSL enabled. It's still needs more testing to cover the FailoverRpcClient and LoadBalancingRpcClient as well as documentation of the new settings.
          Hide
          Mike Percy added a comment -

          Joey do you mind rebasing if necessary and also posting this on Review Board? Thanks!

          Show
          Mike Percy added a comment - Joey do you mind rebasing if necessary and also posting this on Review Board? Thanks!
          Hide
          Joey Echeverria added a comment -

          Will do.

          Show
          Joey Echeverria added a comment - Will do.
          Hide
          Mike Percy added a comment -

          Hi Joey,
          Upon initial review, the changes look good.

          The patch definitely needs a rebase though. I tried to do a simple merge but the files in your patch were heavily modified in the past few weeks, so it will take a human touch to get your changes to apply. Note that most of the implementation of the AvroSink moved into AbstractRpcSink as part of FLUME-1897.

          Some general things to keep in mind, since Ted's patch has been committed now:
          1. SSL should always wrap the compression, if enabled
          2. Watch out for DRY in the AvroSource, especially now that the compression stuff is checked in
          3. Would be great to get negative test cases for non-SSL clients to ensure SSL servers don't work with them

          I'm interested in getting this in, let me know how I can help.

          Show
          Mike Percy added a comment - Hi Joey, Upon initial review, the changes look good. The patch definitely needs a rebase though. I tried to do a simple merge but the files in your patch were heavily modified in the past few weeks, so it will take a human touch to get your changes to apply. Note that most of the implementation of the AvroSink moved into AbstractRpcSink as part of FLUME-1897 . Some general things to keep in mind, since Ted's patch has been committed now: 1. SSL should always wrap the compression, if enabled 2. Watch out for DRY in the AvroSource, especially now that the compression stuff is checked in 3. Would be great to get negative test cases for non-SSL clients to ensure SSL servers don't work with them I'm interested in getting this in, let me know how I can help.
          Hide
          Joey Echeverria added a comment -

          Thanks for the initial review Mike. I've been a little swamped, but I hope to pick this up this weekend and get the patch re-based on trunk. I'll keep you posted.

          -Joey

          Show
          Joey Echeverria added a comment - Thanks for the initial review Mike. I've been a little swamped, but I hope to pick this up this weekend and get the patch re-based on trunk. I'll keep you posted. -Joey
          Hide
          Mike Percy added a comment -

          Thanks Joey. Any update on this ticket?

          Show
          Mike Percy added a comment - Thanks Joey. Any update on this ticket?
          Hide
          Joey Echeverria added a comment -

          Here's a rebased patch that also supports using both SSL and compression at the same time. It also adds documentation on configuring AvroSource and AvroSink with SSL. The only thing missing is the negative test, which I'll add in a bit, but I wanted to get this posted sooner rather than later.

          Show
          Joey Echeverria added a comment - Here's a rebased patch that also supports using both SSL and compression at the same time. It also adds documentation on configuring AvroSource and AvroSink with SSL. The only thing missing is the negative test, which I'll add in a bit, but I wanted to get this posted sooner rather than later.
          Hide
          Jeff Lord added a comment -

          Hey Joey,

          Were you still planning on posting the negative test here?

          Thanks,

          Jeff

          Show
          Jeff Lord added a comment - Hey Joey, Were you still planning on posting the negative test here? Thanks, Jeff
          Hide
          Mike Percy added a comment -

          Joey can you please attach your .p12 and .jks files? They did not come through in the diff.

          You can get them to come through if you use git diff --binary. Recent versions of GNU patch support that format as well.

          Show
          Mike Percy added a comment - Joey can you please attach your .p12 and .jks files? They did not come through in the diff. You can get them to come through if you use git diff --binary. Recent versions of GNU patch support that format as well.
          Hide
          Joey Echeverria added a comment -

          I updated the patch based on the feedback on review board and added the missing binary files to the patch.

          Show
          Joey Echeverria added a comment - I updated the patch based on the feedback on review board and added the missing binary files to the patch.
          Hide
          Joey Echeverria added a comment -

          I also added the negative test. Ideally, when you tried to connect without SSL on one side, it would give you a useful error, but I didn't see anything in the exception chain that would confirm the connection failure was do to mismatched SSL settings.

          Show
          Joey Echeverria added a comment - I also added the negative test. Ideally, when you tried to connect without SSL on one side, it would give you a useful error, but I didn't see anything in the exception chain that would confirm the connection failure was do to mismatched SSL settings.
          Hide
          Mike Percy added a comment -

          +1

          Let's get this into Flume 1.4. I have a couple more points of feedback but I will file those in a follow-up JIRA:

          1. If the server is configured for SSL but the keystore isn't there, the service should not start.
          2. We should fall back to the default behavior of the X509 TrustManagerFactory if there is no trust store specified by passing a null KeyStore, no need to look in the classpath for cacerts.

          Show
          Mike Percy added a comment - +1 Let's get this into Flume 1.4. I have a couple more points of feedback but I will file those in a follow-up JIRA: 1. If the server is configured for SSL but the keystore isn't there, the service should not start. 2. We should fall back to the default behavior of the X509 TrustManagerFactory if there is no trust store specified by passing a null KeyStore, no need to look in the classpath for cacerts.
          Hide
          Mike Percy added a comment -

          Pushed to trunk and flume-1.4 branches. Thanks for your contribution, Joey!

          Show
          Mike Percy added a comment - Pushed to trunk and flume-1.4 branches. Thanks for your contribution, Joey!
          Hide
          Hudson added a comment -

          Integrated in flume-trunk #425 (See https://builds.apache.org/job/flume-trunk/425/)
          FLUME-997. Support secure transport mechanism. (Revision a964e7ab3cfacbafb7e086d49ae2b94195b9c0df)

          Result = UNSTABLE
          mpercy : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=a964e7ab3cfacbafb7e086d49ae2b94195b9c0df
          Files :

          • flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java
          • flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java
          • flume-ng-doc/sphinx/FlumeUserGuide.rst
          • flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java
          • flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java
          • flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java
          • flume-ng-core/src/test/resources/truststore.jks
          • flume-ng-core/src/test/resources/server.p12
          Show
          Hudson added a comment - Integrated in flume-trunk #425 (See https://builds.apache.org/job/flume-trunk/425/ ) FLUME-997 . Support secure transport mechanism. (Revision a964e7ab3cfacbafb7e086d49ae2b94195b9c0df) Result = UNSTABLE mpercy : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=a964e7ab3cfacbafb7e086d49ae2b94195b9c0df Files : flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java flume-ng-doc/sphinx/FlumeUserGuide.rst flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java flume-ng-core/src/test/resources/truststore.jks flume-ng-core/src/test/resources/server.p12

            People

            • Assignee:
              Joey Echeverria
              Reporter:
              Mike Percy
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development