From da280307edefe3dce5d3836f502c9fc3daa4ce6f Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Fri, 25 Jul 2014 08:23:32 +0000
Subject: [PATCH] Code cleanup for password storage schemes. Used AutoRefactor. Reduced variable scopes.

---
 opends/src/server/org/opends/server/extensions/PBKDF2PasswordStorageScheme.java |  368 +++++++++++++++-------------------------------------
 1 files changed, 105 insertions(+), 263 deletions(-)

diff --git a/opends/src/server/org/opends/server/extensions/PBKDF2PasswordStorageScheme.java b/opends/src/server/org/opends/server/extensions/PBKDF2PasswordStorageScheme.java
index 70dc632..56662cb 100644
--- a/opends/src/server/org/opends/server/extensions/PBKDF2PasswordStorageScheme.java
+++ b/opends/src/server/org/opends/server/extensions/PBKDF2PasswordStorageScheme.java
@@ -21,15 +21,15 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2013 ForgeRock AS.
+ *      Copyright 2013-2014 ForgeRock AS.
  */
 package org.opends.server.extensions;
 
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.security.spec.KeySpec;
 import java.util.Arrays;
 import java.util.List;
-import java.security.spec.KeySpec;
 
 import javax.crypto.SecretKeyFactory;
 import javax.crypto.spec.PBEKeySpec;
@@ -48,7 +48,7 @@
 import static org.opends.messages.ExtensionMessages.*;
 import static org.opends.server.extensions.ExtensionsConstants.*;
 import static org.opends.server.loggers.debug.DebugLogger.*;
-import static org.opends.server.util.StaticUtils.getExceptionMessage;
+import static org.opends.server.util.StaticUtils.*;
 
 /**
  * This class defines a Directory Server password storage scheme based on the
@@ -62,14 +62,10 @@
   extends PasswordStorageScheme<PBKDF2PasswordStorageSchemeCfg>
   implements ConfigurationChangeListener<PBKDF2PasswordStorageSchemeCfg>
 {
-  /**
-   * The tracer object for the debug logger.
-   */
+  /** The tracer object for the debug logger. */
   private static final DebugTracer TRACER = getTracer();
 
-  /**
-   * The fully-qualified name of this class.
-   */
+  /** The fully-qualified name of this class. */
   private static final String CLASS_NAME =
        "org.opends.server.extensions.PBKDF2PasswordStorageScheme";
 
@@ -80,19 +76,19 @@
    */
   private static final int NUM_SALT_BYTES = 8;
 
-  // The number of bytes the SHA-1 algorithm produces
+  /** The number of bytes the SHA-1 algorithm produces. */
   private static final int SHA1_LENGTH = 20;
 
-  // The factory used to generate the PBKDF2 hashes.
+  /** The factory used to generate the PBKDF2 hashes. */
   private SecretKeyFactory factory;
 
-  // The lock used to provide threadsafe access to the message digest.
-  private Object factoryLock;
+  /** The lock used to provide thread-safe access to the message digest. */
+  private final Object factoryLock = new Object();
 
-  // The secure random number generator to use to generate the salt values.
+  /** The secure random number generator to use to generate the salt values. */
   private SecureRandom random;
 
-  // The current configuration for this storage scheme.
+  /** The current configuration for this storage scheme. */
   private volatile PBKDF2PasswordStorageSchemeCfg config;
 
 
@@ -106,17 +102,12 @@
     super();
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public void initializePasswordStorageScheme(
                    PBKDF2PasswordStorageSchemeCfg configuration)
          throws ConfigException, InitializationException
   {
-    factoryLock = new Object();
     try
     {
       random = SecureRandom.getInstance(SECURE_PRNG_SHA1);
@@ -131,24 +122,17 @@
     config.addPBKDF2ChangeListener(this);
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
+  /** {@inheritDoc} */
+  @Override
   public boolean isConfigurationChangeAcceptable(
                       PBKDF2PasswordStorageSchemeCfg configuration,
                       List<Message> unacceptableReasons)
   {
-    // The configuration will always be acceptable.
     return true;
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
+  /** {@inheritDoc} */
+  @Override
   public ConfigChangeResult applyConfigurationChange(
       PBKDF2PasswordStorageSchemeCfg configuration)
   {
@@ -156,60 +140,22 @@
     return new ConfigChangeResult(ResultCode.SUCCESS, false);
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public String getStorageSchemeName()
   {
     return STORAGE_SCHEME_NAME_PBKDF2;
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public ByteString encodePassword(ByteSequence plaintext)
          throws DirectoryException
   {
     byte[] saltBytes      = new byte[NUM_SALT_BYTES];
-    byte[] digestBytes;
-    char[] plaintextChars = null;
     int    iterations     = config.getPBKDF2Iterations();
 
-    synchronized(factoryLock)
-    {
-      try
-      {
-        random.nextBytes(saltBytes);
-
-        plaintextChars = plaintext.toString().toCharArray();
-        KeySpec spec = new PBEKeySpec(plaintextChars,
-            saltBytes, iterations, SHA1_LENGTH * 8);
-        digestBytes = factory.generateSecret(spec).getEncoded();
-      }
-      catch (Exception e)
-      {
-        if (debugEnabled())
-        {
-          TRACER.debugCaught(DebugLogLevel.ERROR, e);
-        }
-
-        Message message = ERR_PWSCHEME_CANNOT_ENCODE_PASSWORD.get(
-            CLASS_NAME, getExceptionMessage(e));
-        throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
-                                     message, e);
-      }
-      finally
-      {
-        if (plaintextChars != null)
-          Arrays.fill(plaintextChars, '0');
-      }
-    }
+    byte[] digestBytes = getDigestBytes(plaintext, saltBytes, iterations);
     // Append the salt to the hashed value and base64-the whole thing.
     byte[] hashPlusSalt = new byte[digestBytes.length + NUM_SALT_BYTES];
 
@@ -217,64 +163,34 @@
     System.arraycopy(saltBytes, 0, hashPlusSalt, digestBytes.length,
                      NUM_SALT_BYTES);
 
-    StringBuilder sb = new StringBuilder();
-    sb.append(Integer.toString(iterations));
-    sb.append(':');
-    sb.append(Base64.encode(hashPlusSalt));
-    return ByteString.valueOf(sb.toString());
+    return ByteString.valueOf(iterations + ':' + Base64.encode(hashPlusSalt));
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public ByteString encodePasswordWithScheme(ByteSequence plaintext)
          throws DirectoryException
   {
-    StringBuilder buffer = new StringBuilder();
-    buffer.append('{');
-    buffer.append(STORAGE_SCHEME_NAME_PBKDF2);
-    buffer.append('}');
-
-    buffer.append(encodePassword(plaintext));
-
-    return ByteString.valueOf(buffer.toString());
+    return ByteString.valueOf('{' + STORAGE_SCHEME_NAME_PBKDF2 + '}'
+        + encodePassword(plaintext));
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public boolean passwordMatches(ByteSequence plaintextPassword,
                                  ByteSequence storedPassword)
   {
-
-    // Split the iterations from the stored value (separated by a ":")
+    // Split the iterations from the stored value (separated by a ':')
     // Base64-decode the remaining value and take the last 8 bytes as the salt.
-    int iterations;
-    byte[] saltBytes;
-    byte[] digestBytes = new byte[SHA1_LENGTH];
-    int saltLength = 0;
     try
     {
       String stored = storedPassword.toString();
-      int stored_length = stored.length();
-      int pos = 0;
-      while (pos < stored_length && stored.charAt(pos) != ':')
-      {
-        pos++;
-      }
-      if (pos >= (stored_length - 1) || pos == 0)
-        throw new Exception();
+      int pos = stored.indexOf(':');
 
-      iterations = Integer.parseInt(stored.substring(0, pos));
+      final int iterations = Integer.parseInt(stored.substring(0, pos));
       byte[] decodedBytes = Base64.decode(stored.substring(pos + 1));
 
-      saltLength = decodedBytes.length - SHA1_LENGTH;
+      final int saltLength = decodedBytes.length - SHA1_LENGTH;
       if (saltLength <= 0)
       {
         Message message =
@@ -283,10 +199,12 @@
         ErrorLogger.logError(message);
         return false;
       }
-      saltBytes = new byte[saltLength];
+
+      final byte[] digestBytes = new byte[SHA1_LENGTH];
+      final byte[] saltBytes = new byte[saltLength];
       System.arraycopy(decodedBytes, 0, digestBytes, 0, SHA1_LENGTH);
-      System.arraycopy(decodedBytes, SHA1_LENGTH, saltBytes, 0,
-                       saltLength);
+      System.arraycopy(decodedBytes, SHA1_LENGTH, saltBytes, 0, saltLength);
+      return encodeAndMatch(plaintextPassword, saltBytes, digestBytes, iterations);
     }
     catch (Exception e)
     {
@@ -300,18 +218,19 @@
       ErrorLogger.logError(message);
       return false;
     }
+  }
 
-
+  private boolean encodeAndMatch(ByteSequence plaintextPassword,
+      final byte[] saltBytes, byte[] digestBytes, int iterations)
+  {
     // Use the salt to generate a digest based on the provided plain-text value.
     int plainBytesLength = plaintextPassword.length();
-    byte[] plainPlusSalt = new byte[plainBytesLength + saltLength];
+    byte[] plainPlusSalt = new byte[plainBytesLength + saltBytes.length];
     plaintextPassword.copyTo(plainPlusSalt);
-    System.arraycopy(saltBytes, 0, plainPlusSalt, plainBytesLength,
-                     saltLength);
+    System.arraycopy(saltBytes, 0, plainPlusSalt, plainBytesLength, saltBytes.length);
 
-    byte[] userDigestBytes;
+
     char[] plaintextChars = null;
-
     synchronized (factoryLock)
     {
       try
@@ -320,7 +239,8 @@
         KeySpec spec = new PBEKeySpec(
             plaintextChars, saltBytes,
             iterations, SHA1_LENGTH * 8);
-        userDigestBytes = factory.generateSecret(spec).getEncoded();
+        final byte[] userDigestBytes = factory.generateSecret(spec).getEncoded();
+        return Arrays.equals(digestBytes, userDigestBytes);
       }
       catch (Exception e)
       {
@@ -328,57 +248,52 @@
         {
           TRACER.debugCaught(DebugLogLevel.ERROR, e);
         }
-
         return false;
       }
       finally
       {
         if (plaintextChars != null)
+        {
           Arrays.fill(plaintextChars, '0');
+        }
       }
     }
-
-    return Arrays.equals(digestBytes, userDigestBytes);
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public boolean supportsAuthPasswordSyntax()
   {
-    // This storage scheme does support the authentication password syntax.
     return true;
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public String getAuthPasswordSchemeName()
   {
     return AUTH_PASSWORD_SCHEME_NAME_PBKDF2;
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public ByteString encodeAuthPassword(ByteSequence plaintext)
          throws DirectoryException
   {
     byte[] saltBytes      = new byte[NUM_SALT_BYTES];
-    byte[] digestBytes;
-    char[] plaintextChars = null;
     int    iterations     = config.getPBKDF2Iterations();
+    byte[] digestBytes = getDigestBytes(plaintext, saltBytes, iterations);
 
-    synchronized(factoryLock)
+    // Encode and return the value.
+    return ByteString.valueOf(AUTH_PASSWORD_SCHEME_NAME_PBKDF2 + '$'
+        + iterations + ':' + Base64.encode(saltBytes) + '$'
+        + Base64.encode(digestBytes));
+  }
+
+  private byte[] getDigestBytes(ByteSequence plaintext, byte[] saltBytes,
+      int iterations) throws DirectoryException
+  {
+    char[] plaintextChars = null;
+    synchronized (factoryLock)
     {
       try
       {
@@ -388,7 +303,7 @@
         KeySpec spec = new PBEKeySpec(
             plaintextChars, saltBytes,
             iterations, SHA1_LENGTH * 8);
-        digestBytes = factory.generateSecret(spec).getEncoded();
+        return factory.generateSecret(spec).getEncoded();
       }
       catch (Exception e)
       {
@@ -405,48 +320,25 @@
       finally
       {
         if (plaintextChars != null)
+        {
           Arrays.fill(plaintextChars, '0');
+        }
       }
     }
-    // Encode and return the value.
-    StringBuilder authPWValue = new StringBuilder();
-    authPWValue.append(AUTH_PASSWORD_SCHEME_NAME_PBKDF2);
-    authPWValue.append('$');
-    authPWValue.append(Integer.toString(iterations));
-    authPWValue.append(':');
-    authPWValue.append(Base64.encode(saltBytes));
-    authPWValue.append('$');
-    authPWValue.append(Base64.encode(digestBytes));
-
-    return ByteString.valueOf(authPWValue.toString());
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public boolean authPasswordMatches(ByteSequence plaintextPassword,
                                      String authInfo, String authValue)
   {
-    byte[] saltBytes;
-    byte[] digestBytes;
-    int    iterations;
-
     try
     {
-      int pos = 0;
-      int length = authInfo.length();
-      while (pos < length && authInfo.charAt(pos) != ':')
-      {
-        pos++;
-      }
-      if (pos >= (length - 1) || pos == 0)
-        throw new Exception();
-      iterations = Integer.parseInt(authInfo.substring(0, pos));
-      saltBytes   = Base64.decode(authInfo.substring(pos + 1));
-      digestBytes = Base64.decode(authValue);
+      int pos = authInfo.indexOf(':');
+      int iterations = Integer.parseInt(authInfo.substring(0, pos));
+      byte[] saltBytes   = Base64.decode(authInfo.substring(pos + 1));
+      byte[] digestBytes = Base64.decode(authValue);
+      return encodeAndMatch(plaintextPassword, saltBytes, digestBytes, iterations);
     }
     catch (Exception e)
     {
@@ -454,66 +346,19 @@
       {
         TRACER.debugCaught(DebugLogLevel.ERROR, e);
       }
-
       return false;
     }
-
-
-    int plainBytesLength = plaintextPassword.length();
-    byte[] plainPlusSalt = new byte[plainBytesLength + saltBytes.length];
-    plaintextPassword.copyTo(plainPlusSalt);
-    System.arraycopy(saltBytes, 0, plainPlusSalt, plainBytesLength,
-                     saltBytes.length);
-
-    byte[] userDigestBytes;
-    char[] plaintextChars = null;
-
-    synchronized (factoryLock)
-    {
-      try
-      {
-        plaintextChars = plaintextPassword.toString().toCharArray();
-        KeySpec spec = new PBEKeySpec(
-            plaintextChars, saltBytes,
-            iterations, SHA1_LENGTH * 8);
-        userDigestBytes = factory.generateSecret(spec).getEncoded();
-      }
-      catch (Exception e)
-      {
-        if (debugEnabled())
-        {
-          TRACER.debugCaught(DebugLogLevel.ERROR, e);
-        }
-
-        return false;
-      }
-      finally
-      {
-        if (plaintextChars != null)
-          Arrays.fill(plaintextChars, '0');
-      }
-    }
-
-    return Arrays.equals(digestBytes, userDigestBytes);
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public boolean isReversible()
   {
     return false;
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public ByteString getPlaintextValue(ByteSequence storedPassword)
          throws DirectoryException
   {
@@ -522,12 +367,8 @@
     throw new DirectoryException(ResultCode.CONSTRAINT_VIOLATION, message);
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public ByteString getAuthPasswordPlaintextValue(String authInfo,
                                                   String authValue)
          throws DirectoryException
@@ -537,15 +378,10 @@
     throw new DirectoryException(ResultCode.CONSTRAINT_VIOLATION, message);
   }
 
-
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override()
+  /** {@inheritDoc} */
+  @Override
   public boolean isStorageSchemeSecure()
   {
-    // PBKDF2 should be considered secure.
     return true;
   }
 
@@ -568,19 +404,33 @@
          throws DirectoryException
   {
     byte[] saltBytes      = new byte[NUM_SALT_BYTES];
-    byte[] digestBytes;
-    char[] plaintextChars = null;
     int    iterations     = 10000;
 
+    byte[] digestBytes = getDigestBytes(passwordBytes, saltBytes, iterations);
+    // Append the salt to the hashed value and base64-the whole thing.
+    byte[] hashPlusSalt = new byte[digestBytes.length + NUM_SALT_BYTES];
+
+    System.arraycopy(digestBytes, 0, hashPlusSalt, 0, digestBytes.length);
+    System.arraycopy(saltBytes, 0, hashPlusSalt, digestBytes.length,
+                     NUM_SALT_BYTES);
+
+    return '{' + STORAGE_SCHEME_NAME_PBKDF2 + '}' + iterations + ':' +
+      Base64.encode(hashPlusSalt);
+  }
+
+  private static byte[] getDigestBytes(byte[] plaintext, byte[] saltBytes,
+      int iterations) throws DirectoryException
+  {
+    char[] plaintextChars = null;
     try
     {
       SecureRandom.getInstance(SECURE_PRNG_SHA1).nextBytes(saltBytes);
 
-      plaintextChars = passwordBytes.toString().toCharArray();
+      plaintextChars = plaintext.toString().toCharArray();
       KeySpec spec = new PBEKeySpec(
           plaintextChars, saltBytes,
           iterations, SHA1_LENGTH * 8);
-      digestBytes = SecretKeyFactory
+      return SecretKeyFactory
           .getInstance(MESSAGE_DIGEST_ALGORITHM_PBKDF2)
           .generateSecret(spec).getEncoded();
     }
@@ -599,18 +449,10 @@
     finally
     {
       if (plaintextChars != null)
+      {
         Arrays.fill(plaintextChars, '0');
+      }
     }
-
-    // Append the salt to the hashed value and base64-the whole thing.
-    byte[] hashPlusSalt = new byte[digestBytes.length + NUM_SALT_BYTES];
-
-    System.arraycopy(digestBytes, 0, hashPlusSalt, 0, digestBytes.length);
-    System.arraycopy(saltBytes, 0, hashPlusSalt, digestBytes.length,
-                     NUM_SALT_BYTES);
-
-    return "{" + STORAGE_SCHEME_NAME_PBKDF2 + "}" + iterations + ":" +
-      Base64.encode(hashPlusSalt);
   }
 
 }

--
Gitblit v1.10.0