jclouds - more review comments addressed - pub key url auth, reinit

Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/2bc32d83
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/2bc32d83
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/2bc32d83

Branch: refs/heads/master
Commit: 2bc32d83f633c9b93be1b0f416ba86507bb5af13
Parents: 9e82be1
Author: Alex Heneveld <[email protected]>
Authored: Mon Jan 26 12:25:30 2015 +0000
Committer: Alex Heneveld <[email protected]>
Committed: Tue Jan 27 11:55:34 2015 +0000

----------------------------------------------------------------------
 .../internal/BrooklynInitialization.java        | 17 +++++++++++++++--
 .../java/brooklyn/util/crypto/SecureKeys.java   |  4 +++-
 .../location/basic/LocationConfigUtilsTest.java |  2 +-
 .../location/jclouds/JcloudsLocation.java       | 10 +++++++++-
 .../location/jclouds/JcloudsLocationConfig.java |  4 ++--
 .../brooklyn/launcher/BrooklynWebServer.java    |  6 +++---
 .../util/crypto/AuthorizedKeysParser.java       | 20 +++++++++++---------
 7 files changed, 44 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/core/src/main/java/brooklyn/internal/BrooklynInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/internal/BrooklynInitialization.java 
b/core/src/main/java/brooklyn/internal/BrooklynInitialization.java
index 9227443..83d8d9d 100644
--- a/core/src/main/java/brooklyn/internal/BrooklynInitialization.java
+++ b/core/src/main/java/brooklyn/internal/BrooklynInitialization.java
@@ -18,18 +18,22 @@
  */
 package brooklyn.internal;
 
-import com.google.common.annotations.Beta;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import brooklyn.location.basic.PortRanges;
 import brooklyn.util.crypto.SecureKeys;
 import brooklyn.util.flags.TypeCoercions;
 import brooklyn.util.net.Networking;
 
+import com.google.common.annotations.Beta;
+
 /** Various static initialization tasks are routed through this class,
  * to give us better traceability of their invocation. */ 
 @Beta
 public class BrooklynInitialization {
 
+    private static AtomicBoolean done = new AtomicBoolean(false);
+    
     public static void initTypeCoercionStandardAdapters() {
         TypeCoercions.initStandardAdapters();
     }
@@ -57,12 +61,21 @@ public class BrooklynInitialization {
      * 
      */
     
-    public static void initAll() {
+    public synchronized static void initAll() {
+        if (done.get()) return;
         initTypeCoercionStandardAdapters();
         initSecureKeysBouncyCastleProvider();
         initNetworking();
         initPortRanges();
         initLegacyLanguageExtensions();
+        done.set(true);
+    }
+
+    @SuppressWarnings("deprecation")
+    public synchronized static void reinitAll() {
+        done.set(false);
+        brooklyn.util.BrooklynLanguageExtensions.reinit();
+        initAll();
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/core/src/main/java/brooklyn/util/crypto/SecureKeys.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/crypto/SecureKeys.java 
b/core/src/main/java/brooklyn/util/crypto/SecureKeys.java
index 14612b8..23a46d0 100644
--- a/core/src/main/java/brooklyn/util/crypto/SecureKeys.java
+++ b/core/src/main/java/brooklyn/util/crypto/SecureKeys.java
@@ -50,7 +50,9 @@ import com.google.common.base.Objects;
 import com.google.common.base.Throwables;
 
 /**
- * Utility methods for generating and working with keys
+ * Utility methods for generating and working with keys,
+ * extending the parent class with useful things provided by BouncyCastle 
crypto library.
+ * (Parent class is in a different project where BC is not included as a 
dependency.)
  */
 public class SecureKeys extends SecureKeysWithoutBouncyCastle {
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java 
b/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
index 6f9178c..1d1ad06 100644
--- a/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
+++ b/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
@@ -90,7 +90,7 @@ public class LocationConfigUtilsTest {
         assertTrue(data != null && data.length() > 0);
     }
     
-    @Test(groups="Integration")  // requires ~/.ssh/passphrase-id_rsa
+    @Test(groups="Integration")
     public void testReadsPrivateKeyFileWithPassphrase() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, 
SSH_PRIVATE_KEY_FILE_WITH_PASSPHRASE);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git 
a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
 
b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
index 1059891..23f069a 100644
--- 
a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
+++ 
b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
@@ -763,7 +763,7 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
                     sshMachineLocation.execCommands("Stopping iptables", cmds);
                 }
                 
-                List<String> extraKeyUrlsToAuth = 
setup.get(EXTRA_PUBLIC_KEYS_TO_AUTH);
+                List<String> extraKeyUrlsToAuth = 
setup.get(EXTRA_PUBLIC_KEY_URLS_TO_AUTH);
                 if (extraKeyUrlsToAuth!=null && !extraKeyUrlsToAuth.isEmpty()) 
{
                     List<String> extraKeyDataToAuth = MutableList.of();
                     for (String keyUrl: extraKeyUrlsToAuth) {
@@ -1389,6 +1389,14 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             String privKey = credential.getPrivateKeyData();
             
             if (credential.isEmpty()) {
+                /*
+                 * TODO have an explicit `create_new_key_per_machine` config 
key.
+                 * error if privateKeyData is set in this case.
+                 * publicKeyData automatically added to 
EXTRA_SSH_KEY_URLS_TO_AUTH.
+                 * 
+                 * if this config key is not set, use a key `brooklyn_id_rsa` 
and `.pub` in `MGMT_BASE_DIR`,
+                 * with permission 0600, creating it if necessary, and logging 
the fact that this was created.
+                 */
                 if (!loggedSshKeysHint && 
!config.containsKey(PRIVATE_KEY_FILE)) {
                     loggedSshKeysHint = true;
                     LOG.info("Default SSH keys not found or not usable; will 
create new keys for each machine. "

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
----------------------------------------------------------------------
diff --git 
a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
 
b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
index cc70915..1e52d84 100644
--- 
a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
+++ 
b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
@@ -74,8 +74,8 @@ public interface JcloudsLocationConfig extends 
CloudLocationConfig {
     public static final ConfigKey<String> EXTRA_PUBLIC_KEY_DATA_TO_AUTH = 
ConfigKeys.newStringConfigKey("extraSshPublicKeyData",
         "Additional public key data to add to authorized_keys", null);
     @SuppressWarnings("serial")
-    public static final ConfigKey<List<String>> EXTRA_PUBLIC_KEYS_TO_AUTH = 
ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, 
-        "extraSshPublicKeys", "Additional public keys (files or URLs) to add 
to authorized_keys", null);
+    public static final ConfigKey<List<String>> EXTRA_PUBLIC_KEY_URLS_TO_AUTH 
= ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, 
+        "extraSshPublicKeyUrls", "Additional public keys (files or URLs, in 
SSH2/RFC4716/id_rsa.pub format) to add to authorized_keys", null);
     
     public static final ConfigKey<Boolean> DONT_CREATE_USER = 
ConfigKeys.newBooleanConfigKey("dontCreateUser", 
             "Whether to skip creation of 'user' when provisioning machines 
(default false)", false);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
----------------------------------------------------------------------
diff --git 
a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java 
b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
index 680473c..20dacd0 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynWebServer.java
@@ -49,6 +49,7 @@ import brooklyn.BrooklynVersion;
 import brooklyn.config.BrooklynServerPaths;
 import brooklyn.config.BrooklynServiceAttributes;
 import brooklyn.config.ConfigKey;
+import brooklyn.internal.BrooklynInitialization;
 import brooklyn.launcher.config.CustomResourceLocator;
 import brooklyn.location.PortRange;
 import brooklyn.location.basic.LocalhostMachineProvisioningLocation;
@@ -62,7 +63,6 @@ import brooklyn.rest.filter.HaMasterCheckFilter;
 import brooklyn.rest.filter.LoggingFilter;
 import brooklyn.rest.filter.NoCacheFilter;
 import brooklyn.rest.filter.RequestTaggingFilter;
-import brooklyn.util.BrooklynLanguageExtensions;
 import brooklyn.util.BrooklynNetworkUtils;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableMap;
@@ -435,8 +435,8 @@ public class BrooklynWebServer {
 
         server.setHandler(handlers);
         server.start();
-        //reinit required because grails wipes our language extension bindings
-        BrooklynLanguageExtensions.reinit();
+        //reinit required because some webapps (eg grails) might wipe our 
language extension bindings
+        BrooklynInitialization.reinitAll();
 
         if (managementContext instanceof ManagementContextInternal) {
             ((ManagementContextInternal) 
managementContext).setManagementNodeUri(new URI(getRootUrl()));

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/2bc32d83/utils/common/src/main/java/brooklyn/util/crypto/AuthorizedKeysParser.java
----------------------------------------------------------------------
diff --git 
a/utils/common/src/main/java/brooklyn/util/crypto/AuthorizedKeysParser.java 
b/utils/common/src/main/java/brooklyn/util/crypto/AuthorizedKeysParser.java
index e64454e..c927c09 100644
--- a/utils/common/src/main/java/brooklyn/util/crypto/AuthorizedKeysParser.java
+++ b/utils/common/src/main/java/brooklyn/util/crypto/AuthorizedKeysParser.java
@@ -54,15 +54,15 @@ public class AuthorizedKeysParser {
 
             String type = readType(stream);
             if (type.equals("ssh-rsa")) {
-                BigInteger e = readBigInt(stream);
-                BigInteger m = readBigInt(stream);
+                BigInteger e = readBigInt(stream, 1);
+                BigInteger m = readBigInt(stream, 1);
                 RSAPublicKeySpec spec = new RSAPublicKeySpec(m, e);
                 return KeyFactory.getInstance("RSA").generatePublic(spec);
             } else if (type.equals("ssh-dss")) {
-                BigInteger p = readBigInt(stream);
-                BigInteger q = readBigInt(stream);
-                BigInteger g = readBigInt(stream);
-                BigInteger y = readBigInt(stream);
+                BigInteger p = readBigInt(stream, 1);
+                BigInteger q = readBigInt(stream, 1);
+                BigInteger g = readBigInt(stream, 1);
+                BigInteger y = readBigInt(stream, 1);
                 DSAPublicKeySpec spec = new DSAPublicKeySpec(y, p, q, g);
                 return KeyFactory.getInstance("DSA").generatePublic(spec);
             } else {
@@ -80,8 +80,10 @@ public class AuthorizedKeysParser {
             | ((stream.read() & 0xFF) << 8) | (stream.read() & 0xFF);
     }
     
-    private static byte[] readBytesWithLength(InputStream stream) throws 
IOException {
+    private static byte[] readBytesWithLength(InputStream stream, int minLen) 
throws IOException {
         int len = readInt(stream);
+        if (len<minLen || len>100000)
+            throw new IllegalStateException("Invalid stream header: length 
"+len);
         byte[] result = new byte[len];
         stream.read(result);
         return result;
@@ -96,8 +98,8 @@ public class AuthorizedKeysParser {
         stream.write(buf);
     }
     
-    private static String readType(InputStream stream) throws IOException { 
return new String(readBytesWithLength(stream)); }
-    private static BigInteger readBigInt(InputStream stream) throws 
IOException { return new BigInteger(readBytesWithLength(stream)); }
+    private static String readType(InputStream stream) throws IOException { 
return new String(readBytesWithLength(stream, 0)); }
+    private static BigInteger readBigInt(InputStream stream, int minLen) 
throws IOException { return new BigInteger(readBytesWithLength(stream, 
minLen)); }
 
     public static String encodePublicKey(PublicKey key) {
         ByteArrayOutputStream out = new ByteArrayOutputStream();

Reply via email to