brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [02/21] incubator-brooklyn git commit: validate public, private key and passphrase *before* provisioning
Date Tue, 27 Jan 2015 17:45:09 GMT
validate public, private key and passphrase *before* provisioning

and give good feedback through logging and errors


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

Branch: refs/heads/master
Commit: c3828fbffb6bad0006456e4f828f56f4cfc44bb2
Parents: fa79e20
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Thu Jan 22 14:55:47 2015 +0000
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Thu Jan 22 20:25:21 2015 +0000

----------------------------------------------------------------------
 .../location/basic/LocationConfigKeys.java      |   2 +-
 .../location/basic/LocationConfigUtils.java     | 178 +++++++++++---
 .../java/brooklyn/util/crypto/SecureKeys.java   | 233 ++++++++-----------
 .../location/basic/LocationConfigUtilsTest.java |  73 +++++-
 .../SshMachineLocationIntegrationTest.java      |   2 +-
 .../util/crypto/SecureKeysAndSignerTest.java    |  41 ++++
 .../location/jclouds/JcloudsLocation.java       | 123 +++++++---
 .../location/jclouds/JcloudsLocationConfig.java |   6 +-
 .../util/crypto/AuthorizedKeysParser.java       | 132 +++++++++++
 .../crypto/SecureKeysWithoutBouncyCastle.java   | 161 +++++++++++++
 10 files changed, 741 insertions(+), 210 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java b/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
index 074bf8e..2d341b8 100644
--- a/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
+++ b/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
@@ -53,7 +53,7 @@ public class LocationConfigKeys {
     public static final ConfigKey<String> USER = ConfigKeys.newStringConfigKey("user", 
             "user account for normal access to the remote machine, defaulting to local user", System.getProperty("user.name"));
     
-    public static final ConfigKey<String> PASSWORD = ConfigKeys.newStringConfigKey("password");
+    public static final ConfigKey<String> PASSWORD = ConfigKeys.newStringConfigKey("password", "password to use for ssh; note some images do not allow password-based ssh access");
     public static final ConfigKey<String> PUBLIC_KEY_FILE = ConfigKeys.newStringConfigKey("publicKeyFile", "ssh public key file to use; if blank will infer from privateKeyFile by appending \".pub\"");
     public static final ConfigKey<String> PUBLIC_KEY_DATA = ConfigKeys.newStringConfigKey("publicKeyData", "ssh public key string to use (takes precedence over publicKeyFile)");
     public static final ConfigKey<String> PRIVATE_KEY_FILE = ConfigKeys.newStringConfigKey("privateKeyFile", "a '" + File.pathSeparator + "' separated list of ssh private key files; uses first in list that can be read",

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java b/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
index 0fac5e3..3518a0a 100644
--- a/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
+++ b/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
@@ -20,33 +20,39 @@ package brooklyn.location.basic;
 
 import static brooklyn.util.JavaGroovyEquivalents.groovyTruth;
 
+import java.io.ByteArrayInputStream;
 import java.io.File;
-import java.io.IOException;
+import java.security.KeyPair;
+import java.security.PublicKey;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import brooklyn.config.ConfigKey;
 import brooklyn.entity.basic.ConfigKeys;
+import brooklyn.location.cloud.CloudLocationConfig;
 import brooklyn.management.ManagementContext;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableMap;
+import brooklyn.util.collections.MutableSet;
 import brooklyn.util.config.ConfigBag;
+import brooklyn.util.crypto.AuthorizedKeysParser;
+import brooklyn.util.crypto.SecureKeys;
+import brooklyn.util.crypto.SecureKeys.PassphraseProblem;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.os.Os;
 import brooklyn.util.text.StringFunctions;
 import brooklyn.util.text.Strings;
 
-import com.google.common.base.Charsets;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import com.google.common.io.Files;
 
 public class LocationConfigUtils {
 
@@ -84,12 +90,37 @@ public class LocationConfigUtils {
         }
 
         /** throws if there are any problems */
-        public OsCredential check() {
+        public OsCredential checkNotEmpty() {
+            checkNoErrors();
+            
+            if (!hasKey() && !hasPassword()) {
+                if (warningMessages.size()>0)
+                    throw new IllegalStateException("Could not find credentials: "+warningMessages);
+                else 
+                    throw new IllegalStateException("Could not find credentials");
+            }
+            return this;
+        }
+
+        /** throws if there were errors resolving (e.g. explicit keys, none of which were found/valid, or public key required and not found) 
+         * @return */
+        public OsCredential checkNoErrors() {
             throwOnErrors(true);
+            dirty();
             infer();
             return this;
         }
+        
+        public OsCredential logAnyWarnings() {
+            if (!warningMessages.isEmpty())
+                log.warn("When reading credentials: "+warningMessages);
+            return this;
+        }
 
+        public Set<String> getWarningMessages() {
+            return warningMessages;
+        }
+        
         /** returns either the key or password or null; if both a key and a password this prefers the key unless otherwise set
          * via {@link #preferPassword()} */
         public synchronized String get() {
@@ -172,16 +203,20 @@ public class LocationConfigUtils {
         
         private synchronized void infer() {
             if (!dirty) return;
+            warningMessages.clear(); 
             
             log.debug("Inferring OS credentials");
             privateKeyData = config.get(LocationConfigKeys.PRIVATE_KEY_DATA);
             password = config.get(LocationConfigKeys.PASSWORD);
             publicKeyData = getKeyDataFromDataKeyOrFileKey(config, LocationConfigKeys.PUBLIC_KEY_DATA, LocationConfigKeys.PUBLIC_KEY_FILE);
 
+            KeyPair privateKey = null;
+            
             if (Strings.isBlank(privateKeyData)) {
                 // look up private key files
                 String privateKeyFiles = null;
-                if (tryDefaultKeys || config.containsKey(LocationConfigKeys.PRIVATE_KEY_FILE)) 
+                boolean privateKeyFilesExplicitlySet = config.containsKey(LocationConfigKeys.PRIVATE_KEY_FILE);
+                if (privateKeyFilesExplicitlySet || (tryDefaultKeys && password==null)) 
                     privateKeyFiles = config.get(LocationConfigKeys.PRIVATE_KEY_FILE);
                 if (Strings.isNonBlank(privateKeyFiles)) {
                     Iterator<String> fi = Arrays.asList(privateKeyFiles.split(File.pathSeparator)).iterator();
@@ -192,60 +227,138 @@ public class LocationConfigUtils {
                                 // real URL's won't actual work, due to use of path separator above 
                                 // not real important, but we get it for free if "files" is a list instead.
                                 // using ResourceUtils is useful for classpath resources
-                                privateKeyData = ResourceUtils.create().getResourceAsString(file);
-                                if (Strings.isNonBlank(publicKeyData)) {
-                                    log.debug("Loaded private key data from "+file+" (public key data from explicit config)");
+                                if (file!=null)
+                                    privateKeyData = ResourceUtils.create().getResourceAsString(file);
+                                // else use data already set
+                                
+                                privateKey = getValidatedPrivateKey(file);
+                                
+                                if (privateKeyData==null) {
+                                    // was cleared due to validation error
+                                } else if (Strings.isNonBlank(publicKeyData)) {
+                                    log.debug("Loaded private key data from "+file+" (public key data explicitly set)");
+                                    break;
                                 } else {
+                                    String publicKeyFile = (file!=null ? file+".pub" : "(data)");
                                     try {
-                                        file = file+".pub";
-                                        publicKeyData = ResourceUtils.create().getResourceAsString(file);
-                                        log.debug("Loaded private key data from "+Strings.removeFromEnd(file, ".pub")+
-                                            " and public key data from "+file);
+                                        publicKeyData = ResourceUtils.create().getResourceAsString(publicKeyFile);
+                                        
+                                        log.debug("Loaded private key data from "+file+
+                                            " and public key data from "+publicKeyFile);
                                         break;
                                     } catch (Exception e) {
                                         Exceptions.propagateIfFatal(e);
-                                        log.debug("No public key file "+file+" ; " + 
-                                            (!requirePublicKey ? "this is allowed in this case" : !fi.hasNext() ? "no more files to try" : "trying next file"), e);
-                                        if (requirePublicKey) {
-                                            privateKeyData = null;
+                                        log.debug("No public key file "+publicKeyFile+"; will try extracting from private key");
+                                        publicKeyData = AuthorizedKeysParser.encodePublicKey(privateKey.getPublic());
+                                        
+                                        if (publicKeyData==null) {
+                                            if (requirePublicKey) {
+                                                addWarning("Unable to find or extract public key for "+file, "skipping");
+                                            } else {
+                                                log.debug("Loaded private key data from "+file+" (public key data not found but not required)");
+                                                break;
+                                            }
                                         } else {
-                                            // look for a different private key
+                                            log.debug("Loaded private key data from "+file+" (public key data extracted)");
                                             break;
                                         }
+                                        privateKeyData = null;
                                     }
                                 }
-                                
-                                // TODO check passphrase public key, validation, etc
-                                
+
                             } catch (Exception e) {
                                 Exceptions.propagateIfFatal(e);
-                                log.debug("No private key file "+file+" ; " + (!fi.hasNext() ? "no more files to try" : "trying next file"), e);
+                                String message = "Missing/invalid private key file "+file;
+                                if (privateKeyFilesExplicitlySet) addWarning(message, (!fi.hasNext() ? "no more files to try" : "trying next file")+": "+e);
                             }
                         }
                     }
+                    if (privateKeyFilesExplicitlySet && Strings.isBlank(privateKeyData))
+                        error("No valid private keys found", ""+warningMessages);
                 }
+            } else {
+                privateKey = getValidatedPrivateKey("(data)");
             }
             
             if (privateKeyData!=null) {
                 if (requirePublicKey && Strings.isBlank(publicKeyData)) {
-                    String message = "If explicit "+LocationConfigKeys.PRIVATE_KEY_DATA.getName()+" is supplied, then "
-                        + "the corresponding "+LocationConfigKeys.PUBLIC_KEY_DATA.getName()+" must also be supplied.";
-                    if (warnOnErrors) log.warn(message);
-                    if (throwOnErrors) throw new IllegalStateException(message);
+                    if (privateKey!=null) {
+                        publicKeyData = AuthorizedKeysParser.encodePublicKey(privateKey.getPublic());
+                    }
+                    if (Strings.isBlank(publicKeyData)) {
+                        error("If explicit "+LocationConfigKeys.PRIVATE_KEY_DATA.getName()+" is supplied, then "
+                            + "the corresponding "+LocationConfigKeys.PUBLIC_KEY_DATA.getName()+" must also be supplied.", null);
+                    } else {
+                        log.debug("Public key data extracted");
+                    }
+                }
+                if (doKeyValidation && privateKey!=null && privateKey.getPublic()!=null && Strings.isNonBlank(publicKeyData)) {
+                    PublicKey decoded = null;
+                    try {
+                        decoded = AuthorizedKeysParser.decodePublicKey(publicKeyData);
+                    } catch (Exception e) {
+                        Exceptions.propagateIfFatal(e);
+                        addWarning("Invalid public key: "+decoded);
+                    }
+                    if (decoded!=null && !privateKey.getPublic().equals( decoded )) {
+                        error("Public key inferred from does not match public key extracted from private key", null);
+                    }
                 }
             }
 
             log.debug("OS credential inference: "+this);
             dirty = false;
         }
+
+        private KeyPair getValidatedPrivateKey(String label) {
+            KeyPair privateKey = null;
+            String passphrase = config.get(CloudLocationConfig.PRIVATE_KEY_PASSPHRASE);
+            try {
+                privateKey = SecureKeys.readPem(new ByteArrayInputStream(privateKeyData.getBytes()), passphrase);
+                if (passphrase!=null) {
+                    // get the unencrypted key data for our internal use (jclouds requires this)
+                    privateKeyData = SecureKeys.toPem(privateKey);
+                }
+            } catch (PassphraseProblem e) {
+                if (doKeyValidation) {
+                    if (Strings.isBlank(passphrase))
+                        addWarning("Passphrase required for key '"+label+"'");
+                    else
+                        addWarning("Invalid passphrase for key '"+label+"'");
+                    privateKeyData = null;
+                }
+            } catch (Exception e) {
+                Exceptions.propagateIfFatal(e);
+                if (doKeyValidation) {
+                    addWarning("Unable to parse private key from '"+label+"': unknown format");
+                    privateKeyData = null;
+                }
+            }
+            return privateKey;
+        }
         
+        Set<String> warningMessages = MutableSet.of();
+        
+        private void error(String msg, String logExtension) {
+            addWarning(msg);
+            if (warnOnErrors) log.warn(msg+(logExtension==null ? "" : ": "+logExtension));
+            if (throwOnErrors) throw new IllegalStateException(msg+(logExtension==null ? "" : "; "+logExtension));
+        }
+
+        private void addWarning(String msg) {
+            addWarning(msg, null);
+        }
+        private void addWarning(String msg, String debugExtension) {
+            log.debug(msg+(debugExtension==null ? "" : "; "+debugExtension));
+            warningMessages.add(msg);
+        }
+
         @Override
         public String toString() {
-            // TODO print hash of key?
             return getClass().getSimpleName()+"["+
-                (Strings.isNonBlank(publicKeyData) ? "public-key" : "public-key")+","+
+                (Strings.isNonBlank(publicKeyData) ? publicKeyData : "public-key")+";"+
                 (Strings.isNonBlank(privateKeyData) ? "private-key" : "private-key")+","+
-                (password!=null ? "password" : "no-password")+"]";
+                (password!=null ? "password(len="+password.length()+")" : "no-password")+"]";
         }
     }
 
@@ -324,9 +437,12 @@ public class LocationConfigUtils {
             String file = fi.next();
             if (groovyTruth(file)) {
                 try {
-                    File f = new File(file);
-                    return Files.toString(f, Charsets.UTF_8);
-                } catch (IOException e) {
+                    // see comment above
+                    String result = ResourceUtils.create().getResourceAsString(file);
+                    if (result!=null) return result;
+                    log.debug("Invalid file "+file+" ; " + (!fi.hasNext() ? "no more files to try" : "trying next file")+" (null)");
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
                     log.debug("Invalid file "+file+" ; " + (!fi.hasNext() ? "no more files to try" : "trying next file"), e);
                 }
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/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 02d6bca..0c9acc8 100644
--- a/core/src/main/java/brooklyn/util/crypto/SecureKeys.java
+++ b/core/src/main/java/brooklyn/util/crypto/SecureKeys.java
@@ -19,182 +19,149 @@
 package brooklyn.util.crypto;
 
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringWriter;
 import java.security.KeyPair;
-import java.security.KeyPairGenerator;
-import java.security.KeyStore;
-import java.security.KeyStoreException;
-import java.security.NoSuchAlgorithmException;
+import java.security.PrivateKey;
+import java.security.PublicKey;
 import java.security.Security;
-import java.security.cert.CertificateException;
-import java.security.cert.X509Certificate;
-import java.util.NoSuchElementException;
-
-import javax.net.ssl.TrustManager;
-import javax.net.ssl.TrustManagerFactory;
-import javax.net.ssl.X509TrustManager;
-import javax.security.auth.x500.X500Principal;
 
+import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
 import org.bouncycastle.jce.X509Principal;
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.openssl.PEMDecryptorProvider;
+import org.bouncycastle.openssl.PEMEncryptedKeyPair;
+import org.bouncycastle.openssl.PEMKeyPair;
+import org.bouncycastle.openssl.PEMParser;
 import org.bouncycastle.openssl.PEMReader;
 import org.bouncycastle.openssl.PEMWriter;
 import org.bouncycastle.openssl.PasswordFinder;
+import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter;
+import org.bouncycastle.openssl.jcajce.JcePEMDecryptorProviderBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import brooklyn.util.exceptions.Exceptions;
+import brooklyn.util.stream.Streams;
 
+import com.google.common.base.Objects;
 import com.google.common.base.Throwables;
 
 /**
  * Utility methods for generating and working with keys
  */
-public class SecureKeys {
+public class SecureKeys extends SecureKeysWithoutBouncyCastle {
 
+    private static final Logger log = LoggerFactory.getLogger(SecureKeys.class);
+    
     static { Security.addProvider(new BouncyCastleProvider()); }
     
-    private static KeyPairGenerator defaultKeyPairGenerator = newKeyPairGenerator("RSA", 1024);  
-
-    private SecureKeys() {}
-
-    public static KeyPairGenerator newKeyPairGenerator(String algorithm, int bits) {
-        KeyPairGenerator keyPairGenerator;
-        try {
-            keyPairGenerator = KeyPairGenerator.getInstance(algorithm);
-        } catch (NoSuchAlgorithmException e) {
-            throw Exceptions.propagate(e);
-        }
-        keyPairGenerator.initialize(bits);
-        return keyPairGenerator;
+    public static class PassphraseProblem extends IllegalStateException {
+        private static final long serialVersionUID = -3382824813899223447L;
+        public PassphraseProblem() { super("Passphrase problem with this key"); }
+        public PassphraseProblem(String message) { super("Passphrase problem with this key: "+message); }
     }
     
-    public static KeyPair newKeyPair() {
-        return defaultKeyPairGenerator.generateKeyPair();
+    private SecureKeys() {}
+    
+    /** RFC1773 order, with None for other values. Normally prefer X500Principal. */
+    public static X509Principal getX509PrincipalWithCommonName(String commonName) {
+        return new X509Principal("" + "C=None," + "L=None," + "O=None," + "OU=None," + "CN=" + commonName);
     }
 
-    public static KeyPair newKeyPair(String algorithm, int bits) {
-        return newKeyPairGenerator(algorithm, bits).generateKeyPair();
-    }
+    /** reads RSA or DSA / pem style private key files (viz {@link #toPem(KeyPair)}), extracting also the public key if possible
+     * @throws IllegalStateException on errors, in particular {@link PassphraseProblem} if that is the problem */
+    public static KeyPair readPem(InputStream input, final String passphrase) {
+        // TODO cache is only for fallback "reader" strategy 
+        byte[] cache = Streams.readFully(input);
+        input = new ByteArrayInputStream(cache);
 
-    /** returns a new keystore, of the default type, and initialized to be empty.
-     * (everyone always forgets to load(null,null).) */
-    public static KeyStore newKeyStore() {
         try {
-            KeyStore store = KeyStore.getInstance(KeyStore.getDefaultType());
-            store.load(null, null);
-            return store;
-        } catch (Exception e) {
-            throw Exceptions.propagate(e);
-        }
-    }
+            PEMParser pemParser = new PEMParser(new InputStreamReader(input));
+
+            Object object = pemParser.readObject();
+            pemParser.close();
+
+            JcaPEMKeyConverter converter = new JcaPEMKeyConverter().setProvider("BC");
+            KeyPair kp = null;
+            if (object==null) {
+                throw new IllegalStateException("PEM parsing failed: missing or invalid data");
+            } else if (object instanceof PEMEncryptedKeyPair) {
+                if (passphrase==null) throw new PassphraseProblem("passphrase required");
+                try {
+                    PEMDecryptorProvider decProv = new JcePEMDecryptorProviderBuilder().build(passphrase.toCharArray());
+                    kp = converter.getKeyPair(((PEMEncryptedKeyPair) object).decryptKeyPair(decProv));
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    throw new PassphraseProblem("wrong passphrase");
+                }
+            } else  if (object instanceof PEMKeyPair) {
+                kp = converter.getKeyPair((PEMKeyPair) object);
+            } else if (object instanceof PrivateKeyInfo) {
+                PrivateKey privKey = converter.getPrivateKey((PrivateKeyInfo) object);
+                kp = new KeyPair(null, privKey);
+            } else {
+                throw new IllegalStateException("PEM parser support missing for: "+object);
+            }
+
+            return kp;
 
-    /** returns keystore of default type read from given source */
-    public static KeyStore newKeyStore(InputStream source, String passphrase) {
-        try {
-            KeyStore store = KeyStore.getInstance(KeyStore.getDefaultType());
-            store.load(source, passphrase!=null ? passphrase.toCharArray() : new char[0]);
-            return store;
         } catch (Exception e) {
-            throw Exceptions.propagate(e);
-        }
-    }
+            Exceptions.propagateIfFatal(e);
+
+            // older code relied on PEMReader, now deprecated
+            // replaced with above based on http://stackoverflow.com/questions/14919048/bouncy-castle-pemreader-pemparser
+            // passes the same tests (Jan 2015) but leaving the old code as a fallback for the time being 
+
+            input = new ByteArrayInputStream(cache);
+            try {
+                Security.addProvider(new BouncyCastleProvider());
+                PEMReader pr = new PEMReader(new InputStreamReader(input), new PasswordFinder() {
+                    public char[] getPassword() {
+                        return passphrase!=null ? passphrase.toCharArray() : new char[0];
+                    }
+                });
+                KeyPair result = (KeyPair) pr.readObject();
+                pr.close();
+                if (result==null)
+                    throw Exceptions.propagate(e);
+                
+                log.warn("PEMParser failed when PEMReader succeeded, with "+result+"; had: "+e);
 
-    /** see {@link #getTrustManager(KeyStore, Class)}, matching any type */
-    public static TrustManager getTrustManager(KeyStore trustStore) {
-        return getTrustManager(trustStore, null);
-    }
-    /** returns the trust manager inferred from trustStore, matching the type (if not null);
-     * throws exception if there are none, or if there are multiple */
-    @SuppressWarnings("unchecked")
-    public static <T extends TrustManager> T getTrustManager(KeyStore trustStore, Class<T> type) {
-        try {
-            TrustManagerFactory tmf = TrustManagerFactory.getInstance(
-                    TrustManagerFactory.getDefaultAlgorithm());
-            tmf.init(trustStore);
-            T result = null;
-            for (TrustManager tm: tmf.getTrustManagers()) {
-                if (type==null || type.isInstance(tm)) {
-                    if (result!=null)
-                        throw new IllegalStateException("Multiple trust managers matching "+type+" inferred from "+trustStore);
-                    result = (T)tm;
-                }
-            }
-            if (result!=null)
                 return result;
-            throw new NoSuchElementException("No trust manager matching "+type+" can be inferred from "+trustStore);
-        } catch (Exception e) {
-            throw Exceptions.propagate(e);
-        }
-    }
-    
-    public static X509TrustManager getTrustManager(X509Certificate certificate) {
-        try {
-            KeyStore ks = newKeyStore();
-            ks.setCertificateEntry("", certificate);
-            return getTrustManager(ks, X509TrustManager.class);
-        } catch (KeyStoreException e) {
-            throw Exceptions.propagate(e);
+
+            } catch (Exception e2) {
+                Exceptions.propagateIfFatal(e2);
+                throw Exceptions.propagate(e);
+            }
         }
     }
 
-    /** converts a certificate to the canonical implementation, commonly sun.security.x509.X509CertImpl,
-     * which is required in some places -- the Bouncy Castle X509 impl is not accepted 
-     * (e.g. where certs are chained, passed to trust manager) */
-    public static X509Certificate getCanonicalImpl(X509Certificate inCert) {
-        try {
-            KeyStore store = SecureKeys.newKeyStore();
-            store.setCertificateEntry("to-canonical", inCert);
-            ByteArrayOutputStream out = new ByteArrayOutputStream();
-            store.store(out, "".toCharArray());
-
-            KeyStore store2 = KeyStore.getInstance(KeyStore.getDefaultType());
-            store2.load(new ByteArrayInputStream(out.toByteArray()), "".toCharArray());
-            return (X509Certificate) store2.getCertificate("to-canonical");
-        } catch (Exception e) {
-            throw Exceptions.propagate(e);
-        }
+    /** because KeyPair.equals is not implemented :( */
+    public static boolean equal(KeyPair k1, KeyPair k2) {
+        return Objects.equal(k2.getPrivate(), k1.getPrivate()) && Objects.equal(k2.getPrivate(), k1.getPrivate());
     }
 
-    public static boolean isCertificateAuthorizedBy(X509Certificate candidate, X509Certificate authority) {
-        try {
-            candidate = SecureKeys.getCanonicalImpl(candidate);
-            getTrustManager(authority).checkClientTrusted(new X509Certificate[] { candidate }, "RSA");
-            return true;
-        } catch (CertificateException e) {
-            return false;
-        }
+    /** returns the PEM (base64, ie for id_rsa) string for the private key / key pair;
+     * this starts -----BEGIN PRIVATE KEY----- and ends similarly, like id_rsa.
+     * also see {@link #readPem(InputStream, String)} */
+    public static String toPem(KeyPair key) {
+        return stringPem(key);
     }
-    
-    public static X500Principal getX500PrincipalWithCommonName(String commonName) {
-        return new X500Principal("" + "C=None," + "L=None," + "O=None," + "OU=None," + "CN=" + commonName);
+
+    /** returns id_rsa.pub style file, of public key */
+    public static String toPub(KeyPair key) {
+        return AuthorizedKeysParser.encodePublicKey(key.getPublic());
     }
     
-    /** RFC1773 order, with None for other values. Normally prefer X500Principal. */
-    public static X509Principal getX509PrincipalWithCommonName(String commonName) {
-        return new X509Principal("" + "C=None," + "L=None," + "O=None," + "OU=None," + "CN=" + commonName);
-    }
-
-    public static KeyPair readPem(InputStream input, final String passphrase) {
-        try {
-            Security.addProvider(new BouncyCastleProvider());
-            PEMReader pr = new PEMReader(new InputStreamReader(input), new PasswordFinder() {
-                public char[] getPassword() {
-                    return passphrase!=null ? passphrase.toCharArray() : new char[0];
-                }
-            });
-            KeyPair result = (KeyPair) pr.readObject();
-            pr.close();
-            return result;
-        } catch (Exception e) {
-            throw Throwables.propagate(e);
-        }
+    /** opposite of {@link #toPub(KeyPair)}, given text */
+    public static PublicKey fromPub(String pubText) {
+        return AuthorizedKeysParser.decodePublicKey(pubText);
     }
 
-    /** returns the PEM (base64, ie for id_rsa) string for the private key / key pair */
+    /** @deprecated since 0.7.0, use {@link #toPem(KeyPair)} */ @Deprecated
     public static String stringPem(KeyPair key) {
         try {
             StringWriter sw = new StringWriter();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/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 46a96a1..e515aab 100644
--- a/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
+++ b/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
@@ -21,15 +21,20 @@ package brooklyn.location.basic;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
 
+import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import brooklyn.location.basic.LocationConfigUtils.OsCredential;
 import brooklyn.util.config.ConfigBag;
 
+@Test
 public class LocationConfigUtilsTest {
 
     // set these system properties differently if needed to fix your tests
     public static final String SSH_PRIVATE_KEY_FILE_WITH_TILDE = System.getProperty("sshPrivateKey", "~/.ssh/id_rsa");
     public static final String SSH_PUBLIC_KEY_FILE_WITH_TILDE = System.getProperty("sshPublicKey", "~/.ssh/id_rsa.pub");
+    // these should work as they are on classpath
+    public static final String SSH_PRIVATE_KEY_FILE_WITH_PASSPHRASE = System.getProperty("sshPrivateKeyWithPassphrase", "/brooklyn/util/crypto/sample_rsa_passphrase.pem");
     public static final String SSH_PRIVATE_KEY_FILE = System.getProperty("sshPrivateKeySample", "/brooklyn/location/basic/sample_id_rsa");
     public static final String SSH_PUBLIC_KEY_FILE = System.getProperty("sshPublicKeySample", "/brooklyn/location/basic/sample_id_rsa.pub");
     
@@ -38,26 +43,79 @@ public class LocationConfigUtilsTest {
         config.put(LocationConfigKeys.PRIVATE_KEY_DATA, "mydata");
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, SSH_PRIVATE_KEY_FILE);
         
-        String data = LocationConfigUtils.getOsCredential(config).getPrivateKeyData();
+        OsCredential creds = LocationConfigUtils.getOsCredential(config);
+        Assert.assertTrue(creds.hasKey());
+        // warnings, as it is malformed
+        Assert.assertFalse(creds.getWarningMessages().isEmpty());
+
+        String data = creds.getPrivateKeyData();
         assertEquals(data, "mydata");
     }
-    
-    public void testPreferPubilcKeyDataOverFile() throws Exception {
+
+    @Test(expectedExceptions=IllegalStateException.class)
+    public void testInvalidKeyData() throws Exception {
+        ConfigBag config = ConfigBag.newInstance();
+        config.put(LocationConfigKeys.PRIVATE_KEY_DATA, "mydata");
+        
+        OsCredential creds = LocationConfigUtils.getOsCredential(config);
+        Assert.assertTrue(creds.hasKey());
+        Assert.assertFalse(creds.getWarningMessages().isEmpty());
+        
+        creds.checkNoErrors();
+    }
+
+    public void testPreferPublicKeyDataOverFileAndNoPrivateKeyRequired() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PUBLIC_KEY_DATA, "mydata");
         config.put(LocationConfigKeys.PUBLIC_KEY_FILE, SSH_PUBLIC_KEY_FILE);
+        config.put(LocationConfigKeys.PRIVATE_KEY_FILE, "");
         
-        String data = LocationConfigUtils.getOsCredential(config).getPublicKeyData();
+        OsCredential creds = LocationConfigUtils.getOsCredential(config);
+        String data = creds.getPublicKeyData();
         assertEquals(data, "mydata");
+        Assert.assertNull(creds.get());
+        Assert.assertFalse(creds.hasPassword());
+        Assert.assertFalse(creds.hasKey());
+        // and not even any warnings here
+        Assert.assertTrue(creds.getWarningMessages().isEmpty());
     }
     
     @Test(groups="Integration")  // requires ~/.ssh/id_rsa
     public void testReadsPrivateKeyFileWithTildePath() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, SSH_PRIVATE_KEY_FILE_WITH_TILDE);
-        
-        String data = LocationConfigUtils.getOsCredential(config).skipPassphraseValidation().get();
+
+        // don't mind if it has a passphrase
+        String data = LocationConfigUtils.getOsCredential(config).doKeyValidation(false).get();
+        assertTrue(data != null && data.length() > 0);
+    }
+    
+    @Test(groups="Integration")  // requires ~/.ssh/passphrase-id_rsa
+    public void testReadsPrivateKeyFileWithPassphrase() throws Exception {
+        ConfigBag config = ConfigBag.newInstance();
+        config.put(LocationConfigKeys.PRIVATE_KEY_FILE, SSH_PRIVATE_KEY_FILE_WITH_PASSPHRASE);
+
+        OsCredential cred = LocationConfigUtils.getOsCredential(config).doKeyValidation(false);
+        String data = cred.get();
         assertTrue(data != null && data.length() > 0);
+        Assert.assertFalse(data.isEmpty());
+        
+        cred.doKeyValidation(true);
+        try {
+            cred.checkNoErrors();
+            Assert.fail("check should fail as passphrase needed");
+        } catch (IllegalStateException exception) {
+        }
+
+        config.put(LocationConfigKeys.PRIVATE_KEY_PASSPHRASE, "passphrase");
+        cred.checkNoErrors();
+        
+        config.put(LocationConfigKeys.PRIVATE_KEY_PASSPHRASE, "wrong_passphrase");
+        try {
+            cred.checkNoErrors();
+            Assert.fail("check should fail as passphrase needed");
+        } catch (IllegalStateException exception) {
+        }
     }
     
     public void testReadsPrivateKeyFileWithMultipleColonSeparatedFilesWithGoodLast() throws Exception {
@@ -81,7 +139,8 @@ public class LocationConfigUtilsTest {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PUBLIC_KEY_FILE, SSH_PUBLIC_KEY_FILE_WITH_TILDE);
         
-        String data = LocationConfigUtils.getOsCredential(config).skipPassphraseValidation().getPublicKeyData();
+        // don't mind if it has a passphrase
+        String data = LocationConfigUtils.getOsCredential(config).doKeyValidation(false).getPublicKeyData();
         assertTrue(data != null && data.length() > 0);
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/core/src/test/java/brooklyn/location/basic/SshMachineLocationIntegrationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/location/basic/SshMachineLocationIntegrationTest.java b/core/src/test/java/brooklyn/location/basic/SshMachineLocationIntegrationTest.java
index 99c2cd6..5fbe030 100644
--- a/core/src/test/java/brooklyn/location/basic/SshMachineLocationIntegrationTest.java
+++ b/core/src/test/java/brooklyn/location/basic/SshMachineLocationIntegrationTest.java
@@ -67,7 +67,7 @@ public class SshMachineLocationIntegrationTest {
         SshjToolBuilder builder = SshjTool.builder().host(sm.getAddress().getHostName()).user(sm.getUser());
         
         KeyPair data = sm.findKeyPair();
-        if (data!=null) builder.privateKeyData(SecureKeys.stringPem(data));
+        if (data!=null) builder.privateKeyData(SecureKeys.toPem(data));
         String password = sm.findPassword();
         if (password!=null) builder.password(password);
         SshjTool tool = builder.build();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/core/src/test/java/brooklyn/util/crypto/SecureKeysAndSignerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/util/crypto/SecureKeysAndSignerTest.java b/core/src/test/java/brooklyn/util/crypto/SecureKeysAndSignerTest.java
index 29caf40..1bdb0a3 100644
--- a/core/src/test/java/brooklyn/util/crypto/SecureKeysAndSignerTest.java
+++ b/core/src/test/java/brooklyn/util/crypto/SecureKeysAndSignerTest.java
@@ -22,6 +22,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.nio.charset.Charset;
 import java.security.KeyPair;
+import java.security.PublicKey;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 
@@ -29,6 +30,7 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import brooklyn.util.ResourceUtils;
+import brooklyn.util.crypto.SecureKeys.PassphraseProblem;
 import brooklyn.util.os.Os;
 
 import com.google.common.io.Files;
@@ -93,6 +95,40 @@ public class SecureKeysAndSignerTest {
         checkNonTrivial(key);
     }
 
+    @Test(expectedExceptions=IllegalStateException.class)
+    public void testReadRsaPublicKeyAsPemFails() throws Exception {
+        // should fail; see next test
+        SecureKeys.readPem(ResourceUtils.create(this).getResourceFromUrl("classpath://brooklyn/util/crypto/sample_rsa.pem.pub"), null);
+    }
+    
+    @Test
+    public void testReadRsaPublicKeyAsAuthKeysWorks() throws Exception {
+        PublicKey key = AuthorizedKeysParser.decodePublicKey(
+            ResourceUtils.create(this).getResourceAsString("classpath://brooklyn/util/crypto/sample_rsa.pem.pub"));
+        KeyPair fromPem = SecureKeys.readPem(ResourceUtils.create(this).getResourceFromUrl("classpath://brooklyn/util/crypto/sample_rsa.pem"), null);        
+        Assert.assertEquals(key, fromPem.getPublic());
+    }
+
+    @Test
+    public void testEncodeDecodeRsaPublicKey() throws Exception {
+        String data = ResourceUtils.create(this).getResourceAsString("classpath://brooklyn/util/crypto/sample_rsa.pem.pub");
+        PublicKey key = AuthorizedKeysParser.decodePublicKey(data);
+        String data2 = AuthorizedKeysParser.encodePublicKey(key);
+        Assert.assertTrue(data.contains(data2), "Expected to find '"+data2+"' in '"+data+"'");
+        PublicKey key2 = AuthorizedKeysParser.decodePublicKey(data2);
+        Assert.assertEquals(key2, key);
+    }
+
+    @Test
+    public void testEncodeDecodeDsaPublicKey() throws Exception {
+        String data = ResourceUtils.create(this).getResourceAsString("classpath://brooklyn/util/crypto/sample_dsa.pem.pub");
+        PublicKey key = AuthorizedKeysParser.decodePublicKey(data);
+        String data2 = AuthorizedKeysParser.encodePublicKey(key);
+        Assert.assertTrue(data.contains(data2), "Expected to find '"+data2+"' in '"+data+"'");
+        PublicKey key2 = AuthorizedKeysParser.decodePublicKey(data2);
+        Assert.assertEquals(key2, key);
+    }
+
     @Test
     public void testReadDsaKey() throws Exception {
         KeyPair key = SecureKeys.readPem(ResourceUtils.create(this).getResourceFromUrl("classpath://brooklyn/util/crypto/sample_dsa.pem"), null);
@@ -105,6 +141,11 @@ public class SecureKeysAndSignerTest {
         checkNonTrivial(key);
     }
 
+    @Test(expectedExceptions=PassphraseProblem.class)
+    public void testReadRsaPassphraseWithoutKeyFails() throws Exception {
+        SecureKeys.readPem(ResourceUtils.create(this).getResourceFromUrl("classpath://brooklyn/util/crypto/sample_rsa_passphrase.pem"), null);
+    }
+    
     @Test
     public void testReadRsaPassphraseKeyAndWriteWithoutPassphrase() throws Exception {
         KeyPair key = SecureKeys.readPem(ResourceUtils.create(this).getResourceFromUrl("classpath://brooklyn/util/crypto/sample_rsa_passphrase.pem"), "passphrase");

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/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 db9c9d6..ec76330 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
@@ -30,6 +30,7 @@ import static org.jclouds.scriptbuilder.domain.Statements.exec;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
+import java.security.KeyPair;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -112,6 +113,7 @@ import brooklyn.management.AccessController;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.config.ConfigBag;
+import brooklyn.util.crypto.SecureKeys;
 import brooklyn.util.exceptions.CompoundRuntimeException;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.exceptions.ReferenceWithError;
@@ -195,6 +197,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
     private static final Pattern LIST_PATTERN = Pattern.compile("^\\[(.*)\\]$");
     private static final Pattern INTEGER_PATTERN = Pattern.compile("^\\d*$");
     
+    private static boolean loggedSshKeysHint = false;
+    
     private final Map<String,Map<String, ? extends Object>> tagMapping = Maps.newLinkedHashMap();
 
     @SetFromFlag // so it's persisted
@@ -650,6 +654,9 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 // only happens if something broke above...
                 userCredentials = LoginCredentials.fromCredentials(node.getCredentials());
             }
+            // store the credentials, in case they have changed
+            setup.putIfNotNull(JcloudsLocationConfig.PASSWORD, userCredentials.getOptionalPassword().orNull());
+            setup.putIfNotNull(JcloudsLocationConfig.PRIVATE_KEY_DATA, userCredentials.getOptionalPrivateKey().orNull());
             
             // Wait for the VM to be reachable over SSH
             if (waitForSshable) {
@@ -1306,12 +1313,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
      *   
      * @param image  The image being used to create the VM
      * @param config Configuration for creating the VM
-     * @return       The commands required to create the user, along with the expected login credentials for that user.
+     * @return       The commands required to create the user, along with the expected login credentials for that user,
+     * or null if we are just going to use those from jclouds.
      */
     protected UserCreation createUserStatements(@Nullable Image image, ConfigBag config) {
-        //NB: we ignore private key here because, by default we probably should not be installing it remotely;
-        //also, it may not be valid for first login (it is created before login e.g. on amazon, so valid there;
-        //but not elsewhere, e.g. on rackspace).
+        //NB: private key is not installed remotely, just used to get/validate the public key
         
         LoginCredentials createdUserCreds = null;
         String user = getUser(config);
@@ -1320,21 +1326,23 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         Boolean dontCreateUser = config.get(DONT_CREATE_USER);
         Boolean grantUserSudo = config.get(GRANT_USER_SUDO);
         OsCredential credential = LocationConfigUtils.getOsCredential(config);
-        String newPassword = Strings.isNonBlank(credential.getPassword()) ? credential.getPassword() : Identifiers.makeRandomId(12);
+        credential.checkNoErrors().logAnyWarnings();
+        String passwordToSet = Strings.isNonBlank(credential.getPassword()) ? credential.getPassword() : Identifiers.makeRandomId(12);
         List<Statement> statements = Lists.newArrayList();
         
         if (groovyTruth(dontCreateUser)) {
-            // TODO For dontCreateUser, we probably only want to treat it special if user was explicitly supplied
-            // (rather than it just being the default config key value). If user was explicit, then should
-            // set the password + authorize the key for that user. Presumably the caller knows that this
-            // user pre-exists on the given VM image.
+            // dontCreateUser:
+            // if caller has not specified a user, we'll just continue to use the loginUser;
+            // if caller *has*, we set up our credentials assuming that user and credentials already exist
+            
             if (!groovyTruth(user)) {
-                // loginCreds result will be null; use creds returned by jclouds on the node
+                // createdUserCreds returned from this method will be null; 
+                // we will use the creds returned by jclouds on the node
                 LOG.info("Not setting up any user (subsequently using loginUser {})", user, loginUser);
                 config.put(USER, loginUser);
                 
             } else {
-                LOG.info("Not creating user {}, and not setting its password or authorizing keys", user);
+                LOG.info("Not creating user {}, and not installing its password or authorizing keys (assuming it exists)", user);
 
                 if (credential.isUsingPassword()) {
                     createdUserCreds = LoginCredentials.builder().user(user).password(credential.get()).build();
@@ -1351,48 +1359,91 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             }
 
             // Using the pre-existing loginUser; setup the publicKey/password so can login as expected
-            if (Strings.isNonBlank(newPassword)) {
-                statements.add(new ReplaceShadowPasswordEntry(Sha512Crypt.function(), user, newPassword));
-                createdUserCreds = LoginCredentials.builder().user(user).password(newPassword).build();
+            if (Strings.isNonBlank(passwordToSet)) {
+                statements.add(new ReplaceShadowPasswordEntry(Sha512Crypt.function(), user, passwordToSet));
+                createdUserCreds = LoginCredentials.builder().user(user).password(passwordToSet).build();
             }
             if (Strings.isNonBlank(credential.getPublicKeyData())) {
                 statements.add(new AuthorizeRSAPublicKeys("~"+user+"/.ssh", ImmutableList.of(credential.getPublicKeyData())));
-                if (Strings.isNonBlank(credential.getPrivateKeyData()) && createdUserCreds==null) {
+                if (!credential.isUsingPassword() && Strings.isNonBlank(credential.getPrivateKeyData())) {
                     createdUserCreds = LoginCredentials.builder().user(user).privateKey(credential.getPrivateKeyData()).build();
                 }
             }
             
         } else {
+            String pubKey = credential.getPublicKeyData();
+            String privKey = credential.getPrivateKeyData();
+            
+            if (credential.get()==null) {
+                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. "
+                        + "Create ~/.ssh/id_rsa or "
+                        + "set "+PRIVATE_KEY_FILE.getName()+" / "+PRIVATE_KEY_PASSPHRASE.getName()+" / "+PASSWORD.getName()+" "
+                        + "as appropriate for this location if you wish to be able to log in without Brooklyn.");
+                }
+                KeyPair newKeyPair = SecureKeys.newKeyPair();
+                pubKey = SecureKeys.toPub(newKeyPair);
+                privKey = SecureKeys.toPem(newKeyPair);
+                LOG.debug("Brooklyn key being created for "+user+" at new machine "+this+" is:\n"+privKey);
+            }
+            // ensure credential is not used any more, as we have extracted al useful info
+            credential = null;
+            
             // Create the user
             // note AdminAccess requires _all_ fields set, due to http://code.google.com/p/jclouds/issues/detail?id=1095
             AdminAccess.Builder adminBuilder = AdminAccess.builder()
                     .adminUsername(user)
-                    .adminPassword(newPassword)
-                    .grantSudoToAdminUser(groovyTruth(grantUserSudo))
-                    .resetLoginPassword(true)
-                    .loginPassword(newPassword);
-
-            if (Strings.isNonBlank(credential.getPublicKeyData())) {
-                adminBuilder.authorizeAdminPublicKey(true).adminPublicKey(credential.getPublicKeyData());
+                    .grantSudoToAdminUser(groovyTruth(grantUserSudo));
+                    // login user's password now set to something random
+                    // (could explicitly set with loginPassword)
+            
+            Boolean useKey = null;
+            if (passwordToSet!=null) {
+                adminBuilder.adminPassword(passwordToSet);
+                useKey = false;
             } else {
-                adminBuilder.authorizeAdminPublicKey(false).adminPublicKey("ignored");
+                // will be using a key, but a password should be set
+                adminBuilder.adminPassword(Identifiers.makeRandomId(12));
             }
             
-            // TODO Brittle code! This only works with adminPrivateKey set to non-null; 
-            // otherwise, in AdminAccess.build, if adminUsername != null && adminPassword != null 
-            // then authorizeAdminPublicKey is reset to null!
-            adminBuilder.installAdminPrivateKey(false).adminPrivateKey("ignore");
+            if (config.get(JcloudsLocationConfig.DISABLE_ROOT_AND_PASSWORD_SSH)) {
+                // the default - set root password which we forget, because we have sudo acct
+                // (and lock out root and passwords from ssh)
+                adminBuilder.resetLoginPassword(true);
+                adminBuilder.loginPassword(Identifiers.makeRandomId(12));
+            } else {
+                adminBuilder.resetLoginPassword(false);
+                adminBuilder.loginPassword(Identifiers.makeRandomId(12)+"-ignored");                
+            }
+            
+            if (Strings.isNonBlank(pubKey)) {
+                adminBuilder.authorizeAdminPublicKey(true).adminPublicKey(pubKey);
+                useKey = true;
+            } else {
+                adminBuilder.authorizeAdminPublicKey(false).adminPublicKey(Identifiers.makeRandomId(12)+"-ignored");
+            }
+            
+            if (useKey==null) {
+                throw new IllegalStateException("Misconfiguration: neither a key or password known for the user being created");
+            }
+            
+            // jclouds wants us to give it the private key, otherwise it might refuse to authorize the public key
+            // (in AdminAccess.build, if adminUsername != null && adminPassword != null);
+            // we don't want to give it the private key, but we *do* want the public key authorized;
+            // this code seems to trigger that.
+            // (we build the creds below)
+            adminBuilder.installAdminPrivateKey(false).adminPrivateKey(Identifiers.makeRandomId(12)+"-ignored");
             
             // lock SSH (key only) iff there is a public key and no password supplied
-            boolean useKey = !credential.isUsingPassword() && Strings.isNonBlank(credential.getPublicKeyData()) && Strings.isNonBlank(credential.getPrivateKeyData());
-            adminBuilder.lockSsh(useKey);
+            adminBuilder.lockSsh(useKey && !config.get(JcloudsLocationConfig.DISABLE_ROOT_AND_PASSWORD_SSH));
             
             statements.add(adminBuilder.build());
             
             if (useKey) {
-                createdUserCreds = LoginCredentials.builder().user(user).privateKey(credential.getPrivateKeyData()).build();
-            } else {
-                createdUserCreds = LoginCredentials.builder().user(user).password(newPassword).build();
+                createdUserCreds = LoginCredentials.builder().user(user).privateKey(privKey).build();
+            } else if (passwordToSet!=null) {
+                createdUserCreds = LoginCredentials.builder().user(user).password(passwordToSet).build();
             }
         }
         
@@ -1470,7 +1521,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 throw new IllegalArgumentException("Jclouds node for rebind matching multiple, looking for id="+rawId+" and hostname="+rawHostname+": "+candidateNodes);
 
             NodeMetadata node = Iterables.getOnlyElement(candidateNodes);
-            String pkd = LocationConfigUtils.getOsCredential(setup).getPrivateKeyData();
+            String pkd = LocationConfigUtils.getOsCredential(setup).checkNoErrors().logAnyWarnings().getPrivateKeyData();
             if (Strings.isNonBlank(pkd)) {
                 LoginCredentials expectedCredentials = LoginCredentials.fromCredentials(new Credentials(user, pkd));
                 //override credentials
@@ -1690,7 +1741,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
      */
     protected LoginCredentials extractVmCredentials(ConfigBag setup, NodeMetadata node) {
         String user = getUser(setup);
-        OsCredential localCredentials = LocationConfigUtils.getOsCredential(setup);
+        OsCredential localCredentials = LocationConfigUtils.getOsCredential(setup).checkNoErrors();
         LoginCredentials nodeCredentials = LoginCredentials.fromCredentials(node.getCredentials());
 
         LOG.debug("Credentials extracted for {}: {}/{} with {}/{}", new Object[] { node, 
@@ -1759,8 +1810,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                             setup.getDescription(), Time.makeTimeStringRounded(delayMs),
                             user, vmIp, vmPort,
                             Objects.equal(user, getUser(setup)) ? "" : " (setup user is different: "+getUser(setup)+")",
-                            (password.isPresent() ? password.get() : "<absent>"),
-                            (key.isPresent() ? key.get() : "<absent>"),
+                            password.or("<absent>"),
+                            key.or("<absent>")
                     });
         }
         

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/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 0346940..91a47dd 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationConfig.java
@@ -77,7 +77,11 @@ public interface JcloudsLocationConfig extends CloudLocationConfig {
             "Whether to skip creation of 'user' when provisioning machines (default false)", false);
     public static final ConfigKey<Boolean> GRANT_USER_SUDO = ConfigKeys.newBooleanConfigKey("grantUserSudo",
             "Whether to grant the created user sudo privileges. Irrelevant if dontCreateUser is true. Default: true.", true);
-
+    public static final ConfigKey<Boolean> DISABLE_ROOT_AND_PASSWORD_SSH = ConfigKeys.newBooleanConfigKey("disableRootAndPasswordSsh",
+        "Whether to disable direct SSH access for root and disable password-based SSH, "
+        + "if creating a user with a key-based login; "
+        + "defaults to true (set false to leave root users alone)", true);
+    
     public static final ConfigKey<LoginCredentials> CUSTOM_CREDENTIALS = new BasicConfigKey<LoginCredentials>(LoginCredentials.class,
             "customCredentials", "Custom jclouds LoginCredentials object to be used to connect to the VM", null);
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/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
new file mode 100644
index 0000000..e64454e
--- /dev/null
+++ b/utils/common/src/main/java/brooklyn/util/crypto/AuthorizedKeysParser.java
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache 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://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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 brooklyn.util.crypto;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.math.BigInteger;
+import java.security.KeyFactory;
+import java.security.PublicKey;
+import java.security.interfaces.DSAPublicKey;
+import java.security.interfaces.RSAPublicKey;
+import java.security.spec.DSAPublicKeySpec;
+import java.security.spec.RSAPublicKeySpec;
+
+import brooklyn.util.exceptions.Exceptions;
+
+import com.google.common.io.BaseEncoding;
+
+public class AuthorizedKeysParser {
+
+    public static PublicKey decodePublicKey(String keyLine) {
+        try {
+            ByteArrayInputStream stream = null;
+
+            // look for the Base64 encoded part of the line to decode
+            // both ssh-rsa and ssh-dss begin with "AAAA" due to the length bytes
+            for (String part : keyLine.split(" ")) {
+                if (part.startsWith("AAAA")) {
+                    stream = new ByteArrayInputStream(BaseEncoding.base64().decode(part));
+                    break;
+                }
+            }
+            if (stream == null)
+                throw new IllegalArgumentException("Encoded public key should include phrase beginning AAAA.");
+
+            String type = readType(stream);
+            if (type.equals("ssh-rsa")) {
+                BigInteger e = readBigInt(stream);
+                BigInteger m = readBigInt(stream);
+                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);
+                DSAPublicKeySpec spec = new DSAPublicKeySpec(y, p, q, g);
+                return KeyFactory.getInstance("DSA").generatePublic(spec);
+            } else {
+                throw new IllegalArgumentException("Unknown public key type " + type);
+            }
+            
+        } catch (Exception e) {
+            Exceptions.propagateIfFatal(e);
+            throw new IllegalArgumentException("Error parsing authorized_keys/SSH2 format public key: "+e);
+        }
+    }
+
+    private static int readInt(InputStream stream) throws IOException {
+        return ((stream.read() & 0xFF) << 24) | ((stream.read() & 0xFF) << 16)
+            | ((stream.read() & 0xFF) << 8) | (stream.read() & 0xFF);
+    }
+    
+    private static byte[] readBytesWithLength(InputStream stream) throws IOException {
+        int len = readInt(stream);
+        byte[] result = new byte[len];
+        stream.read(result);
+        return result;
+    }
+    
+    private static void writeInt(OutputStream stream, int v) throws IOException {
+        for (int shift = 24; shift >= 0; shift -= 8)
+            stream.write((v >>> shift) & 0xFF);
+    }
+    private static void writeBytesWithLength(OutputStream stream, byte[] buf) throws IOException {
+        writeInt(stream, buf.length);
+        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)); }
+
+    public static String encodePublicKey(PublicKey key) {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        try {
+            String type = null;
+            if (key==null) { 
+                return null;
+            } else if (key instanceof RSAPublicKey) {
+                type = "ssh-rsa";
+                writeBytesWithLength(out, type.getBytes()); 
+                writeBytesWithLength(out, ((RSAPublicKey)key).getPublicExponent().toByteArray());
+                writeBytesWithLength(out, ((RSAPublicKey)key).getModulus().toByteArray()); 
+            } else if (key instanceof DSAPublicKey) {
+                type = "ssh-dss";
+                writeBytesWithLength(out, type.getBytes());
+                writeBytesWithLength(out, ((DSAPublicKey)key).getParams().getP().toByteArray()); 
+                writeBytesWithLength(out, ((DSAPublicKey)key).getParams().getQ().toByteArray()); 
+                writeBytesWithLength(out, ((DSAPublicKey)key).getParams().getG().toByteArray()); 
+                writeBytesWithLength(out, ((DSAPublicKey)key).getY().toByteArray()); 
+            } else {
+                throw new IllegalStateException("Unsupported public key type for encoding: "+key);
+            }
+            out.close();
+            
+            return type+" "+BaseEncoding.base64().encode(out.toByteArray());
+        } catch (Exception e) {
+            // shouldn't happen, as it's a byte stream...
+            throw Exceptions.propagate(e);
+        }
+    }
+
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c3828fbf/utils/common/src/main/java/brooklyn/util/crypto/SecureKeysWithoutBouncyCastle.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/crypto/SecureKeysWithoutBouncyCastle.java b/utils/common/src/main/java/brooklyn/util/crypto/SecureKeysWithoutBouncyCastle.java
new file mode 100644
index 0000000..3235fbe
--- /dev/null
+++ b/utils/common/src/main/java/brooklyn/util/crypto/SecureKeysWithoutBouncyCastle.java
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache 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://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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 brooklyn.util.crypto;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.util.NoSuchElementException;
+
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+import javax.security.auth.x500.X500Principal;
+
+import brooklyn.util.exceptions.Exceptions;
+
+/**
+ * Utility methods for generating and working with keys, with no BC dependencies
+ */
+public class SecureKeysWithoutBouncyCastle {
+
+    private static KeyPairGenerator defaultKeyPairGenerator = newKeyPairGenerator("RSA", 1024);  
+
+    protected SecureKeysWithoutBouncyCastle() {}
+
+    public static KeyPairGenerator newKeyPairGenerator(String algorithm, int bits) {
+        KeyPairGenerator keyPairGenerator;
+        try {
+            keyPairGenerator = KeyPairGenerator.getInstance(algorithm);
+        } catch (NoSuchAlgorithmException e) {
+            throw Exceptions.propagate(e);
+        }
+        keyPairGenerator.initialize(bits);
+        return keyPairGenerator;
+    }
+    
+    public static KeyPair newKeyPair() {
+        return defaultKeyPairGenerator.generateKeyPair();
+    }
+
+    public static KeyPair newKeyPair(String algorithm, int bits) {
+        return newKeyPairGenerator(algorithm, bits).generateKeyPair();
+    }
+
+    /** returns a new keystore, of the default type, and initialized to be empty.
+     * (everyone always forgets to load(null,null).) */
+    public static KeyStore newKeyStore() {
+        try {
+            KeyStore store = KeyStore.getInstance(KeyStore.getDefaultType());
+            store.load(null, null);
+            return store;
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        }
+    }
+
+    /** returns keystore of default type read from given source */
+    public static KeyStore newKeyStore(InputStream source, String passphrase) {
+        try {
+            KeyStore store = KeyStore.getInstance(KeyStore.getDefaultType());
+            store.load(source, passphrase!=null ? passphrase.toCharArray() : new char[0]);
+            return store;
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        }
+    }
+
+    /** see {@link #getTrustManager(KeyStore, Class)}, matching any type */
+    public static TrustManager getTrustManager(KeyStore trustStore) {
+        return getTrustManager(trustStore, null);
+    }
+    /** returns the trust manager inferred from trustStore, matching the type (if not null);
+     * throws exception if there are none, or if there are multiple */
+    @SuppressWarnings("unchecked")
+    public static <T extends TrustManager> T getTrustManager(KeyStore trustStore, Class<T> type) {
+        try {
+            TrustManagerFactory tmf = TrustManagerFactory.getInstance(
+                    TrustManagerFactory.getDefaultAlgorithm());
+            tmf.init(trustStore);
+            T result = null;
+            for (TrustManager tm: tmf.getTrustManagers()) {
+                if (type==null || type.isInstance(tm)) {
+                    if (result!=null)
+                        throw new IllegalStateException("Multiple trust managers matching "+type+" inferred from "+trustStore);
+                    result = (T)tm;
+                }
+            }
+            if (result!=null)
+                return result;
+            throw new NoSuchElementException("No trust manager matching "+type+" can be inferred from "+trustStore);
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        }
+    }
+    
+    public static X509TrustManager getTrustManager(X509Certificate certificate) {
+        try {
+            KeyStore ks = newKeyStore();
+            ks.setCertificateEntry("", certificate);
+            return getTrustManager(ks, X509TrustManager.class);
+        } catch (KeyStoreException e) {
+            throw Exceptions.propagate(e);
+        }
+    }
+
+    /** converts a certificate to the canonical implementation, commonly sun.security.x509.X509CertImpl,
+     * which is required in some places -- the Bouncy Castle X509 impl is not accepted 
+     * (e.g. where certs are chained, passed to trust manager) */
+    public static X509Certificate getCanonicalImpl(X509Certificate inCert) {
+        try {
+            KeyStore store = newKeyStore();
+            store.setCertificateEntry("to-canonical", inCert);
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            store.store(out, "".toCharArray());
+
+            KeyStore store2 = KeyStore.getInstance(KeyStore.getDefaultType());
+            store2.load(new ByteArrayInputStream(out.toByteArray()), "".toCharArray());
+            return (X509Certificate) store2.getCertificate("to-canonical");
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        }
+    }
+
+    public static boolean isCertificateAuthorizedBy(X509Certificate candidate, X509Certificate authority) {
+        try {
+            candidate = getCanonicalImpl(candidate);
+            getTrustManager(authority).checkClientTrusted(new X509Certificate[] { candidate }, "RSA");
+            return true;
+        } catch (CertificateException e) {
+            return false;
+        }
+    }
+    
+    public static X500Principal getX500PrincipalWithCommonName(String commonName) {
+        return new X500Principal("" + "C=None," + "L=None," + "O=None," + "OU=None," + "CN=" + commonName);
+    }
+    
+}


Mime
View raw message