sts-sponsors team mailing list archive
-
sts-sponsors team
-
Mailing list archive
-
Message #06692
[Merge] ~igor-brovtsin/maas:hide-sensitive-power-data-from-users into maas:master
Igor Brovtsin has proposed merging ~igor-brovtsin/maas:hide-sensitive-power-data-from-users into maas:master.
Commit message:
Hide sensitive power parameters from users
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/440233
This MP hides sensitive power parameters from non-superusers for UI handlers. Superusers access to these values is not limited because power parameters are available to them via CLI.
--
Your team MAAS Committers is subscribed to branch maas:master.
diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index f1c8356..f0f030e 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -73,6 +73,7 @@ from provisioningserver.drivers.pod import InterfaceAttachType
from provisioningserver.drivers.power.registry import (
PowerDriverRegistry,
sanitise_power_parameters,
+ SENSITIVE_VALUE_PLACEHOLDER,
)
from provisioningserver.enum import MACVLAN_MODE_CHOICES
from provisioningserver.logger import get_maas_logger
@@ -403,16 +404,18 @@ class BMC(CleanSave, TimestampedModel):
else:
break
- def get_power_parameters(self):
+ def get_power_parameters(self, mask_sensitive=False):
from maasserver.secrets import SecretManager
power_parameters = self.power_parameters or {}
- return {
- **power_parameters,
- **SecretManager().get_composite_secret(
- "power-parameters", obj=self.as_bmc(), default={}
- ),
- }
+ sensitive = SecretManager().get_composite_secret(
+ "power-parameters", obj=self.as_bmc(), default={}
+ )
+ if mask_sensitive:
+ for key in sensitive.keys():
+ if sensitive[key]:
+ sensitive[key] = SENSITIVE_VALUE_PLACEHOLDER
+ return {**power_parameters, **sensitive}
def set_power_parameters(self, power_parameters):
power_parameters, secrets = sanitise_power_parameters(
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 00202dd..2f2b0bb 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -185,6 +185,7 @@ from provisioningserver.drivers.power.ipmi import IPMI_BOOT_TYPE
from provisioningserver.drivers.power.registry import (
PowerDriverRegistry,
sanitise_power_parameters,
+ SENSITIVE_VALUE_PLACEHOLDER,
)
from provisioningserver.events import EVENT_DETAILS, EVENT_TYPES
from provisioningserver.logger import get_maas_logger, LegacyLogger
@@ -1439,15 +1440,20 @@ class Node(CleanSave, TimestampedModel):
def power_type(self):
return "" if self.bmc is None else self.bmc.power_type
- def get_instance_power_parameters(self):
+ def get_instance_power_parameters(self, mask_sensitive=False):
from maasserver.secrets import SecretManager
power_parameters = self.instance_power_parameters or {}
+ sensitive = SecretManager().get_composite_secret(
+ "power-parameters", obj=self.as_node(), default={}
+ )
+ if mask_sensitive:
+ for key in sensitive.keys():
+ if sensitive[key]:
+ sensitive[key] = SENSITIVE_VALUE_PLACEHOLDER
return {
**power_parameters,
- **SecretManager().get_composite_secret(
- "power-parameters", obj=self.as_node(), default={}
- ),
+ **sensitive,
}
def set_instance_power_parameters(self, power_parameters):
@@ -1463,14 +1469,18 @@ class Node(CleanSave, TimestampedModel):
)
self.instance_power_parameters = power_parameters
- def get_power_parameters(self):
+ def get_power_parameters(self, mask_sensitive=False):
# Overlay instance power parameters over bmc power parameters.
- instance_parameters = self.get_instance_power_parameters()
+ instance_parameters = self.get_instance_power_parameters(
+ mask_sensitive=mask_sensitive
+ )
if not instance_parameters:
instance_parameters = {}
bmc_parameters = {}
if self.bmc and (
- bmc_power_parameters := self.bmc.get_power_parameters()
+ bmc_power_parameters := self.bmc.get_power_parameters(
+ mask_sensitive=mask_sensitive
+ )
):
bmc_parameters = bmc_power_parameters
return {**bmc_parameters, **instance_parameters}
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index b766810..86b05ca 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -77,6 +77,9 @@ from provisioningserver.drivers.pod import (
RequestedMachine,
RequestedMachineInterface,
)
+from provisioningserver.drivers.power.registry import (
+ SENSITIVE_VALUE_PLACEHOLDER,
+)
from provisioningserver.rpc.cluster import DecomposeMachine
from provisioningserver.utils.constraints import LabeledConstraintMap
@@ -646,6 +649,17 @@ class TestBMC(MAASServerTestCase):
self.assertEqual({"foo": "bar"}, bmc.get_power_parameters())
+ def test_get_power_params_masks_sensitiver(self):
+ bmc = factory.make_BMC(power_type="ipmi")
+ secret_manager = SecretManager()
+ secret_manager.set_composite_secret(
+ "power-parameters", {"power_pass": "bar"}, obj=bmc
+ )
+ self.assertEqual(
+ {"power_pass": SENSITIVE_VALUE_PLACEHOLDER},
+ bmc.get_power_parameters(mask_sensitive=True),
+ )
+
class TestPodManager(MAASServerTestCase):
def enable_rbac(self):
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 8afb42b..33ea38d 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -183,7 +183,10 @@ from metadataserver.enum import (
from metadataserver.models import NodeUserData, ScriptResult, ScriptSet
from provisioningserver.drivers.pod import Capabilities, DiscoveredPodHints
from provisioningserver.drivers.power.ipmi import IPMI_BOOT_TYPE
-from provisioningserver.drivers.power.registry import PowerDriverRegistry
+from provisioningserver.drivers.power.registry import (
+ PowerDriverRegistry,
+ SENSITIVE_VALUE_PLACEHOLDER,
+)
from provisioningserver.events import EVENT_DETAILS, EVENT_TYPES
from provisioningserver.refresh.node_info_scripts import (
COMMISSIONING_OUTPUT_NAME,
@@ -6480,6 +6483,25 @@ class TestNodePowerParameters(MAASServerTestCase):
node.save()
self.assertFalse(node.is_sync_healthy)
+ def test_get_power_parameters_masks_sensitive_values(self):
+ parameters = dict(
+ user="tarquin", address="10.1.2.3", power_pass="Test"
+ )
+ node = factory.make_Node(
+ power_type="ipmi", power_parameters=parameters
+ )
+ result = node.get_power_parameters(mask_sensitive=True)
+ self.assertEqual(result["power_pass"], SENSITIVE_VALUE_PLACEHOLDER)
+
+ def test_get_instance_power_parameters_masks_sensitive_values(self):
+ node = factory.make_Node()
+ secret_manager = SecretManager()
+ secret_manager.set_composite_secret(
+ "power-parameters", {"power_pass": "bar"}, obj=node.as_node()
+ )
+ result = node.get_instance_power_parameters(mask_sensitive=True)
+ self.assertEqual(result["power_pass"], SENSITIVE_VALUE_PLACEHOLDER)
+
class TestPowerControlNode(MAASTransactionServerTestCase):
@wait_for_reactor
diff --git a/src/maasserver/websockets/handlers/controller.py b/src/maasserver/websockets/handlers/controller.py
index 67087fb..e16e907 100644
--- a/src/maasserver/websockets/handlers/controller.py
+++ b/src/maasserver/websockets/handlers/controller.py
@@ -192,8 +192,9 @@ class ControllerHandler(NodeHandler):
data["vlan_ids"] = vlan_ids
# include certificate info if present
- certificate = obj.get_power_parameters().get("certificate")
- key = obj.get_power_parameters().get("key")
+ power_params = obj.get_power_parameters()
+ certificate = power_params.get("certificate")
+ key = power_params.get("key")
if certificate and key:
cert = Certificate.from_pem(certificate, key)
data["certificate"] = dehydrate_certificate(cert)
diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
index 63b181f..0c1a6a8 100644
--- a/src/maasserver/websockets/handlers/machine.py
+++ b/src/maasserver/websockets/handlers/machine.py
@@ -411,8 +411,9 @@ class MachineHandler(NodeHandler):
)
# include certificate info if present
- certificate = obj.get_power_parameters().get("certificate")
- key = obj.get_power_parameters().get("key")
+ power_params = obj.get_power_parameters()
+ certificate = power_params.get("certificate")
+ key = power_params.get("key")
if certificate and key:
cert = Certificate.from_pem(certificate, key)
data["certificate"] = dehydrate_certificate(cert)
diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index 5fa038d..41ae627 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -431,7 +431,9 @@ class NodeHandler(TimestampedModelHandler):
data["power_type"] = obj.power_type
data["power_parameters"] = self.dehydrate_power_parameters(
- obj.get_power_parameters()
+ obj.get_power_parameters(
+ mask_sensitive=not self.user.is_superuser
+ )
)
data["power_bmc_node_count"] = (
obj.bmc.node_set.count() if (obj.bmc is not None) else 0
diff --git a/src/maasserver/websockets/handlers/pod.py b/src/maasserver/websockets/handlers/pod.py
index b32a080..af7f7e0 100644
--- a/src/maasserver/websockets/handlers/pod.py
+++ b/src/maasserver/websockets/handlers/pod.py
@@ -140,8 +140,9 @@ class PodHandler(TimestampedModelHandler):
data["boot_vlans"] = []
# include certificate info if present
- certificate = obj.get_power_parameters().get("certificate")
- key = obj.get_power_parameters().get("key")
+ power_params = obj.get_power_parameters()
+ certificate = power_params.get("certificate")
+ key = power_params.get("key")
if certificate and key:
cert = Certificate.from_pem(certificate, key)
data["certificate"] = dehydrate_certificate(cert)
diff --git a/src/maasserver/websockets/handlers/tests/test_controller.py b/src/maasserver/websockets/handlers/tests/test_controller.py
index 038d2c8..c22c27e 100644
--- a/src/maasserver/websockets/handlers/tests/test_controller.py
+++ b/src/maasserver/websockets/handlers/tests/test_controller.py
@@ -169,7 +169,7 @@ class TestControllerHandler(MAASServerTestCase):
# and slowing down the client waiting for the response.
self.assertEqual(
queries,
- 34,
+ 33,
"Number of queries has changed; make sure this is expected.",
)
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index 6b86917..d080b96 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -134,6 +134,9 @@ from metadataserver.enum import (
)
from metadataserver.models.nodekey import NodeKey
from metadataserver.models.scriptset import get_status_from_qs
+from provisioningserver.drivers.power.registry import (
+ SENSITIVE_VALUE_PLACEHOLDER,
+)
from provisioningserver.refresh.node_info_scripts import (
LIST_MODALIASES_OUTPUT_NAME,
LLDP_OUTPUT_NAME,
@@ -814,6 +817,42 @@ class TestMachineHandler(MAASServerTestCase):
[node_info] = list_results["groups"][0]["items"]
self.assertNotIn("certificate", node_info)
+ def test_get_power_params_sensitive_data_hidden_from_non_superuser(self):
+ sample_cert = get_sample_cert()
+ node = factory.make_Node(
+ power_type="lxd",
+ power_parameters={
+ "power_address": "lxd.maas",
+ "certificate": sample_cert.certificate_pem(),
+ "key": sample_cert.private_key_pem(),
+ },
+ )
+ handler = MachineHandler(factory.make_User(), {}, None)
+ result = handler.get({"system_id": node.system_id})
+ self.assertIn("power_parameters", result)
+ self.assertIn("key", result["power_parameters"])
+ self.assertEqual(
+ result["power_parameters"]["key"], SENSITIVE_VALUE_PLACEHOLDER
+ )
+
+ def test_get_power_params_sensitive_data_shown_to_superuser(self):
+ sample_cert = get_sample_cert()
+ node = factory.make_Node(
+ power_type="lxd",
+ power_parameters={
+ "power_address": "lxd.maas",
+ "certificate": sample_cert.certificate_pem(),
+ "key": sample_cert.private_key_pem(),
+ },
+ )
+ handler = MachineHandler(factory.make_admin(), {}, None)
+ result = handler.get({"system_id": node.system_id})
+ self.assertIn("power_parameters", result)
+ self.assertIn("key", result["power_parameters"])
+ self.assertEqual(
+ result["power_parameters"]["key"], sample_cert.private_key_pem()
+ )
+
def test_get_power_params_certificate(self):
sample_cert = get_sample_cert()
node = factory.make_Node(
@@ -867,7 +906,7 @@ class TestMachineHandler(MAASServerTestCase):
queries, _ = count_queries(handler.get, {"system_id": node.system_id})
self.assertEqual(
queries,
- 60,
+ 58,
"Number of queries has changed; make sure this is expected.",
)
diff --git a/src/maasserver/websockets/handlers/tests/test_pod.py b/src/maasserver/websockets/handlers/tests/test_pod.py
index 53211b2..487ab5a 100644
--- a/src/maasserver/websockets/handlers/tests/test_pod.py
+++ b/src/maasserver/websockets/handlers/tests/test_pod.py
@@ -631,6 +631,36 @@ class TestPodHandler(MAASTransactionServerTestCase):
result = handler.full_dehydrate(pod)
self.assertEqual(["edit", "delete", "compose"], result["permissions"])
+ def test_get_power_params_hidden_from_non_superuser(self):
+ sample_cert = get_sample_cert()
+ pod = self.make_pod_with_hints(
+ pod_type="lxd",
+ parameters={
+ "certificate": sample_cert.certificate_pem(),
+ "key": sample_cert.private_key_pem(),
+ },
+ )
+ handler = PodHandler(factory.make_User(), {}, None)
+ result = handler.get({"id": pod.id})
+ self.assertNotIn("power_parameters", result)
+
+ def test_get_power_params_sensitive_data_shown_to_superuser(self):
+ sample_cert = get_sample_cert()
+ pod = self.make_pod_with_hints(
+ pod_type="lxd",
+ parameters={
+ "certificate": sample_cert.certificate_pem(),
+ "key": sample_cert.private_key_pem(),
+ },
+ )
+ handler = PodHandler(factory.make_admin(), {}, None)
+ result = handler.get({"id": pod.id})
+ self.assertIn("power_parameters", result)
+ self.assertIn("key", result["power_parameters"])
+ self.assertEqual(
+ result["power_parameters"]["key"], sample_cert.private_key_pem()
+ )
+
def test_list(self):
admin = factory.make_admin()
handler = PodHandler(admin, {}, None)
diff --git a/src/provisioningserver/drivers/power/registry.py b/src/provisioningserver/drivers/power/registry.py
index 32ab69c..6463d7f 100644
--- a/src/provisioningserver/drivers/power/registry.py
+++ b/src/provisioningserver/drivers/power/registry.py
@@ -32,6 +32,8 @@ from provisioningserver.drivers.power.webhook import WebhookPowerDriver
from provisioningserver.drivers.power.wedge import WedgePowerDriver
from provisioningserver.utils.registry import Registry
+SENSITIVE_VALUE_PLACEHOLDER = "<VALUE HIDDEN BY MAAS>"
+
class PowerDriverRegistry(Registry):
"""Registry for power drivers."""
Follow ups
