diff --git a/README.debrand b/README.debrand deleted file mode 100644 index 01c46d2..0000000 --- a/README.debrand +++ /dev/null @@ -1,2 +0,0 @@ -Warning: This package was configured for automatic debranding, but the changes -failed to apply. diff --git a/SOURCES/freeipa-4.9.6-bf-2.patch b/SOURCES/freeipa-4.9.6-bf-2.patch new file mode 100644 index 0000000..abfc547 --- /dev/null +++ b/SOURCES/freeipa-4.9.6-bf-2.patch @@ -0,0 +1,222 @@ +From fe59e6a0b06926a3d71c6b6f361714d1422d5b0f Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +Date: Thu, 11 Nov 2021 09:58:09 +0200 +Subject: [PATCH 1/2] ipa-kdb: honor SID from the host or service entry + +If the SID was explicitly set for the host or service entry, honor it +when issuing PAC. For normal services and hosts we don't allocate +individual SIDs but for cifs/... principals on domain members we do as +they need to login to Samba domain controller. + +Related: https://pagure.io/freeipa/issue/9031 + +Signed-off-by: Alexander Bokovoy +--- + daemons/ipa-kdb/ipa_kdb_mspac.c | 46 ++++++++++++++++++++------------- + 1 file changed, 28 insertions(+), 18 deletions(-) + +diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c +index 0e0ee3616..6f272f9fe 100644 +--- a/daemons/ipa-kdb/ipa_kdb_mspac.c ++++ b/daemons/ipa-kdb/ipa_kdb_mspac.c +@@ -653,6 +653,28 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, + * clear it after detecting the changes */ + info3->base.acct_flags = ACB_USE_AES_KEYS; + ++ ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, ++ "ipaNTSecurityIdentifier", &strres); ++ if (ret) { ++ /* SID is mandatory for all but host/services */ ++ if (!(is_host || is_service)) { ++ return ret; ++ } ++ info3->base.rid = 0; ++ } else { ++ ret = ipadb_string_to_sid(strres, &sid); ++ free(strres); ++ if (ret) { ++ return ret; ++ } ++ ret = sid_split_rid(&sid, &info3->base.rid); ++ if (ret) { ++ return ret; ++ } ++ } ++ ++ /* If SID was present prefer using it even for hosts and services ++ * but we still need to set the account flags correctly */ + if ((is_host || is_service)) { + /* it is either host or service, so get the hostname first */ + char *sep = strchr(info3->base.account_name.string, '/'); +@@ -661,29 +683,17 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, + sep ? sep + 1 : info3->base.account_name.string); + if (is_master) { + /* Well known RID of domain controllers group */ +- info3->base.rid = 516; ++ if (info3->base.rid == 0) { ++ info3->base.rid = 516; ++ } + info3->base.acct_flags |= ACB_SVRTRUST; + } else { + /* Well known RID of domain computers group */ +- info3->base.rid = 515; ++ if (info3->base.rid == 0) { ++ info3->base.rid = 515; ++ } + info3->base.acct_flags |= ACB_WSTRUST; + } +- } else { +- ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, +- "ipaNTSecurityIdentifier", &strres); +- if (ret) { +- /* SID is mandatory */ +- return ret; +- } +- ret = ipadb_string_to_sid(strres, &sid); +- free(strres); +- if (ret) { +- return ret; +- } +- ret = sid_split_rid(&sid, &info3->base.rid); +- if (ret) { +- return ret; +- } + } + + ret = ipadb_ldap_deref_results(ipactx->lcontext, lentry, &deref_results); +-- +2.33.1 + + +From 21af43550aa0a31e1ec5240578bd64fcbdd4ee24 Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +Date: Thu, 11 Nov 2021 10:16:47 +0200 +Subject: [PATCH 2/2] ipa-kdb: validate domain SID in incoming PAC for trusted + domains for S4U + +Previously, ipadb_check_logon_info() was called only for cross-realm +case. Now we call it for both in-realm and cross-realm cases. In case of +the S4U2Proxy, we would be passed a PAC of the original caller which +might be a principal from the trusted realm. We cannot validate that PAC +against our local client DB entry because this is the proxy entry which +is guaranteed to have different SID. + +In such case, validate the SID of the domain in PAC against our realm +and any trusted doman but skip an additional check of the DB entry in +the S4U2Proxy case. + +Related: https://pagure.io/freeipa/issue/9031 + +Signed-off-by: Alexander Bokovoy +--- + daemons/ipa-kdb/ipa_kdb_mspac.c | 54 ++++++++++++++++++++++++++------- + 1 file changed, 43 insertions(+), 11 deletions(-) + +diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c +index 6f272f9fe..6f7d1ac15 100644 +--- a/daemons/ipa-kdb/ipa_kdb_mspac.c ++++ b/daemons/ipa-kdb/ipa_kdb_mspac.c +@@ -1637,11 +1637,13 @@ 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, ++ krb5_boolean is_s4u, + struct PAC_LOGON_INFO_CTR *info) + { + krb5_error_code kerr = 0; + struct ipadb_context *ipactx; + bool result; ++ bool is_from_trusted_domain = false; + krb5_db_entry *client_actual = NULL; + struct ipadb_e_data *ied = NULL; + int flags = 0; +@@ -1671,14 +1673,36 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + 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 : "", +- sid ? sid : ""); +- return KRB5KDC_ERR_POLICY; ++ /* In S4U case we might be dealing with the PAC issued by the trusted domain */ ++ if (is_s4u && (ipactx->mspac->trusts != NULL)) { ++ /* Iterate through list of trusts and check if this SID belongs to ++ * one of the domains we trust */ ++ for(int i = 0 ; i < ipactx->mspac->num_trusts ; i++) { ++ result = dom_sid_check(&ipactx->mspac->trusts[i].domsid, ++ info->info->info3.base.domain_sid, true); ++ if (result) { ++ is_from_trusted_domain = true; ++ break; ++ } ++ } ++ } ++ ++ 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 or any trusted domain SID: " ++ "local [%s], PAC [%s]", ++ dom ? dom : "", ++ sid ? sid : ""); ++ return KRB5KDC_ERR_POLICY; ++ } ++ } ++ ++ if (is_s4u && is_from_trusted_domain) { ++ /* If the PAC belongs to a user from the trusted domain, we cannot compare SIDs */ ++ return 0; + } + + kerr = ipadb_get_principal(context, client_princ, flags, &client_actual); +@@ -1703,6 +1727,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + goto done; + } + ++ + kerr = ipadb_get_sid_from_pac(memctx, info->info, &client_sid); + if (kerr) { + goto done; +@@ -1956,6 +1981,7 @@ 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_boolean is_s4u, + krb5_data *pac_blob, + struct dom_sid *requester_sid) + { +@@ -1999,8 +2025,11 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context, + + 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); ++ * but we don't need to process further. In S4U2Proxy case when the client ++ * is ours but operates on behalf of the cross-realm principal, we will ++ * search through the trusted domains but otherwise skip the exact SID check ++ * as we are not responsible for the principal from the trusted domain */ ++ kerr = check_logon_info_consistent(context, tmpctx, client_princ, is_s4u, &info); + goto done; + } + +@@ -2251,7 +2280,10 @@ static krb5_error_code ipadb_verify_pac(krb5_context context, + #endif + + kerr = ipadb_check_logon_info(context, +- client_princ, is_cross_realm, &pac_blob, ++ client_princ, ++ is_cross_realm, ++ (flags & KRB5_KDB_FLAGS_S4U), ++ &pac_blob, + requester_sid); + if (kerr != 0) { + goto done; +-- +2.33.1 + diff --git a/SOURCES/freeipa-4.9.6-bf-3.patch b/SOURCES/freeipa-4.9.6-bf-3.patch new file mode 100644 index 0000000..48bb8e0 --- /dev/null +++ b/SOURCES/freeipa-4.9.6-bf-3.patch @@ -0,0 +1,122 @@ +From 7d93bda31ce0b4e0e22c6e464c9138800dcf8b1c Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +Date: Fri, 26 Nov 2021 11:13:51 +0200 +Subject: [PATCH] ipa-kdb: fix requester SID check according to MS-KILE and + MS-SFU updates + +New versions of MS-KILE and MS-SFU after Windows Server November 2021 +security updates add PAC_REQUESTER_SID buffer check behavior: + + - PAC_REQUESTER_SID should only be added for TGT requests + + - if PAC_REQUESTER_SID is present, KDC must verify that the cname on + the ticket resolves to the account with the same SID as the + PAC_REQUESTER_SID. If it doesn't KDC must respond with + KDC_ERR_TKT_REVOKED + +Change requester SID check to skip exact check for non-local +PAC_REQUESTER_SID but harden to ensure it comes from the trusted domains +we know about. + +If requester SID is the same as in PAC, we already do cname vs PAC SID +verification. + +With these changes FreeIPA works against Windows Server 2019 with +November 2021 security fixes in cross-realm S4U2Self operations. + +Fixes: https://pagure.io/freeipa/issue/9031 + +Signed-off-by: Alexander Bokovoy +Reviewed-By: Rob Crittenden +--- + daemons/ipa-kdb/ipa_kdb_mspac.c | 47 ++++++++++++++++++++++++--------- + 1 file changed, 34 insertions(+), 13 deletions(-) + +diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c +index 538cfbba9..1b972c167 100644 +--- a/daemons/ipa-kdb/ipa_kdb_mspac.c ++++ b/daemons/ipa-kdb/ipa_kdb_mspac.c +@@ -1697,7 +1697,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + "local [%s], PAC [%s]", + dom ? dom : "", + sid ? sid : ""); +- return KRB5KDC_ERR_POLICY; ++ return KRB5KDC_ERR_TGT_REVOKED; + } + } + +@@ -1709,7 +1709,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + 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; ++ return KRB5KDC_ERR_TGT_REVOKED; + } + + ied = (struct ipadb_e_data *)client_actual->e_data; +@@ -1743,7 +1743,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + "local [%s] vs PAC [%s]", + local_sid ? local_sid : "", + pac_sid ? pac_sid : ""); +- kerr = KRB5KDC_ERR_POLICY; ++ kerr = KRB5KDC_ERR_TGT_REVOKED; + goto done; + } + +@@ -2005,22 +2005,43 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context, + /* Check that requester SID is the same as in the PAC entry */ + if (requester_sid != NULL) { + struct dom_sid client_sid; ++ bool is_from_trusted_domain = false; + 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 : "", +- req_sid ? req_sid : ""); +- kerr = KRB5KDC_ERR_POLICY; +- goto done; ++ struct ipadb_context *ipactx = ipadb_get_context(context); ++ if (!ipactx || !ipactx->mspac) { ++ return KRB5_KDB_DBNOTINITED; ++ } ++ /* In S4U case we might be dealing with the PAC issued by the trusted domain */ ++ if (is_s4u && (ipactx->mspac->trusts != NULL)) { ++ /* Iterate through list of trusts and check if this SID belongs to ++ * one of the domains we trust */ ++ for(int i = 0 ; i < ipactx->mspac->num_trusts ; i++) { ++ result = dom_sid_check(&ipactx->mspac->trusts[i].domsid, ++ requester_sid, false); ++ if (result) { ++ is_from_trusted_domain = true; ++ break; ++ } ++ } ++ } ++ ++ if (!is_from_trusted_domain) { ++ /* 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 : "", ++ req_sid ? req_sid : ""); ++ kerr = KRB5KDC_ERR_TGT_REVOKED; ++ goto done; ++ } + } + } + +-- +2.31.1 + diff --git a/SOURCES/freeipa-4.9.6-bf.patch b/SOURCES/freeipa-4.9.6-bf.patch new file mode 100644 index 0000000..b6e3174 --- /dev/null +++ b/SOURCES/freeipa-4.9.6-bf.patch @@ -0,0 +1,3979 @@ +From 2ab6c0fc96adb90880f9bc23661ed3997b0a7023 Mon Sep 17 00:00:00 2001 +From: Florence Blanc-Renaud +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 +Reviewed-By: Alexander Bokovoy +Reviewed-By: Rob Crittenden +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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"/> + ++ ++ ++ + + + +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 . ++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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + .../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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-By: Christian Heimes +Reviewed-By: Rob Crittenden +Reviewed-By: Alexander Bokovoy +--- + 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 +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 +Reviewed-by: Andreas Schneider +Reviewed-by: Robert Crittenden +--- + 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 +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 +Reviewed-by: Andreas Schneider +Reviewed-by: Robert Crittenden +--- + 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 : "", ++ sid ? sid : ""); ++ 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 : "", ++ pac_sid ? pac_sid : ""); ++ 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 +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 +Reviewed-by: Rob Crittenden +--- + 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 ++#include + + 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 +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 +Signed-off-by: Alexander Bokovoy +Signed-off-by: Andreas Schneider +Reviewed-by: Rob Crittenden +--- + 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 +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 +Reviewed-by: Rob Crittenden +--- + 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 + #include ]]) ++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 ++ #include ]]) + + CFLAGS="$bck_cflags" + +-- +2.33.1 + + +From 3e88bc63c919003a46fbf8d018d724bf00928c51 Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +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 +Reviewed-by: Rob Crittenden +--- + 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 : "", ++ dom ? dom : "", + sid ? sid : ""); + 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 : "", ++ req_sid ? req_sid : ""); ++ 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 + #include ]]) + ++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 ++ #include ]]) + CFLAGS="$bck_cflags" + + LIBPDB_NAME="" +-- +2.33.1 + + +From 8a088fd0824d73e43b8ca56c04c5899f77619db9 Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +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 +Signed-off-by: Andrew Bartlett +--- + 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 + #include ]]) ++ ++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 ++ #include ]]) + CFLAGS="$bck_cflags" + + LIBPDB_NAME="" +-- +2.33.1 + + +From 5e0fe9bff0ad2786f37a718c8b25b85836c8ff82 Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +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 +--- + 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 +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 +Reviewed-by: Rob Crittenden +--- + 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 + diff --git a/SPECS/ipa.spec b/SPECS/ipa.spec index d45b5f0..5448366 100644 --- a/SPECS/ipa.spec +++ b/SPECS/ipa.spec @@ -68,8 +68,8 @@ %global krb5_kdb_version 8.0 # 0.7.16: https://github.com/drkjam/netaddr/issues/71 %global python_netaddr_version 0.7.19 -# Require 4.7.0 which brings Python 3 bindings -%global samba_version 4.12.3-12 +# Require 4.14.5-6 which brings CVE-2020-25717 fixes in RHEL 8.5.z +%global samba_version 4.14.5-6 %global selinux_policy_version 3.14.3-52 %global slapi_nis_version 0.56.4 %global python_ldap_version 3.1.0-1 @@ -92,9 +92,9 @@ %global krb5_version 1.18.2-29 # 0.7.16: https://github.com/drkjam/netaddr/issues/71 %global python_netaddr_version 0.7.16 -# Require 4.7.0 which brings Python 3 bindings -# Require 4.12 which has DsRGetForestTrustInformation access rights fixes -%global samba_version 2:4.12.10 + +# Require 4.14.6 which brings CVE-2020-25717 fixes +%global samba_version 2:4.14.6 # 3.14.5-45 or later includes a number of interfaces fixes for IPA interface %global selinux_policy_version 3.14.5-45 @@ -191,7 +191,7 @@ Name: %{package_name} Version: %{IPA_VERSION} -Release: 6%{?rc_version:.%rc_version}%{?dist} +Release: 10%{?rc_version:.%rc_version}%{?dist} Summary: The Identity, Policy and Audit system License: GPLv3+ @@ -228,6 +228,11 @@ Patch1001: 1001-Change-branding-to-IPA-and-Identity-Management.patch %endif %endif # RHEL spec file only: END +# SID hardening patches +Patch1100: freeipa-4.9.6-bf.patch +Patch1101: freeipa-4.9.6-bf-2.patch +Patch1102: freeipa-4.9.6-bf-3.patch + # For the timestamp trick in patch application BuildRequires: diffstat @@ -471,6 +476,8 @@ Requires: gssproxy >= 0.7.0-2 Requires: sssd-dbus >= %{sssd_version} Requires: libpwquality Requires: cracklib-dicts +# NDR libraries are internal in Samba and change with version without changing SONAME +Requires: samba-client-libs >= %{samba_version} Provides: %{alt_name}-server = %{version} Conflicts: %{alt_name}-server @@ -1377,6 +1384,7 @@ fi %attr(755,root,root) %{_libexecdir}/ipa/custodia/ipa-custodia-ra-agent %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.config-enable-sid %attr(0755,root,root) %{_libexecdir}/ipa/oddjob/org.freeipa.server.trust-enable-agent %config(noreplace) %{_sysconfdir}/dbus-1/system.d/org.freeipa.server.conf %config(noreplace) %{_sysconfdir}/oddjobd.conf.d/ipa-server.conf @@ -1709,6 +1717,22 @@ fi %changelog +* Thu Nov 30 2021 Rafael Jeffman - 4.9.6-10 +- Bump realease version due to build issue. + Related: RHBZ#2021489 + +* Thu Nov 30 2021 Rafael Jeffman - 4.9.6-9 +- Hardening for CVE-2020-25717, part 3 + Related: RHBZ#2021489 + +* Thu Nov 18 2021 Alexander Bokovoy - 4.9.6-8 +- Hardening for CVE-2020-25717, part 2 +- Related: RHBZ#2021171 + +* Sun Nov 07 2021 Alexander Bokovoy - 4.9.6-7 +- Hardening for CVE-2020-25717 +- Related: RHBZ#2021171 + * Fri Sep 17 2021 Thomas Woerner - 4.9.6-6 - Don't store entries with a usercertificate in the LDAP cache Resolves: RHBZ#1999893