HHH-15514 EntityTestCase Test failures with Hibernate ORM when run under security manager

[scottmarlow]

https://hibernate.atlassian.net/browse/HHH-15514
https://issues.redhat.com/browse/WFLY-16974

Signed-off-by: Scott Marlow smarlow@redhat.com

[scottmarlow]

The code changes are a result of bringing some changes from ORM 5.3 into ORM 6.1.

https://gist.github.com/scottmarlow/28215540cb43ecc31c65f80a01ca8577 shows the ORM 6.1 generated proxy bytecode with this pull request in use.

https://gist.github.com/scottmarlow/2c602fcfa8512b904c4c0776a88b8152 shows the ORM 6.1 generated proxy bytecode without this pull request.

Some bytecode differences are shown below.

Without this pull request, the constructor contained:

static {};
    descriptor: ()V
    flags: (0x0008) ACC_STATIC
    Code:
      stack=6, locals=0, args_size=0
         0: ldc           #4                  // class org/jboss/as/test/integration/jpa/hibernate/entity/Flight
         2: ldc           #129                // String setId
         4: iconst_1
         5: anewarray     #131                // class java/lang/Class
         8: dup
         9: iconst_0
        10: ldc           #110                // class java/lang/Long
        12: aastore
        13: invokevirtual #135                // Method java/lang/Class.getMethod:(Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;
        16: putstatic     #68                 // Field cachedValue$46XiMV0y$m7ui7t3:Ljava/lang/reflect/Method;

Note that the invokevirtual call above is using Java Reflection which is corrected below to instead use existing org/hibernate/bytecode/internal/bytebuddy/HibernateMethodLookupDispatcher.getMethod.

After this pull request is applied, the constructor contains:

static {};
    descriptor: ()V
    flags: (0x0008) ACC_STATIC
    Code:
      stack=6, locals=0, args_size=0
         0: ldc           #4                  // class org/jboss/as/test/integration/jpa/hibernate/entity/Flight
         2: ldc           #129                // String setId
         4: iconst_1
         5: anewarray     #131                // class java/lang/Class
         8: dup
         9: iconst_0
        10: ldc           #110                // class java/lang/Long
        12: aastore
        13: invokestatic  #137                // Method org/hibernate/bytecode/internal/bytebuddy/HibernateMethodLookupDispatcher.getMethod:(Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;
        16: putstatic     #83                 // Field cachedValue$C57bfeB4$m7ui7t3:Ljava/lang/reflect/Method;

[beikov]

I looked at the code and it seems to me that you only moved code from static inner class SecurityManagerClassRewriter into ByteBuddyState. I'm having a hard time understanding how this can affect the generated byte code. Can you help me understand this?

From the bytecode you posted, I understand that the constructor rewriting didn't happen. The only explanation that would make sense is that ByteBuddyState is initialized at a point in time when no security manager is installed yet. Is that the case?

[dreab8]

I looked at the code too and may be we are just missing something but as @beikov I was not able to understand how this change can affect the generated code

[scottmarlow]

I verified locally that testing wildfly with Hibernate ORM 6.1.3.Final sees the failure. I then verified that switching to my local build of Hibernate ORM 6.1.4-SNAPSHOT, the test passes.

I looked at the code and it seems to me that you only moved code from static inner class SecurityManagerClassRewriter into ByteBuddyState. I'm having a hard time understanding how this can affect the generated byte code. Can you help me understand this?

https://github.com/hibernate/hibernate-orm/pull/5328/files#diff-0916713f1d03407fd545d95b4d8b75fbf3e5ace094c609837db7958423981e45R249 contains one of the important changes which replaces the application calling java.lang.Class#getMethod directly, to instead call the Hibernate ORM HibernateMethodLookupDispatcher#getDeclaredMethod which is more trusted (by the WildFly Security Manager) than the application code. In order for the application code to be trusted, the application has to include a custom permissions.xml stating that application code can be trusted to [accessDeclaredMembers](https://docs.oracle.com/javase/8/docs/api/java/lang/RuntimePermission.html).

From the bytecode you posted, I understand that the constructor rewriting didn't happen. The only explanation that would make sense is that ByteBuddyState is initialized at a point in time when no security manager is installed yet. Is that the case?

I did single step through the Hibernate ORM proxy generating code a few times and verified that the security manager is installed (it gets setup during WildFly server startup before any applications are deployed).

[scottmarlow]

I looked at the code too and may be we are just missing something but as @beikov I was not able to understand how this change can affect the generated code

Should we add a comment around the HibernateMethodLookupDispatcher reference? Perhaps something like:
// Instead of the application calling java.lang.Class#getMethod directly, call the Hibernate ORM HibernateMethodLookupDispatcher#getDeclaredMethod which is more trusted (by the Security Manager) than the application code.

[beikov]

Superseded by #5334