Skip to content

Commit af5f05a

Browse files
juliakregerjuliakreger
committed
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
1 parent 638cfc6 commit af5f05a

File tree

10 files changed

+227
-11
lines changed

10 files changed

+227
-11
lines changed

‎ironic_python_agent/agent.py‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
165165

166166
def __init__(self, api_url, advertise_address, listen_address,
167167
ip_lookup_attempts, ip_lookup_sleep, network_interface,
168-
lookup_timeout, lookup_interval, standalone,
168+
lookup_timeout, lookup_interval, standalone, agent_token,
169169
hardware_initialization_delay=0):
170170
super(IronicPythonAgent, self).__init__()
171171
if bool(cfg.CONF.keyfile) != bool(cfg.CONF.certfile):
@@ -214,6 +214,11 @@ def __init__(self, api_url, advertise_address, listen_address,
214214
self.hardware_initialization_delay = hardware_initialization_delay
215215
# IPA will stop serving requests and exit after this is set to False
216216
self.serve_api = True
217+
self.agent_token = agent_token
218+
# Allows this to be turned on by the conductor while running,
219+
# in the event of long running ramdisks where the conductor
220+
# got upgraded somewhere along the way.
221+
self.agent_token_required = cfg.CONF.agent_token_required
217222

218223
def get_status(self):
219224
"""Retrieve a serializable status.
@@ -226,6 +231,26 @@ def get_status(self):
226231
version=self.version
227232
)
228233

234+
def validate_agent_token(self, token):
235+
# We did not get a token, i.e. None and
236+
# we've previously seen a token, which is
237+
# a mid-cluster upgrade case with long-running ramdisks.
238+
if (not token and self.agent_token
239+
and not self.agent_token_required):
240+
# TODO(TheJulia): Rip this out during or after the V
241+
# cycle.
242+
LOG.warning('Agent token for requests are not required '
243+
'by the conductor, yet we received a token. '
244+
'Cluster may be mid-upgrade. Support to '
245+
'not fail in this condition will be removed in '
246+
'the Victoria development cycle.')
247+
# Tell the API everything is okay.
248+
return True
249+
if self.agent_token is not None:
250+
return self.agent_token == token
251+
252+
return False
253+
229254
def _get_route_source(self, dest):
230255
"""Get the IP address to send packages to destination."""
231256
try:
@@ -419,6 +444,25 @@ def run(self):
419444
if config.get('metrics_statsd'):
420445
for opt, val in config.items():
421446
setattr(cfg.CONF.metrics_statsd, opt, val)
447+
token = config.get('agent_token')
448+
if token:
449+
if len(token) >= 32:
450+
LOG.debug('Agent token recorded as designated by '
451+
'the ironic installation.')
452+
self.agent_token = token
453+
# set with-in the API client.
454+
self.api_client.agent_token = token
455+
elif token == '******':
456+
LOG.warning('The agent token has already been '
457+
'retrieved. IPA may not operate as '
458+
'intended and the deployment may fail '
459+
'depending on settings in the ironic '
460+
'deployment.')
461+
else:
462+
LOG.info('An invalid token was received.')
463+
if config.get('agent_token_required'):
464+
self.agent_token_required = True
465+
422466
elif cfg.CONF.inspection_callback_url:
423467
LOG.info('No ipa-api-url configured, Heartbeat and lookup '
424468
'skipped for inspector.')

‎ironic_python_agent/api/app.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,13 @@ def api_run_command(self, request):
214214
or not isinstance(body['params'], dict)):
215215
raise http_exc.BadRequest('Missing or invalid name or params')
216216

217+
token = request.args.get('agent_token', None)
218+
if not self.agent.validate_agent_token(token):
219+
raise http_exc.Unauthorized(
220+
'Token invalid.')
217221
with metrics_utils.get_metrics_logger(__name__).timer('run_command'):
218222
result = self.agent.execute_command(body['name'], **body['params'])
219223
wait = request.args.get('wait')
220-
221224
if wait and wait.lower() == 'true':
222225
result.join()
223-
224226
return jsonify(result)

‎ironic_python_agent/cmd/agent.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,5 @@ def run():
4545
CONF.lookup_timeout,
4646
CONF.lookup_interval,
4747
CONF.standalone,
48+
CONF.agent_token,
4849
CONF.hardware_initialization_delay).run()

‎ironic_python_agent/config.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,17 @@
228228
default=False,
229229
help='If operations should fail if the clock time sync '
230230
'fails to complete successfully.'),
231+
cfg.StrOpt('agent_token',
232+
default=APARAMS.get('ipa-agent-token'),
233+
help='Pre-shared token to use when working with the '
234+
'ironic API. This value is typically supplied by '
235+
'ironic automatically.'),
236+
cfg.BoolOpt('agent_token_required',
237+
default=APARAMS.get('ipa-agent-token-required', False),
238+
help='Control to enforce if API command requests should '
239+
'enforce token validation. The configuration provided '
240+
'by the conductor MAY override this and force this '
241+
'setting to be changed to True in memory.'),
231242
]
232243

233244
CONF.register_cli_opts(cli_opts)

‎ironic_python_agent/ironic_api_client.py‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
# limitations under the License.
1414

1515

16+
from distutils.version import StrictVersion
17+
1618
from oslo_config import cfg
1719
from oslo_log import log
1820
from oslo_serialization import jsonutils
@@ -29,15 +31,18 @@
2931
CONF = cfg.CONF
3032
LOG = log.getLogger(__name__)
3133

34+
# TODO(TheJulia): This should be increased at some point.
3235
MIN_IRONIC_VERSION = (1, 22)
3336
AGENT_VERSION_IRONIC_VERSION = (1, 36)
37+
AGENT_TOKEN_IRONIC_VERSION = (1, 62)
3438

3539

3640
class APIClient(object):
3741
api_version = 'v1'
3842
lookup_api = '/%s/lookup' % api_version
3943
heartbeat_api = '/%s/heartbeat/{uuid}' % api_version
4044
_ironic_api_version = None
45+
agent_token = None
4146

4247
def __init__(self, api_url):
4348
self.api_url = api_url.rstrip('/')
@@ -74,8 +79,16 @@ def _request(self, method, path, data=None, headers=None, **kwargs):
7479
**kwargs)
7580

7681
def _get_ironic_api_version_header(self, version=MIN_IRONIC_VERSION):
77-
version_str = "%d.%d" % version
78-
return {'X-OpenStack-Ironic-API-Version': version_str}
82+
# TODO(TheJulia): It would be great to improve version handling
83+
# logic, but we need to ship a newer version if we can.
84+
ironic_version = "%d.%d" % self._get_ironic_api_version()
85+
agent_token_version = "%d.%d" % AGENT_TOKEN_IRONIC_VERSION
86+
if (StrictVersion(ironic_version)
87+
>= StrictVersion(agent_token_version)):
88+
version = agent_token_version
89+
else:
90+
version = ironic_version
91+
return {'X-OpenStack-Ironic-API-Version': version}
7992

8093
def _get_ironic_api_version(self):
8194
if not self._ironic_api_version:
@@ -97,7 +110,12 @@ def heartbeat(self, uuid, advertise_address):
97110

98111
data = {'callback_url': self._get_agent_url(advertise_address)}
99112

100-
if self._get_ironic_api_version() >= AGENT_VERSION_IRONIC_VERSION:
113+
api_ver = self._get_ironic_api_version()
114+
115+
if api_ver >= AGENT_TOKEN_IRONIC_VERSION:
116+
data['agent_token'] = self.agent_token
117+
118+
if api_ver >= AGENT_VERSION_IRONIC_VERSION:
101119
data['agent_version'] = version.version_info.release_string()
102120
headers = self._get_ironic_api_version_header(
103121
AGENT_VERSION_IRONIC_VERSION)

‎ironic_python_agent/tests/functional/base.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ def setUp(self):
4848
network_interface=None,
4949
lookup_timeout=300,
5050
lookup_interval=1,
51-
standalone=True)
51+
standalone=True,
52+
agent_token=None)
5253
self.process = multiprocessing.Process(
5354
target=self.agent.run)
5455
self.process.start()

‎ironic_python_agent/tests/unit/test_agent.py‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ def setUp(self):
146146
'eth0',
147147
300,
148148
1,
149+
None,
149150
False)
150151
self.agent.ext_mgr = extension.ExtensionManager.\
151152
make_test_instance([extension.Extension('fake', None,
@@ -239,7 +240,8 @@ def test_url_from_mdns_by_default(self, mock_get_managers, mock_wsgi,
239240
'eth0',
240241
300,
241242
1,
242-
False)
243+
False,
244+
None)
243245

244246
def set_serve_api():
245247
self.agent.serve_api = False
@@ -296,7 +298,8 @@ def test_url_from_mdns_explicitly(self, mock_get_managers, mock_wsgi,
296298
'eth0',
297299
300,
298300
1,
299-
False)
301+
False,
302+
None)
300303

301304
def set_serve_api():
302305
self.agent.serve_api = False
@@ -327,6 +330,51 @@ def set_serve_api():
327330
# changed via mdns
328331
self.assertEqual(42, CONF.disk_wait_attempts)
329332

333+
@mock.patch(
334+
'ironic_python_agent.hardware_managers.cna._detect_cna_card',
335+
mock.Mock())
336+
@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
337+
@mock.patch.object(agent.IronicPythonAgent,
338+
'_wait_for_interface', autospec=True)
339+
@mock.patch('oslo_service.wsgi.Server', autospec=True)
340+
@mock.patch.object(hardware, 'get_managers', autospec=True)
341+
def test_run_agent_token(self, mock_get_managers, mock_wsgi,
342+
mock_wait, mock_dispatch):
343+
CONF.set_override('inspection_callback_url', '')
344+
345+
wsgi_server = mock_wsgi.return_value
346+
347+
def set_serve_api():
348+
self.agent.serve_api = False
349+
350+
wsgi_server.start.side_effect = set_serve_api
351+
self.agent.heartbeater = mock.Mock()
352+
self.agent.api_client.lookup_node = mock.Mock()
353+
self.agent.api_client.lookup_node.return_value = {
354+
'node': {
355+
'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c'
356+
},
357+
'config': {
358+
'heartbeat_timeout': 300,
359+
'agent_token': '1' * 128,
360+
'agent_token_required': True
361+
}
362+
}
363+
364+
self.agent.run()
365+
366+
mock_wsgi.assert_called_once_with(CONF, 'ironic-python-agent',
367+
app=self.agent.api,
368+
host=mock.ANY, port=9999)
369+
wsgi_server.start.assert_called_once_with()
370+
mock_wait.assert_called_once_with(mock.ANY)
371+
self.assertEqual([mock.call('list_hardware_info'),
372+
mock.call('wait_for_disks')],
373+
mock_dispatch.call_args_list)
374+
self.agent.heartbeater.start.assert_called_once_with()
375+
self.assertEqual('1' * 128, self.agent.agent_token)
376+
self.assertEqual('1' * 128, self.agent.api_client.agent_token)
377+
330378
@mock.patch('eventlet.sleep', autospec=True)
331379
@mock.patch(
332380
'ironic_python_agent.hardware_managers.cna._detect_cna_card',
@@ -449,7 +497,8 @@ def test_run_with_inspection_without_apiurl(self,
449497
'eth0',
450498
300,
451499
1,
452-
False)
500+
False,
501+
None)
453502
self.assertFalse(hasattr(self.agent, 'api_client'))
454503
self.assertFalse(hasattr(self.agent, 'heartbeater'))
455504

@@ -504,7 +553,8 @@ def test_run_without_inspection_and_apiurl(self,
504553
'eth0',
505554
300,
506555
1,
507-
False)
556+
False,
557+
None)
508558
self.assertFalse(hasattr(self.agent, 'api_client'))
509559
self.assertFalse(hasattr(self.agent, 'heartbeater'))
510560

@@ -704,6 +754,7 @@ def setUp(self):
704754
300,
705755
1,
706756
'agent_ipmitool',
757+
None,
707758
True)
708759

709760
@mock.patch(
@@ -756,6 +807,7 @@ def setUp(self):
756807
network_interface=None,
757808
lookup_timeout=300,
758809
lookup_interval=1,
810+
agent_token=None,
759811
standalone=False)
760812

761813
def test_advertise_address_provided(self, mock_exec, mock_gethostbyname):

‎ironic_python_agent/tests/unit/test_api.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ def test_execute_agent_command_success_with_true_wait(self):
185185

186186
self.assertEqual(200, response.status_code)
187187
self.assertEqual(1, self.mock_agent.execute_command.call_count)
188+
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
188189
args, kwargs = self.mock_agent.execute_command.call_args
189190
self.assertEqual(('do_things',), args)
190191
self.assertEqual({'key': 'value'}, kwargs)
@@ -211,6 +212,7 @@ def test_execute_agent_command_success_with_false_wait(self):
211212

212213
self.assertEqual(200, response.status_code)
213214
self.assertEqual(1, self.mock_agent.execute_command.call_count)
215+
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
214216
args, kwargs = self.mock_agent.execute_command.call_args
215217
self.assertEqual(('do_things',), args)
216218
self.assertEqual({'key': 'value'}, kwargs)
@@ -272,3 +274,56 @@ def test_get_command_result(self):
272274
self.assertEqual(200, response.status_code)
273275
data = response.json
274276
self.assertEqual(serialized_cmd_result, data)
277+
278+
def test_execute_agent_command_with_token(self):
279+
agent_token = str('0123456789' * 10)
280+
command = {
281+
'name': 'do_things',
282+
'params': {'key': 'value',
283+
'wait': False,
284+
'agent_token': agent_token},
285+
}
286+
287+
result = base.SyncCommandResult(command['name'],
288+
command['params'],
289+
True,
290+
{'test': 'result'})
291+
292+
self.mock_agent.validate_agent_token.return_value = True
293+
self.mock_agent.execute_command.return_value = result
294+
295+
with mock.patch.object(result, 'join', autospec=True) as join_mock:
296+
response = self.post_json(
297+
'/commands?wait=false?agent_token=%s' % agent_token,
298+
command)
299+
self.assertFalse(join_mock.called)
300+
301+
self.assertEqual(200, response.status_code)
302+
303+
self.assertEqual(1, self.mock_agent.execute_command.call_count)
304+
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)
305+
args, kwargs = self.mock_agent.execute_command.call_args
306+
self.assertEqual(('do_things',), args)
307+
expected_result = result.serialize()
308+
data = response.json
309+
self.assertEqual(expected_result, data)
310+
311+
def test_execute_agent_command_with_token_invalid(self):
312+
agent_token = str('0123456789' * 10)
313+
command = {
314+
'name': 'do_things',
315+
'params': {'key': 'value',
316+
'wait': False,
317+
'agent_token': agent_token},
318+
}
319+
320+
self.mock_agent.validate_agent_token.return_value = False
321+
response = self.post_json(
322+
'/commands?wait=false?agent_token=%s' % agent_token,
323+
command,
324+
expect_errors=True)
325+
326+
self.assertEqual(401, response.status_code)
327+
328+
self.assertEqual(0, self.mock_agent.execute_command.call_count)
329+
self.assertEqual(1, self.mock_agent.validate_agent_token.call_count)

0 commit comments

Comments
 (0)