Accept and use a TLS certificate from the agent

Accepts the certificate from a heartbeat and stores its path in
driver_internal_info for further usage by the agent client (or
any 3rd party deploy implementations).

Similarly to agent_url, the certificate is protected from further
changes (unless the local copy does not exist) and is removed
on reboot or tear down (unless fast-tracking).

Change-Id: I81b326116e62cd86ad22b533f55d061e5ed53e96
Story: #2007214
Task: #40603
This commit is contained in:
Dmitry Tantsur 2020-08-20 12:26:08 +02:00
parent f6b65cb68f
commit 2b676a6864
18 changed files with 245 additions and 27 deletions

View File

@ -2,6 +2,11 @@
REST API Version History
========================
1.68 (Victoria, master)
-----------------------
Added the ``agent_verify_ca`` parameter to the ramdisk heartbeat API.
1.67 (Victoria, master)
-----------------------

View File

@ -173,9 +173,9 @@ class HeartbeatController(rest.RestController):
"""Controller handling heartbeats from deploy ramdisk."""
@expose.expose(None, types.uuid_or_name, str,
str, str, status_code=http_client.ACCEPTED)
str, str, str, status_code=http_client.ACCEPTED)
def post(self, node_ident, callback_url, agent_version=None,
agent_token=None):
agent_token=None, agent_verify_ca=None):
"""Process a heartbeat from the deploy ramdisk.
:param node_ident: the UUID or logical name of a node.
@ -186,6 +186,7 @@ class HeartbeatController(rest.RestController):
last release before sending agent_version was introduced) will be
assumed.
:param agent_token: randomly generated validation token.
:param agent_verify_ca: TLS certificate to use to connect to the agent.
:raises: NodeNotFound if node with provided UUID or name was not found.
:raises: InvalidUuidOrName if node_ident is not valid name or UUID.
:raises: NoValidHost if RPC topic for node could not be retrieved.
@ -202,6 +203,11 @@ class HeartbeatController(rest.RestController):
cdict = api.request.context.to_policy_values()
policy.authorize('baremetal:node:ipa_heartbeat', cdict, cdict)
if (agent_verify_ca is not None
and not api_utils.allow_verify_ca_in_heartbeat()):
raise exception.InvalidParameterValue(
_('Field "agent_verify_ca" not recognised in this version'))
rpc_node = api_utils.get_rpc_node_with_suffix(node_ident)
dii = rpc_node['driver_internal_info']
agent_url = dii.get('agent_url')
@ -231,4 +237,4 @@ class HeartbeatController(rest.RestController):
api.request.rpcapi.heartbeat(
api.request.context, rpc_node.uuid, callback_url,
agent_version, agent_token, topic=topic)
agent_version, agent_token, agent_verify_ca, topic=topic)

View File

@ -1391,3 +1391,8 @@ def allow_local_link_connection_network_type():
"""Check if network_type is allowed in ports link_local_connection"""
return (api.request.version.minor
>= versions.MINOR_64_LOCAL_LINK_CONNECTION_NETWORK_TYPE)
def allow_verify_ca_in_heartbeat():
"""Check if heartbeat accepts agent_verify_ca."""
return api.request.version.minor >= versions.MINOR_68_HEARTBEAT_VERIFY_CA

View File

@ -105,6 +105,7 @@ BASE_VERSION = 1
# v1.65: Add lessee to the node object.
# v1.66: Add support for node network_data field.
# v1.67: Add support for port_uuid/portgroup_uuid in node vif_attach
# v1.68: Add agent_verify_ca to heartbeat.
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -174,6 +175,7 @@ MINOR_64_LOCAL_LINK_CONNECTION_NETWORK_TYPE = 64
MINOR_65_NODE_LESSEE = 65
MINOR_66_NODE_NETWORK_DATA = 66
MINOR_67_NODE_VIF_ATTACH_PORT = 67
MINOR_68_HEARTBEAT_VERIFY_CA = 68
# When adding another version, update:
# - MINOR_MAX_VERSION
@ -181,7 +183,7 @@ MINOR_67_NODE_VIF_ATTACH_PORT = 67
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
MINOR_MAX_VERSION = MINOR_67_NODE_VIF_ATTACH_PORT
MINOR_MAX_VERSION = MINOR_68_HEARTBEAT_VERIFY_CA
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

View File

@ -248,8 +248,8 @@ RELEASE_MAPPING = {
}
},
'master': {
'api': '1.67',
'rpc': '1.50',
'api': '1.68',
'rpc': '1.51',
'objects': {
'Allocation': ['1.1'],
'Node': ['1.35'],

View File

@ -91,7 +91,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.50'
RPC_API_VERSION = '1.51'
target = messaging.Target(version=RPC_API_VERSION)
@ -3099,11 +3099,12 @@ class ConductorManager(base_manager.BaseConductorManager):
@messaging.expected_exceptions(exception.InvalidParameterValue)
@messaging.expected_exceptions(exception.NoFreeConductorWorker)
def heartbeat(self, context, node_id, callback_url, agent_version=None,
agent_token=None):
agent_token=None, agent_verify_ca=None):
"""Process a heartbeat from the ramdisk.
:param context: request context.
:param node_id: node id or uuid.
:param callback_url: URL to reach back to the ramdisk.
:param agent_version: The version of the agent that is heartbeating. If
not provided it either indicates that the agent that is
heartbeating is a version before sending agent_version was
@ -3111,8 +3112,8 @@ class ConductorManager(base_manager.BaseConductorManager):
RPC version is pinned so the API isn't passing us the
agent_version, in these cases assume agent v3.0.0 (the last release
before sending agent_version was introduced).
:param callback_url: URL to reach back to the ramdisk.
:param agent_token: randomly generated validation token.
:param agent_verify_ca: TLS certificate for the agent.
:raises: NoFreeConductorWorker if there are no conductors to process
this heartbeat request.
"""
@ -3144,9 +3145,13 @@ class ConductorManager(base_manager.BaseConductorManager):
raise exception.InvalidParameterValue(
_('TLS is required by configuration'))
if agent_verify_ca:
agent_verify_ca = utils.store_agent_certificate(
task.node, agent_verify_ca)
task.spawn_after(
self._spawn_worker, task.driver.deploy.heartbeat,
task, callback_url, agent_version)
task, callback_url, agent_version, agent_verify_ca)
@METRICS.timer('ConductorManager.vif_list')
@messaging.expected_exceptions(exception.NetworkError,

View File

@ -103,13 +103,14 @@ class ConductorAPI(object):
heartbeat
| 1.50 - Added set_indicator_state, get_indicator_state and
| get_supported_indicators.
| 1.51 - Added agent_verify_ca 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.50'
RPC_API_VERSION = '1.51'
def __init__(self, topic=None):
super(ConductorAPI, self).__init__()
@ -898,7 +899,7 @@ class ConductorAPI(object):
node_id=node_id, clean_steps=clean_steps)
def heartbeat(self, context, node_id, callback_url, agent_version,
agent_token=None, topic=None):
agent_token=None, agent_verify_ca=None, topic=None):
"""Process a node heartbeat.
:param context: request context.
@ -907,6 +908,7 @@ class ConductorAPI(object):
:param topic: RPC topic. Defaults to self.topic.
:param agent_token: randomly generated validation token.
:param agent_version: the version of the agent that is heartbeating
:param agent_verify_ca: TLS certificate for the agent.
:raises: InvalidParameterValue if an invalid agent token is received.
"""
new_kws = {}
@ -917,6 +919,9 @@ class ConductorAPI(object):
if self.client.can_send_version('1.49'):
version = '1.49'
new_kws['agent_token'] = agent_token
if self.client.can_send_version('1.51'):
version = '1.51'
new_kws['agent_verify_ca'] = agent_verify_ca
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)

View File

@ -16,6 +16,7 @@ import contextlib
import crypt
import datetime
from distutils.version import StrictVersion
import os
import secrets
import time
@ -462,6 +463,8 @@ def wipe_internal_info_on_power_off(node):
# Wipe cached steps since they may change after reboot.
driver_internal_info.pop('agent_cached_deploy_steps', None)
driver_internal_info.pop('agent_cached_clean_steps', None)
# Remove TLS certificate since it's regenerated on each run.
driver_internal_info.pop('agent_verify_ca', None)
node.driver_internal_info = driver_internal_info
@ -473,6 +476,8 @@ def wipe_token_and_url(task):
# Remove agent_url since it will be re-asserted
# upon the next deployment attempt.
info.pop('agent_url', None)
# Remove TLS certificate since it's regenerated on each run.
info.pop('agent_verify_ca', None)
task.node.driver_internal_info = info
@ -1232,3 +1237,45 @@ def get_attached_vif(port):
if inspection_vif:
return (inspection_vif, 'inspecting')
return (None, None)
def store_agent_certificate(node, agent_verify_ca):
"""Store certificate received from the agent and return its path."""
existing_verify_ca = node.driver_internal_info.get(
'agent_verify_ca')
if existing_verify_ca:
if os.path.exists(existing_verify_ca):
try:
with open(existing_verify_ca, 'rt') as fp:
existing_text = fp.read()
except EnvironmentError:
with excutils.save_and_reraise_exception():
LOG.exception('Could not read the existing TLS certificate'
' for node %s', node.uuid)
if existing_text.strip() != agent_verify_ca.strip():
LOG.error('Content mismatch for agent_verify_ca for '
'node %s', node.uuid)
raise exception.InvalidParameterValue(
_('Detected change in ramdisk provided "agent_verify_ca"'))
else:
return existing_verify_ca
else:
LOG.info('Current agent_verify_ca was not found for node '
'%s, assuming take over and storing', node.uuid)
fname = os.path.join(CONF.agent.certificates_path, '%s.crt' % node.uuid)
try:
# FIXME(dtantsur): it makes more sense to create this path on conductor
# start-up, but it requires reworking a ton of unit tests.
os.makedirs(CONF.agent.certificates_path, exist_ok=True)
with open(fname, 'wt') as fp:
fp.write(agent_verify_ca)
except EnvironmentError:
with excutils.save_and_reraise_exception():
LOG.exception('Could not save the TLS certificate for node %s',
node.uuid)
else:
LOG.debug('Saved the custom certificate for node %(node)s to %(file)s',
{'node': node.uuid, 'file': fname})
return fname

View File

@ -149,6 +149,10 @@ opts = [
mutable=True,
help=_('If set to True, callback URLs without https:// will '
'be rejected by the conductor.')),
cfg.StrOpt('certificates_path',
default='/var/lib/ironic/certificates',
help=_('Path for TLS certificates used to validate '
'connections to the ramdisk.')),
]

View File

@ -475,12 +475,14 @@ class DeployInterface(BaseInterface):
"""
pass
def heartbeat(self, task, callback_url, agent_version):
def heartbeat(self, task, callback_url, agent_version,
agent_verify_ca=None):
"""Record a heartbeat for the node.
:param task: A TaskManager instance containing the node to act on.
:param callback_url: a URL to use to call to the ramdisk.
:param agent_version: The version of the agent that is heartbeating
:param agent_verify_ca: TLS certificate for the agent.
:return: None
"""
LOG.warning('Got heartbeat message from node %(node)s, but '

View File

@ -607,12 +607,14 @@ class HeartbeatMixin(object):
manager_utils.rescuing_error_handler(task, last_error)
@METRICS.timer('HeartbeatMixin.heartbeat')
def heartbeat(self, task, callback_url, agent_version):
def heartbeat(self, task, callback_url, agent_version,
agent_verify_ca=None):
"""Process a heartbeat.
:param task: task to work with.
:param callback_url: agent HTTP API URL.
:param agent_version: The version of the agent that is heartbeating
:param agent_verify_ca: TLS certificate for the agent.
"""
# NOTE(pas-ha) immediately skip the rest if nothing to do
if (task.node.provision_state not in self.heartbeat_allowed_states
@ -641,6 +643,8 @@ class HeartbeatMixin(object):
# datetime.datetime.strptime(var, "%Y-%m-%d %H:%M:%S.%f")
driver_internal_info['agent_last_heartbeat'] = str(
timeutils.utcnow().isoformat())
if agent_verify_ca:
driver_internal_info['agent_verify_ca'] = agent_verify_ca
node.driver_internal_info = driver_internal_info
node.save()

View File

@ -77,7 +77,8 @@ class AgentClient(object):
})
def _get_verify(self, node):
return node.driver_info.get('agent_verify_ca', True)
return (node.driver_internal_info.get('agent_verify_ca')
or node.driver_info.get('agent_verify_ca', True))
def _raise_if_typeerror(self, result, node, method):
error = result.get('command_error')

View File

@ -225,7 +225,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None, 'x',
topic='test-topic')
None, topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_ok_with_json(self, mock_heartbeat):
@ -240,7 +240,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
'maybe some magic',
topic='test-topic')
None, topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_ok_by_name(self, mock_heartbeat):
@ -255,7 +255,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
'token',
topic='test-topic')
None, topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_ok_agent_version(self, mock_heartbeat):
@ -271,7 +271,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', '1.4.1',
'meow',
topic='test-topic')
None, topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_old_API_agent_version_error(self, mock_heartbeat):
@ -308,5 +308,33 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', None,
'abcdef1',
'abcdef1', None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_ok_agent_verify_ca(self, mock_heartbeat):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
{'callback_url': 'url',
'agent_token': 'meow',
'agent_verify_ca': '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,
'meow', 'abcdef1',
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_old_API_agent_verify_ca_error(self, mock_heartbeat):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
{'callback_url': 'url',
'agent_token': 'meow',
'agent_verify_ca': 'abcd'},
headers={api_base.Version.string: '1.67'},
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)

View File

@ -7256,7 +7256,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.service.heartbeat(self.context, node.uuid, 'http://callback',
agent_token='magic')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '3.0.0')
'http://callback', '3.0.0', None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@ -7279,7 +7279,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.service.heartbeat(self.context, node.uuid, 'http://callback',
'1.4.1', agent_token='magic')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '1.4.1')
'http://callback', '1.4.1', None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@ -7327,7 +7327,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
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')
'http://callback', '3.0.0', None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@ -7351,7 +7351,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
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')
'http://callback', '3.0.0', None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@ -7449,7 +7449,6 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn
exc = self.assertRaises(messaging.rpc.ExpectedException,
self.service.heartbeat, self.context,
node.uuid, 'http://callback',
@ -7458,6 +7457,33 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertIn('TLS is required', str(exc.exc_info[1]))
self.assertFalse(mock_heartbeat.called)
@mock.patch.object(conductor_utils, 'store_agent_certificate',
autospec=True)
@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_verify_ca(self, mock_spawn,
mock_heartbeat,
mock_store_cert):
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'})
mock_store_cert.return_value = '/path/to/crt'
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', agent_verify_ca='abcd')
mock_heartbeat.assert_called_with(
mock.ANY, mock.ANY, 'http://callback', '3.0.0',
'/path/to/crt')
@mgr_utils.mock_record_keepalive
class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin,

View File

@ -516,7 +516,7 @@ class RPCAPITestCase(db_base.DbTestCase):
node_id='fake-node',
callback_url='http://ramdisk.url:port',
agent_version=None,
version='1.49')
version='1.51')
def test_heartbeat_agent_token(self):
self._test_rpcapi('heartbeat',
@ -525,7 +525,7 @@ class RPCAPITestCase(db_base.DbTestCase):
callback_url='http://ramdisk.url:port',
agent_version=None,
agent_token='xyz1',
version='1.49')
version='1.51')
def test_destroy_volume_connector(self):
fake_volume_connector = db_utils.get_test_volume_connector()

View File

@ -9,6 +9,9 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import os
import tempfile
import time
from unittest import mock
@ -2136,3 +2139,52 @@ class GetAttachedVifTestCase(db_base.DbTestCase):
vif, use = conductor_utils.get_attached_vif(self.port)
self.assertEqual('1', vif)
self.assertEqual('inspecting', use)
class StoreAgentCertificateTestCase(db_base.DbTestCase):
def setUp(self):
super(StoreAgentCertificateTestCase, self).setUp()
self.node = obj_utils.create_test_node(self.context,
driver='fake-hardware')
self.tempdir = tempfile.mkdtemp()
CONF.set_override('certificates_path', self.tempdir, group='agent')
self.fname = os.path.join(self.tempdir, '%s.crt' % self.node.uuid)
def test_store_new(self):
result = conductor_utils.store_agent_certificate(self.node,
'cert text')
self.assertEqual(self.fname, result)
with open(self.fname, 'rt') as fp:
self.assertEqual('cert text', fp.read())
def test_store_existing(self):
old_fname = os.path.join(self.tempdir, 'old.crt')
with open(old_fname, 'wt') as fp:
fp.write('cert text')
self.node.driver_internal_info['agent_verify_ca'] = old_fname
result = conductor_utils.store_agent_certificate(self.node,
'cert text')
self.assertEqual(old_fname, result)
self.assertFalse(os.path.exists(self.fname))
def test_no_change(self):
old_fname = os.path.join(self.tempdir, 'old.crt')
with open(old_fname, 'wt') as fp:
fp.write('cert text')
self.node.driver_internal_info['agent_verify_ca'] = old_fname
self.assertRaises(exception.InvalidParameterValue,
conductor_utils.store_agent_certificate,
self.node, 'new cert text')
self.assertFalse(os.path.exists(self.fname))
def test_take_over(self):
old_fname = os.path.join(self.tempdir, 'old.crt')
self.node.driver_internal_info['agent_verify_ca'] = old_fname
result = conductor_utils.store_agent_certificate(self.node,
'cert text')
self.assertEqual(self.fname, result)
with open(self.fname, 'rt') as fp:
self.assertEqual('cert text', fp.read())

View File

@ -258,6 +258,28 @@ class TestAgentClient(base.TestCase):
timeout=60,
verify='/path/to/agent.crt')
def test__command_verify_internal(self):
response_data = {'status': 'ok'}
self.client.session.post.return_value = MockResponse(response_data)
method = 'standby.run_image'
image_info = {'image_id': 'test_image'}
params = {'image_info': image_info}
self.node.driver_info['agent_verify_ca'] = True
self.node.driver_internal_info['agent_verify_ca'] = '/path/to/crt'
url = self.client._get_command_url(self.node)
body = self.client._get_command_body(method, params)
response = self.client._command(self.node, method, params)
self.assertEqual(response, response_data)
self.client.session.post.assert_called_once_with(
url,
data=body,
params={'wait': 'false'},
timeout=60,
verify='/path/to/crt')
@mock.patch('time.sleep', lambda seconds: None)
def test__command_poll(self):
response_data = {'status': 'ok'}

View File

@ -0,0 +1,4 @@
---
features:
- |
Adds an ability to accept a custom TLS certificate in the heartbeat API.