public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Ivan Erben <ivan@erben.sk>
To: pve-devel@lists.proxmox.com
Cc: Ivan Erben <ivan@erben.sk>
Subject: [PATCH pve-access-control] ad: support nested groups during sync
Date: Wed, 29 Jul 2026 17:44:48 +0200	[thread overview]
Message-ID: <20260729154448.81034-1-ivan@erben.sk> (raw)

Add an optional `use_nested_groups` setting that resolves nested
Active Directory group memberships through
LDAP_MATCHING_RULE_IN_CHAIN.

This enables ACLs based on synchronized Active Directory groups to
work correctly when nested groups are used.

Nested group resolution can be significantly slower in large Active
Directory environments and is therefore disabled by default.

Fixes: #2738

Signed-off-by: Ivan Erben <ivan@erben.sk>
---
 src/PVE/Auth/AD.pm | 184 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 184 insertions(+)

diff --git a/src/PVE/Auth/AD.pm b/src/PVE/Auth/AD.pm
index 5d57012..8f09f0c 100755
--- a/src/PVE/Auth/AD.pm
+++ b/src/PVE/Auth/AD.pm
@@ -2,11 +2,20 @@ package PVE::Auth::AD;
 
 use strict;
 use warnings;
+
 use PVE::Auth::LDAP;
 use PVE::LDAP;
 
+use Net::LDAP::Control::Paged;
+use Net::LDAP::Constant qw(LDAP_CONTROL_PAGED);
+use Net::LDAP::Util qw(escape_filter_value);
+
 use base qw(PVE::Auth::LDAP);
 
+# Active Directory LDAP_MATCHING_RULE_IN_CHAIN
+# Used for transitive/nested group membership resolution.
+use constant AD_MATCHING_RULE_IN_CHAIN => '1.2.840.113556.1.4.1941';
+
 sub type {
     return 'ad';
 }
@@ -63,6 +72,16 @@ sub properties {
             optional => 1,
             maxLength => 256,
         },
+        use_nested_groups => {
+            description =>
+                "Enable nested Active Directory group resolution using "
+                . "LDAP_MATCHING_RULE_IN_CHAIN "
+                . "(1.2.840.113556.1.4.1941). "
+                . "Expands transitive group membership during realm sync. "
+                . "May significantly increase sync time in large AD environments.",
+            type => 'boolean',
+            optional => 1,
+        },
         tfa => PVE::JSONSchema::get_standard_option('tfa'),
     };
 }
@@ -96,9 +115,174 @@ sub options {
         'sync-defaults-options' => { optional => 1 },
         mode => { optional => 1 },
         'case-sensitive' => { optional => 1 },
+        use_nested_groups => { optional => 1 },
     };
 }
 
+sub query_ad_nested_members {
+    my ($ldap, $base_dn, $group_dn, $fallback_members) = @_;
+
+    my $escaped_dn = escape_filter_value($group_dn);
+
+    # Use both objectCategory=person and objectClass=user:
+    #
+    # - objectCategory=person excludes computer objects
+    # - objectClass=user excludes contacts
+    #
+    # Combined filter most closely matches real AD user accounts.
+    my $filter =
+        "(&(objectCategory=person)"
+        . "(objectClass=user)"
+        . "(memberOf:"
+        . AD_MATCHING_RULE_IN_CHAIN
+        . ":=$escaped_dn))";
+
+    my $page = Net::LDAP::Control::Paged->new(size => 900);
+
+    my @args = (
+        base      => $base_dn,
+        scope     => 'subtree',
+        filter    => $filter,
+        attrs     => ['dn'],
+        control   => [$page],
+        timelimit => 120,
+    );
+
+    my %seen;
+    my $cookie;
+    my $had_paging = 0;
+    my $nested_error;
+
+    eval {
+        while (1) {
+            my $mesg = $ldap->search(@args);
+
+            if ($mesg->code) {
+                die $mesg->error;
+            }
+
+            foreach my $entry ($mesg->entries) {
+                $seen{lc($entry->dn)} = $entry->dn;
+            }
+
+            my ($resp) = $mesg->control(LDAP_CONTROL_PAGED);
+            last if !$resp;
+
+            $had_paging = 1;
+            $cookie = $resp->cookie;
+
+            last if !defined($cookie) || !length($cookie);
+
+            $page->cookie($cookie);
+        }
+    };
+
+    if (my $err = $@) {
+        $nested_error = $err;
+    }
+
+    # Explicitly terminate paged search to avoid leaving
+    # server-side paging state allocated.
+    if ($had_paging) {
+        eval {
+            $page->cookie($cookie // '');
+            $page->size(0);
+            $ldap->search(@args);
+        };
+    }
+
+    if ($nested_error) {
+        chomp($nested_error);
+
+        warn "AD nested group lookup failed for '$group_dn': "
+            . "$nested_error - falling back to direct group members\n";
+
+        return $fallback_members;
+    }
+
+    return [ values %seen ];
+}
+
+sub get_groups {
+    my ($class, $config, $realm, $dnmap) = @_;
+
+    # Use default LDAP group handling unless nested
+    # membership resolution is explicitly enabled.
+    return $class->SUPER::get_groups($config, $realm, $dnmap)
+        if !$config->{use_nested_groups};
+
+    my $filter = $config->{group_filter};
+
+    # Use base_dn for nested member searches because
+    # users may exist outside the group subtree.
+    my $group_basedn = $config->{group_dn} // $config->{base_dn};
+    my $user_basedn = $config->{base_dn};
+
+    my $attr = $config->{group_name_attr};
+
+    $config->{group_classes}
+        //= 'groupOfNames, group, univentionGroup, ipausergroup';
+
+    my $classes = [
+        PVE::Tools::split_list($config->{group_classes})
+    ];
+
+    my $ldap = $class->connect_and_bind($config, $realm);
+
+    # Reuse generic LDAP group discovery and perform
+    # nested expansion only for AD realms.
+    my $groups = PVE::LDAP::query_groups(
+        $ldap,
+        $group_basedn,
+        $classes,
+        $filter,
+        $attr,
+    );
+
+    my $ret = {};
+
+    foreach my $group (@$groups) {
+        my $name = $group->{name};
+
+        if (!$name && $group->{dn} =~ m/^[^=]+=([^,]+),/) {
+            $name = PVE::Tools::trim($1);
+        }
+
+        next if !$name;
+
+        $name .= "-$realm";
+
+        eval {
+            PVE::AccessControl::verify_groupname($name);
+        };
+
+        if (my $err = $@) {
+            warn "$err";
+            next;
+        }
+
+        $ret->{$name} = { users => {} };
+
+        my $direct_members = $group->{members} // [];
+
+        my $members = query_ad_nested_members(
+            $ldap,
+            $user_basedn,
+            $group->{dn},
+            $direct_members,
+        );
+
+        foreach my $member (@$members) {
+            my $user = $dnmap->{lc($member)};
+            next if !$user;
+
+            $ret->{$name}->{users}->{$user} = 1;
+        }
+    }
+
+    return $ret;
+}
+
 sub get_users {
     my ($class, $config, $realm) = @_;
 
-- 
2.47.3




                 reply	other threads:[~2026-07-30 14:19 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729154448.81034-1-ivan@erben.sk \
    --to=ivan@erben.sk \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal