[WIP] Use MultiFernet for encryption #123

rpicard · 2015-10-12T20:08:13Z

Facilitates key rotation and uses more secure encryption than what
sqlalchemy-utils does.

Fixes #117 and #119.

This is still a work in progress. I'm having some trouble testing locally. When I try to create new certificates, authorities, roles, etc. I end getting "not created" errors for what seem to be unrelated reasons. I'm hoping you can take a look and see how it works yourself.

TODO:

Migration script or something to deal with legacy users who have different keys that won't work with Fernet.
Should we pluralize the encryption key configuration key now that we support more than one? Maybe we could have a separate configuration key called "LEGACY_KEYS" or something to make it clear which is actively being used for encryption and which are just there for rotation.
Should we throw an exception or something when keys are not Fernet compatible?

rpicard · 2015-10-12T20:09:23Z

@kevgliss If you wouldn't mind taking a look when you have a minute. I have a feeling at least part of the errors I'm seeing have to do with my local environment and not necessarily the changes I've made.

kevgliss · 2015-10-12T22:07:49Z

setup.py

@@ -32,7 +32,6 @@
    'Flask-Bcrypt==0.6.2',
    'Flask-Principal==0.4.0',
    'Flask-Mail==0.9.1',
-    'SQLAlchemy-Utils==0.30.11',


It looks like I also use the JSON type from SQLAlchemy-Utils so we should leave it in for the moment.

kevgliss · 2015-10-12T22:12:57Z

I think this should be okay. I would just add back in SQLAlchemy and see if the tests pass. You could also post a stack trace to see if we can narrow down the errors you are seeing locally.

rpicard · 2015-10-12T23:52:41Z

I still can't seem to save anything locally. Here is the content of lemur.log after trying to save a new issuer.

[removed]

kevgliss · 2015-10-13T00:17:15Z

You will need to make an authority before you can issue any certificates.

2015-10-12 16:27:28,996 ERROR: 400: Bad Request [in /Users/robert.picard/Code/rpicard/lemur/lemur/common/utils.py:60]
Traceback (most recent call last):
  File "/Users/robert.picard/Code/rpicard/lemur/lemur/common/utils.py", line 46, in wrapper
    resp = f(*args, **kwargs)
  File "/Users/robert.picard/Code/rpicard/lemur/lemur/authorities/views.py", line 302, in put
    args = self.reqparse.parse_args()
  File "/Users/robert.picard/.virtualenvs/lemur/lib/python2.7/site-packages/flask_restful/reqparse.py", line 295, in parse_args
    value, found = arg.parse(req, self.bundle_errors)
  File "/Users/robert.picard/.virtualenvs/lemur/lib/python2.7/site-packages/flask_restful/reqparse.py", line 215, in parse
    self.handle_validation_error(ValueError(error_msg), bundle_errors)
  File "/Users/robert.picard/.virtualenvs/lemur/lib/python2.7/site-packages/flask_restful/reqparse.py", line 144, in handle_validation_error
    flask_restful.abort(400, message=msg)
  File "/Users/robert.picard/.virtualenvs/lemur/lib/python2.7/site-packages/flask_restful/__init__.py", line 30, in abort
    original_flask_abort(http_status_code)
  File "/Users/robert.picard/.virtualenvs/lemur/lib/python2.7/site-packages/werkzeug/exceptions.py", line 605, in __call__
    raise self.mapping[code](*args, **kwargs)

Should return a message complaining what might be missing from the request. Which authority are you trying to use? The only one currently packaged with Lemur is the verisign plugin. Thus you would only be able to issue certificates if you had a verisign account.

Are you doing this from the UI or the API? In either case ff you could give me the JSON you are sending to the API, I can probably spot whats missing.

rpicard · 2015-10-13T00:28:27Z

Here's what I'm sending:

{
    "caDN": {
        "commonName": "something",
        "country": "US",
        "location": "Los Gatos",
        "organization": "Netflix",
        "organizationalUnit": "Operations",
        "state": "CA"
    },
    "caDescription": "something",
    "caName": "InternalCA",
    "caSensitivity": "medium",
    "caSigningAlgo": "sha256WithRSA",
    "caType": "root",
    "extensions": {
        "subAltNames": {
            "names": []
        }
    },
    "keyType": "RSA2048",
    "ownerEmail": "ca@ca",
    "pluginName": "verisign-issuer",
    "subAltType": null,
    "subAltValue": null,
    "validityEnd": "2015-10-22T07:00:00.000Z",
    "validityStart": "2015-10-02T07:00:00.000Z"
}

kevgliss · 2015-10-13T00:44:59Z

Can you try with California instead of CA for state? I thought I changed all the defaults but may have missed it.

rpicard · 2015-10-13T00:53:14Z

Tried with the same result.

kevgliss · 2015-10-13T16:05:07Z

Which branch/commit are you working from? When you make the request there should be a more descriptive message in the body of the response.

Also I noticed in your stack trace you are doing a PUT request.

  File "/Users/robert.picard/Code/rpicard/lemur/lemur/authorities/views.py", line 302, in put

The JSON you are sending I would expect to be handled by a POST.

See: https://github.com/Netflix/lemur/blob/master/lemur/authorities/views.py#L302 for the variables allowed in a PUT request (editing an authority).

kevgliss · 2015-10-13T17:02:57Z

I just tested this out and it looks good. Everything encrypts and decrypts like I would expect. I think throwing an exception for improperly formatted keys makes sense. After that is added I will go ahead and merge this in. I will put the migration script in a separate pull request. Thanks!

rpicard · 2015-10-13T19:43:16Z

Thanks a lot. I'm not sure what my issue locally is. I'm doing things through the UI, so I would expect it to POST based on what you said.

In any case, I'll add in that exception and finish this up.

rpicard · 2015-10-13T23:44:09Z

Fernet will actually throw an exception if the key passed to it is not valid. We could add some checks at startup or something to make it easier to debug, but I think as long as the requirement is documented we'll be alright. The exception thrown by Fernet is pretty clear too.

rpicard · 2015-10-13T23:51:01Z

Maybe we should have a comment in the generated configuration file to document the requirement. I'll add it to the docs too.

Facilitates key rotation and uses more secure encryption than what sqlalchemy-utils does. Fixes Netflix#117 and Netflix#119.

rpicard · 2015-10-14T21:51:27Z

Current status: All should be ready to go pending a decision on how best to handle the key requirements. As of now, I've documented the requirement on the administration page.

As it stands, the application will still start with a bad key, but Fernet will throw an exception when it tries to encrypt something. Is there a better channel to communicate errors with users?

kevgliss · 2015-10-14T22:50:24Z

I think that should be enough - we generate good defaults and point out the requirements for those who wander off the beaten path. I am going to merge this in, thanks for the all the work. Feel free to hit me up on gitter if you are still struggling with creating authorities, or open an issue and I can see if I can tease out any more clues.

Use MultiFernet for encryption

rpicard · 2015-10-14T22:50:51Z

Awesome. Thanks for the help in putting this together.

Updated the submodule

kevgliss reviewed Oct 12, 2015
View reviewed changes

rpicard force-pushed the master branch 4 times, most recently from 3d76445 to 927d52d Compare October 12, 2015 23:27

kevgliss mentioned this pull request Oct 13, 2015

Create a migration script for new fernet encrypted keys. #124

Closed

rpicard force-pushed the master branch from 927d52d to 0c83eee Compare October 13, 2015 23:44

Use MultiFernet for encryption

40eb950

Facilitates key rotation and uses more secure encryption than what sqlalchemy-utils does. Fixes Netflix#117 and Netflix#119.

rpicard force-pushed the master branch from 0c83eee to 40eb950 Compare October 13, 2015 23:59

kevgliss added a commit that referenced this pull request Oct 14, 2015

Merge pull request #123 from rpicard/master

394e18f

Use MultiFernet for encryption

kevgliss merged commit 394e18f into Netflix:master Oct 14, 2015

kevgliss mentioned this pull request Oct 15, 2015

Key rotation #119

Closed

kevgliss mentioned this pull request Oct 23, 2015

[WIP] Adding aes - fernet migration #128

Merged

nezdolik pushed a commit to spotify/lemur that referenced this pull request Mar 23, 2023

Issue Netflix#123 - updated the submodule (Netflix#135)

39789f3

Updated the submodule

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Use MultiFernet for encryption #123

[WIP] Use MultiFernet for encryption #123

rpicard commented Oct 12, 2015

rpicard commented Oct 12, 2015

kevgliss Oct 12, 2015

rpicard Oct 12, 2015

kevgliss commented Oct 12, 2015

rpicard commented Oct 12, 2015

kevgliss commented Oct 13, 2015

rpicard commented Oct 13, 2015

kevgliss commented Oct 13, 2015

rpicard commented Oct 13, 2015

kevgliss commented Oct 13, 2015

kevgliss commented Oct 13, 2015

rpicard commented Oct 13, 2015

rpicard commented Oct 13, 2015

rpicard commented Oct 13, 2015

rpicard commented Oct 14, 2015

kevgliss commented Oct 14, 2015

rpicard commented Oct 14, 2015

[WIP] Use MultiFernet for encryption #123

[WIP] Use MultiFernet for encryption #123

Conversation

rpicard commented Oct 12, 2015

rpicard commented Oct 12, 2015

kevgliss Oct 12, 2015

Choose a reason for hiding this comment

rpicard Oct 12, 2015

Choose a reason for hiding this comment

kevgliss commented Oct 12, 2015

rpicard commented Oct 12, 2015

kevgliss commented Oct 13, 2015

rpicard commented Oct 13, 2015

kevgliss commented Oct 13, 2015

rpicard commented Oct 13, 2015

kevgliss commented Oct 13, 2015

kevgliss commented Oct 13, 2015

rpicard commented Oct 13, 2015

rpicard commented Oct 13, 2015

rpicard commented Oct 13, 2015

rpicard commented Oct 14, 2015

kevgliss commented Oct 14, 2015

rpicard commented Oct 14, 2015