Agent token support

Adds support to the agent to receive, store, and return
that token to ironic's API, when supported.

This feature allows ironic and ultimately the agent to
authenticate interactions, when supported, to prevent
malicious abuse of the API endpoint.

Sem-Ver: feature
Change-Id: I6db9117a38be946b785e6f5e75ada1bfdff560ba

ironic_python_agent/agent.py

@@ -165,7 +165,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
 
     def __init__(self, api_url, advertise_address, listen_address,
                  ip_lookup_attempts, ip_lookup_sleep, network_interface,
-                 lookup_timeout, lookup_interval, standalone,
+                 lookup_timeout, lookup_interval, standalone, agent_token,
                  hardware_initialization_delay=0):
         super(IronicPythonAgent, self).__init__()
         if bool(cfg.CONF.keyfile) != bool(cfg.CONF.certfile):
@@ -214,6 +214,11 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
         self.hardware_initialization_delay = hardware_initialization_delay
         # IPA will stop serving requests and exit after this is set to False
         self.serve_api = True
+        self.agent_token = agent_token
+        # Allows this to be turned on by the conductor while running,
+        # in the event of long running ramdisks where the conductor
+        # got upgraded somewhere along the way.
+        self.agent_token_required = cfg.CONF.agent_token_required
 
     def get_status(self):
         """Retrieve a serializable status.
@@ -226,6 +231,26 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
             version=self.version
         )
 
+    def validate_agent_token(self, token):
+        # We did not get a token, i.e. None and
+        # we've previously seen a token, which is
+        # a mid-cluster upgrade case with long-running ramdisks.
+        if (not token and self.agent_token
+            and not self.agent_token_required):
+                # TODO(TheJulia): Rip this out during or after the V
+                # cycle.
+                LOG.warning('Agent token for requests are not required '
+                            'by the conductor, yet we received a token. '
+                            'Cluster may be mid-upgrade. Support to '
+                            'not fail in this condition will be removed in '
+                            'the Victoria development cycle.')
+                # Tell the API everything is okay.
+                return True
+        if self.agent_token is not None:
+            return self.agent_token == token
+
+        return False
+
     def _get_route_source(self, dest):
         """Get the IP address to send packages to destination."""
         try:
@@ -419,6 +444,25 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
                 if config.get('metrics_statsd'):
                     for opt, val in config.items():
                         setattr(cfg.CONF.metrics_statsd, opt, val)
+                token = config.get('agent_token')
+                if token:
+                    if len(token) >= 32:
+                        LOG.debug('Agent token recorded as designated by '
+                                  'the ironic installation.')
+                        self.agent_token = token
+                        # set with-in the API client.
+                        self.api_client.agent_token = token
+                    elif token == '******':
+                        LOG.warning('The agent token has already been '
+                                    'retrieved. IPA may not operate as '
+                                    'intended and the deployment may fail '
+                                    'depending on settings in the ironic '
+                                    'deployment.')
+                    else:
+                        LOG.info('An invalid token was received.')
+                if config.get('agent_token_required'):
+                    self.agent_token_required = True
+
             elif cfg.CONF.inspection_callback_url:
                 LOG.info('No ipa-api-url configured, Heartbeat and lookup '
                          'skipped for inspector.')

ironic_python_agent/api/app.py

@@ -214,11 +214,13 @@ class Application(object):
                 or not isinstance(body['params'], dict)):
             raise http_exc.BadRequest('Missing or invalid name or params')
 
+        token = request.args.get('agent_token', None)
+        if not self.agent.validate_agent_token(token):
+            raise http_exc.Unauthorized(
+                'Token invalid.')
         with metrics_utils.get_metrics_logger(__name__).timer('run_command'):
             result = self.agent.execute_command(body['name'], **body['params'])
             wait = request.args.get('wait')
-
             if wait and wait.lower() == 'true':
                 result.join()
-
             return jsonify(result)

ironic_python_agent/cmd/agent.py

@@ -45,4 +45,5 @@ def run():
                             CONF.lookup_timeout,
                             CONF.lookup_interval,
                             CONF.standalone,
+                            CONF.agent_token,
                             CONF.hardware_initialization_delay).run()

ironic_python_agent/config.py

@@ -228,6 +228,17 @@ cli_opts = [
                 default=False,
                 help='If operations should fail if the clock time sync '
                      'fails to complete successfully.'),
+    cfg.StrOpt('agent_token',
+               default=APARAMS.get('ipa-agent-token'),
+               help='Pre-shared token to use when working with the '
+                    'ironic API. This value is typically supplied by '
+                    'ironic automatically.'),
+    cfg.BoolOpt('agent_token_required',
+                default=APARAMS.get('ipa-agent-token-required', False),
+                help='Control to enforce if API command requests should '
+                     'enforce token validation. The configuration provided '
+                     'by the conductor MAY override this and force this '
+                     'setting to be changed to True in memory.'),
 ]
 
 CONF.register_cli_opts(cli_opts)

ironic_python_agent/ironic_api_client.py

@@ -13,6 +13,8 @@
 # limitations under the License.
 
 
+from distutils.version import StrictVersion
+
 from oslo_config import cfg
 from oslo_log import log
 from oslo_serialization import jsonutils
@@ -29,8 +31,10 @@ from ironic_python_agent import version
 CONF = cfg.CONF
 LOG = log.getLogger(__name__)
 
+# TODO(TheJulia): This should be increased at some point.
 MIN_IRONIC_VERSION = (1, 22)
 AGENT_VERSION_IRONIC_VERSION = (1, 36)
+AGENT_TOKEN_IRONIC_VERSION = (1, 62)
 
 
 class APIClient(object):
@@ -38,6 +42,7 @@ class APIClient(object):
     lookup_api = '/%s/lookup' % api_version
     heartbeat_api = '/%s/heartbeat/{uuid}' % api_version
     _ironic_api_version = None
+    agent_token = None
 
     def __init__(self, api_url):
         self.api_url = api_url.rstrip('/')
@@ -74,8 +79,16 @@ class APIClient(object):
                                     **kwargs)
 
     def _get_ironic_api_version_header(self, version=MIN_IRONIC_VERSION):
-        version_str = "%d.%d" % version
+        # TODO(TheJulia): It would be great to improve version handling
-        return {'X-OpenStack-Ironic-API-Version': version_str}
+        # logic, but we need to ship a newer version if we can.
+        ironic_version = "%d.%d" % self._get_ironic_api_version()
+        agent_token_version = "%d.%d" % AGENT_TOKEN_IRONIC_VERSION
+        if (StrictVersion(ironic_version)
+                >= StrictVersion(agent_token_version)):
+            version = agent_token_version
+        else:
+            version = ironic_version
+        return {'X-OpenStack-Ironic-API-Version': version}
 
     def _get_ironic_api_version(self):
         if not self._ironic_api_version:
@@ -97,7 +110,12 @@ class APIClient(object):
 
         data = {'callback_url': self._get_agent_url(advertise_address)}
 
-        if self._get_ironic_api_version() >= AGENT_VERSION_IRONIC_VERSION:
+        api_ver = self._get_ironic_api_version()
+
+        if api_ver >= AGENT_TOKEN_IRONIC_VERSION:
+            data['agent_token'] = self.agent_token
+
+        if api_ver >= AGENT_VERSION_IRONIC_VERSION:
             data['agent_version'] = version.version_info.release_string()
             headers = self._get_ironic_api_version_header(
                 AGENT_VERSION_IRONIC_VERSION)

ironic_python_agent/tests/functional/base.py

@@ -48,7 +48,8 @@ class FunctionalBase(test_base.BaseTestCase):
             network_interface=None,
             lookup_timeout=300,
             lookup_interval=1,
-            standalone=True)
+            standalone=True,
+            agent_token=None)
         self.process = multiprocessing.Process(
             target=self.agent.run)
         self.process.start()

ironic_python_agent/tests/unit/test_agent.py

@@ -146,6 +146,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
                                              'eth0',
                                              300,
                                              1,
+                                             None,
                                              False)
         self.agent.ext_mgr = extension.ExtensionManager.\
             make_test_instance([extension.Extension('fake', None,
@@ -239,7 +240,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
                                              'eth0',
                                              300,
                                              1,
-                                             False)
+                                             False,
+                                             None)
 
         def set_serve_api():
             self.agent.serve_api = False
@@ -296,7 +298,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
                                              'eth0',
                                              300,
                                              1,
-                                             False)
+                                             False,
+                                             None)
 
         def set_serve_api():
             self.agent.serve_api = False
@@ -327,6 +330,51 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
         # changed via mdns
         self.assertEqual(42, CONF.disk_wait_attempts)
 
+    @mock.patch(
+        'ironic_python_agent.hardware_managers.cna._detect_cna_card',
+        mock.Mock())
+    @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
+    @mock.patch.object(agent.IronicPythonAgent,
+                       '_wait_for_interface', autospec=True)
+    @mock.patch('oslo_service.wsgi.Server', autospec=True)
+    @mock.patch.object(hardware, 'get_managers', autospec=True)
+    def test_run_agent_token(self, mock_get_managers, mock_wsgi,
+                             mock_wait, mock_dispatch):
+        CONF.set_override('inspection_callback_url', '')
+
+        wsgi_server = mock_wsgi.return_value
+
+        def set_serve_api():
+            self.agent.serve_api = False
+
+        wsgi_server.start.side_effect = set_serve_api
+        self.agent.heartbeater = mock.Mock()
+        self.agent.api_client.lookup_node = mock.Mock()
+        self.agent.api_client.lookup_node.return_value = {
+            'node': {
+                'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
+            },
+            'config': {
+                'heartbeat_timeout': 300,
+                'agent_token': '1' * 128,
+                'agent_token_required': True
+            }
+        }
+
+        self.agent.run()
+
+        mock_wsgi.assert_called_once_with(CONF, 'ironic-python-agent',
+                                          app=self.agent.api,
+                                          host=mock.ANY, port=9999)
+        wsgi_server.start.assert_called_once_with()
+        mock_wait.assert_called_once_with(mock.ANY)
+        self.assertEqual([mock.call('list_hardware_info'),
+                          mock.call('wait_for_disks')],
+                         mock_dispatch.call_args_list)
+        self.agent.heartbeater.start.assert_called_once_with()
+        self.assertEqual('1' * 128, self.agent.agent_token)
+        self.assertEqual('1' * 128, self.agent.api_client.agent_token)
+
     @mock.patch('eventlet.sleep', autospec=True)
     @mock.patch(
         'ironic_python_agent.hardware_managers.cna._detect_cna_card',
@@ -449,7 +497,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
                                              'eth0',
                                              300,
                                              1,
-                                             False)
+                                             False,
+                                             None)
         self.assertFalse(hasattr(self.agent, 'api_client'))
         self.assertFalse(hasattr(self.agent, 'heartbeater'))
 
@@ -504,7 +553,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
                                              'eth0',
                                              300,
                                              1,
-                                             False)
+                                             False,
+                                             None)
         self.assertFalse(hasattr(self.agent, 'api_client'))
         self.assertFalse(hasattr(self.agent, 'heartbeater'))
 
@@ -704,6 +754,7 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest):
                                              300,
                                              1,
                                              'agent_ipmitool',
+                                             None,
                                              True)
 
     @mock.patch(
@@ -756,6 +807,7 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
             network_interface=None,
             lookup_timeout=300,
             lookup_interval=1,
+            agent_token=None,
             standalone=False)
 
     def test_advertise_address_provided(self, mock_exec, mock_gethostbyname):

ironic_python_agent/tests/unit/test_api.py

@@ -185,6 +185,7 @@ class TestIronicAPI(ironic_agent_base.IronicAgentTest):
 
         self.assertEqual(200, response.status_code)
         self.assertEqual(1, self.mock_agent.execute_command.call_count)
+        self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
         args, kwargs = self.mock_agent.execute_command.call_args
         self.assertEqual(('do_things',), args)
         self.assertEqual({'key': 'value'}, kwargs)
@@ -211,6 +212,7 @@ class TestIronicAPI(ironic_agent_base.IronicAgentTest):
 
         self.assertEqual(200, response.status_code)
         self.assertEqual(1, self.mock_agent.execute_command.call_count)
+        self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
         args, kwargs = self.mock_agent.execute_command.call_args
         self.assertEqual(('do_things',), args)
         self.assertEqual({'key': 'value'}, kwargs)
@@ -272,3 +274,56 @@ class TestIronicAPI(ironic_agent_base.IronicAgentTest):
         self.assertEqual(200, response.status_code)
         data = response.json
         self.assertEqual(serialized_cmd_result, data)
+
+    def test_execute_agent_command_with_token(self):
+        agent_token = str('0123456789' * 10)
+        command = {
+            'name': 'do_things',
+            'params': {'key': 'value',
+                       'wait': False,
+                       'agent_token': agent_token},
+        }
+
+        result = base.SyncCommandResult(command['name'],
+                                        command['params'],
+                                        True,
+                                        {'test': 'result'})
+
+        self.mock_agent.validate_agent_token.return_value = True
+        self.mock_agent.execute_command.return_value = result
+
+        with mock.patch.object(result, 'join', autospec=True) as join_mock:
+            response = self.post_json(
+                '/commands?wait=false?agent_token=%s' % agent_token,
+                command)
+            self.assertFalse(join_mock.called)
+
+        self.assertEqual(200, response.status_code)
+
+        self.assertEqual(1, self.mock_agent.execute_command.call_count)
+        self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
+        args, kwargs = self.mock_agent.execute_command.call_args
+        self.assertEqual(('do_things',), args)
+        expected_result = result.serialize()
+        data = response.json
+        self.assertEqual(expected_result, data)
+
+    def test_execute_agent_command_with_token_invalid(self):
+        agent_token = str('0123456789' * 10)
+        command = {
+            'name': 'do_things',
+            'params': {'key': 'value',
+                       'wait': False,
+                       'agent_token': agent_token},
+        }
+
+        self.mock_agent.validate_agent_token.return_value = False
+        response = self.post_json(
+            '/commands?wait=false?agent_token=%s' % agent_token,
+            command,
+            expect_errors=True)
+
+        self.assertEqual(401, response.status_code)
+
+        self.assertEqual(0, self.mock_agent.execute_command.call_count)
+        self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)

ironic_python_agent/tests/unit/test_ironic_api_client.py

@@ -151,6 +151,31 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest):
             'agent_version': version.version_info.release_string()}
         self.assertEqual(jsonutils.dumps(expected_data), data)
 
+    def test_successful_heartbeat_with_token(self):
+        response = FakeResponse(status_code=202)
+
+        self.api_client.session.request = mock.Mock()
+        self.api_client.session.request.return_value = response
+        self.api_client._ironic_api_version = (
+            ironic_api_client.AGENT_TOKEN_IRONIC_VERSION)
+        self.api_client.agent_token = 'magical'
+
+        self.api_client.heartbeat(
+            uuid='deadbeef-dabb-ad00-b105-f00d00bab10c',
+            advertise_address=('192.0.2.1', '9999')
+        )
+
+        heartbeat_path = 'v1/heartbeat/deadbeef-dabb-ad00-b105-f00d00bab10c'
+        request_args = self.api_client.session.request.call_args[0]
+        data = self.api_client.session.request.call_args[1]['data']
+        self.assertEqual('POST', request_args[0])
+        self.assertEqual(API_URL + heartbeat_path, request_args[1])
+        expected_data = {
+            'callback_url': 'http://192.0.2.1:9999',
+            'agent_token': 'magical',
+            'agent_version': version.version_info.release_string()}
+        self.assertEqual(jsonutils.dumps(expected_data), data)
+
     def test_heartbeat_agent_version_unsupported(self):
         response = FakeResponse(status_code=202)
 

releasenotes/notes/agent-token-support-1086218cf2a0c917.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Adds support for the agent to receive, store, and return
+    an ``agent token`` from the Ironic deployment to help secure
+    use of the ironic API ``/v1/heartbeat`` endpoint, as well as
+    the API of the ironic-python-agent ramdisk.