Blob Blame History Raw
From 2ab6c0fc96adb90880f9bc23661ed3997b0a7023 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Mon, 27 Sep 2021 17:07:13 +0200
Subject: [PATCH 01/22] Design: Integrate SID configuration into base IPA
 installers

Add design doc for the feature.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
 doc/designs/adtrust/sidconfig.md | 301 +++++++++++++++++++++++++++++++
 doc/designs/index.rst            |   1 +
 2 files changed, 302 insertions(+)
 create mode 100644 doc/designs/adtrust/sidconfig.md

diff --git a/doc/designs/adtrust/sidconfig.md b/doc/designs/adtrust/sidconfig.md
new file mode 100644
index 000000000..f65d10529
--- /dev/null
+++ b/doc/designs/adtrust/sidconfig.md
@@ -0,0 +1,301 @@
+#  Integrate SID configuration into base IPA installers
+
+## Overview
+
+FreeIPA is able to issue Kerberos tickets with PAC data when it is configured
+for trust. The goal of this feature is to allow PAC generation in the
+general use case, even without trust, as it is a first step towards
+IPA-IPA trust.
+
+Reference: https://pagure.io/freeipa/issue/8995
+
+In order to generate PAC data for a kerberos principal, IPA needs to assign
+a SID to the users and groups. IPA installers (`ipa-server-install`,
+`ipa-replica-install`) must handle the configuration related to SID:
+- assign a NetBIOS name to the server
+- assign a domain SID to the IPA domain
+- configure the local id range with primary RID base and secondary RID base
+- enable the sidgen plugin on 389ds server
+
+The code handling these steps is already available in ipa-adtrust-install
+and needs to be moved to the general installers.
+
+## Use Cases
+
+### Fresh install
+As administrator, I want to deploy an IPA topology that automatically
+assigns SIDs to users and groups and issues kerberos tickets with PAC data,
+without the need to configure IPA for trust.
+
+If I later decide to configure IPA for trust, I can still run the command
+`ipa-adtrust-install`.
+
+### Existing installation, no trust
+
+As IPA administrator, I am managing an existing IPA installation without any
+trust. I want to update IPA and have the choice of enabling (or not) the
+generation of SIDs and PAC data.
+
+If I choose to enable PAC data, I just need to run a command that requires
+an admin kerberos ticket. The command handles the SID-related configuration 
+and prompts me whether I want to immediately generate SIDs for
+existing users/groups or run that task later.
+
+### Existing installation, trust configured
+
+In this topology with trust configured (the command `ipa-adtrust-install` has 
+been executed), IPA is already able to issue Kerberos tickets with PAC data.
+Update does not modify the configuration.
+
+### Mixed topology
+
+As IPA administrator, I am managing an existing server setup without trust and
+without PAC data. I want to add a replica into my existing topology and
+automatically enable PAC data generation, without the need to configure IPA
+for trust.
+
+## How to Use
+
+### Fresh installation
+Run `ipa-server-install`, with the ability to define:
+- NetBIOS Name: if not specified, the default value is generated from
+the IPA domain name (transform the left-most label into uppercase, keep only
+ascii characters, digits and dashes).
+- Primary RID base (default if not set: 1000)
+- Secondary RID base (default if not set: 1000000)
+
+On a replica: run `ipa-replica-install`, with the ability to define the same
+parameters as `ipa-server-install`, plus the following parameter:
+- Run SIDgen task immediately: yes/no (default=no)
+
+If conflicting values are specified in `ipa-replica-install`, the installer
+must warn that existing values will be overwritten (same message as when
+`ipa-adtrust-install` is run multiple times with conflicting values).
+
+### Upgrade
+
+- Run `dnf update *ipa-server`. The upgrade doesn't configure SID-related
+options and doesn't enable PAC generation.
+- Obtain a ticket for the admin (or any member of the admins group).
+- Run the new command allowing to setup the SID-related configuration.
+- Run the existing command to modify base RID and secondary base RID:
+```
+ipa idrange-mod --rid-base INT --secondary-rid-base INT RANGENAME
+```
+
+### Mixed topology
+
+The existing server is not setup for PAC data generation. The future replica
+is installed with the latest packages, containing the updated installers.
+
+Run `ipa-replica-install`, with the ability to define:
+- NetBIOS Name: if not specified, the default value is generated from
+the IPA domain name (transform the left-most label into uppercase, keep only
+ascii characters, digits and dashes).
+- Primary RID base (default if not set: 1000)
+- Secondary RID base (default if not set: 1000000)
+- Run SIDgen task immediately: yes/no (default=no)
+
+
+## Design
+
+Installers and SID-related options:
+- the options `--add-sids`, `--netbios-name`, `--rid-base` and 
+`--secondary-rid-base` are moved from ADTrustInstallInterface to a separate
+new InstallInterface: SIDInstallInterface.
+- ADTrustInstallInterface now inherits from SIDInstallInterface.
+- `adtrustinstance.ADTRUSTInstance.__init__` now accepts an additional
+parameter: `fulltrust`. When set to True, the class ADTRUSTInstance configures
+the trust as usual. When set to False, only the SID-related part is executed.
+- `ipa-server-install` and `ipa-replica-install` now always call
+`adtrust.install_check` and `adtrust.install`, but the method is using the
+provided options (especially `options.setup_adtrust`) to know if full
+configuration is required or only the SID-related part.
+
+The `ipa-adtrust-install` code is already written in order to be
+idempotent (can be called multiple times, even with different values), and
+this property is of great value for this feature. It allows to keep
+the changes as minimal as possible to the existing installers, and
+call `ipa-adtrust-install` even if the SID-part is already setup.
+
+New command to enable SID generation after an upgrade:
+as we don't want to automatically enable SID generation during an upgrade,
+a new command is provided in order to manually enable the feature. The command
+only requires an admin ticket (no need to be root) and relies on the admin
+framework.
+
+The command uses a mechanism similar to server_conncheck:
+- the user must have Replication Administrators privilege
+- the user launches an ipa command communicating with IPA framework
+- the admin framework uses Dbus to get access a DBus interface that
+launches a command through oddjob, allowing the command to run as root.
+The oddjob command is a wrapper around `adtrustinstance.ADTRUSTInstance`.
+
+## Implementation
+
+- Dependencies: no additional dependency on ipa-server package
+- Backup and Restore: no new file to backup
+
+## Feature Management
+
+### UI
+
+No new UI for server/replica installation.
+
+`IPA server / ID ranges` tab is already available and displays Primary and 
+Secondary RID base.
+
+`IPA Server / Trusts / Global Trust Configuration` tab already displays the
+NetBIOS Name and Security Identifier.
+
+These values can also be added in the `IPA Server / Configuration` tab.
+
+The User View displays SMB attributes if they exist and could be enhanced
+with a note if the user entry does not have any SID, pointing the admin
+to a procedure in order to generate the missing SIDs.
+
+### CLI
+
+| Command |	Options |
+| --- | ----- |
+| ipa-server-install | [--netbios-name=NETBIOS_NAME] [--rid-base=RID_BASE] [--secondary-rid-base=SECONDARY_RID_BASE]  |
+| ipa-replica-install | [--netbios-name=NETBIOS_NAME] [--rid-base=RID_BASE] [--secondary-rid-base=SECONDARY_RID_BASE] [--add-sids] |
+| ipa config-mod | --enable-sid [--add-sids] [--netbios-name=NETBIOS_NAME]  |
+
+#### `ipa config-mod --enable-sid` details:
+
+The `--enable-sid` option turns the feature on, and `--add-sids` triggers
+the SIDgen task in order to immediately generate SID for existing users or
+groups.
+
+`--add-sids` requires the `--enable-sid`option.
+
+The `--netbios-name` option specifies the NetBIOS Name and is optional. If
+not provided, the NetBIOS Name is generated from the leading part of the
+DNS name.
+
+`--netbios-name` requires the `--enable-sid` option.
+
+
+### Configuration
+
+When the feature is turned on, it is possible to modify the primary and
+secondary RID bases for the local id range with the existing
+`ipa idrange-mod` command.
+
+The NetBIOS Name can be overwritten (with warning) when `ipa-adtrust-install`
+is run.
+
+## Upgrade
+
+The upgrade does not turn on the feature. If the admin wants to enable
+SID generation, he needs to update the packages and run the new command
+`ipa config-mod --enable-sid`.
+
+
+## Test plan
+
+Note: PRCI currently doesn't have the ability to test upgrade.
+
+Scenarios to be tested: fresh install, test PAC generation
+- Add active user testuser (reinitialize password)
+- On IPA server run `kinit -k` to get a ticket for `host/fqdn@REALM`
+- On IPA server run `/usr/libexec/ipa/ipa-print-pac ticket testuser` and
+ensure that PAC data is properly displayed.
+- Same test on IPA replica
+
+Tests outside of PRCI:
+- Existing topology without trust, update one server, ensure SID generation
+hasn't been automatically enabled
+- Existing topology without trust, update one server, manually enable SID
+generation with the new command
+- Existing topology without trust, insert a new replica, ensure SID generation
+has been automatically enabled
+- Existing topology with trust, update one server, ensure SID generation is
+still working
+- Existing topology with trust, insert a new replica, ensure SID generation
+is still working
+- Ensure that `ipa-adtrust-install` is still working with the previous
+scenarios.
+
+## Troubleshooting and debugging
+
+When the feature is turned on (whatever the path, either through fresh
+server installation, fresh replica installation, or with the new command), the
+following LDAP entries are expected:
+
+- `cn=ad,cn=trusts,dc=ipa,dc=test` which is a simple `nsContainer`
+- `cn=ad,cn=etc,dc=ipa,dc=test` which is a simple `nsContainer`
+- `cn=ipa.test,cn=ad,cn=etc,dc=ipa,dc=test`: must define the NetBIOS Name in
+`ipaNTFlatName`, the SID in `ipaNTSecurityIdentifier` and the default SMB
+group in `ipaNTFallbackPrimaryGroup`
+- `cn=Default SMB Group,cn=groups,cn=accounts,dc=ipa,dc=test`: must define
+a SID belonging to the IPA domain SID in `ipaNTSecurityIdentifier`
+
+(replace ipa.test with the actual domain name and dc=ipa,dc=test with the
+actual base DN).
+
+The admin user must have a SID ending with -500 (well-known SID for the
+domain administrator):
+```
+# ipa user-show admin --all | grep ipantsecurityidentifier
+  ipantsecurityidentifier: S-1-5-21-2633809701-976279387-419745629-500
+```
+
+The admins group must have a SID ending with -512 (well-known SID for domain
+administrators group):
+```
+# ipa group-show admins --all | grep ipantsecurityidentifier
+  ipantsecurityidentifier: S-1-5-21-2633809701-976279387-419745629-512
+```
+
+The sidgen plugin must be enabled in /etc/dirsrv/slapd-IPA-TEST/dse.ldif:
+```
+dn: cn=IPA SIDGEN,cn=plugins,cn=config
+cn: IPA SIDGEN
+nsslapd-basedn: dc=ipa,dc=test
+nsslapd-plugin-depends-on-type: database
+nsslapd-pluginDescription: Add a SID to newly added or modified objects with u
+ id pr gid numbers
+nsslapd-pluginEnabled: on
+nsslapd-pluginId: IPA SIDGEN postop plugin
+nsslapd-pluginInitfunc: ipa_sidgen_init
+nsslapd-pluginPath: libipa_sidgen
+nsslapd-pluginType: postoperation
+nsslapd-pluginVendor: FreeIPA project
+nsslapd-pluginVersion: FreeIPA/1.0
+objectClass: top
+objectClass: nsSlapdPlugin
+objectClass: extensibleObject
+
+```
+
+If the PAC data is not generated for a user, ensure that the user contains a
+SID in its `ipantsecurityidentifier` attribute. If the SID is missing, run
+the SIDgen task in order to generate SID for existing users and groups:
+- Find ipa base DN with: `grep basedn /etc/ipa/default.conf | cut -d= -f2-`
+- Copy `/usr/share/ipa/ipa-sidgen-task-run.ldif` to
+`/tmp/ipa-sidgen-task-run.ldif`
+- Edit the copy `/tmp/ipa-sidgen-task-run.ldif` and replace $SUFFIX with
+IPA base DN
+- As root, launch the task (replace IPA-TEST with the proper value):
+`ldapmodify -H ldapi://%2Frun%2Fslapd-IPA-TEST.socket -f /tmp/ipa-sidgen-task-run.ldif`
+
+In order to check if the PAC data gets added to the user ticket, on a server:
+```
+# kinit -k
+# /usr/libexec/ipa/ipa-print-pac ticket USERNAME
+```
+
+If the PAC data is properly added, the `ipa-print-pac` command displays:
+```
+# /usr/libexec/ipa/ipa-print-pac ticket testuser
+Password: 
+Acquired credentials for testuser
+PAC_DATA: struct PAC_DATA
+    num_buffers              : 0x00000005 (5)
+    version                  : 0x00000000 (0)
+    buffers: ARRAY(5)
+        buffers: struct PAC_BUFFER
+...
+```
\ No newline at end of file
diff --git a/doc/designs/index.rst b/doc/designs/index.rst
index cbec1096c..9f7185dc2 100644
--- a/doc/designs/index.rst
+++ b/doc/designs/index.rst
@@ -10,6 +10,7 @@ FreeIPA design documentation
    adtrust/admin-ipa-as-trusted-user.md
    adtrust/sudorules-with-ad-objects.md
    adtrust/auto-private-groups.md
+   adtrust/sidconfig.md
    krb-ticket-policy.md
    extdom-plugin-protocol.md
    expiring-password-notification.md
-- 
2.33.1


From 559b5f305b7dd1f4e6436d09074397a4a780fe8b Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Mon, 27 Sep 2021 08:36:32 +0200
Subject: [PATCH 02/22] SID generation: define SIDInstallInterface

Move the SID-related options into a separate InstallInterface
(--add-sids, --netbios-name, --rid-base and --secondary-rid-base),
make ADTrustInstallInterface inherit from SIDInstallInterface.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipaserver/install/adtrust.py | 56 ++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/ipaserver/install/adtrust.py b/ipaserver/install/adtrust.py
index ea279b56b..0409743ee 100644
--- a/ipaserver/install/adtrust.py
+++ b/ipaserver/install/adtrust.py
@@ -530,43 +530,26 @@ def generate_dns_service_records_help(api):
 
 
 @group
-class ADTrustInstallInterface(ServiceAdminInstallInterface):
+class SIDInstallInterface(ServiceAdminInstallInterface):
     """
-    Interface for the AD trust installer
+    Interface for the SID generation Installer
 
     Knobs defined here will be available in:
     * ipa-server-install
     * ipa-replica-install
     * ipa-adtrust-install
     """
-    description = "AD trust"
-
-    # the following knobs are provided on top of those specified for
-    # admin credentials
+    description = "SID generation"
     add_sids = knob(
         None,
         description="Add SIDs for existing users and groups as the final step"
     )
-    add_agents = knob(
-        None,
-        description="Add IPA masters to a list of hosts allowed to "
-                    "serve information about users from trusted forests"
-    )
-    add_agents = replica_install_only(add_agents)
-    enable_compat = knob(
-        None,
-        description="Enable support for trusted domains for old clients"
-    )
+    add_sids = replica_install_only(add_sids)
     netbios_name = knob(
         str,
         None,
         description="NetBIOS name of the IPA domain"
     )
-    no_msdcs = knob(
-        None,
-        description="Deprecated: has no effect",
-        deprecated=True
-    )
     rid_base = knob(
         int,
         1000,
@@ -578,3 +561,34 @@ class ADTrustInstallInterface(ServiceAdminInstallInterface):
         description="Start value of the secondary range for mapping "
                     "UIDs and GIDs to RIDs"
     )
+
+
+@group
+class ADTrustInstallInterface(SIDInstallInterface):
+    """
+    Interface for the AD trust installer
+
+    Knobs defined here will be available in:
+    * ipa-server-install
+    * ipa-replica-install
+    * ipa-adtrust-install
+    """
+    description = "AD trust"
+
+    # the following knobs are provided on top of those specified for
+    # admin credentials
+    add_agents = knob(
+        None,
+        description="Add IPA masters to a list of hosts allowed to "
+                    "serve information about users from trusted forests"
+    )
+    add_agents = replica_install_only(add_agents)
+    enable_compat = knob(
+        None,
+        description="Enable support for trusted domains for old clients"
+    )
+    no_msdcs = knob(
+        None,
+        description="Deprecated: has no effect",
+        deprecated=True
+    )
-- 
2.33.1


From 32ba72a5dd9ceafd332083898b20deac812ec49d Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Mon, 27 Sep 2021 11:44:43 +0200
Subject: [PATCH 03/22] Installers: configure sid generation in server/replica
 installer

ADTRUSTInstance performs only sid configuration when it is
called without --setup-adtrust.

Update man pages for ipa-server-install and ipa-replica-install
with the SID-related options.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 install/tools/ipa-adtrust-install.in       |  2 +
 install/tools/man/ipa-replica-install.1    | 30 ++++----
 install/tools/man/ipa-server-install.1     | 11 +--
 ipaserver/install/adtrust.py               | 45 +++++------
 ipaserver/install/adtrustinstance.py       | 86 +++++++++++++---------
 ipaserver/install/server/__init__.py       |  5 --
 ipaserver/install/server/install.py        | 11 ++-
 ipaserver/install/server/replicainstall.py | 10 ++-
 8 files changed, 113 insertions(+), 87 deletions(-)

diff --git a/install/tools/ipa-adtrust-install.in b/install/tools/ipa-adtrust-install.in
index 03efc8dec..9cb89ea11 100644
--- a/install/tools/ipa-adtrust-install.in
+++ b/install/tools/ipa-adtrust-install.in
@@ -209,6 +209,8 @@ def main():
         raise ScriptError(
             "Unrecognized error during check of admin rights: %s" % e)
 
+    # Force options.setup_adtrust
+    options.setup_adtrust = True
     adtrust.install_check(True, options, api)
     adtrust.install(True, options, fstore, api)
 
diff --git a/install/tools/man/ipa-replica-install.1 b/install/tools/man/ipa-replica-install.1
index 44fce10ba..7f88303d2 100644
--- a/install/tools/man/ipa-replica-install.1
+++ b/install/tools/man/ipa-replica-install.1
@@ -205,10 +205,7 @@ Do not automatically create DNS SSHFP records.
 \fB\-\-no\-dnssec\-validation\fR
 Disable DNSSEC validation on this server.
 
-.SS "AD TRUST OPTIONS"
-.TP
-\fB\-\-setup\-adtrust\fR
-Configure AD Trust capability on a replica.
+.SS "SID GENERATION OPTIONS"
 .TP
 \fB\-\-netbios\-name\fR=\fINETBIOS_NAME\fR
 The NetBIOS name for the IPA domain. If not provided then this is determined
@@ -227,6 +224,21 @@ ipa\-adtrust\-install is run and scheduled independently. To start this task
 you have to load an edited version of ipa-sidgen-task-run.ldif with the
 ldapmodify command info the directory server.
 .TP
+\fB\-\-rid-base\fR=\fIRID_BASE\fR
+First RID value of the local domain. The first Posix ID of the local domain will
+be assigned to this RID, the second to RID+1 etc. See the online help of the
+idrange CLI for details.
+.TP
+\fB\-\-secondary-rid-base\fR=\fISECONDARY_RID_BASE\fR
+Start value of the secondary RID range, which is only used in the case a user
+and a group share numerically the same Posix ID. See the online help of the
+idrange CLI for details.
+
+.SS "AD TRUST OPTIONS"
+.TP
+\fB\-\-setup\-adtrust\fR
+Configure AD Trust capability on a replica.
+.TP
 \fB\-\-add\-agents\fR
 Add IPA masters to the list that allows to serve information about
 users from trusted forests. Starting with IPA 4.2, a regular IPA master
@@ -240,16 +252,6 @@ information about users from trusted forests only if they are enabled
 via \ipa-adtrust\-install run on any other IPA master. At least SSSD
 version 1.13 on IPA master is required to be able to perform as a trust agent.
 .TP
-\fB\-\-rid-base\fR=\fIRID_BASE\fR
-First RID value of the local domain. The first Posix ID of the local domain will
-be assigned to this RID, the second to RID+1 etc. See the online help of the
-idrange CLI for details.
-.TP
-\fB\-\-secondary-rid-base\fR=\fISECONDARY_RID_BASE\fR
-Start value of the secondary RID range, which is only used in the case a user
-and a group share numerically the same Posix ID. See the online help of the
-idrange CLI for details.
-.TP
 \fB\-\-enable\-compat\fR
 Enables support for trusted domains users for old clients through Schema Compatibility plugin.
 SSSD supports trusted domains natively starting with version 1.9. For platforms that
diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index fdb0f4cb3..e2829ad23 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -230,11 +230,7 @@ Disable DNSSEC validation on this server.
 \fB\-\-allow\-zone\-overlap\fR
 Allow creation of (reverse) zone even if the zone is already resolvable. Using this option is discouraged as it result in later problems with domain name resolution.
 
-.SS "AD TRUST OPTIONS"
-
-.TP
-\fB\-\-setup\-adtrust\fR
-Configure AD Trust capability.
+.SS "SID GENERATION OPTIONS"
 .TP
 \fB\-\-netbios\-name\fR=\fINETBIOS_NAME\fR
 The NetBIOS name for the IPA domain. If not provided, this is determined
@@ -252,6 +248,11 @@ idrange CLI for details.
 Start value of the secondary RID range, which is only used in the case a user
 and a group share numerically the same POSIX ID. See the online help of the
 idrange CLI for details.
+
+.SS "AD TRUST OPTIONS"
+.TP
+\fB\-\-setup\-adtrust\fR
+Configure AD Trust capability.
 .TP
 \fB\-\-enable\-compat\fR
 Enables support for trusted domains users for old clients through Schema Compatibility plugin.
diff --git a/ipaserver/install/adtrust.py b/ipaserver/install/adtrust.py
index 0409743ee..c01748bc0 100644
--- a/ipaserver/install/adtrust.py
+++ b/ipaserver/install/adtrust.py
@@ -413,7 +413,7 @@ def install_check(standalone, options, api):
     global netbios_name
     global reset_netbios_name
 
-    if not standalone:
+    if options.setup_adtrust and not standalone:
         check_for_installed_deps()
 
     realm_not_matching_domain = (api.env.domain.upper() != api.env.realm)
@@ -432,26 +432,27 @@ def install_check(standalone, options, api):
     # Check if /etc/samba/smb.conf already exists. In case it was not generated
     # by IPA, print a warning that we will break existing configuration.
 
-    if adtrustinstance.ipa_smb_conf_exists():
-        if not options.unattended:
-            print("IPA generated smb.conf detected.")
-            if not ipautil.user_input("Overwrite smb.conf?",
-                                      default=False,
-                                      allow_empty=False):
-                raise ScriptError("Aborting installation.")
-
-    elif os.path.exists(paths.SMB_CONF):
-        print("WARNING: The smb.conf already exists. Running "
-              "ipa-adtrust-install will break your existing samba "
-              "configuration.\n\n")
-        if not options.unattended:
-            if not ipautil.user_input("Do you wish to continue?",
-                                      default=False,
-                                      allow_empty=False):
-                raise ScriptError("Aborting installation.")
-
-    if not options.unattended and not options.enable_compat:
-        options.enable_compat = enable_compat_tree()
+    if options.setup_adtrust:
+        if adtrustinstance.ipa_smb_conf_exists():
+            if not options.unattended:
+                print("IPA generated smb.conf detected.")
+                if not ipautil.user_input("Overwrite smb.conf?",
+                                          default=False,
+                                          allow_empty=False):
+                    raise ScriptError("Aborting installation.")
+
+        elif os.path.exists(paths.SMB_CONF):
+            print("WARNING: The smb.conf already exists. Running "
+                  "ipa-adtrust-install will break your existing samba "
+                  "configuration.\n\n")
+            if not options.unattended:
+                if not ipautil.user_input("Do you wish to continue?",
+                                          default=False,
+                                          allow_empty=False):
+                    raise ScriptError("Aborting installation.")
+
+        if not options.unattended and not options.enable_compat:
+            options.enable_compat = enable_compat_tree()
 
     netbios_name, reset_netbios_name = set_and_check_netbios_name(
         options.netbios_name, options.unattended, api)
@@ -467,7 +468,7 @@ def install(standalone, options, fstore, api):
         print("Please wait until the prompt is returned.")
         print("")
 
-    smb = adtrustinstance.ADTRUSTInstance(fstore)
+    smb = adtrustinstance.ADTRUSTInstance(fstore, options.setup_adtrust)
     smb.realm = api.env.realm
     smb.autobind = ipaldap.AUTOBIND_ENABLED
     smb.setup(api.env.host, api.env.realm,
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index a7a403f37..abf5d7faf 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -148,7 +148,7 @@ class ADTRUSTInstance(service.Service):
     OBJC_DOMAIN = "ipaNTDomainAttrs"
     FALLBACK_GROUP_NAME = u'Default SMB Group'
 
-    def __init__(self, fstore=None):
+    def __init__(self, fstore=None, fulltrust=True):
         self.netbios_name = None
         self.reset_netbios_name = None
         self.add_sids = None
@@ -162,10 +162,15 @@ class ADTRUSTInstance(service.Service):
 
         self.fqdn = None
         self.host_netbios_name = None
+        self.fulltrust = fulltrust
 
-        super(ADTRUSTInstance, self).__init__(
-            "smb", service_desc="CIFS", fstore=fstore, service_prefix=u'cifs',
-            keytab=paths.SAMBA_KEYTAB)
+        if self.fulltrust:
+            super(ADTRUSTInstance, self).__init__(
+                "smb", service_desc="CIFS", fstore=fstore,
+                service_prefix=u'cifs',
+                keytab=paths.SAMBA_KEYTAB)
+        else:
+            super(ADTRUSTInstance, self).__init__("SID generation")
 
         self.__setup_default_attributes()
 
@@ -199,12 +204,13 @@ class ADTRUSTInstance(service.Service):
                              api.env.container_cifsdomains,
                              self.suffix)
 
-        self.cifs_agent = DN(('krbprincipalname', self.principal.lower()),
-                             api.env.container_service,
-                             self.suffix)
-        self.host_princ = DN(('fqdn', self.fqdn),
-                             api.env.container_host,
-                             self.suffix)
+        if self.fulltrust:
+            self.cifs_agent = DN(('krbprincipalname', self.principal.lower()),
+                                 api.env.container_service,
+                                 self.suffix)
+            self.host_princ = DN(('fqdn', self.fqdn),
+                                 api.env.container_host,
+                                 self.suffix)
 
 
     def __gen_sid_string(self):
@@ -527,7 +533,7 @@ class ADTRUSTInstance(service.Service):
         try:
             current = api.Backend.ldap2.get_entry(targets_dn)
             members = current.get('memberPrincipal', [])
-            if not(self.principal in members):
+            if self.principal not in members:
                 current["memberPrincipal"] = members + [self.principal]
                 api.Backend.ldap2.update_entry(current)
             else:
@@ -819,45 +825,59 @@ class ADTRUSTInstance(service.Service):
         self.sub_dict['IPA_LOCAL_RANGE'] = get_idmap_range(self.realm)
 
     def create_instance(self):
-        self.step("validate server hostname",
-                  self.__validate_server_hostname)
-        self.step("stopping smbd", self.__stop)
+        if self.fulltrust:
+            self.step("validate server hostname",
+                      self.__validate_server_hostname)
+            self.step("stopping smbd", self.__stop)
         self.step("creating samba domain object", \
                   self.__create_samba_domain_object)
-        self.step("retrieve local idmap range", self.__retrieve_local_range)
-        self.step("writing samba config file", self.__write_smb_conf)
-        self.step("creating samba config registry", self.__write_smb_registry)
-        self.step("adding cifs Kerberos principal",
-                  self.request_service_keytab)
-        self.step("adding cifs and host Kerberos principals to the adtrust agents group", \
+        if self.fulltrust:
+            self.step("retrieve local idmap range",
+                      self.__retrieve_local_range)
+            self.step("writing samba config file", self.__write_smb_conf)
+            self.step("creating samba config registry",
+                      self.__write_smb_registry)
+            self.step("adding cifs Kerberos principal",
+                      self.request_service_keytab)
+            self.step("adding cifs and host Kerberos principals to the "
+                      "adtrust agents group",
                   self.__setup_group_membership)
-        self.step("check for cifs services defined on other replicas", self.__check_replica)
-        self.step("adding cifs principal to S4U2Proxy targets", self.__add_s4u2proxy_target)
+            self.step("check for cifs services defined on other replicas",
+                      self.__check_replica)
+            self.step("adding cifs principal to S4U2Proxy targets",
+                      self.__add_s4u2proxy_target)
         self.step("adding admin(group) SIDs", self.__add_admin_sids)
         self.step("adding RID bases", self.__add_rid_bases)
         self.step("updating Kerberos config", self.__update_krb5_conf)
-        self.step("activating CLDAP plugin", self.__add_cldap_module)
+        if self.fulltrust:
+            self.step("activating CLDAP plugin", self.__add_cldap_module)
         self.step("activating sidgen task", self.__add_sidgen_task)
-        self.step("map BUILTIN\\Guests to nobody group",
-                  self.__map_Guests_to_nobody)
-        self.step("configuring smbd to start on boot", self.__enable)
+        if self.fulltrust:
+            self.step("map BUILTIN\\Guests to nobody group",
+                      self.__map_Guests_to_nobody)
+            self.step("configuring smbd to start on boot", self.__enable)
 
         if self.enable_compat:
-            self.step("enabling trusted domains support for older clients via Schema Compatibility plugin",
+            self.step("enabling trusted domains support for older clients via "
+                      "Schema Compatibility plugin",
                       self.__enable_compat_tree)
 
-        self.step("restarting Directory Server to take MS PAC and LDAP plugins changes into account", \
+        self.step("restarting Directory Server to take MS PAC and LDAP "
+                  "plugins changes into account",
                   self.__restart_dirsrv)
         self.step("adding fallback group", self.__add_fallback_group)
-        self.step("adding Default Trust View", self.__add_default_trust_view)
-        self.step("setting SELinux booleans", \
-                  self.__configure_selinux_for_smbd)
-        self.step("starting CIFS services", self.__start)
+        if self.fulltrust:
+            self.step("adding Default Trust View",
+                      self.__add_default_trust_view)
+            self.step("setting SELinux booleans",
+                      self.__configure_selinux_for_smbd)
+            self.step("starting CIFS services", self.__start)
 
         if self.add_sids:
             self.step("adding SIDs to existing users and groups",
                       self.__add_sids)
-        self.step("restarting smbd", self.__restart_smb)
+        if self.fulltrust:
+            self.step("restarting smbd", self.__restart_smb)
 
         self.start_creation(show_service_name=False)
 
diff --git a/ipaserver/install/server/__init__.py b/ipaserver/install/server/__init__.py
index f9150ee4e..dd104fa0a 100644
--- a/ipaserver/install/server/__init__.py
+++ b/ipaserver/install/server/__init__.py
@@ -432,11 +432,6 @@ class ServerInstallInterface(ServerCertificateInstallInterface,
                     "You cannot specify an --enable-compat option without the "
                     "--setup-adtrust option")
 
-            if self.netbios_name:
-                raise RuntimeError(
-                    "You cannot specify a --netbios-name option without the "
-                    "--setup-adtrust option")
-
             if self.no_msdcs:
                 raise RuntimeError(
                     "You cannot specify a --no-msdcs option without the "
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index b01fd85a5..5ac68e757 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -443,6 +443,7 @@ def install_check(installer):
         print("  * Configure KRA (dogtag) for secret management")
     if options.setup_dns:
         print("  * Configure DNS (bind)")
+    print("  * Configure SID generation")
     if options.setup_adtrust:
         print("  * Configure Samba (smb) and winbind for managing AD trusts")
     if not options.no_pkinit:
@@ -703,8 +704,9 @@ def install_check(installer):
         logger.debug('Starting Directory Server')
         services.knownservices.dirsrv.start(instance_name)
 
-    if options.setup_adtrust:
-        adtrust.install_check(False, options, api)
+    # Always call adtrust.install_check
+    # if --setup-adtrust is not specified, only the SID part is executed
+    adtrust.install_check(False, options, api)
 
     # installer needs to update hosts file when DNS subsystem will be
     # installed or custom addresses are used
@@ -966,8 +968,9 @@ def install(installer):
     if options.setup_dns:
         dns.install(False, False, options)
 
-    if options.setup_adtrust:
-        adtrust.install(False, options, fstore, api)
+    # Always call adtrust installer to configure SID generation
+    # if --setup-adtrust is not specified, only the SID part is executed
+    adtrust.install(False, options, fstore, api)
 
     # Set the admin user kerberos password
     ds.change_admin_password(admin_password)
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index f1fb91036..1857813c0 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1158,8 +1158,9 @@ def promote_check(installer):
             # check addresses here, dns module is doing own check
             no_matching_interface_for_ip_address_warning(config.ips)
 
-        if options.setup_adtrust:
-            adtrust.install_check(False, options, remote_api)
+        # Always call adtrust.install_check
+        # if --setup-adtrust is not specified, only the SID part is executed
+        adtrust.install_check(False, options, remote_api)
 
     except errors.ACIError:
         logger.debug("%s", traceback.format_exc())
@@ -1365,8 +1366,9 @@ def install(installer):
     if options.setup_dns:
         dns.install(False, True, options, api)
 
-    if options.setup_adtrust:
-        adtrust.install(False, options, fstore, api)
+    # Always call adtrust.install
+    # if --setup-adtrust is not specified, only the SID part is executed
+    adtrust.install(False, options, fstore, api)
 
     if options.hidden_replica:
         # Set services to hidden
-- 
2.33.1


From 38efe74bd115658b34259c4dc106565a549bd78b Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Fri, 8 Oct 2021 11:37:26 +0200
Subject: [PATCH 04/22] adtrust install: define constants for rid bases

Define constants for DEFAULT_PRIMARY_RID_BASE = 1000 and
DEFAULT_SECONDARY_RID_BASE = 100000000

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 install/tools/ipa-adtrust-install.in | 5 +++--
 ipaserver/install/adtrust.py         | 7 +++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/install/tools/ipa-adtrust-install.in b/install/tools/ipa-adtrust-install.in
index 9cb89ea11..f245ab342 100644
--- a/install/tools/ipa-adtrust-install.in
+++ b/install/tools/ipa-adtrust-install.in
@@ -64,10 +64,11 @@ def parse_options():
     parser.add_option("--no-msdcs", dest="no_msdcs", action="store_true",
                       default=False, help=SUPPRESS_HELP)
 
-    parser.add_option("--rid-base", dest="rid_base", type=int, default=1000,
+    parser.add_option("--rid-base", dest="rid_base", type=int,
+                      default=adtrust.DEFAULT_PRIMARY_RID_BASE,
                       help="Start value for mapping UIDs and GIDs to RIDs")
     parser.add_option("--secondary-rid-base", dest="secondary_rid_base",
-                      type=int, default=100000000,
+                      type=int, default=adtrust.DEFAULT_SECONDARY_RID_BASE,
                       help="Start value of the secondary range for mapping "
                            "UIDs and GIDs to RIDs")
     parser.add_option("-U", "--unattended", dest="unattended",
diff --git a/ipaserver/install/adtrust.py b/ipaserver/install/adtrust.py
index c01748bc0..6eb6a25ee 100644
--- a/ipaserver/install/adtrust.py
+++ b/ipaserver/install/adtrust.py
@@ -39,6 +39,9 @@ logger = logging.getLogger(__name__)
 netbios_name = None
 reset_netbios_name = False
 
+DEFAULT_PRIMARY_RID_BASE = 1000
+DEFAULT_SECONDARY_RID_BASE = 100000000
+
 
 def netbios_name_error(name):
     logger.error("\nIllegal NetBIOS name [%s].\n", name)
@@ -553,12 +556,12 @@ class SIDInstallInterface(ServiceAdminInstallInterface):
     )
     rid_base = knob(
         int,
-        1000,
+        DEFAULT_PRIMARY_RID_BASE,
         description="Start value for mapping UIDs and GIDs to RIDs"
     )
     secondary_rid_base = knob(
         int,
-        100000000,
+        DEFAULT_SECONDARY_RID_BASE,
         description="Start value of the secondary range for mapping "
                     "UIDs and GIDs to RIDs"
     )
-- 
2.33.1


From 03247e12e512f882fb7f54ac175719f0f19fed11 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Fri, 8 Oct 2021 15:47:06 +0200
Subject: [PATCH 05/22] ipa config: add --enable-sid option

Add new options to ipa config-mod, allowing to enable
SID generation on upgraded servers:
ipa config-mod --enable-sid --add-sids --netbios-name NAME

The new option uses Dbus to launch an oddjob command,
org.freeipa.server.config-enable-sid
that runs the installation steps related to SID generation.

--add-sids is optional and triggers the sid generation task that
populates SID for existing users / groups.
--netbios-name is optional and allows to specify the NetBIOS Name.
When not provided, the NetBIOS name is generated based on the leading
component of the DNS domain name.

This command can be run multiple times.

Fixes: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 API.txt                                       |  5 +-
 VERSION.m4                                    |  4 +-
 freeipa.spec.in                               |  1 +
 install/oddjob/Makefile.am                    |  2 +
 .../etc/oddjobd.conf.d/ipa-server.conf.in     |  6 ++
 .../org.freeipa.server.config-enable-sid.in   | 76 ++++++++++++++
 ipaplatform/base/paths.py                     |  1 +
 ipaserver/install/adtrustinstance.py          |  2 +
 ipaserver/plugins/config.py                   | 99 ++++++++++++++++++-
 ipaserver/rpcserver.py                        |  2 +
 selinux/ipa.te                                |  3 +
 11 files changed, 196 insertions(+), 5 deletions(-)
 create mode 100644 install/oddjob/org.freeipa.server.config-enable-sid.in

diff --git a/API.txt b/API.txt
index 212ef807c..39c179a47 100644
--- a/API.txt
+++ b/API.txt
@@ -1076,11 +1076,13 @@ args: 0,1,1
 option: Str('version?')
 output: Output('result')
 command: config_mod/1
-args: 0,28,3
+args: 0,31,3
+option: Flag('add_sids?', autofill=True, default=False)
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('ca_renewal_master_server?', autofill=False)
 option: Str('delattr*', cli_name='delattr')
+option: Flag('enable_sid?', autofill=True, default=False)
 option: StrEnum('ipaconfigstring*', autofill=False, cli_name='ipaconfigstring', values=[u'AllowNThash', u'KDC:Disable Last Success', u'KDC:Disable Lockout', u'KDC:Disable Default Preauth for SPNs'])
 option: Str('ipadefaultemaildomain?', autofill=False, cli_name='emaildomain')
 option: Str('ipadefaultloginshell?', autofill=False, cli_name='defaultshell')
@@ -1101,6 +1103,7 @@ option: Str('ipaselinuxusermaporder?', autofill=False)
 option: StrEnum('ipauserauthtype*', autofill=False, cli_name='user_auth_type', values=[u'password', u'radius', u'otp', u'pkinit', u'hardened', u'disabled'])
 option: Str('ipauserobjectclasses*', autofill=False, cli_name='userobjectclasses')
 option: IA5Str('ipausersearchfields?', autofill=False, cli_name='usersearch')
+option: Str('netbios_name?', autofill=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr')
diff --git a/VERSION.m4 b/VERSION.m4
index 9f024675f..80260953c 100644
--- a/VERSION.m4
+++ b/VERSION.m4
@@ -86,8 +86,8 @@ define(IPA_DATA_VERSION, 20100614120000)
 #                                                      #
 ########################################################
 define(IPA_API_VERSION_MAJOR, 2)
-define(IPA_API_VERSION_MINOR, 242)
-# Last change: add status options for cert-find
+# Last change: add enable_sid to config
+define(IPA_API_VERSION_MINOR, 245)
 
 
 ########################################################
diff --git a/freeipa.spec.in b/freeipa.spec.in
index fdca43a24..1d1383578 100755
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -1367,6 +1367,7 @@ fi
 %dir %{_libexecdir}/ipa/oddjob
 %attr(0755,root,root) %{_libexecdir}/ipa/oddjob/org.freeipa.server.conncheck
 %attr(0755,root,root) %{_libexecdir}/ipa/oddjob/org.freeipa.server.trust-enable-agent
+%attr(0755,root,root) %{_libexecdir}/ipa/oddjob/org.freeipa.server.config-enable-sid
 %config(noreplace) %{_sysconfdir}/dbus-1/system.d/org.freeipa.server.conf
 %config(noreplace) %{_sysconfdir}/oddjobd.conf.d/ipa-server.conf
 %dir %{_libexecdir}/ipa/certmonger
diff --git a/install/oddjob/Makefile.am b/install/oddjob/Makefile.am
index 7aeb406a2..bd968eaba 100644
--- a/install/oddjob/Makefile.am
+++ b/install/oddjob/Makefile.am
@@ -7,6 +7,7 @@ dbusconfdir = $(sysconfdir)/dbus-1/system.d
 dist_noinst_DATA =		\
 	com.redhat.idm.trust-fetch-domains.in	\
 	org.freeipa.server.trust-enable-agent.in	\
+	org.freeipa.server.config-enable-sid.in \
 	etc/oddjobd.conf.d/oddjobd-ipa-trust.conf.in	\
 	etc/oddjobd.conf.d/ipa-server.conf.in		\
 	$(NULL)
@@ -18,6 +19,7 @@ dist_oddjob_SCRIPTS =				\
 nodist_oddjob_SCRIPTS =				\
 	com.redhat.idm.trust-fetch-domains	\
 	org.freeipa.server.trust-enable-agent	\
+	org.freeipa.server.config-enable-sid \
 	$(NULL)
 
 
diff --git a/install/oddjob/etc/oddjobd.conf.d/ipa-server.conf.in b/install/oddjob/etc/oddjobd.conf.d/ipa-server.conf.in
index 640b510aa..a79640f88 100644
--- a/install/oddjob/etc/oddjobd.conf.d/ipa-server.conf.in
+++ b/install/oddjob/etc/oddjobd.conf.d/ipa-server.conf.in
@@ -17,6 +17,12 @@
                   prepend_user_name="no"
                   argument_passing_method="cmdline"/>
         </method>
+        <method name="config_enable_sid">
+          <helper exec="@ODDJOBDIR@/org.freeipa.server.config-enable-sid"
+                  arguments="10"
+                  prepend_user_name="no"
+                  argument_passing_method="cmdline"/>
+        </method>
       </interface>
       <interface name="org.freedesktop.DBus.Introspectable">
         <allow min_uid="0" max_uid="0"/>
diff --git a/install/oddjob/org.freeipa.server.config-enable-sid.in b/install/oddjob/org.freeipa.server.config-enable-sid.in
new file mode 100644
index 000000000..ff5fb49e2
--- /dev/null
+++ b/install/oddjob/org.freeipa.server.config-enable-sid.in
@@ -0,0 +1,76 @@
+#!/usr/bin/python3
+#
+# Copyright (C) 2021  FreeIPA Contributors see COPYING for license
+#
+
+import logging
+
+from ipalib import api
+from ipalib.install import sysrestore
+from ipaplatform.paths import paths
+from ipapython import ipaldap
+from ipapython.admintool import AdminTool
+from ipaserver.install import adtrust, adtrustinstance
+
+logger = logging.getLogger(__name__)
+
+class IPAConfigEnableSid(AdminTool):
+    command_name = "ipa-enable-sid"
+    log_file_name = paths.IPASERVER_ENABLESID_LOG
+    usage = "%prog"
+    description = "Enable SID generation"
+
+    @classmethod
+    def add_options(cls, parser):
+        super(IPAConfigEnableSid, cls).add_options(parser)
+
+        parser.add_option(
+            "--add-sids",
+            dest="add_sids", default=False, action="store_true",
+            help="Add SIDs for existing users and groups as the final step"
+        )
+
+        parser.add_option(
+            "--netbios-name",
+            dest="netbios_name", default=None,
+            help="NetBIOS name of the IPA domain"
+        )
+
+        parser.add_option(
+            "--reset-netbios-name",
+            dest="reset_netbios_name", default=False, action="store_true",
+            help="Force reset of the existing NetBIOS name"
+        )
+
+
+    def validate_options(self):
+        super(IPAConfigEnableSid, self).validate_options(needs_root=True)
+
+    def run(self):
+        api.bootstrap(in_server=True, confdir=paths.ETC_IPA)
+        api.finalize()
+
+        try:
+            api.Backend.ldap2.connect()
+            fstore = sysrestore.FileStore(paths.SYSRESTORE)
+
+            smb = adtrustinstance.ADTRUSTInstance(fstore, False)
+            smb.realm = api.env.realm
+            smb.autobind = ipaldap.AUTOBIND_ENABLED
+            smb.setup(api.env.host, api.env.realm,
+                      self.options.netbios_name,
+                      self.options.reset_netbios_name,
+                      adtrust.DEFAULT_PRIMARY_RID_BASE,
+                      adtrust.DEFAULT_SECONDARY_RID_BASE,
+                      self.options.add_sids,
+                      enable_compat=False)
+            smb.find_local_id_range()
+            smb.create_instance()
+
+        finally:
+            if api.Backend.ldap2.isconnected():
+                api.Backend.ldap2.disconnect()
+
+        return 0
+
+IPAConfigEnableSid.run_cli()
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index de217d9ef..42a47f1df 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -364,6 +364,7 @@ class BasePathNamespace:
     IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
     IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
     IPARESTORE_LOG = "/var/log/iparestore.log"
+    IPASERVER_ENABLESID_LOG = "/var/log/ipaserver-enable-sid.log"
     IPASERVER_INSTALL_LOG = "/var/log/ipaserver-install.log"
     IPASERVER_ADTRUST_INSTALL_LOG = "/var/log/ipaserver-adtrust-install.log"
     IPASERVER_DNS_INSTALL_LOG = "/var/log/ipaserver-dns-install.log"
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index abf5d7faf..6575e4525 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -334,6 +334,8 @@ class ADTRUSTInstance(service.Service):
         # _ldap_mod does not return useful error codes, so we must check again
         # if the fallback group was created properly.
         try:
+            # Remove entry from cache otherwise get_entry won't find it
+            api.Backend.ldap2.remove_cache_entry(fb_group_dn)
             api.Backend.ldap2.get_entry(fb_group_dn)
         except errors.NotFound:
             self.print_msg("Failed to add fallback group.")
diff --git a/ipaserver/plugins/config.py b/ipaserver/plugins/config.py
index ace66e589..909dc082f 100644
--- a/ipaserver/plugins/config.py
+++ b/ipaserver/plugins/config.py
@@ -17,12 +17,16 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+import dbus
+import dbus.mainloop.glib
+import logging
 
 from ipalib import api
-from ipalib import Bool, Int, Str, IA5Str, StrEnum, DNParam
+from ipalib import Bool, Int, Str, IA5Str, StrEnum, DNParam, Flag
 from ipalib import errors
 from ipalib.constants import MAXHOSTNAMELEN
 from ipalib.plugable import Registry
+from ipalib.request import context
 from ipalib.util import validate_domain_name
 from .baseldap import (
     LDAPObject,
@@ -30,7 +34,12 @@ from .baseldap import (
     LDAPRetrieve)
 from .selinuxusermap import validate_selinuxuser
 from ipalib import _
+from ipapython.admintool import ScriptError
 from ipapython.dn import DN
+from ipaserver.plugins.privilege import principal_has_privilege
+from ipaserver.install.adtrust import set_and_check_netbios_name
+
+logger = logging.getLogger(__name__)
 
 # 389-ds attributes that should be skipped in attribute checks
 OPERATIONAL_ATTRIBUTES = ('nsaccountlock', 'member', 'memberof',
@@ -334,6 +343,24 @@ class config(LDAPObject):
             doc=_('DNSec key master'),
             flags={'virtual_attribute', 'no_create', 'no_update'}
         ),
+        Flag(
+            'enable_sid?',
+            label=_('Setup SID configuration'),
+            doc=_('New users and groups automatically get a SID assigned'),
+            flags={'virtual_attribute', 'no_create'}
+        ),
+        Flag(
+            'add_sids?',
+            label=_('Add SIDs'),
+            doc=_('Add SIDs for existing users and groups'),
+            flags={'virtual_attribute', 'no_create'}
+        ),
+        Str(
+            'netbios_name?',
+            label=_('NetBIOS name of the IPA domain'),
+            doc=_('NetBIOS name of the IPA domain'),
+            flags={'virtual_attribute', 'no_create'}
+        ),
     )
 
     def get_dn(self, *keys, **kwargs):
@@ -473,6 +500,60 @@ class config(LDAPObject):
 class config_mod(LDAPUpdate):
     __doc__ = _('Modify configuration options.')
 
+    def _enable_sid(self, ldap, options):
+        # the user must have the Replication Administrators privilege
+        privilege = 'Replication Administrators'
+        if not principal_has_privilege(self.api, context.principal, privilege):
+            raise errors.ACIError(
+                info=_("not allowed to enable SID generation"))
+
+        # NetBIOS name is either taken from options or generated
+        try:
+            netbios_name, reset_netbios_name = set_and_check_netbios_name(
+                options.get('netbios_name', None), True, self.api)
+        except ScriptError:
+            raise errors.ValidationError(name="NetBIOS name",
+                error=_('Up to 15 characters and only uppercase ASCII letters'
+                        ', digits and dashes are allowed. Empty string is '
+                        'not allowed.'))
+
+        _ret = 0
+        _stdout = ''
+        _stderr = ''
+
+        dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+        method_options = []
+        if options.get('add_sids', False):
+            method_options.extend(["--add-sids"])
+        method_options.extend(["--netbios-name", netbios_name])
+        if reset_netbios_name:
+            method_options.append("--reset-netbios-name")
+        # Dbus definition expects up to 10 arguments
+        method_options.extend([''] * (10 - len(method_options)))
+
+        try:
+            bus = dbus.SystemBus()
+            obj = bus.get_object('org.freeipa.server', '/',
+                                 follow_name_owner_changes=True)
+            server = dbus.Interface(obj, 'org.freeipa.server')
+            _ret, _stdout, _stderr = server.config_enable_sid(*method_options)
+        except dbus.DBusException as e:
+            logger.error('Failed to call org.freeipa.server.config_enable_sid.'
+                         'DBus exception is %s', str(e))
+            raise errors.ExecutionError(message=_('Failed to call DBus'))
+
+        # The oddjob restarts dirsrv, we need to re-establish the conn
+        if self.api.Backend.ldap2.isconnected():
+            self.api.Backend.ldap2.disconnect()
+        self.api.Backend.ldap2.connect(ccache=context.ccache_name)
+
+        if _ret != 0:
+            logger.error("Helper config_enable_sid return code is %d", _ret)
+            raise errors.ExecutionError(
+                message=_('Configuration of SID failed. '
+                          'See details in the error log'))
+
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
         if 'ipadefaultprimarygroup' in entry_attrs:
@@ -600,13 +681,27 @@ class config_mod(LDAPUpdate):
 
         self.obj.validate_domain_resolution_order(entry_attrs)
 
+        # The options add_sids or netbios_name need the enable_sid option
+        for sid_option in ['add_sids', 'netbios_name']:
+            if options.get(sid_option, None) and not options['enable_sid']:
+                opt = "--" + sid_option.replace("_", "-")
+                error_message = _("You cannot specify %s without "
+                                  "the --enable-sid option" % opt)
+                raise errors.ValidationError(
+                    name=opt,
+                    error=error_message)
+
+        if options['enable_sid']:
+            self._enable_sid(ldap, options)
+
         return dn
 
     def exc_callback(self, keys, options, exc, call_func,
                      *call_args, **call_kwargs):
         if (isinstance(exc, errors.EmptyModlist) and
                 call_func.__name__ == 'update_entry' and
-                'ca_renewal_master_server' in options):
+                ('ca_renewal_master_server' in options or
+                 'enable_sid' in options)):
             return
 
         super(config_mod, self).exc_callback(
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index e612528e0..20c536e4d 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -383,6 +383,8 @@ class WSGIExecutioner(Executioner):
         if self.api.env.debug:
             time_start = time.perf_counter_ns()
         try:
+            if 'KRB5CCNAME' in environ:
+                setattr(context, "ccache_name", environ['KRB5CCNAME'])
             if ('HTTP_ACCEPT_LANGUAGE' in environ):
                 lang_reg_w_q = environ['HTTP_ACCEPT_LANGUAGE'].split(',')[0]
                 lang_reg = lang_reg_w_q.split(';')[0]
diff --git a/selinux/ipa.te b/selinux/ipa.te
index 68e109419..cfca2e7c3 100644
--- a/selinux/ipa.te
+++ b/selinux/ipa.te
@@ -155,6 +155,9 @@ manage_dirs_pattern(ipa_helper_t, ipa_var_run_t, ipa_var_run_t)
 manage_files_pattern(ipa_helper_t, ipa_var_run_t, ipa_var_run_t)
 files_pid_filetrans(ipa_helper_t, ipa_var_run_t, { dir file })
 
+manage_files_pattern(ipa_helper_t, ipa_tmp_t, ipa_tmp_t)
+files_tmp_filetrans(ipa_helper_t, ipa_tmp_t, { file })
+
 kernel_read_system_state(ipa_helper_t)
 kernel_read_network_state(ipa_helper_t)
 
-- 
2.33.1


From 1c91d6ce8e9d57796145f31dc3c62d5c404d44a4 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Fri, 8 Oct 2021 17:43:15 +0200
Subject: [PATCH 06/22] ipatests: add test ensuring SIDs are generated for new
 installs

The standard installer now configures all the items needed
for SID generation. Add a new test with the following scenario:
- install IPA server
- create an active user
- ensure the user's entry has an attribute ipantsecurityidentifier
- ensure that the kerberos ticket for the user contains PAC data
by using the utility ipa-print-pac

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipatests/test_integration/test_commands.py | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
index 5d755998e..803340f31 100644
--- a/ipatests/test_integration/test_commands.py
+++ b/ipatests/test_integration/test_commands.py
@@ -1499,3 +1499,43 @@ class TestIPACommandWithoutReplica(IntegrationTest):
         )
         # Run the command again after cache is removed
         self.master.run_command(['ipa', 'user-show', 'ipauser1'])
+
+    def test_sid_generation(self):
+        """
+        Test SID generation
+
+        Check that new users are created with a SID and PAC data is
+        added in their Kerberos tickets.
+        """
+        user = "pacuser"
+        passwd = "Secret123"
+
+        try:
+            # Create a nonadmin user
+            tasks.create_active_user(
+                self.master, user, passwd, first=user, last=user,
+                krb5_trace=True)
+
+            # Check SID is present in the new entry
+            base_dn = str(self.master.domain.basedn)
+            result = tasks.ldapsearch_dm(
+                self.master,
+                'uid={user},cn=users,cn=accounts,{base_dn}'.format(
+                    user=user, base_dn=base_dn),
+                ['ipantsecurityidentifier'],
+                scope='base'
+            )
+            assert 'ipantsecurityidentifier' in result.stdout_text
+
+            # Defaults: host/... principal for service
+            # keytab in /etc/krb5.keytab
+            self.master.run_command(["kinit", '-k'])
+            result = self.master.run_command(
+                [os.path.join(paths.LIBEXEC_IPA_DIR, "ipa-print-pac"),
+                 "ticket", user],
+                stdin_text=(passwd + '\n')
+            )
+            assert "PAC_DATA" in result.stdout_text
+        finally:
+            tasks.kinit_admin(self.master)
+            self.master.run_command(['ipa', 'user-del', user])
-- 
2.33.1


From 7682fa42f1964640ac9718c27570056b59574209 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Tue, 12 Oct 2021 09:57:27 +0200
Subject: [PATCH 07/22] ipatests: interactive install prompts for netbios name

The interactive server installation now prompts for netbios
name confirmation.
Add expected prompt and send response to the installer.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipalib/constants.py                           |  3 +++
 ipaserver/install/adtrustinstance.py          |  5 ++---
 ipatests/test_integration/test_caless.py      |  1 +
 .../test_integration/test_installation.py     | 19 +++++++++++++++++++
 ipatests/test_integration/test_ntp_options.py |  3 +++
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/ipalib/constants.py b/ipalib/constants.py
index 79ea36f08..719a524d6 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -23,6 +23,7 @@ All constants centralised in one file.
 """
 
 import os
+import string
 
 from ipaplatform.constants import constants as _constants
 from ipapython.dn import DN
@@ -343,3 +344,5 @@ SOFTHSM_DNSSEC_TOKEN_LABEL = u'ipaDNSSEC'
 # Apache's mod_ssl SSLVerifyDepth value (Maximum depth of CA
 # Certificates in Client Certificate verification)
 MOD_SSL_VERIFY_DEPTH = '5'
+
+ALLOWED_NETBIOS_CHARS = string.ascii_uppercase + string.digits + '-'
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 6575e4525..776e7e587 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -25,7 +25,6 @@ import errno
 import ldap
 import tempfile
 import uuid
-import string
 import struct
 import re
 import socket
@@ -36,6 +35,8 @@ from ipaserver.install import service
 from ipaserver.install import installutils
 from ipaserver.install.replication import wait_for_task
 from ipalib import errors, api
+from ipalib.constants import ALLOWED_NETBIOS_CHARS
+
 from ipalib.util import normalize_zone
 from ipapython.dn import DN
 from ipapython import ipachangeconf
@@ -53,8 +54,6 @@ if six.PY3:
 
 logger = logging.getLogger(__name__)
 
-ALLOWED_NETBIOS_CHARS = string.ascii_uppercase + string.digits + '-'
-
 UPGRADE_ERROR = """
 Entry %(dn)s does not exist.
 This means upgrade from IPA 2.x to 3.x did not went well and required S4U2Proxy
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 16dfbb320..4481845da 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -60,6 +60,7 @@ def get_install_stdin(cert_passwords=()):
         '',  # Server host name (has default)
     ]
     lines.extend(cert_passwords)  # Enter foo.p12 unlock password
+    lines.extend('IPA') # NetBios name
     lines += [
         'no',   # configure chrony with NTP server or pool address?
         'yes',  # Continue with these values?
diff --git a/ipatests/test_integration/test_installation.py b/ipatests/test_integration/test_installation.py
index 0c96536f0..334279fcd 100644
--- a/ipatests/test_integration/test_installation.py
+++ b/ipatests/test_integration/test_installation.py
@@ -22,6 +22,7 @@ from cryptography import x509 as crypto_x509
 from ipalib import x509
 from ipalib.constants import DOMAIN_LEVEL_0
 from ipalib.constants import IPA_CA_RECORD
+from ipalib.constants import ALLOWED_NETBIOS_CHARS
 from ipalib.sysrestore import SYSRESTORE_STATEFILE, SYSRESTORE_INDEXFILE
 from ipapython.dn import DN
 from ipaplatform.constants import constants
@@ -69,6 +70,17 @@ def server_cleanup(request):
     tasks.uninstall_master(host)
 
 
+def create_netbios_name(host):
+    """
+    Create a NetBIOS name based on the provided host
+    """
+    netbios = ''.join(
+        c for c in host.domain.name.split('.')[0].upper() \
+        if c in ALLOWED_NETBIOS_CHARS
+    )[:15]
+    return netbios
+
+
 class InstallTestBase1(IntegrationTest):
 
     num_replicas = 3
@@ -1171,6 +1183,8 @@ class TestInstallMaster(IntegrationTest):
         hosts = self.master.get_file_contents(paths.HOSTS, encoding='utf-8')
         new_hosts = hosts.replace(original_hostname, new_hostname)
         self.master.put_file_contents(paths.HOSTS, new_hosts)
+        netbios = create_netbios_name(self.master)
+
         try:
             cmd = ['ipa-server-install', '--hostname', new_hostname]
             with self.master.spawn_expect(cmd) as e:
@@ -1193,6 +1207,8 @@ class TestInstallMaster(IntegrationTest):
                 e.sendline(self.master.config.admin_password)
                 e.expect_exact('Password (confirm): ')
                 e.sendline(self.master.config.admin_password)
+                e.expect_exact('NetBIOS domain name [{}]: '.format(netbios))
+                e.sendline(netbios)
                 e.expect_exact('Do you want to configure chrony with '
                                'NTP server or pool address? [no]: ')
                 e.sendline('no')
@@ -1368,6 +1384,7 @@ class TestInstallMasterDNS(IntegrationTest):
         https://pagure.io/freeipa/issue/2575
         """
         cmd = ['ipa-server-install']
+        netbios = create_netbios_name(self.master)
         with self.master.spawn_expect(cmd) as e:
             e.expect_exact('Do you want to configure integrated '
                            'DNS (BIND)? [no]: ')
@@ -1399,6 +1416,8 @@ class TestInstallMasterDNS(IntegrationTest):
             e.expect_exact('Do you want to search for missing reverse '
                            'zones? [yes]: ')
             e.sendline('no')  # irrelevant for this test
+            e.expect_exact('NetBIOS domain name [{}]: '.format(netbios))
+            e.sendline(netbios)
             e.expect_exact('Do you want to configure chrony with NTP '
                            'server or pool address? [no]: ')
             e.sendline('no')  # irrelevant for this test
diff --git a/ipatests/test_integration/test_ntp_options.py b/ipatests/test_integration/test_ntp_options.py
index 27d35043e..efd950991 100644
--- a/ipatests/test_integration/test_ntp_options.py
+++ b/ipatests/test_integration/test_ntp_options.py
@@ -260,6 +260,8 @@ class TestNTPoptions(IntegrationTest):
             "No\n"
             # Server host name [hostname]:
             "\n"
+            # Enter the NetBIOS name for the IPA domain
+            "IPA\n"
             # Do you want to configure chrony with NTP server
             #   or pool address? [no]:
             "Yes\n"
@@ -372,6 +374,7 @@ class TestNTPoptions(IntegrationTest):
         server_input = (
             "\n" +
             "\n"
+            "IPA\n"
             "No\n"
             "Yes"
         )
-- 
2.33.1


From 80e29da65ad3731b8db262fb0d00c70cfe7db0d4 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Fri, 15 Oct 2021 12:46:36 +0200
Subject: [PATCH 08/22] ipatests: adapt expected output with SID

From now on, new users/groups automatically get a SID.
Update the expect test outputs.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipatests/test_xmlrpc/mock_trust.py            | 45 -------------------
 ipatests/test_xmlrpc/objectclasses.py         |  4 +-
 ipatests/test_xmlrpc/test_batch_plugin.py     |  7 ++-
 ipatests/test_xmlrpc/test_group_plugin.py     | 25 ++++++-----
 ipatests/test_xmlrpc/test_idviews_plugin.py   | 19 ++++----
 .../test_kerberos_principal_aliases.py        |  6 +--
 ipatests/test_xmlrpc/test_netgroup_plugin.py  |  4 +-
 ipatests/test_xmlrpc/test_range_plugin.py     |  2 +
 .../test_xmlrpc/test_selinuxusermap_plugin.py |  4 +-
 ipatests/test_xmlrpc/test_service_plugin.py   |  7 ++-
 ipatests/test_xmlrpc/test_trust_plugin.py     |  4 +-
 ipatests/test_xmlrpc/test_user_plugin.py      | 27 ++++++-----
 ipatests/test_xmlrpc/tracker/group_plugin.py  | 11 +++--
 ipatests/test_xmlrpc/tracker/user_plugin.py   |  9 ++--
 ipatests/test_xmlrpc/xmlrpc_test.py           | 12 ++---
 15 files changed, 85 insertions(+), 101 deletions(-)

diff --git a/ipatests/test_xmlrpc/mock_trust.py b/ipatests/test_xmlrpc/mock_trust.py
index 08c1c2914..24a26ed1d 100644
--- a/ipatests/test_xmlrpc/mock_trust.py
+++ b/ipatests/test_xmlrpc/mock_trust.py
@@ -2,55 +2,10 @@
 #
 # Copyright (C) 2016  FreeIPA Contributors see COPYING for license
 #
-from contextlib import contextmanager
 import six
 
 from ipalib import api
-from ipatests.util import MockLDAP
 
-trust_container_dn = "cn=ad,cn=trusts,{basedn}".format(
-    basedn=api.env.basedn)
-trust_container_add = dict(
-    objectClass=[b"nsContainer", b"top"]
-    )
-
-smb_cont_dn = "{cifsdomains},{basedn}".format(
-    cifsdomains=api.env.container_cifsdomains,
-    basedn=api.env.basedn)
-smb_cont_add = dict(
-    objectClass=[b"nsContainer", b"top"]
-    )
-
-
-def create_mock_trust_containers():
-    with MockLDAP() as ldap:
-        ldap.add_entry(trust_container_dn, trust_container_add)
-        ldap.add_entry(smb_cont_dn, smb_cont_add)
-
-
-def remove_mock_trust_containers():
-    with MockLDAP() as ldap:
-        ldap.del_entry(trust_container_dn)
-        ldap.del_entry(smb_cont_dn)
-
-
-@contextmanager
-def mocked_trust_containers():
-    """Mocked trust containers
-
-    Provides containers for the RPC tests:
-    cn=ad,cn=trusts,BASEDN
-    cn=ad,cn=etc,BASEDN
-
-    Upon exiting, it tries to remove the container entries.
-    If the user of the context manager failed to remove
-    all child entries, exiting the context manager will fail.
-    """
-    create_mock_trust_containers()
-    try:
-        yield
-    finally:
-        remove_mock_trust_containers()
 
 def get_range_dn(name):
     format_str = "cn={name},cn=ranges,cn=etc,{basedn}"
diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 6a0efdd18..23bc03a07 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -35,7 +35,7 @@ user_base = [
     u'ipaSshGroupOfPubKeys',
 ]
 
-user = user_base + [u'mepOriginEntry']
+user = user_base + [u'mepOriginEntry', 'ipantuserattrs',]
 
 group = [
     u'top',
@@ -46,7 +46,7 @@ group = [
 ]
 
 externalgroup = group + [u'ipaexternalgroup']
-posixgroup = group + [u'posixgroup']
+posixgroup = group + [u'posixgroup', 'ipantgroupattrs']
 
 host = [
     u'ipasshhost',
diff --git a/ipatests/test_xmlrpc/test_batch_plugin.py b/ipatests/test_xmlrpc/test_batch_plugin.py
index e475081e3..632dd2c79 100644
--- a/ipatests/test_xmlrpc/test_batch_plugin.py
+++ b/ipatests/test_xmlrpc/test_batch_plugin.py
@@ -25,6 +25,7 @@ from ipalib import api
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import Fuzzy, assert_deepequal
 from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_digits,
+                                              fuzzy_set_optional_oc,
                                               fuzzy_uuid)
 from ipapython.dn import DN
 import pytest
@@ -97,7 +98,8 @@ class test_batch(Declarative):
                         result=dict(
                             cn=[group1],
                             description=[u'Test desc 1'],
-                            objectclass=objectclasses.group + [u'posixgroup'],
+                            objectclass=fuzzy_set_optional_oc(
+                                objectclasses.posixgroup, 'ipantgroupattrs'),
                             ipauniqueid=[fuzzy_uuid],
                             gidnumber=[fuzzy_digits],
                             dn=DN(('cn', 'testgroup1'),
@@ -168,7 +170,8 @@ class test_batch(Declarative):
                         result=dict(
                             cn=[group1],
                             description=[u'Test desc 1'],
-                            objectclass=objectclasses.group + [u'posixgroup'],
+                            objectclass=fuzzy_set_optional_oc(
+                                objectclasses.posixgroup, 'ipantgroupattrs'),
                             ipauniqueid=[fuzzy_uuid],
                             gidnumber=[fuzzy_digits],
                             dn=DN(('cn', 'testgroup1'),
diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py
index 11c85feb4..f9a0e2cfe 100644
--- a/ipatests/test_xmlrpc/test_group_plugin.py
+++ b/ipatests/test_xmlrpc/test_group_plugin.py
@@ -27,7 +27,8 @@ import pytest
 from ipalib import errors
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import (
-    fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, add_oc,
+    fuzzy_digits, fuzzy_uuid, fuzzy_set_ci,
+    fuzzy_user_or_group_sid,
     XMLRPC_test, raises_exact
 )
 from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
@@ -61,6 +62,8 @@ def managed_group(request, user):
     tracker.exists = True
     # Managed group gets created when user is created
     tracker.track_create()
+    # Managed groups don't have a SID
+    del tracker.attrs['ipantsecurityidentifier']
     return tracker
 
 
@@ -79,7 +82,7 @@ def user_npg2(request, group):
     del tracker.attrs['mepmanagedentry']
     tracker.attrs.update(
         gidnumber=[u'1000'], description=[], memberof_group=[group.cn],
-        objectclass=add_oc(objectclasses.user_base, u'ipantuserattrs')
+        objectclass=objectclasses.user_base + ['ipantuserattrs']
     )
     return tracker.make_fixture(request)
 
@@ -314,9 +317,9 @@ class TestFindGroup(XMLRPC_test):
                     'gidnumber': [fuzzy_digits],
                     'cn': [u'admins'],
                     'description': [u'Account administrators group'],
-                    'objectclass': fuzzy_set_ci(add_oc(
-                        objectclasses.posixgroup, u'ipantgroupattrs')),
+                    'objectclass': fuzzy_set_ci(objectclasses.posixgroup),
                     'ipauniqueid': [fuzzy_uuid],
+                    'ipantsecurityidentifier': [fuzzy_user_or_group_sid],
                 },
                 {
                     'dn': get_group_dn('editors'),
@@ -324,27 +327,27 @@ class TestFindGroup(XMLRPC_test):
                     'cn': [u'editors'],
                     'description':
                         [u'Limited admins who can edit other users'],
-                    'objectclass': fuzzy_set_ci(add_oc(
-                        objectclasses.posixgroup, u'ipantgroupattrs')),
+                    'objectclass': fuzzy_set_ci(objectclasses.posixgroup),
                     'ipauniqueid': [fuzzy_uuid],
+                    'ipantsecurityidentifier': [fuzzy_user_or_group_sid],
                 },
                 {
                     'dn': get_group_dn(group.cn),
                     'cn': [group.cn],
                     'description': [u'Test desc1'],
                     'gidnumber': [fuzzy_digits],
-                    'objectclass': fuzzy_set_ci(add_oc(
-                        objectclasses.posixgroup, u'ipantgroupattrs')),
+                    'objectclass': fuzzy_set_ci(objectclasses.posixgroup),
                     'ipauniqueid': [fuzzy_uuid],
+                    'ipantsecurityidentifier': [fuzzy_user_or_group_sid],
                 },
                 {
                     'dn': get_group_dn(group2.cn),
                     'cn': [group2.cn],
                     'description': [u'Test desc2'],
                     'gidnumber': [fuzzy_digits],
-                    'objectclass': fuzzy_set_ci(add_oc(
-                        objectclasses.posixgroup, u'ipantgroupattrs')),
+                    'objectclass': fuzzy_set_ci(objectclasses.posixgroup),
                     'ipauniqueid': [fuzzy_uuid],
+                    'ipantsecurityidentifier': [fuzzy_user_or_group_sid],
                 },
             ]), result)
 
@@ -498,6 +501,8 @@ class TestExternalGroup(XMLRPC_test):
         group.track_create()
         del group.attrs['gidnumber']
         group.attrs.update(objectclass=objectclasses.externalgroup)
+        # External group don't have a SID
+        del group.attrs['ipantsecurityidentifier']
         command = group.make_create_command(**dict(external=True))
         result = command()
         group.check_create(result)
diff --git a/ipatests/test_xmlrpc/test_idviews_plugin.py b/ipatests/test_xmlrpc/test_idviews_plugin.py
index be96e27dc..fa535af7a 100644
--- a/ipatests/test_xmlrpc/test_idviews_plugin.py
+++ b/ipatests/test_xmlrpc/test_idviews_plugin.py
@@ -27,7 +27,8 @@ import six
 
 from ipalib import api, errors
 from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, uuid_re, add_oc,
+from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, uuid_re,
+                                              fuzzy_set_optional_oc,
                                               fuzzy_uuid, fuzzy_digits)
 from ipatests.test_xmlrpc.test_user_plugin import get_user_result
 from ipatests.test_xmlrpc.test_group_plugin import get_group_dn
@@ -216,10 +217,7 @@ class test_idviews(Declarative):
                     u'Test',
                     u'User1',
                     'add',
-                    objectclass=add_oc(
-                        objectclasses.user,
-                        u'ipantuserattrs'
-                    )
+                    objectclass=objectclasses.user,
                 ),
             ),
         ),
@@ -237,7 +235,8 @@ class test_idviews(Declarative):
                 result=dict(
                     cn=[idoverridegroup1],
                     description=[u'Test desc 1'],
-                    objectclass=objectclasses.posixgroup,
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     gidnumber=[fuzzy_digits],
                     dn=get_group_dn(idoverridegroup1),
@@ -1624,10 +1623,7 @@ class test_idviews(Declarative):
                     u'Removed',
                     u'User',
                     'add',
-                    objectclass=add_oc(
-                        objectclasses.user,
-                        u'ipantuserattrs'
-                    )
+                    objectclass=objectclasses.user,
                 ),
             ),
         ),
@@ -1645,7 +1641,8 @@ class test_idviews(Declarative):
                 result=dict(
                     cn=[idoverridegroup_removed],
                     description=[u'Removed group'],
-                    objectclass=objectclasses.posixgroup,
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     gidnumber=[fuzzy_digits],
                     dn=get_group_dn(idoverridegroup_removed),
diff --git a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
index 61d3684bd..91c22b08c 100644
--- a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
+++ b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
@@ -19,7 +19,7 @@ from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
 from ipatests.test_xmlrpc.tracker.service_plugin import ServiceTracker
 from ipatests.test_xmlrpc.tracker.stageuser_plugin import StageUserTracker
 from ipatests.test_xmlrpc.mock_trust import (
-    mocked_trust_containers, get_trust_dn, get_trusted_dom_dict,
+    get_trust_dn, get_trusted_dom_dict,
     encode_mockldap_value)
 from ipatests.util import unlock_principal_password, change_principal
 
@@ -62,7 +62,7 @@ def trusted_domain():
     trusted_dom = TRUSTED_DOMAIN_MOCK
 
     # Write the changes
-    with mocked_trust_containers(), MockLDAP() as ldap:
+    with MockLDAP() as ldap:
         ldap.add_entry(trusted_dom['dn'], trusted_dom['ldif'])
         yield trusted_dom
         ldap.del_entry(trusted_dom['dn'])
@@ -83,7 +83,7 @@ def trusted_domain_with_suffix():
     )
 
     # Write the changes
-    with mocked_trust_containers(), MockLDAP() as ldap:
+    with MockLDAP() as ldap:
         ldap.add_entry(trusted_dom['dn'], trusted_dom['ldif'])
         yield trusted_dom
         ldap.del_entry(trusted_dom['dn'])
diff --git a/ipatests/test_xmlrpc/test_netgroup_plugin.py b/ipatests/test_xmlrpc/test_netgroup_plugin.py
index 5004ecbe8..3f1fc0bc1 100644
--- a/ipatests/test_xmlrpc/test_netgroup_plugin.py
+++ b/ipatests/test_xmlrpc/test_netgroup_plugin.py
@@ -24,6 +24,7 @@ Test the `ipaserver/plugins/netgroup.py` module.
 from ipalib import api
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_digits,
+                                              fuzzy_set_optional_oc,
                                               fuzzy_uuid, fuzzy_netgroupdn)
 from ipatests.test_xmlrpc import objectclasses
 from ipapython.dn import DN
@@ -300,7 +301,8 @@ class test_netgroup(Declarative):
                     cn=[group1],
                     description=[u'Test desc 1'],
                     gidnumber=[fuzzy_digits],
-                    objectclass=objectclasses.group + [u'posixgroup'],
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     dn=DN(('cn',group1),('cn','groups'),('cn','accounts'),
                           api.env.basedn),
diff --git a/ipatests/test_xmlrpc/test_range_plugin.py b/ipatests/test_xmlrpc/test_range_plugin.py
index c756bb794..168dbcbf3 100644
--- a/ipatests/test_xmlrpc/test_range_plugin.py
+++ b/ipatests/test_xmlrpc/test_range_plugin.py
@@ -495,6 +495,8 @@ class test_range(Declarative):
                     user1, u'Test', u'User1', 'add',
                     uidnumber=[unicode(user1_uid)],
                     gidnumber=[unicode(user1_uid)],
+                    objectclass=objectclasses.user_base + [u'mepOriginEntry'],
+                    omit=['ipantsecurityidentifier'],
                 ),
             ),
         ),
diff --git a/ipatests/test_xmlrpc/test_selinuxusermap_plugin.py b/ipatests/test_xmlrpc/test_selinuxusermap_plugin.py
index 179b65b6a..662212a63 100644
--- a/ipatests/test_xmlrpc/test_selinuxusermap_plugin.py
+++ b/ipatests/test_xmlrpc/test_selinuxusermap_plugin.py
@@ -25,6 +25,7 @@ from ipaplatform.constants import constants as platformconstants
 
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_digits,
+                                              fuzzy_set_optional_oc,
                                               fuzzy_uuid)
 from ipapython.dn import DN
 from ipatests.util import Fuzzy
@@ -230,7 +231,8 @@ class test_selinuxusermap(Declarative):
                     cn=[group1],
                     description=[u'Test desc 1'],
                     gidnumber=[fuzzy_digits],
-                    objectclass=objectclasses.group + [u'posixgroup'],
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     dn=DN(('cn', group1), ('cn', 'groups'), ('cn', 'accounts'),
                           api.env.basedn),
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index ed634a045..a3e5b740b 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -25,6 +25,7 @@ from ipalib import api, errors
 from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex, XMLRPC_test
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_set_optional_oc
 from ipatests.test_xmlrpc.xmlrpc_test import raises_exact
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.testcert import get_testcert, subject_base
@@ -1113,7 +1114,8 @@ class test_service_allowed_to(Declarative):
                 summary=u'Added group "%s"' % group1,
                 result=dict(
                     cn=[group1],
-                    objectclass=objectclasses.group + [u'posixgroup'],
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     gidnumber=[fuzzy_digits],
                     dn=group1_dn
@@ -1165,7 +1167,8 @@ class test_service_allowed_to(Declarative):
                 summary=u'Added group "%s"' % group2,
                 result=dict(
                     cn=[group2],
-                    objectclass=objectclasses.group + [u'posixgroup'],
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     gidnumber=[fuzzy_digits],
                     dn=group2_dn
diff --git a/ipatests/test_xmlrpc/test_trust_plugin.py b/ipatests/test_xmlrpc/test_trust_plugin.py
index 21c170943..9c91db438 100644
--- a/ipatests/test_xmlrpc/test_trust_plugin.py
+++ b/ipatests/test_xmlrpc/test_trust_plugin.py
@@ -27,6 +27,7 @@ from ipapython.dn import DN
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import (
     Declarative, fuzzy_guid, fuzzy_domain_sid, fuzzy_string, fuzzy_uuid,
+    fuzzy_set_optional_oc,
     fuzzy_digits)
 import pytest
 
@@ -110,7 +111,8 @@ class test_trustconfig(Declarative):
                     cn=[testgroup],
                     description=[u'Test group'],
                     gidnumber=[fuzzy_digits],
-                    objectclass=objectclasses.group + [u'posixgroup'],
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.posixgroup, 'ipantgroupattrs'),
                     ipauniqueid=[fuzzy_uuid],
                     dn=testgroup_dn,
                 ),
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 19af031bc..c2b8f79c8 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -38,7 +38,8 @@ from ipatests.util import (
     assert_deepequal, assert_equal, assert_not_equal, raises)
 from ipatests.test_xmlrpc.xmlrpc_test import (
     XMLRPC_test, fuzzy_digits, fuzzy_uuid, fuzzy_password,
-    Fuzzy, fuzzy_dergeneralizedtime, add_sid, add_oc, raises_exact)
+    fuzzy_user_or_group_sid,
+    Fuzzy, fuzzy_dergeneralizedtime, raises_exact)
 from ipapython.dn import DN
 from ipapython.ipaldap import ldap_initialize
 
@@ -124,7 +125,7 @@ def user_npg(request, group):
     del tracker.attrs['mepmanagedentry']
     tracker.attrs.update(
         description=[], memberof_group=[group.cn],
-        objectclass=add_oc(objectclasses.user_base, u'ipantuserattrs')
+        objectclass=objectclasses.user_base + [u'ipantuserattrs'],
     )
     return tracker.make_fixture(request)
 
@@ -138,7 +139,7 @@ def user_npg2(request, group):
     del tracker.attrs['mepmanagedentry']
     tracker.attrs.update(
         gidnumber=[u'1000'], description=[], memberof_group=[group.cn],
-        objectclass=add_oc(objectclasses.user_base, u'ipantuserattrs')
+        objectclass=objectclasses.user_base + [u'ipantuserattrs'],
     )
     return tracker.make_fixture(request)
 
@@ -645,7 +646,7 @@ class TestCreate(XMLRPC_test):
         testuser.attrs.update(gidnumber=[u'1000'])
         testuser.attrs.update(
             description=[],
-            objectclass=add_oc(objectclasses.user_base, u'ipantuserattrs')
+            objectclass=objectclasses.user_base + [u'ipantuserattrs']
         )
         command = testuser.make_create_command()
         result = command()
@@ -659,9 +660,12 @@ class TestCreate(XMLRPC_test):
             name=u'tuser1', givenname=u'Test', sn=u'Tuser1', uidnumber=999
         )
         testuser.track_create()
+        # When uid is outside of IPA id range, no SID is generated
+        del testuser.attrs['ipantsecurityidentifier']
         testuser.attrs.update(
             uidnumber=[u'999'],
-            gidnumber=[u'999']
+            gidnumber=[u'999'],
+            objectclass=objectclasses.user_base + ['mepOriginEntry']
         )
         command = testuser.make_create_command()
         result = command()
@@ -837,7 +841,7 @@ class TestUserWithUPGDisabled(XMLRPC_test):
         testuser.attrs.update(gidnumber=[u'1000'])
         testuser.attrs.update(
             description=[],
-            objectclass=add_oc(objectclasses.user_base, u'ipantuserattrs')
+            objectclass=objectclasses.user_base + [u'ipantuserattrs'],
         )
         command = testuser.make_create_command()
         result = command()
@@ -860,7 +864,7 @@ class TestUserWithUPGDisabled(XMLRPC_test):
         testuser.attrs.update(gidnumber=[u'1000'])
         testuser.attrs.update(
             description=[],
-            objectclass=add_oc(objectclasses.user_base, u'ipantuserattrs')
+            objectclass=objectclasses.user_base + [u'ipantuserattrs'],
         )
         command = testuser.make_create_command()
         result = command()
@@ -1147,7 +1151,7 @@ def get_user_result(uid, givenname, sn, operation='show', omit=[],
     # sn can be None; this should only be used from `get_admin_result`
     cn = overrides.get('cn', ['%s %s' % (givenname, sn or '')])
     cn[0] = cn[0].strip()
-    result = add_sid(dict(
+    result = dict(
         homedirectory=[u'/home/%s' % uid],
         loginshell=[platformconstants.DEFAULT_SHELL],
         uid=[uid],
@@ -1158,7 +1162,7 @@ def get_user_result(uid, givenname, sn, operation='show', omit=[],
         mail=[u'%s@%s' % (uid, api.env.domain)],
         has_keytab=False,
         has_password=False,
-    ))
+    )
     if sn:
         result['sn'] = [sn]
     if givenname:
@@ -1175,9 +1179,10 @@ def get_user_result(uid, givenname, sn, operation='show', omit=[],
             initials=[givenname[0] + (sn or '')[:1]],
             ipauniqueid=[fuzzy_uuid],
             mepmanagedentry=[get_group_dn(uid)],
-            objectclass=add_oc(objectclasses.user, u'ipantuserattrs'),
+            objectclass=objectclasses.user,
             krbprincipalname=[u'%s@%s' % (uid, api.env.realm)],
-            krbcanonicalname=[u'%s@%s' % (uid, api.env.realm)]
+            krbcanonicalname=[u'%s@%s' % (uid, api.env.realm)],
+            ipantsecurityidentifier=[fuzzy_user_or_group_sid],
         )
     if operation in ('show', 'show-all', 'find', 'mod'):
         result.update(
diff --git a/ipatests/test_xmlrpc/tracker/group_plugin.py b/ipatests/test_xmlrpc/tracker/group_plugin.py
index 8a6f8516e..fb36d7cf1 100644
--- a/ipatests/test_xmlrpc/tracker/group_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/group_plugin.py
@@ -4,6 +4,8 @@
 
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_uuid
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_user_or_group_sid
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_set_optional_oc
 
 from ipatests.test_xmlrpc.tracker.base import Tracker
 from ipatests.util import assert_deepequal, get_group_dn
@@ -21,9 +23,10 @@ class GroupTracker(Tracker):
         'idoverrideuser'
     }
 
-    retrieve_all_keys = retrieve_keys | {u'ipauniqueid', u'objectclass'}
+    retrieve_all_keys = retrieve_keys | {u'ipauniqueid', u'objectclass',
+                                         'ipantsecurityidentifier'}
 
-    create_keys = retrieve_all_keys
+    create_keys = retrieve_all_keys - {u'ipantsecurityidentifier'}
     update_keys = retrieve_keys - {u'dn'}
 
     add_member_keys = retrieve_keys | {u'description'}
@@ -91,7 +94,9 @@ class GroupTracker(Tracker):
             description=[self.description],
             gidnumber=[fuzzy_digits],
             ipauniqueid=[fuzzy_uuid],
-            objectclass=objectclasses.posixgroup,
+            objectclass=fuzzy_set_optional_oc(
+                objectclasses.posixgroup, 'ipantgroupattrs'),
+            ipantsecurityidentifier=[fuzzy_user_or_group_sid],
             )
         self.exists = True
 
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index d5e9ca893..9adbe8d92 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -11,7 +11,7 @@ import six
 from ipatests.util import assert_deepequal, get_group_dn
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import (
-    fuzzy_digits, fuzzy_uuid, raises_exact)
+    fuzzy_digits, fuzzy_uuid, fuzzy_user_or_group_sid, raises_exact)
 from ipatests.test_xmlrpc.tracker.base import Tracker
 from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin
 from ipatests.test_xmlrpc.tracker.certmapdata import CertmapdataMixin
@@ -40,7 +40,8 @@ class UserTracker(CertmapdataMixin, KerberosAliasMixin, Tracker):
         u'l', u'mobile', u'krbextradata', u'krblastpwdchange',
         u'krbpasswordexpiration', u'pager', u'st', u'manager', u'cn',
         u'ipauniqueid', u'objectclass', u'mepmanagedentry',
-        u'displayname', u'gecos', u'initials', u'preserved'}
+        u'displayname', u'gecos', u'initials', u'preserved',
+        'ipantsecurityidentifier'}
 
     retrieve_preserved_keys = (retrieve_keys - {u'memberof_group'}) | {
         u'preserved'}
@@ -122,7 +123,8 @@ class UserTracker(CertmapdataMixin, KerberosAliasMixin, Tracker):
                 api.env.container_deleteuser,
                 api.env.basedn
                 )
-            self.attrs[u'objectclass'] = objectclasses.user_base
+            self.attrs[u'objectclass'] = objectclasses.user_base \
+                + ['ipantuserattrs']
 
         return self.make_command(
             'user_del', self.uid,
@@ -188,6 +190,7 @@ class UserTracker(CertmapdataMixin, KerberosAliasMixin, Tracker):
             mepmanagedentry=[get_group_dn(self.uid)],
             memberof_group=[u'ipausers'],
             nsaccountlock=[u'false'],
+            ipantsecurityidentifier=[fuzzy_user_or_group_sid],
             )
 
         for key in self.kwargs:
diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py
index de2357237..8adb3c0d7 100644
--- a/ipatests/test_xmlrpc/xmlrpc_test.py
+++ b/ipatests/test_xmlrpc/xmlrpc_test.py
@@ -137,6 +137,12 @@ fuzzy_bytes = Fuzzy(type=bytes)
 def fuzzy_set_ci(s):
     return Fuzzy(test=lambda other: set(x.lower() for x in other) == set(y.lower() for y in s))
 
+
+def fuzzy_set_optional_oc(s, oc):
+    return Fuzzy(test=lambda other: set(x.lower() for x in other if x != oc)
+                 == set(y.lower() for y in s if y != oc))
+
+
 try:
     if not api.Backend.rpcclient.isconnected():
         api.Backend.rpcclient.connect()
@@ -152,12 +158,6 @@ adtrust_is_enabled = api.Command['adtrust_is_enabled']()['result']
 sidgen_was_run = api.Command['sidgen_was_run']()['result']
 
 
-def add_sid(d, check_sidgen=False):
-    if adtrust_is_enabled and (not check_sidgen or sidgen_was_run):
-        d['ipantsecurityidentifier'] = (fuzzy_user_or_group_sid,)
-    return d
-
-
 def add_oc(l, oc, check_sidgen=False):
     if adtrust_is_enabled and (not check_sidgen or sidgen_was_run):
         return l + [oc]
-- 
2.33.1


From 3162eb19616c3b17c4cbc659043d10e9fcf358bb Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Fri, 15 Oct 2021 15:55:10 +0200
Subject: [PATCH 09/22] User lifecycle: ignore SID when moving from preserved
 to staged

When a preserved user entry is moved to staged state, the SID
attribute must not be provided to user-stage command (the option
does not exist and the SID will be re-generated anyway).

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipaserver/plugins/user.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py
index e4ee572b2..7a4acf66e 100644
--- a/ipaserver/plugins/user.py
+++ b/ipaserver/plugins/user.py
@@ -978,6 +978,7 @@ class user_stage(LDAPMultiQuery):
                     u'ipauniqueid', u'krbcanonicalname',
                     u'sshpubkeyfp', u'krbextradata',
                     u'ipacertmapdata',
+                    'ipantsecurityidentifier',
                     u'nsaccountlock']
 
     def execute(self, *keys, **options):
-- 
2.33.1


From 4ece13844e1b40379f6515418e9ceed03af9a8b4 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Wed, 20 Oct 2021 10:43:33 +0200
Subject: [PATCH 10/22] ipatests: backup-reinstall-restore needs to clear sssd
 cache

The integration tests that check backup-reinstall-restore
scenario need to clear sssd cache before checking the uid
of the admin user. For instance:
backup: saves the original admin uid
reinstall: creates a new admin uid, potentially cached by SSSD
restore: restores the original admin uid

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 .../test_integration/test_backup_and_restore.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ipatests/test_integration/test_backup_and_restore.py b/ipatests/test_integration/test_backup_and_restore.py
index dcaddb1a1..a10c96f80 100644
--- a/ipatests/test_integration/test_backup_and_restore.py
+++ b/ipatests/test_integration/test_backup_and_restore.py
@@ -311,6 +311,11 @@ class BaseBackupAndRestoreWithDNS(IntegrationTest):
                 tasks.install_master(self.master, setup_dns=True)
             self.master.run_command(['ipa-restore', backup_path],
                                     stdin_text=dirman_password + '\nyes')
+            if reinstall:
+                # If the server was reinstalled, reinstall may have changed
+                # the uid and restore reverts to the original value.
+                # clear the cache to make sure we get up-to-date values
+                tasks.clear_sssd_cache(self.master)
             tasks.resolve_record(self.master.ip, self.example_test_zone)
 
             tasks.kinit_admin(self.master)
@@ -380,6 +385,12 @@ class BaseBackupAndRestoreWithDNSSEC(IntegrationTest):
             self.master.run_command(['ipa-restore', backup_path],
                                     stdin_text=dirman_password + '\nyes')
 
+            if reinstall:
+                # If the server was reinstalled, reinstall may have changed
+                # the uid and restore reverts to the original value.
+                # clear the cache to make sure we get up-to-date values
+                tasks.clear_sssd_cache(self.master)
+
             assert (
                 wait_until_record_is_signed(
                     self.master.ip, self.example_test_zone)
@@ -464,6 +475,12 @@ class BaseBackupAndRestoreWithKRA(IntegrationTest):
             self.master.run_command(['ipa-restore', backup_path],
                                     stdin_text=dirman_password + '\nyes')
 
+            if reinstall:
+                # If the server was reinstalled, reinstall may have changed
+                # the uid and restore reverts to the original value.
+                # clear the cache to make sure we get up-to-date values
+                tasks.clear_sssd_cache(self.master)
+
             tasks.kinit_admin(self.master)
             # retrieve secret after restore
             self.master.run_command([
-- 
2.33.1


From ea740e0a752ae8a22bb70e282a5a3e6a77c5f36f Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Thu, 21 Oct 2021 15:15:45 +0200
Subject: [PATCH 11/22] Webui tests: new idrange now requires base RID

Now that SID are always generated, the creation of a new
local idrange is refused if baserid is missing.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipatests/test_webui/test_range.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_webui/test_range.py b/ipatests/test_webui/test_range.py
index 239c1c442..a72def452 100644
--- a/ipatests/test_webui/test_range.py
+++ b/ipatests/test_webui/test_range.py
@@ -297,8 +297,13 @@ class test_range(range_tasks):
 
         # Without primary and secondary RID bases
         data = self.get_data(pkey, base_rid='', secondary_base_rid='')
-        self.add_record(ENTITY, data, navigate=False)
-        self.delete_record(pkey)
+        self.add_record(ENTITY, data, navigate=False, negative=True)
+        try:
+            assert self.has_form_error('ipabaserid')
+        finally:
+            self.delete_record(pkey)
+
+        self.dialog_button_click('cancel')
 
     @screenshot
     def test_modify_range_with_invalid_or_missing_values(self):
-- 
2.33.1


From b8cb1e3da32a326ac0626c2d3c9a988312593f3a Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Thu, 21 Oct 2021 15:18:12 +0200
Subject: [PATCH 12/22] User plugin: do not return the SID on user creation

The SID is not part of the default user attributes and does not
need to be returned in the user-add output.

Related: https://pagure.io/freeipa/issue/8995
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipaserver/plugins/user.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py
index 7a4acf66e..bc3932d39 100644
--- a/ipaserver/plugins/user.py
+++ b/ipaserver/plugins/user.py
@@ -658,6 +658,10 @@ class user_add(baseuser_add):
 
         entry_attrs.update(newentry)
 
+        # delete ipantsecurityidentifier if present
+        if ('ipantsecurityidentifier' in entry_attrs):
+            del entry_attrs['ipantsecurityidentifier']
+
         if options.get('random', False):
             try:
                 entry_attrs['randompassword'] = unicode(getattr(context, 'randompassword'))
-- 
2.33.1


From f00a52ae45b0c0e5fede4e18e2c9c785fd54ff87 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Thu, 21 Oct 2021 15:22:25 +0200
Subject: [PATCH 13/22] ipatests: update the expected output of user-add cmd

The SID is not expected to be returned by ipa user-add.

Related: https://pagure.io/freeipa/issue/8995
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
 ipatests/test_xmlrpc/test_idviews_plugin.py | 6 ++++--
 ipatests/test_xmlrpc/test_range_plugin.py   | 1 -
 ipatests/test_xmlrpc/test_user_plugin.py    | 3 +++
 ipatests/test_xmlrpc/tracker/user_plugin.py | 5 ++++-
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_idviews_plugin.py b/ipatests/test_xmlrpc/test_idviews_plugin.py
index fa535af7a..9b31f5d13 100644
--- a/ipatests/test_xmlrpc/test_idviews_plugin.py
+++ b/ipatests/test_xmlrpc/test_idviews_plugin.py
@@ -217,7 +217,8 @@ class test_idviews(Declarative):
                     u'Test',
                     u'User1',
                     'add',
-                    objectclass=objectclasses.user,
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.user, 'ipantuserattrs'),
                 ),
             ),
         ),
@@ -1623,7 +1624,8 @@ class test_idviews(Declarative):
                     u'Removed',
                     u'User',
                     'add',
-                    objectclass=objectclasses.user,
+                    objectclass=fuzzy_set_optional_oc(
+                        objectclasses.user, 'ipantuserattrs'),
                 ),
             ),
         ),
diff --git a/ipatests/test_xmlrpc/test_range_plugin.py b/ipatests/test_xmlrpc/test_range_plugin.py
index 168dbcbf3..bf4deebb2 100644
--- a/ipatests/test_xmlrpc/test_range_plugin.py
+++ b/ipatests/test_xmlrpc/test_range_plugin.py
@@ -496,7 +496,6 @@ class test_range(Declarative):
                     uidnumber=[unicode(user1_uid)],
                     gidnumber=[unicode(user1_uid)],
                     objectclass=objectclasses.user_base + [u'mepOriginEntry'],
-                    omit=['ipantsecurityidentifier'],
                 ),
             ),
         ),
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index c2b8f79c8..b093a9f2b 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -1182,6 +1182,9 @@ def get_user_result(uid, givenname, sn, operation='show', omit=[],
             objectclass=objectclasses.user,
             krbprincipalname=[u'%s@%s' % (uid, api.env.realm)],
             krbcanonicalname=[u'%s@%s' % (uid, api.env.realm)],
+        )
+    if operation == 'show-all':
+        result.update(
             ipantsecurityidentifier=[fuzzy_user_or_group_sid],
         )
     if operation in ('show', 'show-all', 'find', 'mod'):
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index 9adbe8d92..6cfb9518f 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -11,6 +11,7 @@ import six
 from ipatests.util import assert_deepequal, get_group_dn
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.xmlrpc_test import (
+    fuzzy_set_optional_oc,
     fuzzy_digits, fuzzy_uuid, fuzzy_user_or_group_sid, raises_exact)
 from ipatests.test_xmlrpc.tracker.base import Tracker
 from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin
@@ -51,6 +52,7 @@ class UserTracker(CertmapdataMixin, KerberosAliasMixin, Tracker):
         u'krbextradata', u'krbpasswordexpiration', u'krblastpwdchange',
         u'krbprincipalkey', u'userpassword', u'randompassword'}
     create_keys = create_keys - {u'nsaccountlock'}
+    create_keys = create_keys - {'ipantsecurityidentifier'}
 
     update_keys = retrieve_keys - {u'dn'}
     activate_keys = retrieve_keys
@@ -175,7 +177,8 @@ class UserTracker(CertmapdataMixin, KerberosAliasMixin, Tracker):
             displayname=[u'%s %s' % (self.givenname, self.sn)],
             cn=[u'%s %s' % (self.givenname, self.sn)],
             initials=[u'%s%s' % (self.givenname[0], self.sn[0])],
-            objectclass=objectclasses.user,
+            objectclass=fuzzy_set_optional_oc(
+                objectclasses.user, 'ipantuserattrs'),
             description=[u'__no_upg__'],
             ipauniqueid=[fuzzy_uuid],
             uidnumber=[fuzzy_digits],
-- 
2.33.1


From a5ac6f27ebc6d47e1aee79b4bb5da039b96e0d52 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Tue, 28 Sep 2021 10:24:32 +0300
Subject: [PATCH 14/22] ipa-kdb: store SID in the principal entry

If the principal entry in LDAP has SID associated with it, store it to
be able to quickly assess the SID when processing PAC.

Also rename string_to_sid to IPA-specific version as it uses different
prototype than Samba version.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Robert Crittenden <rcritten@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb.h               |  7 ++++++
 daemons/ipa-kdb/ipa_kdb_mspac.c         | 31 ++++++++++++++++++-------
 daemons/ipa-kdb/ipa_kdb_mspac_private.h |  1 -
 daemons/ipa-kdb/ipa_kdb_principals.c    | 25 ++++++++++++++++++++
 daemons/ipa-kdb/tests/ipa_kdb_tests.c   | 30 ++++++++++++------------
 5 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index 66a1d74f1..884dff950 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -79,6 +79,7 @@
 #define IPA_USER_AUTH_TYPE "ipaUserAuthType"
 
 struct ipadb_mspac;
+struct dom_sid;
 
 enum ipadb_user_auth {
   IPADB_USER_AUTH_NONE     = 0,
@@ -155,6 +156,8 @@ struct ipadb_e_data {
     bool has_tktpolaux;
     enum ipadb_user_auth user_auth;
     struct ipadb_e_pol_limits pol_limits[IPADB_USER_AUTH_IDX_MAX];
+    bool has_sid;
+    struct dom_sid *sid;
 };
 
 struct ipadb_context *ipadb_get_context(krb5_context kcontext);
@@ -366,3 +369,7 @@ int ipadb_get_enc_salt_types(struct ipadb_context *ipactx, LDAPMessage *entry,
 /* CERTAUTH PLUGIN */
 void ipa_certauth_free_moddata(krb5_certauth_moddata *moddata);
 #endif
+
+int ipadb_string_to_sid(const char *str, struct dom_sid *sid);
+void alloc_sid(struct dom_sid **sid);
+void free_sid(struct dom_sid **sid);
\ No newline at end of file
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 47b12a16f..f3e8657c2 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -80,7 +80,20 @@ static char *memberof_pac_attrs[] = {
 #define AUTHZ_DATA_TYPE_PAD "PAD"
 #define AUTHZ_DATA_TYPE_NONE "NONE"
 
-int string_to_sid(const char *str, struct dom_sid *sid)
+void alloc_sid(struct dom_sid **sid)
+{
+    *sid = malloc(sizeof(struct dom_sid));
+}
+
+void free_sid(struct dom_sid **sid)
+{
+    if (sid != NULL && *sid != NULL) {
+        free(*sid);
+        *sid = NULL;
+    }
+}
+
+int ipadb_string_to_sid(const char *str, struct dom_sid *sid)
 {
     unsigned long val;
     const char *s;
@@ -372,7 +385,7 @@ static krb5_error_code ipadb_add_asserted_identity(struct ipadb_context *ipactx,
 
     /* For S4U2Self, add Service Asserted Identity SID
      * otherwise, add Authentication Authority Asserted Identity SID */
-    ret = string_to_sid((flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) ?
+    ret = ipadb_string_to_sid((flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) ?
                         "S-1-18-2" : "S-1-18-1",
                         arr[sidcount].sid);
     if (ret) {
@@ -655,7 +668,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
             /* SID is mandatory */
             return ret;
         }
-        ret = string_to_sid(strres, &sid);
+        ret = ipadb_string_to_sid(strres, &sid);
         free(strres);
         if (ret) {
             return ret;
@@ -700,7 +713,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
                     }
                 }
                 if (strcasecmp(dval->type, "ipaNTSecurityIdentifier") == 0) {
-                    ret = string_to_sid((char *)dval->vals[0].bv_val, &gsid);
+                    ret = ipadb_string_to_sid((char *)dval->vals[0].bv_val, &gsid);
                     if (ret) {
                         continue;
                     }
@@ -1189,7 +1202,7 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext,
                             }
                             if (strcasecmp(dval->type,
                                            "ipaNTSecurityIdentifier") == 0) {
-                                kerr = string_to_sid((char *)dval->vals[0].bv_val, &sid);
+                                kerr = ipadb_string_to_sid((char *)dval->vals[0].bv_val, &sid);
                                 if (kerr != 0) {
                                     continue;
                                 }
@@ -2434,7 +2447,7 @@ ipadb_adtrusts_fill_sid_blacklist(char **source_sid_blacklist,
     }
 
     for (i = 0; i < len; i++) {
-         (void) string_to_sid(source[i], &sid_blacklist[i]);
+         (void) ipadb_string_to_sid(source[i], &sid_blacklist[i]);
     }
 
     *result_sids = sid_blacklist;
@@ -2594,7 +2607,7 @@ ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
             goto done;
         }
 
-        ret = string_to_sid(t[n].domain_sid, &t[n].domsid);
+        ret = ipadb_string_to_sid(t[n].domain_sid, &t[n].domsid);
         if (ret && t[n].domain_sid != NULL) {
             ret = EINVAL;
             goto done;
@@ -2812,7 +2825,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, bool force_rein
         goto done;
     }
 
-    ret = string_to_sid(resstr, &ipactx->mspac->domsid);
+    ret = ipadb_string_to_sid(resstr, &ipactx->mspac->domsid);
     if (ret) {
         kerr = ret;
         free(resstr);
@@ -2865,7 +2878,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, bool force_rein
                 goto done;
             }
             if (ret == 0) {
-                ret = string_to_sid(resstr, &gsid);
+                ret = ipadb_string_to_sid(resstr, &gsid);
                 if (ret) {
                     free(resstr);
                     kerr = ret;
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_private.h b/daemons/ipa-kdb/ipa_kdb_mspac_private.h
index 8c8a3a001..3696c3c6c 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac_private.h
+++ b/daemons/ipa-kdb/ipa_kdb_mspac_private.h
@@ -51,7 +51,6 @@ struct ipadb_adtrusts {
     size_t *upn_suffixes_len;
 };
 
-int string_to_sid(const char *str, struct dom_sid *sid);
 char *dom_sid_string(TALLOC_CTX *memctx, const struct dom_sid *dom_sid);
 krb5_error_code filter_logon_info(krb5_context context, TALLOC_CTX *memctx,
                                   krb5_data realm, struct PAC_LOGON_INFO_CTR *info);
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 0a98ff054..15f3df4fe 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -79,6 +79,8 @@ static char *std_principal_attrs[] = {
     "ipatokenRadiusConfigLink",
     "krbAuthIndMaxTicketLife",
     "krbAuthIndMaxRenewableAge",
+    "ipaNTSecurityIdentifier",
+    "ipaUniqueID",
 
     "objectClass",
     NULL
@@ -594,6 +596,7 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
     char *restring;
     char *uidstring;
     char **authz_data_list;
+    char *princ_sid;
     krb5_timestamp restime;
     bool resbool;
     int result;
@@ -963,6 +966,27 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
         ipadb_parse_authind_policies(kcontext, lcontext, lentry, entry, ua);
     }
 
+    /* Add SID if it is associated with the principal account */
+    ied->has_sid = false;
+    ied->sid = NULL;
+    ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
+                                 "ipaNTSecurityIdentifier", &princ_sid);
+    if (ret == 0 && princ_sid != NULL) {
+        alloc_sid(&ied->sid);
+        if (ied->sid == NULL) {
+            kerr = KRB5_KDB_INTERNAL_ERROR;
+            free(princ_sid);
+            goto done;
+        }
+        ret = ipadb_string_to_sid(princ_sid, ied->sid);
+        free(princ_sid);
+        if (ret != 0) {
+            kerr = ret;
+            goto done;
+        }
+        ied->has_sid = true;
+    }
+
     kerr = 0;
 
 done:
@@ -1568,6 +1592,7 @@ void ipadb_free_principal_e_data(krb5_context kcontext, krb5_octet *e_data)
 	}
 	free(ied->authz_data);
 	free(ied->pol);
+	free_sid(&ied->sid);
 	free(ied);
     }
 }
diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
index 0b51ffb96..d38dfd841 100644
--- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c
+++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c
@@ -105,7 +105,7 @@ static int setup(void **state)
     /* make sure data is not read from LDAP */
     ipa_ctx->mspac->last_update = time(NULL) - 1;
 
-    ret = string_to_sid(DOM_SID, &ipa_ctx->mspac->domsid);
+    ret = ipadb_string_to_sid(DOM_SID, &ipa_ctx->mspac->domsid);
     assert_int_equal(ret, 0);
 
     ipa_ctx->mspac->num_trusts = 1;
@@ -121,7 +121,7 @@ static int setup(void **state)
     ipa_ctx->mspac->trusts[0].domain_sid = strdup(DOM_SID_TRUST);
     assert_non_null(ipa_ctx->mspac->trusts[0].domain_sid);
 
-    ret = string_to_sid(DOM_SID_TRUST, &ipa_ctx->mspac->trusts[0].domsid);
+    ret = ipadb_string_to_sid(DOM_SID_TRUST, &ipa_ctx->mspac->trusts[0].domsid);
     assert_int_equal(ret, 0);
 
     ipa_ctx->mspac->trusts[0].len_sid_blocklist_incoming = 1;
@@ -129,7 +129,7 @@ static int setup(void **state)
                            ipa_ctx->mspac->trusts[0].len_sid_blocklist_incoming,
                            sizeof(struct dom_sid));
     assert_non_null(ipa_ctx->mspac->trusts[0].sid_blocklist_incoming);
-    ret = string_to_sid(BLOCKLIST_SID,
+    ret = ipadb_string_to_sid(BLOCKLIST_SID,
                         &ipa_ctx->mspac->trusts[0].sid_blocklist_incoming[0]);
     assert_int_equal(ret, 0);
 
@@ -216,7 +216,7 @@ static void test_filter_logon_info(void **state)
     assert_int_equal(kerr, EINVAL);
 
     /* wrong domain SID */
-    ret = string_to_sid("S-1-5-21-1-1-1", &dom_sid);
+    ret = ipadb_string_to_sid("S-1-5-21-1-1-1", &dom_sid);
     assert_int_equal(ret, 0);
     info->info->info3.base.domain_sid = &dom_sid;
 
@@ -224,7 +224,7 @@ static void test_filter_logon_info(void **state)
     assert_int_equal(kerr, EINVAL);
 
     /* matching domain SID */
-    ret = string_to_sid(DOM_SID_TRUST, &dom_sid);
+    ret = ipadb_string_to_sid(DOM_SID_TRUST, &dom_sid);
     assert_int_equal(ret, 0);
     info->info->info3.base.domain_sid = &dom_sid;
 
@@ -292,7 +292,7 @@ static void test_filter_logon_info(void **state)
         }
 
         for (d = 0; d < info->info->info3.sidcount; d++) {
-            ret = string_to_sid(test_data[c].sids[d],
+            ret = ipadb_string_to_sid(test_data[c].sids[d],
                                 info->info->info3.sids[d].sid);
             assert_int_equal(ret, 0);
         }
@@ -434,7 +434,7 @@ static void test_get_authz_data_types(void **state)
     krb5_free_principal(test_ctx->krb5_ctx, non_nfs_princ);
 }
 
-static void test_string_to_sid(void **state)
+static void test_ipadb_string_to_sid(void **state)
 {
     int ret;
     struct dom_sid sid;
@@ -442,25 +442,25 @@ static void test_string_to_sid(void **state)
                               {21, 2127521184, 1604012920, 1887927527, 72713,
                                0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
 
-    ret = string_to_sid(NULL, &sid);
+    ret = ipadb_string_to_sid(NULL, &sid);
     assert_int_equal(ret, EINVAL);
 
-    ret = string_to_sid("abc", &sid);
+    ret = ipadb_string_to_sid("abc", &sid);
     assert_int_equal(ret, EINVAL);
 
-    ret = string_to_sid("S-", &sid);
+    ret = ipadb_string_to_sid("S-", &sid);
     assert_int_equal(ret, EINVAL);
 
-    ret = string_to_sid("S-ABC", &sid);
+    ret = ipadb_string_to_sid("S-ABC", &sid);
     assert_int_equal(ret, EINVAL);
 
-    ret = string_to_sid("S-123", &sid);
+    ret = ipadb_string_to_sid("S-123", &sid);
     assert_int_equal(ret, EINVAL);
 
-    ret = string_to_sid("S-1-123-1-2-3-4-5-6-7-8-9-0-1-2-3-4-5-6", &sid);
+    ret = ipadb_string_to_sid("S-1-123-1-2-3-4-5-6-7-8-9-0-1-2-3-4-5-6", &sid);
     assert_int_equal(ret, EINVAL);
 
-    ret = string_to_sid("S-1-5-21-2127521184-1604012920-1887927527-72713",
+    ret = ipadb_string_to_sid("S-1-5-21-2127521184-1604012920-1887927527-72713",
                         &sid);
     assert_int_equal(ret, 0);
     assert_memory_equal(&exp_sid, &sid, sizeof(struct dom_sid));
@@ -531,7 +531,7 @@ int main(int argc, const char *argv[])
                                         setup, teardown),
         cmocka_unit_test_setup_teardown(test_filter_logon_info,
                                         setup, teardown),
-        cmocka_unit_test(test_string_to_sid),
+        cmocka_unit_test(test_ipadb_string_to_sid),
         cmocka_unit_test_setup_teardown(test_dom_sid_string,
                                         setup, teardown),
         cmocka_unit_test_setup_teardown(test_check_trusted_realms,
-- 
2.33.1


From fb9a718c4e65c2da2c95f3dcf43aa2326f41482d Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Tue, 28 Sep 2021 10:26:30 +0300
Subject: [PATCH 15/22] ipa-kdb: enforce SID checks when generating PAC

Check that a domain SID and a user SID in the PAC passed to us are what
they should be for the local realm's principal.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Robert Crittenden <rcritten@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb.h       |   3 +-
 daemons/ipa-kdb/ipa_kdb_mspac.c | 112 ++++++++++++++++++++++++++++----
 2 files changed, 100 insertions(+), 15 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index 884dff950..708e9545e 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -372,4 +372,5 @@ void ipa_certauth_free_moddata(krb5_certauth_moddata *moddata);
 
 int ipadb_string_to_sid(const char *str, struct dom_sid *sid);
 void alloc_sid(struct dom_sid **sid);
-void free_sid(struct dom_sid **sid);
\ No newline at end of file
+void free_sid(struct dom_sid **sid);
+bool dom_sid_check(const struct dom_sid *sid1, const struct dom_sid *sid2, bool exact_check);
\ No newline at end of file
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index f3e8657c2..59b77f5d8 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -236,7 +236,7 @@ static struct dom_sid *dom_sid_dup(TALLOC_CTX *memctx,
  * dom_sid_check() is supposed to be used with sid1 representing domain SID
  * and sid2 being either domain or resource SID in the domain
  */
-static bool dom_sid_check(const struct dom_sid *sid1, const struct dom_sid *sid2, bool exact_check)
+bool dom_sid_check(const struct dom_sid *sid1, const struct dom_sid *sid2, bool exact_check)
 {
     int c, num;
 
@@ -1404,6 +1404,82 @@ static void filter_logon_info_log_message_rid(struct dom_sid *sid, uint32_t rid)
     }
 }
 
+static krb5_error_code check_logon_info_consistent(krb5_context context,
+                                                   TALLOC_CTX *memctx,
+                                                   krb5_const_principal client_princ,
+                                                   struct PAC_LOGON_INFO_CTR *info)
+{
+    krb5_error_code kerr = 0;
+    struct ipadb_context *ipactx;
+    bool result;
+    krb5_db_entry *client_actual = NULL;
+    struct ipadb_e_data *ied = NULL;
+    int flags = 0;
+#ifdef KRB5_KDB_FLAG_ALIAS_OK
+    flags = KRB5_KDB_FLAG_ALIAS_OK;
+#endif
+
+    ipactx = ipadb_get_context(context);
+    if (!ipactx || !ipactx->mspac) {
+        return KRB5_KDB_DBNOTINITED;
+    }
+
+    /* check exact sid */
+    result = dom_sid_check(&ipactx->mspac->domsid,
+                           info->info->info3.base.domain_sid, true);
+    if (!result) {
+        /* memctx is freed by the caller */
+        char *sid = dom_sid_string(memctx, info->info->info3.base.domain_sid);
+        char *dom = dom_sid_string(memctx, &ipactx->mspac->domsid);
+        krb5_klog_syslog(LOG_ERR, "PAC issue: PAC record claims domain SID different "
+                                  "to local domain SID: local [%s], PAC [%s]",
+				  dom ? dom : "<failed to display>",
+                                  sid ? sid : "<failed to display>");
+        return KRB5KDC_ERR_POLICY;
+    }
+
+    kerr = ipadb_get_principal(context, client_princ, flags, &client_actual);
+    if (kerr != 0) {
+        krb5_klog_syslog(LOG_ERR, "PAC issue: ipadb_get_principal failed.");
+        return KRB5KDC_ERR_POLICY;
+    }
+
+    ied = (struct ipadb_e_data *)client_actual->e_data;
+    if (ied == NULL || ied->magic != IPA_E_DATA_MAGIC) {
+        krb5_klog_syslog(LOG_ERR, "PAC issue: client e_data fetching failed.");
+        kerr = EINVAL;
+        goto done;
+    }
+
+    if (!ied->has_sid || ied->sid == NULL) {
+        /* Kerberos principal might have no SID associated in the DB entry.
+         * If this is host or service, we'll associate RID -515 or -516 in PAC
+         * depending on whether this is a domain member or domain controller
+         * but since this is not recorded in the DB entry, we the check for
+         * SID is not needed */
+        goto done;
+    }
+
+    result = dom_sid_check(ied->sid, info->info->info3.sids[0].sid, true);
+    if (!result) {
+        /* memctx is freed by the caller */
+        char *local_sid = dom_sid_string(memctx, ied->sid);
+        char *pac_sid = dom_sid_string(memctx, info->info->info3.sids[0].sid);
+        krb5_klog_syslog(LOG_ERR, "PAC issue: client principal has a SID "
+                                  "different from what PAC claims. "
+                                  "local [%s] vs PAC [%s]",
+                                  local_sid ? local_sid : "<failed to display>",
+                                  pac_sid ? pac_sid : "<failed to display>");
+        kerr = KRB5KDC_ERR_POLICY;
+        goto done;
+    }
+
+done:
+    ipadb_free_principal(context, client_actual);
+
+    return kerr;
+}
+
 krb5_error_code filter_logon_info(krb5_context context,
                                   TALLOC_CTX *memctx,
                                   krb5_data realm,
@@ -1631,12 +1707,14 @@ krb5_error_code filter_logon_info(krb5_context context,
 
 
 static krb5_error_code ipadb_check_logon_info(krb5_context context,
-                                              krb5_data origin_realm,
+                                              krb5_const_principal client_princ,
+                                              krb5_boolean is_cross_realm,
                                               krb5_data *pac_blob)
 {
     struct PAC_LOGON_INFO_CTR info;
     krb5_error_code kerr;
     TALLOC_CTX *tmpctx;
+    krb5_data origin_realm = client_princ->realm;
 
     tmpctx = talloc_new(NULL);
     if (!tmpctx) {
@@ -1648,6 +1726,13 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context,
         goto done;
     }
 
+    if (!is_cross_realm) {
+        /* For local realm case we need to check whether the PAC is for our user
+         * but we don't need to process further */
+        kerr = check_logon_info_consistent(context, tmpctx, client_princ, &info);
+        goto done;
+    }
+
     kerr = filter_logon_info(context, tmpctx, origin_realm, &info);
     if (kerr) {
         goto done;
@@ -1873,19 +1958,18 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
         goto done;
     }
 
-    /* Now that the PAC is verified augment it with additional info if
-     * it is coming from a different realm */
-    if (is_cross_realm) {
-        kerr = krb5_pac_get_buffer(context, old_pac,
-                                   KRB5_PAC_LOGON_INFO, &pac_blob);
-        if (kerr != 0) {
-            goto done;
-        }
+    /* Now that the PAC is verified, do additional checks.
+     * Augment it with additional info if it is coming from a different realm */
+    kerr = krb5_pac_get_buffer(context, old_pac,
+                               KRB5_PAC_LOGON_INFO, &pac_blob);
+    if (kerr != 0) {
+        goto done;
+    }
 
-        kerr = ipadb_check_logon_info(context, client_princ->realm, &pac_blob);
-        if (kerr != 0) {
-            goto done;
-        }
+    kerr = ipadb_check_logon_info(context,
+                                  client_princ, is_cross_realm, &pac_blob);
+    if (kerr != 0) {
+        goto done;
     }
     /* extract buffers and rebuilt pac from scratch so that when re-signing
      * with a different cksum type does not cause issues due to mismatching
-- 
2.33.1


From 8c05cd0ffe74b8ec7943b31571bc11400af54171 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Thu, 14 Oct 2021 11:28:02 +0300
Subject: [PATCH 16/22] ipa-kdb: use entry DN to compare aliased entries in S4U
 operations

When working with aliased entries, we need a reliable way to detect
whether two principals reference the same database entry. This is
important in S4U checks.

Ideally, we should be using SIDs for these checks as S4U requires PAC
record presence which cannot be issued without a SID associated with an
entry. This is true for user principals and a number of host/service
principals associated with Samba. Other service principals do not have
SIDs because we do not allocate POSIX IDs to them in FreeIPA. When PAC
is issued for these principals, they get SID of a domain computer or
domain controller depending on their placement (IPA client or IPA
server).

Since 389-ds always returns unique entry DN for the same entry, rely on
this value instead. We could have used ipaUniqueID but for Kerberos
principals created through the KDB (kadmin/kdb5_util) we don't have
ipaUniqueID in the entry.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_delegation.c | 36 +++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_delegation.c b/daemons/ipa-kdb/ipa_kdb_delegation.c
index 5ae5e0d9d..bfa344b9f 100644
--- a/daemons/ipa-kdb/ipa_kdb_delegation.c
+++ b/daemons/ipa-kdb/ipa_kdb_delegation.c
@@ -21,6 +21,8 @@
  */
 
 #include "ipa_kdb.h"
+#include <strings.h>
+#include <unicase.h>
 
 static char *acl_attrs[] = {
     "objectClass",
@@ -188,10 +190,41 @@ krb5_error_code ipadb_check_allowed_to_delegate(krb5_context kcontext,
                                                 const krb5_db_entry *server,
                                                 krb5_const_principal proxy)
 {
-    krb5_error_code kerr;
+    krb5_error_code kerr, result;
     char *srv_principal = NULL;
+    krb5_db_entry *proxy_entry = NULL;
+    struct ipadb_e_data *ied_server, *ied_proxy;
     LDAPMessage *res = NULL;
 
+    /* Handle the case where server == proxy, this is allowed in S4U*/
+    kerr = ipadb_get_principal(kcontext, proxy,
+                               KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY,
+                               &proxy_entry);
+    if (kerr) {
+        goto done;
+    }
+
+    ied_server = (struct ipadb_e_data *) server->e_data;
+    ied_proxy = (struct ipadb_e_data *) proxy_entry->e_data;
+
+    /* If we have SIDs for both entries, compare SIDs */
+    if ((ied_server->has_sid && ied_server->sid != NULL) &&
+        (ied_proxy->has_sid && ied_proxy->sid != NULL)) {
+
+        if (dom_sid_check(ied_server->sid, ied_proxy->sid, true)) {
+            kerr = 0;
+            goto done;
+        }
+    }
+
+    /* Otherwise, compare entry DNs */
+    kerr = ulc_casecmp(ied_server->entry_dn, strlen(ied_server->entry_dn),
+                       ied_proxy->entry_dn, strlen(ied_proxy->entry_dn),
+                       NULL, NULL, &result);
+    if (kerr == 0 && result == 0) {
+        goto done;
+    }
+
     kerr = krb5_unparse_name(kcontext, server->princ, &srv_principal);
     if (kerr) {
         goto done;
@@ -208,6 +241,7 @@ krb5_error_code ipadb_check_allowed_to_delegate(krb5_context kcontext,
     }
 
 done:
+    ipadb_free_principal(kcontext, proxy_entry);
     krb5_free_unparsed_name(kcontext, srv_principal);
     ldap_msgfree(res);
     return kerr;
-- 
2.33.1


From 07055573acf1af9d31a2a4da79f1eebc34b805fc Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Thu, 28 Oct 2021 11:01:08 +0300
Subject: [PATCH 17/22] ipa-kdb: S4U2Proxy target should use a service name
 without realm

According to new Samba Kerberos tests and [MS-SFU] 3.2.5.2.4
'KDC Replies with Service Ticket', the target should not include the
realm.

Fixes: https://pagure.io/freeipa/issue/9031

Pair-programmed-with: Andreas Schneider <asn@redhat.com>
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Signed-off-by: Andreas Schneider <asn@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 59b77f5d8..f729b9f2e 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -1847,7 +1847,10 @@ static krb5_error_code ipadb_add_transited_service(krb5_context context,
     krb5_free_data_contents(context, &pac_blob);
     memset(&pac_blob, 0, sizeof(krb5_data));
 
-    kerr = krb5_unparse_name(context, proxy->princ, &tmpstr);
+    kerr = krb5_unparse_name_flags(context, proxy->princ,
+                                   KRB5_PRINCIPAL_UNPARSE_NO_REALM |
+                                   KRB5_PRINCIPAL_UNPARSE_DISPLAY,
+                                   &tmpstr);
     if (kerr != 0) {
         goto done;
     }
-- 
2.33.1


From d6b97c1b869b511b381e166cc233a1fa7416a928 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Sat, 30 Oct 2021 10:08:34 +0300
Subject: [PATCH 18/22] ipa-kdb: add support for PAC_UPN_DNS_INFO_EX

CVE-2020-25721 mitigation: KDC must provide the new HAS_SAM_NAME_AND_SID
buffer with sAMAccountName and ObjectSID values associated with the
principal.

The mitigation only works if NDR library supports the
PAC_UPN_DNS_INFO_EX buffer type. In case we cannot detect it at compile
time, a warning will be displayed at configure stage.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 41 +++++++++++++++++++++++++++++++--
 server.m4                       |  7 ++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index f729b9f2e..75cb4f3b7 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -812,6 +812,25 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
     return ret;
 }
 
+static krb5_error_code ipadb_get_sid_from_pac(TALLOC_CTX *ctx,
+                                              struct PAC_LOGON_INFO *info,
+                                              struct dom_sid *sid)
+{
+    struct dom_sid *client_sid = NULL;
+    /* Construct SID from the PAC */
+    if (info->info3.base.rid == 0) {
+        client_sid = info->info3.sids[0].sid;
+    } else {
+        client_sid = dom_sid_dup(ctx, info->info3.base.domain_sid);
+        if (!client_sid) {
+            return ENOMEM;
+        }
+        sid_append_rid(client_sid, info->info3.base.rid);
+    }
+    *sid = *client_sid;
+    return 0;
+}
+
 static krb5_error_code ipadb_get_pac(krb5_context kcontext,
                                      krb5_db_entry *client,
                                      unsigned int flags,
@@ -830,6 +849,7 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
     enum ndr_err_code ndr_err;
     union PAC_INFO pac_upn;
     char *principal = NULL;
+    struct dom_sid client_sid;
 
     /* When no client entry is there, we cannot generate MS-PAC */
     if (!client) {
@@ -930,6 +950,18 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
         pac_upn.upn_dns_info.flags |= PAC_UPN_DNS_FLAG_CONSTRUCTED;
     }
 
+    kerr = ipadb_get_sid_from_pac(tmpctx, pac_info.logon_info.info, &client_sid);
+    if (kerr) {
+        goto done;
+    }
+
+#ifdef HAVE_PAC_UPN_DNS_INFO_EX
+    /* Add samAccountName and a SID */
+    pac_upn.upn_dns_info.flags |= PAC_UPN_DNS_FLAG_HAS_SAM_NAME_AND_SID;
+    pac_upn.upn_dns_info.ex.sam_name_and_sid.samaccountname = pac_info.logon_info.info->info3.base.account_name.string;
+    pac_upn.upn_dns_info.ex.sam_name_and_sid.objectsid = &client_sid;
+#endif
+
     ndr_err = ndr_push_union_blob(&pac_data, tmpctx, &pac_upn,
                                   PAC_TYPE_UPN_DNS_INFO,
                                   (ndr_push_flags_fn_t)ndr_push_PAC_INFO);
@@ -1415,6 +1447,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context,
     krb5_db_entry *client_actual = NULL;
     struct ipadb_e_data *ied = NULL;
     int flags = 0;
+    struct dom_sid client_sid;
 #ifdef KRB5_KDB_FLAG_ALIAS_OK
     flags = KRB5_KDB_FLAG_ALIAS_OK;
 #endif
@@ -1460,11 +1493,15 @@ static krb5_error_code check_logon_info_consistent(krb5_context context,
         goto done;
     }
 
-    result = dom_sid_check(ied->sid, info->info->info3.sids[0].sid, true);
+    kerr = ipadb_get_sid_from_pac(memctx, info->info, &client_sid);
+    if (kerr) {
+        goto done;
+    }
+    result = dom_sid_check(ied->sid, &client_sid, true);
     if (!result) {
         /* memctx is freed by the caller */
         char *local_sid = dom_sid_string(memctx, ied->sid);
-        char *pac_sid = dom_sid_string(memctx, info->info->info3.sids[0].sid);
+        char *pac_sid = dom_sid_string(memctx, &client_sid);
         krb5_klog_syslog(LOG_ERR, "PAC issue: client principal has a SID "
                                   "different from what PAC claims. "
                                   "local [%s] vs PAC [%s]",
diff --git a/server.m4 b/server.m4
index 3a2c3ae33..65c82d25a 100644
--- a/server.m4
+++ b/server.m4
@@ -101,6 +101,13 @@ AC_CHECK_MEMBER(
     [AC_MSG_NOTICE([struct PAC_DOMAIN_GROUP_MEMBERSHIP is not available])],
                  [[#include <ndr.h>
                    #include <gen_ndr/krb5pac.h>]])
+AC_CHECK_MEMBER(
+    [struct PAC_UPN_DNS_INFO.ex],
+    [AC_DEFINE([HAVE_PAC_UPN_DNS_INFO_EX], [1],
+               [union PAC_UPN_DNS_INFO_EX is available.])],
+    [AC_MSG_NOTICE([union PAC_UPN_DNS_INFO_EX is not available, account protection is not active])],
+                 [[#include <ndr.h>
+                   #include <gen_ndr/krb5pac.h>]])
 
 CFLAGS="$bck_cflags"
 
-- 
2.33.1


From 3e88bc63c919003a46fbf8d018d724bf00928c51 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Sat, 30 Oct 2021 10:09:27 +0300
Subject: [PATCH 19/22] ipa-kdb: add support for PAC_REQUESTER_SID buffer

CVE-2020-25721 mitigation: KDC must provide the new PAC_REQUESTER_SID
buffer with ObjectSID value associated with the requester's principal.

The mitigation only works if NDR library supports the PAC_REQUESTER_SID
buffer type. In case we cannot detect it at compile time, a warning will
be displayed at configure stage.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 131 +++++++++++++++++++++++++++++++-
 server.m4                       |   7 ++
 2 files changed, 134 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 75cb4f3b7..ca6daccf1 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -812,6 +812,55 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
     return ret;
 }
 
+#ifdef HAVE_PAC_REQUESTER_SID
+static krb5_error_code ipadb_get_requester_sid(krb5_context context,
+                                               krb5_pac pac,
+                                               struct dom_sid *sid)
+{
+    enum ndr_err_code ndr_err;
+    krb5_error_code ret;
+    DATA_BLOB pac_requester_sid_in;
+    krb5_data k5pac_requester_sid_in;
+    union PAC_INFO info;
+    TALLOC_CTX *tmp_ctx;
+    struct ipadb_context *ipactx;
+
+    ipactx = ipadb_get_context(context);
+    if (!ipactx) {
+        return KRB5_KDB_DBNOTINITED;
+    }
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    ret = krb5_pac_get_buffer(context, pac, PAC_TYPE_REQUESTER_SID,
+                              &k5pac_requester_sid_in);
+    if (ret != 0) {
+        talloc_free(tmp_ctx);
+        return ret;
+    }
+
+    pac_requester_sid_in = data_blob_const(k5pac_requester_sid_in.data,
+                                           k5pac_requester_sid_in.length);
+
+    ndr_err = ndr_pull_union_blob(&pac_requester_sid_in, tmp_ctx, &info,
+                                  PAC_TYPE_REQUESTER_SID,
+                                  (ndr_pull_flags_fn_t)ndr_pull_PAC_INFO);
+    krb5_free_data_contents(context, &k5pac_requester_sid_in);
+    if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+            talloc_free(tmp_ctx);
+            return EINVAL;
+    }
+
+    *sid = info.requester_sid.sid;
+
+    talloc_free(tmp_ctx);
+    return 0;
+}
+#endif
+
 static krb5_error_code ipadb_get_sid_from_pac(TALLOC_CTX *ctx,
                                               struct PAC_LOGON_INFO *info,
                                               struct dom_sid *sid)
@@ -976,6 +1025,33 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
 
     kerr = krb5_pac_add_buffer(kcontext, *pac, KRB5_PAC_UPN_DNS_INFO, &data);
 
+#ifdef HAVE_PAC_REQUESTER_SID
+    {
+        union PAC_INFO pac_requester_sid;
+        /* == Package PAC_REQUESTER_SID == */
+        memset(&pac_requester_sid, 0, sizeof(pac_requester_sid));
+
+        pac_requester_sid.requester_sid.sid = client_sid;
+
+        ndr_err = ndr_push_union_blob(&pac_data, tmpctx, &pac_requester_sid,
+                                    PAC_TYPE_REQUESTER_SID,
+                                    (ndr_push_flags_fn_t)ndr_push_PAC_INFO);
+        if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+            kerr = KRB5_KDB_INTERNAL_ERROR;
+            goto done;
+        }
+
+        data.magic = KV5M_DATA;
+        data.data = (char *)pac_data.data;
+        data.length = pac_data.length;
+
+        kerr = krb5_pac_add_buffer(kcontext, *pac, PAC_TYPE_REQUESTER_SID, &data);
+        if (kerr) {
+            goto done;
+        }
+    }
+#endif
+
 done:
     ldap_msgfree(results);
     talloc_free(tmpctx);
@@ -1457,7 +1533,19 @@ static krb5_error_code check_logon_info_consistent(krb5_context context,
         return KRB5_KDB_DBNOTINITED;
     }
 
-    /* check exact sid */
+    /* We are asked to verify the PAC for our own principal,
+     * check that our own view on the PAC details is up to date */
+    if (ipactx->mspac->domsid.num_auths == 0) {
+        /* Force re-init of KDB's view on our domain */
+        kerr = ipadb_reinit_mspac(ipactx, true);
+        if (kerr != 0) {
+            krb5_klog_syslog(LOG_ERR,
+                             "PAC issue: unable to update realm's view on PAC info");
+            return KRB5KDC_ERR_POLICY;
+        }
+    }
+
+    /* check exact domain SID */
     result = dom_sid_check(&ipactx->mspac->domsid,
                            info->info->info3.base.domain_sid, true);
     if (!result) {
@@ -1466,7 +1554,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context,
         char *dom = dom_sid_string(memctx, &ipactx->mspac->domsid);
         krb5_klog_syslog(LOG_ERR, "PAC issue: PAC record claims domain SID different "
                                   "to local domain SID: local [%s], PAC [%s]",
-				  dom ? dom : "<failed to display>",
+                                  dom ? dom : "<failed to display>",
                                   sid ? sid : "<failed to display>");
         return KRB5KDC_ERR_POLICY;
     }
@@ -1746,12 +1834,14 @@ krb5_error_code filter_logon_info(krb5_context context,
 static krb5_error_code ipadb_check_logon_info(krb5_context context,
                                               krb5_const_principal client_princ,
                                               krb5_boolean is_cross_realm,
-                                              krb5_data *pac_blob)
+                                              krb5_data *pac_blob,
+                                              struct dom_sid *requester_sid)
 {
     struct PAC_LOGON_INFO_CTR info;
     krb5_error_code kerr;
     TALLOC_CTX *tmpctx;
     krb5_data origin_realm = client_princ->realm;
+    bool result;
 
     tmpctx = talloc_new(NULL);
     if (!tmpctx) {
@@ -1763,6 +1853,28 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context,
         goto done;
     }
 
+    /* Check that requester SID is the same as in the PAC entry */
+    if (requester_sid != NULL) {
+        struct dom_sid client_sid;
+        kerr = ipadb_get_sid_from_pac(tmpctx, info.info, &client_sid);
+        if (kerr) {
+            goto done;
+        }
+        result = dom_sid_check(&client_sid, requester_sid, true);
+        if (!result) {
+            /* memctx is freed by the caller */
+            char *pac_sid = dom_sid_string(tmpctx, &client_sid);
+            char *req_sid = dom_sid_string(tmpctx, requester_sid);
+            krb5_klog_syslog(LOG_ERR, "PAC issue: PAC has a SID "
+                                      "different from what PAC requester claims. "
+                                      "PAC [%s] vs PAC requester [%s]",
+                                      pac_sid ? pac_sid : "<failed to display>",
+                                      req_sid ? req_sid : "<failed to display>");
+            kerr = KRB5KDC_ERR_POLICY;
+            goto done;
+        }
+    }
+
     if (!is_cross_realm) {
         /* For local realm case we need to check whether the PAC is for our user
          * but we don't need to process further */
@@ -1962,6 +2074,8 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
     krb5_data pac_blob = { 0 , 0, NULL};
     bool is_cross_realm = false;
     size_t i;
+    struct dom_sid *requester_sid = NULL;
+    struct dom_sid req_sid;
 
     kerr = krb5_pac_parse(context,
                           authdata[0]->contents,
@@ -2006,8 +2120,17 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
         goto done;
     }
 
+    memset(&req_sid, '\0', sizeof(struct dom_sid));
+#ifdef HAVE_PAC_REQUESTER_SID
+    kerr = ipadb_get_requester_sid(context, old_pac, &req_sid);
+    if (kerr == 0) {
+        requester_sid = &req_sid;
+    }
+#endif
+
     kerr = ipadb_check_logon_info(context,
-                                  client_princ, is_cross_realm, &pac_blob);
+                                  client_princ, is_cross_realm, &pac_blob,
+                                  requester_sid);
     if (kerr != 0) {
         goto done;
     }
diff --git a/server.m4 b/server.m4
index 65c82d25a..648423dd4 100644
--- a/server.m4
+++ b/server.m4
@@ -109,6 +109,13 @@ AC_CHECK_MEMBER(
                  [[#include <ndr.h>
                    #include <gen_ndr/krb5pac.h>]])
 
+AC_CHECK_MEMBER(
+    [struct PAC_REQUESTER_SID.sid],
+    [AC_DEFINE([HAVE_PAC_REQUESTER_SID], [1],
+               [struct PAC_REQUESTER_SID is available.])],
+    [AC_MSG_NOTICE([struct PAC_REQUESTER_SID is not available, account protection is not active])],
+                 [[#include <ndr.h>
+                   #include <gen_ndr/krb5pac.h>]])
 CFLAGS="$bck_cflags"
 
 LIBPDB_NAME=""
-- 
2.33.1


From 8a088fd0824d73e43b8ca56c04c5899f77619db9 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Sat, 30 Oct 2021 09:10:09 +0300
Subject: [PATCH 20/22] ipa-kdb: add PAC_ATTRIBUTES_INFO PAC buffer support

PAC_ATTRIBUTES_INFO PAC buffer allows both client and KDC to tell
whether a PAC structure was requested by the client or it was provided
by the KDC implicitly. Kerberos service then can continue processing or
deny access in case client explicitly requested to operate without PAC.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
 daemons/ipa-kdb/Makefile.am     |   2 +
 daemons/ipa-kdb/ipa_kdb_mspac.c | 143 ++++++++++++++++++++++++++++++++
 server.m4                       |   8 ++
 3 files changed, 153 insertions(+)

diff --git a/daemons/ipa-kdb/Makefile.am b/daemons/ipa-kdb/Makefile.am
index 14c0546e0..5775d4086 100644
--- a/daemons/ipa-kdb/Makefile.am
+++ b/daemons/ipa-kdb/Makefile.am
@@ -61,6 +61,7 @@ ipadb_la_LIBADD = 		\
 	$(UNISTRING_LIBS)	\
 	$(SSSCERTMAP_LIBS)	\
 	$(top_builddir)/util/libutil.la	\
+        -lsamba-errors          \
 	$(NULL)
 
 if HAVE_CMOCKA
@@ -104,6 +105,7 @@ ipa_kdb_tests_LDADD =          \
        -lkdb5                  \
        -lsss_idmap             \
        -lsamba-security-samba4 \
+       -lsamba-errors          \
        $(NULL)
 
 appdir = $(libexecdir)/ipa
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index ca6daccf1..8a77a3538 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -880,6 +880,87 @@ static krb5_error_code ipadb_get_sid_from_pac(TALLOC_CTX *ctx,
     return 0;
 }
 
+#ifdef HAVE_PAC_ATTRIBUTES_INFO
+static krb5_error_code ipadb_client_requested_pac(krb5_context context,
+                                                  krb5_pac pac,
+                                                  TALLOC_CTX *mem_ctx,
+                                                  krb5_boolean *requested_pac)
+{
+    enum ndr_err_code ndr_err;
+    krb5_data k5pac_attrs_in;
+    DATA_BLOB pac_attrs_in;
+    union PAC_INFO pac_attrs;
+    krb5_error_code ret;
+
+    *requested_pac = true;
+
+    ret = krb5_pac_get_buffer(context, pac, PAC_TYPE_ATTRIBUTES_INFO,
+                                &k5pac_attrs_in);
+    if (ret != 0) {
+            return ret == ENOENT ? 0 : ret;
+    }
+
+    pac_attrs_in = data_blob_const(k5pac_attrs_in.data,
+                                   k5pac_attrs_in.length);
+
+    ndr_err = ndr_pull_union_blob(&pac_attrs_in, mem_ctx, &pac_attrs,
+                                  PAC_TYPE_ATTRIBUTES_INFO,
+                                  (ndr_pull_flags_fn_t)ndr_pull_PAC_INFO);
+    krb5_free_data_contents(context, &k5pac_attrs_in);
+    if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+            NTSTATUS nt_status = ndr_map_error2ntstatus(ndr_err);
+            krb5_klog_syslog(LOG_ERR, "can't parse the PAC ATTRIBUTES_INFO: %s\n",
+                                        nt_errstr(nt_status));
+            return KRB5_KDB_INTERNAL_ERROR;
+    }
+
+    if (pac_attrs.attributes_info.flags & (PAC_ATTRIBUTE_FLAG_PAC_WAS_GIVEN_IMPLICITLY
+                                           | PAC_ATTRIBUTE_FLAG_PAC_WAS_REQUESTED)) {
+            *requested_pac = true;
+    } else {
+            *requested_pac = false;
+    }
+
+    return 0;
+}
+
+static krb5_error_code ipadb_get_pac_attrs_blob(TALLOC_CTX *mem_ctx,
+                                                const krb5_boolean *pac_request,
+                                                DATA_BLOB *pac_attrs_data)
+{
+    union PAC_INFO pac_attrs;
+    enum ndr_err_code ndr_err;
+
+    memset(&pac_attrs, 0, sizeof(pac_attrs));
+
+    *pac_attrs_data = data_blob_null;
+
+    /* Set the length of the flags in bits. */
+    pac_attrs.attributes_info.flags_length = 2;
+
+    if (pac_request == NULL) {
+            pac_attrs.attributes_info.flags
+                    |= PAC_ATTRIBUTE_FLAG_PAC_WAS_GIVEN_IMPLICITLY;
+    } else if (*pac_request) {
+            pac_attrs.attributes_info.flags
+                    |= PAC_ATTRIBUTE_FLAG_PAC_WAS_REQUESTED;
+    }
+
+    ndr_err = ndr_push_union_blob(pac_attrs_data, mem_ctx, &pac_attrs,
+                                    PAC_TYPE_ATTRIBUTES_INFO,
+                                    (ndr_push_flags_fn_t)ndr_push_PAC_INFO);
+    if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+            NTSTATUS nt_status = ndr_map_error2ntstatus(ndr_err);
+            krb5_klog_syslog(LOG_ERR, "can't create PAC ATTRIBUTES_INFO: %s\n",
+                            nt_errstr(nt_status));
+            return KRB5_KDB_INTERNAL_ERROR;
+    }
+
+    return 0;
+}
+
+#endif
+
 static krb5_error_code ipadb_get_pac(krb5_context kcontext,
                                      krb5_db_entry *client,
                                      unsigned int flags,
@@ -1025,6 +1106,26 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
 
     kerr = krb5_pac_add_buffer(kcontext, *pac, KRB5_PAC_UPN_DNS_INFO, &data);
 
+#ifdef HAVE_PAC_ATTRIBUTES_INFO
+    /* == Add implicit PAC type attributes info as we always try to generate PAC == */
+    {
+        DATA_BLOB pac_attrs_data;
+
+        kerr = ipadb_get_pac_attrs_blob(tmpctx, NULL, &pac_attrs_data);
+        if (kerr) {
+            goto done;
+        }
+        data.magic = KV5M_DATA;
+        data.data = (char *)pac_attrs_data.data;
+        data.length = pac_attrs_data.length;
+
+        kerr = krb5_pac_add_buffer(kcontext, *pac, PAC_TYPE_ATTRIBUTES_INFO, &data);
+        if (kerr) {
+            goto done;
+        }
+    }
+#endif
+
 #ifdef HAVE_PAC_REQUESTER_SID
     {
         union PAC_INFO pac_requester_sid;
@@ -2165,6 +2266,48 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
             continue;
         }
 
+#ifdef HAVE_PAC_ATTRIBUTES_INFO
+        if (types[i] == PAC_TYPE_ATTRIBUTES_INFO &&
+            pac_blob.length != 0) {
+            /* == Check whether PAC was requested or given implicitly == */
+            DATA_BLOB pac_attrs_data;
+            krb5_boolean pac_requested;
+
+            TALLOC_CTX *tmpctx = talloc_new(NULL);
+            if (tmpctx == NULL) {
+                kerr = ENOMEM;
+                krb5_pac_free(context, new_pac);
+                goto done;
+            }
+
+            kerr = ipadb_client_requested_pac(context, old_pac, tmpctx, &pac_requested);
+            if (kerr != 0) {
+                talloc_free(tmpctx);
+                krb5_pac_free(context, new_pac);
+                goto done;
+            }
+
+            kerr = ipadb_get_pac_attrs_blob(tmpctx, &pac_requested, &pac_attrs_data);
+            if (kerr) {
+                talloc_free(tmpctx);
+                krb5_pac_free(context, new_pac);
+                goto done;
+            }
+            data.magic = KV5M_DATA;
+            data.data = (char *)pac_attrs_data.data;
+            data.length = pac_attrs_data.length;
+
+            kerr = krb5_pac_add_buffer(context, new_pac, PAC_TYPE_ATTRIBUTES_INFO, &data);
+            if (kerr) {
+                talloc_free(tmpctx);
+                krb5_pac_free(context, new_pac);
+                goto done;
+            }
+
+            continue;
+        }
+#endif
+
         if (types[i] == KRB5_PAC_DELEGATION_INFO &&
             (flags & KRB5_KDB_FLAG_CONSTRAINED_DELEGATION)) {
             /* skip it here, we will add it explicitly later */
diff --git a/server.m4 b/server.m4
index 648423dd4..5a14af06a 100644
--- a/server.m4
+++ b/server.m4
@@ -116,6 +116,14 @@ AC_CHECK_MEMBER(
     [AC_MSG_NOTICE([struct PAC_REQUESTER_SID is not available, account protection is not active])],
                  [[#include <ndr.h>
                    #include <gen_ndr/krb5pac.h>]])
+
+AC_CHECK_MEMBER(
+    [struct PAC_ATTRIBUTES_INFO.flags],
+    [AC_DEFINE([HAVE_PAC_ATTRIBUTES_INFO], [1],
+               [struct PAC_ATTRIBUTES_INFO is available.])],
+    [AC_MSG_NOTICE([struct PAC_ATTRIBUTES_INFO is not available, account protection is not active])],
+                 [[#include <ndr.h>
+                   #include <gen_ndr/krb5pac.h>]])
 CFLAGS="$bck_cflags"
 
 LIBPDB_NAME=""
-- 
2.33.1


From 5e0fe9bff0ad2786f37a718c8b25b85836c8ff82 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Fri, 29 Oct 2021 22:30:53 +0300
Subject: [PATCH 21/22] ipa-kdb: Use proper account flags for Kerberos
 principal in PAC

As part of CVE-2020-25717 mitigations, Samba expects correct user
account flags in the PAC. This means for services and host principals we
should be using ACB_WSTRUST or ACB_SVRTRUST depending on whether they
run on IPA clients ("workstation" or "domain member") or IPA servers
("domain controller").

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 8a77a3538..0e0ee3616 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -648,6 +648,11 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
     info3->base.logon_count = 0; /* we do not have this info yet */
     info3->base.bad_password_count = 0; /* we do not have this info yet */
 
+    /* Use AES keys by default to detect changes.
+     * This bit is not used by Windows clients and servers so we can
+     * clear it after detecting the changes */
+    info3->base.acct_flags = ACB_USE_AES_KEYS;
+
     if ((is_host || is_service)) {
         /* it is either host or service, so get the hostname first */
         char *sep = strchr(info3->base.account_name.string, '/');
@@ -655,11 +660,13 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
                             ipactx,
                             sep ? sep + 1 : info3->base.account_name.string);
         if (is_master) {
-            /* Well know RID of domain controllers group */
+            /* Well known RID of domain controllers group */
             info3->base.rid = 516;
+            info3->base.acct_flags |= ACB_SVRTRUST;
         } else {
-            /* Well know RID of domain computers group */
+            /* Well known RID of domain computers group */
             info3->base.rid = 515;
+            info3->base.acct_flags |= ACB_WSTRUST;
         }
     } else {
         ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
@@ -799,9 +806,13 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
     /* always zero out, not used for Krb, only NTLM */
     memset(&info3->base.LMSessKey, '\0', sizeof(info3->base.LMSessKey));
 
-    /* TODO: fill based on objectclass, user vs computer, etc... */
-    info3->base.acct_flags = ACB_NORMAL; /* samr_AcctFlags */
+    /* If account type was not set before, default to ACB_NORMAL */
+    if (!(info3->base.acct_flags & ~ACB_USE_AES_KEYS)) {
+        info3->base.acct_flags |= ACB_NORMAL; /* samr_AcctFlags */
+    }
 
+    /* Clear ACB_USE_AES_KEYS as it is not used by Windows */
+    info3->base.acct_flags &= ~ACB_USE_AES_KEYS;
     info3->base.sub_auth_status = 0;
     info3->base.last_successful_logon = 0;
     info3->base.last_failed_logon = 0;
-- 
2.33.1


From dae8997fc6ffd2ee7af35209c2876586408a2cde Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Sat, 30 Oct 2021 10:49:37 +0300
Subject: [PATCH 22/22] SMB: switch IPA domain controller role

As a part of CVE-2020-25717 mitigations, Samba now assumes 'CLASSIC
PRIMARY DOMAIN CONTROLLER' server role does not support Kerberos
operations.  This is the role that IPA domain controller was using for
its hybrid NT4/AD-like operation.

Instead, 'IPA PRIMARY DOMAIN CONTROLLER' server role was introduced in
Samba. Switch to this role for new installations and during the upgrade
of servers running ADTRUST role.

Fixes: https://pagure.io/freeipa/issue/9031

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
---
 install/share/smb.conf.registry.template |  2 +-
 ipaserver/install/adtrustinstance.py     | 14 +++++++++++++-
 ipaserver/install/server/upgrade.py      | 15 +++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/install/share/smb.conf.registry.template b/install/share/smb.conf.registry.template
index c55a0c307..1d1d12161 100644
--- a/install/share/smb.conf.registry.template
+++ b/install/share/smb.conf.registry.template
@@ -5,7 +5,7 @@ realm = $REALM
 kerberos method = dedicated keytab
 dedicated keytab file = /etc/samba/samba.keytab
 create krb5 conf = no
-server role = CLASSIC PRIMARY DOMAIN CONTROLLER
+server role = $SERVER_ROLE
 security = user
 domain master = yes
 domain logons = yes
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 776e7e587..ba794f5f1 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -146,6 +146,8 @@ class ADTRUSTInstance(service.Service):
     OBJC_GROUP = "ipaNTGroupAttrs"
     OBJC_DOMAIN = "ipaNTDomainAttrs"
     FALLBACK_GROUP_NAME = u'Default SMB Group'
+    SERVER_ROLE_OLD = "CLASSIC PRIMARY DOMAIN CONTROLLER"
+    SERVER_ROLE_NEW = "IPA PRIMARY DOMAIN CONTROLLER"
 
     def __init__(self, fstore=None, fulltrust=True):
         self.netbios_name = None
@@ -555,7 +557,16 @@ class ADTRUSTInstance(service.Service):
         with tempfile.NamedTemporaryFile(mode='w') as tmp_conf:
             tmp_conf.write(conf)
             tmp_conf.flush()
-            ipautil.run([paths.NET, "conf", "import", tmp_conf.name])
+            try:
+                ipautil.run([paths.NET, "conf", "import", tmp_conf.name])
+            except ipautil.CalledProcessError as e:
+                if e.returncode == 255:
+                    # We have old Samba that doesn't support IPA DC server role
+                    # re-try again with the older variant, upgrade code will
+                    # take care to change the role later when Samba is upgraded
+                    # as well.
+                    self.sub_dict['SERVER_ROLE'] = self.SERVER_ROLE_OLD
+                    self.__write_smb_registry()
 
     def __map_Guests_to_nobody(self):
         map_Guests_to_nobody()
@@ -757,6 +768,7 @@ class ADTRUSTInstance(service.Service):
             LDAPI_SOCKET=self.ldapi_socket,
             FQDN=self.fqdn,
             SAMBA_DIR=paths.SAMBA_DIR,
+            SERVER_ROLE=self.SERVER_ROLE_NEW,
         )
 
     def setup(self, fqdn, realm_name, netbios_name,
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 75bf26b8e..f8a32d468 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -358,6 +358,21 @@ def upgrade_adtrust_config():
         else:
             logger.warning("Error updating Samba registry: %s", e)
 
+    logger.info("[Change 'server role' from "
+                "'CLASSIC PRIMARY DOMAIN CONTROLLER' "
+                "to 'IPA PRIMARY DOMAIN CONTROLLER' in Samba configuration]")
+
+    args = [paths.NET, "conf", "setparm", "global",
+            "server role", "IPA PRIMARY DOMAIN CONTROLLER"]
+
+    try:
+        ipautil.run(args)
+    except ipautil.CalledProcessError as e:
+        # Only report an error if return code is not 255
+        # which indicates that the new server role is not supported
+        # and we don't need to do anything
+        if e.returncode != 255:
+            logger.warning("Error updating Samba registry: %s", e)
 
 def ca_configure_profiles_acl(ca):
     logger.info('[Authorizing RA Agent to modify profiles]')
-- 
2.33.1