Uploaded image for project: 'OpenAM'
  1. OpenAM
  2. OPENAM-13085

WSFederation Active Request Profile authentication request hangs on input-less scripted modules

    Details

    • Sprint:
      AM Sustaining Sprint 51
    • Story Points:
      2
    • Needs backport:
      No
    • Support Ticket IDs:
    • Needs QA verification:
      Yes
    • Functional tests:
      No
    • Are the reproduction steps defined?:
      Yes but I used my own steps. (If so, please add them in a new comment)

      Description

      Bug description

      The DefaultWsFedAuthenticator that used for authenticating WSFed and used mostly for the ActiveRequest profile will loop into infinite if there is no Callback from the authentication chains. This causes HIGH CPU spin as well as thread stuck in this

      For the non-returning and high CPU spinning thread, it is seen doing AuthContextLocal.getRequirements and AuthContextLocal.hasMoreRequirements :

       

              at com.sun.identity.authentication.server.AuthContextLocal.getRequirements(AuthContextLocal.java:597)
              at com.sun.identity.authentication.AuthContext.getRequirements(AuthContext.java:1151)
              at org.forgerock.openam.saml2.plugins.DefaultWsFedAuthenticator.authenticate(DefaultWsFedAuthenticator.java:54)
              at com.sun.identity.wsfederation.servlet.ActiveRequest.authenticateEndUser(ActiveRequest.java:288)
              at com.sun.identity.wsfederation.servlet.ActiveRequest.process(ActiveRequest.java:152)
              at com.sun.identity.wsfederation.servlet.WSFederationServlet.doPost(WSFederationServlet.java:104)
      
              at com.sun.identity.authentication.server.AuthContextLocal.hasMoreRequirements(AuthContextLocal.java:550)
              at com.sun.identity.authentication.AuthContext.hasMoreRequirements(AuthContext.java:1096)
              at org.forgerock.openam.saml2.plugins.DefaultWsFedAuthenticator.authenticate(DefaultWsFedAuthenticator.java:53)
              at com.sun.identity.wsfederation.servlet.ActiveRequest.authenticateEndUser(ActiveRequest.java:288)
              at com.sun.identity.wsfederation.servlet.ActiveRequest.process(ActiveRequest.java:152)
      

       The cause is that the Callback is empty (with just PagePropertiesCallback).

      How to reproduce the issue

      Details steps outlining how to recreate the issue (remove this text)

      1. The seen code below is standard and can be unit tested as it will run by itself
      2. If you use a realm /test and create a chain with a Script module that does not
        require any inputs then the above will hang.
      Expected behaviour
      Login successful and not hang
      
      Current behaviour
      Request hang and high cpu with modules that does not take any values
      

      Work around

      a) You may want to take DefaultWSFedAuthenticator and create a version that
      does not have those offending line

      b) Compile and bundle that into AM

      c) Goto WSFed IDP tab and change default class “org.forgerock.openam.saml2.plugins.DefaultWsFedAuthenticator” in the class settings under “Active Requestor Profile settings” to your custom class

      d) Restart and test.

      Code analysis

      org.forgerock.openam.saml2.plugin.DefaultWSFedAuthenticator.java
      ...            AuthContext authContext = new AuthContext(realm);
                  authContext.login(request, response);
      
                  while (authContext.hasMoreRequirements()) {
                      Callback[] callbacks = authContext.getRequirements();
                      if (callbacks == null || callbacks.length == 0) { // <-----BAD CODE
                          continue;
                      }
                      List<Callback> missing = new ArrayList<>();
                      for (Callback callback : callbacks) {
                          if (callback instanceof NameCallback) {
                              NameCallback nc = (NameCallback) callback;
                              nc.setName(username);
                          } else if (callback instanceof PasswordCallback) {
                              PasswordCallback pc = (PasswordCallback) callback;
                              pc.setPassword(password);
                          } else {
                              missing.add(callback);
                          }
                      }
      
                      if (missing.size() > 0) {
                          throw ActiveRequestorException.newSenderException("unableToAuthenticate");
                      }
                      authContext.submitRequirements(callbacks);
                  }
      

      The issue is the extra check

                     if (callbacks == null || callbacks.length == 0) {
                          continue;
                      }
      

      For Authentication modules that is no callbacks (other than PagePropertiesCallback),
      this cause the loop between hasMoreRequirements() and getRequirements. So those 3 lines are not needed

      Fix is to just have

       if (callbacks == null) {
          continue;
      }
      

        Attachments

          Activity

            People

            • Assignee:
              chee-weng.chea C-Weng C
              Reporter:
              chee-weng.chea C-Weng C
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: