A Bug involving MethodHandles, Nestmates, Reflection and @CallerSensitive

[Johannes Kuhn]

Thanks Mandy.

Yes, @CS is a complicated beast.
I also implemented a part of JDK-6824466 for my "proxies should use 
hidden classes prototype". (Only for the "constructor for 
serialization", as hidden classes can't be mentioned in the constant pool.)

For the @CS method that can be called virtually - 
Thread.getContextClassLoader, I thought about those two interfaces:

interface GetCCLClassLoader {ClassLoader getContextClassLoader(); }
interface GetCCLObject {Object getContextClassLoader(); }

Insight: If a class extends Thread and implements GetCCLObject, javac 
will add a bridge method - which is the caller now. Have to think more 
about what this actually means.

The entire topic is very complex - but my current believe is that 
JDK-6824466 is a basic building block for a lot of other work in that 
area. It also has quite a few prototypes that have been developed 
independently - suggesting that it is indeed a basic building block.

I did a quick look though your prototype, one question I could not 
answer was "what prevents me from naming my hidden class 
Something$$InjectedInvoker?".

I will try to dig deeper into that, sure. I don't think that there will 
be any fully satisfying solution in the next months. And then I have to 
convince people that those changes don't expose any security issues - 
which will be quite hard.

But if you have some starter bugs that I could fix, let me know, might 
help to get some reputation and familiarity with the entire process.

Thank you for your time listening to my ideas, I appreciate it :).

- Johannes

On 18-Jan-21 22:52, Mandy Chung wrote:
> Hi Johannes,
> 
> There has been a couple of the prototypes regarding @CS methods (Alan 
> did one and I did another) in the context of JDK-6824466. There are lots 
> of security consideration regarding @CS methods. You are welcome to go 
> deeper in that area.  If you are looking for starter bugs to fix, that 
> won't be a quick patch.
> 
> I also came up with a patch for JDK-8013527 when working on 
> JDK-6824466.  It's buried in 
> https://github.com/openjdk/jdk/compare/master...mlchung:method-invoke. I 
> will extract that fix and post a PR on my proposed fix.
> 
> Mandy
> 
> On 1/16/21 10:07 AM, Johannes Kuhn wrote:
>> After digging though the JBS, I found this comment from John Rose[1].
>>
>> I like the general idea, but I don't think it's necessary to use a 
>> native method as stub. Instead it could be written like this:
>>
>> class Class {
>>   @CallerSensitive
>>   @ForceInline
>>   public static Class<?> forName(String name) {
>>       return forName(name, Reflection.getCallerClass());
>>   }
>>   private static Class<?> forName(String name, Class<?> caller) {
>>       // original implementation
>>   }
>> }
>>
>> By doing this, binding to the caller could be done by returning a 
>> MethodHandle that actually calls the private method - with the lookup 
>> class injected as argument (MethodHandles.insertArguments).
>>
>> Problem are methods that could be virtually invoked 
>> (getContextClassLoader). Therefore it might be useful to keep the old 
>> binding logic around. It also reduces the number of places where this 
>> change has to be done - it's only required for the 
>> hyper- at CallerSensitive methods.
>>
>> I will try to write a prototype that demonstrates that this approach 
>> is feasible.
>>
>> - Johannes
>>
>> [1]: 
>> https://bugs.openjdk.java.net/browse/JDK-8020968?focusedCommentId=13611844&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13611844
>>
>> On 09-Dec-20 21:09, Johannes Kuhn wrote:
>>> On 09-Dec-20 19:44, Mandy Chung wrote:
>>>>
>>>>
>>>> On 12/8/20 6:02 PM, Johannes Kuhn wrote:
>>>>> There are a lot of things to consider when trying to fix JDK-8013527.
>>>>
>>>> Exactly in particular security implication!  What is clear is that 
>>>> the expected lookup class should not be the injected class.   The 
>>>> key message here is that we can't fix JDK-8257874 until we fix 
>>>> JDK-8013527 unfortunately.
>>>>
>>>> Mandy
>>>>
>>> Yeah, if JDK-8013527 is fixed it might fix JDK-8257874 as a byproduct.
>>> If Lookup.lookup() can determine the original caller, then 
>>> Field.set*/Method.invoke could do the same.
>>> Special care has to be taken that no other class could spoof such an 
>>> injected invoker.
>>>
>>> Too complicated for me :). JDK-8013527 needs a sound design to 
>>> approach fixing it IMHO.
>>>
>>> - Johannes
>>>
>