Add validation of DirectoryService/AdditionalSssdConfigs: a failure is emitted when the customer is overriding protected properties. Extend AD integration tests to cover the use of DirectoryService/AdditionalSssdConfigs to set a simple access provider.

Signed-off-by: Giacomo Marciani <mgiacomo@amazon.com>

Author: gmarciani

CHANGELOG.md

@@ -8,9 +8,10 @@ CHANGELOG
 - Execute SSH key creation alongside with the creation of HOME directory, i.e.
   during SSH login, when switching to another user and when executing a command as another user.
 - Add support for both FQDN and LDAP Distinguished Names in the configuration parameter `DirectoryService/DomainName`. The new validator now checks both the syntaxes.
-- New `update_directory_service_password.sh` script deployed on the head node supports the manual update of the Active Directory password in the SSSD configuration. 
+- New `update_directory_service_password.sh` script deployed on the head node supports the manual update of the Active Directory password in the SSSD configuration.
   The password is retrieved by the AWS Secrets Manager as from the cluster configuration.
 - Add support to deploy API infrastructure in environments without a default VPC.
+- Add validation for `DirectoryService/AdditionalSssdConfigs` to fail in case of invalid overrides.
 
 **CHANGES**
 - Disable deeper C-States in x86_64 official AMIs and AMIs created through `build-image` command, to guarantee high performance and low latency.

cli/src/pcluster/config/cluster_config.py

@@ -83,6 +83,7 @@
     SharedStorageNameValidator,
 )
 from pcluster.validators.directory_service_validators import (
+    AdditionalSssdConfigsValidator,
     DomainAddrValidator,
     DomainNameValidator,
     LdapTlsReqCertValidator,
@@ -709,6 +710,12 @@ def _register_validators(self):
             )
         if self.ldap_tls_req_cert:
             self._register_validator(LdapTlsReqCertValidator, ldap_tls_reqcert=self.ldap_tls_req_cert)
+        if self.additional_sssd_configs:
+            self._register_validator(
+                AdditionalSssdConfigsValidator,
+                additional_sssd_configs=self.additional_sssd_configs,
+                ldap_access_filter=self.ldap_access_filter,
+            )
 
 
 class ClusterIam(Resource):

cli/src/pcluster/constants.py

@@ -156,3 +156,6 @@
 # see https://docs.aws.amazon.com/cdk/v2/guide/getting_started.html
 NODEJS_MIN_VERSION = "10.13.0"
 NODEJS_INCOMPATIBLE_VERSION_RANGE = ["13.0.0", "13.6.0"]
+
+# DirectoryService
+DIRECTORY_SERVICE_RESERVED_SETTINGS = {"id_provider": "ldap"}

cli/src/pcluster/validators/directory_service_validators.py

@@ -12,6 +12,7 @@
 import re
 from urllib.parse import urlparse
 
+from pcluster.constants import DIRECTORY_SERVICE_RESERVED_SETTINGS
 from pcluster.validators.common import FailureLevel, Validator
 
 
@@ -78,3 +79,32 @@ def _validate(self, ldap_tls_reqcert):
                 f"For security reasons it's recommended to use {' or '.join(values_requiring_cert_validation)}",
                 FailureLevel.WARNING,
             )
+
+
+class AdditionalSssdConfigsValidator(Validator):
+    """AdditionalSssdConfigs validator."""
+
+    def _validate(self, additional_sssd_configs, ldap_access_filter):
+        """Validate that AdditionalSssdConfigs does not introduce unacceptable values."""
+        for config_key, accepted_value in DIRECTORY_SERVICE_RESERVED_SETTINGS.items():
+            if config_key in additional_sssd_configs:
+                actual_value = additional_sssd_configs[config_key]
+                if actual_value != accepted_value:
+                    self._add_failure(
+                        f"Cannot override the SSSD property '{config_key}' "
+                        f"with value '{actual_value}'. "
+                        f"Allowed value is: '{accepted_value}'. "
+                        "Please refer to ParallelCluster official documentation for more information.",
+                        FailureLevel.ERROR,
+                    )
+
+        if "access_provider" in additional_sssd_configs:
+            actual_access_provider = additional_sssd_configs["access_provider"]
+            if ldap_access_filter is not None and actual_access_provider != "ldap":
+                self._add_failure(
+                    "Cannot override the SSSD property 'access_provider' "
+                    f"with value '{actual_access_provider}' when LdapAccessFilter is specified. "
+                    "Allowed value is: 'ldap'. "
+                    "Please refer to ParallelCluster official documentation for more information.",
+                    FailureLevel.ERROR,
+                )

cli/tests/pcluster/validators/test_directory_service_validators.py

@@ -11,6 +11,7 @@
 import pytest
 
 from pcluster.validators.directory_service_validators import (
+    AdditionalSssdConfigsValidator,
     DomainAddrValidator,
     DomainNameValidator,
     LdapTlsReqCertValidator,
@@ -108,3 +109,44 @@ def test_domain_addr_protocol(domain_addr, additional_sssd_configs, expected_mes
 def test_ldap_tls_reqcert_validator(ldap_tls_reqcert, expected_message):
     actual_failures = LdapTlsReqCertValidator().execute(ldap_tls_reqcert=ldap_tls_reqcert)
     assert_failure_messages(actual_failures, expected_message)
+
+
+@pytest.mark.parametrize(
+    "additional_sssd_configs, ldap_access_filter, expected_message",
+    [
+        ("WHATEVER", "WHATEVER", None),
+        (
+            {"id_provider": "ldap", "whatever_property": "WHATEVER"},
+            "WHATEVER",
+            None,
+        ),
+        (
+            {"access_provider": "ldap", "whatever_property": "WHATEVER"},
+            "WHATEVER",
+            None,
+        ),
+        (
+            {"access_provider": "NOT_ldap", "whatever_property": "WHATEVER"},
+            None,
+            None,
+        ),
+        (
+            {"id_provider": "NOT_ldap", "whatever_property": "WHATEVER"},
+            "WHATEVER",
+            "Cannot override the SSSD property 'id_provider' with value 'NOT_ldap'. Allowed value is: 'ldap'. "
+            "Please refer to ParallelCluster official documentation for more information.",
+        ),
+        (
+            {"access_provider": "NOT_ldap", "whatever_property": "WHATEVER"},
+            "WHATEVER",
+            "Cannot override the SSSD property 'access_provider' with value 'NOT_ldap' "
+            "when LdapAccessFilter is specified. Allowed value is: 'ldap'. "
+            "Please refer to ParallelCluster official documentation for more information.",
+        ),
+    ],
+)
+def test_additional_sssd_configs(additional_sssd_configs, ldap_access_filter, expected_message):
+    actual_failures = AdditionalSssdConfigsValidator().execute(
+        additional_sssd_configs=additional_sssd_configs, ldap_access_filter=ldap_access_filter
+    )
+    assert_failure_messages(actual_failures, expected_message)

tests/integration-tests/tests/ad_integration/test_ad_integration.py

@@ -811,6 +811,8 @@ def test_ad_integration(
     _run_user_workloads(users, test_datadir, remote_command_executor)
     logging.info("Testing pcluster update and generate ssh keys for user")
     _check_ssh_key_generation(users[0], remote_command_executor, scheduler_commands, False)
+
+    # Verify access control with ldap access provider.
     updated_config_file = pcluster_config_reader(config_file="pcluster.config.update.yaml", **config_params)
     cluster.update(str(updated_config_file), force_update="true")
     # Reset stateful connection variables after the cluster update
@@ -823,6 +825,20 @@ def test_ad_integration(
         logging.info(f"Checking SSH access for user {user.alias}")
         _check_ssh_auth(user=user, expect_success=user.alias != "PclusterUser2")
 
+    # Verify access control with simple access provider.
+    # With this test we also verify that AdditionalSssdConfigs is working properly.
+    updated_config_file = pcluster_config_reader(config_file="pcluster.config.update2.yaml", **config_params)
+    cluster.update(str(updated_config_file), force_update="true")
+    # Reset stateful connection variables after the cluster update
+    remote_command_executor = RemoteCommandExecutor(cluster)
+    scheduler_commands = get_scheduler_commands(scheduler, remote_command_executor)
+    for user in users:
+        user.reset_stateful_connection_objects(remote_command_executor)
+    _check_ssh_key_generation(users[1], remote_command_executor, scheduler_commands, True)
+    for user in users:
+        logging.info(f"Checking SSH access for user {user.alias}")
+        _check_ssh_auth(user=user, expect_success=user.alias != "PclusterUser0")
+
 
 def _check_ssh_auth(user, expect_success=True):
     try:

tests/integration-tests/tests/ad_integration/test_ad_integration/test_ad_integration/pcluster.config.update2.yaml

@@ -0,0 +1,54 @@
+Image:
+  Os: {{ os }}
+HeadNode:
+  InstanceType: {{ head_node_instance_type }}
+  Networking:
+    SubnetId: {{ public_subnet_id }}
+  Ssh:
+    KeyName: {{ key_name }}
+  Imds:
+    Secured: {{ imds_secured }}
+Scheduling:
+  Scheduler: {{ scheduler }}
+  SlurmQueues:
+    - Name: compute
+      ComputeResources:
+        - Name: cit
+          InstanceType: {{ compute_instance_type }}
+          MinCount: 2
+          MaxCount: 150
+      Networking:
+        SubnetIds:
+          - {{ private_subnet_id }}
+Monitoring:
+  Logs:
+    CloudWatch:
+      Enabled: true
+      RetentionInDays: 14
+SharedStorage:
+  - MountDir: /shared
+    Name: fsx
+    StorageType: FsxLustre
+    FsxLustreSettings:
+      StorageCapacity: 2400
+  - MountDir: /ebs
+    Name: ebs
+    StorageType: Ebs
+  - MountDir: /efs
+    Name: efs
+    StorageType: Efs
+DirectoryService:
+  DomainName: {{ ldap_search_base }}
+  DomainAddr: {{ ldap_uri }}
+  PasswordSecretArn: {{ password_secret_arn }}
+  DomainReadOnlyUser: {{ ldap_default_bind_dn }}
+  LdapTlsCaCert: {{ ldap_tls_ca_cert }}
+  LdapTlsReqCert: {{ ldap_tls_req_cert }}
+  GenerateSshKeysForUsers: true
+  AdditionalSssdConfigs:
+    debug_level: "0x1ff"
+    {% if directory_protocol == "ldap" %}
+    ldap_auth_disable_tls_never_use_in_production: True
+    {% endif %}
+    access_provider: simple
+    simple_deny_users: PclusterUser0