AMBARI-2397. Unencrypted master key stored in temporary file. (swagle)

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1494425 13f79535-47bb-0310-9956-ffa450edef68

ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java

@@ -36,6 +36,7 @@ import java.io.InputStream;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 
 /**
@@ -225,6 +226,7 @@ public class Configuration {
   private Map<String, String> configsMap;
 
   private CredentialProvider credentialProvider = null;
+  private volatile boolean credentialProviderInitialized = false;
 
   public Configuration() {
     this(readConfigFile());
@@ -311,8 +313,11 @@ public class Configuration {
     }
   }
 
-  private void loadCredentialProvider() {
-    if (credentialProvider == null) {
+  private synchronized void loadCredentialProvider() {
+    if (credentialProviderInitialized) {
+      return;
+    }
+    else {
       try {
         this.credentialProvider = new CredentialProvider(null,
           getMasterKeyLocation(), isMasterKeyPersisted());
@@ -323,6 +328,7 @@ public class Configuration {
         }
         this.credentialProvider = null;
       }
+      credentialProviderInitialized = true;
     }
   }
 
@@ -491,6 +497,7 @@ public class Configuration {
   public String getDatabasePassword() {
     String passwdProp = properties.getProperty(SERVER_JDBC_USER_PASSWD_KEY);
     String dbpasswd = readPasswordFromStore(passwdProp);
+
     if (dbpasswd != null)
       return dbpasswd;
     else

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java

@@ -114,6 +114,10 @@ public class AmbariServer {
   }
 
   public void run() throws Exception {
+    // Initialize meta info before heartbeat monitor
+    ambariMetaInfo.init();
+    LOG.info("********* Meta Info initialized **********");
+
     performStaticInjection();
     addInMemoryUsers();
     server = new Server();
@@ -310,9 +314,6 @@ public class AmbariServer {
       serverForAgent.setStopAtShutdown(true);
       springAppContext.start();
 
-      LOG.info("********* Initializing Meta Info **********");
-      ambariMetaInfo.init();
-
       String osType = getServerOsType();
       if (osType == null || osType.isEmpty()) {
         throw new RuntimeException(Configuration.OS_VERSION_KEY + " is not "

ambari-server/src/main/java/org/apache/ambari/server/security/encryption/CredentialProvider.java

@@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Random;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -44,10 +45,6 @@ public class CredentialProvider {
 
   public CredentialProvider(String masterKey, String masterKeyLocation,
               boolean isMasterKeyPersisted) throws AmbariException {
-    if (masterKeyLocation == null)
-      throw new IllegalArgumentException("Master key location needed for " +
-        "Credential Provider initialization.");
-
     MasterKeyService masterKeyService;
     if (masterKey != null) {
       masterKeyService = new MasterKeyServiceImpl(masterKey);
@@ -75,8 +72,13 @@ public class CredentialProvider {
     addAliasToCredentialStore(alias, passwordString);
   }
 
-  private void addAliasToCredentialStore(String alias, String passwordString)
+  public void addAliasToCredentialStore(String alias, String passwordString)
     throws AmbariException {
+    if (alias == null || alias.isEmpty())
+      throw new IllegalArgumentException("Alias cannot be null or empty.");
+    if (passwordString == null || passwordString.isEmpty())
+      throw new IllegalArgumentException("Empty or null password not allowed" +
+        ".");
     keystoreService.addCredential(alias, passwordString);
   }
 
@@ -101,6 +103,10 @@ public class CredentialProvider {
       strPasswd.length() - 1);
   }
 
+  protected CredentialStoreService getKeystoreService() {
+    return keystoreService;
+  }
+
   /**
    * Credential Store entry point
    * args[0] => Action (GET/PUT)
@@ -124,9 +130,10 @@ public class CredentialProvider {
         System.exit(1);
       }
       // None - To avoid incorrectly assuming redirection as argument
-      if (args.length > 3 && !args[2].isEmpty() && !args[2].equalsIgnoreCase
+      if (args.length > 3 && !args[3].isEmpty() && !args[3].equalsIgnoreCase
         ("None")) {
         masterKey = args[3];
+        LOG.debug("Master key provided as an argument.");
       }
       try {
         credentialProvider = new CredentialProvider(masterKey,
@@ -136,7 +143,7 @@ public class CredentialProvider {
         ex.printStackTrace();
         System.exit(1);
       }
-      LOG.info("action => " + action + ", alias =>" + alias);
+      LOG.info("action => " + action + ", alias => " + alias);
       if (action.equalsIgnoreCase("PUT")) {
         String password = null;
         if (args.length > 2 && !args[2].isEmpty()) {

ambari-server/src/main/java/org/apache/ambari/server/security/encryption/CredentialStoreServiceImpl.java

@@ -59,6 +59,7 @@ public class CredentialStoreServiceImpl implements CredentialStoreService {
 
     final File keyStoreFile = new File(keyStoreDir + File.separator +
       CREDENTIALS_SUFFIX);
+    LOG.debug("keystoreFile => " + keyStoreFile.getAbsolutePath());
     if (!isCredentialStoreCreated) {
       createCredentialStore();
     }
@@ -95,8 +96,6 @@ public class CredentialStoreServiceImpl implements CredentialStoreService {
     if (ks != null && alias != null && !alias.isEmpty()) {
       try {
         LOG.debug("keystore = " + ks.aliases());
-        LOG.debug("masterkey = " + new String(masterService.getMasterSecret()
-        ));
         Key key = ks.getKey(alias, masterService.getMasterSecret());
         if (key == null) {
           throw new AmbariException("Credential not found for alias: " +

ambari-server/src/main/java/org/apache/ambari/server/security/encryption/MasterKeyServiceImpl.java

@@ -85,6 +85,8 @@ public class MasterKeyServiceImpl implements MasterKeyService {
    * @param isPersisted
    */
   public MasterKeyServiceImpl(String masterFileLocation, boolean isPersisted) {
+    if (masterFileLocation == null || masterFileLocation.isEmpty())
+      throw new IllegalArgumentException("Master Key location not provided.");
     if (isPersisted) {
       File masterFile = new File(masterFileLocation);
       if (masterFile.exists()) {
@@ -104,8 +106,8 @@ public class MasterKeyServiceImpl implements MasterKeyService {
       if (key != null) {
         this.master = key.toCharArray();
       } else {
-        LOG.error("Master key: " + Configuration.MASTER_KEY_ENV_PROP + " is" +
-          " not provided as a System property or an environment varialble.");
+        LOG.error("Master key is not provided as a System property or an " +
+          "environment varialble.");
       }
     }
   }
@@ -146,11 +148,16 @@ public class MasterKeyServiceImpl implements MasterKeyService {
           File keyFile = new File(keyPath);
           if (keyFile.exists()) {
             try {
-              key = FileUtils.readFileToString(keyFile);
+              initializeFromFile(keyFile);
+              if (this.master != null)
+                key = new String(this.master);
               FileUtils.deleteQuietly(keyFile);
             } catch (IOException e) {
               LOG.error("Cannot read master key from file: " + keyPath);
               e.printStackTrace();
+            } catch (Exception e) {
+              LOG.error("Cannot read master key from file: " + keyPath);
+              e.printStackTrace();
             }
           }
         }
@@ -205,10 +212,6 @@ public class MasterKeyServiceImpl implements MasterKeyService {
       this.master = new String(aes.decrypt(Base64.decodeBase64(parts[0]),
         Base64.decodeBase64(parts[1]), Base64.decodeBase64(parts[2])),
         "UTF8").toCharArray();
-      LOG.info("key: " + line);
-      LOG.info("master: " + new String(aes.decrypt(Base64.decodeBase64(parts[0]),
-        Base64.decodeBase64(parts[1]), Base64.decodeBase64(parts[2])),
-        "UTF8"));
     } catch (IOException e) {
       e.printStackTrace();
       throw e;

ambari-server/src/main/python/ambari-server.py

@@ -1960,9 +1960,8 @@ def start(args):
       masterKey = get_validated_string_input("Please provide master key " +\
                     "for unlocking credential store: ", "", ".*", "", False)
       tempDir = tempfile.gettempdir()
-      tempFilePath = tempDir + os.sep + "ambari.passwd"
-      with open(tempFilePath, 'w+') as file:
-        file.write(masterKey)
+      tempFilePath = tempDir + os.sep + "masterkey"
+      save_master_key(masterKey, tempFilePath, True)
       os.chmod(tempFilePath, stat.S_IREAD | stat.S_IWRITE)
 
       if tempFilePath is not None:
@@ -2300,7 +2299,7 @@ def setup_master_key(resetKey=False):
         persist)
     elif not persist and masterKeyFile is not None:
       try:
-        #os.remove(masterKeyFile)
+        os.remove(masterKeyFile)
         print_warning_msg("Master key exists although security " +\
                           "is disabled. location: " + str(masterKeyFile))
       except Exception, e:
@@ -2310,6 +2309,11 @@ def setup_master_key(resetKey=False):
     update_properties({SECURITY_KEY_IS_PERSISTED : persist})
 
   if resetKey:
+    # Blow up the credential store made with previous key
+    store_file = get_credential_store_location(properties)
+    if os.path.exists(store_file):
+      os.remove(store_file)
+
     # Encrypt the passwords with new key
     try:
       db_password_alias = properties[JDBC_PASSWORD_PROPERTY]
@@ -2318,13 +2322,21 @@ def setup_master_key(resetKey=False):
       print_warning_msg("KeyError: " + str(e))
 
     if db_password_alias is not None and is_alias_string(db_password_alias):
-      configure_database_password(True, False)
+      configure_database_password(True, key, True)
 
     if ldap_password_alias is not None and is_alias_string(ldap_password_alias):
       configure_ldap_password(True)
 
   return key, True, persist
 
+def get_credential_store_location(properties):
+  store_loc = properties[SECURITY_KEYS_DIR]
+  if store_loc is None or store_loc == "":
+    store_loc = "/var/lib/ambari-server/keys/credentials.jceks"
+  else:
+    store_loc += os.sep + "credentials.jceks"
+  return store_loc
+
 def get_master_key_location(properties):
   keyLocation = properties[SECURITY_MASTER_KEY_LOCATION]
   if keyLocation is None or keyLocation == "":
@@ -2374,7 +2386,7 @@ def read_passwd_for_alias(alias, masterKey=""):
     os.chmod(tempFilePath, stat.S_IREAD | stat.S_IWRITE)
     file.close()
 
-    if masterKey is None:
+    if masterKey is None or masterKey == "":
       masterKey = "None"
 
     command = SECURITY_PROVIDER_GET_CMD.format(jdk_path,
@@ -2401,7 +2413,7 @@ def save_passwd_for_alias(alias, passwd, masterKey=""):
                       "JDK manually to " + JDK_INSTALL_DIR)
       return 1
 
-    if masterKey is None:
+    if masterKey is None or masterKey == "":
       masterKey = "None"
 
     command = SECURITY_PROVIDER_PUT_CMD.format(jdk_path, get_conf_dir(),

ambari-server/src/test/java/org/apache/ambari/server/security/encryption/MasterKeyServiceTest.java

@@ -38,14 +38,14 @@ import static org.powermock.api.easymock.PowerMock.replayAll;
 import static org.powermock.api.easymock.PowerMock.verifyAll;
 
 @RunWith(PowerMockRunner.class)
-@PowerMockIgnore("javax.crypto.*")
+@PowerMockIgnore({"javax.crypto.*", "org.apache.log4j.*"})
 @PrepareForTest({ MasterKeyServiceImpl.class })
 public class MasterKeyServiceTest extends TestCase {
   @Rule
   public TemporaryFolder tmpFolder = new TemporaryFolder();
   private String fileDir;
   private static final Log LOG = LogFactory.getLog
-    (CredentialStoreServiceTest.class);
+    (MasterKeyServiceTest.class);
 
   @Override
   protected void setUp() throws Exception {
@@ -70,7 +70,7 @@ public class MasterKeyServiceTest extends TestCase {
   }
 
   @Test
-  public void testReadFromEnv() throws Exception {
+  public void testReadFromEnvAsKey() throws Exception {
     Map<String, String> mapRet = new HashMap<String, String>();
     mapRet.put(Configuration.MASTER_KEY_ENV_PROP, "ThisisSomePassPhrase");
     mockStatic(System.class);
@@ -84,6 +84,29 @@ public class MasterKeyServiceTest extends TestCase {
       new String(ms.getMasterSecret()));
   }
 
+  @Test
+  public void testReadFromEnvAsPath() throws Exception {
+    // Create a master key
+    MasterKeyService ms = new MasterKeyServiceImpl("ThisisSomePassPhrase",
+      fileDir + File.separator + "master", true);
+    Assert.assertTrue(ms.isMasterKeyInitialized());
+    File f = new File(fileDir + File.separator + "master");
+    Assert.assertTrue(f.exists());
+
+    Map<String, String> mapRet = new HashMap<String, String>();
+    mapRet.put(Configuration.MASTER_KEY_LOCATION, f.getAbsolutePath());
+    mockStatic(System.class);
+    expect(System.getenv()).andReturn(mapRet);
+    replayAll();
+    ms = new MasterKeyServiceImpl();
+    verifyAll();
+    Assert.assertTrue(ms.isMasterKeyInitialized());
+    Assert.assertNotNull(ms.getMasterSecret());
+    Assert.assertEquals("ThisisSomePassPhrase",
+      new String(ms.getMasterSecret()));
+    Assert.assertFalse(f.exists());
+  }
+
   @Override
   protected void tearDown() throws Exception {
     tmpFolder.delete();

ambari-server/src/test/python/TestAmbaryServer.py

@@ -1299,6 +1299,7 @@ class TestAmbariServer(TestCase):
     self.assertTrue(setup_db_mock.called)
 
 
+  @patch.object(ambari_server, 'save_master_key')
   @patch('os.chmod', autospec=True)
   @patch.object(ambari_server, 'get_validated_string_input')
   @patch("os.environ")
@@ -1323,7 +1324,8 @@ class TestAmbariServer(TestCase):
                  print_error_msg_mock, find_jdk_mock, search_file_mock,
                  print_info_msg_mock, popenMock, openMock, pexistsMock,
                  killMock, get_ambari_properties_mock, os_environ_mock,
-                 get_validated_string_input_method, os_chmod_method):
+                 get_validated_string_input_method, os_chmod_method,
+                 save_master_key_method):
     args = MagicMock()
 
     f = MagicMock()
@@ -1487,6 +1489,7 @@ class TestAmbariServer(TestCase):
     ambari_server.start(args)
 
     self.assertFalse(get_validated_string_input_method.called)
+    self.assertFalse(save_master_key_method.called)
     popen_arg = popenMock.call_args[1]['env']
     self.assertEquals(os_environ_mock.copy.return_value, popen_arg)
 
@@ -1505,6 +1508,7 @@ class TestAmbariServer(TestCase):
     ambari_server.start(args)
 
     self.assertTrue(get_validated_string_input_method.called)
+    self.assertTrue(save_master_key_method.called)
     popen_arg = popenMock.call_args[1]['env']
     self.assertEquals(os_environ_mock.copy.return_value, popen_arg)