Enable SecureStore configuration during init

Add new flag for the init program:

 * --secure-store-lib

That enables configuration of SecureStore during init. It will
automatically discover implementations of SecureStore interface inside
jar file. If there is not exactly one implementation the init will fail.

During init the SecureStore jar file will be added to Gerrit classpath.
Then after init this file will be copied to $gerrit_site/lib directory.
The discovered value of 'secureStoreImpl' will be saved in the
gerrit.config file.

This change also introduces a @SecureStoreClassName String binding early
in the startup so that this can be injected anywhere without worrying
about having the @GerritServerConfig bound (which itself requires an
injection of the SecureStore).

If an already initialized site with a custom secure store is init-ed
again and the --secure-store-lib option is not specified then the
gerrit.secureStoreClass is honored to ensure we use the same secure
store implementation. If the --secure-store-lib option is specified then
the gerrit.secureStoreClass is ignored during init and will be
overwritten with the new secure store.

Without this modification schema updates will fail because of wrong
password when custom SecureStore is used.

Change-Id: Iae22bbdace0d9c7e7db0690c4bf522176fc3308e
Signed-off-by: Dariusz Luksza <dariusz@luksza.org>

gerrit-pgm/BUCK

@@ -41,6 +41,7 @@ java_library(
   deps = DEPS + [
     ':init-api',
     ':util',
+    '//gerrit-common:annotations',
     '//gerrit-lucene:lucene',
     '//lib:args4j',
     '//lib:gwtjsonrpc',

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java

@@ -67,7 +67,9 @@ import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
 import com.google.gerrit.server.plugins.PluginRestApiModule;
 import com.google.gerrit.server.schema.DataSourceProvider;
 import com.google.gerrit.server.schema.SchemaVersionCheck;
+import com.google.gerrit.server.securestore.DefaultSecureStore;
 import com.google.gerrit.server.securestore.SecureStore;
+import com.google.gerrit.server.securestore.SecureStoreClassName;
 import com.google.gerrit.server.securestore.SecureStoreProvider;
 import com.google.gerrit.server.ssh.NoSshKeyCache;
 import com.google.gerrit.server.ssh.NoSshModule;
@@ -355,6 +357,8 @@ public class Daemon extends SiteProgram {
       protected void configure() {
         bind(GerritOptions.class).toInstance(new GerritOptions(headless, slave));
         if (test) {
+          bind(String.class).annotatedWith(SecureStoreClassName.class)
+              .toInstance(DefaultSecureStore.class.getName());
           bind(SecureStore.class).toProvider(SecureStoreProvider.class);
         }
       }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java

@@ -27,13 +27,13 @@ import com.google.gerrit.pgm.init.api.ConsoleUI;
 import com.google.gerrit.pgm.util.ErrorLogFile;
 import com.google.gerrit.server.config.GerritServerConfigModule;
 import com.google.gerrit.server.config.SitePath;
-import com.google.gerrit.server.securestore.SecureStore;
-import com.google.gerrit.server.securestore.SecureStoreProvider;
+import com.google.gerrit.server.securestore.SecureStoreClassName;
 import com.google.gerrit.server.util.HostPlatform;
 import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
 import com.google.inject.Inject;
 import com.google.inject.Module;
+import com.google.inject.util.Providers;
 
 import org.kohsuke.args4j.Option;
 
@@ -59,6 +59,10 @@ public class Init extends BaseInit {
   @Option(name = "--install-plugin", usage = "Install given plugin without asking")
   private List<String> installPlugins;
 
+  @Option(name = "--secure-store-lib",
+      usage = "Path to jar providing SecureStore implementation class")
+  private String secureStoreLib;
+
   @Inject
   Browser browser;
 
@@ -104,7 +108,8 @@ public class Init extends BaseInit {
       protected void configure() {
         bind(File.class).annotatedWith(SitePath.class).toInstance(getSitePath());
         bind(Browser.class);
-        bind(SecureStore.class).toProvider(SecureStoreProvider.class);
+        bind(String.class).annotatedWith(SecureStoreClassName.class)
+            .toProvider(Providers.of(getConfiguredSecureStoreClass()));
       }
     });
     modules.add(new GerritServerConfigModule());
@@ -132,6 +137,11 @@ public class Init extends BaseInit {
     return skipPlugins;
   }
 
+  @Override
+  protected String getSecureStoreLib() {
+    return secureStoreLib;
+  }
+
   void start(SiteRun run) throws Exception {
     if (run.flags.autoStart) {
       if (HostPlatform.isWin32()) {

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java

@@ -15,11 +15,14 @@
 package com.google.gerrit.pgm.init;
 
 import static com.google.gerrit.server.schema.DataSourceProvider.Context.SINGLE_USER;
+import static com.google.inject.Scopes.SINGLETON;
 import static com.google.inject.Stage.PRODUCTION;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.gerrit.common.Die;
+import com.google.gerrit.common.IoUtil;
 import com.google.gerrit.pgm.init.api.ConsoleUI;
 import com.google.gerrit.pgm.init.api.InitFlags;
 import com.google.gerrit.pgm.init.api.InstallPlugins;
@@ -28,8 +31,12 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.SitePath;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.plugins.JarScanner;
 import com.google.gerrit.server.schema.SchemaUpdater;
 import com.google.gerrit.server.schema.UpdateUI;
+import com.google.gerrit.server.securestore.SecureStore;
+import com.google.gerrit.server.securestore.SecureStoreClassName;
+import com.google.gerrit.server.securestore.SecureStoreProvider;
 import com.google.gwtorm.jdbc.JdbcExecutor;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.gwtorm.server.OrmException;
@@ -44,12 +51,14 @@ import com.google.inject.Module;
 import com.google.inject.Provider;
 import com.google.inject.TypeLiteral;
 import com.google.inject.spi.Message;
+import com.google.inject.util.Providers;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.File;
 import java.io.FileNotFoundException;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -132,6 +141,10 @@ public class BaseInit extends SiteProgram {
     return false;
   }
 
+  protected String getSecureStoreLib() {
+    return null;
+  }
+
   /**
    * Invoked before site init is called.
    *
@@ -197,6 +210,17 @@ public class BaseInit extends SiteProgram {
     final ConsoleUI ui = getConsoleUI();
     final File sitePath = getSitePath();
     final List<Module> m = new ArrayList<>();
+    final SecureStoreInitData secureStoreInitData = discoverSecureStoreClass();
+    final String currentSecureStoreClassName = getConfiguredSecureStoreClass();
+
+    if (secureStoreInitData != null && currentSecureStoreClassName != null
+        && !currentSecureStoreClassName.equals(secureStoreInitData.className)) {
+      String err =
+          String.format(
+              "Different secure store was previously configured: %s.",
+              currentSecureStoreClassName);
+      die(err, new RuntimeException("secure store mismatch"));
+    }
 
     m.add(new InitModule(standalone, initDb));
     m.add(new AbstractModule() {
@@ -210,6 +234,21 @@ public class BaseInit extends SiteProgram {
         bind(new TypeLiteral<List<String>>() {}).annotatedWith(
             InstallPlugins.class).toInstance(plugins);
         bind(PluginsDistribution.class).toInstance(pluginsDistribution);
+
+        String secureStoreClassName;
+        if (secureStoreInitData != null) {
+          secureStoreClassName = secureStoreInitData.className;
+        } else {
+          secureStoreClassName = currentSecureStoreClassName;
+        }
+        if (secureStoreClassName != null) {
+          ui.message("Using secure store: %s\n", secureStoreClassName);
+        }
+        bind(SecureStoreInitData.class).toProvider(
+            Providers.of(secureStoreInitData));
+        bind(String.class).annotatedWith(SecureStoreClassName.class)
+            .toProvider(Providers.of(secureStoreClassName));
+        bind(SecureStore.class).toProvider(SecureStoreProvider.class).in(SINGLETON);
       }
     });
 
@@ -240,6 +279,39 @@ public class BaseInit extends SiteProgram {
     return ConsoleUI.getInstance(false);
   }
 
+  private SecureStoreInitData discoverSecureStoreClass() {
+    String secureStore = getSecureStoreLib();
+    if (Strings.isNullOrEmpty(secureStore)) {
+      return null;
+    }
+
+    try {
+      File secureStoreLib = new File(secureStore);
+      if (!secureStoreLib.exists()) {
+        throw new InvalidSecureStoreException(String.format(
+            "File %s doesn't exist", secureStore));
+      }
+      JarScanner scanner = new JarScanner(secureStoreLib);
+      List<String> secureStores =
+          scanner.findImplementationsOf(SecureStore.class);
+      if (secureStores.isEmpty()) {
+        throw new InvalidSecureStoreException(String.format(
+            "Cannot find class implementing %s interface in %s",
+            SecureStore.class.getName(), secureStore));
+      }
+      if (secureStores.size() > 1) {
+        throw new InvalidSecureStoreException(String.format(
+            "%s has more that one implementation of %s interface",
+            secureStore, SecureStore.class.getName()));
+      }
+      IoUtil.loadJARs(secureStoreLib);
+      return new SecureStoreInitData(secureStoreLib, secureStores.get(0));
+    } catch (IOException e) {
+      throw new InvalidSecureStoreException(String.format("%s is not a valid jar",
+          secureStore));
+    }
+  }
+
   public static class SiteRun {
     public final ConsoleUI ui;
     public final SitePaths site;

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitHttpd.java

@@ -154,10 +154,10 @@ class InitHttpd implements InitStep {
       return;
     }
 
-    String ssl_pass = flags.sec.getString("http", null, "sslKeyPassword");
+    String ssl_pass = flags.sec.get("http", null, "sslKeyPassword");
     if (ssl_pass == null || ssl_pass.isEmpty()) {
       ssl_pass = SignedToken.generateRandomKey();
-      flags.sec.setString("httpd", null, "sslKeyPassword", ssl_pass);
+      flags.sec.set("httpd", null, "sslKeyPassword", ssl_pass);
     }
 
     hostname = ui.readString(hostname, "Certificate server name");

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InvalidSecureStoreException.java

@@ -0,0 +1,27 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed 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 com.google.gerrit.pgm.init;
+
+public class InvalidSecureStoreException extends RuntimeException {
+  private static final long serialVersionUID = 1L;
+
+  public InvalidSecureStoreException(String message) {
+    super(message);
+  }
+
+  public InvalidSecureStoreException(String message, Throwable why) {
+    super(message, why);
+  }
+}

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SecureStoreInitData.java

@@ -0,0 +1,27 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed 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 com.google.gerrit.pgm.init;
+
+import java.io.File;
+
+class SecureStoreInitData {
+  final File jarFile;
+  final String className;
+
+  SecureStoreInitData(File jar, String className) {
+    this.className = className;
+    this.jarFile = jar;
+  }
+}

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java

@@ -19,12 +19,15 @@ import static com.google.gerrit.pgm.init.api.InitUtil.die;
 import static com.google.gerrit.pgm.init.api.InitUtil.extract;
 import static com.google.gerrit.pgm.init.api.InitUtil.mkdir;
 import static com.google.gerrit.pgm.init.api.InitUtil.savePublic;
-import static com.google.gerrit.pgm.init.api.InitUtil.saveSecure;
 import static com.google.gerrit.pgm.init.api.InitUtil.version;
 
+import com.google.common.io.Files;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.pgm.init.api.ConsoleUI;
 import com.google.gerrit.pgm.init.api.InitFlags;
 import com.google.gerrit.pgm.init.api.InitStep;
+import com.google.gerrit.pgm.init.api.Section;
+import com.google.gerrit.pgm.init.api.Section.Factory;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.mail.OutgoingEmail;
 import com.google.inject.Binding;
@@ -33,6 +36,7 @@ import com.google.inject.Injector;
 import com.google.inject.TypeLiteral;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -42,13 +46,19 @@ public class SitePathInitializer {
   private final InitFlags flags;
   private final SitePaths site;
   private final List<InitStep> steps;
+  private final Factory sectionFactory;
+  private final SecureStoreInitData secureStoreInitData;
 
   @Inject
   public SitePathInitializer(final Injector injector, final ConsoleUI ui,
-      final InitFlags flags, final SitePaths site) {
+      final InitFlags flags, final SitePaths site,
+      final Section.Factory sectionFactory,
+      final @Nullable SecureStoreInitData secureStoreInitData) {
     this.ui = ui;
     this.flags = flags;
     this.site = site;
+    this.sectionFactory = sectionFactory;
+    this.secureStoreInitData = secureStoreInitData;
     this.steps = stepsOf(injector);
   }
 
@@ -83,8 +93,8 @@ public class SitePathInitializer {
       step.run();
     }
 
+    saveSecureStore();
     savePublic(flags.cfg);
-    saveSecure(flags.sec);
 
     extract(site.gerrit_sh, getClass(), "gerrit.sh");
     chmod(0755, site.gerrit_sh);
@@ -120,6 +130,15 @@ public class SitePathInitializer {
     }
   }
 
+  private void saveSecureStore() throws IOException {
+    if (secureStoreInitData != null) {
+      File dst = new File(site.lib_dir, secureStoreInitData.jarFile.getName());
+      Files.copy(secureStoreInitData.jarFile, dst);
+      Section gerritSection = sectionFactory.get("gerrit", null);
+      gerritSection.set("secureStoreClass", secureStoreInitData.className);
+    }
+  }
+
   private void extractMailExample(String orig) throws Exception {
     File ex = new File(site.mail_dir, orig + ".example");
     extract(ex, OutgoingEmail.class, orig);

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_x.java

@@ -16,13 +16,13 @@ package com.google.gerrit.pgm.init;
 
 import static com.google.gerrit.pgm.init.api.InitUtil.die;
 import static com.google.gerrit.pgm.init.api.InitUtil.savePublic;
-import static com.google.gerrit.pgm.init.api.InitUtil.saveSecure;
 
 import com.google.gerrit.pgm.init.api.ConsoleUI;
 import com.google.gerrit.pgm.init.api.InitFlags;
 import com.google.gerrit.pgm.init.api.InitStep;
 import com.google.gerrit.pgm.init.api.Section;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
 import com.google.gerrit.server.util.SocketUtil;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -63,7 +63,7 @@ class UpgradeFrom2_0_x implements InitStep {
   private final ConsoleUI ui;
 
   private final FileBasedConfig cfg;
-  private final FileBasedConfig sec;
+  private final SecureStore sec;
   private final File site_path;
   private final File etc_dir;
   private final Section.Factory sections;
@@ -117,7 +117,6 @@ class UpgradeFrom2_0_x implements InitStep {
     // believed to be empty) file.
     //
     cfg.load();
-    sec.load();
 
     final Properties oldprop = readGerritServerProperties();
     if (oldprop != null) {
@@ -140,7 +139,7 @@ class UpgradeFrom2_0_x implements InitStep {
 
       String password = oldprop.getProperty("password");
       if (password != null && !password.isEmpty()) {
-        sec.setString("database", null, "password", password);
+        sec.set("database", null, "password", password);
       }
     }
 
@@ -148,13 +147,12 @@ class UpgradeFrom2_0_x implements InitStep {
 
     values = cfg.getStringList("ldap", null, "password");
     cfg.unset("ldap", null, "password");
-    sec.setStringList("ldap", null, "password", Arrays.asList(values));
+    sec.setList("ldap", null, "password", Arrays.asList(values));
 
     values = cfg.getStringList("sendemail", null, "smtpPass");
     cfg.unset("sendemail", null, "smtpPass");
-    sec.setStringList("sendemail", null, "smtpPass", Arrays.asList(values));
+    sec.setList("sendemail", null, "smtpPass", Arrays.asList(values));
 
-    saveSecure(sec);
     savePublic(cfg);
   }
 
@@ -251,7 +249,7 @@ class UpgradeFrom2_0_x implements InitStep {
       database.set("username", username);
     }
     if (password != null && !password.isEmpty()) {
-      sec.setString("database", null, "password", password);
+      sec.set("database", null, "password", password);
     }
   }
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/InitFlags.java

@@ -16,6 +16,7 @@ package com.google.gerrit.pgm.init.api;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
@@ -39,19 +40,19 @@ public class InitFlags {
   public boolean skipPlugins;
 
   public final FileBasedConfig cfg;
-  public final FileBasedConfig sec;
+  public final SecureStore sec;
   public final List<String> installPlugins;
 
   @VisibleForTesting
   @Inject
   public InitFlags(final SitePaths site,
+      final SecureStore secureStore,
       final @InstallPlugins List<String> installPlugins) throws IOException,
       ConfigInvalidException {
+    sec = secureStore;
     this.installPlugins = installPlugins;
     cfg = new FileBasedConfig(site.gerrit_config, FS.DETECTED);
-    sec = new FileBasedConfig(site.secure_config, FS.DETECTED);
 
     cfg.load();
-    sec.load();
   }
 }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/InitUtil.java

@@ -14,13 +14,11 @@
 
 package com.google.gerrit.pgm.init.api;
 
-import static com.google.gerrit.common.FileUtil.chmod;
 import static com.google.gerrit.common.FileUtil.modified;
 
 import com.google.gerrit.common.Die;
 
 import org.eclipse.jgit.internal.storage.file.LockFile;
-import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.IO;
@@ -53,26 +51,6 @@ public class InitUtil {
     }
   }
 
-  public static void saveSecure(final FileBasedConfig sec) throws IOException {
-    if (modified(sec)) {
-      final byte[] out = Constants.encode(sec.toText());
-      final File path = sec.getFile();
-      final LockFile lf = new LockFile(path, FS.DETECTED);
-      if (!lf.lock()) {
-        throw new IOException("Cannot lock " + path);
-      }
-      try {
-        chmod(0600, new File(path.getParentFile(), path.getName() + ".lock"));
-        lf.write(out);
-        if (!lf.commit()) {
-          throw new IOException("Cannot commit write to " + path);
-        }
-      } finally {
-        lf.unlock();
-      }
-    }
-  }
-
   public static void mkdir(final File path) {
     if (!path.isDirectory() && !path.mkdir()) {
       throw die("Cannot make directory " + path);

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/Section.java

@@ -16,6 +16,7 @@ package com.google.gerrit.pgm.init.api;
 
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
@@ -36,16 +37,19 @@ public class Section {
   private final ConsoleUI ui;
   private final String section;
   private final String subsection;
+  private final SecureStore secureStore;
 
   @Inject
   public Section(final InitFlags flags, final SitePaths site,
-      final ConsoleUI ui, @Assisted("section") final String section,
+      final SecureStore secureStore, final ConsoleUI ui,
+      @Assisted("section") final String section,
       @Assisted("subsection") @Nullable final String subsection) {
     this.flags = flags;
     this.site = site;
     this.ui = ui;
     this.section = section;
     this.subsection = subsection;
+    this.secureStore = secureStore;
   }
 
   public String get(String name) {
@@ -142,7 +146,7 @@ public class Section {
   public String password(final String username, final String password) {
     final String ov = getSecure(password);
 
-    String user = flags.sec.getString(section, subsection, username);
+    String user = flags.sec.get(section, subsection, username);
     if (user == null) {
       user = get(username);
     }
@@ -187,14 +191,14 @@ public class Section {
   }
 
   public String getSecure(String name) {
-    return flags.sec.getString(section, subsection, name);
+    return flags.sec.get(section, subsection, name);
   }
 
   public void setSecure(String name, String value) {
     if (value != null) {
-      flags.sec.setString(section, subsection, name, value);
+      secureStore.set(section, subsection, name, value);
     } else {
-      flags.sec.unset(section, subsection, name);
+      secureStore.unset(section, subsection, name);
     }
   }
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SecureStoreException.java

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.gerrit.server.securestore;
+package com.google.gerrit.pgm.util;
 
 public class SecureStoreException extends RuntimeException {
   private static final long serialVersionUID = 5581700510568485065L;

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java

@@ -24,14 +24,14 @@ import com.google.gerrit.lifecycle.LifecycleModule;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerConfigModule;
 import com.google.gerrit.server.config.SitePath;
+import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.LocalDiskRepositoryManager;
 import com.google.gerrit.server.schema.DataSourceModule;
 import com.google.gerrit.server.schema.DataSourceProvider;
 import com.google.gerrit.server.schema.DataSourceType;
 import com.google.gerrit.server.schema.DatabaseModule;
 import com.google.gerrit.server.schema.SchemaModule;
-import com.google.gerrit.server.securestore.SecureStore;
-import com.google.gerrit.server.securestore.SecureStoreProvider;
+import com.google.gerrit.server.securestore.SecureStoreClassName;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.AbstractModule;
 import com.google.inject.Binding;
@@ -41,15 +41,21 @@ import com.google.inject.Injector;
 import com.google.inject.Key;
 import com.google.inject.Module;
 import com.google.inject.Provider;
+import com.google.inject.ProvisionException;
 import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 import com.google.inject.name.Names;
 import com.google.inject.spi.Message;
+import com.google.inject.util.Providers;
 
+import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
 import org.kohsuke.args4j.Option;
 
 import java.io.File;
+import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.sql.Connection;
 import java.sql.SQLException;
@@ -98,7 +104,8 @@ public abstract class SiteProgram extends AbstractProgram {
       @Override
       protected void configure() {
         bind(File.class).annotatedWith(SitePath.class).toInstance(sitePath);
-        bind(SecureStore.class).toProvider(SecureStoreProvider.class);
+        bind(String.class).annotatedWith(SecureStoreClassName.class)
+            .toProvider(Providers.of(getConfiguredSecureStoreClass()));
       }
     };
     modules.add(sitePathModule);
@@ -180,6 +187,29 @@ public abstract class SiteProgram extends AbstractProgram {
     }
   }
 
+  protected final String getConfiguredSecureStoreClass() {
+    Module m = new AbstractModule() {
+      @Override
+      protected void configure() {
+        bind(File.class).annotatedWith(SitePath.class).toInstance(sitePath);
+        bind(SitePaths.class);
+      }
+    };
+    Injector i = Guice.createInjector(m);
+    SitePaths site = i.getInstance(SitePaths.class);
+    FileBasedConfig cfg = new FileBasedConfig(site.gerrit_config, FS.DETECTED);
+    if (!cfg.getFile().exists()) {
+      return null;
+    }
+
+    try {
+      cfg.load();
+      return cfg.getString("gerrit", null, "secureStoreClass");
+    } catch (IOException | ConfigInvalidException e) {
+      throw new ProvisionException(e.getMessage(), e);
+    }
+  }
+
   private String getDbType(Provider<DataSource> dsProvider) {
     String dbProductName;
     try (Connection conn = dsProvider.get().getConnection()) {

gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java

@@ -28,8 +28,10 @@ import com.google.gerrit.pgm.init.api.ConsoleUI;
 import com.google.gerrit.pgm.init.api.InitFlags;
 import com.google.gerrit.pgm.init.api.Section;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.IO;
@@ -40,6 +42,7 @@ import java.io.FileWriter;
 import java.io.IOException;
 import java.io.Writer;
 import java.util.Collections;
+import java.util.List;
 
 
 public class UpgradeFrom2_0_xTest extends InitTestCase {
@@ -71,13 +74,14 @@ public class UpgradeFrom2_0_xTest extends InitTestCase {
     old.setString("sendemail", null, "smtpPass", "email.s3kr3t");
     old.save();
 
+    final InMemorySecureStore secureStore = new InMemorySecureStore();
     final InitFlags flags =
-        new InitFlags(site, Collections.<String> emptyList());
+        new InitFlags(site, secureStore, Collections.<String> emptyList());
     final ConsoleUI ui = createStrictMock(ConsoleUI.class);
     Section.Factory sections = new Section.Factory() {
       @Override
       public Section get(String name, String subsection) {
-        return new Section(flags, site, ui, name, subsection);
+        return new Section(flags, site, secureStore, ui, name, subsection);
       }
     };
 
@@ -99,18 +103,46 @@ public class UpgradeFrom2_0_xTest extends InitTestCase {
     }
 
     FileBasedConfig cfg = new FileBasedConfig(site.gerrit_config, FS.DETECTED);
-    FileBasedConfig sec = new FileBasedConfig(site.secure_config, FS.DETECTED);
     cfg.load();
-    sec.load();
 
     assertEquals("email.user", cfg.getString("sendemail", null, "smtpUser"));
     assertNull(cfg.getString("sendemail", null, "smtpPass"));
-    assertEquals("email.s3kr3t", sec.getString("sendemail", null, "smtpPass"));
+    assertEquals("email.s3kr3t", secureStore.get("sendemail", null, "smtpPass"));
 
     assertEquals("ldap.user", cfg.getString("ldap", null, "username"));
     assertNull(cfg.getString("ldap", null, "password"));
-    assertEquals("ldap.s3kr3t", sec.getString("ldap", null, "password"));
+    assertEquals("ldap.s3kr3t", secureStore.get("ldap", null, "password"));
 
     u.run();
   }
+
+  private static class InMemorySecureStore implements SecureStore {
+    private final Config cfg = new Config();
+
+    @Override
+    public String get(String section, String subsection, String name) {
+      return cfg.getString(section, subsection, name);
+    }
+
+    @Override
+    public String[] getList(String section, String subsection, String name) {
+      return cfg.getStringList(section, subsection, name);
+    }
+
+    @Override
+    public void set(String section, String subsection, String name, String value) {
+      cfg.setString(section, subsection, name, value);
+    }
+
+    @Override
+    public void setList(String section, String subsection, String name,
+        List<String> values) {
+      cfg.setStringList(section, subsection, name, values);
+    }
+
+    @Override
+    public void unset(String section, String subsection, String name) {
+      cfg.unset(section, subsection, name);
+    }
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerConfigModule.java

@@ -16,6 +16,8 @@ package com.google.gerrit.server.config;
 
 import static com.google.inject.Scopes.SINGLETON;
 
+import com.google.gerrit.server.securestore.SecureStore;
+import com.google.gerrit.server.securestore.SecureStoreProvider;
 import com.google.inject.AbstractModule;
 
 import org.eclipse.jgit.lib.Config;
@@ -28,5 +30,6 @@ public class GerritServerConfigModule extends AbstractModule {
     bind(TrackingFooters.class).toProvider(TrackingFootersProvider.class).in(SINGLETON) ;
     bind(Config.class).annotatedWith(GerritServerConfig.class).toProvider(
         GerritServerConfigProvider.class).in(SINGLETON);
+    bind(SecureStore.class).toProvider(SecureStoreProvider.class).in(SINGLETON);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritServerConfigProvider.java

@@ -37,7 +37,7 @@ class GerritServerConfigProvider implements Provider<Config> {
   private final SecureStore secureStore;
 
   @Inject
-  GerritServerConfigProvider(final SitePaths site, final SecureStore secureStore) {
+  GerritServerConfigProvider(SitePaths site, SecureStore secureStore) {
     this.site = site;
     this.secureStore = secureStore;
   }

gerrit-server/src/main/java/com/google/gerrit/server/plugins/CopyConfigModule.java

@@ -24,7 +24,6 @@ import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.securestore.SecureStore;
-import com.google.gerrit.server.securestore.SecureStoreProvider;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
@@ -117,12 +116,19 @@ class CopyConfigModule extends AbstractModule {
     return serverIdentProvider.get();
   }
 
+  @Inject
+  private SecureStore secureStore;
+
+  @Provides
+  SecureStore getSecureStore() {
+    return secureStore;
+  }
+
   @Inject
   CopyConfigModule() {
   }
 
   @Override
   protected void configure() {
-    bind(SecureStore.class).toProvider(SecureStoreProvider.class);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStoreClassName.java

@@ -0,0 +1,12 @@
+package com.google.gerrit.server.securestore;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+
+import java.lang.annotation.Retention;
+
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface SecureStoreClassName {
+}

gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStoreData.java

@@ -1,78 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed 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 com.google.gerrit.server.securestore;
-
-import com.google.common.base.MoreObjects;
-
-import java.io.File;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.Objects;
-
-public class SecureStoreData {
-  public final File pluginFile;
-  public final String storeName;
-  public final String className;
-
-  public SecureStoreData(String pluginName, String className, File jarFile,
-      String storeName) {
-    this.className = className;
-    this.pluginFile = jarFile;
-    this.storeName = String.format("%s/%s", pluginName, storeName);
-  }
-
-  public String getStoreName() {
-    return storeName;
-  }
-
-  public Class<? extends SecureStore> load() {
-    return load(pluginFile);
-  }
-
-  @SuppressWarnings("unchecked")
-  public Class<? extends SecureStore> load(File pluginFile) {
-    try {
-      URL[] pluginJarUrls = new URL[] {pluginFile.toURI().toURL()};
-      ClassLoader currentCL = Thread.currentThread().getContextClassLoader();
-      final URLClassLoader newClassLoader =
-          new URLClassLoader(pluginJarUrls, currentCL);
-      Thread.currentThread().setContextClassLoader(newClassLoader);
-      return (Class<? extends SecureStore>) newClassLoader.loadClass(className);
-    } catch (Exception e) {
-      throw new SecureStoreException(String.format(
-          "Cannot load secure store implementation for %s", storeName), e);
-    }
-  }
-
-  @Override
-  public String toString() {
-    return MoreObjects.toStringHelper(this).add("storeName", storeName)
-        .add("className", className).add("file", pluginFile).toString();
-  }
-
-  @Override
-  public boolean equals(Object obj) {
-    if (obj instanceof SecureStoreData) {
-      SecureStoreData o = (SecureStoreData) obj;
-      return storeName.equals(o.storeName);
-    }
-    return false;
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(storeName);
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStoreProvider.java

@@ -15,69 +15,55 @@
 package com.google.gerrit.server.securestore;
 
 import com.google.common.base.Strings;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.SiteLibraryLoaderUtil;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.Provider;
+import com.google.inject.Singleton;
 
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.storage.file.FileBasedConfig;
-import org.eclipse.jgit.util.FS;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.File;
-import java.io.IOException;
 
+@Singleton
 public class SecureStoreProvider implements Provider<SecureStore> {
   private static final Logger log = LoggerFactory
       .getLogger(SecureStoreProvider.class);
 
   private final File libdir;
   private final Injector injector;
-  private final String secureStoreClassName;
-
-  private SecureStore instance;
+  private final String className;
 
   @Inject
-  SecureStoreProvider(
+  protected SecureStoreProvider(
       Injector injector,
-      SitePaths sitePaths) {
-    FileBasedConfig cfg =
-        new FileBasedConfig(sitePaths.gerrit_config, FS.DETECTED);
-    try {
-      cfg.load();
-    } catch (IOException | ConfigInvalidException e) {
-      throw new RuntimeException("Cannot read gerrit.config file", e);
-    }
+      SitePaths sitePaths,
+      @Nullable @SecureStoreClassName String className) {
     this.injector = injector;
     this.libdir = sitePaths.lib_dir;
-    this.secureStoreClassName =
-        cfg.getString("gerrit", null, "secureStoreClass");
+    this.className = className;
   }
 
   @Override
-  public SecureStore get() {
-    if (instance == null) {
-      instance = injector.getInstance(getSecureStoreImpl());
-    }
-    return instance;
+  public synchronized SecureStore get() {
+    return injector.getInstance(getSecureStoreImpl());
   }
 
   @SuppressWarnings("unchecked")
   private Class<? extends SecureStore> getSecureStoreImpl() {
-    if (Strings.isNullOrEmpty(secureStoreClassName)) {
+    if (Strings.isNullOrEmpty(className)) {
       return DefaultSecureStore.class;
     }
 
     SiteLibraryLoaderUtil.loadSiteLib(libdir);
     try {
-      return (Class<? extends SecureStore>) Class.forName(secureStoreClassName);
+      return (Class<? extends SecureStore>) Class.forName(className);
     } catch (ClassNotFoundException e) {
       String msg =
-          String.format("Cannot load secure store class: %s",
-              secureStoreClassName);
+          String.format("Cannot load secure store class: %s", className);
       log.error(msg, e);
       throw new RuntimeException(msg, e);
     }