-
Notifications
You must be signed in to change notification settings - Fork 370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use non-RFC 8009 enctypes to sign KDC checksum in PAC structure #1268
Conversation
@greghudson I have an alternative approach in FreeIPA by implementing I have an example here:
this is caused by the following issue in krb5kdc.log:
This 'unknown client' was supposed to be |
c7ea9b0
to
e8169b7
Compare
Just so I'm clear: this is a KDC signature buffer that Microsoft doesn't need to validate, but the buffer size (larger than 20 bytes) nevertheless causes a rejection? |
From dochelp response I believe that they check the size before parsing the buffer. If buffer is of not expected size, it is considered a failure. Since this buffer's presence is required (table in MS-PAC 2.4, https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-pac/3341cfa2-6ef5-42e0-b7bc-4544884bf399, says: "KDC (privilege server) checksum (section 2.8). PAC structures MUST contain one buffer of this type. Additional KDC checksum buffers MUST be ignored."), a failure to parse is considered a rejection. Overally, buffer types 1, 6, 7, and 10 (0xA) MUST be present when PAC structure is present, according to the same table in MS-PAC 2.4. |
A bit to add, the size check is Windows implementation detail, judging by the dochelp engineer's response in analysis of the TTT trace we provided. It might make sense, indeed: there are three possible signature types, with predefined sizes, so checking size is cheaper than performing a checksum comparison for a general case. If size is wrong, no need to parse the buffer and calculate a checksum. In my code I use -1 for the salt. MS-PAC 2.8.2 says "The Key Usage Value MUST be KERB_NON_KERB_CKSUM_SALT (17) [MS-KILE] (section 3.1.5.9)." Should we we use a specific checksum salt in the call to KDB? |
TODO: fix |
I'm not sure if this is the best solution to the interop problem, but all of the solutions I can think of carry significant costs: The current PR solution is not future-proof for new enctypes, and will fail requests (even ones for tickets that would work without PACs) if the krbtgt entry only contains aes-sha2 keys, as we can see in the CI failure. And of course it weakens the security of the realm; realm-internal S4U operations would be relying on SHA-1 checksums when the realm is configured to avoid the use of SHA-1. (This would not be a practical security concern today, as collision attacks against SHA-1 do not affect HMAC checksums. But it could be a compliance issue, and of course it's a general concern.) Instead of looking for a key of a different enctype, we could just use the aes-sha1 checksum algorithms with the aes-sha2 keys. This would be going outside the usual bounds of RFC 3961 and would be taking advantage of laxness that we currently have in libk5crypto but might not always want to have. And it would still weaken the security of the realm. However, it would eliminate the administrative limitations on realm configuration. Instead of using a different key type or checksum type, we could truncate the checksums, if the buffer size is the only cause of the interop issue. This would not work for non-deterministic checksum types (there currently are none, but RFC 3961 does not rule them out and there have been some in the past), and would add complexity to pac.c:verify_checksum() as we could no longer use krb5_c_verify_checksum() for truncated checksums. The weakening of the realm security would be perhaps lessened as we would only be reducing the bit strength of the SHA-2 signatures, not the algorithmic strength. If I understand the interop issue correctly, it only applies to cross-realm TGTs issued by an MIT krb5 KDC and processed by an AD KDC in another realm. (That is, the PACs with overlarge KDC signature buffers aren't being rejected by Windows Kerberos services, only by AD KDCs.) If this is correct, we could make the behavior change conditional on issuing or processing a cross-realm TGT, at the cost of higher complexity. Such tickets usually aren't fed back to the issuing realm (though in theory it's possible to renew a cross-TGT) so the security and logistical concerns would be mitigated somewhat. Anything we do is likely to become a historical burden if Microsoft relaxes their PAC parsing or implements aes-sha2. |
Yes, current PR is merely a proposal to invoke a practical discussion. It looks like if we want to use aes-sha2 checksum for realm-internal tickets with PAC, we would still be in violation of the MS-PAC 2.8.2, even though this would be accepted by the MIT realm's KDCs that know how to issue PACs and work with aes-sha2 by default -- at this point these are MIT krb5 upstream and FreeIPA 4.9.10+ KDCs. Samba AD, both MIT- and Heimdal-based, does not support aes-sha2 because Microsoft AD does not have supprot for aes-sha2. Any cross-realm interop with AD KDCs would need to use aes-sha1 checksum, as you noted. How hard would be to apply the checksum change only to cross-realm TGTs?
and it seems to work for me because FreeIPA always has aes-sha1 keys when deployed with trust to Active Directory. If KDB driver could influence this choice dynamically, that would be the best. Changing kdc configuration is problematic. |
though it seems to not work in all cases. I am testing with my experimental FreeIPA KDB driver that supports krb5 1.20 so it prints some additional debug information. It used to work fine before this change for in-realm cases. I am missing code paths for verification, of course:
|
Right. It's a bad specification that doesn't permit new enctypes, but it's their spec and we took on that cost by using it in the core KDC code. For realm-internal operations we have to decide whether it's better to violate the spec or to downgrade enctypes. For cross-realm operations we obviously need to interoperate.
Because the KDC might know about the foreign realm's capabilities? I can't think of any other reason to consult the KDB for the KDC signature key. I thought of another potential solution: we could derive an aes-sha1 key using krb5_c_derive_prfplus() for the KDC signature. That wouldn't be abusing the crypto library, wouldn't require an aes-sha1 key in the krbtgt principal, and wouldn't require adding complexity to pac.c as checksum truncation would. We would still be downgrading enctypes; my primary practical concern there is that I could imagine Fedora wanting to disable SHA-1 in its OpenSSL library at some point. If we did this for all PACs then we have another upgrade like 1.20 where cross-version KDCs would reject each others' tickets for S4U operations (not the end of the world, I think). If we do this for only cross-TGT PACs then there would be much less likely to be a practical issue with upgrades, although we would still run the risk of interoperation issues with realm-internal MIcrosoft application servers. |
One realm-internal setup I can think about is running Microsoft SQL Server on a Linux host enrolled into MIT Kerberos-based realm (we have this with RHEL IdM). MS SQL Server uses S4U operations actively to refresh group membership information of users logged into the server. On the other hand, judging by https://rhel.pkgs.org/8/mssql-server-2019-x86_64/mssql-server-15.0.4249.2-1.x86_64.rpm.html, it is built against krb5-libs, so may be we are lucky here if parsing and verifying PAC is done by the library.
FreeIPA and Samba AD have separate entries that represent trust objects on top of cross-realm principals. These entries include details about trust configuration (direction details, transitivity properties, SID filtering, encryption types supported, etc). I am thinking about future situation when FreeIPA would be able to trust another FreeIPA and we would like to not downgrade the signature.
Fedora and RHEL aren't issue here: as long as Microsoft's Active Directory implementation is using SHA-1, there will be a way to enable SHA-1-based checksums and encryption types. It is, in fact, the case in RHEL 9 right now: while by default RHEL IdM uses SHA-2 encryption types, SHA-1 checksums supported and even required when trust to Active Directory is considered. This is documented in RHEL documentation:
Right. I need to test realm-internal MS SQL server case. Being able to limit this change to cross-realm TGT PACs will help. |
I have some new thoughts on this after doing the initial work on the full PAC signatures (PR #1284), although they may not lead anywhere productive:
|
@josephsutton1 Do we have tests for the privsvr checksums to verify Windows behavior in Samba which Greg talks about? |
I don't know that they cover every edge case you're interested in, but we do have some tests for the new checksum type. See Samba commit a50a2be622afaa7a280312ea12f5eb9c9a0c41da, which adds tests in compatability_tests.py and s4u_tests.py, as well as having existing tests check for the new signature. The environment variable |
I'd like to wrap this up soonish since it's better to handle it before the 1.21 release. I'll make the simplifying assumption that we will use key derivation rather than key searching to force the enctype. The options are, then:
Does anyone have preferences? |
Hi @greghudson. We have been discussing this with @jrisc. I think the best approach would be a blend of (2) and (3) without an explicit interface for KDB. As you pointed in a different discussion we had, KDB driver can already inform KDC about per-principal suggestions with string attributes. In FreeIPA we can modify KDB driver to attach a particular string attribute to cross-realm principals for trust to Active Directory. We can also attach the same attribute to in-realm principals associated with clients which do not understand new encryption types. This mostly would apply to Windows clients coerced to join 'IPA domain' or to Microsoft software like SQLServer running on Linux hosts. I am still trying to understand whether the latter is a real problem for us. If string attribute is supported, it will become easy way to configure use of aes256-sha1 key. I guess, it would also allow easier validation as well on the KDC side. What do you think? |
That seems like a good approach, as any KDB module could use it (some automatically and some with an explicit kadmin setstr). |
@greghudson any suggestion what to use for that attribute? There is already |
What name, you mean? I was thinking |
Ok, good enough. Thank you. |
e9289a0
to
077a7e9
Compare
I have pushed an implementation of the design we discussed. |
Thank you, this looks good to me. |
077a7e9
to
654f257
Compare
Reviving this discussion. I have extended FreeIPA to provide the hint for enctype of the cross-realm tgt and it is accepted. However, we still fail the operation for S4U2Self+S4U2Proxy case because while a service gets the correct krbtgt when asking for S4U2Self, the next S4U2Proxy request fails with
Below is a full trace output. I also have network traces and a keytab.
|
Network trace and keytabs from the test environment: |
I can't view the PACs within the tickets in that network trace using Wireshark, I think because neither of the keytabs contains the cross-realm TGT key (krbtgt/AD2022-C3GO.TEST@IPA.TEST). The local krbtgt key for IPA.TEST might also be helpful. |
Sorry, attaching an updated keytab (and the same trace for convenience) that contains all keys from both sides. My specific problem is the packet 103 that has evidence ticket with etype 20 (using AES256-CTS-HMAC-SHA384-192) instead of etype 18 (AES256-CTS-HMAC-SHA1-96).
|
Tentatively, I think this is a client problem, not a KDC problem. The failing request is made to AD2022-C3GO.TEST, but both the header ticket and evidence tickets are issued by IPA.TEST (and not for cross-realm TGTs). The final query to the remote realm in an RBCD exchange. In an RBCD flow, the request to the remote realm should have cross-TGTs in both the header and evidence tickets, with the impersonator's PAC in the header ticket and the impersonated client's PAC in the evidence ticket. The AD KDC is probably not complaining about the PAC checksum, but rather the enc-part of the header or evidence tickets (which also use enctype 20). In the traces, we start with an expected S4U2Self flow for an enterprise client principal: use AS-REQs to identify the realm as AD2022-C3GO.TEST and then a couple of TGS-REQs to get Administrator@AD2022-C3GO.TEST -> cifs/master.ipa.test@IPA.TEST tickets. Then things seem to go off the rails, as there is another realm identification leading up to the failing TGS request. I am not sure where we are in the code at that point, as the trace logging for client S4U operations is unfortunately a bit weak. |
The mystery of the repeated realm identification is explained by looking carefully at the command line:
kvno -P does not take an argument, so both cifs/master.ipa.test@IPA.TEST and cifs/ad1-c3go.ad2022-c3go.test@AD2022-C3GO.TEST are treated as positional arguments. So kvno does:
The second S4U2Proxy request fails because cross-realm S4U2Proxy must use referrals. This kvno invocation instead explicitly specifies a foreign realm for the server principal. We went back and forth on how to handle this case in the PR implementing RBCD (#912 (comment)). We decided to make this absurd-seeming S4U2Proxy request because it can succeed if the apparently-foreign service realm is actually an alias for the local realm. Try editing the command line to read:
|
Thank you. I tried the suggested command and it did fail on KDC side not finding
and
This last TGS-REQ has the following request in packet 75:
|
In the trace output it looks like a client is enforcing our realm:
In addition to that, the client sets principal type to |
The referral realm is a client-side construction to work with the existing API, not a protocol concept. That trace doesn't indicate the client enforcing the local realm. The name type may be an issue, although it would not be usual to use an enterprise name for the service principal. If this were a GSS application using a host-based name, the name type would be KRB5_NT_SRV_HST. kvno can use a service principal of this type if you use "-S cifs" and then the hostname in hte positional argument, although then you have to make sure that hostname canonicalization doesn't mess things up (e.g. set dns_canonicalize_hostname = false in [libdefaults]). Alternatively the kvno -u option can force the name type to NT_UNKNOWN. On the KDC, referrals can be generated by the KDB layerr, or at the KDC layer using [domain_realm] configuration. Ordinarily the KDC layer only generates referrals for SRV_HST service principals, although it can be configured to generate them for NT_UNKNOWN by setting host_based_services in the kdc.conf realm configuration. |
This was helpful, thank you. I added support for issuing a referral for the server principal and got a bit further. Now I get an issue at the S4U2Proxy request on IPA KDC side with
|
From the protocol trace, this is the initial S4U2Proxy request from cifs/master.ipa.test@IPA.TEST to IPA.TEST. The error originates from tgs_policy.c:503, probably because is_referral is true. This test is only supposed to be applied to cross-realm S4U2Proxy requests (see line 475), which this is not(*). The is_crossrealm flag is determined in do_tgs_req.c:686:
In the request, the header server realm (the server realm of the ticket inside the PA-TGS-REQ padata) is IPA.TEST, and the request server realm is also IPA.TEST. So the cross-realm flag shouldn't be set, unless for some reason the KDB module put a realm other than IPA.TEST in the principal entry when the header server (krbtgt/IPA.TEST@IPA.TEST) was looked up. (*) Obviously this is a cross-realm S4U2Proxy flow, but the specific definition of a cross-realm TGS request we use within the KDC code is a request where the TGT originated from a different realm. If IPA.TEST successfully issued a referral, the subsequent request to the AD realm would be a cross-realm request. |
We can ignore that last one, it was my mistake of short-circuiting server referral case to client referral one in KDB code. |
My current version gets further, I only get this denial on the final step of S4U2Proxy request from AD DC. I am yet to find out what is missing. Here is a trace: |
I thought to look at the extended error data in the KRB-ERROR, but it only raises more questions. Wireshark can't interpret it:
Which means it found a 0x30 (SEQUENCE) where it expected 0xA1 (CONTEXT 1). Looking at the bytes by hand, I see:
and then 235 bytes of an apparently non-ASN.1 payload. There are several mysteries here:
|
I've seen the same issue (undocumented returned error payload) in past as well. When I asked dochelp@ about it in https://lists.samba.org/archive/cifs-protocol/2022-September/003769.html, I eventually found out it was AD DC hinting about PA types it supports -- in this case PA 136:
|
Oh, of course. This isn't actually an advertisement of padata type support; rather, because the request used FAST TGS, the error is protected using FAST as well. The actual Microsoft extended error (if there is one) is inside the payload. It's unfortunate that Wireshark (at least the version I have installed) can't decode it, as decoding it by hand would be labor-intensive. |
Aha, thank you. Yes, adding wireshark support for this would be great. From the checksum algorithm replacement feature side, this is now working well. Any idea when krb5 1.21 would be out? We have customers who are waking up to Microsoft's deadlines to enforce PAC full checksum enforcement and would really like to get these improvements. |
Active Directory does not support RFC 8009 encryption types and [MS-PAC] 2.8.2 spec outlines how cryptographic system has to be selected to produce a KDC checksum. FreeIPA 4.9.10+ defaults to RFC 8009 enctypes, resulting in a PAC buffer with a checksum rejected by Active Directory.
If a local TGT key is one of RFC 8009 encryption types, downgrade the key to use for checksum to either ENCTYPE_AES128_CTS_HMAC_SHA1_96 or ENCTYPE_AES256_CTS_HMAC_SHA1_96 if they available.
Discussed with Microsoft dochelp@ in
https://lists.samba.org/archive/cifs-protocol/2022-September/003761.html
Signed-off-by: Alexander Bokovoy abokovoy@redhat.com