From 6bf68bbf43df707de42d66b84475f0ae3bdf418c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Fernandes?= Date: Sat, 4 Jan 2025 00:02:59 +0000 Subject: [PATCH] Allow the v2 encryptor to serialize messages with `Marshal` (#44) Marshal and JSON serialization are unavoidably different. A key difference being that JSON serializes ruby symbols as strings, while Marshal preserves them as symbols. * Support UTF-8 data when using the JSON serializer * Replace all instance of 'BINARY' with `Encoding::BINARY` --- lib/rack/session/encryptor.rb | 14 +- test/spec_session_encryptor.rb | 241 ++++++++++++++++++--------------- 2 files changed, 142 insertions(+), 113 deletions(-) diff --git a/lib/rack/session/encryptor.rb b/lib/rack/session/encryptor.rb index 8d861d5..851245b 100644 --- a/lib/rack/session/encryptor.rb +++ b/lib/rack/session/encryptor.rb @@ -223,12 +223,13 @@ class V2 # # Considerations about V2: # - # 1) It serializes messages in JSON, period. - # - # 2) It uses non URL-safe Base64 encoding as it's faster than its + # 1) It uses non URL-safe Base64 encoding as it's faster than its # URL-safe counterpart - as of Ruby 3.2, Base64.urlsafe_encode64 is - # roughly equivalent to do Base64.strict_encode64(data).tr("-_", - # "+/") - and cookie values don't need to be URL-safe. + # roughly equivalent to + # + # Base64.strict_encode64(data).tr("-_", "+/") + # + # - and cookie values don't need to be URL-safe. def initialize(secret, opts = {}) raise ArgumentError, 'secret must be a String' unless secret.is_a?(String) @@ -246,9 +247,8 @@ def initialize(secret, opts = {}) end @options = { - pad_size: 32, purpose: nil + serialize_json: false, pad_size: 32, purpose: nil }.update(opts) - @options[:serialize_json] = true # Enforce JSON serialization @cipher_secret = secret.dup.force_encoding(Encoding::BINARY).slice!(0, 32) @cipher_secret.freeze diff --git a/test/spec_session_encryptor.rb b/test/spec_session_encryptor.rb index ce24d21..eeb95fe 100644 --- a/test/spec_session_encryptor.rb +++ b/test/spec_session_encryptor.rb @@ -10,102 +10,118 @@ require 'json' require 'securerandom' -module EncryptorTests - def self.included(_base) - describe 'encryptor' do - it 'initialize does not destroy key string' do - encryptor_class.new(@secret) +def all_versions_tests(opts = {}) + Module.new do + define_method(:default_opts) do + opts + end - @secret.size.must_equal 64 - end + def self.included(_base) + describe 'encryptor' do + it 'initialize does not destroy key string' do + new_encryptor(@secret) - it 'initialize raises ArgumentError on invalid key' do - -> { encryptor_class.new ['foo'] }.must_raise ArgumentError - end + @secret.size.must_equal 64 + end - it 'initialize raises ArgumentError on short key' do - -> { encryptor_class.new 'key' }.must_raise ArgumentError - end + it 'initialize raises ArgumentError on invalid key' do + -> { new_encryptor ['foo'] }.must_raise ArgumentError + end - it 'decrypts an encrypted message' do - encryptor = encryptor_class.new(@secret) + it 'initialize raises ArgumentError on short key' do + -> { new_encryptor 'key' }.must_raise ArgumentError + end - message = encryptor.encrypt({ 'foo' => 'bar' }) + it 'decrypts an encrypted message' do + encryptor = new_encryptor(@secret) - encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) - end + message = encryptor.encrypt({ 'foo' => 'bar' }) - it 'decrypt raises InvalidSignature for tampered messages' do - encryptor = encryptor_class.new(@secret) + encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) + end - message = encryptor.encrypt({ 'foo' => 'bar' }) + it 'decrypt raises InvalidSignature for tampered messages' do + encryptor = new_encryptor(@secret) - decoded_message = Base64.urlsafe_decode64(message) - tampered_message = Base64.urlsafe_encode64(decoded_message.tap do |m| - m[m.size - 1] = (m[m.size - 1].unpack1('C') ^ 1).chr - end) + message = encryptor.encrypt({ 'foo' => 'bar' }) - lambda { - encryptor.decrypt(tampered_message) - }.must_raise Rack::Session::Encryptor::InvalidSignature - end + decoded_message = Base64.urlsafe_decode64(message) + tampered_message = Base64.urlsafe_encode64(decoded_message.tap do |m| + m[m.size - 1] = (m[m.size - 1].unpack1('C') ^ 1).chr + end) - it 'decrypts an encrypted message with purpose' do - encryptor = encryptor_class.new(@secret, purpose: 'testing') + lambda { + encryptor.decrypt(tampered_message) + }.must_raise Rack::Session::Encryptor::InvalidSignature + end - message = encryptor.encrypt({ 'foo' => 'bar' }) + it 'decrypts an encrypted message with purpose' do + encryptor = new_encryptor(@secret, purpose: 'testing') - encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) - end + message = encryptor.encrypt({ 'foo' => 'bar' }) - # The V1 encryptor defaults to the Marshal serializer, while the V2 - # encryptor always uses the JSON serializer. This means that we are - # indirectly covering both serializers. - it 'decrypts an encrypted message with UTF-8 data' do - encryptor = encryptor_class.new(@secret) + encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) + end - encrypted_message = encryptor.encrypt({ 'foo' => 'bar 😀' }) - decrypted_message = encryptor.decrypt(encrypted_message) + it 'decrypts an encrypted message with UTF-8 data' do + encryptor = new_encryptor(@secret) - decrypted_message.must_equal({ 'foo' => 'bar 😀' }) - end + encrypted_message = encryptor.encrypt({ 'foo' => 'bar 😀' }) + decrypted_message = encryptor.decrypt(encrypted_message) - it 'decrypts raises InvalidSignature without purpose' do - encryptor = encryptor_class.new(@secret, purpose: 'testing') - other_encryptor = encryptor_class.new(@secret) + decrypted_message.must_equal({ 'foo' => 'bar 😀' }) + end - message = other_encryptor.encrypt({ 'foo' => 'bar' }) + it 'decrypts raises InvalidSignature without purpose' do + encryptor = new_encryptor(@secret, purpose: 'testing') + other_encryptor = new_encryptor(@secret) - -> { encryptor.decrypt(message) }.must_raise Rack::Session::Encryptor::InvalidSignature - end + message = other_encryptor.encrypt({ 'foo' => 'bar' }) - it 'decrypts raises InvalidSignature with different purpose' do - encryptor = encryptor_class.new(@secret, purpose: 'testing') - other_encryptor = encryptor_class.new(@secret, purpose: 'other') + -> { encryptor.decrypt(message) }.must_raise Rack::Session::Encryptor::InvalidSignature + end - message = other_encryptor.encrypt({ 'foo' => 'bar' }) + it 'decrypts raises InvalidSignature with different purpose' do + encryptor = new_encryptor(@secret, purpose: 'testing') + other_encryptor = new_encryptor(@secret, purpose: 'other') - -> { encryptor.decrypt(message) }.must_raise Rack::Session::Encryptor::InvalidSignature - end + message = other_encryptor.encrypt({ 'foo' => 'bar' }) - it 'initialize raises ArgumentError on invalid pad_size' do - -> { encryptor_class.new @secret, pad_size: :bar }.must_raise ArgumentError - end + -> { encryptor.decrypt(message) }.must_raise Rack::Session::Encryptor::InvalidSignature + end - it 'initialize raises ArgumentError on to short pad_size' do - -> { encryptor_class.new @secret, pad_size: 1 }.must_raise ArgumentError - end + it 'initialize raises ArgumentError on invalid pad_size' do + -> { new_encryptor @secret, pad_size: :bar }.must_raise ArgumentError + end - it 'initialize raises ArgumentError on to long pad_size' do - -> { encryptor_class.new @secret, pad_size: 8023 }.must_raise ArgumentError - end + it 'initialize raises ArgumentError on to short pad_size' do + -> { new_encryptor @secret, pad_size: 1 }.must_raise ArgumentError + end + + it 'initialize raises ArgumentError on to long pad_size' do + -> { new_encryptor @secret, pad_size: 8023 }.must_raise ArgumentError + end + + it 'decrypts an encrypted message without pad_size' do + encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: nil) - it 'decrypts an encrypted message without pad_size' do - encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: nil) + message = encryptor.encrypt({ 'foo' => 'bar' }) - message = encryptor.encrypt({ 'foo' => 'bar' }) + encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) + end - encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) + it 'encryptor with pad_size increases message size' do + no_pad_encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: nil) + pad_encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: 64) + + message_without = Base64.urlsafe_decode64(no_pad_encryptor.encrypt('')) + message_with = Base64.urlsafe_decode64(pad_encryptor.encrypt('')) + message_size_diff = message_with.bytesize - message_without.bytesize + + message_with.bytesize.must_be :>, message_without.bytesize + serializer = default_opts[:serialize_json] ? JSON : Marshal + message_size_diff.must_equal 64 - serializer.dump('').bytesize - 2 + end end end end @@ -116,15 +132,24 @@ def setup @secret = SecureRandom.random_bytes(64) end + def new_encryptor(secret, opts = {}) + if respond_to?(:default_opts) + encryptor_class.new(secret, default_opts.merge(opts)) + else + encryptor_class.new(secret, opts) + end + end + describe 'V1' do def encryptor_class Rack::Session::Encryptor::V1 end - include EncryptorTests + include all_versions_tests(serialize_json: false) + include all_versions_tests(serialize_json: true) it 'encryptor with pad_size has message payload size to multiple of pad_size' do - encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: 24) + encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: 24) message = encryptor.encrypt({ 'foo' => 'bar' * 4 }) decoded_message = Base64.urlsafe_decode64(message) @@ -136,24 +161,12 @@ def encryptor_class (encrypted_payload.bytesize % 24).must_equal 0 end - it 'encryptor with pad_size increases message size' do - no_pad_encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: nil) - pad_encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: 64) - - message_without = Base64.urlsafe_decode64(no_pad_encryptor.encrypt('')) - message_with = Base64.urlsafe_decode64(pad_encryptor.encrypt('')) - message_size_diff = message_with.bytesize - message_without.bytesize - - message_with.bytesize.must_be :>, message_without.bytesize - message_size_diff.must_equal 64 - Marshal.dump('').bytesize - 2 - end - # This test checks the one-time message key IS NOT used as the cipher key. # Doing so would remove the confidentiality assurances as the key is # essentially included in plaintext then. it 'uses a secret cipher key for encryption and decryption' do cipher = OpenSSL::Cipher.new('aes-256-ctr') - encryptor = encryptor_class.new(@secret) + encryptor = new_encryptor(@secret) message = encryptor.encrypt({ 'foo' => 'bar' }) raw_message = Base64.urlsafe_decode64(message) @@ -178,7 +191,7 @@ def encryptor_class end it 'it calls set_cipher_key with the correct key' do - encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: 24) + encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: 24) message = encryptor.encrypt({ 'foo' => 'bar' }) message_key = Base64.urlsafe_decode64(message).slice(1, 32) @@ -194,6 +207,24 @@ def encryptor_class encryptor.decrypt message end end + + it 'preserves symbols when payloads are not encoded into JSON' do + encryptor = new_encryptor(@secret, purpose: 'testing', serialize_json: false) + + encrypted_message = encryptor.encrypt({ foo: 'bar' }) + decrypted_message = encryptor.decrypt(encrypted_message) + + decrypted_message.must_equal({ foo: 'bar' }) + end + + it 'does not preserves symbols when payloads are encoded into JSON' do + encryptor = new_encryptor(@secret, purpose: 'testing', serialize_json: true) + + encrypted_message = encryptor.encrypt({ foo: 'bar' }) + decrypted_message = encryptor.decrypt(encrypted_message) + + decrypted_message.must_equal({ 'foo' => 'bar' }) + end end describe 'V2' do @@ -201,10 +232,11 @@ def encryptor_class Rack::Session::Encryptor::V2 end - include EncryptorTests + include all_versions_tests(serialize_json: false) + include all_versions_tests(serialize_json: true) it 'encryptor with pad_size has message payload size to multiple of pad_size' do - encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: 24) + encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: 24) message = encryptor.encrypt({ 'foo' => 'bar' * 4 }) decoded_message = Base64.strict_decode64(message) @@ -216,20 +248,8 @@ def encryptor_class (encrypted_payload.bytesize % 24).must_equal 0 end - it 'encryptor with pad_size increases message size' do - no_pad_encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: nil) - pad_encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: 64) - - message_without = Base64.strict_decode64(no_pad_encryptor.encrypt('')) - message_with = Base64.strict_decode64(pad_encryptor.encrypt('')) - message_size_diff = message_with.bytesize - message_without.bytesize - - message_with.bytesize.must_be :>, message_without.bytesize - message_size_diff.must_equal 64 - JSON.dump('').bytesize - 2 - end - it 'raises InvalidMessage on version mismatch' do - encryptor = encryptor_class.new(@secret, purpose: 'testing') + encryptor = new_encryptor(@secret, purpose: 'testing') message = encryptor.encrypt({ 'foo' => 'bar' }) decoded_message = Base64.strict_decode64(message) @@ -244,7 +264,7 @@ def encryptor_class # essentially included in plaintext then. it 'uses a secret cipher key for encryption and decryption' do cipher = OpenSSL::Cipher.new('aes-256-gcm') - encryptor = encryptor_class.new(@secret) + encryptor = new_encryptor(@secret) message = encryptor.encrypt({ 'foo' => 'bar' }) raw_message = Base64.strict_decode64(message) @@ -264,7 +284,7 @@ def encryptor_class end it 'it calls set_cipher_key with the correct key' do - encryptor = encryptor_class.new(@secret, purpose: 'testing', pad_size: 24) + encryptor = new_encryptor(@secret, purpose: 'testing', pad_size: 24) message = encryptor.encrypt({ 'foo' => 'bar' }) message_key = Base64.strict_decode64(message).slice(1, 32) @@ -281,13 +301,22 @@ def encryptor_class end end - it 'ignores serialize_json' do - encryptor_no_json = encryptor_class.new(@secret, purpose: 'testing', serialize_json: false) - encryptor = encryptor_class.new(@secret, purpose: 'testing', serialize_json: true) + it 'preserves symbols when payloads are not encoded into JSON' do + encryptor = new_encryptor(@secret, purpose: 'testing', serialize_json: false) - message = encryptor_no_json.encrypt({ 'foo' => 'bar' }) + encrypted_message = encryptor.encrypt({ foo: 'bar' }) + decrypted_message = encryptor.decrypt(encrypted_message) - encryptor.decrypt(message).must_equal({ 'foo' => 'bar' }) + decrypted_message.must_equal({ foo: 'bar' }) + end + + it 'does not preserves symbols when payloads are encoded into JSON' do + encryptor = new_encryptor(@secret, purpose: 'testing', serialize_json: true) + + encrypted_message = encryptor.encrypt({ foo: 'bar' }) + decrypted_message = encryptor.decrypt(encrypted_message) + + decrypted_message.must_equal({ 'foo' => 'bar' }) end end