search

jmazzi / crypt_keeper

Transparent ActiveRecord encryption
http://jmazzi.github.com/crypt_keeper/
MIT License
288 stars 98 forks source link

Returned records are always dirty - issues with plaintext accessors and database fields being named the same #29

Closed gabetax closed 11 years ago

gabetax commented 11 years ago

I've found a couple issues related to both the plaintext accessor methods and the encrypted database field names being the same, and being converted with callbacks:

  1. The record is always inherently dirty.
  2. Because the record is always dirty, the encrypted attributes get updated on every save, regardless of whether they changed.

An example of where this may cause an issue: if I'm trying to use an auditing library like paper_trail, it's recording the attributes as part of a callback. Because crypt_keeper users a before_save callback to encrypt the fields, it's possible that some libraries will grab the plain-text attribute values.

I find attr_encrypted's approach much less problematic for this case. The database fields are named encrypted_foo, and then it creates accessors foo and foo=. The added benefit is that the attributes are lazy-loaded, which gives a persistent performance benefit. You also don't need to worry about any other callbacks or Observers. Additionally, direct manipulation of an encrypted field, e.g. User.update_column(:encrypted_ssn, '"\\xc30d040..") is more explicitly communicative.

This may be too sweeping of a suggested change for a library that's emphasizing not being too complicated. These aspects may be better as a documentation change, and I could try adapting your pgcrypto() code into a custom attr_encrypted-compatible Encryptor object.

jmazzi commented 11 years ago

Yeah, I'm aware of the dirty issue. I've been kicking around a few ideas, just not sure which avenue I want to take yet.

jmazzi commented 11 years ago

@gabetax can you give the feature/dirty_tracking branch a try?

jmazzi commented 11 years ago

@gabetax did you try out this branch?

gabetax commented 11 years ago

I actually went ahead extracted the pgcrypto portion of cryptkeeper into an Encryptor module for use with attr_encrypted shortly after I encountered this issue, which solved my immediate problem: https://github.com/gabetax/attr_encrypted_pgcrypto

I can try swapping my app back to use crypt_keeper to check the paper_trail compatibility if you'd like.

jmazzi commented 11 years ago

Ah, cool! Nope, just close it out. Its working already.

Sent from my phone On Apr 11, 2013 5:42 PM, "Gabe Martin-Dempesy" notifications@github.com wrote:

I actually went ahead extracted the pgcrypto portion of cryptkeeper into an Encryptor module for use with attr_encrypted shortly after I encountered this issue, which solved my immediate problem: https://github.com/gabetax/attr_encrypted_pgcrypto

I can try swapping my app back to use crypt_keeper to check the paper_trail compatibility if you'd like.

— Reply to this email directly or view it on GitHubhttps://github.com/jmazzi/crypt_keeper/issues/29#issuecomment-16263499 .