openstack
/
octavia
Code Issues Proposed changes

Encrypt certs and keys 

Octavia creates certificates and keys to manage encrypted
communication channel to amphorae.
When debug is enabled, the python taskflow module will log
all the information we provide to tasks (and sub-flows)
when we create amphorae or handle with anything related to
certificates and keys management (rotations, etc).

There are ways to tell taskflow to exclude specific things
from being logged (e.g., I136081045787c1bbe3ee846d5845a34201c57864).
While this handles some information in specific flows from being
logged, it is susceptive to code changes.

To avoid an everlasting whack-a-mole game, this patch will merely
encrypt sensitive information so we can safely log it and decrypts
it only when we need to use it.

Conflicts:
       octavia/controller/worker/controller_worker.py
       octavia/controller/worker/tasks/database_tasks.py

Change-Id: I06d329ca53bc36bd27f7870ae7c7ca0cf18575b2
(cherry picked from commit ae7c87f54a)
This commit is contained in:
Nir Magnezi 2018-12-20 17:09:26 +02:00
parent ec4c88e23e
commit dc4c0b6249
14 changed files with 105 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
devstack/plugin.sh
View File

@ -303,6 +303,7 @@ function octavia_configure {
iniset $OCTAVIA_CONF certificates ca_certificate ${OCTAVIA_CERTS_DIR}/ca_01.pem
iniset $OCTAVIA_CONF certificates ca_private_key ${OCTAVIA_CERTS_DIR}/private/cakey.pem
iniset $OCTAVIA_CONF certificates ca_private_key_passphrase foobar
iniset $OCTAVIA_CONF certificates server_certs_key_passphrase insecure-key-do-not-use-this-key
if [[ "$OCTAVIA_USE_LEGACY_RBAC" == "True" ]]; then
cp $OCTAVIA_DIR/etc/policy/admin_or_owner-policy.json $OCTAVIA_CONF_DIR/policy.json

13
octavia/certificates/common/local.py
View File

@ -29,6 +29,9 @@ TLS_KEY_DEFAULT = os.environ.get(
'OS_OCTAVIA_TLS_CA_KEY', '/etc/ssl/private/ssl-cert-snakeoil.key'
)
TLS_PKP_DEFAULT = os.environ.get('OS_OCTAVIA_CA_KEY_PASS')
TLS_PASS_AMPS_DEFAULT = os.environ.get('TLS_PASS_AMPS_DEFAULT',
'insecure-key-do-not-use-this-key')
TLS_DIGEST_DEFAULT = os.environ.get('OS_OCTAVIA_CA_SIGNING_DIGEST', 'sha256')
TLS_STORAGE_DEFAULT = os.environ.get(
'OS_OCTAVIA_TLS_STORAGE', '/var/lib/octavia/certificates/'
@ -47,6 +50,12 @@ certgen_opts = [
default=TLS_PKP_DEFAULT,
help='Passphrase for the Private Key. Defaults'
' to env[OS_OCTAVIA_CA_KEY_PASS] or None.'),
cfg.StrOpt('server_certs_key_passphrase',
default=TLS_PASS_AMPS_DEFAULT,
help='Passphrase for encrypting Amphora Certificates and '
'Private Keys. Defaults to env[TLS_PASS_AMPS_DEFAULT] or '
'insecure-key-do-not-use-this-key',
required=True),
cfg.StrOpt('signing_digest',
default=TLS_DIGEST_DEFAULT,
help='Certificate signing digest. Defaults'
@ -60,10 +69,6 @@ certmgr_opts = [
'Defaults to env[OS_OCTAVIA_TLS_STORAGE].')
]
CONF = cfg.CONF
CONF.register_opts(certgen_opts, group='certificates')
CONF.register_opts(certmgr_opts, group='certificates')
class LocalCert(cert.Cert):
"""Representation of a Cert for local storage."""

4
octavia/common/config.py
View File

@ -25,6 +25,7 @@ from oslo_db import options as db_options
from oslo_log import log as logging
import oslo_messaging as messaging
from octavia.certificates.common import local
from octavia.common import constants
from octavia.common import utils
from octavia.i18n import _
@ -609,6 +610,9 @@ cfg.CONF.register_opts(neutron_opts, group='neutron')
cfg.CONF.register_opts(quota_opts, group='quotas')
cfg.CONF.register_opts(local.certgen_opts, group='certificates')
cfg.CONF.register_opts(local.certmgr_opts, group='certificates')
# Ensure that the control exchange is set correctly
messaging.set_transport_defaults(control_exchange='octavia')
_SQL_CONNECTION_DEFAULT = 'sqlite://'

14
octavia/common/utils.py
View File

@ -26,6 +26,7 @@ import netaddr
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import excutils
import six
from stevedore import driver as stevedore_driver
CONF = cfg.CONF
@ -90,6 +91,19 @@ def ip_netmask_to_cidr(ip, netmask):
return "{ip}/{netmask}".format(ip=net.network, netmask=net.prefixlen)
def get_six_compatible_value(value, six_type=six.string_types):
if six.PY3 and isinstance(value, six_type):
value = value.encode('utf-8')
return value
def get_six_compatible_server_certs_key_passphrase():
key = CONF.certificates.server_certs_key_passphrase
if six.PY3 and isinstance(key, six.string_types):
key = key.encode('utf-8')
return base64.urlsafe_b64encode(key)
class exception_logger(object):
"""Wrap a function and log raised exception

30
octavia/controller/worker/controller_worker.py
View File

@ -70,23 +70,6 @@ class ControllerWorker(base_taskflow.BaseTaskFlowEngine):
self._l7policy_repo = repo.L7PolicyRepository()
self._l7rule_repo = repo.L7RuleRepository()
self._exclude_result_logging_tasks = (
constants.ROLE_STANDALONE + '-' +
constants.CREATE_AMP_FOR_LB_SUBFLOW + '-' +
constants.GENERATE_SERVER_PEM,
constants.ROLE_BACKUP + '-' +
constants.CREATE_AMP_FOR_LB_SUBFLOW + '-' +
constants.GENERATE_SERVER_PEM,
constants.ROLE_MASTER + '-' +
constants.CREATE_AMP_FOR_LB_SUBFLOW + '-' +
constants.GENERATE_SERVER_PEM,
constants.GENERATE_SERVER_PEM_TASK,
constants.FAILOVER_AMPHORA_FLOW + '-' +
constants.CREATE_AMP_FOR_LB_SUBFLOW + '-' +
constants.GENERATE_SERVER_PEM,
constants.CREATE_AMP_FOR_LB_SUBFLOW + '-' +
constants.UPDATE_CERT_EXPIRATION)
super(ControllerWorker, self).__init__()
@tenacity.retry(
@ -112,9 +95,7 @@ class ControllerWorker(base_taskflow.BaseTaskFlowEngine):
store={constants.BUILD_TYPE_PRIORITY:
constants.LB_CREATE_SPARES_POOL_PRIORITY}
)
with tf_logging.DynamicLoggingListener(
create_amp_tf, log=LOG,
hide_inputs_outputs_of=self._exclude_result_logging_tasks):
with tf_logging.DynamicLoggingListener(create_amp_tf, log=LOG):
create_amp_tf.run()
@ -351,9 +332,7 @@ class ControllerWorker(base_taskflow.BaseTaskFlowEngine):
topology=topology, listeners=lb.listeners)
create_lb_tf = self._taskflow_load(create_lb_flow, store=store)
with tf_logging.DynamicLoggingListener(
create_lb_tf, log=LOG,
hide_inputs_outputs_of=self._exclude_result_logging_tasks):
with tf_logging.DynamicLoggingListener(create_lb_tf, log=LOG):
create_lb_tf.run()
def delete_load_balancer(self, load_balancer_id, cascade=False):
@ -843,10 +822,7 @@ class ControllerWorker(base_taskflow.BaseTaskFlowEngine):
role=amp.role, load_balancer=lb),
store=stored_params)
with tf_logging.DynamicLoggingListener(
failover_amphora_tf, log=LOG,
hide_inputs_outputs_of=self._exclude_result_logging_tasks):
with tf_logging.DynamicLoggingListener(failover_amphora_tf, log=LOG):
failover_amphora_tf.run()
def failover_amphora(self, amphora_id):

6
octavia/controller/worker/tasks/amphora_driver_tasks.py
View File

@ -13,6 +13,7 @@
# under the License.
#
from cryptography import fernet
from oslo_config import cfg
from oslo_log import log as logging
import six
@ -22,6 +23,7 @@ from taskflow.types import failure
from octavia.amphorae.driver_exceptions import exceptions as driver_except
from octavia.common import constants
from octavia.common import utils
from octavia.controller.worker import task_utils as task_utilities
from octavia.db import api as db_apis
from octavia.db import repositories as repo
@ -272,7 +274,9 @@ class AmphoraCertUpload(BaseAmphoraTask):
def execute(self, amphora, server_pem):
"""Execute cert_update_amphora routine."""
LOG.debug("Upload cert in amphora REST driver")
self.amphora_driver.upload_cert_amp(amphora, server_pem)
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
self.amphora_driver.upload_cert_amp(amphora, fer.decrypt(server_pem))
class AmphoraUpdateVRRPInterface(BaseAmphoraTask):

6
octavia/controller/worker/tasks/cert_task.py
View File

@ -13,10 +13,12 @@
# under the License.
#
from cryptography import fernet
from oslo_config import cfg
from stevedore import driver as stevedore_driver
from taskflow import task
from octavia.common import utils
CONF = cfg.CONF
CERT_VALIDITY = 2 * 365 * 24 * 60 * 60
@ -44,5 +46,7 @@ class GenerateServerPEMTask(BaseCertTask):
cert = self.cert_generator.generate_cert_key_pair(
cn=amphora_id,
validity=CERT_VALIDITY)
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
return cert.certificate + cert.private_key
return fer.encrypt(cert.certificate + cert.private_key)

7
octavia/controller/worker/tasks/compute_tasks.py
View File

@ -15,6 +15,7 @@
import time
from cryptography import fernet
from oslo_config import cfg
from oslo_log import log as logging
from stevedore import driver as stevedore_driver
@ -25,6 +26,7 @@ from octavia.amphorae.backends.agent import agent_jinja_cfg
from octavia.common import constants
from octavia.common import exceptions
from octavia.common.jinja import user_data_jinja_cfg
from octavia.common import utils
from octavia.controller.worker import amphora_rate_limit
CONF = cfg.CONF
@ -134,8 +136,11 @@ class CertComputeCreate(ComputeCreate):
# load client certificate
with open(CONF.controller_worker.client_ca, 'r') as client_ca:
ca = client_ca.read()
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
config_drive_files = {
'/etc/octavia/certs/server.pem': server_pem,
'/etc/octavia/certs/server.pem': fer.decrypt(server_pem),
'/etc/octavia/certs/client_ca.pem': ca}
return super(CertComputeCreate, self).execute(
amphora_id, config_drive_files=config_drive_files,

8
octavia/controller/worker/tasks/database_tasks.py
View File

@ -13,6 +13,7 @@
# under the License.
#
from cryptography import fernet
from oslo_config import cfg
from oslo_db import exception as odb_exceptions
from oslo_log import log as logging
@ -27,6 +28,7 @@ from taskflow.types import failure
from octavia.common import constants
from octavia.common import data_models
import octavia.common.tls_utils.cert_parser as cert_parser
from octavia.common import utils
from octavia.controller.worker import task_utils as task_utilities
from octavia.db import api as db_apis
from octavia.db import repositories as repo
@ -892,7 +894,11 @@ class UpdateAmphoraDBCertExpiration(BaseDatabaseTask):
"""
LOG.debug("Update DB cert expiry date of amphora id: %s", amphora_id)
cert_expiration = cert_parser.get_cert_expiration(server_pem)
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
cert_expiration = cert_parser.get_cert_expiration(
fer.decrypt(server_pem))
LOG.debug("Certificate expiration date is %s ", cert_expiration)
self.amphora_repo.update(db_apis.get_session(), amphora_id,
cert_expiration=cert_expiration)

9
octavia/tests/unit/controller/worker/tasks/test_amphora_driver_tasks.py
View File

@ -13,6 +13,7 @@
# under the License.
#
from cryptography import fernet
import mock
from oslo_config import cfg
from oslo_config import fixture as oslo_fixture
@ -22,6 +23,7 @@ from taskflow.types import failure
from octavia.amphorae.driver_exceptions import exceptions as driver_except
from octavia.common import constants
from octavia.common import data_models
from octavia.common import utils
from octavia.controller.worker.tasks import amphora_driver_tasks
from octavia.db import repositories as repo
import octavia.tests.unit.base as base
@ -503,12 +505,15 @@ class TestAmphoraDriverTasks(base.TestCase):
mock_listener_repo_get,
mock_listener_repo_update,
mock_amphora_repo_update):
pem_file_mock = 'test-perm-file'
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
pem_file_mock = fer.encrypt(
utils.get_six_compatible_value('test-pem-file'))
amphora_cert_upload_mock = amphora_driver_tasks.AmphoraCertUpload()
amphora_cert_upload_mock.execute(_amphora_mock, pem_file_mock)
mock_driver.upload_cert_amp.assert_called_once_with(
_amphora_mock, pem_file_mock)
_amphora_mock, fer.decrypt(pem_file_mock))
def test_amphora_update_vrrp_interface(self,
mock_driver,

14
octavia/tests/unit/controller/worker/tasks/test_cert_task.py
View File

@ -13,21 +13,31 @@
# under the License.
#
from cryptography import fernet
import mock
from octavia.certificates.common import local
from octavia.common import utils
from octavia.controller.worker.tasks import cert_task
import octavia.tests.unit.base as base
class TestCertTasks(base.TestCase):
@mock.patch('stevedore.driver.DriverManager.driver')
def test_execute(self, mock_driver):
dummy_cert = local.LocalCert('test_cert', 'test_key')
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
dummy_cert = local.LocalCert(
utils.get_six_compatible_value('test_cert'),
utils.get_six_compatible_value('test_key'))
mock_driver.generate_cert_key_pair.side_effect = [dummy_cert]
c = cert_task.GenerateServerPEMTask()
pem = c.execute('123')
self.assertEqual(
pem, dummy_cert.get_certificate() + dummy_cert.get_private_key())
fer.decrypt(pem),
dummy_cert.get_certificate() +
dummy_cert.get_private_key()
)
mock_driver.generate_cert_key_pair.assert_called_once_with(
cn='123', validity=cert_task.CERT_VALIDITY)

14
octavia/tests/unit/controller/worker/tasks/test_compute_tasks.py
View File

@ -13,6 +13,7 @@
# under the License.
#
from cryptography import fernet
import mock
from oslo_config import cfg
from oslo_config import fixture as oslo_fixture
@ -20,6 +21,7 @@ from oslo_utils import uuidutils
from octavia.common import constants
from octavia.common import exceptions
from octavia.common import utils
from octavia.controller.worker.tasks import compute_tasks
from octavia.tests.common import utils as test_utils
import octavia.tests.unit.base as base
@ -270,13 +272,17 @@ class TestComputeTasks(base.TestCase):
@mock.patch('stevedore.driver.DriverManager.driver')
def test_compute_create_cert(self, mock_driver, mock_conf, mock_jinja):
createcompute = compute_tasks.CertComputeCreate()
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
mock_driver.build.return_value = COMPUTE_ID
path = '/etc/octavia/certs/ca_01.pem'
self.useFixture(test_utils.OpenFixture(path, 'test'))
# Test execute()
compute_id = createcompute.execute(_amphora_mock.id, 'test_cert',
test_cert = fer.encrypt(
utils.get_six_compatible_value('test_cert')
)
compute_id = createcompute.execute(_amphora_mock.id, test_cert,
server_group_id=SERVER_GRPOUP_ID
)
@ -293,7 +299,7 @@ class TestComputeTasks(base.TestCase):
port_ids=[],
user_data=None,
config_drive_files={
'/etc/octavia/certs/server.pem': 'test_cert',
'/etc/octavia/certs/server.pem': fer.decrypt(test_cert),
'/etc/octavia/certs/client_ca.pem': 'test',
'/etc/octavia/amphora-agent.conf': 'test_conf'},
server_group_id=SERVER_GRPOUP_ID)
@ -307,7 +313,7 @@ class TestComputeTasks(base.TestCase):
self.assertRaises(TypeError,
createcompute.execute,
_amphora_mock,
config_drive_files='test_cert')
config_drive_files=test_cert)
# Test revert()

43
octavia/tests/unit/controller/worker/tasks/test_database_tasks.py
View File

@ -15,6 +15,7 @@
import random
from cryptography import fernet
import mock
from oslo_db import exception as odb_exceptions
from oslo_utils import uuidutils
@ -23,6 +24,7 @@ from taskflow.types import failure
from octavia.common import constants
from octavia.common import data_models
from octavia.common import utils
from octavia.controller.worker.tasks import database_tasks
from octavia.db import repositories as repo
import octavia.tests.unit.base as base
@ -82,42 +84,6 @@ _vip_mock.subnet_id = SUBNET_ID
_vip_mock.ip_address = VIP_IP
_vrrp_group_mock = mock.MagicMock()
_cert_mock = mock.MagicMock()
_pem_mock = """Junk
-----BEGIN CERTIFICATE-----
MIIBhDCCAS6gAwIBAgIGAUo7hO/eMA0GCSqGSIb3DQEBCwUAMA8xDTALBgNVBAMT
BElNRDIwHhcNMTQxMjExMjI0MjU1WhcNMjUxMTIzMjI0MjU1WjAPMQ0wCwYDVQQD
EwRJTUQzMFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAKHIPXo2pfD5dpnpVDVz4n43
zn3VYsjz/mgOZU0WIWjPA97mvulb7mwb4/LB4ijOMzHj9XfwP75GiOFxYFs8O80C
AwEAAaNwMG4wDwYDVR0TAQH/BAUwAwEB/zA8BgNVHSMENTAzgBS6rfnABCO3oHEz
NUUtov2hfXzfVaETpBEwDzENMAsGA1UEAxMESU1EMYIGAUo7hO/DMB0GA1UdDgQW
BBRiLW10LVJiFO/JOLsQFev0ToAcpzANBgkqhkiG9w0BAQsFAANBABtdF+89WuDi
TC0FqCocb7PWdTucaItD9Zn55G8KMd93eXrOE/FQDf1ScC+7j0jIHXjhnyu6k3NV
8el/x5gUHlc=
-----END CERTIFICATE-----
Junk should be ignored by x509 splitter
-----BEGIN CERTIFICATE-----
MIIBhDCCAS6gAwIBAgIGAUo7hO/DMA0GCSqGSIb3DQEBCwUAMA8xDTALBgNVBAMT
BElNRDEwHhcNMTQxMjExMjI0MjU1WhcNMjUxMTIzMjI0MjU1WjAPMQ0wCwYDVQQD
EwRJTUQyMFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAJYHqnsisVKTlwVaCSa2wdrv
CeJJzqpEVV0RVgAAF6FXjX2Tioii+HkXMR9zFgpE1w4yD7iu9JDb8yTdNh+NxysC
AwEAAaNwMG4wDwYDVR0TAQH/BAUwAwEB/zA8BgNVHSMENTAzgBQt3KvN8ncGj4/s
if1+wdvIMCoiE6ETpBEwDzENMAsGA1UEAxMEcm9vdIIGAUo7hO+mMB0GA1UdDgQW
BBS6rfnABCO3oHEzNUUtov2hfXzfVTANBgkqhkiG9w0BAQsFAANBAIlJODvtmpok
eoRPOb81MFwPTTGaIqafebVWfBlR0lmW8IwLhsOUdsQqSzoeypS3SJUBpYT1Uu2v
zEDOmgdMsBY=
-----END CERTIFICATE-----
Junk should be thrown out like junk
-----BEGIN CERTIFICATE-----
MIIBfzCCASmgAwIBAgIGAUo7hO+mMA0GCSqGSIb3DQEBCwUAMA8xDTALBgNVBAMT
BHJvb3QwHhcNMTQxMjExMjI0MjU1WhcNMjUxMTIzMjI0MjU1WjAPMQ0wCwYDVQQD
EwRJTUQxMFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAI+tSJxr60ogwXFmgqbLMW7K
3fkQnh9sZBi7Qo6AzUnfe/AhXoisib651fOxKXCbp57IgzLTv7O9ygq3I+5fQqsC
AwEAAaNrMGkwDwYDVR0TAQH/BAUwAwEB/zA3BgNVHSMEMDAugBR73ZKSpjbsz9tZ
URkvFwpIO7gB4KETpBEwDzENMAsGA1UEAxMEcm9vdIIBATAdBgNVHQ4EFgQULdyr
zfJ3Bo+P7In9fsHbyDAqIhMwDQYJKoZIhvcNAQELBQADQQBenkZ2k7RgZqgj+dxA
D7BF8MN1oUAOpyYqAjkGddSEuMyNmwtHKZI1dyQ0gBIQdiU9yAG2oTbUIK4msbBV
uJIQ
-----END CERTIFICATE-----"""
_compute_mock = mock.MagicMock()
_compute_mock.lb_network_ip = LB_NET_IP
_compute_mock.cached_zone = CACHED_ZONE
@ -1072,6 +1038,11 @@ class TestDatabaseTasks(base.TestCase):
mock_get_cert_exp):
update_amp_cert = database_tasks.UpdateAmphoraDBCertExpiration()
key = utils.get_six_compatible_server_certs_key_passphrase()
fer = fernet.Fernet(key)
_pem_mock = fer.encrypt(
utils.get_six_compatible_value('test_cert')
)
update_amp_cert.execute(_amphora_mock.id, _pem_mock)
repo.AmphoraRepository.update.assert_called_once_with(

15
releasenotes/notes/encrypt-certs-and-keys-5175d7704d8df3ce.yaml Normal file
View File

@ -0,0 +1,15 @@
---
security:
- |
As a followup to the fix that resolved CVE-2018-16856, Octavia will now
encrypt certificates and keys used for secure communication with amphorae,
in its internal workflows. Octavia used to exclude debug-level log prints
for specific tasks and flows that were explicitly specified by name, a
method that is susceptive to code changes.
other:
- |
Added a new option named server_certs_key_passphrase under the certificates
section. The default value gets copied from an environment variable named
TLS_PASS_AMPS_DEFAULT. In a case where TLS_PASS_AMPS_DEFAULT is not set,
and the operator did not fill any other value directly,
'insecure-key-do-not-use-this-key' will be used.