openstack/ironic
Code Issues Proposed changes

Pre-shared agent token

Browse Source 
In order to improve security of the lookup/heartbeat
endpoints, we need to generate and provide temporary tokens
to the initial callers, if supported, to facilitate the
verification of commands.

This is the first patch in an entire series which utimately
enables the endpoint communication to be better secured.

The idea behind this started in private story 2006634 which
is locked as a security related filing covering multiple
aspects of ironic/ironic-python-agent interaction centered
around miss-use and generally exposed endpoints. That story
will remain marked as a private bug because it has several
different items covered, some of which did not prove to be
actually exploitable, but spawned stories 2006777, 2006773,
2007025, and is ultimately similar to Story 1526748.

Operationally this is a minimally invasive security
enhancement to lay the foundation to harden interactions
with the agent. This will take place over a series of
patches to both Ironic and the Ironic-Python-Agent.

Also see "Security of /heartbeat and /lookup endpoints"
in http://lists.openstack.org/pipermail/openstack-discuss/2019-November/010789.html

Story: 2007025
Task: 37818

Change-Id: I0118007cac3d6548e9d41c5e615a819150b6ef1a
This commit is contained in:
Julia Kreger 2019-11-01 13:29:36 -07:00
parent d86b0f61d7
commit bb3b2349f9
26 changed files with 637 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/source/contributor/webapi-version-history.rst
View File

@ -2,6 +2,13 @@
REST API Version History
========================
1.62 (Ussuri, master)
---------------------
This version of the API is to signify capability of an ironic deployment
to support the ``agent token`` functionality with the
``ironic-python-agent``.
1.61 (Ussuri, master)
---------------------

3
ironic/api/controllers/v1/node.py
View File

@ -1245,6 +1245,9 @@ class Node(base.APIBase):
if self.instance_info.get('image_url'):
self.instance_info['image_url'] = "******"
if self.driver_internal_info.get('agent_secret_token'):
self.driver_internal_info['agent_secret_token'] = "******"
update_state_in_older_versions(self)
hide_fields_in_newer_versions(self)

36
ironic/api/controllers/v1/ramdisk.py
View File

@ -39,7 +39,7 @@ _LOOKUP_RETURN_FIELDS = ('uuid', 'properties', 'instance_info',
'driver_internal_info')
def config():
def config(token):
return {
'metrics': {
'backend': CONF.metrics.agent_backend,
@ -52,7 +52,8 @@ def config():
'statsd_host': CONF.metrics_statsd.agent_statsd_host,
'statsd_port': CONF.metrics_statsd.agent_statsd_port
},
'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout
'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout,
'agent_token': token
}
@ -72,8 +73,9 @@ class LookupResult(base.APIBase):
@classmethod
def convert_with_links(cls, node):
token = node.driver_internal_info.get('agent_secret_token')
node = node_ctl.Node.convert_with_links(node, _LOOKUP_RETURN_FIELDS)
return cls(node=node, config=config())
return cls(node=node, config=config(token))
class LookupController(rest.RestController):
@ -149,15 +151,27 @@ class LookupController(rest.RestController):
and node.provision_state not in self.lookup_allowed_states):
raise exception.NotFound()
return LookupResult.convert_with_links(node)
if api_utils.allow_agent_token() or CONF.require_agent_token:
try:
topic = api.request.rpcapi.get_topic_for(node)
except exception.NoValidHost as e:
e.code = http_client.BAD_REQUEST
raise
found_node = api.request.rpcapi.get_node_with_token(
api.request.context, node.uuid, topic=topic)
else:
found_node = node
return LookupResult.convert_with_links(found_node)
class HeartbeatController(rest.RestController):
"""Controller handling heartbeats from deploy ramdisk."""
@expose.expose(None, types.uuid_or_name, str,
str, status_code=http_client.ACCEPTED)
def post(self, node_ident, callback_url, agent_version=None):
str, str, status_code=http_client.ACCEPTED)
def post(self, node_ident, callback_url, agent_version=None,
agent_token=None):
"""Process a heartbeat from the deploy ramdisk.
:param node_ident: the UUID or logical name of a node.
@ -197,6 +211,14 @@ class HeartbeatController(rest.RestController):
raise exception.Invalid(
_('Detected change in ramdisk provided '
'"callback_url"'))
# NOTE(TheJulia): If tokens are required, lets go ahead and fail the
# heartbeat very early on.
token_required = CONF.require_agent_token
if token_required and agent_token is None:
LOG.error('Agent heartbeat received for node %(node)s '
'without an agent token.', {'node': node_ident})
raise exception.InvalidParameterValue(
_('Agent token is required for heartbeat processing.'))
try:
topic = api.request.rpcapi.get_topic_for(rpc_node)
@ -206,4 +228,4 @@ class HeartbeatController(rest.RestController):
api.request.rpcapi.heartbeat(
api.request.context, rpc_node.uuid, callback_url,
agent_version, topic=topic)
agent_version, agent_token, topic=topic)

5
ironic/api/controllers/v1/utils.py
View File

@ -1316,3 +1316,8 @@ def allow_allocation_owner():
Version 1.60 of the API added the owner field to the allocation object.
"""
return api.request.version.minor >= versions.MINOR_60_ALLOCATION_OWNER
def allow_agent_token():
"""Check if agent token is available."""
return api.request.version.minor >= versions.MINOR_62_AGENT_TOKEN

4
ironic/api/controllers/v1/versions.py
View File

@ -99,6 +99,7 @@ BASE_VERSION = 1
# v1.59: Add support vendor data in configdrives.
# v1.60: Add owner to the allocation object.
# v1.61: Add retired and retired_reason to the node object.
# v1.62: Add agent_token support for agent communication.
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -162,6 +163,7 @@ MINOR_58_ALLOCATION_BACKFILL = 58
MINOR_59_CONFIGDRIVE_VENDOR_DATA = 59
MINOR_60_ALLOCATION_OWNER = 60
MINOR_61_NODE_RETIRED = 61
MINOR_62_AGENT_TOKEN = 62
# When adding another version, update:
# - MINOR_MAX_VERSION
@ -169,7 +171,7 @@ MINOR_61_NODE_RETIRED = 61
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
MINOR_MAX_VERSION = MINOR_61_NODE_RETIRED
MINOR_MAX_VERSION = MINOR_62_AGENT_TOKEN
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

12
ironic/cmd/conductor.py
View File

@ -54,9 +54,21 @@ def warn_about_missing_default_boot_option(conf):
'an explicit value for it during the transition period')
def warn_about_agent_token_deprecation(conf):
if not conf.require_agent_token:
LOG.warning('The ``[DEFAULT]require_agent_token`` option is not '
'set and support for ironic-python-agents that do not '
'utilize agent tokens, along with the configuration '
'option will be removed in the W development cycle. '
'Please upgrade your ironic-python-agent version, and '
'consider adopting the require_agent_token setting '
'during the Victoria development cycle.')
def issue_startup_warnings(conf):
warn_about_unsafe_shred_parameters(conf)
warn_about_missing_default_boot_option(conf)
warn_about_agent_token_deprecation(conf)
def main():

4
ironic/common/release_mappings.py
View File

@ -214,8 +214,8 @@ RELEASE_MAPPING = {
}
},
'master': {
'api': '1.61',
'rpc': '1.48',
'api': '1.62',
'rpc': '1.49',
'objects': {
'Allocation': ['1.1'],
'Node': ['1.33', '1.32'],

5
ironic/conductor/cleaning.py
View File

@ -213,6 +213,9 @@ def do_next_clean_step(task, step_index):
driver_internal_info.pop('clean_step_index', None)
driver_internal_info.pop('cleaning_reboot', None)
driver_internal_info.pop('cleaning_polling', None)
driver_internal_info.pop('agent_secret_token', None)
driver_internal_info.pop('agent_secret_token_pregenerated', None)
# Remove agent_url
if not utils.fast_track_able(task):
driver_internal_info.pop('agent_url', None)
@ -271,6 +274,8 @@ def do_node_clean_abort(task, step_name=None):
info.pop('cleaning_polling', None)
info.pop('skip_current_clean_step', None)
info.pop('agent_url', None)
info.pop('agent_secret_token', None)
info.pop('agent_secret_token_pregenerated', None)
node.driver_internal_info = info
node.save()
LOG.info(info_message)

4
ironic/conductor/deployments.py
View File

@ -133,7 +133,7 @@ def start_deploy(task, manager, configdrive=None, event='deploy'):
def do_node_deploy(task, conductor_id=None, configdrive=None):
"""Prepare the environment and deploy a node."""
node = task.node
utils.del_secret_token(node)
try:
if configdrive:
if isinstance(configdrive, dict):
@ -392,6 +392,8 @@ def do_next_deploy_step(task, step_index, conductor_id):
# Finished executing the steps. Clear deploy_step.
node.deploy_step = None
driver_internal_info = node.driver_internal_info
driver_internal_info.pop('agent_secret_token', None)
driver_internal_info.pop('agent_secret_token_pregenerated', None)
driver_internal_info['deploy_steps'] = None
driver_internal_info.pop('deploy_step_index', None)
driver_internal_info.pop('deployment_reboot', None)

76
ironic/conductor/manager.py
View File

@ -90,7 +90,7 @@ class ConductorManager(base_manager.BaseConductorManager):
# NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's.
# NOTE(pas-ha): This also must be in sync with
# ironic.common.release_mappings.RELEASE_MAPPING['master']
RPC_API_VERSION = '1.48'
RPC_API_VERSION = '1.49'
target = messaging.Target(version=RPC_API_VERSION)
@ -1008,6 +1008,8 @@ class ConductorManager(base_manager.BaseConductorManager):
node.instance_info = {}
node.instance_uuid = None
driver_internal_info = node.driver_internal_info
driver_internal_info.pop('agent_secret_token', None)
driver_internal_info.pop('agent_secret_token_pregenerated', None)
driver_internal_info.pop('instance', None)
driver_internal_info.pop('clean_steps', None)
driver_internal_info.pop('root_uuid_or_disk_id', None)
@ -2924,7 +2926,8 @@ class ConductorManager(base_manager.BaseConductorManager):
@METRICS.timer('ConductorManager.heartbeat')
@messaging.expected_exceptions(exception.NoFreeConductorWorker)
def heartbeat(self, context, node_id, callback_url, agent_version=None):
def heartbeat(self, context, node_id, callback_url, agent_version=None,
agent_token=None):
"""Process a heartbeat from the ramdisk.
:param context: request context.
@ -2945,10 +2948,43 @@ class ConductorManager(base_manager.BaseConductorManager):
if agent_version is None:
agent_version = '3.0.0'
token_required = CONF.require_agent_token
# NOTE(dtantsur): we acquire a shared lock to begin with, drivers are
# free to promote it to an exclusive one.
with task_manager.acquire(context, node_id, shared=True,
purpose='heartbeat') as task:
# NOTE(TheJulia): The "token" line of defense.
# either tokens are required and they are present,
# or a token is present in general and needs to be
# validated.
if token_required or utils.is_agent_token_present(task.node):
if not utils.is_agent_token_valid(task.node, agent_token):
LOG.error('Invalid agent_token receieved for node '
'%(node)s', {'node': node_id})
raise exception.InvalidParameterValue(
'Invalid or missing agent token received.')
elif utils.is_agent_token_supported(agent_version):
LOG.error('Suspicious activity detected for node %(node)s '
'when attempting to heartbeat. Heartbeat '
'request has been rejected as the version of '
'ironic-python-agent indicated in the heartbeat '
'operation should support agent token '
'functionality.',
{'node': task.node.uuid})
raise exception.InvalidParameterValue(
'Invalid or missing agent token received.')
else:
LOG.warning('Out of date agent detected for node '
'%(node)s. Agent version %(version) '
'reported. Support for this version is '
'deprecated.',
{'node': task.node.uuid,
'version': agent_version})
# TODO(TheJulia): raise an exception as of the
# ?Victoria? development cycle.
task.spawn_after(
self._spawn_worker, task.driver.deploy.heartbeat,
task, callback_url, agent_version)
@ -3301,6 +3337,42 @@ class ConductorManager(base_manager.BaseConductorManager):
LOG.exception('Unexpected exception when taking over '
'allocation %s', allocation.uuid)
@METRICS.timer('ConductorManager.get_node_with_token')
@messaging.expected_exceptions(exception.NodeLocked,
exception.Invalid)
def get_node_with_token(self, context, node_id):
"""Add secret agent token to node.
:param context: request context.
:param node_id: node ID or UUID.
:returns: Secret token set for the node.
:raises: NodeLocked, if node has an exclusive lock held on it
:raises: Invalid, if the node already has a token set.
"""
LOG.debug("RPC get_node_with_token called for the node %(node_id)s",
{'node_id': node_id})
with task_manager.acquire(context, node_id,
purpose='generate_token',
shared=True) as task:
node = task.node
if utils.is_agent_token_present(task.node):
LOG.warning('An agent token generation request is being '
'refused as one is already present for '
'node %(node)s',
{'node': node_id})
# Allow lookup to work by returning a value, it is just an
# unusable value that can't be verified against.
# This is important if the agent lookup has occured with
# pre-generation of tokens with virtual media usage.
node.driver_internal_info['agent_secret_token'] = "******"
return node
task.upgrade_lock()
LOG.debug('Generating agent token for node %(node)s',
{'node': task.node.uuid})
utils.add_secret_token(task.node)
task.node.save()
return task.node
@METRICS.timer('get_vendor_passthru_metadata')
def get_vendor_passthru_metadata(route_dict):

22
ironic/conductor/rpcapi.py
View File

@ -99,13 +99,15 @@ class ConductorAPI(object):
| 1.46 - Added reset_interfaces to update_node
| 1.47 - Added support for conductor groups
| 1.48 - Added allocation API
| 1.49 - Added get_node_with_token and agent_token argument to
heartbeat
"""
# NOTE(rloo): This must be in sync with manager.ConductorManager's.
# NOTE(pas-ha): This also must be in sync with
# ironic.common.release_mappings.RELEASE_MAPPING['master']
RPC_API_VERSION = '1.48'
RPC_API_VERSION = '1.49'
def __init__(self, topic=None):
super(ConductorAPI, self).__init__()
@ -811,7 +813,7 @@ class ConductorAPI(object):
node_id=node_id, clean_steps=clean_steps)
def heartbeat(self, context, node_id, callback_url, agent_version,
topic=None):
agent_token=None, topic=None):
"""Process a node heartbeat.
:param context: request context.
@ -825,6 +827,9 @@ class ConductorAPI(object):
if self.client.can_send_version('1.42'):
version = '1.42'
new_kws['agent_version'] = agent_version
if self.client.can_send_version('1.49'):
version = '1.49'
new_kws['agent_token'] = agent_token
cctxt = self.client.prepare(topic=topic or self.topic, version=version)
return cctxt.call(context, 'heartbeat', node_id=node_id,
callback_url=callback_url, **new_kws)
@ -1133,3 +1138,16 @@ class ConductorAPI(object):
"""
cctxt = self.client.prepare(topic=topic or self.topic, version='1.48')
return cctxt.call(context, 'destroy_allocation', allocation=allocation)
def get_node_with_token(self, context, node_id, topic=None):
"""Request the node from the conductor with an agent token
:param context: request context.
:param node_id: node ID or UUID.
:param topic: RPC topic. Defaults to self.topic.
:raises: NodeLocked if node is locked by another conductor.
:returns: A Node object with agent token.
"""
cctxt = self.client.prepare(topic=topic or self.topic, version='1.49')
return cctxt.call(context, 'get_node_with_token', node_id=node_id)

94
ironic/conductor/utils.py
View File

@ -14,6 +14,9 @@
import contextlib
import datetime
from distutils.version import StrictVersion
import random
import string
import time
from openstack.baremetal import configdrive as os_configdrive
@ -1005,3 +1008,94 @@ def get_node_next_clean_steps(task, skip_current_step=True):
def get_node_next_deploy_steps(task, skip_current_step=True):
return _get_node_next_steps(task, 'deploy',
skip_current_step=skip_current_step)
def add_secret_token(node):
"""Adds a secret token to driver_internal_info for IPA verification."""
characters = string.ascii_letters + string.digits
token = ''.join(
random.SystemRandom().choice(characters) for i in range(128))
i_info = node.driver_internal_info
i_info['agent_secret_token'] = token
node.driver_internal_info = i_info
def del_secret_token(node):
"""Deletes the IPA agent secret token.
Removes the agent token secret from the driver_internal_info field
from the Node object.
:param node: Node object
"""
i_info = node.driver_internal_info
i_info.pop('agent_secret_token', None)
node.driver_internal_info = i_info
def is_agent_token_present(node):
"""Determines if an agent token is present upon a node.
:param node: Node object
:returns: True if an agent_secret_token value is present in a node
driver_internal_info field.
"""
# TODO(TheJulia): we should likely record the time when we add the token
# and then compare if it was in the last ?hour? to act as an additional
# guard rail, but if we do that we will want to check the last heartbeat
# because the heartbeat overrides the age of the token.
# We may want to do this elsewhere or nowhere, just a thought for the
# future.
return node.driver_internal_info.get(
'agent_secret_token', None) is not None
def is_agent_token_valid(node, token):
"""Validates if a supplied token is valid for the node.
:param node: Node object
:token: A token value to validate against the driver_internal_info field
agent_sercret_token.
:returns: True if the supplied token matches the token recorded in the
supplied node object.
"""
if token is None:
# No token is never valid.
return False
known_token = node.driver_internal_info.get('agent_secret_token', None)
return known_token == token
def is_agent_token_supported(agent_version):
# NOTE(TheJulia): This is hoped that 6.x supports
# agent token capabilities and realistically needs to be updated
# once that version of IPA is out there in some shape or form.
# This allows us to gracefully allow older agent's that were
# launched via pre-generated agent_tokens, to still work
# and could likely be removed at some point down the road.
version = str(agent_version).replace('.dev', 'b', 1)
return StrictVersion(version) > StrictVersion('6.1.0')
def is_agent_token_pregenerated(node):
"""Determines if the token was generated for out of band configuration.
Ironic supports the ability to provide configuration data to the agent
through the a virtual floppy or as part of the virtual media image
which is attached to the BMC.
This method helps us identify WHEN we did so as we don't need to remove
records of the token prior to rebooting the token. This is important as
tokens provided through out of band means presist in the virtual media
image, are loaded as part of the agent ramdisk, and do not require
regeneration of the token upon the initial lookup, ultimately making
the overall usage of virtual media and pregenerated tokens far more
secure.
:param node: Node Object
:returns: True if the token was pregenerated as indicated by the node's
driver_internal_info field.
False in all other cases.
"""
return node.driver_internal_info.get(
'agent_secret_token_pregenerated', False)

9
ironic/conf/default.py
View File

@ -327,6 +327,15 @@ service_opts = [
('json-rpc', _('use JSON RPC transport'))],
help=_('Which RPC transport implementation to use between '
'conductor and API services')),
cfg.BoolOpt('require_agent_token',
default=False,
help=_('Used to require the use of agent tokens. These '
'tokens are used to guard the api lookup endpoint and '
'conductor heartbeat processing logic to authenticate '
'transactions with the ironic-python-agent. Tokens '
'are provided only upon the first lookup of a node '
'and may be provided via out of band means through '
'the use of virtual media.')),
]
utils_opts = [

4
ironic/drivers/modules/agent_base.py
View File

@ -166,6 +166,10 @@ def _cleaning_reboot(task):
# Signify that we've rebooted
driver_internal_info = task.node.driver_internal_info
driver_internal_info['cleaning_reboot'] = True
if not driver_internal_info.get('agent_secret_token_pregenerated', False):
# Wipes out the existing recorded token because the machine will
# need to re-establish the token.
driver_internal_info.pop('agent_secret_token', None)
task.node.driver_internal_info = driver_internal_info
task.node.save()

22
ironic/drivers/modules/deploy_utils.py
View File

@ -1486,6 +1486,24 @@ def get_async_step_return_state(node):
return states.CLEANWAIT if node.clean_step else states.DEPLOYWAIT
def _check_agent_token_prior_to_agent_reboot(driver_internal_info):
"""Removes the agent token if it was not pregenerated.
Removal of the agent token in cases where it is not pregenerated
is a vital action prior to rebooting the agent, as without doing
so the agent will be unable to establish communication with
the ironic API after the reboot. Effectively locking itself out
as in cases where the value is not pregenerated, it is not
already included in the payload and must be generated again
upon lookup.
:param driver_internal_info: The driver_interal_info dict object
from a Node object.
"""
if not driver_internal_info.get('agent_secret_token_pregenerated', False):
driver_internal_info.pop('agent_secret_token', None)
def set_async_step_flags(node, reboot=None, skip_current_step=None,
polling=None):
"""Sets appropriate reboot flags in driver_internal_info based on operation
@ -1515,6 +1533,10 @@ def set_async_step_flags(node, reboot=None, skip_current_step=None,
fields = cleaning if node.clean_step else deployment
if reboot is not None:
info[fields['reboot']] = reboot
if reboot:
# If rebooting, we must ensure that we check and remove
# an agent token if necessary.
_check_agent_token_prior_to_agent_reboot(info)
if skip_current_step is not None:
info[fields['skip']] = skip_current_step
if polling is not None:

2
ironic/objects/node.py
View File

@ -171,6 +171,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
d.get('driver_info', {}), "******")
d['instance_info'] = strutils.mask_dict_password(
d.get('instance_info', {}), "******")
d['driver_internal_info'] = strutils.mask_dict_password(
d.get('driver_internal_info', {}), "******")
return d
def _validate_property_values(self, properties):

9
ironic/tests/unit/api/controllers/v1/test_node.py
View File

@ -605,6 +605,15 @@ class TestListNodes(test_api_base.BaseApiTest):
headers={api_base.Version.string: '1.61'})
self.assertIn('retired', response)
def test_get_one_with_no_agent_secret(self):
node = obj_utils.create_test_node(
self.context,
driver_internal_info={'agent_secret_token': 'abcdefg'})
response = self.get_json('/nodes/%s' % (node.uuid),
headers={api_base.Version.string: '1.52'})
token_value = response['driver_internal_info']['agent_secret_token']
self.assertEqual('******', token_value)
def test_detail(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)

44
ironic/tests/unit/api/controllers/v1/test_ramdisk.py
View File

@ -50,6 +50,16 @@ class TestLookup(test_api_base.BaseApiTest):
fixtures.MockPatchObject(rpcapi.ConductorAPI, 'get_conductor_for',
autospec=True)).mock
self.mock_get_conductor_for.return_value = 'fake.conductor'
self.mock_get_node_with_token = self.useFixture(
fixtures.MockPatchObject(rpcapi.ConductorAPI,
'get_node_with_token',
autospec=True)).mock
def _set_secret_mock(self, node, token_value):
driver_internal = node.driver_internal_info
driver_internal['agent_secret_token'] = token_value
node.driver_internal_info = driver_internal
self.mock_get_node_with_token.return_value = node
def _check_config(self, data):
expected_metrics = {
@ -65,9 +75,12 @@ class TestLookup(test_api_base.BaseApiTest):
'statsd_host': CONF.metrics_statsd.agent_statsd_host,
'statsd_port': CONF.metrics_statsd.agent_statsd_port
},
'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout
'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout,
'agent_token': mock.ANY
}
self.assertEqual(expected_metrics, data['config'])
self.assertIsNotNone(data['config']['agent_token'])
self.assertNotEqual('******', data['config']['agent_token'])
def test_nothing_provided(self):
response = self.get_json(
@ -95,6 +108,7 @@ class TestLookup(test_api_base.BaseApiTest):
self.assertEqual(http_client.NOT_FOUND, response.status_int)
def test_found_by_addresses(self):
self._set_secret_mock(self.node, 'some-value')
obj_utils.create_test_port(self.context,
node_id=self.node.id,
address=self.addresses[1])
@ -109,6 +123,7 @@ class TestLookup(test_api_base.BaseApiTest):
@mock.patch.object(ramdisk.LOG, 'warning', autospec=True)
def test_ignore_malformed_address(self, mock_log):
self._set_secret_mock(self.node, '123456')
obj_utils.create_test_port(self.context,
node_id=self.node.id,
address=self.addresses[1])
@ -125,6 +140,7 @@ class TestLookup(test_api_base.BaseApiTest):
self.assertTrue(mock_log.called)
def test_found_by_uuid(self):
self._set_secret_mock(self.node, 'this_thing_on?')
data = self.get_json(
'/lookup?addresses=%s&node_uuid=%s' %
(','.join(self.addresses), self.node.uuid),
@ -135,6 +151,7 @@ class TestLookup(test_api_base.BaseApiTest):
self._check_config(data)
def test_found_by_only_uuid(self):
self._set_secret_mock(self.node, 'xyzabc')
data = self.get_json(
'/lookup?node_uuid=%s' % self.node.uuid,
headers={api_base.Version.string: str(api_v1.max_version())})
@ -153,6 +170,7 @@ class TestLookup(test_api_base.BaseApiTest):
def test_no_restrict_lookup(self):
CONF.set_override('restrict_lookup', False, 'api')
self._set_secret_mock(self.node2, '234567890')
data = self.get_json(
'/lookup?addresses=%s&node_uuid=%s' %
(','.join(self.addresses), self.node2.uuid),
@ -163,6 +181,7 @@ class TestLookup(test_api_base.BaseApiTest):
self._check_config(data)
def test_fast_deploy_lookup(self):
self._set_secret_mock(self.node, 'abcxyz')
CONF.set_override('fast_track', True, 'deploy')
for provision_state in [states.ENROLL, states.MANAGEABLE,
states.AVAILABLE]:
@ -203,7 +222,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
node.uuid, 'url', None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@ -216,7 +235,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
node.uuid, 'url', None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@ -229,7 +248,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
node.uuid, 'url', None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@ -243,7 +262,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', '1.4.1',
node.uuid, 'url', '1.4.1', None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@ -268,3 +287,18 @@ class TestHeartbeat(test_api_base.BaseApiTest):
headers={api_base.Version.string: str(api_v1.max_version())},
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_ok_agent_token(self, mock_heartbeat):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
{'callback_url': 'url',
'agent_token': 'abcdef1'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
'abcdef1',
topic='test-topic')

6
ironic/tests/unit/api/controllers/v1/test_utils.py
View File

@ -565,6 +565,12 @@ class TestCheckAllowFields(base.TestCase):
mock_request.version.minor = 54
self.assertFalse(utils.allow_deploy_templates())
def test_allow_agent_token(self, mock_request):
mock_request.version.minor = 62
self.assertTrue(utils.allow_agent_token())
mock_request.version.minor = 61
self.assertFalse(utils.allow_agent_token())
@mock.patch.object(api, 'request')
class TestNodeIdent(base.TestCase):

3
ironic/tests/unit/conductor/test_deployments.py
View File

@ -358,7 +358,8 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
expected_calls = [
mock.call(node.uuid,
{'version': mock.ANY,
'instance_info': expected_instance_info}),
'instance_info': expected_instance_info,
'driver_internal_info': mock.ANY}),
mock.call(node.uuid,
{'version': mock.ANY,
'last_error': mock.ANY}),

196
ironic/tests/unit/conductor/test_manager.py
View File

@ -6885,6 +6885,10 @@ class DoNodeTakeOverTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
@mgr_utils.mock_record_keepalive
class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
def _fake_spawn(self, conductor_obj, func, *args, **kwargs):
func(*args, **kwargs)
return mock.MagicMock()
@mock.patch('ironic.drivers.modules.fake.FakePower.validate')
@mock.patch('ironic.drivers.modules.fake.FakeBoot.validate')
@mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console')
@ -7031,6 +7035,10 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertEqual(states.NOSTATE, node.target_provision_state)
self.assertIsNone(node.last_error)
# TODO(TheJulia): We should double check if these heartbeat tests need
# to move. I have this strange feeling we were lacking rpc testing of
# heartbeat until we did adoption testing....
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
@ -7046,10 +7054,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.reset_mock()
def fake_spawn(conductor_obj, func, *args, **kwargs):
func(*args, **kwargs)
return mock.MagicMock()
mock_spawn.side_effect = fake_spawn
mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
@ -7070,16 +7075,191 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.reset_mock()
def fake_spawn(conductor_obj, func, *args, **kwargs):
func(*args, **kwargs)
return mock.MagicMock()
mock_spawn.side_effect = fake_spawn
mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(
self.context, node.uuid, 'http://callback', '1.4.1')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '1.4.1')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_with_no_required_agent_token(self, mock_spawn,
mock_heartbeat):
"""Tests that we kill the heartbeat attempt very early on."""
self.config(require_agent_token=True)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE)
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
self.assertRaises(
exception.InvalidParameterValue, self.service.heartbeat,
self.context, node.uuid, 'http://callback', agent_token=None)
self.assertFalse(mock_heartbeat.called)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_with_required_agent_token(self, mock_spawn,
mock_heartbeat):
"""Test heartbeat works when token matches."""
self.config(require_agent_token=True)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE,
driver_internal_info={'agent_secret_token': 'a secret'})
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback',
agent_token='a secret')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '3.0.0')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_with_agent_token(self, mock_spawn,
mock_heartbeat):
"""Test heartbeat works when token matches."""
self.config(require_agent_token=False)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE,
driver_internal_info={'agent_secret_token': 'a secret'})
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback',
agent_token='a secret')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '3.0.0')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_invalid_agent_token(self, mock_spawn,
mock_heartbeat):
"""Heartbeat fails when it does not match."""
self.config(require_agent_token=False)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE,
driver_internal_info={'agent_secret_token': 'a secret'})
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
self.assertRaises(exception.InvalidParameterValue,
self.service.heartbeat, self.context,
node.uuid, 'http://callback',
agent_token='evil', agent_version='5.0.0b23')
self.assertFalse(mock_heartbeat.called)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_invalid_agent_token_older_version(
self, mock_spawn, mock_heartbeat):
"""Heartbeat is rejected if token is received that is invalid."""
self.config(require_agent_token=False)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE,
driver_internal_info={'agent_secret_token': 'a secret'})
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
# Intentionally sending an older client in case something fishy
# occurs.
self.assertRaises(exception.InvalidParameterValue,
self.service.heartbeat, self.context,
node.uuid, 'http://callback',
agent_token='evil', agent_version='4.0.0')
self.assertFalse(mock_heartbeat.called)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_invalid_when_token_on_file_older_agent(
self, mock_spawn, mock_heartbeat):
"""Heartbeat rejected if a token is on file."""
self.config(require_agent_token=False)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE,
driver_internal_info={'agent_secret_token': 'a secret'})
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
self.assertRaises(exception.InvalidParameterValue,
self.service.heartbeat, self.context,
node.uuid, 'http://callback',
agent_token=None, agent_version='4.0.0')
self.assertFalse(mock_heartbeat.called)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)
def test_heartbeat_invalid_newer_version(
self, mock_spawn, mock_heartbeat):
"""Heartbeat rejected if client should be sending a token."""
self.config(require_agent_token=False)
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE)
self._start_service()
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
self.assertRaises(exception.InvalidParameterValue,
self.service.heartbeat, self.context,
node.uuid, 'http://callback',
agent_token=None, agent_version='6.1.5')
self.assertFalse(mock_heartbeat.called)
@mgr_utils.mock_record_keepalive
class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin,

17
ironic/tests/unit/conductor/test_rpcapi.py
View File

@ -486,7 +486,16 @@ class RPCAPITestCase(db_base.DbTestCase):
node_id='fake-node',
callback_url='http://ramdisk.url:port',
agent_version=None,
version='1.42')
version='1.49')
def test_heartbeat_agent_token(self):
self._test_rpcapi('heartbeat',
'call',
node_id='fake-node',
callback_url='http://ramdisk.url:port',
agent_version=None,
agent_token='xyz1',
version='1.49')
def test_destroy_volume_connector(self):
fake_volume_connector = db_utils.get_test_volume_connector()
@ -555,6 +564,12 @@ class RPCAPITestCase(db_base.DbTestCase):
version='1.43',
node_id=self.fake_node['uuid'])
def test_get_node_with_token(self):
self._test_rpcapi('get_node_with_token',
'call',
version='1.49',
node_id=self.fake_node['uuid'])
def _test_can_send_rescue(self, can_send):
rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
with mock.patch.object(rpcapi.client,

34
ironic/tests/unit/conductor/test_utils.py
View File

@ -2018,3 +2018,37 @@ class GetNodeNextStepsTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, node.uuid) as task:
step_index = conductor_utils.get_node_next_clean_steps(task)
self.assertEqual(0, step_index)
class AgentTokenUtilsTestCase(tests_base.TestCase):
def setUp(self):
super(AgentTokenUtilsTestCase, self).setUp()
self.node = obj_utils.get_test_node(self.context,
driver='fake-hardware')
def test_add_secret_token(self):
self.assertNotIn('agent_secret_token', self.node.driver_internal_info)
conductor_utils.add_secret_token(self.node)
self.assertEqual(
128, len(self.node.driver_internal_info['agent_secret_token']))
def test_del_secret_token(self):
conductor_utils.add_secret_token(self.node)
self.assertIn('agent_secret_token', self.node.driver_internal_info)
conductor_utils.del_secret_token(self.node)
self.assertNotIn('agent_secret_token', self.node.driver_internal_info)
def test_is_agent_token_present(self):
# This should always be False as the token has not been added yet.
self.assertFalse(conductor_utils.is_agent_token_present(self.node))
conductor_utils.add_secret_token(self.node)
self.assertTrue(conductor_utils.is_agent_token_present(self.node))
def test_is_agent_token_supported(self):
self.assertTrue(
conductor_utils.is_agent_token_supported('6.1.1.dev39'))
self.assertTrue(
conductor_utils.is_agent_token_supported('6.2.1'))
self.assertFalse(
conductor_utils.is_agent_token_supported('6.0.0'))

24
ironic/tests/unit/drivers/modules/test_agent_base.py
View File

@ -1559,11 +1559,35 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
def test__cleaning_reboot(self, mock_reboot, mock_prepare, mock_build_opt):
with task_manager.acquire(self.context, self.node['uuid'],
shared=False) as task:
i_info = task.node.driver_internal_info
i_info['agent_secret_token'] = 'magicvalue01'
task.node.driver_internal_info = i_info
agent_base._cleaning_reboot(task)
self.assertTrue(mock_build_opt.called)
self.assertTrue(mock_prepare.called)
mock_reboot.assert_called_once_with(task, states.REBOOT)
self.assertTrue(task.node.driver_internal_info['cleaning_reboot'])
self.assertNotIn('agent_secret_token',
task.node.driver_internal_info)
@mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', spec_set=True,
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
def test__cleaning_reboot_pregenerated_token(
self, mock_reboot, mock_prepare, mock_build_opt):
with task_manager.acquire(self.context, self.node['uuid'],
shared=False) as task:
i_info = task.node.driver_internal_info
i_info['agent_secret_token'] = 'magicvalue01'
i_info['agent_secret_token_pregenerated'] = True
task.node.driver_internal_info = i_info
agent_base._cleaning_reboot(task)
self.assertTrue(mock_build_opt.called)
self.assertTrue(mock_prepare.called)
mock_reboot.assert_called_once_with(task, states.REBOOT)
self.assertIn('agent_secret_token',
task.node.driver_internal_info)
@mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', spec_set=True,

21
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -2939,10 +2939,14 @@ class AsyncStepTestCase(db_base.DbTestCase):
def test_set_async_step_flags_deploying_set_all(self):
self.node.deploy_step = {'step': 'create_configuration',
'interface': 'raid'}
self.node.driver_internal_info = {}
self.node.driver_internal_info = {
'agent_secret_token': 'test',
'agent_secret_token_pregenerated': True}
expected = {'deployment_reboot': True,
'deployment_polling': True,
'skip_current_deploy_step': True}
'skip_current_deploy_step': True,
'agent_secret_token': 'test',
'agent_secret_token_pregenerated': True}
self.node.save()
utils.set_async_step_flags(self.node, reboot=True,
skip_current_step=True,
@ -2957,3 +2961,16 @@ class AsyncStepTestCase(db_base.DbTestCase):
utils.set_async_step_flags(self.node, reboot=True)
self.assertEqual({'deployment_reboot': True},
self.node.driver_internal_info)
def test_set_async_step_flags_clears_non_pregenerated_token(self):
self.node.clean_step = {'step': 'create_configuration',
'interface': 'raid'}
self.node.driver_internal_info = {'agent_secret_token': 'test'}
expected = {'cleaning_reboot': True,
'cleaning_polling': True,
'skip_current_clean_step': True}
self.node.save()
utils.set_async_step_flags(self.node, reboot=True,
skip_current_step=True,
polling=True)
self.assertEqual(expected, self.node.driver_internal_info)

6
ironic/tests/unit/objects/test_node.py
View File

@ -40,18 +40,24 @@ class TestNodeObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
def test_as_dict_insecure(self):
self.node.driver_info['ipmi_password'] = 'fake'
self.node.instance_info['configdrive'] = 'data'
self.node.driver_internal_info['agent_secret_token'] = 'abc'
d = self.node.as_dict()
self.assertEqual('fake', d['driver_info']['ipmi_password'])
self.assertEqual('data', d['instance_info']['configdrive'])
self.assertEqual('abc',
d['driver_internal_info']['agent_secret_token'])
# Ensure the node can be serialised.
jsonutils.dumps(d)
def test_as_dict_secure(self):
self.node.driver_info['ipmi_password'] = 'fake'
self.node.instance_info['configdrive'] = 'data'
self.node.driver_internal_info['agent_secret_token'] = 'abc'
d = self.node.as_dict(secure=True)
self.assertEqual('******', d['driver_info']['ipmi_password'])
self.assertEqual('******', d['instance_info']['configdrive'])
self.assertEqual('******',
d['driver_internal_info']['agent_secret_token'])
# Ensure the node can be serialised.
jsonutils.dumps(d)