OCA / server-tools

Tools for Odoo Administrators to improve some technical features on Odoo.
https://odoo-community.org/psc-teams/tools-30
GNU Affero General Public License v3.0
677 stars 1.47k forks source link

RFC: Encrypted Fields Module #471

Closed lasley closed 7 years ago

lasley commented 8 years ago

In some secure contexts, at-rest encryption is required at the database layer. My specific context being Protected Health Information covered under HIPAA. Some PCI data (Cardholder name, etc) is also required to be encrypted at rest.

I made a crude module a while back in order to meet this need - https://github.com/laslabs/odoo-base/blob/9.0/fields_encrypted/fields.py

It has worked great in some long term tests so we are about to begin the build out of the real module. Assuming this is something that OCA wants, I will bring it here instead.

I'm probably forgetting some details, but from a high level this is what I am thinking. Please let me know if you have any suggestions or if there is some overlap with pre-existing resources.

Currently the module:

The plan:

Pitfalls:

hbrunn commented 8 years ago

I'm toying with something like that too for a while: https://github.com/hbrunn/server-tools/tree/8.0-base_encrypted_field/base_encrypted_field My approach is to have a group of records share a symmetric encryption key that is encrypted with a public key generated for every user. Also the architecture tries hard to have all crypto being done on the client side so that the key never leaves the client's computer.

lasley commented 8 years ago

I like what you have here, particularly the client-side aspect & PGP vs AES. My implementation's security depends on separation of server infrastructure, and still has a weak point of exploitation from the Odoo filesystem with an attack surface as wide as the application's keys.

Am I correct in reading that this is storing the public and private keys in the Odoo database? I am trying to ward against an attack at the database layer, so anything stored in that becomes a weak point in the event of a DB dump.

I know it would slow encryption down considerably, but this could maybe be circumvented by using a filesystem attachment field for the private key. This would still have an open attack vector at the application & filesystem layer, but would at least close the database hole.

Where were you at with this? Anything you remember that may be helpful as I tinker with it?

elicoidal commented 8 years ago

This feature would be a great addition clearly. My main problem today to install HR/payroll in customers is about the confidentiality of the data (contract/payslips at least) with the IT staff, which could be solved with this!

kdhagemann commented 8 years ago

+1 definetly!

This would also make a big step towards SOX / J-SOX compliance.

@Eric: This would mean, that the key to decrypt ie the HR/payroll data needs to be shared across a certain authorized group of users, right?

--> generically spoken: Encrypting different data entities with different keys where the matching public key is distributed to specific users/user-groups (ideally client-side decryption with a key stored on a token like usb-stick etc)...

Klaus Hagemann OpenSource IT Systems (mobile)

Am 02.07.2016 um 02:53 schrieb Eric Caudal notifications@github.com:

This feature would be a great addition clearly. My main problem today to install HR/payroll in customers is about the confidentiality of the data (contract/payslips at least) with the IT staff, which could be solved with this!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

hbrunn commented 8 years ago

the public and private keys are stored in the database, but the private key is encrypted with a passphrase chosen by the user, and this is supposed to never leave the browser: https://github.com/hbrunn/server-tools/blob/8.0-base_encrypted_field/base_encrypted_field/static/src/js/base_encrypted_field.js#L64, https://github.com/hbrunn/server-tools/blob/8.0-base_encrypted_field/base_encrypted_field/wizards/base_encrypted_field_get_passphrase.py

Next step would be to encrypt groups of fields with a symmetric key which is encrypted with all users' public keys that are supposed to have access. This is roughly what GPG does for multiple mail recipients, and has the advantage that you don't need to touch the encrypted data for adding or removing users.

I've roughly this data structure in mind:

class EncryptedField(models.Model):
    _name = 'encrypted.field'
    field_id = fields.Many2one('ir.model.fields')
    user_ids = fields.One2many('encrypted.field.user', 'encrypted_field_id')
    record_ids = fields.One2many('encrypted.field.record')

class EncryptedFieldUser(models.Model):
    _name = 'encrypted.field.user'
    user_id = fields.Many2one('res.users')
    encrypted_field_id = fields.Many2one('encrypted.field')
    key = fields.Text('TripleSec key encrypted by users public GPG key') # https://keybase.io/triplesec

class EncryptedFieldRecord(models.Model):
    _name  = 'encrypted.field.record'
    encrypted_field_id = fields.Many2one('encrypted.field')
    model_id = fields.Many2one('ir.model.model')
    res_id = fields.Integer()

So determining who has access to some field is just a join over two tables, giving a user access is a question of adding one record (this needs to be done by a user who already has access because we need to have access to the symmetric key), retracting access is a matter of deleting the same record.

This is still not secure against an evil admin (who could inject JS code to record the passphrase), but the only way to make this tamper proof would be a local application, and with this, we get a whole class of different problems.

hbrunn commented 8 years ago

I finished my proof of concept: https://github.com/OCA/server-tools/pull/472

mtelahun commented 8 years ago

As in all things crypto it can be very dangerous to roll your own mechanism. In addition to the actual encryption mechanism you also have to think about key management, storage, transmission and revocation. It is incredibly easy to get it inadvertently wrong. It might be better to build a solution based on Google's keyczar than to try to roll our own. It's a proven solution and already in use by at least one other project (Django). Admittedly, it has it's own problems (like secure storage of keys), but I think solving those would be much better than trying to create yet another crypto mechanism.

hbrunn commented 8 years ago

Very important point. The choice is a bit restricted in this case because (at least for what i have in mind) we need a cross browser implementation of all crypto involved. I was not aware of https://github.com/mitro-co/keyczarjs, this might be an option too.

lasley commented 8 years ago

Looks like the KeyCzarJS library only implements asymmetric though, so that would limit us quite a bit in terms of what we can do with it.

Technically the encryption could take place server side, but then we are exposing all necessary aspects for decryption at a focal point outside of client control. In the US this means that the service provider is responsible for maintaining record of that data in the event of a government warrant, which is a no-go for one of my intended applications.

OpenPgpJs seems to implement every aspect of what we need & includes security audit reports, so I'm not sure what would be considered a roll your own in the PoC. The only crypto implemented outside of the library is the group permission and key handling, which would need to be rolled ourselves no matter the framework from what I can tell.

mtelahun commented 8 years ago

@lasley a crypto system is much more than just encrypting and decrypting stuff. For example, what happens if one of the user's keys is compromised? How does the system handle the revocation of the key? The system has to be able to securely create, manage, transmit, revoke, etc keys.

The browser, as it currently stands, is a BAD place to do encryption (except under some circumstances that don't apply in our case). You cannot trust the browser. @hbrunn already mentioned the biggest drawback: you can't trust the javascript code running in your browser is the same code the app developer wants you to have. The js can be compromised in so many different ways (for example: server compromise or MITM attack) and you as the end-user would never know. The app developer would never know. The only way this can be solved is by using some sort of digitally signed js loading mechanism.

The other problem with doing in-browser encryption in Odoo's case is that it will only work for browser clients. All other clients (erppeek, oerpscenario, etc) will have to implement their own crypto mechanism if they want to access encrypted fields, which will be another security nightmare all on its own.

The proper place to do encryption is on the server, transparently. You control who has access to a field using the usual record rules. To improve security you use a separate daemon for creating keys, encryption, and decryption. I have thought about linking keyczar and python-keepassx in this way but never had the time to pursue it.

As an example, this is the way oracle handles database encryption. The database administrator and security administrator roles are separate. A separate security daemon handles key management and encryption/decryption. Each encrypted column has its own encryption key which is stored encrypted by a master encryption key. The master encryption key is unlocked by a password known only to the security manager. The master encryption key needs to be unlocked every time the database is started unless you enable auto-login for the security administrator in the security daemon. How is this secure?

hbrunn commented 8 years ago

which would be the circumstances that don't apply in our case? My plan is to give the user (or a group of users) control over their data. I don't see how this can be done anywhere else than in the browser (or a browser plugin if integrity of the transmitted code is an issue). If this is compromised, you're fucked in a lot of ways already, access to the crypto keys is a smaller problem you have then to my understanding. In case that's not obvious: Your scheme keeps keys with the company in question. In times of NSLs, every key a company has (legal) access to must be seen as compromised in the first place, so this is absolutely no option for what I need. What you describe can be done with pgcrypto or standard disk encryption, no need for Odoo-specific stuff here. But that's only complementing, not replacing encryption controlled by users in my view. But yes, I'm by no means an expert on this topic, so I asked a security company for a quote for a security analysis of my PoC, if this doesn't turn out to be forbiddingly expensive, I'll start asking around in the community to fund an audit.

lasley commented 8 years ago

@hbrunn - I have funds on this, let me know.

Also agree with the need for client side encryption. Honestly the only downside I see here are the scaling concerns when data starts getting in the gigs - and maybe what happens if a user gets disconnected in the middle on a re-encryption session (once that's implemented)

mtelahun commented 8 years ago

One of the use-cases I know about for in-browser encryption is encrypting credit-card details with the payment acquirer's public key before sending it over the wire (so even if SSL session is compromised, cc details would not be)

@hbrunn if your main concern is secure data even in the face of an NSL, then yes my described solution would not work for you. But I would submit that it is a limited use-case that would not apply in the majority of Odoo use-cases. I am talking about scenarios where you want to protect confidential data like credit card details, salaries, etc. where things like NSLs are irrelevant. The only thing I will add to this is that if an NSL issuing organization is interested in the encrypted data defeating browser based encryption would most likely be child's play for them (for example with a MITM attack). In this case you are better off with a custom (client-side) solution than browser based crypto.

Still, assuming you could solve the issue of securely loading js (or using a browser extension) there is still the problem of compatibility between browsers and between clients. You would have to develop plugins for most commonly used browsers or mandate the use of only one browser. You still would not be able to access the data from other tools. I think these are big disadvantages for a proposed generic encryption mechanism.

Obviously, you are the one who is doing the actual work so you get to decide how you are going to implement it. But I hope you consider the points I've made.

hbrunn commented 8 years ago

I think your input is valuable, just because it's so easy to mess up encryption (and it happens so often). So surely thanks for your points.

Markieta commented 8 years ago

Hi @lasley

I have been interested in encrypting fields for the past month and have come across your module. However I don't know how to use it!

This is what I currently have:

EncryptedText(string="Master Password", required=True)

In my view I have password="True" to hide the entry, but EncryptedText doesn't seem to support this.

How do you encrypt the field on Save and decrypt when reading from the database?

Could you please add better documentation and usage instructions to your repo? I would really appreciate it!

lasley commented 8 years ago

Hi @Markieta -

Thanks for your interest, but I would not recommend using either of the two modules presented here at this time.

Both are proofs of concept, are not known secure, and are guaranteed to wreck your data in a lot of cases.

We will update this ticket when there is more news regarding this feature.

elicoidal commented 7 years ago

@lasley should we be closing this PR for the reasons exposed in https://github.com/OCA/server-tools/issues/471#issuecomment-232454378

lasley commented 7 years ago

@elicoidal - I have disbanded the original plan & gone for an external approach in #673. I think this is still fine to close though, the interest has been gauged, issues exposed, and no more is happening in this ticket.

dreispt commented 4 years ago

FYI I created a proof of concept, based on the existing data_encryption module in OCA/server-env: https://github.com/OCA/server-env/pull/50