CVE-2019-3895

@@ -69,6 +69,7 @@
     ca_private_key_path: "{{ ca_private_key_path }}"
     ca_passphrase: "{{ ca_passphrase }}"
     client_cert_path: "{{ client_cert_path }}"
+    auth_project_name: "{{ auth_project_name }}"
   environment:
     OS_USERNAME: "{{ os_username }}"
     OS_USER_DOMAIN_NAME: "Default"

@@ -41,3 +41,15 @@
       src: "manager-post-deploy.conf.j2"
       selevel: s0
       setype: svirt_sandbox_file_t
+  - name: gather facts about the service project
+    shell: |
+      openstack project show "{{ auth_project_name }}" -c id -f value
+    register: project_id_result
+  - name: setting [controller_worker]/amp_image_owner_id
+    become: true
+    become_user: root
+    ini_file:
+      path: "{{ octavia_confd_prefix }}/etc/octavia/conf.d/common/post-deploy.conf"
+      section: controller_worker
+      option: amp_image_owner_id
+      value: "{{ project_id_result.stdout }}"

@@ -19,13 +19,38 @@
       amphora_image: "{{ (image_file_result.stat.path | basename | splitext)[0] }}"
     when: amphora_image is not defined and image_file_result.stat.exists and not symlnk_check.stat.islnk
 
-  - name: check there an image in glance already
+  - name: gather facts about the service project
     shell: |
-      openstack image show {{ amphora_image }} -c checksum -f value
+      openstack project show "{{ auth_project_name }}" -c id -f value
+    register: project_id_result
+
+  - name: check there's an image in glance already
+    shell: |
+      openstack image list --property owner={{ project_id_result.stdout }} --private --name {{ amphora_image }} -c ID -f value
+    environment:
+      OS_USERNAME: "{{ auth_username }}"
+      OS_PASSWORD: "{{ auth_password }}"
+      OS_PROJECT_NAME: "{{ auth_project_name }}"
+    register: glance_id_result
+    ignore_errors: true
+
+  - name: set image id fact
+    set_fact:
+      image_id: "{{ glance_id_result.stdout }}"
+    when: glance_id_result.rc == 0
+
+  - name: get checksum if there's an image in glance already
+    shell: |
+      openstack image show {{ glance_id_result.stdout }} -c checksum -f value
+    environment:
+      OS_USERNAME: "{{ auth_username }}"
+      OS_PASSWORD: "{{ auth_password }}"
+      OS_PROJECT_NAME: "{{ auth_project_name }}"
+    when: image_id is defined
     register: glance_results
     ignore_errors: true
 
-  - name: get md5 from glance if image already exists there
+  - name: set current_md5 fact from glance if image already exists there
     set_fact:
       current_md5: "{{ glance_results.stdout }}"
     when: glance_results.rc == 0
@@ -37,10 +62,14 @@
 
   - name: move existing image if the names match and the md5s are not the same
     shell: |
-      ts=`openstack image show {{ amphora_image }} -f value -c created_at`
+      ts=`openstack image show {{ image_id }} -f value -c created_at`
       ts=${ts//:/}
       ts=${ts//-/}
-      openstack image set {{ amphora_image }} --name "{{ amphora_image }}_$ts"
+      openstack image set {{ image_id }} --name "{{ amphora_image }}_$ts"
+    environment:
+      OS_USERNAME: "{{ auth_username }}"
+      OS_PASSWORD: "{{ auth_password }}"
+      OS_PROJECT_NAME: "{{ auth_project_name }}"
     when: replace_image is defined and replace_image
 
   - name: decide whether to upload new image
@@ -73,7 +102,11 @@
         --container-format bare --tag {{ amp_image_tag }} \
         --file {{ raw_filename|default(image_filename) }} \
         --property hw_architecture={{ amp_hw_arch }} \
-        {{ amphora_image }}
+        --private {{ amphora_image }}
+    environment:
+      OS_USERNAME: "{{ auth_username }}"
+      OS_PASSWORD: "{{ auth_password }}"
+      OS_PROJECT_NAME: "{{ auth_project_name }}"
     register: image_result
     changed_when: "image_result.stdout != ''"
     when: image_file_result.stat.exists and upload_image is defined

@@ -0,0 +1,10 @@
+---
+security:
+  - |
+    Fixed a vulnerability where an attacker may cause new Octavia amphorae to
+    run based on any arbitrary image (CVE-2019-3895).
+fixes:
+  - |
+    Ensure [controller_worker]/amp_image_owner_id is set. This configuration
+    option restricts Glance image selection to a specific owner ID. This is a
+    recommended security setting.

@@ -25,6 +25,11 @@ function build_octavia_worker_image {
         $OCTAVIA_DIR/diskimage-create/diskimage-create.sh -s 2 -o $OCTAVIA_AMP_IMAGE_FILE
     fi
     upload_image file://${OCTAVIA_AMP_IMAGE_FILE} $TOKEN
+
+    image_id=$(glance image-list --property-filter name=${OCTAVIA_AMP_IMAGE_NAME} | awk '/ amphora-x64-haproxy / {print $2}')
+    owner_id=$(glance image-show ${image_id} | awk '/ owner / {print $4}')
+    iniset $OCTAVIA_CONF controller_worker amp_image_owner_id ${owner_id}
+
 }
 
 function create_octavia_accounts {

@@ -135,6 +135,9 @@
 # parameters is needed. Using tags is the recommended way to refer to images.
 # amp_image_id =
 # amp_image_tag =
+# Optional owner ID used to restrict glance images to one owner ID.
+# This is a recommended security setting.
+# amp_image_owner_id =
 # Nova parameters to use when booting amphora
 # amp_flavor_id =
 # amp_ssh_key_name =

@@ -216,6 +216,10 @@
                deprecated_for_removal=True,
                deprecated_reason='Superseded by amp_image_tag option.',
                help=_('Glance image id for the Amphora image to boot')),
+    cfg.StrOpt('amp_image_owner_id',
+               default='',
+               help=_('Restrict glance image selection to a specific '
+                      'owner ID.  This is a recommended security setting.')),
     cfg.StrOpt('amp_ssh_key_name',
                default='',
                help=_('SSH key name used to boot the Amphora')),

@@ -331,3 +331,4 @@
 AMP_ACTION_START = 'start'
 AMP_ACTION_STOP = 'stop'
 AMP_ACTION_RELOAD = 'reload'
+GLANCE_IMAGE_ACTIVE = 'active'

@@ -22,7 +22,7 @@ class ComputeBase(object):
 
     @abc.abstractmethod
     def build(self, name="amphora_name", amphora_flavor=None,
-              image_id=None, image_tag=None,
+              image_id=None, image_tag=None, image_owner=None,
               key_name=None, sec_groups=None, network_ids=None,
               config_drive_files=None, user_data=None, server_group_id=None):
         """Build a new amphora.

@@ -28,22 +28,22 @@ def __init__(self):
         self.computeconfig = {}
 
     def build(self, name="amphora_name", amphora_flavor=None,
-              image_id=None, image_tag=None,
+              image_id=None, image_tag=None, image_owner=None,
               key_name=None, sec_groups=None, network_ids=None,
               config_drive_files=None, user_data=None, port_ids=None,
               server_group_id=None):
         LOG.debug("Compute %s no-op, build name %s, amphora_flavor %s, "
-                  "image_id %s, image_tag %s, key_name %s, sec_groups %s, "
-                  "network_ids %s, config_drive_files %s, user_data %s, "
-                  "port_ids %s, server_group_id %s",
+                  "image_id %s, image_tag %s, image_owner %s, key_name %s, "
+                  "sec_groups %s, network_ids %s, config_drive_files %s, "
+                  "user_data %s, port_ids %s, server_group_id %s",
                   self.__class__.__name__,
-                  name, amphora_flavor, image_id, image_tag,
+                  name, amphora_flavor, image_id, image_tag, image_owner,
                   key_name, sec_groups, network_ids, config_drive_files,
                   user_data, port_ids, server_group_id)
         self.computeconfig[(name, amphora_flavor, image_id, image_tag,
-                            key_name, user_data)] = (
+                            image_owner, key_name, user_data)] = (
             name, amphora_flavor,
-            image_id, image_tag, key_name, sec_groups,
+            image_id, image_tag, image_owner, key_name, sec_groups,
             network_ids, config_drive_files,
             user_data, port_ids, 'build')
         compute_id = uuidutils.generate_uuid()
@@ -87,13 +87,13 @@ def __init__(self):
         self.driver = NoopManager()
 
     def build(self, name="amphora_name", amphora_flavor=None,
-              image_id=None, image_tag=None,
+              image_id=None, image_tag=None, image_owner=None,
               key_name=None, sec_groups=None, network_ids=None,
               config_drive_files=None, user_data=None, port_ids=None,
               server_group_id=None):
 
         compute_id = self.driver.build(name, amphora_flavor,
-                                       image_id, image_tag,
+                                       image_id, image_tag, image_owner,
                                        key_name, sec_groups, network_ids,
                                        config_drive_files, user_data, port_ids,
                                        server_group_id)

@@ -32,11 +32,21 @@
 CONF.import_group('nova', 'octavia.common.config')
 
 
-def _extract_amp_image_id_by_tag(client, image_tag):
-    images = list(client.images.list(
-        filters={'tag': [image_tag]},
-        sort='created_at:desc',
-        limit=2))
+def _extract_amp_image_id_by_tag(client, image_tag, image_owner):
+    if image_owner:
+        images = list(client.images.list(
+            filters={'tag': [image_tag],
+                     'owner': image_owner,
+                     'status': constants.GLANCE_IMAGE_ACTIVE},
+            sort='created_at:desc',
+            limit=2))
+    else:
+        images = list(client.images.list(
+            filters={'tag': [image_tag],
+                     'status': constants.GLANCE_IMAGE_ACTIVE},
+            sort='created_at:desc',
+            limit=2))
+
     if not images:
         raise exceptions.GlanceNoTaggedImages(tag=image_tag)
     image_id = images[0]['id']
@@ -50,15 +60,15 @@ def _extract_amp_image_id_by_tag(client, image_tag):
     return image_id
 
 
-def _get_image_uuid(client, image_id, image_tag):
+def _get_image_uuid(client, image_id, image_tag, image_owner):
     if image_id:
         if image_tag:
             LOG.warning(
                 _LW("Both amp_image_id and amp_image_tag options defined. "
-                    "Using the former."))
+                    "Using the amp_image_id."))
         return image_id
 
-    return _extract_amp_image_id_by_tag(client, image_tag)
+    return _extract_amp_image_id_by_tag(client, image_tag, image_owner)
 
 
 class VirtualMachineManager(compute_base.ComputeBase):
@@ -84,7 +94,7 @@ def __init__(self):
         self.server_groups = self._nova_client.server_groups
 
     def build(self, name="amphora_name", amphora_flavor=None,
-              image_id=None, image_tag=None,
+              image_id=None, image_tag=None, image_owner=None,
               key_name=None, sec_groups=None, network_ids=None,
               port_ids=None, config_drive_files=None, user_data=None,
               server_group_id=None):
@@ -126,7 +136,7 @@ def build(self, name="amphora_name", amphora_flavor=None,
                 "group": server_group_id}
 
             image_id = _get_image_uuid(
-                self._glance_client, image_id, image_tag)
+                self._glance_client, image_id, image_tag, image_owner)
             amphora = self.manager.create(
                 name=name, image=image_id, flavor=amphora_flavor,
                 key_name=key_name, security_groups=sec_groups,

@@ -83,6 +83,7 @@ def execute(self, amphora_id, ports=None, config_drive_files=None,
                 amphora_flavor=CONF.controller_worker.amp_flavor_id,
                 image_id=CONF.controller_worker.amp_image_id,
                 image_tag=CONF.controller_worker.amp_image_tag,
+                image_owner=CONF.controller_worker.amp_image_owner_id,
                 key_name=key_name,
                 sec_groups=CONF.controller_worker.amp_secgroup_list,
                 network_ids=network_ids,

@@ -35,18 +35,19 @@ def test__get_image_uuid_tag(self):
         with mock.patch.object(nova_common,
                                '_extract_amp_image_id_by_tag',
                                return_value='fakeid') as extract:
-            image_id = nova_common._get_image_uuid(client, '', 'faketag')
+            image_id = nova_common._get_image_uuid(client, '', 'faketag', None)
         self.assertEqual('fakeid', image_id)
-        extract.assert_called_with(client, 'faketag')
+        extract.assert_called_with(client, 'faketag', None)
 
     def test__get_image_uuid_notag(self):
         client = mock.Mock()
-        image_id = nova_common._get_image_uuid(client, 'fakeid', '')
+        image_id = nova_common._get_image_uuid(client, 'fakeid', '', None)
         self.assertEqual('fakeid', image_id)
 
     def test__get_image_uuid_id_beats_tag(self):
         client = mock.Mock()
-        image_id = nova_common._get_image_uuid(client, 'fakeid', 'faketag')
+        image_id = nova_common._get_image_uuid(client, 'fakeid',
+                                               'faketag', None)
         self.assertEqual('fakeid', image_id)
 
 
@@ -62,15 +63,16 @@ def test_no_images(self):
         self.client.images.list.return_value = []
         self.assertRaises(
             exceptions.GlanceNoTaggedImages,
-            nova_common._extract_amp_image_id_by_tag, self.client, 'faketag')
+            nova_common._extract_amp_image_id_by_tag, self.client,
+            'faketag', None)
 
     def test_single_image(self):
         images = [
             {'id': uuidutils.generate_uuid(), 'tag': 'faketag'}
         ]
         self.client.images.list.return_value = images
         image_id = nova_common._extract_amp_image_id_by_tag(self.client,
-                                                            'faketag')
+                                                            'faketag', None)
         self.assertIn(image_id, images[0]['id'])
 
     def test_multiple_images_returns_one_of_images(self):
@@ -80,7 +82,7 @@ def test_multiple_images_returns_one_of_images(self):
         ]
         self.client.images.list.return_value = images
         image_id = nova_common._extract_amp_image_id_by_tag(self.client,
-                                                            'faketag')
+                                                            'faketag', None)
         self.assertIn(image_id, [image['id'] for image in images])
 
 

@@ -87,6 +87,11 @@ def setUp(self):
     @mock.patch('stevedore.driver.DriverManager.driver')
     def test_compute_create(self, mock_driver, mock_conf, mock_jinja):
 
+        image_owner_id = uuidutils.generate_uuid()
+        conf = oslo_fixture.Config(cfg.CONF)
+        conf.config(group="controller_worker",
+                    amp_image_owner_id=image_owner_id)
+
         createcompute = compute_tasks.ComputeCreate()
 
         mock_driver.build.return_value = COMPUTE_ID
@@ -100,6 +105,7 @@ def test_compute_create(self, mock_driver, mock_conf, mock_jinja):
             amphora_flavor=AMP_FLAVOR_ID,
             image_id=AMP_IMAGE_ID,
             image_tag=AMP_IMAGE_TAG,
+            image_owner=image_owner_id,
             key_name=AMP_SSH_KEY_NAME,
             sec_groups=AMP_SEC_GROUPS,
             network_ids=AMP_NET,
@@ -134,6 +140,9 @@ def test_compute_create(self, mock_driver, mock_conf, mock_jinja):
 
         createcompute.revert(COMPUTE_ID, _amphora_mock.id)
 
+        conf.config(group="controller_worker",
+                    amp_image_owner_id='')
+
     @mock.patch('jinja2.Environment.get_template')
     @mock.patch('octavia.amphorae.backends.agent.'
                 'agent_jinja_cfg.AgentJinjaTemplater.'
@@ -160,6 +169,7 @@ def test_compute_create_user_data(self, mock_driver,
             amphora_flavor=AMP_FLAVOR_ID,
             image_id=AMP_IMAGE_ID,
             image_tag=AMP_IMAGE_TAG,
+            image_owner='',
             key_name=AMP_SSH_KEY_NAME,
             sec_groups=AMP_SEC_GROUPS,
             network_ids=AMP_NET,
@@ -219,6 +229,7 @@ def test_compute_create_without_ssh_access(self, mock_driver,
             amphora_flavor=AMP_FLAVOR_ID,
             image_id=AMP_IMAGE_ID,
             image_tag=AMP_IMAGE_TAG,
+            image_owner='',
             key_name=None,
             sec_groups=AMP_SEC_GROUPS,
             network_ids=AMP_NET,
@@ -276,6 +287,7 @@ def test_compute_create_cert(self, mock_driver, mock_conf, mock_jinja):
             amphora_flavor=AMP_FLAVOR_ID,
             image_id=AMP_IMAGE_ID,
             image_tag=AMP_IMAGE_TAG,
+            image_owner='',
             key_name=AMP_SSH_KEY_NAME,
             sec_groups=AMP_SEC_GROUPS,
             network_ids=AMP_NET,

@@ -0,0 +1,6 @@
+---
+security:
+  - Allows the operator to optionally restrict the amphora
+    glance image selection to a specific owner id.
+    This is a recommended security setting for clouds that
+    allow user uploadable images.