PR64902: Keys returned by KeyPairGenerator don't use standardised algorithm names

PR64904: KeyPairGenerator.genKeyPair() fails if not explicitly initialised

2015-02-02  Andrew John Hughes  <gnu_andrew@member.fsf.org>

	PR classpath/64902
	PR classpath/64904
	* NEWS: Updated.
	* gnu/java/security/jce/sig/KeyPairGeneratorAdapter.java:
	(KeyPairGeneratorAdapter(String)): Filter incoming generator
	names so all standardised names are handled (e.g. the
	DiffieHellman alias for DH.
	(generateKeyPair()): Check whether the generator has been
	initialized and initialize it with defaults if not.
	(getAlgorithm()): Return the standardised name, not the
	internal one.
	(localiseName(String)): Convert requested standardised
	name to the internal equivalent.
	* gnu/java/security/key/IKeyPairGenerator.java:
	(isInitialized()): New method to check whether the generator
	has been initialized.
	(getDefaultKeySize()): Return the default key size used
	by the generator.
	* gnu/java/security/key/dss/DSSKey.java:
	(getAlgorithm()): Return standard "DSA" rather than "dsa".
	* gnu/java/security/key/dss/DSSKeyPairGenerator.java:
	(initialized): Flag to indicate whether the generator has
	been initialized or not.
	(initLock): Lock to prevent multiple concurrent initializations.
	(setup(Map)): Wrap initialization in a lock and set initialized
	flag when done.
	(isInitialized()): Returns the value of the initialized flag.
	(getDefaultKeySize()): Returns the default key size.
	* gnu/java/security/key/rsa/GnuRSAKey.java:
	(getAlgorithm()): Return standard "RSA" rather than "rsa".
	* gnu/java/security/key/rsa/RSAKeyPairGenerator.java:
	(initialized): Flag to indicate whether the generator has
	been initialized or not.
	(initLock): Lock to prevent multiple concurrent initializations.
	(setup(Map)): Wrap initialization in a lock and set initialized
	flag when done.
	(isInitialized()): Returns the value of the initialized flag.
	(getDefaultKeySize()): Returns the default key size.
	* gnu/javax/crypto/key/dh/GnuDHKey.java:
	(getAlgorithm()): Return standard "DH" rather than "dh".
	* gnu/javax/crypto/key/dh/GnuDHKeyPairGenerator.java:
	(initialized): Flag to indicate whether the generator has
	been initialized or not.
	(initLock): Lock to prevent multiple concurrent initializations.
	(setup(Map)): Wrap initialization in a lock and set initialized
	flag when done.
	(isInitialized()): Returns the value of the initialized flag.
	(getDefaultKeySize()): Returns the default key size.
	* gnu/javax/crypto/key/srp6/SRPKeyPairGenerator.java:
	(initialized): Flag to indicate whether the generator has
	been initialized or not.
	(initLock): Lock to prevent multiple concurrent initializations.
	(setup(Map)): Wrap initialization in a lock and set initialized
	flag when done.
	(isInitialized()): Returns the value of the initialized flag.
	(getDefaultKeySize()): Returns the default key size.

6 files changed, 247 insertions, 92 deletions

diff --git a/gnu/java/security/jce/sig/KeyPairGeneratorAdapter.java b/gnu/java/security/jce/sig/KeyPairGeneratorAdapter.java
index 9d3e5efdd..cd612aaa5 100644
--- a/gnu/java/security/jce/sig/KeyPairGeneratorAdapter.java
+++ b/gnu/java/security/jce/sig/KeyPairGeneratorAdapter.java
@@ -1,5 +1,5 @@
 /* KeyPairGeneratorAdapter.java --
-   Copyright 2001, 2002, 2006 Free Software Foundation, Inc.
+   Copyright 2001, 2002, 2006, 2015 Free Software Foundation, Inc.
 
 This file is a part of GNU Classpath.
 
@@ -38,6 +38,7 @@ exception statement from your version.  */
 
 package gnu.java.security.jce.sig;
 
+import gnu.java.security.Registry;
 import gnu.java.security.key.IKeyPairGenerator;
 import gnu.java.security.key.KeyPairGeneratorFactory;
 
@@ -79,7 +80,7 @@ public abstract class KeyPairGeneratorAdapter
   {
     super(kpgName);
 
-    this.adaptee = KeyPairGeneratorFactory.getInstance(kpgName);
+    this.adaptee = KeyPairGeneratorFactory.getInstance(localiseName(kpgName));
   }
 
   public abstract void initialize(int keysize, SecureRandom random);
@@ -90,6 +91,59 @@ public abstract class KeyPairGeneratorAdapter
 
   public KeyPair generateKeyPair()
   {
+    if (!adaptee.isInitialized())
+      initialize(adaptee.getDefaultKeySize());
+
     return adaptee.generate();
   }
+
+  /**
+   * The {@code java.security.*} methods are expected to return
+   * standard algorithm names, as listed in
+   * <a href="http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#KeyPairGenerator">
+   * the on-line Oracle documentation</a>.
+   *
+   * @return the name specified by the Oracle documentation.
+   */
+  @Override
+  public String getAlgorithm()
+  {
+    String alg = super.getAlgorithm();
+
+    if ("dh".equals(alg))
+      return "DH";
+    if ("dsa".equals(alg))
+      return "DSA";
+    if ("dss".equals(alg))
+      return "DSA";
+    if ("rsa".equals(alg))
+      return "RSA";
+    return alg;
+  }
+
+  /**
+   * The user may specify an algorithm using the names specified in
+   * <a href="http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#KeyPairGenerator">
+   * the on-line Oracle documentation</a>. We should translate them
+   * to names recognised by the GNU registry.
+   *
+   * @param kpgName the generator name, which may be its standardised
+   *                name.
+   * @return the name specified by the GNU registry.
+   */
+  private static String localiseName(String kpgName)
+  {
+    /**
+    if ("DiffieHellman".equals(kpgName))
+      return Registry.DH_KPG;
+    if ("DH".equals(kpgName))
+      return Registry.DH_KPG;
+    if ("DSA".equals(kpgName))
+      return Registry.DSA_KPG;
+    if ("RSA".equals(kpgName))
+      return Registry.RSA_KPG;
+    */
+    return kpgName;
+  }
+
 }
diff --git a/gnu/java/security/key/IKeyPairGenerator.java b/gnu/java/security/key/IKeyPairGenerator.java
index e7d96b22a..55c62da7a 100644
--- a/gnu/java/security/key/IKeyPairGenerator.java
+++ b/gnu/java/security/key/IKeyPairGenerator.java
@@ -1,5 +1,5 @@
 /* IKeyPairGenerator.java --
-   Copyright 2001, 2002, 2006, 2014 Free Software Foundation, Inc.
+   Copyright 2001, 2002, 2006, 2014, 2015 Free Software Foundation, Inc.
 
 This file is a part of GNU Classpath.
 
@@ -70,4 +70,19 @@ public interface IKeyPairGenerator
    * @return a new keypair.
    */
   KeyPair generate();
+
+  /**
+   * Returns true if this instance has been initialized.
+   *
+   * @return true if this instance has been initialized.
+   */
+  boolean isInitialized();
+
+  /**
+   * Returns the default key size used by this generator.
+   *
+   * @return the default key size of this generator.
+   */
+  int getDefaultKeySize();
+
 }
diff --git a/gnu/java/security/key/dss/DSSKey.java b/gnu/java/security/key/dss/DSSKey.java
index 0641ff872..ed356ef38 100644
--- a/gnu/java/security/key/dss/DSSKey.java
+++ b/gnu/java/security/key/dss/DSSKey.java
@@ -135,7 +135,7 @@ public abstract class DSSKey
 
   public String getAlgorithm()
   {
-    return Registry.DSS_KPG;
+    return "DSA";
   }
 
   /** @deprecated see getEncoded(int). */
diff --git a/gnu/java/security/key/dss/DSSKeyPairGenerator.java b/gnu/java/security/key/dss/DSSKeyPairGenerator.java
index 9a19947fe..fe1eeb683 100644
--- a/gnu/java/security/key/dss/DSSKeyPairGenerator.java
+++ b/gnu/java/security/key/dss/DSSKeyPairGenerator.java
@@ -1,5 +1,5 @@
 /* DSSKeyPairGenerator.java --
-   Copyright 2001, 2002, 2003, 2006, 2010, 2014 Free Software Foundation, Inc.
+   Copyright 2001, 2002, 2003, 2006, 2010, 2014, 2015 Free Software Foundation, Inc.
 
 This file is a part of GNU Classpath.
 
@@ -51,6 +51,9 @@ import java.security.PublicKey;
 import java.security.SecureRandom;
 import java.security.spec.DSAParameterSpec;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
 
 /**
@@ -211,6 +214,12 @@ public class DSSKeyPairGenerator
   /** Preferred encoding format of generated keys. */
   private int preferredFormat;
 
+  /** Flag to indicate whether the generator has been initialized */
+  private AtomicBoolean initialized = new AtomicBoolean(false);
+
+  /** Lock to prevent concurrent initialization */
+  private Lock initLock = new ReentrantLock();
+
   @Override
   public String name()
   {
@@ -228,77 +237,88 @@ public class DSSKeyPairGenerator
   @Override
   public void setup(Map<String,Object> attributes)
   {
-    // find out the modulus length
-    Integer l = (Integer) attributes.get(MODULUS_LENGTH);
-    L = (l == null ? DEFAULT_MODULUS_LENGTH : l.intValue());
-    if ((L % 64) != 0 || L < 512 || L > 1024)
-      throw new IllegalArgumentException(MODULUS_LENGTH);
-
-    // should we use the default pre-computed params?
-    Boolean useDefaults = (Boolean) attributes.get(USE_DEFAULTS);
-    if (useDefaults == null)
-      useDefaults = Boolean.TRUE;
-
-    Boolean strictDefaults = (Boolean) attributes.get(STRICT_DEFAULTS);
-    if (strictDefaults == null)
-      strictDefaults = Boolean.FALSE;
-
-    // are we given a set of DSA params or we shall use/generate our own?
-    DSAParameterSpec params = (DSAParameterSpec) attributes.get(DSS_PARAMETERS);
-    if (params != null)
+    initLock.lock();
+    try
       {
-        p = params.getP();
-        q = params.getQ();
-        g = params.getG();
-      }
-    else if (useDefaults.equals(Boolean.TRUE))
-      {
-        switch (L)
-          {
-          case 512:
-            p = KEY_PARAMS_512.getP();
-            q = KEY_PARAMS_512.getQ();
-            g = KEY_PARAMS_512.getG();
-            break;
-          case 768:
-            p = KEY_PARAMS_768.getP();
-            q = KEY_PARAMS_768.getQ();
-            g = KEY_PARAMS_768.getG();
-            break;
-          case 1024:
-            p = KEY_PARAMS_1024.getP();
-            q = KEY_PARAMS_1024.getQ();
-            g = KEY_PARAMS_1024.getG();
-            break;
-          default:
-            if (strictDefaults.equals(Boolean.TRUE))
-              throw new IllegalArgumentException(
-                  "Does not provide default parameters for " + L
-                  + "-bit modulus length");
-            else
-              {
-                p = null;
-                q = null;
-                g = null;
-              }
-          }
+	// find out the modulus length
+	Integer l = (Integer) attributes.get(MODULUS_LENGTH);
+	L = (l == null ? DEFAULT_MODULUS_LENGTH : l.intValue());
+	if ((L % 64) != 0 || L < 512 || L > 1024)
+	  throw new IllegalArgumentException("The modulus length must be a " +
+					     "factor of 64 between 512 and 1024");
+
+	// should we use the default pre-computed params?
+	Boolean useDefaults = (Boolean) attributes.get(USE_DEFAULTS);
+	if (useDefaults == null)
+	  useDefaults = Boolean.TRUE;
+
+	Boolean strictDefaults = (Boolean) attributes.get(STRICT_DEFAULTS);
+	if (strictDefaults == null)
+	  strictDefaults = Boolean.FALSE;
+
+	// are we given a set of DSA params or we shall use/generate our own?
+	DSAParameterSpec params = (DSAParameterSpec) attributes.get(DSS_PARAMETERS);
+	if (params != null)
+	  {
+	    p = params.getP();
+	    q = params.getQ();
+	    g = params.getG();
+	  }
+	else if (useDefaults.equals(Boolean.TRUE))
+	  {
+	    switch (L)
+	      {
+	      case 512:
+		p = KEY_PARAMS_512.getP();
+		q = KEY_PARAMS_512.getQ();
+		g = KEY_PARAMS_512.getG();
+		break;
+	      case 768:
+		p = KEY_PARAMS_768.getP();
+		q = KEY_PARAMS_768.getQ();
+		g = KEY_PARAMS_768.getG();
+		break;
+	      case 1024:
+		p = KEY_PARAMS_1024.getP();
+		q = KEY_PARAMS_1024.getQ();
+		g = KEY_PARAMS_1024.getG();
+		break;
+	      default:
+		if (strictDefaults.equals(Boolean.TRUE))
+		  throw new IllegalArgumentException(
+                    "Does not provide default parameters for " + L
+                    + "-bit modulus length");
+		else
+		  {
+		    p = null;
+		    q = null;
+		    g = null;
+		  }
+	      }
+	  }
+	else
+	  {
+	    p = null;
+	    q = null;
+	    g = null;
+	  }
+	// do we have a SecureRandom, or should we use our own?
+	rnd = (SecureRandom) attributes.get(SOURCE_OF_RANDOMNESS);
+	// what is the preferred encoding format
+	Integer formatID = (Integer) attributes.get(PREFERRED_ENCODING_FORMAT);
+	preferredFormat = formatID == null ? DEFAULT_ENCODING_FORMAT
+                                           : formatID.intValue();
+	// set the seed-key
+	byte[] kb = new byte[20]; // we need 160 bits of randomness
+	nextRandomBytes(kb);
+	XKEY = new BigInteger(1, kb).setBit(159).setBit(0);
+
+	initialized.set(false);
       }
-    else
+    finally
       {
-        p = null;
-        q = null;
-        g = null;
+	initLock.unlock();
       }
-    // do we have a SecureRandom, or should we use our own?
-    rnd = (SecureRandom) attributes.get(SOURCE_OF_RANDOMNESS);
-    // what is the preferred encoding format
-    Integer formatID = (Integer) attributes.get(PREFERRED_ENCODING_FORMAT);
-    preferredFormat = formatID == null ? DEFAULT_ENCODING_FORMAT
-                                       : formatID.intValue();
-    // set the seed-key
-    byte[] kb = new byte[20]; // we need 160 bits of randomness
-    nextRandomBytes(kb);
-    XKEY = new BigInteger(1, kb).setBit(159).setBit(0);
   }
 
   @Override
@@ -306,6 +326,7 @@ public class DSSKeyPairGenerator
   {
     if (p == null)
       {
+	System.err.println("Attempting to generate parameters for modulus of length " + L);
         BigInteger[] params = new FIPS186(L, rnd).generateParameters();
         seed = params[FIPS186.DSA_PARAMS_SEED];
         counter = params[FIPS186.DSA_PARAMS_COUNTER];
@@ -383,4 +404,27 @@ public class DSSKeyPairGenerator
 
     return prng;
   }
+
+  @Override
+  public boolean isInitialized()
+  {
+    boolean initFlag = false;
+
+    initLock.lock();
+    try
+      {
+	initFlag = initialized.get();
+      }
+    finally
+      {
+	initLock.unlock();
+      }
+    return initFlag;
+  }
+
+  @Override
+  public int getDefaultKeySize()
+  {
+    return DEFAULT_MODULUS_LENGTH;
+  }
 }
diff --git a/gnu/java/security/key/rsa/GnuRSAKey.java b/gnu/java/security/key/rsa/GnuRSAKey.java
index f8d89af38..543ebdc56 100644
--- a/gnu/java/security/key/rsa/GnuRSAKey.java
+++ b/gnu/java/security/key/rsa/GnuRSAKey.java
@@ -97,7 +97,7 @@ public abstract class GnuRSAKey
 
   public String getAlgorithm()
   {
-    return Registry.RSA_KPG;
+    return "RSA";
   }
 
   /** @deprecated see getEncoded(int). */
diff --git a/gnu/java/security/key/rsa/RSAKeyPairGenerator.java b/gnu/java/security/key/rsa/RSAKeyPairGenerator.java
index a7fc21efe..5f321ce5b 100644
--- a/gnu/java/security/key/rsa/RSAKeyPairGenerator.java
+++ b/gnu/java/security/key/rsa/RSAKeyPairGenerator.java
@@ -1,5 +1,5 @@
 /* RSAKeyPairGenerator.java --
-   Copyright 2001, 2002, 2003, 2006, 2010, 2014 Free Software Foundation, Inc.
+   Copyright 2001, 2002, 2003, 2006, 2010, 2014, 2015 Free Software Foundation, Inc.
 
 This file is a part of GNU Classpath.
 
@@ -50,6 +50,9 @@ import java.security.PublicKey;
 import java.security.SecureRandom;
 import java.security.spec.RSAKeyGenParameterSpec;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
 
 /**
@@ -127,6 +130,12 @@ public class RSAKeyPairGenerator
   /** Preferred encoding format of generated keys. */
   private int preferredFormat;
 
+  /** Flag to indicate whether the generator has been initialized */
+  private AtomicBoolean initialized = new AtomicBoolean(false);
+
+  /** Lock to prevent concurrent initialization */
+  private Lock initLock = new ReentrantLock();
+
   // implicit 0-arguments constructor
 
   @Override
@@ -147,28 +156,38 @@ public class RSAKeyPairGenerator
   {
     if (Configuration.DEBUG)
       log.entering(this.getClass().getName(), "setup", attributes);
-    // do we have a SecureRandom, or should we use our own?
-    rnd = (SecureRandom) attributes.get(SOURCE_OF_RANDOMNESS);
-    // are we given a set of RSA params or we shall use our own?
-    RSAKeyGenParameterSpec params = (RSAKeyGenParameterSpec) attributes.get(RSA_PARAMETERS);
-    // find out the modulus length
-    if (params != null)
+    initLock.lock();
+    try
       {
-        L = params.getKeysize();
-        e = params.getPublicExponent();
+	// do we have a SecureRandom, or should we use our own?
+	rnd = (SecureRandom) attributes.get(SOURCE_OF_RANDOMNESS);
+	// are we given a set of RSA params or we shall use our own?
+	RSAKeyGenParameterSpec params = (RSAKeyGenParameterSpec) attributes.get(RSA_PARAMETERS);
+	// find out the modulus length
+	if (params != null)
+	  {
+	    L = params.getKeysize();
+	    e = params.getPublicExponent();
+	  }
+	else
+	  {
+	    Integer l = (Integer) attributes.get(MODULUS_LENGTH);
+	    L = (l == null ? DEFAULT_MODULUS_LENGTH : l.intValue());
+	  }
+	if (L < 1024)
+	  throw new IllegalArgumentException("Invalid modulus size");
+
+	// what is the preferred encoding format
+	Integer formatID = (Integer) attributes.get(PREFERRED_ENCODING_FORMAT);
+	preferredFormat = formatID == null ? DEFAULT_ENCODING_FORMAT
+                                           : formatID.intValue();
+
+	initialized.set(true);
       }
-    else
+    finally
       {
-        Integer l = (Integer) attributes.get(MODULUS_LENGTH);
-        L = (l == null ? DEFAULT_MODULUS_LENGTH : l.intValue());
+	initLock.unlock();
       }
-    if (L < 1024)
-      throw new IllegalArgumentException(MODULUS_LENGTH);
-
-    // what is the preferred encoding format
-    Integer formatID = (Integer) attributes.get(PREFERRED_ENCODING_FORMAT);
-    preferredFormat = formatID == null ? DEFAULT_ENCODING_FORMAT
-                                       : formatID.intValue();
     if (Configuration.DEBUG)
       log.exiting(this.getClass().getName(), "setup");
   }
@@ -247,4 +266,27 @@ public class RSAKeyPairGenerator
 
     return prng;
   }
+
+  @Override
+  public boolean isInitialized()
+  {
+    boolean initFlag = false;
+
+    initLock.lock();
+    try
+      {
+	initFlag = initialized.get();
+      }
+    finally
+      {
+	initLock.unlock();
+      }
+    return initFlag;
+  }
+
+  @Override
+  public int getDefaultKeySize()
+  {
+    return DEFAULT_MODULUS_LENGTH;
+  }
 }