diff --git a/lib/crypt_keeper.rb b/lib/crypt_keeper.rb index bdca203..7306f66 100644 --- a/lib/crypt_keeper.rb +++ b/lib/crypt_keeper.rb @@ -6,6 +6,7 @@ require 'crypt_keeper/provider/base' require 'crypt_keeper/provider/aes_new' require 'crypt_keeper/provider/mysql_aes_new' +require 'crypt_keeper/provider/postgres_base' require 'crypt_keeper/provider/postgres_pgp' require 'crypt_keeper/provider/postgres_pgp_public_key' diff --git a/lib/crypt_keeper/helper.rb b/lib/crypt_keeper/helper.rb index b6d6d3c..8085b6a 100644 --- a/lib/crypt_keeper/helper.rb +++ b/lib/crypt_keeper/helper.rb @@ -3,10 +3,38 @@ module Helper module SQL private - # Private: Sanitize an sql query and then execute it - def escape_and_execute_sql(query) + # Private: Sanitize an sql query and then execute it. + # + # query - the sql query + # new_transaction - if the query should run inside a new transaction + # + # Returns the ActiveRecord response. + def escape_and_execute_sql(query, new_transaction: false) query = ::ActiveRecord::Base.send :sanitize_sql_array, query - ::ActiveRecord::Base.connection.execute(query).first + + if CryptKeeper.silence_logs? + ::ActiveRecord::Base.logger.silence do + execute_sql(query, new_transaction: new_transaction) + end + else + execute_sql(query, new_transaction: new_transaction) + end + end + + # Private: Executes the query. + # + # query - the sql query + # new_transaction - if the query should run inside a new transaction + # + # Returns an Array. + def execute_sql(query, new_transaction: false) + if new_transaction + ::ActiveRecord::Base.transaction(requires_new: true) do + ::ActiveRecord::Base.connection.execute(query).first + end + else + ::ActiveRecord::Base.connection.execute(query).first + end end end diff --git a/lib/crypt_keeper/log_subscriber/mysql_aes.rb b/lib/crypt_keeper/log_subscriber/mysql_aes.rb index 9dd98cd..1039c01 100644 --- a/lib/crypt_keeper/log_subscriber/mysql_aes.rb +++ b/lib/crypt_keeper/log_subscriber/mysql_aes.rb @@ -14,8 +14,6 @@ def sql(event) payload = event.payload[:sql] .encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '') - return if CryptKeeper.silence_logs? && payload =~ filter - event.payload[:sql] = payload.gsub(filter) do |_| "#{$1}([FILTERED])" end diff --git a/lib/crypt_keeper/log_subscriber/postgres_pgp.rb b/lib/crypt_keeper/log_subscriber/postgres_pgp.rb index dbd65aa..ab24218 100644 --- a/lib/crypt_keeper/log_subscriber/postgres_pgp.rb +++ b/lib/crypt_keeper/log_subscriber/postgres_pgp.rb @@ -4,7 +4,7 @@ module CryptKeeper module LogSubscriber module PostgresPgp - FILTER = /(\(*)pgp_(sym|pub)_(?decrypt|encrypt)(\(+.*\)+)/im + FILTER = /(\(*)(?pgp_sym_encrypt|pgp_sym_decrypt|pgp_pub_encrypt|pgp_pub_decrypt|pgp_key_id)(\(+.*\)+)/im # Public: Prevents sensitive data from being logged # @@ -13,9 +13,6 @@ module PostgresPgp # Returns a boolean. def sql(event) payload = crypt_keeper_payload_parse(event.payload[:sql]) - - return if CryptKeeper.silence_logs? && payload =~ FILTER - event.payload[:sql] = crypt_keeper_filter_postgres_log(payload) super(event) end diff --git a/lib/crypt_keeper/provider/postgres_base.rb b/lib/crypt_keeper/provider/postgres_base.rb new file mode 100644 index 0000000..0a42dc1 --- /dev/null +++ b/lib/crypt_keeper/provider/postgres_base.rb @@ -0,0 +1,28 @@ +require 'crypt_keeper/log_subscriber/postgres_pgp' + +module CryptKeeper + module Provider + class PostgresBase < Base + include CryptKeeper::Helper::SQL + include CryptKeeper::LogSubscriber::PostgresPgp + + INVALID_DATA_ERROR = "Wrong key or corrupt data".freeze + + # Public: Checks if value is already encrypted. + # + # Returns boolean + def encrypted?(value) + begin + escape_and_execute_sql(["SELECT pgp_key_id(?)", value.to_s], + new_transaction: true)['pgp_key_id'].present? + rescue ActiveRecord::StatementInvalid => e + if e.message.include?(INVALID_DATA_ERROR) + false + else + raise + end + end + end + end + end +end diff --git a/lib/crypt_keeper/provider/postgres_pgp.rb b/lib/crypt_keeper/provider/postgres_pgp.rb index c994aba..a653817 100644 --- a/lib/crypt_keeper/provider/postgres_pgp.rb +++ b/lib/crypt_keeper/provider/postgres_pgp.rb @@ -1,11 +1,6 @@ -require 'crypt_keeper/log_subscriber/postgres_pgp' - module CryptKeeper module Provider - class PostgresPgp < Base - include CryptKeeper::Helper::SQL - include CryptKeeper::LogSubscriber::PostgresPgp - + class PostgresPgp < PostgresBase attr_accessor :key attr_accessor :pgcrypto_options @@ -37,8 +32,12 @@ def encrypt(value) # Returns a plaintext string def decrypt(value) rescue_invalid_statement do - escape_and_execute_sql(["SELECT pgp_sym_decrypt(?, ?)", - value, key])['pgp_sym_decrypt'] + if encrypted?(value) + escape_and_execute_sql(["SELECT pgp_sym_decrypt(?, ?)", + value, key])['pgp_sym_decrypt'] + else + value + end end end diff --git a/lib/crypt_keeper/provider/postgres_pgp_public_key.rb b/lib/crypt_keeper/provider/postgres_pgp_public_key.rb index ce0d5d8..a2e5f6c 100644 --- a/lib/crypt_keeper/provider/postgres_pgp_public_key.rb +++ b/lib/crypt_keeper/provider/postgres_pgp_public_key.rb @@ -1,10 +1,6 @@ -require 'crypt_keeper/log_subscriber/postgres_pgp' - module CryptKeeper module Provider - class PostgresPgpPublicKey < Base - include CryptKeeper::Helper::SQL - + class PostgresPgpPublicKey < PostgresBase attr_accessor :key def initialize(options = {}) @@ -33,25 +29,13 @@ def encrypt(value) # # Returns a plaintext string def decrypt(value) - if @private_key.present? - escape_and_execute_sql(["SELECT pgp_pub_decrypt(?, dearmor(?), ?)", value, @private_key, @key])['pgp_pub_decrypt'] + if @private_key.present? && encrypted?(value) + escape_and_execute_sql(["SELECT pgp_pub_decrypt(?, dearmor(?), ?)", + value, @private_key, @key])['pgp_pub_decrypt'] else value end end - - # Public: Attempts to extract a PGP key id. If it's successful, it returns true - # - # Returns boolean - def encrypted?(value) - begin - ActiveRecord::Base.transaction(requires_new: true) do - escape_and_execute_sql(["SELECT pgp_key_id(?)", value.to_s])['pgp_key_id'].present? - end - rescue ActiveRecord::StatementInvalid - false - end - end end end end diff --git a/lib/crypt_keeper/version.rb b/lib/crypt_keeper/version.rb index aaaca39..648ec92 100644 --- a/lib/crypt_keeper/version.rb +++ b/lib/crypt_keeper/version.rb @@ -1,3 +1,3 @@ module CryptKeeper - VERSION = "1.1.0" + VERSION = "1.1.1" end diff --git a/spec/crypt_keeper/log_subscriber/mysql_aes_spec.rb b/spec/crypt_keeper/log_subscriber/mysql_aes_spec.rb index bc6a543..781f66c 100644 --- a/spec/crypt_keeper/log_subscriber/mysql_aes_spec.rb +++ b/spec/crypt_keeper/log_subscriber/mysql_aes_spec.rb @@ -46,11 +46,5 @@ should_log_scrubbed_query(input: input_query, output: output_query) end - - it "skips logging if CryptKeeper.silence_logs is set" do - CryptKeeper.silence_logs = true - - should_not_log_query(input_query) - end end end diff --git a/spec/crypt_keeper/log_subscriber/postgres_pgp_spec.rb b/spec/crypt_keeper/log_subscriber/postgres_pgp_spec.rb index 0b22e7b..a6f7e23 100644 --- a/spec/crypt_keeper/log_subscriber/postgres_pgp_spec.rb +++ b/spec/crypt_keeper/log_subscriber/postgres_pgp_spec.rb @@ -13,44 +13,32 @@ CryptKeeper::Provider::PostgresPgp.new key: 'secret' end - let(:input_query) do - "SELECT pgp_sym_encrypt('encrypt_value', 'encrypt_key'), pgp_sym_decrypt('decrypt_value', 'decrypt_key') FROM DUAL;" - end - - let(:output_query) do - "SELECT encrypt([FILTERED]) FROM DUAL;" - end - - let(:input_search_query) do - "SELECT \"sensitive_data\".* FROM \"sensitive_data\" WHERE ((pgp_sym_decrypt('f'), 'tool') = 'blah')) AND secret = 'testing'" - end - - let(:output_search_query) do - "SELECT \"sensitive_data\".* FROM \"sensitive_data\" WHERE decrypt([FILTERED]) AND secret = 'testing'" - end + let(:queries) { + { + "SELECT pgp_sym_encrypt('encrypt_value', 'encrypt_key') FROM DUAL;" => "SELECT pgp_sym_encrypt([FILTERED]) FROM DUAL;", + "SELECT pgp_sym_decrypt('encrypt_value') FROM DUAL;" => "SELECT pgp_sym_decrypt([FILTERED]) FROM DUAL;", + "SELECT pgp_key_id('encrypt_value') FROM DUAL;" => "SELECT pgp_key_id([FILTERED]) FROM DUAL;", + "SELECT \"sensitive_data\".* FROM \"sensitive_data\" WHERE ((pgp_sym_decrypt('f'), 'tool') = 'blah')) AND secret = 'testing'" => "SELECT \"sensitive_data\".* FROM \"sensitive_data\" WHERE pgp_sym_decrypt([FILTERED]) AND secret = 'testing'", + } + } it "filters pgp functions" do - should_log_scrubbed_query(input: input_query, output: output_query) + queries.each do |k, v| + should_log_scrubbed_query(input: k, output: v) + end end it "filters pgp functions in lowercase" do - should_log_scrubbed_query(input: input_query.downcase, output: output_query.downcase.gsub(/filtered/, 'FILTERED')) - end - - it "filters pgp functions when searching" do - should_log_scrubbed_query(input: input_search_query, output: output_search_query) + queries.each do |k, v| + should_log_scrubbed_query(input: k.downcase, output: v.downcase.gsub(/filtered/, 'FILTERED')) + end end it "forces string encodings" do - input_query = "SELECT pgp_sym_encrypt('hi \255', 'test') FROM DUAL;" - - should_log_scrubbed_query(input: input_query, output: output_query) - end - - it "skips logging if CryptKeeper.silence_logs is set" do - CryptKeeper.silence_logs = true - - should_not_log_query(input_query) + queries.each do |k, v| + k = "#{k}\255" + should_log_scrubbed_query(input: k, output: v) + end end end @@ -68,27 +56,23 @@ CryptKeeper::Provider::PostgresPgpPublicKey.new key: 'secret', public_key: public_key, private_key: private_key end - let(:input_query) do - "SELECT pgp_pub_encrypt('test', dearmor('#{public_key} - '))" - end - - let(:output_query) do - "SELECT encrypt([FILTERED])" - end + let(:queries) { + { + "SELECT pgp_pub_encrypt('test', dearmor('#{public_key}')) FROM DUAL;" => "SELECT pgp_pub_encrypt([FILTERED]) FROM DUAL;", + "SELECT pgp_pub_decrypt('test', dearmor('#{public_key}'), '#{private_key}') FROM DUAL;" => "SELECT pgp_pub_decrypt([FILTERED]) FROM DUAL;", + } + } it "filters pgp functions" do - should_log_scrubbed_query(input: input_query, output: output_query) + queries.each do |k, v| + should_log_scrubbed_query(input: k, output: v) + end end it "filters pgp functions in lowercase" do - should_log_scrubbed_query(input: input_query.downcase, output: output_query.downcase.gsub(/filtered/, 'FILTERED')) - end - - it "skips logging if CryptKeeper.silence_logs is set" do - CryptKeeper.silence_logs = true - - should_not_log_query(input_query) + queries.each do |k, v| + should_log_scrubbed_query(input: k.downcase, output: v.downcase.gsub(/filtered/, 'FILTERED')) + end end end end diff --git a/spec/crypt_keeper/provider/postgres_pgp_public_key_spec.rb b/spec/crypt_keeper/provider/postgres_pgp_public_key_spec.rb index 63f7313..f971e7e 100644 --- a/spec/crypt_keeper/provider/postgres_pgp_public_key_spec.rb +++ b/spec/crypt_keeper/provider/postgres_pgp_public_key_spec.rb @@ -47,6 +47,7 @@ describe "#decrypt" do specify { expect(subject.decrypt(cipher_text)).to eq(plain_text) } specify { expect(subject.decrypt(integer_cipher_text)).to eq(integer_plain_text.to_s) } + specify { expect(subject.decrypt(plain_text)).to eq(plain_text) } it "does not decrypt w/o private key" do pgp = described_class.new key: ENCRYPTION_PASSWORD, public_key: public_key diff --git a/spec/crypt_keeper/provider/postgres_pgp_spec.rb b/spec/crypt_keeper/provider/postgres_pgp_spec.rb index e763814..fc0cf33 100644 --- a/spec/crypt_keeper/provider/postgres_pgp_spec.rb +++ b/spec/crypt_keeper/provider/postgres_pgp_spec.rb @@ -44,6 +44,7 @@ describe "#decrypt" do specify { expect(subject.decrypt(cipher_text)).to eq(plain_text) } specify { expect(subject.decrypt(integer_cipher_text)).to eq(integer_plain_text.to_s) } + specify { expect(subject.decrypt(plain_text)).to eq(plain_text) } it "filters StatementInvalid errors" do begin diff --git a/spec/support/logging.rb b/spec/support/logging.rb index c4bb5c5..d9431d6 100644 --- a/spec/support/logging.rb +++ b/spec/support/logging.rb @@ -47,11 +47,8 @@ def debug(message) def should_log_scrubbed_query(input:, output:) queries = sql(input) - valid_input = queries.none? { |line| line.include? input } - expect(valid_input).to eq(true), "found unscrubbed SQL query logged!" - - valid_output = queries.any? { |line| line.include? output } - expect(valid_output).to eq(true), "output query was not logged!" + expect(queries.join("\n")).to_not include(input) + expect(queries.join("\n")).to include(output) end # Public: Verifies that the given input query was not logged.