Commit Graph

806 Commits

Author SHA1 Message Date
Francisco Trivino
3de5e4e1f7 ipa_cldap: fix memory leak
ipa_cldap_encode_netlogon() allocates memory to store binary data as part of
berval (bv_val) when processing a CLDAP packet request from a worker. The
data is used by ipa_cldap_respond() but bv_val is not freed later on.

This commit is adding the corresponding free() after ipa_cldap_respond()
is completed.

Discovered by LeakSanitizer

Fixes: https://pagure.io/freeipa/issue/9110
Signed-off-by: Francisco Trivino <ftrivino@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Thierry Bordaz <tbordaz@redhat.com>
2022-02-11 13:31:34 +02:00
Julien Rische
35e94bee0e ipa-kdb: do not remove keys for hardened auth-enabled users
Since 5d51ae5, principal keys were dropped in case user auth indicator
was not including password. Thereafter, the key removal behavior was
removed by 15ff9c8 in the context of the kdcpolicy plugin introduction.
Support for hardened pre-auth methods (FAST and SPAKE) was added in
d057040, and the removal of principal keys was restored afterwards by
f0d12b7, but not taking the new hardened auth indicator into account.

Fixes: https://pagure.io/freeipa/issue/9065
Related to: https://pagure.io/freeipa/issue/8001

Signed-off-by: Julien Rische <jrische@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Francisco Trivino <ftrivino@redhat.com>
2022-02-02 15:42:45 +01:00
Alexander Bokovoy
2dc752018c ipa-kdb: refactor KDB driver to prepare for KDB version 9
MIT Kerberos 1.20 changes DAL interface around PAC record issuance:
sign_authdata callback is removed and replaced with issue_pac one.
The signatures are different and logic changed as well.

Prepare for KDB version 9 by moving PAC implementation into separate
source files. ipa_kdb_mspac.c is left with most of the common code.

FreeIPA supports sign_authdata callback since KDB version 6, move current
implementation to ipa_kdb_mspac_v6.c.

KDB version 8 actually changed sign_authdata interface and we accounted
to that in ipa_kdb.c with a stub that re-uses v6 version. Keep it as it
is right now.

Finally, add KDB version 9 stub files. Compiling against MIT Kerberos
1.20 does not work yet, thus explicit #error message in ipa_kdb.c. This
will be worked on later.

Related: https://pagure.io/freeipa/issue/9083

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2022-01-24 17:38:24 -05:00
Alexander Bokovoy
637412acfd 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 <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-29 21:00:42 +02:00
Alexander Bokovoy
bda59110f1 ipa-kdb: issue PAC_REQUESTER_SID only for TGTs
MS-KILE 3.3.5.6.4.8 in revision after Windows Server November 2021
security fixes added the following requirement:

- PAC_REQUESTER_SID is only added in TGT case (including referrals and
  tickets to RODCs)

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-29 21:00:42 +02:00
Alexander Bokovoy
9043b8d534 Fix use of comparison functions to avoid GCC bug 95189
Due to a bug in GCC 9 and GCC 10 optimizing code, all C library
comparison functions should be used with explicit result comparison in
the code to avoid problems described in

http://r6.ca/blog/20200929T023701Z.html

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189

The code below is affected:

```
    if (strcmp(a, b) || !strcmp(c, d)) ...
```

while the code below is not affected:

```
    if (strcmp(a, b) != 0 || strcmp(c, d)) == 0
```

for all C library cmp functions and related:

 - strcmp(), strncmp()
 - strcasecmp(), strncasecmp()
 - stricmp(), strnicmp()
 - memcmp()

This PR idea is based on the pull request by 'Nicolas Williams <nico@twosigma.com>'
to Heimdal Kerberos: https://github.com/heimdal/heimdal/pull/855

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-23 10:31:34 +01:00
Alexander Bokovoy
0c08faf3c1 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 <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-11 16:13:56 -05:00
Alexander Bokovoy
4062e7b963 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 <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-11 16:13:56 -05:00
Alexander Bokovoy
3042a1d544 ipa-kdb: Use proper account flags for Kerberos principal in PAC
As part of CVE-2020-25717 mitigations, Samba expects correct user
account flags in the PAC. This means for services and host principals we
should be using ACB_WSTRUST or ACB_SVRTRUST depending on whether they
run on IPA clients ("workstation" or "domain member") or IPA servers
("domain controller").

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
0022bd70d9 ipa-kdb: add PAC_ATTRIBUTES_INFO PAC buffer support
PAC_ATTRIBUTES_INFO PAC buffer allows both client and KDC to tell
whether a PAC structure was requested by the client or it was provided
by the KDC implicitly. Kerberos service then can continue processing or
deny access in case client explicitly requested to operate without PAC.

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
9a0bcbbe82 ipa-kdb: add support for PAC_REQUESTER_SID buffer
CVE-2020-25721 mitigation: KDC must provide the new PAC_REQUESTER_SID
buffer with ObjectSID value associated with the requester's principal.

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

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
23336160f2 ipa-kdb: add support for PAC_UPN_DNS_INFO_EX
CVE-2020-25721 mitigation: KDC must provide the new HAS_SAM_NAME_AND_SID
buffer with sAMAccountName and ObjectSID values associated with the
principal.

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

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
6828273b56 ipa-kdb: S4U2Proxy target should use a service name without realm
According to new Samba Kerberos tests and [MS-SFU] 3.2.5.2.4
'KDC Replies with Service Ticket', the target should not include the
realm.

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

Pair-programmed-with: Andreas Schneider <asn@redhat.com>
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Signed-off-by: Andreas Schneider <asn@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
443a99060c ipa-kdb: use entry DN to compare aliased entries in S4U operations
When working with aliased entries, we need a reliable way to detect
whether two principals reference the same database entry. This is
important in S4U checks.

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

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

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
637653a533 ipa-kdb: enforce SID checks when generating PAC
Check that a domain SID and a user SID in the PAC passed to us are what
they should be for the local realm's principal.

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Robert Crittenden <rcritten@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Alexander Bokovoy
6cfb9b7193 ipa-kdb: store SID in the principal entry
If the principal entry in LDAP has SID associated with it, store it to
be able to quickly assess the SID when processing PAC.

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

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Robert Crittenden <rcritten@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-11-10 15:00:27 -05:00
Pavel Březina
7a9301c5f9 kdb: fix typo in ipa_kdcpolicy_check_as
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-09-22 09:36:39 +02:00
Sumit Bose
d743219a9a extdom: return LDAP_NO_SUCH_OBJECT if domains differ
If a client sends a request to lookup an object from a given trusted
domain by UID or GID and an object with matching ID is only found in a
different domain the extdom should return LDAP_NO_SUCH_OBJECT to
indicate to the client that the requested ID does not exists in the
given domain.

Resolves: https://pagure.io/freeipa/issue/8965
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-09-01 13:45:25 -04:00
Alexander Bokovoy
b4b2c10e23 ds: Support renaming of a replication plugin in 389-ds
IPA topology plugin depends on the replication plugin but
389-ds cannot handle older alias querying in the plugin
configuration with 'nsslapd-plugin-depends-on-named: ..' attribute

See https://github.com/389ds/389-ds-base/issues/4786 for details

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
2021-06-01 17:09:28 +03:00
Alexander Bokovoy
1cc2671182 ipa-otpd: handle LDAP timeout in a better way
When LDAP server disconnects ipa-otpd client connection due to a
timeout, ipa-otpd instance would stop and report an issue. This confuses
systemd service reporting, so for these situations we better to shut
down gracefully.

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Simo Sorce <ssorce@redhat.com>
2021-04-23 11:13:36 +03:00
Florence Blanc-Renaud
73ced07e2e dnssec: concurrency issue when disabling old replica key
When dnssec role is removed from the local node, the uninstaller
creates a new replica key and marks the older replica keys as disabled
(both in the local HSM and in LDAP).
If ipa-ods-exporter runs in the middle of this operation, the old replica
key may be disabled in the local HSM but not yet in LDAP and
ipa-ods-exporter believes that it is a new replica key that needs to be
imported from LDAP to local hsm. The op fails as there is already the key
in the local HSM.

The error can be ignored, ipa-ods-exporter simply needs to log a warning.

Fixes: https://pagure.io/freeipa/issue/8654
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2021-03-09 16:52:38 +01:00
Florence Blanc-Renaud
3b1a56f588 dnssec: fix ipa-ods-exporter crash when master key missing
When a master key is missing from the local HSM, ipa-ods-exporter crashes.
This can happen when the DNSSEC master role is moved from one node to
another with the following scenario:
- install server1 with dns + dnssec
- install server2 without dns
- disable dnssec from server1
- install dns + dnssec on server2

With the above scenario, server2 never had the opportunity to get
the master key (this happens only when the replica is already
configured as DNS server and has put its public replica key in LDAP +
the current DNSSEC master wraps its master key with the replica key).

ipa-ods-exporter can only log an error instead of crashing.

Related: https://pagure.io/freeipa/issue/8654
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2021-03-09 16:52:38 +01:00
Alexander Bokovoy
3fc75b15fa ipa-kdb: mark test functions as static
No need to define missing prototypes to single use test functions.

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-03-01 10:44:25 -05:00
Alexander Bokovoy
3037de2820 ipa-kdb: reformat ipa_kdb_certauth
Add prototype to the exported function

Replace few tabs by spaces and mark static code as static.

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-03-01 10:44:25 -05:00
Alexander Bokovoy
f996c21edb ipa-kdb: add missing prototypes
On Fedora 33 GCC defaults to -Wmissing-prototypes and emits warnings
about function prototypes missing. If -Werror is specified, this breaks
compilation.

We also default to -Werror=implicit-function-declaration

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-03-01 10:44:25 -05:00
Alexander Bokovoy
1d2a14a2d2 ipa-kdb: fix compiler warnings
There are few fields in KDB structures that have 'conflicting' types but
need to be compared. They come from MIT Kerberos and we have no choice
here.

In the same way, SID structures have own requirements.

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-03-01 10:44:25 -05:00
Alexander Bokovoy
47d49aa7b7 ipa-kdb: do not use OpenLDAP functions with NULL LDAP context
Calling to ipadb_get_connection() will remove LDAP context if any error
happens. This means upper layers must always verify that LDAP context
exists after such calls.

ipadb_get_user_auth() may re-read global configuration and that may fail
and cause IPA context to have NULL LDAP context.

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-03-01 10:44:25 -05:00
Alexander Bokovoy
578e4df6b1 ipa-sam: return NetBIOS domain name instead of DNS one
[MS-NRPC] section 2.2.1.4.11 requires that the structure
NETLOGON_VALIDATION_SAM_INFO has the same values as defined in the
KERB_VALIDATION_INFO structure from [MS-PAC] section 2.5.

Samba's netr_SamBaseInfo.domain_name corresponds to
KERB_VALIDATION_INFO.LogonDomainName and must be a NetBIOS name of the
domain, not a DNS one.

Failure to provide NetBIOS name here actually breaks
netr_LogonSamLogonEx call issued by IPA-enrolled Samba domain member
which is confused by the returned value:

[2021/01/30 16:36:36.636010,  0, pid=1633, effective(0, 0), real(0, 0), class=winbind]
  ../../source3/winbindd/winbindd_util.c: 175(add_trusted_domain)
  add_trusted_domain: SID [S-1-5-21-3342930694-1632731913-1318603033]
  already used by domain [INTERNAL], expected [internal.example.test]
[2021/01/30 16:36:36.636050, 10, pid=1633, effective(0, 0), real(0, 0), class=winbind]
  ../../source3/winbindd/winbindd_util.c:362(add_trusted_domain_from_auth)
  add_trusted_domain_from_auth: Adding domain [internal.example.test]
  with sid [S-1-5-21-3342930694-1632731913-1318603033] failed
[2021/01/30 16:36:36.636060,  0, pid=1633, effective(0, 0), real(0, 0), class=winbind]
  ../../source3/winbindd/winbindd_pam_auth_crap.c:169(winbindd_pam_auth_crap_done)
  winbindd_pam_auth_crap_done: add_trusted_domain_from_auth failed
[2021/01/30 16:36:36.636079, 10, pid=1633, effective(0, 0), real(0, 0), class=winbind]
  ../../source3/winbindd/winbindd.c:814(process_request_done)
  process_request_done: [smbd(1650):PAM_AUTH_CRAP]: NT_STATUS_LOGON_FAILURE

Fixes: https://pagure.io/freeipa/issue/8636
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Sumit Bose <sbose@redhat.com>
2021-02-02 09:41:00 +02:00
JoeDrane
94242563d5 Update ipa_sam.c
fixed typo in debug message on line 4040.

Signed-off-by: JoeDrane <joe@drane.io>
Reviewed-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-22 12:21:33 -05:00
Alexander Bokovoy
08d7d90ab0 ipasam: derive parent domain for subdomains automatically
[MS-ADTS] 6.1.6.7.13 defines 'trustPartner' attribute as containing a
FQDN of the trusted domain. In practice, for a subdomain of a forest, it
would be FQDN of the subdomain itself in the trusted domain entry in the
parent domain. This is reflected as ipaNTTrustPartner attribute in
FreeIPA.

Remove ipaNTTrustPartner from the searches that use NetBIOS name. We
match cn of that entry already.

Use RDN value of the entry to derive DNS domain name in case
ipaNTTrustPartner is missing.

For subdomains, set trust attributes to 0 and trust flags to mark them
as being within the forest. This will trigger winbindd to not ask for
credentials to reach those domain controllers directly.

Fixes: https://pagure.io/freeipa/issue/8576
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-22 12:21:33 -05:00
Alexander Bokovoy
a1e2fe9c32 ipasam: free trusted domain context on failure
The context is hanging off a talloc memory context passed into the
function so it will eventually be freed. It is better, though, to free
it immediately when we exit from the fill_pdb_trusted_domain() function.

Related: https://pagure.io/freeipa/issue/8576
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-22 12:21:33 -05:00
Alexander Bokovoy
75882516c4 ipasam: allow search of users by user principal name (UPN)
lookup_name() in Samba may call PASSDB API to search by a UPN (e.g.
username@suffix). Support this call by detecting '@' in the passed name
and setting up filter to be

  (&(objectClass=ipaNTUserAttrs)(objectClass=krbPrincipalAux)(krbPrincipalName:caseIgnoreIA5Match:=%s))

instead of

  (&(objectClass=ipaNTUserAttrs)(uid=%s))

The result of the search would still contain a proper user entry as we
always have krbPrincipalName in LDAP entries of IPA users. Note that the
match must be case-insensitive because otherwise krbPrincipalName is
matched with exact case in the schema. We use the same matching override
in KDB driver already.

Fixes: https://pagure.io/freeipa/issue/8661
Signed-of-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-22 12:21:33 -05:00
Alexander Bokovoy
e6f8d8bc9b ipasam: implement PASSDB getgrnam call
ipasam already implemented retrieval of groups for MS-SAMR calls.
However, it did not have implementation of a group retrieval for the
path of lookup_name() function in Samba. The lookup_name() is used in
many places in smbd and winbindd.

With this change it will be possible to resolve IPA groups in Windows UI
(Security tab) and console (net localgroup ...). When Global Catalog
service is enabled, it will be possible to search for those groups as
well.

In Active Directory, security groups can be domain, domain local, local
and so on. In IPA, only domain groups exposed through ipasam because
SID generation plugin only supports adding SIDs to POSIX groups and
users. Thus, non-POSIX groups are not going to have SIDs associated and
will not be visible in both UNIX and Windows environments.

Group retrieval in Samba is implemented as a mapping between NT and
POSIX groups. IPA doesn't have explicit mapping tables. Instead, any
POSIX group in IPA that has a SID associated with it is considered a
domain group for Samba.

Finally, additional ACI is required to ensure attributes looked up by
ipasam are always readable by the trust agents.

Fixes: https://pagure.io/freeipa/issue/8660
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-22 12:21:33 -05:00
Alexander Bokovoy
968f8ada65 ipa-kdb: provide correct logon time in MS-PAC from authentication time
When MS-PAC structure is created, we get passed the time of
authentication from KDC. Use this to record logon time in MS-PAC
structure.

Set allow password change time to the last password change. We need to
refer to the actual password policy here in future.

Also use INT64_MAX to represent the resulting value for logoff
and kickoff times according to MS-PAC 2.6.

Fixes: https://pagure.io/freeipa/issue/8659
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-22 12:21:33 -05:00
Peter Keresztes Schmidt
a355646c37 build: Unify compiler warning flags used
Related: https://pagure.io/freeipa/issue/8351
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2021-01-15 14:11:56 +01:00
Rob Crittenden
a129e6f042 ipa_kdb: Fix memory leak
ipadb_get_principal() allocates client_actual. Call
ipadb_free_principal to release it.

Rather than spreading the free() amongst the code introduce
done as a target to match behavior in similar functions.

Discovered by coverity.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2021-01-15 10:01:28 +01:00
Rob Crittenden
6f46fac264 ipa-kdb: Fix logic to prevent NULL pointer dereference
Discovered by coverity

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2021-01-15 10:01:28 +01:00
Rob Crittenden
0096f4b985 ipa-kdb: Change mspac base RID logic from OR to AND
The purpose is to set a default if the RID doesn't match
expectations.

Discovered by coverity

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2021-01-15 10:01:28 +01:00
Rob Crittenden
cbf7bcbed5 Add missing break statement to password quality switch
Discovered by coverity.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2021-01-15 10:01:28 +01:00
Slava Aseev
4c0398433e ipa-kdb: handle dates up to 2106-02-07 06:28:16
krb5 uses the negative part of krb5_timestamp to store time values
after 2038:
https://k5wiki.kerberos.org/wiki/Projects/Timestamps_after_2038
In other words, krb5 uses krb5_timestamp (signed int) with
unsigned arithmetic for expanding the timestamp's upper bound.

This commit:
  - adds some helper functions for working with krb5_timestamp as
    unsigned (actually copied from
    https://github.com/krb5/krb5/blob/master/src/include/k5-int.h)
  - replaces operations with krb5_timestamp's by these new functions

Fixes: https://pagure.io/freeipa/issue/8028
Signed-off-by: Slava Aseev <ptrnine@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
2020-12-18 20:38:40 +02:00
Alexander Bokovoy
35362d3033 ipa-kdb: use predefined filters for a wild-card searches
In case we've got a principal name as '*', we don't need to specify
the principal itself, use pre-defined filter for a wild-card search.

Previously, we had to escape the '*' as specifying it with an explicit
matching rule would have violated RFC 4515 section 3. However, since we
don't really need to specify a different matching rule for a wild-card
search, we can remove this part completely.

Use this change as an opportunity to simplify the code and reduce
number of duplicated filter constants -- if extra filter is NULL, we can
simply pass "" and use _EXTRA filter constants to format the final
filter.

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
2020-12-18 13:32:34 +02:00
Alexander Bokovoy
38a86e3367 systemd: enforce en_US.UTF-8 locale in systemd units
Python code does detection of the system encoding based on the locale
settings. On RHEL 8.4 development images we somehow get LANG=en_US which
defaults to iso8859-1 _inside_ the systemd-started service, even though
the whole environment defaults to LANG=en_US.UTF-8.

When instrumented with ExecStartPre=/usr/bin/locale, the following
output can be seen:

locale[45481]: LANG=en_US
locale[45481]: LC_CTYPE="en_US"
locale[45481]: LC_NUMERIC="en_US"
locale[45481]: LC_TIME="en_US"
locale[45481]: LC_COLLATE="en_US"
locale[45481]: LC_MONETARY="en_US"
locale[45481]: LC_MESSAGES="en_US"
locale[45481]: LC_PAPER="en_US"
locale[45481]: LC_NAME="en_US"
locale[45481]: LC_ADDRESS="en_US"
locale[45481]: LC_TELEPHONE="en_US"
locale[45481]: LC_MEASUREMENT="en_US"
locale[45481]: LC_IDENTIFICATION="en_US"
locale[45481]: LC_ALL=
ipactl[45483]: Unexpected error
ipactl[45483]: SystemEncodingError: System encoding must be UTF-8, 'iso8859-1' is not supported. Set LC_ALL="C.UTF-8", or LC_ALL="" and LC_CTYPE="C.UTF-8".
systemd[1]: ipa.service: Main process exited, code=exited, status=1/FAILURE

Set the environment to explicit LC_ALL=C.UTF-8 to please the Python
code. FreeIPA server side only cares about actual encoding, not the
language itself. We already use LC_ALL=C.UTF-8 in httpd service snippet.

Fixes: https://pagure.io/freeipa/issue/8617
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
2020-12-10 14:38:05 +02:00
Alexander Bokovoy
0da6a57b40 ad trust: accept subordinate domains of the forest trust root
Commit 8b6d1ab854 added support for
subordinate UPN suffixes but missed the case where subordinate UPN is a
subdomain of the forest root domain and not mentioned in the UPN
suffixes list.

Correct this situation by applying the same check to the trusted domain
name as well.

Fixes: https://pagure.io/freeipa/issue/8554
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2020-11-26 12:00:57 +02:00
Alexander Bokovoy
b36f224892 ipa-otpd: fix gcc complaints in Rawhide
In file included from /usr/include/string.h:519,
                 from /usr/include/lber.h:30,
                 from /usr/include/ldap.h:30,
                 from internal.h:27,
                 from main.c:31:
In function 'strncpy',
    inlined from 'main' at main.c:237:5:
/usr/include/bits/string_fortified.h:91:10: warning: 'strncpy' specified bound 255 equals destination size [-Wstringop-truncation]
   91 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: https://pagure.io/freeipa/issue/8585
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-11-17 18:48:24 +02:00
Alexander Bokovoy
d99b7d0b01 ipa-sam: fix gcc complaints on Rawhide
In file included from /usr/include/string.h:519,
                  from /usr/include/lber.h:30,
                  from /usr/include/ldap.h:30,
                  from ipa_sam.c:12:
 In function 'strncpy',
     inlined from 'save_sid_to_secret' at ipa_sam.c:4478:2,
     inlined from 'pdb_init_ipasam' at ipa_sam.c:4985:12:
 /usr/include/bits/string_fortified.h:91:10: warning: 'strncpy' specified bound 255 equals destination size [-Wstringop-truncation]
    91 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: https://pagure.io/freeipa/issue/8585
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-11-17 18:48:24 +02:00
Alexander Bokovoy
fc11c56544 ipa-kdb: fix gcc complaints in kdb tests
We use string_to_sid() from internal Samba libraries, so we have to link
to it properly.

In addition, size_t is (long unsigned int), just cast to (int) in
asprintf.

Fixes: https://pagure.io/freeipa/issue/8585
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-11-17 18:48:24 +02:00
Alexander Bokovoy
f513a55ded ipa-kdb: fix gcc complaints
In file included from /usr/include/string.h:519,
                 from /usr/include/lber.h:30,
                 from /usr/include/ldap.h:30,
                 from ipa_kdb.h:37,
                 from ipa_kdb_mspac.c:26:
In function 'strncpy',
    inlined from 'get_server_netbios_name' at ipa_kdb_mspac.c:2358:5,
    inlined from 'ipadb_reinit_mspac' at ipa_kdb_mspac.c:2813:39:
/usr/include/bits/string_fortified.h:91:10: warning: 'strncpy' specified bound 255 equals destination size [-Wstringop-truncation]
   91 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: https://pagure.io/freeipa/issue/8585
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
2020-11-17 18:48:24 +02:00
Robbie Harwood
0d67180f7d ipa-kdb: implement AS-REQ lifetime jitter
Jitter is always enabled, so there is no additional configuration.

An earlier version of this patch was authored by Becky Shanley.

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

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2020-11-17 14:27:28 +02:00
Rob Crittenden
3ab3578b36 On password reset also set krbLastAdminUnlock to unlock account
This fixes the case where an account is locked on one or more servers
and the password is reset by an administrator. The account would
remain locked on those servers for the duration of the lockout.

This is done by setting krbLastAdminUnlock to the current date and
time. The lockout plugin will see this and unlock the account. Since
the value should be replicated along with the password any server
that has the new password will also be unlocked.

This does incur an additional attribute that must be replicated,
whether it is needed or not, but since lockout is computed
per-server this is the only guaranteed way to be sure that the
account will be unlocked everywhere.

My original thought was to grab password replication events and detect
whether the user was locked out and unlock them. On any given server
you can only know if the user is locked out on that server by
computing it. Doing this would require generalizing the lockout code
so it could be computed on password change. krbLastFailedAuth could
be wiped which would unlock the account on that master (the attribute
is not replicated by default).

So it is complexity vs additional replication. Assuming that admin
reset is relatively rare let's start with that. This doesn't lock
us into this solution for the future.

We could set this attribute on user-driven password changes as
well but the original ask and my thinking are that if you forgot
your password and got locked out, how can you change it yourself?
Upon reflection I guess a user could fat-finger it a bunch of times
against one IPA server then have a revelation and log in against a
different server. So they would still be locked out for the duration
on the first one. I'm not sure the extra replication is worth it for
user-generated password changes or that users would be saavy enough
to try another server for the change.

https://pagure.io/freeipa/issue/8551

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
2020-11-11 10:29:25 +02:00
Alexander Bokovoy
81cbee4e3f ipa-kdb: fix crash in MS-PAC cache init code
When initializing UPN suffixes, we calculate their sizes and didn't use
the right variable to allocate their size. This affects us if there are
more than one UPN suffix available for a trust due to memory corruption
while filling in sizes.

Add unit test for multiple UPN suffixes.

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

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
2020-11-06 16:32:42 -05:00