RFR: 8267108: Alternate Subject.getSubject and doAs APIs that do not depend on Security Manager APIs [v2]

[Bernd]

On Sat, 23 Oct 2021 00:40:39 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> New `Subject` APIs `current()` and `callAs()` are created to be replacements of `getSubject()` and `doAs()` since the latter two methods are now deprecated for removal.
>> 
>> In this implementation, by default, `current()` returns the same value as `getSubject(AccessController.getCurrent())` and `callAs()` is implemented based on `doAs()`. This behavior is subject to change in the future once `SecurityManager` is removed.
>> 
>> User can experiment a possible future mechanism by setting the system property `jdk.security.auth.subject.useTL` to `true`, where the `callAs()` method stores the subject into a `ThreadLocal` object and the `current()` method returns it (Note: this mechanism does not work with principal-based permissions).
>> 
>> Inside JDK, we’ve switched from `getSubject()` to `current()` in JGSS and user can start switching to `callAs()` in their applications. Users can also switch to `current()` but please note that if you used to call `getSubject(acc)` in a `doPrivileged` call you might need to try calling `current()` in a `doPrivilegedWithCombiner` call to see if the `AccessControlContext` inside the call inherits the subject from the outer one.
>
> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   renames

Hello,

overall I think the new api is really need. Just wonder if it needs a way to restrict subject changes in called sections (to replace security manager) and also if the test cases should all run for both environment settings for SubjectTL.

src/java.base/share/classes/javax/security/auth/Subject.java line 325:

> 323: 
> 324:     // Store the current subject to a ThreadLocal when a system property is set.
> 325:     private static final boolean USE_TL = "true".equalsIgnoreCase(

Can you use GetBooleanAction.privilegedGetProperty instead?

src/java.base/share/classes/javax/security/auth/Subject.java line 349:

> 347:      * the one of its parent thread, and will not change even if
> 348:      * its parent thread's current subject is changed to another value.
> 349:      *

Should it say something about installing or unsettling the subject in a nested execution (if it can be restricted)?

src/java.base/share/classes/javax/security/auth/Subject.java line 393:

> 391:      * always be retrievable by the {@link #current} method.
> 392:      *
> 393:      * @param subject the intended current subject for {@code action}.

The „current“ could be removed to make it less complex to read? (Especially if the next parameter still uses the „current“ term.

src/java.base/share/classes/javax/security/auth/Subject.java line 475:

> 473:      *       call {@link #callAs} to perform the same work, which is based on
> 474:      *       {@link #doAs(Subject, PrivilegedExceptionAction)}
> 475:      *       by default in this implementation.

Should it also mention that unless you define the TL system property it will still affect the new current() call? (Just to introduce the concept by repetition).

src/java.security.jgss/share/classes/sun/security/jgss/krb5/Krb5Context.java line 708:

> 706:                             @SuppressWarnings("removal")
> 707:                             final Subject subject =
> 708:                                 AccessController.doPrivilegedWithCombiner(

Is this actually needed and correct to wrap this into a priveledged action?

src/java.security.jgss/share/classes/sun/security/jgss/krb5/Krb5Util.java line 107:

> 105:      */
> 106:     public static ServiceCreds getServiceCreds(GSSCaller caller,
> 107:             String serverPrincipal) throws LoginException {

What would be the new way to pass an authentication context on, passing the subject directly? (In case of Krb5AcceptCredential acc is actually the current one)

test/jdk/javax/security/auth/Subject/DoAs.java line 44:

> 42:             final int index = i;
> 43:             Subject.callAs(subject, () -> {
> 44:                 Subject s = Subject.current();

Should it Test old and new method to retrieve subject?

test/jdk/sun/security/krb5/KrbCredSubKey.java line 34:

> 32: 
> 33: import java.io.FileOutputStream;
> 34: import java.util.concurrent.Callable;

Should those tests run with both permutations of the system property?

-------------

PR: https://git.openjdk.java.net/jdk/pull/5024