From 9630f4ceaf3568d022501bb8a2462acadaa7f324 Mon Sep 17 00:00:00 2001 From: Winston Durand Date: Fri, 12 Jun 2020 12:14:57 -0700 Subject: [PATCH] prepend client middleware and append server middleware --- README.md | 37 ++++++++++-------- lib/sidekiq/encrypted_args.rb | 4 +- spec/encypted_args_spec.rb | 74 ++++++++++++++++++++++++----------- spec/spec_helper.rb | 33 ++++++++++++++++ 4 files changed, 107 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index eaec5f7..3906fef 100644 --- a/README.md +++ b/README.md @@ -24,14 +24,14 @@ Sidekiq::EncryptedArgs.configure!(secret: "YourSecretKey") If the secret is not set, the value of the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` environment variable will be used as the secret. If this variable is not set, job arguments will not be encrypted. -The call to `Sidekiq::EncryptedArgs.configure!` will append the encryption middleware to the end of the client and server middleware chains. You add the middlewares manually if you need more control over where they appear in the stacks. +The call to `Sidekiq::EncryptedArgs.configure!` will append the encryption middleware to the end of the client and server middleware chains. You can add the middlewares manually if you need more control over where they appear in the stacks. ```ruby Sidekiq::EncryptedArgs.secret = "YourSecretKey" Sidekiq.configure_client do |config| config.client_middleware do |chain| - chain.add Sidekiq::EncryptedArgs::ClientMiddleware + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware end end @@ -39,8 +39,11 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Sidekiq::EncryptedArgs::ServerMiddleware end + + # register client middleware on the server so that starting jobs in a Sidekiq::Worker also get encrypted args + # https://github.com/mperham/sidekiq/wiki/Middleware#client-middleware-registered-in-both-places config.client_middleware do |chain| - chain.add Sidekiq::EncryptedArgs::ClientMiddleware + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware end end ``` @@ -62,22 +65,24 @@ class SecretWorker end ``` -You can also encrypt just specific arguments with an array of either argument names or indexes. This can be useful to preserve visibility into non-sensitive arguments that might be useful for troubleshooting or other reasons. Both of these examples will encrypt just the second argument to the `perform` method. +You can also choose to only encrypt specific arguments with an array of either argument names (symbols or strings) or indexes. This is useful to preserve visibility into non-sensitive arguments for troubleshooting or other reasons. Both of these examples encrypt just the second argument to the `perform` method. ```ruby - # Pass in a list of argument names that should be encrypted - sidekiq_options encrypted_args: [:arg_2] +# Pass in a list of argument names that should be encrypted +sidekiq_options encrypted_args: [:arg_2] +# or +sidekiq_options encrypted_args: ["arg_2"] - def perform(arg_1, arg_2, arg_3) - end +def perform(arg_1, arg_2, arg_3) +end ``` ```ruby - # Pass in an array of integer values indicating which argument positions should be encrypted - sidekiq_options encrypted_args: [1] +# Pass in an array of integers indicating which argument positions should be encrypted +sidekiq_options encrypted_args: [1] - def perform(arg_1, arg_2, arg_3) - end +def perform(arg_1, arg_2, arg_3) +end ``` You don't need to change anything else about your workers. All of the arguments passed to the `perform` method will already be unencrypted when the method is called. @@ -87,15 +92,15 @@ You don't need to change anything else about your workers. All of the arguments If you need to roll your secret, you can simply provide an array when setting the secret. ```ruby -Sidekiq::EncryptedArgs.secret = ["CurrentSecret", "OldSecret"] +Sidekiq::EncryptedArgs.secret = ["CurrentSecret", "OldSecret", "EvenOlderSecret"] ``` -The left most key will be considered the current key and will be used for encrypting arguments. However, all of the keys will be tried when decrypting. This allows you to switch you secret keys without breaking jobs already enqueued in Redis. +The first (left most) key will be considered the current key, and is used for encrypting arguments. When decrypting, we iterate over the secrets list until we find the correct one. This allows you to switch you secret keys without breaking jobs already enqueued in Redis. -If you are using the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` envrionment variable to specify your secret, you can delimit multiple keys with a spaces. +If you are using the `SIDEKIQ_ENCRYPTED_ARGS_SECRET` environment variable to specify your secret, you can delimit multiple keys with a spaces. You can also safely add encryption to an existing worker. Any jobs that are already enqueued will still run even without having the arguments encrypted in Redis. ## Encryption -Encrypted arguments are stored using AES-256-GCM with a key derived from your secret using PBKDF2. +Encrypted arguments are stored using AES-256-GCM with a key derived from your secret using PBKDF2. For more info on the underlying encryption, refer to the [SecretKeys](https://github.com/bdurand/secret_keys) gem. diff --git a/lib/sidekiq/encrypted_args.rb b/lib/sidekiq/encrypted_args.rb index e7c7771..f25f3c8 100644 --- a/lib/sidekiq/encrypted_args.rb +++ b/lib/sidekiq/encrypted_args.rb @@ -38,7 +38,7 @@ def configure!(secret: nil) Sidekiq.configure_client do |config| config.client_middleware do |chain| - chain.add Sidekiq::EncryptedArgs::ClientMiddleware + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware end end @@ -47,7 +47,7 @@ def configure!(secret: nil) chain.add Sidekiq::EncryptedArgs::ServerMiddleware end config.client_middleware do |chain| - chain.add Sidekiq::EncryptedArgs::ClientMiddleware + chain.prepend Sidekiq::EncryptedArgs::ClientMiddleware end end end diff --git a/spec/encypted_args_spec.rb b/spec/encypted_args_spec.rb index b5371cd..6fb1c2f 100644 --- a/spec/encypted_args_spec.rb +++ b/spec/encypted_args_spec.rb @@ -30,7 +30,7 @@ expect(decrypted).to eq(attributes) end - it "should be able to set mulitiple keys to try for decrypting so a key can be gracefully rolled" do + it "should be able to set multiple keys to try for decrypting so a key can be gracefully rolled" do Sidekiq::EncryptedArgs.secret = "key_1" encrypted_1 = Sidekiq::EncryptedArgs.encrypt("foobar") @@ -81,30 +81,58 @@ expect(Sidekiq::EncryptedArgs.decrypt(1)).to eq 1 end - it "should configure the Sidekiq client middleware" do - allow(Sidekiq).to receive(:server?).and_return false - Sidekiq::EncryptedArgs.configure! - expect(Sidekiq.client_middleware.exists?(Sidekiq::EncryptedArgs::ClientMiddleware)).to eq true - end + context "loading middleware" do + around(:each) do |example| + with_empty_middleware do + example.run + end + end - it "should configure the Sidekiq server middleware" do - allow(Sidekiq).to receive(:server?).and_return true - Sidekiq::EncryptedArgs.configure! - expect(Sidekiq.server_middleware.exists?(Sidekiq::EncryptedArgs::ServerMiddleware)).to eq true - end + it "should configure the Sidekiq client middleware" do + as_sidekiq_client! + Sidekiq::EncryptedArgs.configure! + expect(Sidekiq.client_middleware.exists?(Sidekiq::EncryptedArgs::ClientMiddleware)).to eq true + expect(Sidekiq.server_middleware.exists?(Sidekiq::EncryptedArgs::ServerMiddleware)).to eq false + end - it "should set the secret from the configure! method" do - Sidekiq::EncryptedArgs.secret = nil - Sidekiq::EncryptedArgs.configure!(secret: "Foo") - encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) - expect(encryptors).to_not eq nil - end + it "should configure the Sidekiq server middleware" do + as_sidekiq_server! + Sidekiq::EncryptedArgs.configure! + expect(Sidekiq.client_middleware.exists?(Sidekiq::EncryptedArgs::ClientMiddleware)).to eq true + expect(Sidekiq.server_middleware.exists?(Sidekiq::EncryptedArgs::ServerMiddleware)).to eq true + end + + it "should set the secret from the configure! method" do + Sidekiq::EncryptedArgs.secret = nil + Sidekiq::EncryptedArgs.configure!(secret: "Foo") + encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + expect(encryptors).to_not eq nil + end + + it "should not overwrite the secret if it is not provided to the configure! method" do + Sidekiq::EncryptedArgs.secret = "Foo" + encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + Sidekiq::EncryptedArgs.configure!(secret: nil) + expect(encryptors).to_not eq [] + expect(encryptors).to eq Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + end + + it "should load client middleware first and server middleware last on server" do + as_sidekiq_server! + Sidekiq.server_middleware.add(EmptyMiddleware) + Sidekiq.client_middleware.add(EmptyMiddleware) + Sidekiq::EncryptedArgs.configure!(secret: "0xDEADBEEF") - it "should not overwrite the secret if it is not provided to the configure! method" do - Sidekiq::EncryptedArgs.secret = "Foo" - encryptors = Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) - Sidekiq::EncryptedArgs.configure! - expect(encryptors).to_not eq nil - expect(encryptors).to eq Sidekiq::EncryptedArgs.instance_variable_get(:@encryptors) + expect(Sidekiq.client_middleware.map(&:klass)).to eq [Sidekiq::EncryptedArgs::ClientMiddleware, EmptyMiddleware] + expect(Sidekiq.server_middleware.map(&:klass)).to eq [EmptyMiddleware, Sidekiq::EncryptedArgs::ServerMiddleware] + end + + it "should load client middleware first on client" do + as_sidekiq_client! + Sidekiq.client_middleware.add(EmptyMiddleware) + Sidekiq::EncryptedArgs.configure!(secret: "0xDEADBEEF") + + expect(Sidekiq.client_middleware.map(&:klass)).to eq [Sidekiq::EncryptedArgs::ClientMiddleware, EmptyMiddleware] + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4a57461..337407e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -24,6 +24,39 @@ end end +# Reset all middleware for nested context and then restore. +# +# @note Middleware args are not preserved +def with_empty_middleware + # Save the middleware context + server_middleware = Sidekiq.server_middleware.entries.map(&:klass) + client_middleware = Sidekiq.client_middleware.entries.map(&:klass) + Sidekiq.server_middleware.clear + Sidekiq.client_middleware.clear + + yield + + # Clear anything added and restore all previously registered middleware + Sidekiq.server_middleware.clear + Sidekiq.client_middleware.clear + server_middleware.each { |m| Sidekiq.server_middleware.add(m) } + client_middleware.each { |m| Sidekiq.client_middleware.add(m) } +end + +def as_sidekiq_server! + allow(Sidekiq).to receive(:server?).and_return true +end + +def as_sidekiq_client! + allow(Sidekiq).to receive(:server?).and_return false +end + +class EmptyMiddleware + def call(*args) + yield + end +end + class RegularWorker include Sidekiq::Worker