Re: Fix proposal for bug JDK-8221642
On 31/01/2022 09:21, Andreas Rosenberg wrote: Hi Mandy, thanks for your comments. Yes, the correct solution is to examine each caller sensitive method. Surely my idea is not perfect, as the problem with the ResourceBundle and the bootstrap loader shows. I had the hope that cases like this could be solved by implementing a "proxy module object" that could define the correct behavior for such cases (e.g. the correct class loader). As far as I understood, the #getModule() call could be used for this. At least the class loader issue could probably be solved this way, just as an idea. I'm not very familiar with all the aspects of module usage, but this way you had at least a kind of definition in Java, how native code should be seen regarding module usage. My search for "@CallerSensitive" gave me 149 hits in java files, so this is quite a task to examine all. My fear is that we my run into another exception in a few month and the fixes for such problems will not arrive in a few days and we are facing the same problem again. So a global solution would be preferable. Of course you are worried about strange side effects or maybe even security /safety issues, but my hope was that somebody here had the expertise to give a good estimation on this. Regarding permissions: if you don't have any Java stack frames on the stack, that means a native application is using Java code as a kind of library (e.g. we use it to read/write MS Excel via Apache POI). In such cases the native app must care about that. I could imagine, that there could be use cases that the native app wants to limit permissions for a certain Java component (e.g. a WebView that may load data from external sites). In such cases you must define permissions for the component, but this should work as soon as there is at least one additional Java stack frame on the stack. Right? Moving this to core-libs-dev as that is where most of the @CS methods are. There are about 25 API classes in java.base that have @CS methods, most of these are reflection or related. So I don't think it's too bad. A good starting point out would be a test (maybe based on the existing exeCallerAccessTest) to make it easy to exercise each of these methods to check their behavior. As Mandy pointed out, there are for implications for access control and security to have an intermediate frame that reports its class in java.base and is full privileged so care is required. An intermediate frame of a class generated into an unnamed module may be an alternative to explore as that would limit access to public members of public classes that are in packages exported unconditionally. -Alan
Re: Fix proposal for bug JDK-8221642
Your proposal is essentially for all JNI code with no caller frame to default to java.base, which gets all permissions. It means that it could break encapsulation to access any members. Arguably one could consider JNI have superpower. In addition, default to java.base may not make sense for some Java APIs, ResouroceBundle::getBundle(String bundlename) is one example. It uses the caller class's loader to load the resource bundle. Default to java.base means it defaults to the bootstrap loader which can't find the resource bundle on the class path for example. For the ResourceBundle case, it seems that the unnamed module defined by the system class loader might be an appropriate default. The proper way is to examine each caller-sensitive method and investigate what makes sense when invoked by JNI code with no caller frame. JDK-8177155 is the RFE for such task. System::getLogger, Logger::getLogger, and core reflection API are looked at but more to follow up. I created https://bugs.openjdk.java.net/browse/JDK-8280902 to follow up the ResourceBundle::getBundle issue. Mandy [1] https://bugs.openjdk.java.net/browse/JDK-8177155 On 1/28/22 3:15 AM, Andreas Rosenberg wrote: Hi Mandy, thanks for looking at my problem. Yes, "setAccessible" is one of the problems, but our main issue is related to "ResourceBundle". I've created a small example that shows the problem:https://github.com/anrose00/JniSensitiveCaller <https://urldefense.com/v3/__https://github.com/anrose00/JniSensitiveCaller__;!!ACWV5N9M2RV99hQ!YQ77Xchy8f8FZNrtmRPANSCuvfN1KicygQkw2EDl7d-0B1ohBWgs87EWtsw-U84jWQ$> Any comments on my proposal would be great. Andreas *From:*Mandy Chung *Sent:* Freitag, 28. Januar 2022 02:54 *To:* Andreas Rosenberg *Cc:* hotspot-...@openjdk.java.net; 'core-libs-dev' *Subject:* Re: Fix proposal for bug JDK-8221642 I see how NPE is thrown (from `AccessibleObject::setAccessible` and `trySetAccessible`). The proper fix should follow the rule as the access check that it can set the accessible flag only on public members of a public type that is exported unconditionally. The fix is straight forward but involves spec change. I'll post PR soon. Mandy On 1/27/22 8:45 AM, Mandy Chung wrote: Hi Andreas, What methods are you calling that throws NPE? Do you have the stack trace to share? The spec of AccessibleObject was updated for JDK-8221530 if there is no caller frame when calling from JNI: "The check when invoked by JNI code with no Java class on the stack only succeeds if the member and the declaring class are public, and the class is in a package that is exported to all modules." I think AccessibleObject::canAccess, setAccessible, trySetAccessible should follow the same rule. Mandy On 1/27/22 2:19 AM, Andreas Rosenberg wrote: Hi, this is my first posting regarding to JDK contribution, so this may be the wrong place to ask. Please point me in the right direction in this case. We are using Java rather heavily via JNI on a custom application. For a long time we did stick to JRE 1.8 for various reasons. My task is to plan an upgrade to a more recent JDK version and while doing some test I encountered bugs related to this: JDK-8227491(JNI - caller sensitive methods). We are parsing Java class files to auto gen the JNI code for our application, and are also using reflection. The workaround given is clumsy and needs manual intervention, so I was looking for a more elegant solution. The problem is: a caller sensitive method wants to determine the caller class for security checks. In case of a JNI call no Java stack frame exists, so the JVM function "jclass JVM_GetCallerClass(JNIEnv* env)" answers NULL which leads to NPEs. My idea is this: create an internal proxy class inside "java.base" that reflects this case (e.g. "java.lang.NativeCall" or "java.lang.NativeCode"). This class is final and implements nothing. Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and instead of answering NULL in case of a JNI call, it should do this to answer the class proxy: return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall"); This would have the following advantages: - JNI code could again simply call "caller sensitive methods" without the need to make an additional wrapper class - it would be more a expressive way on the Java side to detect "the callee is native code" than checking for null - it would fit better into the framework I already applied this fix on my own copy of the JDK 17 sources and it works pretty well for us. As there are probably security consideration
RE: Fix proposal for bug JDK-8221642
Hi Mandy, thanks for looking at my problem. Yes, "setAccessible" is one of the problems, but our main issue is related to "ResourceBundle". I've created a small example that shows the problem: https://github.com/anrose00/JniSensitiveCaller Any comments on my proposal would be great. Andreas From: Mandy Chung Sent: Freitag, 28. Januar 2022 02:54 To: Andreas Rosenberg Cc: hotspot-...@openjdk.java.net; 'core-libs-dev' Subject: Re: Fix proposal for bug JDK-8221642 I see how NPE is thrown (from `AccessibleObject::setAccessible` and `trySetAccessible`). The proper fix should follow the rule as the access check that it can set the accessible flag only on public members of a public type that is exported unconditionally. The fix is straight forward but involves spec change. I'll post PR soon. Mandy On 1/27/22 8:45 AM, Mandy Chung wrote: Hi Andreas, What methods are you calling that throws NPE? Do you have the stack trace to share? The spec of AccessibleObject was updated for JDK-8221530 if there is no caller frame when calling from JNI: "The check when invoked by JNI code with no Java class on the stack only succeeds if the member and the declaring class are public, and the class is in a package that is exported to all modules." I think AccessibleObject::canAccess, setAccessible, trySetAccessible should follow the same rule. Mandy On 1/27/22 2:19 AM, Andreas Rosenberg wrote: Hi, this is my first posting regarding to JDK contribution, so this may be the wrong place to ask. Please point me in the right direction in this case. We are using Java rather heavily via JNI on a custom application. For a long time we did stick to JRE 1.8 for various reasons. My task is to plan an upgrade to a more recent JDK version and while doing some test I encountered bugs related to this: JDK-8227491 (JNI - caller sensitive methods). We are parsing Java class files to auto gen the JNI code for our application, and are also using reflection. The workaround given is clumsy and needs manual intervention, so I was looking for a more elegant solution. The problem is: a caller sensitive method wants to determine the caller class for security checks. In case of a JNI call no Java stack frame exists, so the JVM function "jclass JVM_GetCallerClass(JNIEnv* env)" answers NULL which leads to NPEs. My idea is this: create an internal proxy class inside "java.base" that reflects this case (e.g. "java.lang.NativeCall" or "java.lang.NativeCode"). This class is final and implements nothing. Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and instead of answering NULL in case of a JNI call, it should do this to answer the class proxy: return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall"); This would have the following advantages: - JNI code could again simply call "caller sensitive methods" without the need to make an additional wrapper class - it would be more a expressive way on the Java side to detect "the callee is native code" than checking for null - it would fit better into the framework I already applied this fix on my own copy of the JDK 17 sources and it works pretty well for us. As there are probably security considerations involved, advice from experts is required. But from my understanding the Java security model is designed for the main app being writing in Java. In this case there are always Java stacks frames available as parents for caller sensitive methods, so the proposed fix would not affect the behavior. This assumes that "GetCallerClass" only answers NULL for the JNI case. This needs verification. If the main app is native code which uses JNI, the Java security model can only affect the Java part and as soon as an additional Java stack frame has been generated a regular Java class will be found and the "standard behavior" should apply again. Comments appreciated. It this fix looks reasonable, what are the steps to get it implemented and integrated into the official source tree? Best regards, Andy
Re: Fix proposal for bug JDK-8221642
I see how NPE is thrown (from `AccessibleObject::setAccessible` and `trySetAccessible`). The proper fix should follow the rule as the access check that it can set the accessible flag only on public members of a public type that is exported unconditionally. The fix is straight forward but involves spec change. I'll post PR soon. Mandy On 1/27/22 8:45 AM, Mandy Chung wrote: Hi Andreas, What methods are you calling that throws NPE? Do you have the stack trace to share? The spec of AccessibleObject was updated for JDK-8221530 if there is no caller frame when calling from JNI: "The check when invoked by JNI code with no Java class on the stack only succeeds if the member and the declaring class are public, and the class is in a package that is exported to all modules." I think AccessibleObject::canAccess, setAccessible, trySetAccessible should follow the same rule. Mandy On 1/27/22 2:19 AM, Andreas Rosenberg wrote: Hi, this is my first posting regarding to JDK contribution, so this may be the wrong place to ask. Please point me in the right direction in this case. We are using Java rather heavily via JNI on a custom application. For a long time we did stick to JRE 1.8 for various reasons. My task is to plan an upgrade to a more recent JDK version and while doing some test I encountered bugs related to this: JDK-8227491 (JNI - caller sensitive methods). We are parsing Java class files to auto gen the JNI code for our application, and are also using reflection. The workaround given is clumsy and needs manual intervention, so I was looking for a more elegant solution. The problem is: a caller sensitive method wants to determine the caller class for security checks. In case of a JNI call no Java stack frame exists, so the JVM function "jclass JVM_GetCallerClass(JNIEnv* env)" answers NULL which leads to NPEs. My idea is this: create an internal proxy class inside "java.base" that reflects this case (e.g. "java.lang.NativeCall" or "java.lang.NativeCode"). This class is final and implements nothing. Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and instead of answering NULL in case of a JNI call, it should do this to answer the class proxy: return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall"); This would have the following advantages: - JNI code could again simply call "caller sensitive methods" without the need to make an additional wrapper class - it would be more a expressive way on the Java side to detect "the callee is native code" than checking for null - it would fit better into the framework I already applied this fix on my own copy of the JDK 17 sources and it works pretty well for us. As there are probably security considerations involved, advice from experts is required. But from my understanding the Java security model is designed for the main app being writing in Java. In this case there are always Java stacks frames available as parents for caller sensitive methods, so the proposed fix would not affect the behavior. This assumes that "GetCallerClass" only answers NULL for the JNI case. This needs verification. If the main app is native code which uses JNI, the Java security model can only affect the Java part and as soon as an additional Java stack frame has been generated a regular Java class will be found and the "standard behavior" should apply again. Comments appreciated. It this fix looks reasonable, what are the steps to get it implemented and integrated into the official source tree? Best regards, Andy
Re: Fix proposal for bug JDK-8221642
Hi Andreas, What methods are you calling that throws NPE? Do you have the stack trace to share? The spec of AccessibleObject was updated for JDK-8221530 if there is no caller frame when calling from JNI: "The check when invoked by JNI code with no Java class on the stack only succeeds if the member and the declaring class are public, and the class is in a package that is exported to all modules." I think AccessibleObject::canAccess, setAccessible, trySetAccessible should follow the same rule. Mandy On 1/27/22 2:19 AM, Andreas Rosenberg wrote: Hi, this is my first posting regarding to JDK contribution, so this may be the wrong place to ask. Please point me in the right direction in this case. We are using Java rather heavily via JNI on a custom application. For a long time we did stick to JRE 1.8 for various reasons. My task is to plan an upgrade to a more recent JDK version and while doing some test I encountered bugs related to this: JDK-8227491 (JNI - caller sensitive methods). We are parsing Java class files to auto gen the JNI code for our application, and are also using reflection. The workaround given is clumsy and needs manual intervention, so I was looking for a more elegant solution. The problem is: a caller sensitive method wants to determine the caller class for security checks. In case of a JNI call no Java stack frame exists, so the JVM function "jclass JVM_GetCallerClass(JNIEnv* env)" answers NULL which leads to NPEs. My idea is this: create an internal proxy class inside "java.base" that reflects this case (e.g. "java.lang.NativeCall" or "java.lang.NativeCode"). This class is final and implements nothing. Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and instead of answering NULL in case of a JNI call, it should do this to answer the class proxy: return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall"); This would have the following advantages: - JNI code could again simply call "caller sensitive methods" without the need to make an additional wrapper class - it would be more a expressive way on the Java side to detect "the callee is native code" than checking for null - it would fit better into the framework I already applied this fix on my own copy of the JDK 17 sources and it works pretty well for us. As there are probably security considerations involved, advice from experts is required. But from my understanding the Java security model is designed for the main app being writing in Java. In this case there are always Java stacks frames available as parents for caller sensitive methods, so the proposed fix would not affect the behavior. This assumes that "GetCallerClass" only answers NULL for the JNI case. This needs verification. If the main app is native code which uses JNI, the Java security model can only affect the Java part and as soon as an additional Java stack frame has been generated a regular Java class will be found and the "standard behavior" should apply again. Comments appreciated. It this fix looks reasonable, what are the steps to get it implemented and integrated into the official source tree? Best regards, Andy
