8270137: Kerberos Credential Retrieval from Cache not Working in Cross-Realm Setup

[martinuy]

I'd like to propose a fix for JDK-8270137 [1].

This bug is triggered when using a previously stored referral ticket (in the Referrals Cache) at the moment of following a S4U2Proxy cross-realm referral. The mistakenly-used referral ticket matched the client and service names but it was obtained as a result of a non-S4U2Proxy request. In fact, it was the middle service that got it while trying to determine the backend service realm in a previous S4U2Proxy communication. The mistakenly-used referral ticket was not bind to the impersonated user (in other words, it was not obtained attaching the user's TGS as part of a S4U2Proxy request) and, thus, must not be used.

Even when one possible approach to fix this issue could be to be more selective at the moment of getting referral tickets from the Cache (that is: do not get anything from the Cache if it's for a S4U2Proxy request), I decided to go one step further and enhance the Referrals Cache. With this enhancement, we add more information to the stored referral tickets such as a footprint of the TGS (in the case of S4U2Proxy requests) or the user principal (in the case of S4U2Self requests). We now allow to store S4U2Proxy and S4U2Self referrals tickets but those will be re-used only if there is a perfect match of the TGS or user principal. As an example, if a middle service tries to replicate the exact S4U2Self communication for exactly the same user, cached referral tickets should be okay. With this enhancement, we increase the use of the Cache and the performance (time, network resources, etc.).

The ReferralsTest is enhanced to reflect these new scenarios and now uses cached S4U2Proxy/S4U2Self referral tickets.

No regressions observed in jdk/sun/security/krb5.

--
[1] - https://bugs.openjdk.java.net/browse/JDK-8270137

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8270137: Kerberos Credential Retrieval from Cache not Working in Cross-Realm Setup

Reviewers

• Weijun Wang (@wangweij - Reviewer) ⚠️ Review applies to 4260067

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5036/head:pull/5036
$ git checkout pull/5036

Update a local copy of the PR:
$ git checkout pull/5036
$ git pull https://git.openjdk.java.net/jdk pull/5036/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5036

View PR using the GUI difftool:
$ git pr show -t 5036

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5036.diff

[bridgekeeper]

👋 Welcome back mbalao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@martinuy The following label will be automatically applied to this pull request:

• security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (3e6f2db7)
• 01: Full - Incremental (4cb4b3e0)
• 00: Full (4260067e)

[Raytion]

Thanks for your effort. We tested this patch on our internal environment and it works fine. It definetly makes sense to store Proxy tickets in the cache to reduce network load instead of our proposed "do not use cache for Proxy tickets" approach.

[wangweij]

Looks fine. Some small comments.

[openjdk]

@martinuy This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8270137: Kerberos Credential Retrieval from Cache not Working in Cross-Realm Setup

Reviewed-by: weijun

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 24 new commits pushed to the master branch:

• 35b399a: 8269130: Replace usages of Collection.toArray() with Collection.toArray(T[]) to avoid redundant array copying
• 2b05fae: 8260262: Use common code in function unmap_shared() in perfMemory_posix.cpp
• f2599ad: 8272196: Remove unused class ParStrongRootsScope
• 1f88134: 8271869: AArch64: build errors with GCC11 in frame::saved_oop_result
• 089e83b: 8266490: Extend the OSContainer API to support the pids controller of cgroups
• 2384e12: 8270098: ZGC: ZBarrierSetC2::clone_at_expansion fails with "Guard against surprises" assert
• d53d94b: 8271925: ZGC: Arraycopy stub passes invalid oop to load barrier
• 3b899ef: 8272168: some hotspot runtime/logging tests don't check exit code
• abdc107: 8270454: G1: Simplify region index comparison
• eb6f3fe: 8272169: runtime/logging/LoaderConstraintsTest.java doesn't build test.Empty
• ... and 14 more: https://git.openjdk.java.net/jdk/compare/e882087f7abb21eb776af8c14bc62e7624ebb7f3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[martinuy]

/integrate

[openjdk]

Going to push as commit 67869b4.
Since your change was applied there have been 24 commits pushed to the master branch:

• 35b399a: 8269130: Replace usages of Collection.toArray() with Collection.toArray(T[]) to avoid redundant array copying
• 2b05fae: 8260262: Use common code in function unmap_shared() in perfMemory_posix.cpp
• f2599ad: 8272196: Remove unused class ParStrongRootsScope
• 1f88134: 8271869: AArch64: build errors with GCC11 in frame::saved_oop_result
• 089e83b: 8266490: Extend the OSContainer API to support the pids controller of cgroups
• 2384e12: 8270098: ZGC: ZBarrierSetC2::clone_at_expansion fails with "Guard against surprises" assert
• d53d94b: 8271925: ZGC: Arraycopy stub passes invalid oop to load barrier
• 3b899ef: 8272168: some hotspot runtime/logging tests don't check exit code
• abdc107: 8270454: G1: Simplify region index comparison
• eb6f3fe: 8272169: runtime/logging/LoaderConstraintsTest.java doesn't build test.Empty
• ... and 14 more: https://git.openjdk.java.net/jdk/compare/e882087f7abb21eb776af8c14bc62e7624ebb7f3...master

Your commit was automatically rebased without conflicts.

[openjdk]

@martinuy Pushed as commit 67869b4.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[openjdk]

@martinuy Unknown command backport - for a list of valid commands use /help.