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

Remove SSL parameters from Redis connection logging to avoid exception #4532

Merged
merged 1 commit into from Apr 18, 2020
Merged

Remove SSL parameters from Redis connection logging to avoid exception #4532

merged 1 commit into from Apr 18, 2020

Conversation

geoffharcourt
Copy link
Contributor

In 3f9c4bf the Redis connection options began to be cloned (via dumping
and re-marshalling) to avoid issues with password redaction in logging
altering the connection options and breaking authentication with Sentinels.

Unfortunately, this change caused an exception on boot for users of
Redis over SSL. The OpenSSL::X509::Store object used for SSL certs is
not yet dumpable in the bundled OpenSSL wrapper for current Rubies
(although it does in master as of ruby/openssl#281).

The fix here prunes the ssl_params options out of the Redis
configuration options before the dumping and marshalling. It's probably
better not to include those in logging anyway for privacy purposes.

Fix #4531

In 3f9c4bf the Redis connection options began to be cloned (via dumping
and re-marshalling) to avoid issues with password redaction in logging
altering the connection options and breaking authentication with Sentinels.

Unfortunately, this change caused an exception on boot for users of
Redis over SSL. The `OpenSSL::X509::Store` object used for SSL certs is
not yet dumpable in the bundled OpenSSL wrapper for current Rubies
(although it does in master as of ruby/openssl#281).

The fix here prunes the `ssl_params` options out of the Redis
configuration options before the dumping and marshalling. It's probably
better not to include those in logging anyway for privacy purposes.

Fix #4531
@mperham
Copy link
Collaborator

mperham commented Apr 28, 2020

Unfortunately I learned today that Hash#except is an ActiveSupport monkeypatch. This needs to use stdlib only.

@geoffharcourt
Copy link
Contributor Author

@mperham see proposed solution in #4545.

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.

Possible regression in Sidekiq 6.0.7 with Redis and SSL certs
2 participants