14 Replies Latest reply on Nov 20, 2008 11:35 AM by Anil Saldanha

    PrivilegedBlock location

    Anil Saldanha Master

      Scott, given the following stack trace, where do you think the privileged block should be placed such that appropriate permission ("getClassLoader") can be provided? I feel that it needs to go in the aop project but I also have the suspicion that users of aop need to arrive with the "getClassLoader" permission.

      Thanks for the second opinion.

      23:22:07,502 ERROR [STDERR] access: access denied (java.lang.RuntimePermission getClassLoader)
      23:22:07,502 ERROR [STDERR] java.lang.Exception: Stack trace
      23:22:07,503 ERROR [STDERR] at java.lang.Thread.dumpStack(Thread.java:1158)
      23:22:07,503 ERROR [STDERR] at java.security.AccessControlContext.checkPermission(AccessControlContext.java:253)
      23:22:07,503 ERROR [STDERR] at java.security.AccessController.checkPermission(AccessController.java:427)
      23:22:07,503 ERROR [STDERR] at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
      23:22:07,503 ERROR [STDERR] at java.lang.Class.getClassLoader(Class.java:588)
      23:22:07,503 ERROR [STDERR] at org.jboss.aop.advice.GenericAspectFactory.getClazz(GenericAspectFactory.java:123)
      23:22:07,503 ERROR [STDERR] at org.jboss.aop.advice.GenericAspectFactory.createPerInstance(GenericAspectFactory.java:175)
      23:22:07,503 ERROR [STDERR] at org.jboss.aop.InstanceAdvisorDelegate.initializeAspects(InstanceAdvisorDelegate.java:109)
      23:22:07,503 ERROR [STDERR] at org.jboss.aop.InstanceAdvisorDelegate.initialize(InstanceAdvisorDelegate.java:71)
      23:22:07,503 ERROR [STDERR] at org.jboss.aop.ClassInstanceAdvisor.setAdvisorAndInitialise(ClassInstanceAdvisor.java:84)
      23:22:07,503 ERROR [STDERR] at org.jboss.aop.ClassInstanceAdvisor.<init>(ClassInstanceAdvisor.java:68)
      23:22:07,503 ERROR [STDERR] at org.jboss.jms.client.delegate.ClientSessionDelegate._getInstanceAdvisor(ClientSessionDelegate.java)
      23:22:07,503 ERROR [STDERR] at org.jboss.jms.client.delegate.ClientSessionDelegate.createMessage(ClientSessionDelegate.java)
      23:22:07,503 ERROR [STDERR] at org.jboss.jms.client.JBossSession.createMessage(JBossSession.java:124)
      23:22:07,503 ERROR [STDERR] at org.jboss.test.security.ejb.RunAsWithRolesMDB.sendReply(RunAsWithRolesMDB.java:130)
      23:22:07,503 ERROR [STDERR] at org.jboss.test.security.ejb.RunAsWithRolesMDB.onMessage(RunAsWithRolesMDB.java:105)
      


        • 1. Re: PrivilegedBlock location
          Scott Stark Master

          The code calling the aop layer should have the getClassLoader permission since its expected that loading/generating aspects will need access to it, and user code should not be able to do this without specific permissions.

          In this case the ClientSessionDelegate should have a privileged block.

          • 2. Re: PrivilegedBlock location
            Anil Saldanha Master

             

            "scott.stark@jboss.org" wrote:
            The code calling the aop layer should have the getClassLoader permission since its expected that loading/generating aspects will need access to it, and user code should not be able to do this without specific permissions.

            In this case the ClientSessionDelegate should have a privileged block.


            Ok. There goes my night of sleep. I had this nagging doubt. If it would be aop, then the messaging release could go ahead. They use aop everywhere.

            • 3. Re: PrivilegedBlock location
              Scott Stark Master

              Oh, but createMessage() here is a joinpoint generated by the aop layer which should have had the privileged block. It would have to be moved to the JBossSession class.

              • 4. Re: PrivilegedBlock location
                Anil Saldanha Master

                 

                "scott.stark@jboss.org" wrote:
                Oh, but createMessage() here is a joinpoint generated by the aop layer which should have had the privileged block. It would have to be moved to the JBossSession class.


                At this very moment, my hands got tired adding privileged blocks to the JBossSession class because it uses an aspect (delegate). The privileged block needs to go in JBossSession.



                • 5. Re: PrivilegedBlock location
                  Scott Stark Master

                   

                  "anil.saldhana@jboss.com" wrote:

                  Ok. There goes my night of sleep. I had this nagging doubt. If it would be aop, then the messaging release could go ahead. They use aop everywhere.


                  AOP should be generating privileged blocks for the calling code since it does live in the messaging codebase. The block would be in the org.jboss.jms.client.delegate.ClientSessionDelegate.createMessage generated code. I assume since this is statically generated it does have the jboss-messaging.jar codebase so that that is what would be assigned the getClassLoader permission. If that is how it works that is the correct way.



                  • 6. Re: PrivilegedBlock location
                    Anil Saldanha Master

                    org.jboss.jms.client.delegate.ClientSessionDelegate.createMessage

                    /**
                     * This invocation should either be handled by the client-side interceptor chain or by the
                     * server-side endpoint.
                     */
                     public MessageProxy createMessage() throws JMSException
                     {
                     throw new IllegalStateException("This invocation should not be handled here!");
                     }
                    


                    • 7. Re: PrivilegedBlock location
                      Anil Saldanha Master

                       

                      "anil.saldhana@jboss.com" wrote:
                      org.jboss.jms.client.delegate.ClientSessionDelegate.createMessage

                      /**
                       * This invocation should either be handled by the client-side interceptor chain or by the
                       * server-side endpoint.
                       */
                       public MessageProxy createMessage() throws JMSException
                       {
                       throw new IllegalStateException("This invocation should not be handled here!");
                       }
                      


                      Think they weave this code.

                      • 8. Re: PrivilegedBlock location
                        Scott Stark Master

                        That is right, dumping out the org.jboss.jms.client.delegate.ClientSessionDelegate.createMessage disassembly shows a new method that is the code calling into the aop layer:

                        [699][valkyrie: lib]$ javap -classpath jboss-messaging.jar -c org.jboss.jms.client.delegate.ClientSessionDelegate
                        
                        ...
                        
                        public org.jboss.jms.message.MessageProxy createMessage() throws javax.jms.JMSException;
                         Code:
                         0: getstatic #939; //Field aop$MethodInfo_createMessage7458165539797107901:Ljava/lang/ref/WeakReference;
                         3: invokevirtual #941; //Method java/lang/ref/Reference.get:()Ljava/lang/Object;
                         6: checkcast #449; //class org/jboss/aop/MethodInfo
                         9: astore_1
                         10: aload_0
                         11: invokevirtual #943; //Method _getInstanceAdvisor:()Lorg/jboss/aop/InstanceAdvisor;
                         14: checkcast #418; //class org/jboss/aop/ClassInstanceAdvisor
                         17: astore_2
                         18: aload_1
                         19: invokevirtual #945; //Method org/jboss/aop/JoinPointInfo.getInterceptors:()[Lorg/jboss/aop/advice/Interceptor;
                         22: astore_3
                         23: aload_3
                         24: aconst_null
                         25: checkcast #459; //class "[Ljava/lang/Object;"
                         28: if_acmpne 46
                         31: aload_2
                         32: aconst_null
                         33: if_acmpeq 43
                         36: aload_2
                         37: getfield #947; //Field org/jboss/aop/ClassInstanceAdvisor.hasInstanceAspects:Z
                         40: ifne 46
                         43: goto 97
                         46: aload_2
                         47: aconst_null
                         48: if_acmpeq 57
                         51: aload_2
                         52: aload_3
                         53: invokevirtual #949; //Method org/jboss/aop/ClassInstanceAdvisor.getInterceptors:([Lorg/jboss/aop/advice/Interceptor;)[Lorg/jboss/aop/advice/Interceptor;
                         56: astore_3
                         57: new #936; //class org/jboss/jms/client/delegate/ClientSessionDelegate$createMessage_7458165539797107901
                         60: dup
                         61: aload_1
                         62: aload_3
                         63: invokespecial #951; //Method org/jboss/jms/client/delegate/ClientSessionDelegate$createMessage_7458165539797107901."<init>":(Lorg/jboss/aop/MethodInfo;[Lorg/jboss/aop/advice/Interceptor;)V
                         66: astore 4
                         68: aload 4
                         70: aload_0
                         71: invokevirtual #953; //Method org/jboss/aop/joinpoint/InvocationBase.setTargetObject:(Ljava/lang/Object;)V
                         74: aload 4
                         76: aload_0
                         77: putfield #955; //Field org/jboss/jms/client/delegate/ClientSessionDelegate$createMessage_7458165539797107901.typedTargetObject:Lorg/jboss/jms/client/delegate/ClientSessionDelegate;
                         80: aload 4
                         82: getstatic #957; //Field aop$classAdvisor$aop:Lorg/jboss/aop/ClassAdvisor;
                         85: invokevirtual #959; //Method org/jboss/aop/joinpoint/InvocationBase.setAdvisor:(Lorg/jboss/aop/Advisor;)V
                         88: aload 4
                         90: invokevirtual #961; //Method org/jboss/aop/joinpoint/MethodInvocation.invokeNext:()Ljava/lang/Object;
                         93: checkcast #963; //class org/jboss/jms/message/MessageProxy
                         96: areturn
                         97: aload_0
                         98: invokevirtual #965; //Method org$jboss$jms$client$delegate$ClientSessionDelegate$createMessage$aop:()Lorg/jboss/jms/message/MessageProxy;
                         101: areturn
                        



                        • 9. Re: PrivilegedBlock location
                          Anil Saldanha Master

                          http://viewvc.jboss.org/cgi-bin/viewvc.cgi/messaging/branches/Branch_1_4/integration/AS5/etc/aop-messaging-client.xml?revision=4986&view=markup

                          This is the pointcut file. Giving me more pointers. I think I need to look for the aspect classes in the project to add the priv blocks.

                          • 10. Re: PrivilegedBlock location
                            Scott Stark Master

                            That won't help as the org.jboss.jms.client.container.SessionAspect which is being used for the ClientSession createMessage method for example is the target of the call, not the code that you see disassembled here. Its the joinpoint code generated by aop that should include a privileged block to allow for a getClassLoader permission assignment. Your correct to say this can be pushed to aop.

                            • 11. Re: PrivilegedBlock location
                              Flavia Rainone Master

                              Kabir is out on vacation. I'm taking a look at this now.

                              Actualy, I don't need to edit the weaved code. The fix should go in the GenericAspectFactory class, the one who makes the getClassLoader call. Plus, this factory is called only the first time the joinpoint gets executed, so it will generate a smaller impact if I put the security check there.

                              • 12. Re: PrivilegedBlock location
                                Clebert Suconic Master

                                Flavia,

                                are you doing a check like?

                                if (System.getSecurityManager() == null)
                                {
                                 cl = clazz.getClassLoader();
                                }
                                else
                                {
                                 ..... security block
                                }
                                



                                If you are.. all we need on JBM is an AOP release.


                                And this won't affect weaved code, right?

                                • 13. Re: PrivilegedBlock location
                                  Clebert Suconic Master

                                  Anil,

                                  Do you have a way to validate if AOP fixed the security issue you were seeing before AOP cuts the release?

                                  I mean.. is there a way to validate if there are more?

                                  • 14. Re: PrivilegedBlock location
                                    Anil Saldanha Master

                                     

                                    "clebert.suconic@jboss.com" wrote:
                                    Anil,

                                    Do you have a way to validate if AOP fixed the security issue you were seeing before AOP cuts the release?

                                    I mean.. is there a way to validate if there are more?


                                    There are security manager tests in JBAS that I am currently using. Basically the MDB related ones are choking.

                                    To really vet the messaging project, you will need to have tons of disparate examples running under a security manager, in your own project. :)