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

#encrypted? returns true for a plain text string #63

Closed
jschorr opened this issue Sep 29, 2016 · 4 comments
Closed

#encrypted? returns true for a plain text string #63

jschorr opened this issue Sep 29, 2016 · 4 comments

Comments

@jschorr
Copy link

jschorr commented Sep 29, 2016

For some reason, using the 'encrypted?' method against the following string returns true when it shouldn't. I've tried with numerous other strings and can't get the above behavior. Also, with the same exact key and iv, I can encrypt and decrypt just fine:

`irb(main):001:0> require_relative("boot")
=> true

irb(main):002:0> str = "LIDIA MARGARITA MARTINEZ"
=> "LIDIA MARGARITA MARTINEZ"

irb(main):003:0> enc = SymmetricEncryption.encrypt(str)
=> "UPWRrAPEY+ngnsdZP0C3M6tJlwfnf6Lu3V7DDXjXLm4="

irb(main):005:0> dec = SymmetricEncryption.decrypt(enc)
=> "LIDIA MARGARITA MARTINEZ"

irb(main):006:0> dec == str
=> true

Here is where the issue occurs:

irb(main):007:0> SymmetricEncryption.encrypted?(str)
=> true
`

@jschorr
Copy link
Author

jschorr commented Sep 29, 2016

Closing this issue as regenerating the keys fixed it. Wondering if maybe the recent openssl updates affected this (wouldn't be surprised).

@jschorr jschorr closed this as completed Sep 29, 2016
@jschorr jschorr reopened this Oct 5, 2016
@jschorr
Copy link
Author

jschorr commented Oct 5, 2016

This happened again today with a different string, so I'm not going to rely upon this method for now.

irb(main):014:0> require_relative("boot")
=> true
irb(main):015:0> SymmetricEncryption.encrypted?("61 JONESVILE AIRPORT ROAD")
=> true

this is in development and my config/symmetric-encryption.rb file looks like:

development:   &development_defaults
# Since the key to encrypt and decrypt with must NOT be stored along with the
# source code, we only hold a RSA key that is used to unlock the file
# containing the actual symmetric encryption key
private_rsa_key: |
    -----BEGIN RSA PRIVATE KEY-----
    REMOVED FOR SECURITY
    -----END RSA PRIVATE KEY-----

  # List Symmetric Key files in the order of current / latest first
  ciphers:
    -
      # Filename containing Symmetric Encryption Key encrypted using the
      # RSA public key derived from the private key above
      key_filename:      /etc/app-keys/keynamehere-development.key
      iv_filename:       /etc/app-keys/keynamehere-development.iv
      cipher_name:       aes-256-cbc
      encoding:          :base64strict
      version:           1
      always_add_header: true

@reidmorrison
Copy link
Owner

Correct, using this method with random data will result in false positives. The method attempts to decrypt the data and since it is valid base64 it will decode, and then it attempts to decrypt it with OpenSSL. In a small amount of cases it will decrypt without OpenSSL raising an exception.

Since you have always_add_header: true consider looking for the header instead of using encrypted?.

In this case where all encryption keys have always_add_header: true it would be better for the encrypted? method to check for the header rather than even attempting to decrypt the data.

I have the following method locally that I started but never committed to extract the header:

module SymmetricEncryption
  # Returns the header for the encrypted string
  # Returns [nil] if no header is present
  def self.header(encrypted_and_encoded_string)
    raise(SymmetricEncryption::ConfigError, 'Call SymmetricEncryption.load! or SymmetricEncryption.cipher= prior to encrypting or decrypting data') unless @@cipher
    return if encrypted_and_encoded_string.nil? || (encrypted_and_encoded_string == '')

    # Decode before decrypting supplied string
    decoded = cipher.encoder.decode(encrypted_and_encoded_string.to_s)
    return if decoded.nil? || decoded.empty?

    Cipher.parse_header!(decoded)
  end
end

If you want to give a try and give me some feedback I will be happy to publish it in a new gem. The part that is not finished is what should be returned from this method.

@jschorr
Copy link
Author

jschorr commented Oct 7, 2016

It works fine if I adjust the "decoded" line to Base64.decode64(encrypted_and_encoded_string.to_s). Otherwise, I get the following error:

irb(main):017:0> SymmetricEncryption.header("QEVuQwEAtk4oTfFfkcFZkE19U2BEHw==") NoMethodError: undefined methodencoder' for #SymmetricEncryption::Cipher:0x0055cea2d52db8`

No need for a new gem, I'll just use this way of doing it to check if a string is encrypted. Thanks.

@jschorr jschorr closed this as completed Oct 7, 2016
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

No branches or pull requests

2 participants