From 027de63b3d26e70f4c91e8e8db4cd3a787bc7f27 Mon Sep 17 00:00:00 2001 From: Micah Lee Date: Fri, 10 Feb 2023 13:03:37 -0500 Subject: [PATCH 1/2] Add basic dev environment to this repo To make it easier to run tests in a container. --- dev/Dockerfile.dev | 7 +++++++ dev/docker-compose.yml | 8 ++++++++ 2 files changed, 15 insertions(+) create mode 100644 dev/Dockerfile.dev create mode 100644 dev/docker-compose.yml diff --git a/dev/Dockerfile.dev b/dev/Dockerfile.dev new file mode 100644 index 0000000..219b658 --- /dev/null +++ b/dev/Dockerfile.dev @@ -0,0 +1,7 @@ +FROM ruby + +COPY ./ /src/ + +WORKDIR /src + +RUN bundle diff --git a/dev/docker-compose.yml b/dev/docker-compose.yml new file mode 100644 index 0000000..233ec2a --- /dev/null +++ b/dev/docker-compose.yml @@ -0,0 +1,8 @@ +version: '3' +services: + dev: + build: + context: .. + dockerfile: dev/Dockerfile.dev + volumes: + - ../:/src From c9408a06c944fd9fca9f781a61d417a459e6970b Mon Sep 17 00:00:00 2001 From: Micah Lee Date: Fri, 10 Feb 2023 13:19:09 -0500 Subject: [PATCH 2/2] Improve thread safety of Symmetric cipher Previously using Symmetric encryption from multiple threads could result in errors from OpenSSL. This commit adds a mutex to synchronize access to the OpenSSL instance for encryption and decryption. --- CHANGELOG.md | 5 +++++ lib/slosilo/symmetric.rb | 43 ++++++++++++++++++++++++---------------- lib/slosilo/version.rb | 2 +- spec/symmetric_spec.rb | 25 +++++++++++++++++++++-- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a143f3..d287dc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# v3.0.1 + + * The symmetric cipher class now encrypts and decrypts in a thread-safe manner. + [cyberark/slosilo#31](https://github.com/cyberark/slosilo/pull/31) + # v3.0.0 * Transition to Ruby 3. Consuming projects based on Ruby 2 shall use slosilo V2.X.X. diff --git a/lib/slosilo/symmetric.rb b/lib/slosilo/symmetric.rb index f6db06a..7c783f0 100644 --- a/lib/slosilo/symmetric.rb +++ b/lib/slosilo/symmetric.rb @@ -5,6 +5,7 @@ class Symmetric def initialize @cipher = OpenSSL::Cipher.new 'aes-256-gcm' # NB: has to be lower case for whatever reason. + @cipher_mutex = Mutex.new end # This lets us do a final sanity check in migrations from older encryption versions @@ -13,14 +14,18 @@ def cipher_name end def encrypt plaintext, opts = {} - @cipher.reset - @cipher.encrypt - @cipher.key = (opts[:key] or raise("missing :key option")) - @cipher.iv = iv = random_iv - @cipher.auth_data = opts[:aad] || "" # Nothing good happens if you set this to nil, or don't set it at all - ctext = @cipher.update(plaintext) + @cipher.final - tag = @cipher.auth_tag(TAG_LENGTH) - "#{VERSION_MAGIC}#{tag}#{iv}#{ctext}" + # All of these operations in OpenSSL must occur atomically, so we + # synchronize their access to make this step thread-safe. + @cipher_mutex.synchronize do + @cipher.reset + @cipher.encrypt + @cipher.key = (opts[:key] or raise("missing :key option")) + @cipher.iv = iv = random_iv + @cipher.auth_data = opts[:aad] || "" # Nothing good happens if you set this to nil, or don't set it at all + ctext = @cipher.update(plaintext) + @cipher.final + tag = @cipher.auth_tag(TAG_LENGTH) + "#{VERSION_MAGIC}#{tag}#{iv}#{ctext}" + end end def decrypt ciphertext, opts = {} @@ -28,19 +33,23 @@ def decrypt ciphertext, opts = {} raise "Invalid version magic: expected #{VERSION_MAGIC} but was #{version}" unless version == VERSION_MAGIC - @cipher.reset - @cipher.decrypt - @cipher.key = opts[:key] - @cipher.iv = iv - @cipher.auth_tag = tag - @cipher.auth_data = opts[:aad] || "" - @cipher.update(ctext) + @cipher.final + # All of these operations in OpenSSL must occur atomically, so we + # synchronize their access to make this step thread-safe. + @cipher_mutex.synchronize do + @cipher.reset + @cipher.decrypt + @cipher.key = opts[:key] + @cipher.iv = iv + @cipher.auth_tag = tag + @cipher.auth_data = opts[:aad] || "" + @cipher.update(ctext) + @cipher.final + end end - + def random_iv @cipher.random_iv end - + def random_key @cipher.random_key end diff --git a/lib/slosilo/version.rb b/lib/slosilo/version.rb index e64f621..27091ff 100644 --- a/lib/slosilo/version.rb +++ b/lib/slosilo/version.rb @@ -1,3 +1,3 @@ module Slosilo - VERSION = "3.0.0" + VERSION = "3.0.1" end diff --git a/spec/symmetric_spec.rb b/spec/symmetric_spec.rb index 234e1bf..c3d4686 100644 --- a/spec/symmetric_spec.rb +++ b/spec/symmetric_spec.rb @@ -14,8 +14,29 @@ expect(subject.encrypt(plaintext, key: key, aad: auth_data)).to eq(ciphertext) end end - + describe '#decrypt' do + + it "doesn't fail when called by multiple threads" do + threads = [] + + begin + # Verify we can successfuly decrypt using many threads without OpenSSL + # errors. + 1000.times do + threads << Thread.new do + 100.times do + expect( + subject.decrypt(ciphertext, key: key, aad: auth_data) + ).to eq(plaintext) + end + end + end + ensure + threads.each(&:join) + end + end + it "decrypts with AES-256-GCM" do expect(subject.decrypt(ciphertext, key: key, aad: auth_data)).to eq(plaintext) end @@ -56,7 +77,7 @@ end end end - + describe '#random_iv' do it "generates a random iv" do expect_any_instance_of(OpenSSL::Cipher).to receive(:random_iv).and_return :iv