CVE-2020-5229 (Java, published 2020-01-30T20:15Z)
Description: value: Opencast before 8.1 stores passwords using the rather outdated and cryptographically insecure MD5 hash algorithm. Furthermore, the hashes are salted using the username instead of a random salt, causing hashes for users with the same username and password to collide which is problematic especially for popular users like the default `admin` user. This essentially means that for an attacker, it might be feasible to reconstruct a user's password given access to these hashes. Note that attackers needing access to the hashes means that they must gain access to the database in which these are stored first to be able to start cracking the passwords. The problem is addressed in Opencast 8.1 which now uses the modern and much stronger bcrypt password hashing algorithm for storing passwords. Note, that old hashes remain MD5 until the password is updated. For a list of users whose password hashes are stored using MD5, take a look at the `/user-utils/users/md5.json` REST endpoint.
Severity: MEDIUM (CVSS v3 base score: 8.1)
Weakness: CWE-327 \u2014 Use of a Broken or Risky Cryptographic Algorithm
The product uses a broken or risky cryptographic algorithm or protocol.
Repository: https://github.com/opencast/opencast
Files changed:
etc/security/mh_default_org.xml
modules/common-jpa-impl/src/main/java/org/opencastproject/security/impl/jpa/JpaUser.java
modules/kernel/src/main/java/org/opencastproject/kernel/security/CustomPasswordEncoder.java
modules/userdirectory/src/main/java/org/opencastproject/userdirectory/JpaUserAndRoleProvider.java
modules/userdirectory/src/main/java/org/opencastproject/userdirectory/endpoint/UserEndpoint.java
modules/userdirectory/src/test/java/org/opencastproject/userdirectory/JpaUserProviderTest.java
Commit message: Replace MD5 with bcrypt for password hashing

User passwords are stored in the database using the rather outdated and
cryptographically insecure MD5 hash algorithm. Furthermore, the hashes
are salted using the username instead of a random salt, causing hashes
for users with the same username and password to collide which is
problematic especially for popular users like the default admin user.

This essentially means that for an attacker, it might be feasible to
reconstruct a user's password given access to these hashes.

Note that attackers needing access to the hashes means that they must
gain access to the database in which these are stored first to be able
to start cracking the passwords.
Patches

The patch makes Opencast now uses the modern and much stronger bcrypt
password hashing algorithm for storing passwords.  Note, that old hashes
remain MD5 until the password is updated.

For a list of users whose password hashes are stored using MD5, the REST endpoint `/user-utils/users/md5.json` is added.


Vulnerable code:
      <sec:password-encoder hash="md5"><sec:salt-source user-property="username" /></sec:password-encoder>
/**
 * Licensed to The Apereo Foundation under one or more contributor license
 * agreements. See the NOTICE file distributed with this work for additional
 * information regarding copyright ownership.
 *
 *
 * The Apereo Foundation licenses this file to you under the Educational
 * Community License, Version 2.0 (the "License"); you may not use this file
 * except in compliance with the License. You may obtain a copy of the License
 * at:
 *
 *   http://opensource.org/licenses/ecl2.txt
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
 * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
 * License for the specific language governing permissions and limitations under
 * the License.
 *
 */

package org.opencastproject.util;

import org.apache.commons.codec.digest.DigestUtils;

/**
 * A password encoder that md5 hashes a password with a salt.
 */
public final class PasswordEncoder {

  /**
   * Private constructor to disallow construction of this utility class.
   */
  private PasswordEncoder() {
  }

  /**
   * Encode a clear text password.
   *
   * @param clearText
   *          the password
   * @param salt
   *          the salt. See http://en.wikipedia.org/wiki/Salt_%28cryptography%29
   * @return the encoded password
   * @throws IllegalArgumentException
   *           if clearText or salt are null
   */
  public static String encode(String clearText, Object salt) throws IllegalArgumentException {
    if (clearText == null || salt == null)
      throw new IllegalArgumentException("clearText and salt must not be null");
    return DigestUtils.md5Hex(clearText + "{" + salt.toString() + "}");
  }

}
import org.opencastproject.util.PasswordEncoder;
    String encodedPassword = PasswordEncoder.encode(user.getPassword(), user.getUsername());
      encodedPassword = PasswordEncoder.encode(user.getPassword(), user.getUsername());
import org.opencastproject.util.PasswordEncoder;
    assertEquals(PasswordEncoder.encode(user.getPassword(), user.getUsername()), loadUser.getPassword());
    assertEquals(PasswordEncoder.encode(user.getPassword(), user.getUsername()), loadUser.getPassword());
    assertEquals(PasswordEncoder.encode(newPassword, user.getUsername()), loadUpdatedUser.getPassword());

Fixed code:

  <bean id="passwordEncoder" class="org.opencastproject.kernel.security.CustomPasswordEncoder" />

      <sec:password-encoder ref="passwordEncoder">
        <!-- This salt is used only for decoding legacy MD5 hased passwords -->
        <sec:salt-source user-property="username" />
      </sec:password-encoder>
  @NamedQuery(name = "User.findInsecureHash",
              query = "select u from JpaUser u where length(u.password) = 32 and u.organization.id = :org"),
/**
 * Licensed to The Apereo Foundation under one or more contributor license
 * agreements. See the NOTICE file distributed with this work for additional
 * information regarding copyright ownership.
 *
 *
 * The Apereo Foundation licenses this file to you under the Educational
 * Community License, Version 2.0 (the "License"); you may not use this file
 * except in compliance with the License. You may obtain a copy of the License
 * at:
 *
 *   http://opensource.org/licenses/ecl2.txt
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
 * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
 * License for the specific language governing permissions and limitations under
 * the License.
 *
 */

package org.opencastproject.kernel.security;

import org.apache.commons.codec.digest.DigestUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.security.authentication.encoding.PasswordEncoder;
import org.springframework.security.crypto.bcrypt.BCrypt;

/**
 *
 */
public class CustomPasswordEncoder implements PasswordEncoder {
  private Logger logger = LoggerFactory.getLogger(CustomPasswordEncoder.class);

  /**
   * Encode the raw password for storage using BCrypt.
   * @param rawPassword raw password to encrypt/hash
   * @return hashed password
   */
  public String encodePassword(final String rawPassword) {
    return encodePassword(rawPassword, null);
  }

  /**
   * Encode the raw password for storage using BCrypt.
   * @param rawPassword raw password to encrypt/hash
   * @param ignored This parameter will not be used. A random salt is generated by BCrypt.
   * @return hashed password
   */
  @Override
  public String encodePassword(final String rawPassword, final Object ignored) {
    return BCrypt.hashpw(rawPassword, BCrypt.gensalt());
  }

  /**
   * Verify the encoded password obtained from storage matches the submitted raw
   * password after it too is encoded. Returns true if the passwords match, false if
   * they do not. The stored password itself is never decoded.
   *
   * @param rawPassword the raw password to encode and match
   * @param encodedPassword the encoded password from storage to compare with
   * @return true if the raw password, after encoding, matches the encoded password from storage
   */
  @Override
  public boolean isPasswordValid(String encodedPassword, String rawPassword, Object salt) {
    // Test MD5 encoded hash
    if (encodedPassword.length() == 32) {
      final String hash = md5Encode(rawPassword, salt);
      logger.debug("Checking md5 hashed password '{}' against encoded password '{}'", hash, encodedPassword);
      return hash.equals(encodedPassword);
    }

    // Test BCrypt encoded hash
    logger.debug("Verifying BCrypt hash {}", encodedPassword);
    return BCrypt.checkpw(rawPassword, encodedPassword);
  }

  /**
   * Encode a clear text password using Opencast's legacy MD5 based hashing with salt.
   * The username was used as salt for this.
   *
   * @param clearText
   *          the password
   * @param salt
   *          the salt
   * @return the hashed password
   * @throws IllegalArgumentException
   *           if clearText or salt are null
   */
  public static String md5Encode(String clearText, Object salt) throws IllegalArgumentException {
    if (clearText == null || salt == null)
      throw new IllegalArgumentException("clearText and salt must not be null");
    return DigestUtils.md5Hex(clearText + "{" + salt.toString() + "}");
  }
}
import org.opencastproject.kernel.security.CustomPasswordEncoder;
import javax.persistence.TypedQuery;
  /** Password encoder for storing user passwords */
  private CustomPasswordEncoder passwordEncoder = new CustomPasswordEncoder();

  /**
   * List all users with insecure password hashes
   */
  public List<User> findInsecurePasswordHashes() {
    final String orgId = securityService.getOrganization().getId();
    EntityManager em = null;
    try {
      em = emf.createEntityManager();
      TypedQuery<User> q = em.createNamedQuery("User.findInsecureHash", User.class);
      q.setParameter("org", orgId);
      return q.getResultList();
    } finally {
      if (em != null)
        em.close();
    }
  }

    addUser(user, false);
  }

  /**
   * Adds a user to the persistence
   *
   * @param user
   *          the user to add
   * @param passwordEncoded
   *          if the password is already encoded or should be encoded
   *
   * @throws org.opencastproject.security.api.UnauthorizedException
   *          if the user is not allowed to create other user with the given roles
   */
  public void addUser(JpaUser user, final boolean passwordEncoded) throws UnauthorizedException {
    String encodedPassword = passwordEncoded ? user.getPassword() : passwordEncoder.encodePassword(user.getPassword());
    return updateUser(user, false);
  }

  /**
   * Updates a user to the persistence
   *
   * @param user
   *          the user to save
   * @param passwordEncoded
   *          if the password is already encoded or should be encoded
   * @throws NotFoundException
   * @throws org.opencastproject.security.api.UnauthorizedException
   *          if the current user is not allowed to update user with the given roles
   */
  public User updateUser(JpaUser user, final boolean passwordEncoded) throws NotFoundException, UnauthorizedException {
      if (passwordEncoded) {
        encodedPassword = user.getPassword();
      } else {
        encodedPassword = passwordEncoder.encodePassword(user.getPassword());
      }
  @GET
  @Path("users/md5.json")
  @Produces(MediaType.APPLICATION_JSON)
  @RestQuery(
      name = "users-with-insecure-hashing",
      description = "Returns a list of users which passwords are stored using MD5 hashes",
      returnDescription = "Returns a JSON representation of the list of matching user accounts",
      reponses = {
      @RestResponse(
          responseCode = SC_OK,
          description = "The user accounts.")
  })
  public JaxbUserList getUserWithInsecurePasswordHashingAsJson() {
    JaxbUserList userList = new JaxbUserList();
    for (User user: jpaUserAndRoleProvider.findInsecurePasswordHashes()) {
      userList.add(user);
    }
    return userList;
  }

import org.opencastproject.kernel.security.CustomPasswordEncoder;
  private CustomPasswordEncoder passwordEncoder = new CustomPasswordEncoder();
    assertTrue(passwordEncoder.isPasswordValid(loadUser.getPassword(), user.getPassword(), null));
    assertTrue(passwordEncoder.isPasswordValid(loadUser.getPassword(), user.getPassword(), null));
    assertTrue(passwordEncoder.isPasswordValid(loadUpdatedUser.getPassword(), newPassword, null));