EncryptedType uses static IV per key

[rpicard]

EncryptedType uses AES in CBC mode. The IV that it uses is not random though.

https://github.com/kvesteri/sqlalchemy-utils/blob/master/sqlalchemy_utils/types/encrypted.py#L56

Given a single key, it will use the SHA256 hash of that key for all encryption. It looks like it will use the first 16 bytes of that hash as the IV for each operation.

This link is a good primer on why this is bad: http://security.stackexchange.com/a/1097

[mehcode]

From what I understand we do it this way so that encrypt("b") == encrypt("b") to allow for queryability of encrypted values. There is the https://github.com/kvesteri/sqlalchemy-utils/blob/master/sqlalchemy_utils/types/encrypted.py#L94 which uses (from the documentation):

• AES in CBC mode with a 128-bit key for encryption; using PKCS7 padding.
• HMAC using SHA256 for authentication.
• Initialization vectors are generated using os.urandom().

---

If you're suggesting that the Fernet engine be the default I'd honestly be fine with that as long as we push it out as a minor version bump.

[rpicard]

Making the Fernet engine the default sounds good to me. Getting the HMAC for free is a win too.

[rpicard]

With that said, if AesEngine is kept around it should still be fixed.

[mehcode]

With that said, if AesEngine is kept around it should still be fixed.

Except it's done that way to allow querying. Depending on the why of your encryption it may be fine.

[rpicard]

Using a static IV per key compromises security. If users are willing to accept that to gain the convenience it provides, then that's up to them. If you want to facilitate that it's up to you. If that's your decision, my opinion is that the documentation should make it clear beyond any question that using those features reduces the security of the system.

That's my point of view. Remediate this as you see fit. 👍

[mehcode]

I'm going to recommend (and then probably not get time to do it for a while so if anyone wants to jump in it'd be appreciated):

• Remove user-configured engine selection (we only have two and they are both AES) and default to an internal selection of the Fernet engine
• Add a property (named along the lines of reproducible or queryable) that switches to the as-standing AesEngine
• Add a warning block in the documentation that explains what the security risk is.

I agree that leaking a database encrypted with static IVs would allow a line of reasoning about the encrypted data that otherwise wouldn't exist (for instance, if we were talking about salaries of employees you could see that 2 people make the same amount). But I don't agree that its broken. Security is a matter of degree. Normally only data that should be encrypted gets encrypted. However if your client instructs you to encrypt the world, no matter what, then allowing some data to be queryable is a thing that would need to happen.

[konstantinoskostis]

@mehcode , @rpicard hello and i am very sorry i am responding so late. First i would like to say that all of your above comments are absolutely correct. To be honest when i started the EncryptedType i wanted to give users the possibility to also query values in the DB and that is why the IV is not randomly generated. It was just a decision, maybe a wrong one... When i have the time i will try to make the changes proposed by @mehcode in the above comment.

[MichaelCG8]

So it's been a little over 5 years since this ticket was opened, so here's a summary of updates that I've noticed when looking at this.

This ticket was opened when the latest release was 0.31.0, and we're now on 0.36.8.

The file https://github.com/kvesteri/sqlalchemy-utils/blob/0.31.0/sqlalchemy_utils/types/encrypted.py has been replaced by the directory https://github.com/kvesteri/sqlalchemy-utils/tree/master/sqlalchemy_utils/types/encrypted/

The classes AesEngine and FernetEngine still exist, and there is a new AesGcmEngine.

AesEngine and AesGcmEngine have docstrings explaining that the former allows searching by value of an encrypted column, but is less secure than the latter, which does not allow this searching. The Fernet docstring does not mention that it does not enable this searching.

In the SQLAlchemy-Utils docs, the only mention of encryption is in https://sqlalchemy-utils.readthedocs.io/en/latest/data_types.html#module-sqlalchemy_utils.types.encrypted.encrypted_type which does not have a docstring so the user will need to look at the source code without prompting to see the docstrings of the engines and to see that AesEngine is the default. It used to have a docstring but it now inherits from StringEncryptedType and the docstring has been moved there. StringEncryptedType does not appear in the documentation apart from in the source code. It looks like the docstring wasn't updated though when it was moved from StringEncryptedType to EncryptedType, so it is still using EncryptedType in it's example code rather than StringEncryptedType.

The changes proposed by @mehcode still look appropriate to me, with the addition that the documentation should make clear that EncryptedType inherits from StringEncryptedType and the docstring of StringEncryptedType should be available in the documentation with some notes about the engines.

Neither databases or encryption are my domain, but this seems fairly simple so I'll have a go at it this weekend.

Is anyone aware of any part of https://github.com/sqlalchemy/sqlalchemy or related projects that could use documentation updates based on this?

[MichaelCG8]

Pull request opened. #499
I ended up keeping the engine parameter so that when queryable is False the user has the choice of AesGcmEngine and FernetEngine with the latter being the default. If engine is specified it must be compatible with the value of queryable.

[tucked]

Looks like the move was called out above, but here are permalinks...
The original link:

sqlalchemy-utils/sqlalchemy_utils/types/encrypted.py

Line 56 in 4a9b7c7

self.iv = self.secret_key[:16]

The current location of the same code:

sqlalchemy-utils/sqlalchemy_utils/types/encrypted/encrypted_type.py

Line 82 in 5c4ca8c

self.iv = self.secret_key[:16]

pipenv check trips on this: https://pyup.io/v/42194/742/

Anyone know of a suitable replacement until this is fixed?