symmetric-encryption icon indicating copy to clipboard operation
symmetric-encryption copied to clipboard

Decrypted values included in Rails JSON

Open kylefox opened this issue 5 years ago • 2 comments

I'm not sure if this is something symmetric-encryption can or should address, but I wanted to bring it up because of the potential security implications for Rails applications.

By default, #as_json includes encrypted attributes:

class Person < ApplicationRecord
  attribute :ssn, :encrypted
end

Person.create(ssn: 'top_secret').as_json
# => { id: 1, ssn: 'top_secret' }

I can't think of many scenarios where you'd want sensitive data included in the JSON representation. But I can imagine scenarios where it's inadvertently leaked, for example:

render json: @person

It's simple enough to mitigate this issue by overriding as_json to exclude the sensitive attributes:

class Person < ApplicationRecord
  attribute :ssn, :encrypted

  def as_json(*)
    super(except: [:ssn])
  end
end

It would be ideal if the Rails 5+ Attributes API made it possible for attributes to exclude themselves from JSON, but unfortunately that doesn't seem possible. 😕Maybe there's another way to do this using functionality built into Rails.

What do you think about adding a new module (concern) that, when included, automatically excluded encrypted attributes from as_json? For example, something like SymmetricEncryption::RestrictedAttributes?

I think at the very least this should all be mentioned in the Frameworks Guide. I'd be happy to put together a pull request if there's agreement.

kylefox avatar Jun 02 '19 00:06 kylefox

I'm just a dude on the street, but I anticipated this problem and am intentionally NOT using the ActiveRecord integration of SymmetricEncryption for exactly this reason. Automatic decryption of sensitive data isn't acceptable, at least for me. Example, you've overridden as_json, but what about https://api.rubyonrails.org/classes/ActiveRecord/AttributeMethods.html#method-i-attribute_for_inspect ? Rails.logger.info(person) will still log the plaintext value unless you mess with attribute_for_inspect.

I decided that playing whack-a-mole with ActiveRecord is definitely NOT how I want to secure my secrets and am instead requiring my team to manually decrypt the secret values when and where they need them. This is kind of like putting the dangerous acids on the top shelf where you need to intentionally get a step-stool to reach them, instead of just leaving them on the bench.

hovissimo avatar Aug 20 '20 19:08 hovissimo

The apparent lack of concern about a potential security flaw is itself worrying 😰

I'd be happy to contribute to some kind of fix, but I was hoping my original post would spark some discussion around what kind of solution could be agreed upon and ultimately merged...

kylefox avatar Aug 20 '20 19:08 kylefox