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

[mark . reinhold]

2021/6/29 5:17:39 -0700, anika.westb...@raytion.com:
> Dear Developers,
> 
> we have the problem with Kerberos and AdoptOpenJDK in a cross-realm
> setup that the first request succeeds, but subsequent requests
> fail. The reason is that the ticket from the referrals cache does not
> work for proxy requests. We opened this ticket:
> https://github.com/adoptium/adoptium-support/issues/318
> 
> We also attached a patch to the ticket that solves the problem for
> us. If someone could check it out so the patch could make it into the
> next update release, that would make us and our customer very happy.
> 
> If there is anything we can do further to ease your life, please let
> us know. We are not fully used to your workflows but would want to
> make sure we are playing according to the rules.

The best way to submit a bug report against the JDK is via
https://bugreport.java.com.  Please include your patch in that
submission.  For IP clarity, we cannot take in patches posted to
non-OpenJDK infrastructure.

> In case you would agree that this is a bug and will be fixed, is there
> any estimate on likelihood of getting into one of the subsequent
> releases and when? This would be super helpful to hear, any hint or
> pointer is highly appreciated.

In general we cannot make any promises.  If you need actual support,
you might consider entering into a contractual relationship with one
of the many commercial providers of JDK builds.

- Mark

Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller [v2]

[Jaikiran Pai]

On Wed, 30 Jun 2021 15:45:25 GMT, Weijun Wang  wrote:

>> Add a cache to record which sources have called `System::setSecurityManager` 
>> and only print out warning lines once for each.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update cache key from String to Class

FWIW - I built a local JDK with this PR and gave it a try against one of the 
original Apache Ant project example where the warnings were previously flooding 
the stderr. With this change, I now see the warning message just once (as 
expected) for that example.

-

PR: https://git.openjdk.java.net/jdk17/pull/166

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

[Anika Westburg]

Dear Developers,

we have the problem with Kerberos and AdoptOpenJDK in a cross-realm setup that 
the first request succeeds, but subsequent requests fail. The reason is that 
the ticket from the referrals cache does not work for proxy requests. We opened 
this ticket:
https://github.com/adoptium/adoptium-support/issues/318

We also attached a patch to the ticket that solves the problem for us. If 
someone could check it out so the patch could make it into the next update 
release, that would make us and our customer very happy.

If there is anything we can do further to ease your life, please let us know. 
We are not fully used to your workflows but would want to make sure we are 
playing according to the rules.

In case you would agree that this is a bug and will be fixed, is there any 
estimate on likelihood of getting into one of the subsequent releases and when? 
This would be super helpful to hear, any hint or pointer is highly appreciated.

Kind regards
Anika Westburg




Anika Westburg

Raytion GmbH | Benrather Straße 18 – 20 | 40213 Düsseldorf | Germany
T +49 211 55 02 66 0
anika.westb...@raytion.com | www.raytion.com

Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller [v2]

[Weijun Wang]

> Add a cache to record which sources have called `System::setSecurityManager` 
> and only print out warning lines once for each.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  update cache key from String to Class

-

Changes:
  - all: https://git.openjdk.java.net/jdk17/pull/166/files
  - new: https://git.openjdk.java.net/jdk17/pull/166/files/a9188921..c158d4bf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk17=166=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=166=00-01

  Stats: 9 lines in 1 file changed: 0 ins; 0 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/166.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/166/head:pull/166

PR: https://git.openjdk.java.net/jdk17/pull/166

Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller [v2]

[Weijun Wang]

On Wed, 30 Jun 2021 06:32:04 GMT, Alan Bateman  wrote:

>> I hope this is uncommon but if that class is loaded by a `ClassLoader` again 
>> and again then it will be different each time. I'll investigate more.
>
> WeakHashMap, Boolean>, where the key is the caller, should be okay 
> (assume synchronization of course). Even with applications that do call 
> setSecurityManager then the map is probably going to be one entry. I wouldn't 
> expect it is common to create custom class loaders that load code that sets a 
> security manager, meaning it is more likely that the container sets the SM 
> rather have each plugin/application/whatever try to set the system-wide SM.

Thanks. Switched to Class as cache key. New commit pushed.

-

PR: https://git.openjdk.java.net/jdk17/pull/166

Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v2]

[Kevin Walls]

On Wed, 30 Jun 2021 11:49:51 GMT, Сергей Цыпанов 
 wrote:

>> In some JDK classes there's still the following hashCode() implementation:
>> 
>> long objNum;
>> 
>> public int hashCode() {
>> return (int) objNum;
>> }
>> 
>> This outdated expression should be replaced with Long.hashCode(long) as it
>> 
>> - uses all bits of the original value, does not discard any information 
>> upfront. For example, depending on how you are generating the IDs, the upper 
>> bits could change more frequently (or the opposite).
>> 
>> - does not introduce any bias towards values with more ones (zeros), as it 
>> would be the case if the two halves were combined with an OR (AND) operation.
>> 
>> See https://stackoverflow.com/a/4045083
>> 
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into 8268764
>  - 8268764: Use Long.hashCode() instead of int-cast where applicable

The changes look good to me, we have done the same thing elsewhere.  This 
changes things in different functional areas, which is maybe unusual, but seems 
fine for a small change as long as nobody objects.

Some of the files also need a (C) year update.

-

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

Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v2]

[Сергей Цыпанов]

> In some JDK classes there's still the following hashCode() implementation:
> 
> long objNum;
> 
> public int hashCode() {
> return (int) objNum;
> }
> 
> This outdated expression should be replaced with Long.hashCode(long) as it
> 
> - uses all bits of the original value, does not discard any information 
> upfront. For example, depending on how you are generating the IDs, the upper 
> bits could change more frequently (or the opposite).
> 
> - does not introduce any bias towards values with more ones (zeros), as it 
> would be the case if the two halves were combined with an OR (AND) operation.
> 
> See https://stackoverflow.com/a/4045083
> 
> This is related to https://github.com/openjdk/jdk/pull/4309

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains two additional 
commits since the last revision:

 - Merge branch 'master' into 8268764
 - 8268764: Use Long.hashCode() instead of int-cast where applicable

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4491/files
  - new: https://git.openjdk.java.net/jdk/pull/4491/files/27233ae1..12a1d3ac

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4491=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4491=00-01

  Stats: 44011 lines in 878 files changed: 23038 ins; 17963 del; 3010 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4491.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4491/head:pull/4491

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

Re: Authorization layer API and low level access checks.

[Peter Firmstone]

A draft Authorization implementation, untested.

--
Regards,
 
Peter Firmstone



/**
 * Authorization class, instances contain the domains and Subject of the
 * Authorization context, used for Authorization decisions by Guard
 * implementations.  Provides static utility methods to make 
privilgedCall's

 * and record the current context.
 *
 * @author peter
 */
public final class Authorization {

    private static final ProtectionDomain MY_DOMAIN = 
Authorization.class.getProtectionDomain();


    private static final Authorization PRIVILEGED =
    new Authorization(new ProtectionDomain []{ MY_DOMAIN });

    private static final Authorization UNPRIVILEGED
    = new Authorization(
    new ProtectionDomain[]{
    new ProtectionDomain(
    new CodeSource(null, (Certificate [])null), null
    )
    }
    );

    private static final ThreadLocal INHERITED_CONTEXT
    = new ThreadLocal();

    private static final Guard GUARD_REGISTER_CHECK =
    GuardBuilder.getInstance("RUNTIME").get("registerGuard", 
(String) null);


    private static final Guard GUARD_SUBJECT =
GuardBuilder.getInstance("AUTH").get("getSubjectFromAuthorization", null);

    private static final Set> GUARDS =
    RC.set(Collections.newSetFromMap(new 
ConcurrentHashMap<>()), Ref.WEAK, 0);




    /**
 * Elevates the privileges of the Callable to those granted to the 
Subject
 * and ProtectionDomain's of the Callable and it's call stack, 
including the

 * ProtectionDomain of the caller of this method.
 *
 * @param 
 * @param c
 * @return
 */
    public static  Callable privilegedCall(Callable c){
    Authorization auth = INHERITED_CONTEXT.get();
    try {
    INHERITED_CONTEXT.set(PRIVILEGED);
    if (auth != null){
    return privilegedCall(auth.getSubject(), c);
    } else {
    return new CallableWrapper<>(new 
Authorization(captureCallerDomain(null), null), c);

    }
    } finally {
    INHERITED_CONTEXT.set(auth);
    }
    }

    /**
 * Elevates the privileges of the Callable to those granted to the 
Subject
 * and ProtectionDomain's of the Callable and it's call stack, 
including the

 * ProtectionDomain of the caller of this method.
 *
 * @param 
 * @param subject
 * @param c
 * @return
 */
    public static  Callable privilegedCall(Subject subject, 
Callable c){

    Authorization authorization = INHERITED_CONTEXT.get();
    try {
    INHERITED_CONTEXT.set(PRIVILEGED);
    Set p = subject != null ? 
subject.getPrincipals() : null;
    Principal [] principals = p != null ? p.toArray(new 
Principal[p.size()]) : null;
    return new CallableWrapper<>(new 
Authorization(captureCallerDomain(principals), subject), c);

    } finally {
    INHERITED_CONTEXT.set(authorization);
    }
    }

    /**
 * Elevates the privileges of the Callable to those granted to the 
Subject
 * and ProtectionDomain's of the Callable and it's call stack, 
including the

 * ProtectionDomain of the caller of this method and the Authorization
 * context provided.
 *
 * @param 
 * @param ac
 * @param c
 * @return
 */
    public static  Callable privilegedCall(Authorization ac, 
Callable c){
    if (c == null) throw new IllegalArgumentException("Callable 
cannot be null");

    if (ac != null){
    Authorization authorization = INHERITED_CONTEXT.get();
    try {
    INHERITED_CONTEXT.set(PRIVILEGED);
    Subject subject = ac.getSubject();
    Set p = subject != null ? 
subject.getPrincipals() : null;
    Principal [] principals = p != null ? p.toArray(new 
Principal[p.size()]) : null;
    Set domains = 
captureCallerDomain(principals);

    ac.checkEach((ProtectionDomain t) -> {
    if (MY_DOMAIN.equals(t)) return;
    if (principals != null){
    domains.add(
    new ProtectionDomainKey(t, principals)
    );
    } else {
    domains.add(new ProtectionDomainKey(t));
    }
    });
    Authorization auth = new Authorization(domains, subject);
    return new CallableWrapper<>(auth, c);
    } finally {
    INHERITED_CONTEXT.set(authorization);
    }
    } else {
    return privilegedCall(c);
    }
    }

    private static Set captureCallerDomain(Principal 
[] principals){

    Set options = new HashSet<>();
    options.add(Option.RETAIN_CLASS_REFERENCE);
    StackWalker walker = StackWalker.getInstance(options);
    List frames = walker.walk(s ->
    s.dropWhile(f -> 

Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller

[Alan Bateman]

On 30/06/2021 08:19, Bernd Eckenfels wrote:
Hello, sorry for being unpopular, but I just hate it to waste 
developer resources,


I realy think this deprecation message should be re-considered, it 
broke a lot of things, the amount of work to implement a caching 
solution feels like a waste of time and on top of it, there is no 
clear replacement strategy published yet.


I would restrict deprication (for removal if you really insist) to 
javadoc and not poison stdout/stderr.




The purpose of the warning message is to make it very clear that 
applications that call System.setSecurityManager in a running VM will 
fail in the future. It is not hugely different to the "Illegal 
reflective access" warning that were emitted in JDK 9 to JDK 15. Maybe 
you could clarify what you mean by "it broke a lot of things". If you 
have something that is sensitive to warnings going to stderr then I 
would have expected the "Illegal reflective access" warnings to have 
caused a lot more issues.


-Alan.

Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller

[Bernd Eckenfels]

Hello, sorry for being unpopular, but I just hate it to waste developer 
resources,

I realy think this deprecation message should be re-considered, it broke a lot 
of things, the amount of work to implement a caching solution feels like a 
waste of time and on top of it, there is no clear replacement strategy 
published yet.

I would restrict deprication (for removal if you really insist) to javadoc and 
not poison stdout/stderr.

I think we stirred up enough PR that a message was received by maintainers, no 
need to further damage java reputation by breaking perfectly working inter 
process interfaces. (This is btw true for all such warnings). And I also think 
top priority should be to publish a go-forward route which should not depend 
solely on MR-Jars,

Gruss
Bernd

--
http://bernd.eckenfels.net

Von: security-dev  im Auftrag von Daniel 
Fuchs 
Gesendet: Tuesday, June 29, 2021 8:50:08 PM
An: core-libs-...@openjdk.java.net ; 
security-dev@openjdk.java.net 
Betreff: Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager 
should only appear once for each caller

On Mon, 28 Jun 2021 21:09:43 GMT, Weijun Wang  wrote:

> Add a cache to record which sources have called `System::setSecurityManager` 
> and only print out warning lines once for each.

src/java.base/share/classes/java/lang/System.java line 337:

> 335: = Collections.synchronizedMap(new WeakHashMap<>());
> 336: }
> 337:

I wonder about the use of a WeakHashMap here. That may work well when the 
source is an interned string (a class name) which will be strongly referenced 
elsewhere and may be garbage collected if the class is unloaded, but in the 
case where it contains the name of the source jar then that string will only be 
referenced by the weak hashmap, and therefore it could be garbage collected any 
time - which would cause the mapping to be removed. In that case you cannot 
guarantee that the warning will be emitted only once.

-

PR: https://git.openjdk.java.net/jdk17/pull/166

Re: Initial feedback from Apache Ant users on recent security manager warning messages

[Jaikiran Pai]

On 30/06/21 12:10 pm, Alan Bateman wrote:

On 30/06/2021 05:51, Jaikiran Pai wrote:


In the case we are dealing with, the class is always 
"org.apache.tools.ant.types.Permissions". It will always be loaded by 
one single classloader (so classloaded just once). However, multiple 
different instances of this class will get created during the 
lifetime of the build and each such instance of 
org.apache.tools.ant.types.Permissions can/will invoke this 
setSecurityManager method.
If I read this correctly, the caller of setSecurityManager is code in 
org.apache.tools.ant.types.Permission. There may be many instances of 
Permissions but from the perspective of setSecurityManager then it's 
all the same caller Class.


Correct.

-Jaikiran

Re: Initial feedback from Apache Ant users on recent security manager warning messages

[Alan Bateman]

On 30/06/2021 05:51, Jaikiran Pai wrote:


In the case we are dealing with, the class is always 
"org.apache.tools.ant.types.Permissions". It will always be loaded by 
one single classloader (so classloaded just once). However, multiple 
different instances of this class will get created during the lifetime 
of the build and each such instance of 
org.apache.tools.ant.types.Permissions can/will invoke this 
setSecurityManager method.
If I read this correctly, the caller of setSecurityManager is code in 
org.apache.tools.ant.types.Permission. There may be many instances of 
Permissions but from the perspective of setSecurityManager then it's all 
the same caller Class.


-Alan

Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller

[Alan Bateman]

On Tue, 29 Jun 2021 21:18:35 GMT, Weijun Wang  wrote:

>> Using a synchronized WeakHashMap with the class as the key would not prevent 
>> class unloading.
>> Using a non-weak set or map to strings would keep the strings around for the 
>> life of the runtime.
>
> I hope this is uncommon but if that class is loaded by a `ClassLoader` again 
> and again then it will be different each time. I'll investigate more.

WeakHashMap, Boolean>, where the key is the caller, should be okay 
(assume synchronization of course). Even with applications that do call 
setSecurityManager then the map is probably going to be one entry. I wouldn't 
expect it is common to create custom class loaders that load code that sets a 
security manager, meaning it is more likely that the container sets the SM 
rather have each plugin/application/whatever try to set the system-wide SM.

-

PR: https://git.openjdk.java.net/jdk17/pull/166