From 00392c6d1ba617d587ec79540149b99357cc2589 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Wed, 20 Nov 2024 10:15:09 +0000 Subject: [PATCH 1/3] Switch from MAC address stored in Nautobot asset_tag to custom field We have to change the graphql filter - you could search in Nautobot's built-in string attributes by passing an array of possible values to find, but that doesn't work for custom fields. --- .../understack_workflows/nautobot_device.py | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 1694bc857..327f52ac6 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -1,4 +1,3 @@ -import json import re from dataclasses import dataclass from ipaddress import IPv4Interface @@ -139,26 +138,26 @@ def switches_for(nautobot, chassis_info: ChassisInfo) -> dict: def nautobot_switches(nautobot, mac_addresses: set[str]) -> dict[str, dict]: """Get switches by MAC address. + Assumes switch MAC addresses are present in Nautobot in a custom field on + Device called chassis_mac_address. + Assumes that MAC addresses in Nautobot are normalized to upcase AA:BB:CC:DD:EE:FF form. - We store the MAC address in the nautobot device "asset tag" field because - there was nowhere else to put it. - returns a dict[mac_address] -> dict switch information indexed by mac """ - formatted_list = json.dumps(list(mac_addresses)) + pattern = "|".join(mac_addresses) query = ( """{ - devices(asset_tag: %s){ + devices(cf_chassis_mac_address__re: "(%s)"){ id name - mac: asset_tag + mac: cf_chassis_mac_address location { id name } rack { id name } } }""" - % formatted_list + % pattern ) result = nautobot.graphql.query(query) @@ -177,9 +176,8 @@ def nautobot_switch(all_switches, interface): raise Exception( f"Looking for a switch in nautobot that matches the LLDP " f"info reported by server BMC - " - f"No device found in nautobot with asset_tag {mac_address} " - f"nor the calculated base mac address {base_mac_address}" - f"{all_switches}" + f"No device in nautobot with chassis_mac_address {mac_address}, " + f"nor the calculated base mac address {base_mac_address}." ) return switch @@ -238,7 +236,7 @@ def nautobot_server(nautobot, serial: str) -> NautobotDevice | None: id name device {{ id name - mac: asset_tag + mac: cf_chassis_mac_address location {{ id name }} rack {{ id name }} }} From 29b987e6989d5f4602b392cce97a6fda495f658c Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Wed, 20 Nov 2024 18:15:30 +0000 Subject: [PATCH 2/3] Stamp out any question of injection attacks against graphql query Learn the lessons of the 1990s and use "variables", as God intended. --- .../tests/test_nautobot_device.py | 4 ++-- .../understack_workflows/nautobot_device.py | 21 +++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/python/understack-workflows/tests/test_nautobot_device.py b/python/understack-workflows/tests/test_nautobot_device.py index 5c4d39aac..60c2b4918 100644 --- a/python/understack-workflows/tests/test_nautobot_device.py +++ b/python/understack-workflows/tests/test_nautobot_device.py @@ -26,8 +26,8 @@ def update(self, *_): pass class Graphql: - def query(self, graphql): - if "61:80" in graphql: + def query(self, graphql, variables=None): + if "pattern" in graphql and variables: return FakeNautobot.SwitchResponse() if "33GSW04" in graphql: return FakeNautobot.GraphqlResponse( diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 327f52ac6..80a8cb72c 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -148,19 +148,18 @@ def nautobot_switches(nautobot, mac_addresses: set[str]) -> dict[str, dict]: """ pattern = "|".join(mac_addresses) - query = ( - """{ - devices(cf_chassis_mac_address__re: "(%s)"){ - id name - mac: cf_chassis_mac_address - location { id name } - rack { id name } + query = """ + query($pattern: [String!]){ + devices(cf_chassis_mac_address__re: $pattern){ + id name + mac: cf_chassis_mac_address + location { id name } + rack { id name } + } } - }""" - % pattern - ) + """ - result = nautobot.graphql.query(query) + result = nautobot.graphql.query(query, variables={"pattern": pattern}) if not result.json or result.json.get("errors"): raise Exception(f"Nautobot switch graphql query failed: {result}") switches = result.json["data"]["devices"] From 84afca58aded371452312a8919b6a6be61f72c7d Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Wed, 20 Nov 2024 19:44:00 +0000 Subject: [PATCH 3/3] Avoid graphql query injection vulnerability in remaining graphql queries Use the "variables" feature to avoid string interpolation in making these queries. --- .../tests/test_nautobot_device.py | 6 +-- .../main/undersync_device.py | 12 +++-- .../understack_workflows/nautobot_device.py | 50 ++++++++++--------- 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/python/understack-workflows/tests/test_nautobot_device.py b/python/understack-workflows/tests/test_nautobot_device.py index 60c2b4918..64c855706 100644 --- a/python/understack-workflows/tests/test_nautobot_device.py +++ b/python/understack-workflows/tests/test_nautobot_device.py @@ -26,10 +26,10 @@ def update(self, *_): pass class Graphql: - def query(self, graphql, variables=None): - if "pattern" in graphql and variables: + def query(self, graphql, variables: dict): + if "pattern" in variables: return FakeNautobot.SwitchResponse() - if "33GSW04" in graphql: + if "serial" in variables: return FakeNautobot.GraphqlResponse( "json_samples/bmc_chassis_info/R7615/nautobot_graphql_response_server_device_33GSW04.json" ) diff --git a/python/understack-workflows/understack_workflows/main/undersync_device.py b/python/understack-workflows/understack_workflows/main/undersync_device.py index e10c892f5..ae5a96325 100644 --- a/python/understack-workflows/understack_workflows/main/undersync_device.py +++ b/python/understack-workflows/understack_workflows/main/undersync_device.py @@ -60,9 +60,15 @@ def update_nautobot_for_provisioning( def vlan_group_id_for(device_id, nautobot): - result = nautobot.session.graphql.query( - f'{{device(id: "{device_id}") {{ rel_vlan_group_to_devices {{id}}}}}}' - ) + query = """ + query($device_id: ID!){ + device(id: $device_id) { + rel_vlan_group_to_devices {id} + } + } + """ + variables = {"device_id": device_id} + result = nautobot.session.graphql.query(query=query, variables=variables) if not result.json or result.json.get("errors"): raise Exception(f"Nautobot vlan_group graphql query failed: {result}") return result.json["data"]["device"]["rel_vlan_group_to_devices"]["id"] diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 80a8cb72c..4d8aa331f 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -222,33 +222,35 @@ def _parse_manufacturer(name: str) -> str: def nautobot_server(nautobot, serial: str) -> NautobotDevice | None: - query = f"""{{ - devices(serial: ["{serial}"]){{ - id name - location {{ id name }} - rack {{ id name }} - interfaces {{ + query = """ + query($serial: String!){ + devices(serial: [$serial]){ id name - type description mac_address - status {{ name }} - connected_interface {{ + location { id name } + rack { id name } + interfaces { id name - device {{ + type description mac_address + status { name } + connected_interface { id name - mac: cf_chassis_mac_address - location {{ id name }} - rack {{ id name }} - }} - }} - ip_addresses {{ - id host - parent {{ prefix }} - }} - }} - }} - }}""" - - result = nautobot.graphql.query(query) + device { + id name + mac: cf_chassis_mac_address + location { id name } + rack { id name } + } + } + ip_addresses { + id host + parent { prefix } + } + } + } + } + """ + + result = nautobot.graphql.query(query, variables={"serial": serial}) if not result.json or result.json.get("errors"): raise Exception(f"Nautobot server graphql query failed: {result}")