search

scalyr / scalyr-fluentd

The fluentd plugin for inserting log messages and metrics in Scalyr.
Apache License 2.0
5 stars 5 forks source link

Change the default of `ssl_ca_bundle_path` to be unset and let the library handle it #21

Closed yanscalyr closed 3 years ago

yanscalyr commented 3 years ago

From my testing not setting the value of ssl_ca_bundle_path makes the library look in an appropriate place on its own. This helps with installing our plugin on top of the standard fluentd docker image because the previous default was incorrect for it. I'm sure this makes the plugin easier to use for many other systems as well.

Kami commented 3 years ago

If not specifying a path makes it using system CA bundle and it knows how to handle various different locations on different distributions, I'm fine with this change, but we need to make sure we have good end to end tests for it to make sure CA verification and hostname check is indeed performed.

yanscalyr commented 3 years ago

All tests now pass in CircleCI, but when running them locally the test_bad_ssl_certificates test fails because no message about "certificate verification failed" is output, instead we get a 401 status. I would think certificate failure should take precedence but maybe it depends on the machine somehow... I don't know. I would appreciate if someone gave this a shot on their end. @Kami ?

Kami commented 3 years ago

I was curious how it works and it looks like it uses OpenSSL::X509::DEFAULT_CERT_FILE constant when trying to find a default CA certificates bundle file which should use.

And it appears it correctly fails if that file is not available.

ruby -ropenssl -e 'puts OpenSSL::X509::DEFAULT_CERT_FILE'

Having said that, it would be good to add another test case which verifies that cert validation indeed correctly fails if that file is not present (we can temporary move that file for a single test and then move it back in place).

yanscalyr commented 3 years ago

I was curious how it works and it looks like it uses OpenSSL::X509::DEFAULT_CERT_FILE constant when trying to find a default CA certificates bundle file which should use.

And it appears it correctly fails if that file is not available.

ruby -ropenssl -e 'puts OpenSSL::X509::DEFAULT_CERT_FILE'

Having said that, it would be good to add another test case which verifies that cert validation indeed correctly fails if that file is not present (we can temporary move that file for a single test and then move it back in place).

Looks like it can't find the file at OpenSSL::X509::DEFAULT_CERT_FILE in the docker image, which is weird since the test that leaves the path unset doesn't throw any cert errors. Is there maybe fallback paths or something...

Kami commented 3 years ago

Locally I received an exception if the file didn't exist (I tried Ruby 1.9.3, 2.1.0 and 2.6.0), but I need to check Docker image to see what is going on and why it's not failing.

yanscalyr commented 3 years ago

Looks like there actually is fallbacks if OpenSSL::X509::DEFAULT_CERT_FILE isn't present, since there is also a OpenSSL::X509::DEFAULT_CERT_DIR constant. I think we will need to move both of these to get the failures to validate that we expect.

yanscalyr commented 3 years ago

That seemed to resolve the issue in CircleCI, I guess it's a good thing it's so hard to make it not find valid certs. Locally on my own machine it still throws a 4XX exception instead of an SSLError for that test, and I still have no explanation for that behavior, maybe even more fallbacks that I'm not aware of...

yanscalyr commented 3 years ago

"With mac default Ruby it seems to ignore the documented behavior of "will use system default certificates unless you specify a certificate file or directory", and uses them even if you specify something. This results in the test where we specify a bad path to fail by verifying to certificate anyway. If we install a stable ruby version from rvm then the above works as expected, but it still seems to look in an unusual place for certificates. This means in the test where we specify nothing but move any certificated away from the normally expected locations which are defined in OpenSSL::X509::DEFAULT_CERT_FILE and OpenSSL::X509::DEFAULT_CERT_DIR constants. It is in the documentation that these are "usually" the locations so I guess it is somewhat expected that this test setup wont work in every environment. Ive had no luck finding the real locations unfortunately. The interesting parts of the OpenSSL library aren't written in Ruby so I can't look through them as easily to see whats going on."

That is the result of investigating why local tests act strangely, for now that is enough to make us confident in the Circle tests, so I'm merging this in.