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

Decrypted values included in Rails JSON #128

Open
kylefox opened this issue Jun 2, 2019 · 2 comments
Open

Decrypted values included in Rails JSON #128

kylefox opened this issue Jun 2, 2019 · 2 comments

Comments

@kylefox
Copy link
Contributor

kylefox commented Jun 2, 2019

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.

@hovissimo
Copy link

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.

@kylefox
Copy link
Contributor Author

kylefox commented Aug 20, 2020

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...

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