tagomoris / fluent-plugin-secure-forward

Other
140 stars 30 forks source link

Private certs and SSL verify issue #56

Closed lakshmi-kannan closed 7 years ago

lakshmi-kannan commented 7 years ago

Problem

I have a fluentd forwarder/client talking to a fluentd aggregator via SSL. I rolled my own certs and I want to disable SSL verification. I therefore set "enable_strict_verification" to "no" in the config (see config snippets below) but still the node fails to connect to the aggregator and fails with following error:

2017-02-08 11:56:06 +0000 [warn]: failed to establish SSL connection error_class=OpenSSL::SSL::SSLError error=# host="" address="" port=24284

System info

root@fluentd-client001:/home/ubuntu# cat /etc/lsb-release DISTRIB_ID=Ubuntu DISTRIB_RELEASE=16.04 DISTRIB_CODENAME=xenial DISTRIB_DESCRIPTION="Ubuntu 16.04.1 LTS"

root@fluentd-client001:/home/ubuntu# sudo apt-cache policy td-agent td-agent: Installed: 2.3.4-0 Candidate: 2.3.4-0 Version table: *** 2.3.4-0 500 500 http://packages.treasuredata.com/2/ubuntu/xenial xenial/contrib amd64 Packages 100 /var/lib/dpkg/status

root@fluentd-client001:/home/ubuntu# /opt/td-agent/embedded/bin/fluent-gem list | grep fluent-plugin-secure-forward fluent-plugin-secure-forward (0.4.3) root@fluentd-client001:/home/ubuntu#

Configs

Forwarder

# Log Forwarding
<match *.**>
  @type secure_forward
  shared_key <REDACTED>
  flush_interval 60s
  self_hostname fluentd-client001
  secure yes
  enable_strict_verification no  # I want to use my own certs 
  ca_cert_path /etc/ssl/td-agent/td-agent.pem

  <server>
    host <REDACTED>
    port 24284
    hostlabel logs001.uswest2.stackstorm.net
  </server>
</match>

Aggregator

# Listen to incoming data over SSL
<source>
  type secure_forward
  shared_key <REDACTED>
  self_hostname localhost
  secure yes
  ca_cert_path /etc/ssl/td-agent/td-agent.pem
  ca_private_key_path /etc/ssl/td-agent/td-agent.key
  bind <REDACTED>
</source>
lakshmi-kannan commented 7 years ago

I tried to look at the source myself and I think I found a bug in the implementation. I am not a ruby expert but this code looks wrong https://github.com/tagomoris/fluent-plugin-secure-forward/commit/f1fa701f3bcc2c237da922ad88aa506c6a784bbb#diff-02c1517105a46b18172c993e13647c42R254

First, the default is set to VERIFY_PEER https://github.com/tagomoris/fluent-plugin-secure-forward/commit/f1fa701f3bcc2c237da922ad88aa506c6a784bbb#diff-02c1517105a46b18172c993e13647c42R254.

There is no check of enable_strict_verification config there and then the actual SSL connect happens which then throws a verify exception which is caught here https://github.com/tagomoris/fluent-plugin-secure-forward/commit/f1fa701f3bcc2c237da922ad88aa506c6a784bbb#diff-02c1517105a46b18172c993e13647c42R269.

The return basically ensures that the check of enable_strict_verification here https://github.com/tagomoris/fluent-plugin-secure-forward/commit/f1fa701f3bcc2c237da922ad88aa506c6a784bbb#diff-02c1517105a46b18172c993e13647c42R275 is not invoked.

What's the intent of this enable_strict_verification?

BTW https://github.com/tagomoris/fluent-plugin-secure-forward/issues/55 seems related.

tagomoris commented 7 years ago

SSLSocket of ruby doesn't verify the common name of certificates against the hostname now actually connected to. It requires to call SSLSocket#post_connection_check(hostname) to do it after SSLSocket#connect succeeded. The enable_strict_verification is the parameter to specify to do it. As far as I re-checked, there are no obvious bug around that code.

lakshmi-kannan commented 7 years ago

Ahh, you are right. So it's doing CN verification. I think the variable should be renamed to enable_hostname_verfication just to be clear. Usually verify in python world and in other places (like httpie) refers to verifying if a cert is signed or not. BTW, I think what we really need is a code block that's like

unless @sender.verify_ssl
  context.verify_mode = OpenSSL::SSL::VERIFY_NONE
end

verify_ssl by default should be true.

http://stackoverflow.com/questions/1113422/how-to-bypass-ssl-certificate-verification-in-open-uri

You can also use enable_strict_verification config for the same. Wdyt?

tagomoris commented 7 years ago

@lakshmi-kannan Do you want to skip all verification steps about certificates? If so, specify secure no. It's to skip setting context.verify_mode, and the default is VERIFY_NONE. https://docs.ruby-lang.org/en/2.4.0/OpenSSL/SSL/SSLContext.html

lakshmi-kannan commented 7 years ago

Ahh, secure no did the trick it looks like but is counter intuitive in terms of its name as a configuration parameter. Final working config

Forwarder:

# Log Forwarding
<match *.**>
  @type secure_forward
  shared_key <REDACTED>
  flush_interval 60s
  self_hostname fluentd-client001
  secure no
  enable_strict_verification no
  ca_cert_path /etc/ssl/td-agent/td-agent.pem

  <server>
    host <IP>
    port 24284
    hostlabel logs001.uswest2.stackstorm.net
  </server>
</match>

Aggregator:

# Listen to incoming data over SSL
<source>
  type secure_forward
  shared_key <REDACTED>
  self_hostname localhost
  secure no
  ca_cert_path /etc/ssl/td-agent/td-agent.pem
  ca_private_key_path /etc/ssl/td-agent/td-agent.key
  bind <IP>
</source>

This document https://github.com/tagomoris/fluent-plugin-secure-forward#using-insecure-self-signed-certificates should be updated to include the ca_cert_path and ca_private_key_path. Otherwise, it's misleading and it looks like secure no implies no SSL. I'll leave it up to you to address that and will close the issue. Thanks for the help!