12 Replies Latest reply on Nov 21, 2008 2:53 PM by Ales Justin

    JBMICROCONT-385 - Privileged actions for GenericBeanFactory.

    Adrian Brock Master

      I like the way you create a blocker issue when you haven't even discussed it in the
      forums. :-)

      It could just as easily have been an aop or ejb3 problem since it is they
      that are trying to createBean() and the caller has no rights.

      In fact, the initial issue is definetly an MC problem.

      It shouldn't be trying to use the TCL at all which is where it is failing.
      The reason it is trying to use it, is because the caller
      doesn't have the getClassLoader permission to retrieve the classloader
      from the KernelControllerContext.
      So that should be in a privileged action.

      But equally, I've gone beyond that basic fix, since in my opionion, the
      createBean() should run under the privileges of whoever registered the
      GenericBeanFactory in the MC.

      So I've changed it to do that as well.

      This required a change to AbstractKernelControllerContext::getAccessControlContext()
      to make it public.

      Equally, since that information should be protected against misuse,
      I've required the caller to have
      new SecurityPermission("getAccessControlContext");

      I'm not sure if that is the correct permission or whether this should be
      one of the fine grained permissions described on a different thread.

      You can find the test(s) for these in
      org.jboss.test.kernel.controller.test.GenericBeanFactoryAccessControlTestCase

      The first test registers a GenericBeanFactory in a context that
      can access the System properties. It shows the constructor
      can still do that when you use createBean().
      It also tests that createBean() uses the correct classloader (not the TCL).

      The second test is similar, except the context registering the GBF
      doesn't have the rights to get the system properties.

      NOTE: Just because createBean() now restores your privileges from
      GBF registration time, that doesn't mean you shouldn't use privilege blocks.
      Who knows in what security context somebody will create a GBF for your bean? :-)

        • 1. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
          Ales Justin Master

           

          "adrian@jboss.org" wrote:
          I like the way you create a blocker issue when you haven't even discussed it in the
          forums. :-)

          Had to get your attention somehow. :-)

          OTOH, it was discussed all over the place - jboss-dev ml, internal emails, ...
          So not really a huge blocker surprise there. ;-)

          "adrian@jboss.org" wrote:

          It could just as easily have been an aop or ejb3 problem since it is they
          that are trying to createBean() and the caller has no rights.

          That's what I was explaining to Anil.
          Eventually decided to push the issue to you,
          since I still somehow felt it was ours to deal with -
          and it turned out to be true. :-)

          • 2. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
            Adrian Brock Master

             

            "adrian@jboss.org" wrote:

            This required a change to AbstractKernelControllerContext::getAccessControlContext()
            to make it public.


            We might want to create an interface for this contract since it is not good
            that the GBF depends upon a particular implementation of the KernelControllerContext
            interface?

            • 3. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
              Ales Justin Master

               

              "adrian@jboss.org" wrote:

              We might want to create an interface for this contract since it is not good
              that the GBF depends upon a particular implementation of the KernelControllerContext
              interface?

              Probably doesn't belong to KCC,
              more like our InvokeDispatchContext?

              What would be a good name? :-)
              Considering probable future usages/additions.
              e.g. AccessControlContextAware?

              • 4. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                Adrian Brock Master

                 

                "alesj" wrote:
                "adrian@jboss.org" wrote:
                I like the way you create a blocker issue when you haven't even discussed it in the
                forums. :-)

                Had to get your attention somehow. :-)

                OTOH, it was discussed all over the place - jboss-dev ml,


                Only reference I see is an embedded comment in the VFSPermissions thread
                by Anil saying that he is no expert on AOP so he'll create a blocker issue for me
                to investigate the problem.

                How does he know there is even a real problem if he hasn't investigated it?


                internal emails, ...


                Ah! Sorry, my psychic powers aren't working very well today. :-)

                • 5. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                  Anil Saldanha Master

                   

                  "adrian@jboss.org" wrote:

                  Only reference I see is an embedded comment in the VFSPermissions thread
                  by Anil saying that he is no expert on AOP so he'll create a blocker issue for me
                  to investigate the problem.


                  I just created an issue. Assigning priority to the JIRA issue is Ales' job. ;) There is an api (createBean) and an attempt to get the CL (from mc). So looking from the stack trace, I evaluated that the MC should be doing a priv op of the CL. The expectation from mc api for AOP is to create a bean. To make that happen, is the mc's api (createBean) job.

                  You can either force all users of the mc api to have the getCL permission or have it internally. The mc designers have to tell us. :)



                  • 6. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                    Adrian Brock Master

                     

                    "alesj" wrote:
                    "adrian@jboss.org" wrote:

                    We might want to create an interface for this contract since it is not good
                    that the GBF depends upon a particular implementation of the KernelControllerContext
                    interface?

                    Probably doesn't belong to KCC,
                    more like our InvokeDispatchContext?

                    What would be a good name? :-)


                    Don't really care about names, except when they obviously wrong....


                    Considering probable future usages/additions.
                    e.g. AccessControlContextAware?


                    "Aware" is usually for when you want a callback, e.g.
                    if the controller was going to do a setAccessControlContext()
                    Like having a bean that is KernelControllerContextAware
                    meaning there is a setKernelControllerContext().

                    Since this is just a getter interface, why not call it something like
                    ControllerContextAccessControlContextProvider? Its a mouthful
                    so you can probably shorten it? ;-)
                    Provider is the usual name if something is not
                    building, generating, loading, locating, etc.

                    Speaking of "locating", who is using the horrible KernelLocator
                    that has sneaked into the MC project despite all my explanations
                    about this being the wrong way to do it?

                    Basically, there never has been a singleton kernel in the MC (or the old JMX kernel
                    for that matter - although there we hacked it with the locateJBoss() method).

                    Having a class trying to emulate it, is bound to break
                    when somebody does create more than one.

                    • 7. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                      Adrian Brock Master

                       

                      "adrian@jboss.org" wrote:

                      Having a class trying to emulate it, is bound to break
                      when somebody does create more than one.


                      Its also a security issue since there's no control on who can set the singleton,
                      only on who can access it.

                      • 8. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                        Ales Justin Master

                         

                        "adrian@jboss.org" wrote:

                        Speaking of "locating", who is using the horrible KernelLocator
                        that has sneaked into the MC project despite all my explanations
                        about this being the wrong way to do it?

                        Basically, there never has been a singleton kernel in the MC (or the old JMX kernel
                        for that matter - although there we hacked it with the locateJBoss() method).

                        Having a class trying to emulate it, is bound to break
                        when somebody does create more than one.

                        I'll give you zero attempts to guess. ;-)

                        Unless Scott can prove that Diesler was twisting his arm,
                        there is no excuse for putting this in. :-)

                         * @author Scott.Stark@jboss.org
                         * @version $Revision:$
                         */
                        public class KernelLocator implements KernelControllerContextAware
                        


                        • 9. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                          Adrian Brock Master

                           

                          "alesj" wrote:

                          Unless Scott can prove that Diesler was twisting his arm,
                          there is no excuse for putting this in. :-)


                          The correct fix is store this on whatever is really a singleton in your code,
                          e.g. the webserver or something representing the ejb subsystem, etc.

                          Then later when you realise its not really a singleton (e.g. two webservers in
                          the same JVM) there's only your project that has to unwind the braindeath. ;-)

                          • 10. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                            Scott Stark Master

                            I don't know if anything is using KernelLocator. It was put in to avoid duplicates elsewhere, but ws is still using their own:

                            [599][valkyrie: deployers]$ grep -r KernelLocator .
                            ./jbossws.deployer/META-INF/jbossws-container-jboss-beans.xml: <bean name="WSKernelLocator" class="org.jboss.wsf.spi.util.KernelLocator">
                            



                            • 11. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                              Anil Saldanha Master

                              Did the fix for JBMICROCONT-385 get into any mc release that is being used by AS5 as the sec mgr tests are still failing on this one?

                              • 12. Re: JBMICROCONT-385 - Privileged actions for GenericBeanFact
                                Ales Justin Master

                                 

                                "anil.saldhana@jboss.com" wrote:
                                Did the fix for JBMICROCONT-385 get into any mc release that is being used by AS5 as the sec mgr tests are still failing on this one?

                                There was no release yet with this included.
                                It should be early next week.