Netflix / lemur

Repository for the Lemur Certificate Manager
Apache License 2.0
1.72k stars 323 forks source link

Certificates encrypted with static IV per key #117

Closed rpicard closed 8 years ago

rpicard commented 8 years ago

The IV is static per key at least. Lemur is using sqlalchemy_utils to encrypt certificates. This in turn encrypts with AES in CBC mode.

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.

rpicard commented 8 years ago

Also reported upstream: https://github.com/kvesteri/sqlalchemy-utils/issues/166

rpicard commented 8 years ago

I think a reasonable fix would be either using the FernetEngine with sqlalchemy-utils or removing the middle-man and using the cryptography package directly. Using Fernet, with or without sqlalchemy-utils, also has the benefit of adding an HMAC.

kevgliss commented 8 years ago

Makes sense, is simply passing engine=fernet to EncryptedType enough? Will need to think about migrating existing key pairs to the new encryption scheme.

Ditching sqlalchemy_utils may be a good idea, I have to see if I use it for anything else. Do you want to take a stab at creating a TypeDecorator?

If we do end up re-writing it, MultiFernet looks like it might make a nice addition, especially if someone has a requirement to rotate keys periodically.

rpicard commented 8 years ago

Yeah, I'll work on making a MultiFernet TypeDecorator over the next couple of days.

rpicard commented 8 years ago

We have a few options for taking the user's key and passing it to Fernet:

  1. Require that the user's keys are 32 bytes, base64 encoded. In the documentation we can recommend doing Fernet.generate_key() or something similarly secure. We could even add a command to the CLI to insert a random key into the config.
  2. Run the user's key through HKDF to expand it to the right size.
  3. Run the user's key through PBKDF to expand it to the right size, plus add some protection against brute forcing when they have a password instead of a securely random and long key. This would require a salt, which brings up the question of where to store it. We could either store it in the database somewhere, have the user put it in their configuration, or use a static salt. This would prevent rainbow tables from helping with the brute forcing.

I prefer the first option, but it does sacrifice some convenience, so I don't want to make assumptions about your priorities.

@kevgliss Do you have any thoughts?

kevgliss commented 8 years ago

@rpicard I prefer the first option as well, this this is the approach I take when generating configuration files for the user.

https://github.com/Netflix/lemur/blob/master/lemur/manage.py#L167

And also for the "lock" and "unlock" functions that is also using fernet.

https://github.com/Netflix/lemur/blob/master/lemur/manage.py#L419

rpicard commented 8 years ago

Perfect. :100:

rpicard commented 8 years ago

Is this try / except to catch the "working outside of application context" error? I don't fully understand what is causing that error. Is it just that there isn't a current_app when running something like lemur init?

https://github.com/Netflix/lemur/blob/master/lemur/utils.py#L19

kevgliss commented 8 years ago

Correct, it is an unfortunate hack, flask-script needs a flask app object. I use flask-script to run create_config that generates configurations for a user... because they don't have one yet, but the sqlalchemy_utils type-decorator needs something. This was my way of breaking that circular dependency.

Ideally flask-script wouldn't need current_app to function. I looked at http://click.pocoo.org/5/ which doesn't need an app object, but I wasn't sold on the decorator soup it tends to lean on.

rpicard commented 8 years ago

Okay, that explains the problem I had. Like you said, it's an unfortunate hack, but I can live with it.

rpicard commented 8 years ago

I've been running in Docker, but I'm trying to debug some things and to do so, I want to just run on my machine normally. I'm now getting this error when I run lemur init. The database is being created just fine.

https://gist.github.com/rpicard/db776eebc25742460eff

rpicard commented 8 years ago

My bad. I missed the lemur db init step.