Harden PAC processing #6076
Closed
Harden PAC processing #6076
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This PR intentionally lacks changes changes for |
|
I'm going to look into failures tomorrow, one of the issues is a missing domain SID on the service principal, I know where to check... |
|
One more note: we need to add processing of the 'server role' with older value for the configuration with old samba. Looks like the value of |
this part is probably happening shortly after SID was associated with the domain but before KDB driver noticed this change. We definitely need to reinit mspac object cache if this is detected. I am going to add it here. |
|
Out of other Azure CI failures, one looks like a flaky 'user-add returns an object with ipaNTUserAttrs or not' type. @flo-renaud, does it sound familiar? |
abbra
force-pushed
the
harden-pac-processing
branch
from
01751ee
to
18356c4
Compare
I found the root cause, working on a fix. |
|
With the latest fixes, I don't see anymore Kerberos-related issues. New PAC buffers will be visible once Samba updates land in Fedora. |
abbra
force-pushed
the
harden-pac-processing
branch
from
18356c4
to
f1a9480
Compare
|
FYI, GATING upgrade_1_to_1 encounters a crash in 389-ds contentsync plugin. I opened an issue for 389-ds, as it is unrelated to the changes in this PR: 389ds/389-ds-base#4998 |
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>
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>
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>
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>
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>
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>
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>
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>
As a part of CVE-2020-25717 mitigations, Samba now assumes 'CLASSIC PRIMARY DOMAIN CONTROLLER' server role does not support Kerberos operations. This is the role that IPA domain controller was using for its hybrid NT4/AD-like operation. Instead, 'IPA PRIMARY DOMAIN CONTROLLER' server role was introduced in Samba. Switch to this role for new installations and during the upgrade of servers running ADTRUST role. Fixes: https://pagure.io/freeipa/issue/9031 Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com> Reviewed-by: Rob Crittenden <rcritten@redhat.com>
abbra
force-pushed
the
harden-pac-processing
branch
from
f1a9480
to
cae0633
Compare
rcritten
added
ack
|
master:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement suggestions outlined in https://www.samba.org/samba/security/CVE-2020-25721.html
Implement PAC_UPN_DNS_INFO_EX, PAC_ATTRIBUTES_INFO, PAC_REQUESTER_SID, and other hardening improvements as suggested by Samba Team and Microsoft.
Additional information:
Microsoft: https://support.microsoft.com/en-us/topic/kb5008380-authentication-updates-cve-2021-42287-9dafac11-e0d0-4cb8-959a-143bd0201041
Samba Team: https://www.samba.org/samba/latest_news.html#4.15.2