Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added plugin tcp with ssl #1333

Closed
wants to merge 1 commit into from
Closed

Added plugin tcp with ssl #1333

wants to merge 1 commit into from

Conversation

SilverDragon135
Copy link

Hi, I would like to embed this plugin in fluentd. There is no problem with any changes you would like.

Errors:

  • Installation from my fork (exact fluentd branch copy with added plugin) did not installed this plugin - maybe it is necessary to add it somewhere in setup script (did not found it yet).
  • Plugin is tested with official released td-agent, so it does not use new 0.14 plugin API. (If you will agree with future merging we can improve it of course).

@cosmo0920
Copy link
Contributor

I'm wondering that fluent-plugin-secure-forward is not enough for your use case in v0.12?

Plugin is tested with official released td-agent, so it does not use new 0.14 plugin API. (If you will agree with future merging we can improve it of course).

👎 Not using v0.14 API is not welcomed in this repository for now.
And could you take a look this document?
https://github.com/fluent/fluentd/blob/master/CONTRIBUTING.md#patch-guidelines
This document encourages to write unit test.

FYI: It would be better to implement this transferring via tls feature in in_forward plugin.
And this feature will be implemented in near future.
see: https://github.com/fluent/fluentd/wiki/Roadmap:-New-Features-and-Major-Changes#2017-q1

@SilverDragon135
Copy link
Author

Sorry I forgot to mention, that the purpose of this plugin is to collect logs from log collectors like nxlog, which support tcp vith ssl. I know new fluentd should work on windows, but what we wanted to cover wider variety of collectors/systems. Reason is simple - Imagine organisation divided to groups - the target is to provide every group environment they know and not to force everyone to use fluentd.

Reason for encrypt traffic from clients is partially about privacy.

The problem with in_forward was, that it requires preshared passphrase and cannot cooperate with nxlog.

If you agree I will check unit test for that and the 0.14 API support ASAP.

@cosmo0920
Copy link
Contributor

If you agree I will check unit test for that and the 0.14 API support ASAP.

server socket plugin helper might help you: #1312 😉

@tagomoris
Copy link
Member

In near future (some versions later), server plugin helper will support TLS transport. It'll be very easy to implement in_tcp on TLS after that.

@SilverDragon135
Copy link
Author

Ok for now I will check API 0.14 and unit test and keep it with ssl. Later (after server plugin will support tls) it can be optional to switch between ssl and tls based on user preferences.

@SilverDragon135
Copy link
Author

Could you please point me to some literature, why i should not use ssl ? I found out, that level of security should be the same as tls.

@tagomoris
Copy link
Member

All SSL versions have critical vulnerabilities (TLS 1.0 too). Almost major web browsers now reject to support all SSL versions.

I know that many people says "SSL" including TLS, but we need to show we're going to reject to support SSL versions.

@SilverDragon135
Copy link
Author

SilverDragon135 commented Nov 29, 2016

OK thank you a lot. I checked the OpenSSL server and it supports tls: TLSv1, TLSv1_1, TLSv1_2. So I expect it would be sufficient to setup TLSv1_1 and TLSv1_2 right ?

Is missing something else for TLS support in server.rb except:
https://github.com/fluent/fluentd/blob/c5c87099958181bb390c8d8e1b5f2b2b0ebbcca8/lib/fluent/plugin_helper/server.rb#L90
https://github.com/fluent/fluentd/blob/c5c87099958181bb390c8d8e1b5f2b2b0ebbcca8/lib/fluent/plugin_helper/server.rb#L266
https://github.com/fluent/fluentd/blob/c5c87099958181bb390c8d8e1b5f2b2b0ebbcca8/lib/fluent/plugin_helper/server.rb#L140 ? (and add paramters for certificates)

Maybe I can take a look on that. Btw I saw ruby last week first time, so please be patient with me :).

Edit: It looks like even ServerEngine doesn´t support tls too, so from this perspective it looks like there is lot to do and it would be better edit my plugin to support just tls as temporary fix and in the future reimplement it trough ServerEngine.

@tagomoris
Copy link
Member

Missing points are right.
ServerEngine's implementation is not needed, because it provides only binding sockets. Fluentd code can wrap that socket with OpenSSL::SSL::SSLServer.

The difficult point is that the async I/O interface of server plugin helper is currently implemented using cool.io. It doesn't have any support for TLS transportation. So we should wrap it, or write some code to implement async I/O via TLS on cool.io, or add completely different one to support TLS.

@SilverDragon135
Copy link
Author

Yes, I checked cool.io before, but it is not supported anymore. (See https://github.com/tarcieri/cool.io).
Replaced with: https://github.com/celluloid/celluloid-io. It should provide wrapper for OpenSSL. It looked for me much more complicated than using just OpenSSL by itself, but i did not consider async IO.

In this case the way could be port server plugin helper to celluloid. I did not studying it in deep, but APIs looks compatible - something like find&replace cool.io with celluliod.io. But of course not expecting it will be fast. Testing will take time.

I will try it in my fork, but still drowning little bit.

@repeatedly
Copy link
Member

repeatedly commented Nov 29, 2016

We don't have a plan to swtich from cool.io to celluloid-io because
this is heavy and slower than cool.io.
Fluentd also focuses on performance and lightweight, so we can't use celluloid-io.

@SilverDragon135
Copy link
Author

What about EventMachine ? I don´t like idea to support almost dead framework. EventMachine seems to be supported and with tls support.

@repeatedly
Copy link
Member

repeatedly commented Nov 29, 2016

We tested EventMachine before but API is not good and sometimes crash on high volume environment.
This is why we still use cool.io.

@SilverDragon135
Copy link
Author

Sorry guys for now I can only rewrite my plugin to suit 0.14 API and add unit test (no time for cool.io tunning). If you would like you can provide it in 0.14 fluentd as temporary solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants