Merge "add encryption to secret datasource config fields"

congress/common/config.py

@@ -77,6 +77,8 @@ core_opts = [
                      'engines.'),
     cfg.StrOpt('policy_library_path', default='/etc/congress/library',
                help=_('The directory containing library policy files.')),
+    cfg.StrOpt('encryption_key_path', default='/etc/congress/keys',
+               help=_('The directory containing encryption keys.')),
     cfg.BoolOpt('distributed_architecture',
                 deprecated_for_removal=True,
                 deprecated_reason='distributed architecture is now the only '

congress/db/datasources.py

@@ -25,6 +25,7 @@ from sqlalchemy.orm import exc as db_exc
 from congress.db import api as db
 from congress.db import db_ds_table_data as table_data
 from congress.db import model_base
+from congress import encryption
 
 
 class Datasource(model_base.BASE, model_base.HasId):
@@ -46,8 +47,39 @@ class Datasource(model_base.BASE, model_base.HasId):
         self.enabled = enabled
 
 
+def _encrypt_secret_config_fields(ds_db_obj, secret_config_fields):
+    '''encrypt secret config fields'''
+    config = json.loads(ds_db_obj.config)
+    if config is None:  # nothing to encrypt
+        return ds_db_obj  # return original obj
+    if '__encrypted_fields' in config:
+        raise Exception('Attempting to encrypt already encrypted datasource '
+                        'DB object. This should not occer.')
+    for field in secret_config_fields:
+        config[field] = encryption.encrypt(config[field])
+    config['__encrypted_fields'] = secret_config_fields
+    ds_db_obj.config = json.dumps(config)
+    return ds_db_obj
+
+
+def _decrypt_secret_config_fields(ds_db_obj):
+    '''de-encrypt previously encrypted secret config fields'''
+    config = json.loads(ds_db_obj.config)
+    if config is None:
+        return ds_db_obj  # return original object
+    if '__encrypted_fields' not in config:  # not previously encrypted
+        return ds_db_obj  # return original object
+    else:
+        for field in config['__encrypted_fields']:
+            config[field] = encryption.decrypt(config[field])
+        del config['__encrypted_fields']
+        ds_db_obj.config = json.dumps(config)
+        return ds_db_obj
+
+
 def add_datasource(id_, name, driver, config, description,
-                   enabled, session=None):
+                   enabled, session=None, secret_config_fields=None):
+    secret_config_fields = secret_config_fields or []
     session = session or db.get_session()
     with session.begin(subtransactions=True):
         datasource = Datasource(
@@ -57,6 +89,7 @@ def add_datasource(id_, name, driver, config, description,
             config=config,
             description=description,
             enabled=enabled)
+        _encrypt_secret_config_fields(datasource, secret_config_fields)
         session.add(datasource)
     return datasource
 
@@ -93,9 +126,9 @@ def get_datasource(name_or_id, session=None):
 def get_datasource_by_id(id_, session=None):
     session = session or db.get_session()
     try:
-        return (session.query(Datasource).
-                filter(Datasource.id == id_).
-                one())
+        return _decrypt_secret_config_fields(session.query(Datasource).
+                                             filter(Datasource.id == id_).
+                                             one())
     except db_exc.NoResultFound:
         pass
 
@@ -103,14 +136,14 @@ def get_datasource_by_id(id_, session=None):
 def get_datasource_by_name(name, session=None):
     session = session or db.get_session()
     try:
-        return (session.query(Datasource).
-                filter(Datasource.name == name).
-                one())
+        return _decrypt_secret_config_fields(session.query(Datasource).
+                                             filter(Datasource.name == name).
+                                             one())
     except db_exc.NoResultFound:
         pass
 
 
 def get_datasources(session=None, deleted=False):
     session = session or db.get_session()
-    return (session.query(Datasource).
-            all())
+    return [_decrypt_secret_config_fields(ds_obj)
+            for ds_obj in session.query(Datasource).all()]

congress/dse2/datasource_manager.py

@@ -59,6 +59,7 @@ class DSManagerService(data_service.DataService):
         if update_db:
             LOG.debug("updating db")
             try:
+                driver_info = self.node.get_driver_info(req['driver'])
                 # Note(thread-safety): blocking call
                 datasource = datasources_db.add_datasource(
                     id_=req['id'],
@@ -66,7 +67,8 @@ class DSManagerService(data_service.DataService):
                     driver=req['driver'],
                     config=req['config'],
                     description=req['description'],
-                    enabled=req['enabled'])
+                    enabled=req['enabled'],
+                    secret_config_fields=driver_info.get('secret', []))
             except db_exc.DBDuplicateEntry:
                 raise exception.DatasourceNameInUse(value=req['name'])
             except db_exc.DBError:

congress/encryption.py

@@ -0,0 +1,97 @@
+# Copyright (c) 2017 VMware, Inc. All rights reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Encryption module for handling passwords in Congress."""
+from __future__ import print_function
+from __future__ import division
+from __future__ import absolute_import
+
+import io
+import os
+
+from cryptography import fernet
+from cryptography.fernet import Fernet
+from oslo_config import cfg
+from oslo_log import log as logging
+
+LOG = logging.getLogger(__name__)
+
+__key = None
+__fernet = None
+
+
+def _get_key_file_path():
+    return os.path.join(cfg.CONF.encryption_key_path, 'aes_key')
+
+
+def key_file_exists():
+    return os.path.isfile(_get_key_file_path())
+
+
+def read_key_from_file():
+    with io.open(_get_key_file_path(), 'r', encoding='ascii') as key_file:
+        key = str(key_file.read()).encode('ascii')
+    return key
+
+
+def create_new_key_file():
+    dir_path = os.path.dirname(_get_key_file_path())
+    if not os.path.isdir(dir_path):
+        os.makedirs(dir_path, mode=0o700)  # important: restrictive permissions
+    key = Fernet.generate_key()
+    # first create file with restrictive permissions, then write key
+    # two separate file opens because each version supports
+    # permissions and encoding respectively, but neither supports both.
+    with os.fdopen(os.open(_get_key_file_path(), os.O_CREAT | os.O_WRONLY,
+                           0o600), 'w'):
+        pass
+    with io.open(_get_key_file_path(), 'w', encoding='ascii') as key_file:
+        key_file.write(key.decode('ascii'))
+    return key
+
+
+def initialize_key():
+    '''initialize key.'''
+    global __key
+    global __fernet
+    if key_file_exists():
+        __key = read_key_from_file()
+    else:
+        __key = create_new_key_file()
+
+    __fernet = Fernet(__key)
+
+
+def initialize_if_needed():
+    '''initialize key if not already initialized.'''
+    global __fernet
+    if not __fernet:
+        initialize_key()
+
+
+def encrypt(string):
+    initialize_if_needed()
+    return __fernet.encrypt(string.encode('utf-8')).decode('utf-8')
+
+
+class InvalidToken(fernet.InvalidToken):
+    pass
+
+
+def decrypt(string):
+    initialize_if_needed()
+    try:
+        return __fernet.decrypt(string.encode('utf-8')).decode('utf-8')
+    except fernet.InvalidToken as exc:
+        raise InvalidToken(exc)

congress/server/congress_server.py

@@ -38,7 +38,7 @@ from congress.db import api as db_api
 # This appears in main() too.  Removing either instance breaks something.
 config.init(sys.argv[1:])
 from congress.common import eventlet_server
-
+from congress import encryption
 from congress import harness
 
 LOG = logging.getLogger(__name__)
@@ -145,6 +145,7 @@ def main():
         sys.exit("ERROR: Unable to find configuration file via default "
                  "search paths ~/.congress/, ~/, /etc/congress/, /etc/) and "
                  "the '--config-file' option!")
+    encryption.initialize_key()
     if cfg.CONF.replicated_policy_engine and not (
             db_api.is_mysql() or db_api.is_postgres()):
         if db_api.is_sqlite():

congress/tests/db/test_datasources.py

@@ -16,6 +16,8 @@ from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
 
+import json
+
 from oslo_utils import uuidutils
 
 from congress.db import datasources
@@ -31,14 +33,15 @@ class TestDbDatasource(base.SqlTestCase):
             id_=id_,
             name="hiya",
             driver="foo",
-            config='{user: foo}',
+            config={'user': 'foo'},
             description="hello",
             enabled=True)
         self.assertEqual(id_, source.id)
         self.assertEqual("hiya", source.name)
         self.assertEqual("foo", source.driver)
         self.assertEqual("hello", source.description)
-        self.assertEqual('"{user: foo}"', source.config)
+        self.assertEqual({'user': 'foo', '__encrypted_fields': []},
+                         json.loads(source.config))
         self.assertTrue(source.enabled)
 
     def test_delete_datasource(self):
@@ -47,7 +50,7 @@ class TestDbDatasource(base.SqlTestCase):
             id_=id_,
             name="hiya",
             driver="foo",
-            config='{user: foo}',
+            config={'user': 'foo'},
             description="hello",
             enabled=True)
         self.assertTrue(datasources.delete_datasource(id_))
@@ -62,7 +65,7 @@ class TestDbDatasource(base.SqlTestCase):
             id_=id_,
             name="hiya",
             driver="foo",
-            config='{user: foo}',
+            config={'user': 'foo'},
             description="hello",
             enabled=True)
         db_ds_table_data.store_ds_table_data(
@@ -79,7 +82,7 @@ class TestDbDatasource(base.SqlTestCase):
             id_=id_,
             name="hiya",
             driver="foo",
-            config='{user: foo}',
+            config={'user': 'foo'},
             description="hello",
             enabled=True)
         source = datasources.get_datasource_by_name('hiya')
@@ -87,7 +90,7 @@ class TestDbDatasource(base.SqlTestCase):
         self.assertEqual("hiya", source.name)
         self.assertEqual("foo", source.driver)
         self.assertEqual("hello", source.description)
-        self.assertEqual('"{user: foo}"', source.config)
+        self.assertEqual({'user': 'foo'}, json.loads(source.config))
         self.assertTrue(source.enabled)
 
     def test_get_datasource_by_id(self):
@@ -96,7 +99,7 @@ class TestDbDatasource(base.SqlTestCase):
             id_=id_,
             name="hiya",
             driver="foo",
-            config='{user: foo}',
+            config={'user': 'foo'},
             description="hello",
             enabled=True)
         source = datasources.get_datasource(id_)
@@ -104,7 +107,7 @@ class TestDbDatasource(base.SqlTestCase):
         self.assertEqual("hiya", source.name)
         self.assertEqual("foo", source.driver)
         self.assertEqual("hello", source.description)
-        self.assertEqual('"{user: foo}"', source.config)
+        self.assertEqual({'user': 'foo'}, json.loads(source.config))
         self.assertTrue(source.enabled)
 
     def test_get_datasource(self):
@@ -113,7 +116,7 @@ class TestDbDatasource(base.SqlTestCase):
             id_=id_,
             name="hiya",
             driver="foo",
-            config='{user: foo}',
+            config={'user': 'foo'},
             description="hello",
             enabled=True)
         sources = datasources.get_datasources()
@@ -121,5 +124,23 @@ class TestDbDatasource(base.SqlTestCase):
         self.assertEqual("hiya", sources[0].name)
         self.assertEqual("foo", sources[0].driver)
         self.assertEqual("hello", sources[0].description)
-        self.assertEqual('"{user: foo}"', sources[0].config)
+        self.assertEqual({'user': 'foo'}, json.loads(sources[0].config))
+        self.assertTrue(sources[0].enabled)
+
+    def test_get_datasource_with_encryption(self):
+        id_ = uuidutils.generate_uuid()
+        datasources.add_datasource(
+            id_=id_,
+            name="hiya",
+            driver="foo",
+            config={'user': 'foo'},
+            description="hello",
+            enabled=True,
+            secret_config_fields=['user'])
+        sources = datasources.get_datasources()
+        self.assertEqual(id_, sources[0].id)
+        self.assertEqual("hiya", sources[0].name)
+        self.assertEqual("foo", sources[0].driver)
+        self.assertEqual("hello", sources[0].description)
+        self.assertEqual({'user': 'foo'}, json.loads(sources[0].config))
         self.assertTrue(sources[0].enabled)

congress/tests/dse2/test_dse_node.py

@@ -317,6 +317,8 @@ class TestDseNode(base.SqlTestCase):
         node = services['node']
         ds_manager = services['ds_manager']
         ds = self._get_datasource_request()
+        mock_driver_info.return_value = {'secret': [],
+                                         'module': mock.MagicMock()}
         ds_manager.add_datasource(ds)
         mock_driver_info.side_effect = [exception.DriverNotFound]
         node.delete_missing_driver_datasources()

congress/tests/etc/congress.conf.test

@@ -1,3 +1,6 @@
+[DEFAULT]
+encryption_key_path = 'congress/tests/etc/keys'
+
 [database]
 connection = 'sqlite://'
 # connection = mysql+pymysql://root:password@127.0.0.1/congress?charset=utf8

congress/tests/etc/congress.conf.test.ha_pe1

@@ -4,6 +4,7 @@ auth_strategy = noauth
 datasource_sync_period = 5
 debug = True
 replicated_policy_engine = True
+encryption_key_path = 'congress/tests/etc/keys'
 
 [database]
 # connection = mysql+pymysql://root:password@127.0.0.1/congress?charset=utf8

congress/tests/etc/congress.conf.test.ha_pe2

@@ -4,6 +4,7 @@ auth_strategy = noauth
 datasource_sync_period = 5
 debug = True
 replicated_policy_engine = True
+encryption_key_path = 'congress/tests/etc/keys'
 
 [database]
 # connection = mysql+pymysql://root:password@127.0.0.1/congress?charset=utf8

releasenotes/notes/encrypt-secret-fields-19c9d21aeb51a064.yaml

@@ -0,0 +1,16 @@
+---
+prelude: >
+upgrade:
+  - A new config option `encryption_key_path` has been added to the DEFAULT
+    section to specify the path to the directory containing encryption keys for
+    encrypting secret fields in datasource config. The default value
+    (/etc/congress/keys) works for most deployments. A new key will be
+    automatically generated and placed in the directory specified by the
+    config option.
+
+security:
+  - Secret fields in datasource configuration are now encrypted using Fernet
+    (AES-128 CBC; HMAC-SHA256).
+    Existing datasources are unaffected. To encrypt the secret
+    fields of existing datasources, simply delete and re-add after Congress
+    upgrade.

requirements.txt

@@ -23,6 +23,7 @@ python-cinderclient>=3.0.0 # Apache-2.0
 python-swiftclient>=3.2.0 # Apache-2.0
 python-ironicclient>=1.14.0 # Apache-2.0
 alembic>=0.8.10 # MIT
+cryptography>=1.6,!=2.0  # BSD/Apache-2.0
 python-dateutil>=2.4.2 # BSD
 python-glanceclient>=2.7.0 # Apache-2.0
 Routes>=2.3.1 # MIT