Skip to content
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

Implement Kerberos encryption types from RFC8009 (AES HMAC-SHA2 familly) #1684

Closed
wants to merge 5 commits into from

Conversation

jrisc
Copy link

@jrisc jrisc commented Jan 16, 2024

This pull request implements the following features in the Impacket library:

It also modifies the getST.py script:

The motivation for these changes is to test the Bronze-Bit exploit (CVE-2020-17049) against FreeIPA. It cannot be done with the current implementation because getST.py is designed to work against Active Directory only. FreeIPA differs on the following points:

  • Supports and priorities AES HMAC-SHA2 encryption types
    • These encryption types are not supported by Active Directory yet
  • Requires KDC-REQ-BODY checksums in TGS-REQ
  • By default, does not allow encryption types or checksum types relying on MD5
    • Since the PA-FOR-USER sequence in S4U2Self requests requires HMAC-MD5, it is incompatible with FreeIPA

The PA_S4U_X509_USER sequence basically serves the same purpose as PA-FOR-USER, but it uses the checksum type associated with the encryption type of the session key. Hence it can be any of the HMAC-MD5, HMAC-SHA1 or HMAC-SHA2 ones.

To be mentioned that FreeIPA does not have the same approach as Active Directory when it comes to service principals. For Active Directory, an SPN has to be owned by a user account, and S4U requests can use this account name rather than the actual proxy service principal. This is why getST.py expects an identity with the domain/username format as final argument (the actual Active Directory format is domain\username format though, with \). This is all AD-specific. The standard Kerberos format is username@realm for users and primary/hostname@realm for services. In addition, FreeIPA service principals are meant to be used directly in S4U requests, since they don't have an "owner" in the AD sense.

As a consequence, the identify to pass to getST.py in order for it to work with a FreeIPA-managed service principal was something like EXAMPLE.COM/HTTP/web.example.com (EXAMPLE.COM as realm, and HTTP/web.example.com as service principal). This was quite confusing, so I included a commit to add support for standard Kerberos principals in addition to AD usernames.

IMPORTANT: This pull request requires Legrandin/pycryptodome#781 to work. The SP800_108_Counter() function in its current form does not allow passing null bytes in label, while this is required by RFC8009.

Copy link
Author

@jrisc jrisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea about what is causing this 2 and 4 bytes offset in KDC-REQ-BODY DER encoding is welcome.

@@ -318,6 +375,19 @@ def doS4U(self, tgt, cipher, oldSessionKey, sessionKey, nthash, aesKey, kdcHost)

seq_set(authenticator, 'cname', clientName.components_to_asn1)

reqBodyEncCksumType = cksumtype_for_enctype(cipher.enctype)

reqBodyEnc = encoder.encode(reqBody)[2:]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, the 2 first bytes of the encoded KDC-REQ-BODY sequence are not used for computing the checksum. I am not sure what they represent, but I suppose there is a better way to do this...


reqBodyEncCksumType = cksumtype_for_enctype(cipher.enctype)

reqBodyEnc = encoder.encode(reqBody)[4:]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here, but this time the 4 first bytes are irrelevant for computing the KDC-REQ-BODY checksum.

@anadrianmanrique anadrianmanrique added the in review This issue or pull request is being analyzed label Feb 1, 2024
@anadrianmanrique
Copy link
Contributor

@jrisc
First, apologies for the delayed response.
Second, what an amazing work that you've done.
We discussed about this PR with the team and this is what we decided:
### impacket's kerberos stack it's not going to support (in the short/mid term) platforms other than Micosoft/AD. Supporting other platforms involves for us to regression the code in these environments. This is not a priority at the moment.
Being said that, if some of these features that this PR it's adding support to, it is present in the Micorosft's implementation, we can consider those specific changes ( as we will be able to test and regression it in the future ).
So questions are:
is there a way to exercise this whole code in a Microsoft environment?
is there a way to exercise parts of this code in a Microsoft environment? if so, can be the PR changes be reduced/scoped to that code?
Let us know your thoughts
Thanks!

@abbra
Copy link

abbra commented Mar 11, 2024

@anadrianmanrique thanks for the response.

Microsoft is working to add RFC8009 support too, as announced by Steve Syfuhs (Microsoft Kerberos developer) in https://syfuhs.net/improvements-in-windows-kerberos-architecture. They need that to make sure they are FIPS 140-3 compliant (they submitted a set of crypto modules for that already).

So you should be expecting this coming to Windows any time soon.

@abbra
Copy link

abbra commented May 29, 2024

FYI, Microsoft has release public preview of Windows Server 2025 that includes support for RFC8009 enctypes. You might want to consider this work before Microsoft releases this version later this year to production.

See https://learn.microsoft.com/en-us/windows-server/get-started/whats-new-windows-server-2025 for more details.

@anadrianmanrique anadrianmanrique removed the in review This issue or pull request is being analyzed label Sep 19, 2024
@abbra
Copy link

abbra commented Sep 19, 2024

@anadrianmanrique Any reason why you closed this pull request? It implements RFC8009 support and I do not see any reason to close it without merging.

@anadrianmanrique
Copy link
Contributor

Hello @abbra .
Reasons to close the PR were pretty much explained in #1684 (comment).
We plan to support RFC8009 in the context of Microsoft's Active Directory, that's why we linked this PR with #1812, to be used as a further reference.

@abbra
Copy link

abbra commented Sep 20, 2024

@anadrianmanrique sorry, I still do not buy that answer. It is, in fact, not an answer at all, given that Windows Server 2025 does now support RFC8009. The code in this PR does not depend on anything that Windows Server 2025 does not provide, once you enabled it to use RFC8009 encryption types.

The code from this PR can be used to operate against non-Windows Server 2025 systems such as FreeIPA deployments but the code itself is not dependent on FreeIPA or anything outside Windows Server 2025-based deployment.

I welcome your plans to add support of RFC8009 encryption types in the context of Active Directory implementation. I guess my primary question would be how closing down this PR without the actual code review contributes to furthering that goal.

@anadrianmanrique
Copy link
Contributor

anadrianmanrique commented Sep 24, 2024

@abbra, this PR adds supports for freeIPA environments. This implies not only support for RFC8009 but also changes related to the freeIPA specifications. Those were enumerated in the first comment:

  • PA_S4U_X509_USER ASN.1 sequence
  • Support for standard Kerberos principal format
  • Add the KDC-REQ-BODY checksum to S4U2Self and S4U2Proxy TGS-REQs
  • Replace the PA-FOR-USER sequence by the PA_S4U_X509_USER one

In addition, changes relay on development version of cryptodome. Our dependencies are always stable versions.

And lastly, the PR code contains python syntax errors => ( https://github.com/fortra/impacket/actions/runs/10964903059/job/30449542759 )

Based on this, we decided to close the PR. At the same time we created a branch with these changes in order to use it a base code for windows 2025 support.

If you are interested in modify this PR, remove code related to freeIPA support and test it against windows 2025, I'd be more than happy to reopen it.

@abbra
Copy link

abbra commented Sep 25, 2024

@anadrianmanrique you are welcome to pick up these changes as you'll require them to be compatible with up to date Active Directory implementation. All of them are part of MS-KILE, MS-PAC, and MS-SFU specifications' requirements that Microsoft's Active Directory implements and enforces. Both Samba AD and FreeIPA do implement these changes, though Samba AD currently lacks support for RFC 8009 encryption types on AD DC side.

  • PA_S4U_X509_USER -> MS-SFU 2.2.2
  • KDC-REQ-BODY checksum is required by MS-SFU 2.2.2 as well, this is part of S4U2Self protocol transition. This is really something that Windows systems implement since Windows Server 2008 (see note 4 of MS-SFU spec), it is not specific to FreeIPA at all:

<4> Section 2.2.2: Windows Vista, Windows Server 2008, Windows 7, and Windows Server 2008 R2
send the PA-S4U-X509-USER padata type alone if the user's certificate is available. If the user's
certificate is not available, it sends both the PA-S4U-X509-USER padata type and the PA-FOR-USER
padata type. When the PA-S4U-X509-USER padata type is used without the user's certificate, the
certificate field is not present.
Except in Windows Server 2003, Windows domain controllers first look for the information in the PA-
S4U-X509-USER padata type if present; if it is not present Windows domain controllers look at the PA-
FOR-USER padata type.
In Windows 2000 Server, Windows Server 2003, and Windows Server 2008 operating system with
Service Pack 2 (SP2), KDCs do not add the PA-S4U-X509-USER padata type in the encrypted-pa-data
field in TGS-REP.

Regarding the error, looks like it is an incomplete refactoring as the classes were renamed by @jrisc:

  • GSSAPI_AES256_SHA2 -> GSSAPI_AES256_SHA384
  • GSSAPI_AES128_SHA2 -> GSSAPI_AES128_SHA256

@jrisc is currently off on vacation and I cannot push to his branch so I'll ask him to update once he is back to the office.

@jrisc
Copy link
Author

jrisc commented Sep 30, 2024

@anadrianmanrique I just fixed the GSSAPI class naming mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants