Uploaded image for project: 'WSO2 Identity Server'
  1. WSO2 Identity Server
  2. IDENTITY-6208

Possible Connection Leak in RegistryRecoveryDataStore

    Details

    • Type: Patch
    • Status: Resolved
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: 5.1.0-GA, 5.2.0-GA, 5.3.0-GA
    • Fix Version/s: 5.4.0-M3
    • Component/s: identity-mgt
    • Labels:
      None
    • Estimated Complexity:
      Moderate
    • Test cases added:
      Yes

      Description

      Ref: https://github.com/wso2/carbon-identity-framework/blob/master/components/identity-mgt/org.wso2.carbon.identity.mgt/src/main/java/org/wso2/carbon/identity/mgt/store/RegistryRecoveryDataStore.java#L257-L310

      In the below segment, if the

          private void deleteOldResourcesIfFound(Registry registry, String userName, String secretKeyPath) {
      
              Collection collection = null;
              try {
                  collection = (Collection) registry.get(secretKeyPath.toLowerCase());
              } catch (RegistryException e) {
                  log.error("Error while deleting the old confirmation code. Unable to find data collection in registry." + e);
              }
      
              //Introduced property to fix resource not being introduced deleted when special characters are present.
              String userNameToValidate = userName;
              String useHashedUserName =
                      IdentityMgtConfig.getInstance().getProperty(USE_HASHED_USERNAME_PROPERTY);
              if (Boolean.parseBoolean(useHashedUserName)) {
                  String hashAlg = IdentityMgtConfig.getInstance().getProperty(USERNAME_HASH_ALG_PROPERTY);
                  try {
                      userNameToValidate = hashString(userName, hashAlg);
                  } catch (NoSuchAlgorithmException e) {
                      log.error("Invalid hash algorithm " + hashAlg, e);
                  }
              }
      
              try {
                  if (collection != null) {
                      String[] resources = collection.getChildren();
                      for (String resource : resources) {
                          String[] splittedResource = resource.split("___");
                          if (splittedResource.length == 3) {
                              //PRIMARY USER STORE
                              if (resource.contains("___" + userNameToValidate.toLowerCase() + "___")) {
      
                                  registry.beginTransaction();
                                  // Check whether the resource still exists for concurrent cases.
                                  if (registry.resourceExists(resource)) {
                                      registry.delete(resource);
                                      registry.commitTransaction();
                                  } else {
                                      // Already deleted by another thread. Do nothing.
                                      registry.rollbackTransaction();
                                      if (log.isDebugEnabled()) {
                                          log.debug("Confirmation code already deleted in path of resource : " + resource);
                                      }
                                  }
                              }
                          } else if (splittedResource.length == 2) {
                              //SECONDARY USER STORE. Resource is a collection.
                              deleteOldResourcesIfFound(registry, userName, resource);
                          }
                      }
                  }
              } catch (RegistryException e) {
                  log.error("Error while deleting the old confirmation code \n" + e);
              }
          }
      

      In the above segment,

        if (registry.resourceExists(resource)) {
               registry.delete(resource);
               registry.commitTransaction();
        }
      

      if the registry.delete() fails with an exception, database connections will not be closed. This mean lead to a connection leak since the connections used for registry transactions are not released since a commit or rollback doesnt happen,

      Possible fix,

              boolean isTransactionSucceeded = false;
              try {
                  if (collection != null) {
                      String[] resources = collection.getChildren();
                      for (String resource : resources) {
                          String[] splittedResource = resource.split("___");
                          if (splittedResource.length == 3) {
                              //PRIMARY USER STORE
                              if (resource.contains("___" + userNameToValidate.toLowerCase() + "___")) {
      
                                  registry.beginTransaction();
                                  // Check whether the resource still exists for concurrent cases.
                                  if (registry.resourceExists(resource)) {
                                      registry.delete(resource);
                                      isTransactionSucceeded = true;
                                  } else {
                                      // Already deleted by another thread. Do nothing.
                                      if (log.isDebugEnabled()) {
                                          log.debug("Confirmation code already deleted in path of resource : " + resource);
                                      }
                                  }
                              }
                          } else if (splittedResource.length == 2) {
                              //SECONDARY USER STORE. Resource is a collection.
                              deleteOldResourcesIfFound(registry, userName, resource);
                          }
                      }
                  }
              } catch (RegistryException e) {
                  log.error("Error while deleting the old confirmation code \n" + e);
              } finally {
                  try {
                      if (isTransactionSucceeded) {
                          registry.commitTransaction();
                      } else {
                          registry.rollbackTransaction();
                      }
                  } catch (RegistryException e) {
                      log.error("Error while processing registry transaction", e);
                  }
              }
      

        Attachments

          Activity

            People

            • Assignee:
              isura@wso2.com Isura Karunaratne
              Reporter:
              farasatha@wso2.com Farasath Ahamed
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: