Let ConfidentialKey & Secret do something from unit tests

core/pom.xml

@@ -895,5 +895,16 @@ THE SOFTWARE.
         <findbugs.failOnError>true</findbugs.failOnError>
       </properties>
     </profile>
+    <profile>
+      <id>all-tests</id>
+      <activation>
+        <property>
+          <name>!test</name>
+        </property>
+      </activation>
+      <properties>
+        <maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
+      </properties>
+    </profile>
   </profiles>
 </project>

core/src/main/java/hudson/util/HistoricalSecrets.java

@@ -81,9 +81,15 @@ public class HistoricalSecrets {
      */
     @Deprecated
     /*package*/ static SecretKey getLegacyKey() throws GeneralSecurityException {
-        String secret = Secret.SECRET;
-        if(secret==null)    return Jenkins.get().getSecretKeyAsAES128();
-        return Util.toAes128Key(secret);
+        if (Secret.SECRET != null) {
+            return Util.toAes128Key(Secret.SECRET);
+        }
+        Jenkins j = Jenkins.getInstanceOrNull();
+        if (j != null) {
+            return j.getSecretKeyAsAES128();
+        } else {
+            return Util.toAes128Key("mock");
+        }
     }
 
     static final String MAGIC = "::::MAGIC::::";

core/src/main/java/hudson/util/Secret.java

@@ -31,7 +31,6 @@
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import jenkins.util.SystemProperties;
 import java.util.Arrays;
-import jenkins.model.Jenkins;
 import hudson.Util;
 import jenkins.security.CryptoConfidentialKey;
 import org.kohsuke.stapler.Stapler;
@@ -289,8 +288,10 @@ public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingCont
     private static final String PROVIDER = SystemProperties.getString(Secret.class.getName()+".provider");
 
     /**
-     * For testing only. Override the secret key so that we can test this class without {@link Jenkins}.
+     * For testing only.
+     * @deprecated Normally unnecessary.
      */
+    @Deprecated
     /*package*/ static String SECRET = null;
 
     /**

core/src/main/java/jenkins/security/ConfidentialStore.java

@@ -10,10 +10,14 @@
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import java.io.IOException;
+import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.util.Iterator;
+import java.util.Map;
+import java.util.Random;
 import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -51,6 +55,9 @@ public abstract class ConfidentialStore {
      */
     protected abstract @CheckForNull byte[] load(ConfidentialKey key) throws IOException;
 
+    // TODO consider promoting to public, and offering a default implementation of randomBytes (via the usual Util.isOverridden binary compat trick)
+    abstract SecureRandom secureRandom();
+
     /**
      * Works like {@link SecureRandom#nextBytes(byte[])}.
      *
@@ -64,7 +71,10 @@ public abstract class ConfidentialStore {
     public static @Nonnull ConfidentialStore get() {
         if (TEST!=null) return TEST.get();
 
-        Jenkins j = Jenkins.get();
+        Jenkins j = Jenkins.getInstanceOrNull();
+        if (j == null) {
+            return Mock.INSTANCE;
+        }
         Lookup lookup = j.lookup;
         ConfidentialStore cs = lookup.get(ConfidentialStore.class);
         if (cs==null) {
@@ -91,10 +101,60 @@ public abstract class ConfidentialStore {
         return cs;
     }
 
-    /**
-     * Testing only. Used for testing {@link ConfidentialKey} without {@link Jenkins}
+   /**
+     * @deprecated No longer needed.
      */
+    @Deprecated
     /*package*/ static ThreadLocal<ConfidentialStore> TEST = null;
 
+    static final class Mock extends ConfidentialStore {
+
+        static final Mock INSTANCE = new Mock();
+
+        private final SecureRandom rand;
+
+        private final Map<String, byte[]> data = new ConcurrentHashMap<>();
+
+        Mock() {
+            // Use a predictable seed to make tests more reproducible.
+            try {
+                rand = SecureRandom.getInstance("SHA1PRNG");
+            } catch (NoSuchAlgorithmException x) {
+                throw new AssertionError("https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#SecureRandom", x);
+            }
+            rand.setSeed(new byte[] {1, 2, 3, 4});
+        }
+
+        void clear() {
+            data.clear();
+        }
+
+        @Override
+        protected void store(ConfidentialKey key, byte[] payload) throws IOException {
+            LOGGER.fine(() -> "storing " + key.getId() + " " + hudson.Util.getDigestOf(hudson.Util.toHexString(payload)));
+            data.put(key.getId(), payload);
+        }
+
+        @Override
+        protected byte[] load(ConfidentialKey key) throws IOException {
+            byte[] payload = data.get(key.getId());
+            LOGGER.fine(() -> "loading " + key.getId() + " " + (payload != null ? hudson.Util.getDigestOf(hudson.Util.toHexString(payload)) : "null"));
+            return payload;
+        }
+
+        @Override
+        SecureRandom secureRandom() {
+            return rand;
+        }
+
+        @Override
+        public byte[] randomBytes(int size) {
+            byte[] random = new byte[size];
+            rand.nextBytes(random);
+            return random;
+        }
+
+    }
+
     private static final Logger LOGGER = Logger.getLogger(ConfidentialStore.class.getName());
 }

core/src/main/java/jenkins/security/DefaultConfidentialStore.java

@@ -140,6 +140,11 @@ private File getFileFor(ConfidentialKey key) {
         return new File(rootDir, key.getId());
     }
 
+    @Override
+    SecureRandom secureRandom() {
+        return sr;
+    }
+
     public byte[] randomBytes(int size) {
         byte[] random = new byte[size];
         sr.nextBytes(random);

core/src/main/java/jenkins/security/RSAConfidentialKey.java

@@ -30,7 +30,6 @@
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.PrivateKey;
-import java.security.SecureRandom;
 import java.security.interfaces.RSAPrivateCrtKey;
 import java.security.interfaces.RSAPrivateKey;
 import java.security.interfaces.RSAPublicKey;
@@ -79,7 +78,7 @@ protected synchronized RSAPrivateKey getPrivateKey() {
                 byte[] payload = load();
                 if (payload == null) {
                     KeyPairGenerator gen = KeyPairGenerator.getInstance("RSA");
-                    gen.initialize(2048, new SecureRandom()); // going beyond 2048 requires crypto extension
+                    gen.initialize(2048, cs.secureRandom()); // going beyond 2048 requires crypto extension
                     KeyPair keys = gen.generateKeyPair();
                     priv = (RSAPrivateKey) keys.getPrivate();
                     pub = (RSAPublicKey) keys.getPublic();

core/src/test/java/hudson/util/SecretRewriterTest.java

@@ -22,9 +22,6 @@
 
 public class SecretRewriterTest {
 
-    @Rule
-    public MockSecretRule mockSecretRule = new MockSecretRule();
-
     @Rule
     public ConfidentialStoreRule confidentialStoreRule = new ConfidentialStoreRule();
 

core/src/test/java/hudson/util/SecretTest.java

@@ -43,9 +43,6 @@ public class SecretTest {
     @Rule
     public ConfidentialStoreRule confidentialStore = new ConfidentialStoreRule();
 
-    @Rule
-    public MockSecretRule mockSecretRule = new MockSecretRule();
-
     private static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("\\{?[A-Za-z0-9+/]+={0,2}}?");
 
     @Test

core/src/test/java/jenkins/security/ConfidentialStoreRule.java

@@ -2,28 +2,15 @@
 
 import org.junit.rules.ExternalResource;
 
-import org.junit.rules.TemporaryFolder;
 
 /**
- * Test rule that injects a temporary {@link DefaultConfidentialStore}
- * @author Kohsuke Kawaguchi
+ * Test rule that makes {@link ConfidentialStore#get} be reset for each test.
  */
 public class ConfidentialStoreRule extends ExternalResource {
-    private final TemporaryFolder tmp = new TemporaryFolder();
 
     @Override
     protected void before() throws Throwable {
-        tmp.create();
-        ConfidentialStore.TEST.set(new DefaultConfidentialStore(tmp.getRoot()));
+        ConfidentialStore.Mock.INSTANCE.clear();
     }
 
-    @Override
-    protected void after() {
-        ConfidentialStore.TEST.set(null);
-        tmp.delete();
-    }
-
-    static {
-        ConfidentialStore.TEST = new ThreadLocal<>();
-    }
 }

test/src/test/java/jenkins/security/RekeySecretAdminMonitorTest.java

@@ -19,9 +19,12 @@
 import java.io.File;
 import java.io.IOException;
 import java.lang.annotation.Annotation;
+import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -81,7 +84,7 @@ private void putSomeOldData(File dir) throws Exception {
     private void verifyRewrite(File dir) throws Exception {
         File xml = new File(dir, "foo.xml");
         Pattern pattern = Pattern.compile("<foo>"+plain_regex_match+"</foo>");
-        assertTrue(pattern.matcher(FileUtils.readFileToString(xml).trim()).matches());
+        MatcherAssert.assertThat(FileUtils.readFileToString(xml, StandardCharsets.UTF_8).trim(), Matchers.matchesRegex(pattern));
     }
 
     // TODO sometimes fails: "Invalid request submission: {json=[Ljava.lang.String;@2c46358e, .crumb=[Ljava.lang.String;@35661457}"