Axis2
  1. Axis2
  2. AXIS2-4493

Disengagement of module which is not globally engaged removes module's handlers from the global phase chain

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.5
    • Fix Version/s: None
    • Component/s: kernel
    • Labels:
      None
    • Environment:
      Tomcat 5.5.26, Axis2 war 1.4.1/1.5, Rampart 1.4

      Description

      We have an Axis2 1.4.1 runtime, which has Rampart module deployed, but not globally engaged (using the default axis2.xml from axis2.war)

      We have two services, both of them are secured (Rampart is engaged for each service by using <module ref="rampart"/> in services.xml).

      When Rampart is disengaged on one of the services, it happens that the Rampart handler is removed from the security phase. This practically disables access to the other secured service, since secured service requests are not handled by Rampart anymore (no handler in security phase) and when attempting to invoke it, Axis2 throws a mustUnderstand check failed for the security header:

      [ERROR] Must Understand check failed for header http://docs.oasis-open.org/wss/
      004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd : Security

      We examined the module disengage logic, and it seems that the handler removal takes place in AxisOperation#onDisengage(AxisModule module). The code there would remove module's handlers from global chains if the module is not engaged on service nor on gloabal level. What is the reason for this, and what if the module is engaged on another service (as in our case)?

      I attach two simple services you can simulate this with. They are based on Rampart's Tomcat policy sample, but are secured
      with a simple Username supporting token.

      Steps to simulate the problem:

      1. Deploy both services to Axis2 1.4.1/1.5 webapp having Rampart deployed (but not engaged in axis2.xml)
      2. Using the web administration intefrace, disengage Rampart module on one of the services
      3. Try to invoke the other service using the Client.java class (or by any other means)

      1. ut-secured-service2.aar
        4 kB
        Detelin Yordanov
      2. ut-secured-service1.zip
        6 kB
        Detelin Yordanov
      3. ut-secured-service1.aar
        4 kB
        Detelin Yordanov
      4. patch.txt
        0.7 kB
        Amila Chinthaka Suriarachchi

        Activity

        Hide
        Detelin Yordanov added a comment -

        Hi,
        This might work, but with proper synchronization so that it is not possible to engage this module in the time slot after the isModuleEnagedAtServiceLevel(module) invokation and prior to disengageModuleFromGlobalChain(module).

        Show
        Detelin Yordanov added a comment - Hi, This might work, but with proper synchronization so that it is not possible to engage this module in the time slot after the isModuleEnagedAtServiceLevel(module) invokation and prior to disengageModuleFromGlobalChain(module).
        Hide
        Amila Chinthaka Suriarachchi added a comment -

        What about adding a check like this?

        if (!config.isEngaged(module.getName()) && !config.isModuleEnagedAtServiceLevel(module))

        { PhaseResolver phaseResolver = new PhaseResolver(config); phaseResolver.disengageModuleFromGlobalChains(module); }

        public boolean isModuleEnagedAtServiceLevel(AxisModule axisModule){
        AxisServiceGroup axisServiceGroup = null;
        AxisService axisService = null;
        AxisOperation axisOperation = null;
        for (Iterator<AxisServiceGroup> serviceGroups = this.getServiceGroups(); serviceGroups.hasNext() {
        axisServiceGroup = serviceGroups.next();
        if (axisServiceGroup.isEngaged(axisModule))

        { return true; }

        else {
        for (Iterator<AxisService> services = axisServiceGroup.getServices(); services.hasNext() {
        axisService = services.next();
        if (axisService.isEngaged(axisModule))

        { return true; }

        else {
        for (Iterator<AxisOperation> operations = axisService.getOperations(); operations.hasNext(){
        axisOperation = operations.next();
        if (axisOperation.isEngaged(axisModule))

        { return true; }

        }
        }
        }
        }
        }
        return false;
        }

        so that we don't remove global handlers only if it is engaged to another service. if it is already engaged to another
        service then that is definitely a bug and need to fix.

        Show
        Amila Chinthaka Suriarachchi added a comment - What about adding a check like this? if (!config.isEngaged(module.getName()) && !config.isModuleEnagedAtServiceLevel(module)) { PhaseResolver phaseResolver = new PhaseResolver(config); phaseResolver.disengageModuleFromGlobalChains(module); } public boolean isModuleEnagedAtServiceLevel(AxisModule axisModule){ AxisServiceGroup axisServiceGroup = null; AxisService axisService = null; AxisOperation axisOperation = null; for (Iterator<AxisServiceGroup> serviceGroups = this.getServiceGroups(); serviceGroups.hasNext() { axisServiceGroup = serviceGroups.next(); if (axisServiceGroup.isEngaged(axisModule)) { return true; } else { for (Iterator<AxisService> services = axisServiceGroup.getServices(); services.hasNext() { axisService = services.next(); if (axisService.isEngaged(axisModule)) { return true; } else { for (Iterator<AxisOperation> operations = axisService.getOperations(); operations.hasNext() { axisOperation = operations.next(); if (axisOperation.isEngaged(axisModule)) { return true; } } } } } } return false; } so that we don't remove global handlers only if it is engaged to another service. if it is already engaged to another service then that is definitely a bug and need to fix.
        Hide
        Detelin Yordanov added a comment -

        I will, but are you sure this fix won't cause any backward incompatibility issues?

        Show
        Detelin Yordanov added a comment - I will, but are you sure this fix won't cause any backward incompatibility issues?
        Hide
        Amila Chinthaka Suriarachchi added a comment -

        Can you please attach a patch?

        Show
        Amila Chinthaka Suriarachchi added a comment - Can you please attach a patch?
        Hide
        Detelin Yordanov added a comment -

        Hi Amila,
        I tested with an Axis2 kernel jar compiled with the given patch and found out that it does not fix the problem. We need to comment out this logic on two more places in order to make it work:

        • AxisService#onDisengage()
        • AxisConfiguration#onDisengage()
        Show
        Detelin Yordanov added a comment - Hi Amila, I tested with an Axis2 kernel jar compiled with the given patch and found out that it does not fix the problem. We need to comment out this logic on two more places in order to make it work: AxisService#onDisengage() AxisConfiguration#onDisengage()
        Hide
        Detelin Yordanov added a comment -

        Hi,
        I just tested with the given patch, and got an assertion failure in org.apache.axis2.deployment.ModuleDisengagementTest#testOperationEngageOperationDisengage().

        Of course the test can be fixed to remove the assertion, but this makes me wonder what was the initial idea to have this handler removal logic and whether we are not going to break existing usage by removing it (this can happen if somebody has implemented a handler and does not check whether its module is engaged or not, but relies on Axis2 to remove it on disengagement).

        Show
        Detelin Yordanov added a comment - Hi, I just tested with the given patch, and got an assertion failure in org.apache.axis2.deployment.ModuleDisengagementTest#testOperationEngageOperationDisengage(). Of course the test can be fixed to remove the assertion, but this makes me wonder what was the initial idea to have this handler removal logic and whether we are not going to break existing usage by removing it (this can happen if somebody has implemented a handler and does not check whether its module is engaged or not, but relies on Axis2 to remove it on disengagement).
        Hide
        Detelin Yordanov added a comment -

        Yes, you are right. Modules which add operations to the service must be handled with care - these operations must be removed on module disengagement, but only if the module is not engaged on any other operation in the service.
        Because if other operations have this module engaged they will require its special operations to be available on the service.

        Show
        Detelin Yordanov added a comment - Yes, you are right. Modules which add operations to the service must be handled with care - these operations must be removed on module disengagement, but only if the module is not engaged on any other operation in the service. Because if other operations have this module engaged they will require its special operations to be available on the service.
        Hide
        Amila Chinthaka Suriarachchi added a comment -

        can you test with the attached patch?

        I think this similar problem can happen with operation disengage as well.

        //removing operations added at the time of module engagemnt
        HashMap<QName, AxisOperation> moduleOperations = module.getOperations();
        if (moduleOperations != null) {
        Iterator<AxisOperation> moduleOperations_itr = moduleOperations.values().iterator();
        while (moduleOperations_itr.hasNext())

        { AxisOperation operation = moduleOperations_itr.next(); service.removeOperation(operation.getName()); }

        }

        it removes the added module operations if one operation get disengaged.

        Show
        Amila Chinthaka Suriarachchi added a comment - can you test with the attached patch? I think this similar problem can happen with operation disengage as well. //removing operations added at the time of module engagemnt HashMap<QName, AxisOperation> moduleOperations = module.getOperations(); if (moduleOperations != null) { Iterator<AxisOperation> moduleOperations_itr = moduleOperations.values().iterator(); while (moduleOperations_itr.hasNext()) { AxisOperation operation = moduleOperations_itr.next(); service.removeOperation(operation.getName()); } } it removes the added module operations if one operation get disengaged.
        Hide
        Detelin Yordanov added a comment -

        Two test services secured with simple Username supporting token and the Java project for the first one (second is just the same, only name is changed in services.xml).

        Show
        Detelin Yordanov added a comment - Two test services secured with simple Username supporting token and the Java project for the first one (second is just the same, only name is changed in services.xml).

          People

          • Assignee:
            Unassigned
            Reporter:
            Detelin Yordanov
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development