CVE-2013-0158: Insufficient Information (NVD-CWE-noinfo)
value: Unspecified vulnerability in Jenkins before 1.498, Jenkins LTS before 1.480.2, and Jenkins Enterprise 1.447.x before 1.447.6.1 and 1.466.x before 1.466.12.1, when a slave is attached and anonymous read access is enabled, allows remote attackers to obtain the master cryptographic key via unknown vectors.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2013-02-24T22:55Z
Repository: https://github.com/jenkinsci/jenkins
Language: Java
Files changed: core/pom.xml, core/src/main/java/hudson/DNSMultiCast.java, core/src/main/java/hudson/UDPBroadcastThread.java, core/src/main/java/hudson/console/AnnotatedLargeText.java, core/src/main/java/hudson/model/Job.java, core/src/main/java/hudson/model/UsageStatistics.java, core/src/main/java/hudson/security/RememberMeServicesProxy.java, core/src/main/java/hudson/security/SecurityRealm.java, core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java, core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java, core/src/main/java/hudson/slaves/SlaveComputer.java, core/src/main/java/hudson/util/Secret.java, core/src/main/java/jenkins/model/Jenkins.java, core/src/main/java/jenkins/security/ApiTokenProperty.java, core/src/main/java/jenkins/security/ConfidentialKey.java, core/src/main/java/jenkins/security/ConfidentialStore.java, core/src/main/java/jenkins/security/CryptoConfidentialKey.java, core/src/main/java/jenkins/security/DefaultConfidentialStore.java, core/src/main/java/jenkins/security/HMACConfidentialKey.java, core/src/main/java/jenkins/security/HexStringConfidentialKey.java, core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java, core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java, core/src/main/resources/hudson/slaves/SlaveComputer/slave-agent.jnlp.jelly, core/src/test/java/hudson/util/SecretTest.groovy, core/src/test/java/jenkins/security/ConfidentialStoreRule.java, core/src/test/java/jenkins/security/CryptoConfidentialKeyTest.groovy, core/src/test/java/jenkins/security/DefaultConfidentialStoreTest.groovy, core/src/test/java/jenkins/security/HMACConfidentialKeyTest.groovy, core/src/test/java/jenkins/security/HexStringConfidentialKeyTest.groovy, pom.xml, test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
Commit date: 2013-01-05 09:26:05 -0800
Commit message: [SECURITY-49] Deprecating Jenkins.getSecretKey()

We are replacing it by the ConfidentialStore class and the
ConfidentialKey class, which provides purpose-specific confidential
information that are separated from each other.

In this way, not all eggs are in one basket, and in case of a
compromise, the impact will contained.

Also replaced several insecure use of digest(secret|messsage) or
digest(message|secret) by HMAC.

Diff: diff --git a/core/pom.xml b/core/pom.xml
index f9c4c4778..b54b13a9e 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -766,10 +766,39 @@ THE SOFTWARE.
               <classpathPrefix />
             </manifest>
           </archive>
         </configuration>
       </plugin>
+      <plugin><!-- run unit test in src/test/java -->
+        <groupId>org.kohsuke.gmaven</groupId>
+        <artifactId>gmaven-plugin</artifactId>
+        <!-- version specified in grandparent pom -->
+        <executions>
+          <execution>
+            <goals>
+              <goal>testCompile</goal>
+            </goals>
+            <configuration>
+              <sources>
+                <fileset>
+                  <directory>${project.basedir}/src/test/java</directory>
+                  <includes>
+                    <include>**/*.groovy</include>
+                  </includes>
+                </fileset>
+              </sources>
+            </configuration>
+          </execution>
+        </executions>
+        <dependencies>
+          <dependency>
+            <groupId>org.codehaus.groovy</groupId>
+            <artifactId>groovy-all</artifactId>
+            <version>1.8.5</version>
+          </dependency>
+        </dependencies>
+      </plugin>
     </plugins>
   </build>
 
   <reporting>
     <plugins>
diff --git a/core/src/main/java/hudson/DNSMultiCast.java b/core/src/main/java/hudson/DNSMultiCast.java
index 574086275..8d029b6ae 100644
--- a/core/src/main/java/hudson/DNSMultiCast.java
+++ b/core/src/main/java/hudson/DNSMultiCast.java
@@ -44,11 +44,11 @@ public class DNSMultiCast implements Closeable {
 
                     TcpSlaveAgentListener tal = jenkins.getTcpSlaveAgentListener();
                     if (tal!=null)
                         props.put("slave-port",String.valueOf(tal.getPort()));
 
-                    props.put("server-id", Util.getDigestOf(jenkins.getSecretKey()));
+                    props.put("server-id", jenkins.getLegacyInstanceId());
 
                     URL jenkins_url = new URL(rootURL);
                     int jenkins_port = jenkins_url.getPort();
                     if (jenkins_port == -1) {
                         jenkins_port = 80;
diff --git a/core/src/main/java/hudson/UDPBroadcastThread.java b/core/src/main/java/hudson/UDPBroadcastThread.java
index 79318d668..168279ba4 100644
--- a/core/src/main/java/hudson/UDPBroadcastThread.java
+++ b/core/src/main/java/hudson/UDPBroadcastThread.java
@@ -84,11 +84,11 @@ public class UDPBroadcastThread extends Thread {
                 TcpSlaveAgentListener tal = jenkins.getTcpSlaveAgentListener();
 
                 StringBuilder rsp = new StringBuilder("<hudson>");
                 tag(rsp,"version", Jenkins.VERSION);
                 tag(rsp,"url", jenkins.getRootUrl());
-                tag(rsp,"server-id", Util.getDigestOf(jenkins.getSecretKey()));
+                tag(rsp,"server-id", jenkins.getLegacyInstanceId());
                 tag(rsp,"slave-port",tal==null?null:tal.getPort());
 
                 for (UDPBroadcastFragment f : UDPBroadcastFragment.all())
                     f.buildFragment(rsp,sender);
 
diff --git a/core/src/main/java/hudson/console/AnnotatedLargeText.java b/core/src/main/java/hudson/console/AnnotatedLargeText.java
index 9f685c16a..7dfe1e18d 100644
--- a/core/src/main/java/hudson/console/AnnotatedLargeText.java
+++ b/core/src/main/java/hudson/console/AnnotatedLargeText.java
@@ -27,10 +27,11 @@ import com.trilead.ssh2.crypto.Base64;
 import jenkins.model.Jenkins;
 import hudson.remoting.ObjectInputStreamEx;
 import hudson.util.IOException2;
 import hudson.util.Secret;
 import hudson.util.TimeUnit2;
+import jenkins.security.CryptoConfidentialKey;
 import org.apache.commons.io.output.ByteArrayOutputStream;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.framework.io.ByteBuffer;
@@ -114,12 +115,11 @@ public class AnnotatedLargeText<T> extends LargeText {
 
     private ConsoleAnnotator createAnnotator(StaplerRequest req) throws IOException {
         try {
             String base64 = req!=null ? req.getHeader("X-ConsoleAnnotator") : null;
             if (base64!=null) {
-                Cipher sym = Secret.getCipher("AES");
-                sym.init(Cipher.DECRYPT_MODE, Jenkins.getInstance().getSecretKeyAsAES128());
+                Cipher sym = PASSING_ANNOTATOR.decrypt();
 
                 ObjectInputStream ois = new ObjectInputStreamEx(new GZIPInputStream(
                         new CipherInputStream(new ByteArrayInputStream(Base64.decode(base64.toCharArray())),sym)),
                         Jenkins.getInstance().pluginManager.uberClassLoader);
                 try {
@@ -129,12 +129,10 @@ public class AnnotatedLargeText<T> extends LargeText {
                         return (ConsoleAnnotator)ois.readObject();
                 } finally {
                     ois.close();
                 }
             }
-        } catch (GeneralSecurityException e) {
-            throw new IOException2(e);
         } catch (ClassNotFoundException e) {
             throw new IOException2(e);
         }
         // start from scratch
         return ConsoleAnnotator.initial(context==null ? null : context.getClass());
@@ -156,23 +154,22 @@ public class AnnotatedLargeText<T> extends LargeText {
     public long writeHtmlTo(long start, Writer w) throws IOException {
         ConsoleAnnotationOutputStream caw = new ConsoleAnnotationOutputStream(
                 w, createAnnotator(Stapler.getCurrentRequest()), context, charset);
         long r = super.writeLogTo(start,caw);
 
-        try {
-            ByteArrayOutputStream baos = new ByteArrayOutputStream();
-            Cipher sym = Secret.getCipher("AES");
-            sym.init(Cipher.ENCRYPT_MODE, Jenkins.getInstance().getSecretKeyAsAES128());
-            ObjectOutputStream oos = new ObjectOutputStream(new GZIPOutputStream(new CipherOutputStream(baos,sym)));
-            oos.writeLong(System.currentTimeMillis()); // send timestamp to prevent a replay attack
-            oos.writeObject(caw.getConsoleAnnotator());
-            oos.close();
-            StaplerResponse rsp = Stapler.getCurrentResponse();
-            if (rsp!=null)
-                rsp.setHeader("X-ConsoleAnnotator", new String(Base64.encode(baos.toByteArray())));
-        } catch (GeneralSecurityException e) {
-            throw new IOException2(e);
-        }
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        Cipher sym = PASSING_ANNOTATOR.encrypt();
+        ObjectOutputStream oos = new ObjectOutputStream(new GZIPOutputStream(new CipherOutputStream(baos,sym)));
+        oos.writeLong(System.currentTimeMillis()); // send timestamp to prevent a replay attack
+        oos.writeObject(caw.getConsoleAnnotator());
+        oos.close();
+        StaplerResponse rsp = Stapler.getCurrentResponse();
+        if (rsp!=null)
+            rsp.setHeader("X-ConsoleAnnotator", new String(Base64.encode(baos.toByteArray())));
         return r;
     }
 
+    /**
+     * Used for sending the state of ConsoleAnnotator to the client, because we are deserializing this object later.
+     */
+    private static final CryptoConfidentialKey PASSING_ANNOTATOR = new CryptoConfidentialKey(AnnotatedLargeText.class,"consoleAnnotator");
 }
diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java
index 5702275c5..71db51bf5 100644
--- a/core/src/main/java/hudson/model/Job.java
+++ b/core/src/main/java/hudson/model/Job.java
@@ -60,10 +60,11 @@ import hudson.util.TextFile;
 import hudson.widgets.HistoryWidget;
 import hudson.widgets.HistoryWidget.Adapter;
 import hudson.widgets.Widget;
 import jenkins.model.Jenkins;
 import jenkins.model.ProjectNamingStrategy;
+import jenkins.security.HexStringConfidentialKey;
 import jenkins.util.io.OnMaster;
 import net.sf.json.JSONException;
 import net.sf.json.JSONObject;
 import org.jfree.chart.ChartFactory;
 import org.jfree.chart.JFreeChart;
@@ -306,12 +307,12 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      * Builds up the environment variable map that's sufficient to identify a process
      * as ours. This is used to kill run-away processes via {@link ProcessTree#killAll(Map)}.
      */
     public EnvVars getCharacteristicEnvVars() {
         EnvVars env = new EnvVars();
-        env.put("JENKINS_SERVER_COOKIE",Util.getDigestOf("ServerID:"+ Jenkins.getInstance().getSecretKey()));
-        env.put("HUDSON_SERVER_COOKIE",Util.getDigestOf("ServerID:"+ Jenkins.getInstance().getSecretKey())); // Legacy compatibility
+        env.put("JENKINS_SERVER_COOKIE",SERVER_COOKIE.get());
+        env.put("HUDSON_SERVER_COOKIE",SERVER_COOKIE.get()); // Legacy compatibility
         env.put("JOB_NAME",getFullName());
         return env;
     }
 
     /**
@@ -1311,6 +1312,8 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
     }
 
     public BuildTimelineWidget getTimeline() {
         return new BuildTimelineWidget(getBuilds());
     }
+
+    private final static HexStringConfidentialKey SERVER_COOKIE = new HexStringConfidentialKey(Job.class,"serverCookie",16);
 }
diff --git a/core/src/main/java/hudson/model/UsageStatistics.java b/core/src/main/java/hudson/model/UsageStatistics.java
index 83da162b5..34382a3f4 100644
--- a/core/src/main/java/hudson/model/UsageStatistics.java
+++ b/core/src/main/java/hudson/model/UsageStatistics.java
@@ -119,45 +119,45 @@ public class UsageStatistics extends PageDecorator {
 
     /**
      * Gets the encrypted usage stat data to be sent to the Hudson server.
      */
     public String getStatData() throws IOException {
-        Jenkins h = Jenkins.getInstance();
+        Jenkins j = Jenkins.getInstance();
 
         JSONObject o = new JSONObject();
         o.put("stat",1);
-        o.put("install", Util.getDigestOf(h.getSecretKey()));
-        o.put("servletContainer",h.servletContext.getServerInfo());
+        o.put("install", j.getLegacyInstanceId());
+        o.put("servletContainer", j.servletContext.getServerInfo());
         o.put("version", Jenkins.VERSION);
 
         List<JSONObject> nodes = new ArrayList<JSONObject>();
-        for( Computer c : h.getComputers() ) {
+        for( Computer c : j.getComputers() ) {
             JSONObject  n = new JSONObject();
-            if(c.getNode()==h) {
+            if(c.getNode()==j) {
                 n.put("master",true);
                 n.put("jvm-vendor", System.getProperty("java.vm.vendor"));
                 n.put("jvm-version", System.getProperty("java.version"));
             }
             n.put("executors",c.getNumExecutors());
-            DescriptorImpl descriptor = h.getDescriptorByType(DescriptorImpl.class);
+            DescriptorImpl descriptor = j.getDescriptorByType(DescriptorImpl.class);
             n.put("os", descriptor.get(c));
             nodes.add(n);
         }
         o.put("nodes",nodes);
 
         List<JSONObject> plugins = new ArrayList<JSONObject>();
-        for( PluginWrapper pw : h.getPluginManager().getPlugins() ) {
+        for( PluginWrapper pw : j.getPluginManager().getPlugins() ) {
             if(!pw.isActive())  continue;   // treat disabled plugins as if they are uninstalled
             JSONObject p = new JSONObject();
             p.put("name",pw.getShortName());
             p.put("version",pw.getVersion());
             plugins.add(p);
         }
         o.put("plugins",plugins);
 
         JSONObject jobs = new JSONObject();
-        List<TopLevelItem> items = h.getItems();
+        List<TopLevelItem> items = j.getItems();
         for (TopLevelItemDescriptor d : Items.all()) {
             int cnt=0;
             for (TopLevelItem item : items) {
                 if(item.getDescriptor()==d)
                     cnt++;
diff --git a/core/src/main/java/hudson/security/RememberMeServicesProxy.java b/core/src/main/java/hudson/security/RememberMeServicesProxy.java
index 78d128477..6b4ba001a 100644
--- a/core/src/main/java/hudson/security/RememberMeServicesProxy.java
+++ b/core/src/main/java/hudson/security/RememberMeServicesProxy.java
@@ -21,23 +21,25 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
 package hudson.security;
 
+import jenkins.model.Jenkins;
+import jenkins.security.ConfidentialStore;
 import org.acegisecurity.ui.rememberme.RememberMeServices;
 import org.acegisecurity.Authentication;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 /**
  * {@link RememberMeServices} proxy.
  *
  * <p>
- * In Hudson, we need {@link jenkins.model.Jenkins} instance to perform remember-me service,
- * because it relies on {@link jenkins.model.Jenkins#getSecretKey()}. However, security
- * filters can be initialized before Hudson is initialized.
+ * In Jenkins, we need {@link Jenkins} instance to perform remember-me service,
+ * because it relies on {@link ConfidentialStore}. However, security
+ * filters can be initialized before Jenkins is initialized.
  * (See #1210 for example.)
  *
  * <p>
  * So to work around the problem, we use a proxy.
  *
diff --git a/core/src/main/java/hudson/security/SecurityRealm.java b/core/src/main/java/hudson/security/SecurityRealm.java
index 9721fd252..910f7f390 100644
--- a/core/src/main/java/hudson/security/SecurityRealm.java
+++ b/core/src/main/java/hudson/security/SecurityRealm.java
@@ -514,14 +514,25 @@ public abstract class SecurityRealm extends AbstractDescribableImpl<SecurityReal
             this.manager = manager;
             this.userDetails = userDetails;
             this.rememberMe = rememberMe;
         }
 
+        @SuppressWarnings("deprecation")
         private static RememberMeServices createRememberMeService(UserDetailsService uds) {
             // create our default TokenBasedRememberMeServices, which depends on the availability of the secret key
             TokenBasedRememberMeServices2 rms = new TokenBasedRememberMeServices2();
             rms.setUserDetailsService(uds);
+            /*
+                TokenBasedRememberMeServices needs to be used in conjunction with RememberMeAuthenticationProvider,
+                and both needs to use the same key (this is a reflection of a poor design in AcgeiSecurity, if you ask me)
+                and various security plugins have its own groovy script that configures them.
+
+                So if we change this, it creates a painful situation for those plugins by forcing them to choose
+                to work with earlier version of Jenkins or newer version of Jenkins, and not both.
+
+                So we keep this here.
+             */
             rms.setKey(Jenkins.getInstance().getSecretKey());
             rms.setParameter("remember_me"); // this is the form field name in login.jelly
             return rms;
         }
     }
diff --git a/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java b/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java
index e69f1eb9e..f1612e72a 100644
--- a/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java
+++ b/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java
@@ -21,14 +21,14 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
 package hudson.security;
 
+import jenkins.security.HMACConfidentialKey;
 import org.acegisecurity.ui.rememberme.TokenBasedRememberMeServices;
 import org.acegisecurity.userdetails.UserDetails;
 import org.acegisecurity.Authentication;
-import org.apache.commons.codec.digest.DigestUtils;
 
 /**
  * {@link TokenBasedRememberMeServices} with modification so as not to rely
  * on the user password being available.
  *
@@ -39,15 +39,20 @@ import org.apache.commons.codec.digest.DigestUtils;
  * @author Kohsuke Kawaguchi
  */
 public class TokenBasedRememberMeServices2 extends TokenBasedRememberMeServices {
     @Override
     protected String makeTokenSignature(long tokenExpiryTime, UserDetails userDetails) {
-        String expectedTokenSignature = DigestUtils.md5Hex(userDetails.getUsername() + ":" + tokenExpiryTime + ":"
+        String expectedTokenSignature = MAC.mac(userDetails.getUsername() + ":" + tokenExpiryTime + ":"
                 + "N/A" + ":" + getKey());
         return expectedTokenSignature;
     }
 
     @Override
     protected String retrievePassword(Authentication successfulAuthentication) {
         return "N/A";
     }
+
+    /**
+     * Used to compute the token signature securely.
+     */
+    private static final HMACConfidentialKey MAC = new HMACConfidentialKey(TokenBasedRememberMeServices.class,"mac");
 }
diff --git a/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java b/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java
index 09588106d..a1b9a8e9c 100644
--- a/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java
+++ b/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java
@@ -116,11 +116,12 @@ public class DefaultCrumbIssuer extends CrumbIssuer {
     
     @Extension
     public static final class DescriptorImpl extends CrumbIssuerDescriptor<DefaultCrumbIssuer> implements ModelObject {
 
         public DescriptorImpl() {
-            super(Jenkins.getInstance().getSecretKey(), System.getProperty("hudson.security.csrf.requestfield", ".crumb"));
+            // salt just needs to be unique, and it doesn't have to be a secret
+            super(Jenkins.getInstance().getLegacyInstanceId(), System.getProperty("hudson.security.csrf.requestfield", ".crumb"));
             load();
         }
 
         @Override
         public String getDisplayName() {
diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java
index 89f4bf911..df0627442 100644
--- a/core/src/main/java/hudson/slaves/SlaveComputer.java
+++ b/core/src/main/java/hudson/slaves/SlaveComputer.java
@@ -57,10 +57,11 @@ import java.nio.charset.Charset;
 import java.util.concurrent.Future;
 import java.security.Security;
 
 import hudson.util.io.ReopenableFileOutputStream;
 import jenkins.model.Jenkins;
+import jenkins.slaves.JnlpSlaveAgentProtocol;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpRedirect;
@@ -126,10 +127,14 @@ public class SlaveComputer extends Computer {
     @Override
     public boolean isAcceptingTasks() {
         return acceptingTasks;
     }
 
+    public String getJnlpMac() {
+        return JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(getName());
+    }
+
     /**
      * Allows a {@linkplain hudson.slaves.ComputerLauncher} or a {@linkplain hudson.slaves.RetentionStrategy} to
      * suspend tasks being accepted by the slave computer.
      *
      * @param acceptingTasks {@code true} if the slave can accept tasks.
diff --git a/core/src/main/java/hudson/util/Secret.java b/core/src/main/java/hudson/util/Secret.java
index 7055628f5..32cba8dce 100644
--- a/core/src/main/java/hudson/util/Secret.java
+++ b/core/src/main/java/hudson/util/Secret.java
@@ -29,12 +29,15 @@ import com.thoughtworks.xstream.converters.UnmarshallingContext;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import com.trilead.ssh2.crypto.Base64;
 import jenkins.model.Jenkins;
 import hudson.Util;
+import jenkins.security.CryptoConfidentialKey;
 import org.kohsuke.stapler.Stapler;
 
+import javax.crypto.BadPaddingException;
+import javax.crypto.IllegalBlockSizeException;
 import javax.crypto.SecretKey;
 import javax.crypto.Cipher;
 import java.io.Serializable;
 import java.io.UnsupportedEncodingException;
 import java.io.IOException;
@@ -42,12 +45,12 @@ import java.security.GeneralSecurityException;
 
 /**
  * Glorified {@link String} that uses encryption in the persisted form, to avoid accidental exposure of a secret.
  *
  * <p>
- * Note that since the cryptography relies on {@link jenkins.model.Jenkins#getSecretKey()}, this is not meant as a protection
- * against code running in the same VM, nor against an attacker who has local file system access. 
+ * This is not meant as a protection against code running in the same VM, nor against an attacker
+ * who has local file system access on Jenkins master.
  *
  * <p>
  * {@link Secret}s can correctly read-in plain text password, so this allows the existing
  * String field to be updated to {@link Secret}.
  *
@@ -94,13 +97,17 @@ public final class Secret implements Serializable {
     public int hashCode() {
         return value.hashCode();
     }
 
     /**
-     * Turns {@link jenkins.model.Jenkins#getSecretKey()} into an AES key.
+     * Turns {@link Jenkins#getSecretKey()} into an AES key.
+     *
+     * @deprecated
+     * This is no longer the key we use to encrypt new information, but we still need this
+     * to be able to decrypt what's already persisted.
      */
-    private static SecretKey getKey() throws UnsupportedEncodingException, GeneralSecurityException {
+    /*package*/ static SecretKey getLegacyKey() throws UnsupportedEncodingException, GeneralSecurityException {
         String secret = SECRET;
         if(secret==null)    return Jenkins.getInstance().getSecretKeyAsAES128();
         return Util.toAes128Key(secret);
     }
 
@@ -109,12 +116,11 @@ public final class Secret implements Serializable {
      *
      * @see #toString() 
      */
     public String getEncryptedValue() {
         try {
-            Cipher cipher = getCipher("AES");
-            cipher.init(Cipher.ENCRYPT_MODE, getKey());
+            Cipher cipher = KEY.encrypt();
             // add the magic suffix which works like a check sum.
             return new String(Base64.encode(cipher.doFinal((value+MAGIC).getBytes("UTF-8"))));
         } catch (GeneralSecurityException e) {
             throw new Error(e); // impossible
         } catch (UnsupportedEncodingException e) {
@@ -127,25 +133,38 @@ public final class Secret implements Serializable {
      * if the given cipher text was invalid.
      */
     public static Secret decrypt(String data) {
         if(data==null)      return null;
         try {
+            byte[] in = Base64.decode(data.toCharArray());
+            Secret s = tryDecrypt(KEY.decrypt(), in);
+            if (s!=null)    return s;
+
+            // try our historical key for backward compatibility
             Cipher cipher = getCipher("AES");
-            cipher.init(Cipher.DECRYPT_MODE, getKey());
-            String plainText = new String(cipher.doFinal(Base64.decode(data.toCharArray())), "UTF-8");
-            if(plainText.endsWith(MAGIC))
-                return new Secret(plainText.substring(0,plainText.length()-MAGIC.length()));
-            return null;
+            cipher.init(Cipher.DECRYPT_MODE, getLegacyKey());
+            return tryDecrypt(cipher, in);
         } catch (GeneralSecurityException e) {
             return null;
         } catch (UnsupportedEncodingException e) {
             throw new Error(e); // impossible
         } catch (IOException e) {
             return null;
         }
     }
 
+    private static Secret tryDecrypt(Cipher cipher, byte[] in) throws UnsupportedEncodingException {
+        try {
+            String plainText = new String(cipher.doFinal(in), "UTF-8");
+            if(plainText.endsWith(MAGIC))
+                return new Secret(plainText.substring(0,plainText.length()-MAGIC.length()));
+            return null;
+        } catch (GeneralSecurityException e) {
+            return null;
+        }
+    }
+
     /**
      * Workaround for JENKINS-6459 / http://java.net/jira/browse/GLASSFISH-11862
      * This method uses specific provider selected via hudson.util.Secret.provider system property
      * to provide a workaround for the above bug where default provide gives an unusable instance.
      * (Glassfish Enterprise users should set value of this property to "SunJCE")
@@ -205,14 +224,19 @@ public final class Secret implements Serializable {
      * @see #getCipher(String)
      */
     private static final String PROVIDER = System.getProperty(Secret.class.getName()+".provider");
 
     /**
-     * For testing only. Override the secret key so that we can test this class without {@link jenkins.model.Jenkins}.
+     * For testing only. Override the secret key so that we can test this class without {@link Jenkins}.
      */
     /*package*/ static String SECRET = null;
 
+    /**
+     * The key that encrypts the data on disk.
+     */
+    private static final CryptoConfidentialKey KEY = new CryptoConfidentialKey(Secret.class.getName());
+
     private static final long serialVersionUID = 1L;
 
     static {
         Stapler.CONVERT_UTILS.register(new org.apache.commons.beanutils.Converter() {
             public Secret convert(Class type, Object value) {
diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
index 116d98646..bb9b8efc5 100755
--- a/core/src/main/java/jenkins/model/Jenkins.java
+++ b/core/src/main/java/jenkins/model/Jenkins.java
@@ -194,10 +194,12 @@ import hudson.views.ViewsTabBar;
 import hudson.widgets.Widget;
 import jenkins.ExtensionComponentSet;
 import jenkins.ExtensionRefreshException;
 import jenkins.InitReactorRunner;
 import jenkins.model.ProjectNamingStrategy.DefaultProjectNamingStrategy;
+import jenkins.security.ConfidentialKey;
+import jenkins.security.ConfidentialStore;
 import net.sf.json.JSONObject;
 import org.acegisecurity.AccessDeniedException;
 import org.acegisecurity.AcegiSecurityException;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.GrantedAuthority;
@@ -992,23 +994,45 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
 
     /**
      * Returns a secret key that survives across container start/stop.
      * <p>
      * This value is useful for implementing some of the security features.
+     *
+     * @deprecated
+     *      Due to the past security advisory, this value should not be used any more to protect sensitive information.
+     *      See {@link ConfidentialStore} and {@link ConfidentialKey} for how to store secrets.
      */
     public String getSecretKey() {
         return secretKey;
     }
 
     /**
      * Gets {@linkplain #getSecretKey() the secret key} as a key for AES-128.
      * @since 1.308
+     * @deprecated
+     *       See {@link #getSecretKey()}.
      */
     public SecretKey getSecretKeyAsAES128() {
         return Util.toAes128Key(secretKey);
     }
 
+    /**
+     * Returns the unique identifier of this Jenkins that has been historically used to identify
+     * this Jenkins to the outside world.
+     *
+     * <p>
+     * This form of identifier is weak in that it can be impersonated by others. See
+     * https://wiki.jenkins-ci.org/display/JENKINS/Instance+Identity for more modern form of instance ID
+     * that can be challenged and verified.
+     *
+     * @since 1.498
+     */
+    @SuppressWarnings("deprecation")
+    public String getLegacyInstanceId() {
+        return Util.getDigestOf(getSecretKey());
+    }
+
     /**
      * Gets the SCM descriptor by name. Primarily used for making them web-visible.
      */
     public Descriptor<SCM> getScm(String shortClassName) {
         return findDescriptor(shortClassName,SCM.all());
diff --git a/core/src/main/java/jenkins/security/ApiTokenProperty.java b/core/src/main/java/jenkins/security/ApiTokenProperty.java
index 94c10f117..f1e06efa2 100644
--- a/core/src/main/java/jenkins/security/ApiTokenProperty.java
+++ b/core/src/main/java/jenkins/security/ApiTokenProperty.java
@@ -29,11 +29,10 @@ import hudson.model.Descriptor.FormException;
 import hudson.model.User;
 import hudson.model.UserProperty;
 import hudson.model.UserPropertyDescriptor;
 import hudson.util.HttpResponses;
 import hudson.util.Secret;
-import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.StaplerRequest;
@@ -101,14 +100,14 @@ public class ApiTokenProperty extends UserProperty {
          * When we are creating a default {@link ApiTokenProperty} for User,
          * we need to make sure it yields the same value for the same user,
          * because there's no guarantee that the property is saved.
          *
          * But we also need to make sure that an attacker won't be able to guess
-         * the initial API token value. So we take the seed by hasing the instance secret key + user ID.
+         * the initial API token value. So we take the seed by hashing the secret + user ID.
          */
         public ApiTokenProperty newInstance(User user) {
-            return new ApiTokenProperty(Util.getDigestOf(Jenkins.getInstance().getSecretKey() + ":" + user.getId()));
+            return new ApiTokenProperty(API_KEY_SEED.mac(user.getId()));
         }
 
         public HttpResponse doChangeToken(@AncestorInPath User u, StaplerResponse rsp) throws IOException {
             ApiTokenProperty p = u.getProperty(ApiTokenProperty.class);
             if (p==null) {
@@ -121,6 +120,11 @@ public class ApiTokenProperty extends UserProperty {
             return HttpResponses.html(Messages.ApiTokenProperty_ChangeToken_Success());
         }
     }
 
     private static final SecureRandom RANDOM = new SecureRandom();
+
+    /**
+     * We don't want an API key that's too long, so cut the length to 16 (which produces 32-letter MAC code in hexdump)
+     */
+    private static final HMACConfidentialKey API_KEY_SEED = new HMACConfidentialKey(ApiTokenProperty.class,"seed",16);
 }
diff --git a/core/src/main/java/jenkins/security/ConfidentialKey.java b/core/src/main/java/jenkins/security/ConfidentialKey.java
new file mode 100644
index 000000000..a0167f478
--- /dev/null
+++ b/core/src/main/java/jenkins/security/ConfidentialKey.java
@@ -0,0 +1,57 @@
+package jenkins.security;
+
+import hudson.scm.SCM;
+import hudson.tasks.Builder;
+import hudson.util.Secret;
+import jenkins.slaves.JnlpSlaveAgentProtocol;
+
+import javax.annotation.CheckForNull;
+import java.io.IOException;
+
+/**
+ * Confidential information that gets stored as a singleton in Jenkins, mostly some random token value.
+ *
+ * <p>
+ * The actual value is persisted via {@link ConfidentialStore}, but each use case that requires
+ * a secret like this should use a separate {@link ConfidentialKey} instance so that one compromised
+ * {@link ConfidentialKey} (say through incorrect usage and failure to protect it) shouldn't compromise
+ * all the others.
+ *
+ * <p>
+ * {@link ConfidentialKey} is ultimately a sequence of bytes,
+ * but for convenience, a family of subclasses are provided to represent the secret in different formats.
+ * See {@link HexStringConfidentialKey} and {@link HMACConfidentialKey} for example. In addition to the programming
+ * ease, these use case specific subtypes make it harder for vulnerability to creep in by making it harder
+ * for the secret to leak.
+ *
+ * <p>
+ * The {@link ConfidentialKey} subtypes are expected to be used as a singleton, like {@link JnlpSlaveAgentProtocol#SLAVE_SECRET}.
+ * For code that relies on XStream for persistence (such as {@link Builder}s, {@link SCM}s, and other fragment objects
+ * around builds and jobs), {@link Secret} provides more convenient way of storing secrets.
+ *
+ * @author Kohsuke Kawaguchi
+ * @see Secret
+ * @since 1.498
+ */
+public abstract class ConfidentialKey {
+    /**
+     * Name of the key. This is used as the file name.
+     */
+    private final String id;
+
+    protected ConfidentialKey(String id) {
+        this.id = id;
+    }
+
+    protected @CheckForNull byte[] load() throws IOException {
+        return ConfidentialStore.get().load(this);
+    }
+
+    protected void store(byte[] payload) throws IOException {
+        ConfidentialStore.get().store(this,payload);
+    }
+
+    public String getId() {
+        return id;
+    }
+}
diff --git a/core/src/main/java/jenkins/security/ConfidentialStore.java b/core/src/main/java/jenkins/security/ConfidentialStore.java
new file mode 100644
index 000000000..0c768f0af
--- /dev/null
+++ b/core/src/main/java/jenkins/security/ConfidentialStore.java
@@ -0,0 +1,61 @@
+package jenkins.security;
+
+import hudson.Extension;
+import jenkins.model.Jenkins;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import java.io.IOException;
+import java.security.SecureRandom;
+
+/**
+ * The actual storage for the data held by {@link ConfidentialKey}s, and the holder
+ * of the master secret.
+ *
+ * <p>
+ * This class is only relevant for the implementers of {@link ConfidentialKey}s.
+ * Most plugin code should interact with {@link ConfidentialKey}s.
+ *
+ * <p>
+ * OEM distributions of Jenkins can provide a custom {@link ConfidentialStore} implementation
+ * by writing a subclass, mark it with {@link Extension} annotation, package it as a Jenkins module,
+ * and bundling it with the war file.
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.498
+ */
+public abstract class ConfidentialStore {
+    /**
+     * Persists the payload of {@link ConfidentialKey} to a persisted storage (such as disk.)
+     * The expectation is that the persisted form is secure.
+     */
+    protected abstract void store(ConfidentialKey key, byte[] payload) throws IOException;
+
+    /**
+     * Reverse operation of {@link #store(ConfidentialKey, byte[])}
+     *
+     * @return
+     *      null the data has not been previously persisted, or if the data was tampered.
+     */
+    protected abstract @CheckForNull byte[] load(ConfidentialKey key) throws IOException;
+
+    /**
+     * Works like {@link SecureRandom#nextBytes(byte[])}.
+     *
+     * This enables implementations to consult other entropy sources, if it's available.
+     */
+    public abstract byte[] randomBytes(int size);
+
+    /**
+     * Retrieves the currently active singleton instance of {@link ConfidentialStore}.
+     */
+    public static @Nonnull ConfidentialStore get() {
+        if (TEST!=null) return TEST.get();
+        return Jenkins.getInstance().getExtensionList(ConfidentialStore.class).get(0);
+    }
+
+    /**
+     * Testing only. Used for testing {@link ConfidentialKey} without {@link Jenkins}
+     */
+    /*package*/ static ThreadLocal<ConfidentialStore> TEST = null;
+}
diff --git a/core/src/main/java/jenkins/security/CryptoConfidentialKey.java b/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
new file mode 100644
index 000000000..dd1dad9e3
--- /dev/null
+++ b/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
@@ -0,0 +1,76 @@
+package jenkins.security;
+
+import hudson.util.Secret;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+
+/**
+ * {@link ConfidentialKey} that stores a {@link SecretKey} for shared-secret cryptography (AES).
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.498
+ */
+public class CryptoConfidentialKey extends ConfidentialKey {
+    private volatile SecretKey secret;
+    public CryptoConfidentialKey(String id) {
+        super(id);
+    }
+
+    public CryptoConfidentialKey(Class owner, String shortName) {
+        this(owner.getName()+'.'+shortName);
+    }
+
+    private SecretKey getKey() {
+        try {
+            if (secret==null) {
+                synchronized (this) {
+                    if (secret==null) {
+                        byte[] payload = load();
+                        if (payload==null) {
+                            payload = ConfidentialStore.get().randomBytes(256);
+                            store(payload);
+                        }
+                        // Due to the stupid US export restriction JDK only ships 128bit version.
+                        secret = new SecretKeySpec(payload,0,128/8, ALGORITHM);
+                    }
+                }
+            }
+            return secret;
+        } catch (IOException e) {
+            throw new Error("Failed to load the key: "+getId(),e);
+        }
+    }
+
+    /**
+     * Returns a {@link Cipher} object for encrypting with this key.
+     */
+    public Cipher encrypt() {
+        try {
+            Cipher cipher = Secret.getCipher(ALGORITHM);
+            cipher.init(Cipher.ENCRYPT_MODE, getKey());
+            return cipher;
+        } catch (GeneralSecurityException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    /**
+     * Returns a {@link Cipher} object for decrypting with this key.
+     */
+    public Cipher decrypt() {
+        try {
+            Cipher cipher = Secret.getCipher(ALGORITHM);
+            cipher.init(Cipher.DECRYPT_MODE, getKey());
+            return cipher;
+        } catch (GeneralSecurityException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+
+    private static final String ALGORITHM = "AES";
+}
diff --git a/core/src/main/java/jenkins/security/DefaultConfidentialStore.java b/core/src/main/java/jenkins/security/DefaultConfidentialStore.java
new file mode 100644
index 000000000..23e42c4d5
--- /dev/null
+++ b/core/src/main/java/jenkins/security/DefaultConfidentialStore.java
@@ -0,0 +1,145 @@
+package jenkins.security;
+
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.Util;
+import hudson.util.IOException2;
+import hudson.util.IOUtils;
+import hudson.util.Secret;
+import hudson.util.TextFile;
+import jenkins.model.Jenkins;
+
+import javax.crypto.Cipher;
+import javax.crypto.CipherInputStream;
+import javax.crypto.CipherOutputStream;
+import javax.crypto.SecretKey;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+
+/**
+ * Default portable implementation of {@link ConfidentialStore} that uses
+ * a directory inside $JENKINS_HOME.
+ *
+ * The master key is also stored in this same directory.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+@Extension(ordinal=-99999) // small ordinal value to allow other higher ones to take over
+public class DefaultConfidentialStore extends ConfidentialStore {
+    private final SecureRandom sr = new SecureRandom();
+
+    /**
+     * Directory that stores individual keys.
+     */
+    private final File rootDir;
+
+    /**
+     * The master key.
+     *
+     * The sole purpose of the master key is to encrypt individual keys on the disk.
+     * Because leaking this master key compromises all the individual keys, we must not let
+     * this master key used for any other purpose, hence the protected access.
+     */
+    private final SecretKey masterKey;
+
+    public DefaultConfidentialStore() throws IOException, InterruptedException {
+        this(new File(Jenkins.getInstance().getRootDir(),"secrets"));
+    }
+
+    public DefaultConfidentialStore(File rootDir) throws IOException, InterruptedException {
+        this.rootDir = rootDir;
+        if (rootDir.mkdirs()) {
+            // protect this directory. but don't change the permission of the existing directory
+            // in case the administrator changed this.
+            new FilePath(rootDir).chmod(0700);
+        }
+
+        TextFile masterSecret = new TextFile(new File(rootDir,"master.key"));
+        if (!masterSecret.exists()) {
+            // we are only going to use small number of bits (since export control limits AES key length)
+            // but let's generate a long enough key anyway
+            masterSecret.write(Util.toHexString(randomBytes(128)));
+        }
+        this.masterKey = Util.toAes128Key(masterSecret.readTrim());
+    }
+
+    /**
+     * Persists the payload of {@link ConfidentialKey} to the disk.
+     */
+    @Override
+    protected void store(ConfidentialKey key, byte[] payload) throws IOException {
+        CipherOutputStream cos=null;
+        FileOutputStream fos=null;
+        try {
+            Cipher sym = Secret.getCipher("AES");
+            sym.init(Cipher.ENCRYPT_MODE, masterKey);
+            cos = new CipherOutputStream(fos=new FileOutputStream(getFileFor(key)), sym);
+            cos.write(payload);
+            cos.write(MAGIC);
+        } catch (GeneralSecurityException e) {
+            throw new IOException2("Failed to persist the key: "+key.getId(),e);
+        } finally {
+            IOUtils.closeQuietly(cos);
+            IOUtils.closeQuietly(fos);
+        }
+    }
+
+    /**
+     * Reverse operation of {@link #store(ConfidentialKey, byte[])}
+     *
+     * @return
+     *      null the data has not been previously persisted.
+     */
+    @Override
+    protected byte[] load(ConfidentialKey key) throws IOException {
+        CipherInputStream cis=null;
+        FileInputStream fis=null;
+        try {
+            File f = getFileFor(key);
+            if (!f.exists())    return null;
+
+            Cipher sym = Secret.getCipher("AES");
+            sym.init(Cipher.DECRYPT_MODE, masterKey);
+            cis = new CipherInputStream(fis=new FileInputStream(f), sym);
+            byte[] bytes = IOUtils.toByteArray(cis);
+            return verifyMagic(bytes);
+        } catch (GeneralSecurityException e) {
+            throw new IOException2("Failed to persist the key: "+key.getId(),e);
+        } finally {
+            IOUtils.closeQuietly(cis);
+            IOUtils.closeQuietly(fis);
+        }
+    }
+
+    /**
+     * Verifies that the given byte[] has the MAGIC trailer, to verify the integrity of the decryption process.
+     */
+    private byte[] verifyMagic(byte[] payload) {
+        int payloadLen = payload.length-MAGIC.length;
+        if (payloadLen<0)   return null;    // obviously broken
+
+        for (int i=0; i<MAGIC.length; i++) {
+            if (payload[payloadLen+i]!=MAGIC[i])
+                return null;    // broken
+        }
+        byte[] truncated = new byte[payloadLen];
+        System.arraycopy(payload,0,truncated,0,truncated.length);
+        return truncated;
+    }
+
+    private File getFileFor(ConfidentialKey key) {
+        return new File(rootDir, key.getId());
+    }
+
+    public byte[] randomBytes(int size) {
+        byte[] random = new byte[size];
+        sr.nextBytes(random);
+        return random;
+    }
+
+    private static final byte[] MAGIC = "::::MAGIC::::".getBytes();
+}
diff --git a/core/src/main/java/jenkins/security/HMACConfidentialKey.java b/core/src/main/java/jenkins/security/HMACConfidentialKey.java
new file mode 100644
index 000000000..71a0c2eb2
--- /dev/null
+++ b/core/src/main/java/jenkins/security/HMACConfidentialKey.java
@@ -0,0 +1,144 @@
+package jenkins.security;
+
+import hudson.Util;
+
+import javax.crypto.KeyGenerator;
+import javax.crypto.Mac;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.security.GeneralSecurityException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+
+/**
+ * {@link ConfidentialKey} that's used for creating a token by hashing some information with secret
+ * (such as <tt>hash(msg|secret)</tt>).
+ *
+ * <p>
+ * This provides more secure version of it by using HMAC.
+ * See http://rdist.root.org/2009/10/29/stop-using-unsafe-keyed-hashes-use-hmac/ for background.
+ * This implementation also never leaks the secret value to outside, so it makes it impossible
+ * for the careless caller to misuse the key (thus protecting ourselves from our own stupidity!)
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.498
+ */
+public class HMACConfidentialKey extends ConfidentialKey {
+    private volatile SecretKey key;
+    private final int length;
+
+    /**
+     * @param length
+     *      Byte length of the HMAC code.
+     *      By default we use HMAC-SHA256, which produces 256bit (=32bytes) HMAC,
+     *      but if different use cases requires a shorter HMAC, specify the desired length here.
+     *      Note that when using {@link #mac(String)}, string encoding causes the length to double.
+     *      So if you want to get 16-letter HMAC, you specify 8 here.
+     */
+    public HMACConfidentialKey(String id, int length) {
+        super(id);
+        this.length = length;
+    }
+
+    /**
+     * Calls into {@link #HMACConfidentialKey(String, int)} with the longest possible HMAC length.
+     */
+    public HMACConfidentialKey(String id) {
+        this(id,Integer.MAX_VALUE);
+    }
+
+    /**
+     * Calls into {@link #HMACConfidentialKey(String, int)} by combining the class name and the shortName
+     * as the ID.
+     */
+    public HMACConfidentialKey(Class owner, String shortName, int length) {
+        this(owner.getName()+'.'+shortName,length);
+    }
+
+    public HMACConfidentialKey(Class owner, String shortName) {
+        this(owner,shortName,Integer.MAX_VALUE);
+    }
+
+
+    /**
+     * Computes the message authentication code for the specified byte sequence.
+     */
+    public byte[] mac(byte[] message) {
+        return chop(createMac().doFinal(message));
+    }
+
+    /**
+     * Convenience method for verifying the MAC code.
+     */
+    public boolean checkMac(byte[] message, byte[] mac) {
+        return Arrays.equals(mac(message),mac);
+    }
+
+    /**
+     * Computes the message authentication code and return it as a string.
+     * While redundant, often convenient.
+     */
+    public String mac(String message) {
+        try {
+            return Util.toHexString(mac(message.getBytes("UTF-8")));
+        } catch (UnsupportedEncodingException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    /**
+     * Verifies MAC constructed from {@link #mac(String)}
+     */
+    public boolean checkMac(String message, String mac) {
+        return mac(message).equals(mac);
+    }
+
+    private byte[] chop(byte[] mac) {
+        if (mac.length<=length)  return mac; // already too short
+
+        byte[] b = new byte[length];
+        System.arraycopy(mac,0,b,0,b.length);
+        return b;
+    }
+
+    /**
+     * Creates a new {@link Mac} object.
+     */
+    public Mac createMac() {
+        try {
+            Mac mac = Mac.getInstance(ALGORITHM);
+            mac.init(getKey());
+            return mac;
+        } catch (GeneralSecurityException e) {
+            // Javadoc says HmacSHA256 must be supported by every Java implementation.
+            throw new Error(ALGORITHM+" not supported?",e);
+        }
+    }
+
+    private SecretKey getKey() {
+        if (key==null) {
+            synchronized (this) {
+                if (key==null) {
+                    try {
+                        byte[] encoded = load();
+                        if (encoded==null) {
+                            KeyGenerator kg = KeyGenerator.getInstance(ALGORITHM);
+                            SecretKey key = kg.generateKey();
+                            store(encoded=key.getEncoded());
+                        }
+                        key = new SecretKeySpec(encoded,ALGORITHM);
+                    } catch (IOException e) {
+                        throw new Error("Failed to load the key: "+getId(),e);
+                    } catch (NoSuchAlgorithmException e) {
+                        throw new Error("Failed to load the key: "+getId(),e);
+                    }
+                }
+            }
+        }
+        return key;
+    }
+
+    private static final String ALGORITHM = "HmacSHA256";
+}
diff --git a/core/src/main/java/jenkins/security/HexStringConfidentialKey.java b/core/src/main/java/jenkins/security/HexStringConfidentialKey.java
new file mode 100644
index 000000000..75fb259bd
--- /dev/null
+++ b/core/src/main/java/jenkins/security/HexStringConfidentialKey.java
@@ -0,0 +1,65 @@
+package jenkins.security;
+
+import hudson.Util;
+
+import java.io.IOException;
+
+/**
+ * {@link ConfidentialKey} that is the random hexadecimal string of length N.
+ *
+ * <p>
+ * This is typically used as a unique ID, as a hex dump is suitable for printing, copy-pasting,
+ * as well as use as an identifier.
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.498
+ */
+public class HexStringConfidentialKey extends ConfidentialKey {
+    private final int length;
+
+    private volatile String secret;
+
+    /**
+     * @param length
+     *      Length of the hexadecimal string.
+     */
+    public HexStringConfidentialKey(String id, int length) {
+        super(id);
+        if (length%2!=0)
+            throw new IllegalArgumentException("length must be even: "+length);
+        this.length = length;
+    }
+
+    public HexStringConfidentialKey(Class owner, String shortName, int length) {
+        this(owner.getName()+'.'+shortName,length);
+    }
+
+    /**
+     * Returns the persisted hex string value.
+     *
+     * If the value isn't persisted, a new random value is created.
+     *
+     * @throws Error
+     *      If the secret fails to load. Not throwing a checked exception is for the convenience
+     *      of the caller.
+     */
+    public String get() {
+        try {
+            if (secret==null) {
+                synchronized (this) {
+                    if (secret==null) {
+                        byte[] payload = load();
+                        if (payload==null) {
+                            payload = ConfidentialStore.get().randomBytes(length/2);
+                            store(payload);
+                        }
+                        secret = Util.toHexString(payload).substring(0,length);
+                    }
+                }
+            }
+            return secret;
+        } catch (IOException e) {
+            throw new Error("Failed to load the key: "+getId(),e);
+        }
+    }
+}
diff --git a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java
index 2050ce1ec..664f5e382 100644
--- a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java
+++ b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java
@@ -6,10 +6,11 @@ import hudson.remoting.Channel;
 import hudson.remoting.Channel.Listener;
 import hudson.remoting.Engine;
 import hudson.slaves.SlaveComputer;
 import jenkins.AgentProtocol;
 import jenkins.model.Jenkins;
+import jenkins.security.HMACConfidentialKey;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
@@ -29,14 +30,14 @@ import java.util.logging.Logger;
  * executed on the master, so in a way this is like an rsh service.
  * Therefore, it is important that we reject connections from
  * unauthorized remote slaves.
  *
  * <p>
- * The approach here is to have {@link Jenkins#getSecretKey() a secret key} on the master.
- * This key is sent to the slave inside the <tt>.jnlp</tt> file
+ * We do this by computing HMAC of the slave name.
+ * This code is sent to the slave inside the <tt>.jnlp</tt> file
  * (this file itself is protected by HTTP form-based authentication that
- * we use everywhere else in Hudson), and the slave sends this
+ * we use everywhere else in Jenkins), and the slave sends this
  * token back when it connects to the master.
  * Unauthorized slaves can't access the protected <tt>.jnlp</tt> file,
  * so it can't impersonate a valid slave.
  *
  * <p>
@@ -82,16 +83,19 @@ public class JnlpSlaveAgentProtocol extends AgentProtocol {
             in = new DataInputStream(socket.getInputStream());
             out = new PrintWriter(socket.getOutputStream(),true);
         }
 
         protected void run() throws IOException, InterruptedException {
-            if(!getSecretKey().equals(in.readUTF())) {
+            final String secret = in.readUTF();
+            final String nodeName = in.readUTF();
+
+            if(!SLAVE_SECRET.mac(nodeName).equals(secret)) {
                 error(out, "Unauthorized access");
                 return;
             }
 
-            final String nodeName = in.readUTF();
+
             SlaveComputer computer = (SlaveComputer) Jenkins.getInstance().getComputer(nodeName);
             if(computer==null) {
                 error(out, "No such slave: "+nodeName);
                 return;
             }
@@ -136,18 +140,19 @@ public class JnlpSlaveAgentProtocol extends AgentProtocol {
                 e.printStackTrace(logw);
                 throw e;
             }
         }
 
-        protected String getSecretKey() {
-            return Jenkins.getInstance().getSecretKey();
-        }
-
         protected void error(PrintWriter out, String msg) throws IOException {
             out.println(msg);
             LOGGER.log(Level.WARNING,Thread.currentThread().getName()+" is aborted: "+msg);
             socket.close();
         }
     }
 
     private static final Logger LOGGER = Logger.getLogger(JnlpSlaveAgentProtocol.class.getName());
+
+    /**
+     * This secret value is used as a seed for slaves.
+     */
+    public static final HMACConfidentialKey SLAVE_SECRET = new HMACConfidentialKey(JnlpSlaveAgentProtocol.class,"secret");
 }
diff --git a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java
index 413486e38..86da2de8a 100644
--- a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java
+++ b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java
@@ -55,16 +55,17 @@ public class JnlpSlaveAgentProtocol2 extends JnlpSlaveAgentProtocol {
         @Override
         protected void run() throws IOException, InterruptedException {
             Properties request = new Properties();
             request.load(new ByteArrayInputStream(in.readUTF().getBytes("UTF-8")));
 
-            if(!getSecretKey().equals(request.getProperty("Secret-Key"))) {
+            final String nodeName = request.getProperty("Node-Name");
+
+            if(!SLAVE_SECRET.mac(nodeName).equals(request.getProperty("Secret-Key"))) {
                 error(out, "Unauthorized access");
                 return;
             }
 
-            final String nodeName = request.getProperty("Node-Name");
             SlaveComputer computer = (SlaveComputer) Jenkins.getInstance().getComputer(nodeName);
             if(computer==null) {
                 error(out, "No such slave: "+nodeName);
                 return;
             }
diff --git a/core/src/main/resources/hudson/slaves/SlaveComputer/slave-agent.jnlp.jelly b/core/src/main/resources/hudson/slaves/SlaveComputer/slave-agent.jnlp.jelly
index 32962d7b3..317743245 100644
--- a/core/src/main/resources/hudson/slaves/SlaveComputer/slave-agent.jnlp.jelly
+++ b/core/src/main/resources/hudson/slaves/SlaveComputer/slave-agent.jnlp.jelly
@@ -29,11 +29,13 @@ THE SOFTWARE.
   <j:set var="rootURL" value="${app.rootUrl}" />
 
   <!--
     See http://www.dallaway.com/acad/webstart/ for obtaining the certificate.
   -->
-  <l:isAdminOrTest test="true">
+  <j:getStatic var="connect" className="hudson.model.Computer" field="CONNECT"/>
+  ${it.checkPermission(connect)}
+
     <!-- See http://java.sun.com/j2se/1.5.0/docs/guide/javaws/developersguide/syntax.html for the syntax -->
     <jnlp spec="1.0+"
         codebase="${rootURL}computer/${h.encode(it.node.nodeName)}/">
 
       <information>
@@ -59,11 +61,11 @@ THE SOFTWARE.
         <jar href="${rootURL}jnlpJars/remoting.jar"/>
         <property name="hudson.showWindowsServiceInstallLink" value="true" />
       </resources>
 
       <application-desc main-class="hudson.remoting.jnlp.Main">
-        <argument>${app.secretKey}</argument>
+        <argument>${it.jnlpMac}</argument>
         <argument>${it.node.nodeName}</argument>
         <j:if test="${it.launcher.tunnel!=null}">
           <argument>-tunnel</argument>
           <argument>${it.launcher.tunnel}</argument>
         </j:if>
@@ -85,7 +87,6 @@ THE SOFTWARE.
           <argument>-url</argument>
           <argument>${app.rootUrlFromRequest}</argument>
         </j:if>
       </application-desc>
     </jnlp>
-  </l:isAdminOrTest>
 </j:jelly>
diff --git a/core/src/test/java/hudson/util/SecretTest.groovy b/core/src/test/java/hudson/util/SecretTest.groovy
new file mode 100644
index 000000000..ef9385029
--- /dev/null
+++ b/core/src/test/java/hudson/util/SecretTest.groovy
@@ -0,0 +1,119 @@
+/*
+ * The MIT License
+ * 
+ * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package hudson.util
+
+import com.trilead.ssh2.crypto.Base64;
+import jenkins.model.Jenkins
+import jenkins.security.ConfidentialStoreRule;
+import org.junit.After
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+
+import javax.crypto.Cipher;
+import java.security.SecureRandom;
+
+import hudson.Util;
+
+/**
+ * @author Kohsuke Kawaguchi
+ */
+public class SecretTest {
+    @Rule
+    public ConfidentialStoreRule confidentialStore = new ConfidentialStoreRule()
+
+    @Test @Before
+    void setUp() {
+        def sr = new SecureRandom();
+        byte[] random = new byte[32];
+        sr.nextBytes(random);
+        Secret.SECRET = Util.toHexString(random);
+    }
+
+    @Test @After
+    void tearDown() {
+        Secret.SECRET = null;
+    }
+
+    @Test
+    void testEncrypt() {
+        def secret = Secret.fromString("abc");
+        assert "abc"==secret.plainText;
+
+        // make sure we got some encryption going
+        println secret.encryptedValue;
+        assert !"abc".equals(secret.encryptedValue);
+
+        // can we round trip?
+        assert secret==Secret.fromString(secret.encryptedValue);
+    }
+
+    @Test
+    void testDecrypt() {
+        assert "abc"==Secret.toString(Secret.fromString("abc"))
+    }
+
+    @Test
+    void testSerialization() {
+        def s = Secret.fromString("Mr.Jenkins");
+        def xml = Jenkins.XSTREAM.toXML(s);
+        assert !xml.contains(s.plainText)
+        assert xml.contains(s.encryptedValue)
+
+        def o = Jenkins.XSTREAM.fromXML(xml);
+        assert o==s : xml;
+    }
+
+    public static class Foo {
+        Secret password;
+    }
+
+    /**
+     * Makes sure the serialization form is backward compatible with String.
+     */
+    @Test
+    void testCompatibilityFromString() {
+        def tagName = Foo.class.name.replace("\$","_-");
+        def xml = "<$tagName><password>secret</password></$tagName>";
+        def foo = new Foo();
+        Jenkins.XSTREAM.fromXML(xml, foo);
+        assert "secret"==Secret.toString(foo.password)
+    }
+
+    /**
+     * Secret persisted with Jenkins.getSecretKey() should still decrypt OK.
+     */
+    @Test
+    void migrationFromLegacyKeyToConfidentialStore() {
+        def legacy = Secret.legacyKey
+        ["Hello world","","\u0000unprintable"].each { str ->
+            def cipher = Secret.getCipher("AES");
+            cipher.init(Cipher.ENCRYPT_MODE, legacy);
+            def old = new String(Base64.encode(cipher.doFinal((str + Secret.MAGIC).getBytes("UTF-8"))))
+            def s = Secret.fromString(old)
+            assert s.plainText==str : "secret by the old key should decrypt"
+            assert s.encryptedValue!=old : "but when encrypting, ConfidentialKey should be in use"
+        }
+    }
+}
diff --git a/core/src/test/java/hudson/util/SecretTest.java b/core/src/test/java/hudson/util/SecretTest.java
deleted file mode 100644
index 24102d358..000000000
--- a/core/src/test/java/hudson/util/SecretTest.java
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * The MIT License
- * 
- * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
- * 
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- * 
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- * 
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-package hudson.util;
-
-import jenkins.model.Jenkins;
-import junit.framework.TestCase;
-
-import java.security.SecureRandom;
-
-import hudson.Util;
-
-/**
- * @author Kohsuke Kawaguchi
- */
-public class SecretTest extends TestCase {
-    @Override protected void setUp() throws Exception {
-        SecureRandom sr = new SecureRandom();
-        byte[] random = new byte[32];
-        sr.nextBytes(random);
-        Secret.SECRET = Util.toHexString(random);
-
-    }
-
-    @Override protected void tearDown() throws Exception {
-        Secret.SECRET = null;
-    }
-
-    public void testEncrypt() {
-        Secret secret = Secret.fromString("abc");
-        assertEquals("abc",secret.getPlainText());
-
-        // make sure we got some encryption going
-        System.out.println(secret.getEncryptedValue());
-        assertTrue(!"abc".equals(secret.getEncryptedValue()));
-
-        // can we round trip?
-        assertEquals(secret,Secret.fromString(secret.getEncryptedValue()));
-    }
-
-    public void testDecrypt() {
-        assertEquals("abc",Secret.toString(Secret.fromString("abc")));
-    }
-
-    public void testSerialization() {
-        Secret s = Secret.fromString("Mr.Hudson");
-        String xml = Jenkins.XSTREAM.toXML(s);
-        assertTrue(xml, !xml.contains(s.getPlainText()));
-        assertTrue(xml, xml.contains(s.getEncryptedValue()));
-        Object o = Jenkins.XSTREAM.fromXML(xml);
-        assertEquals(xml, o, s);
-    }
-
-    public static class Foo {
-        Secret password;
-    }
-
-    /**
-     * Makes sure the serialization form is backward compatible with String.
-     */
-    public void testCompatibilityFromString() {
-        String tagName = Foo.class.getName().replace("$","_-");
-        String xml = "<"+tagName+"><password>secret</password></"+tagName+">";
-        Foo foo = new Foo();
-        Jenkins.XSTREAM.fromXML(xml, foo);
-        assertEquals("secret",Secret.toString(foo.password));
-    }
-}
diff --git a/core/src/test/java/jenkins/security/ConfidentialStoreRule.java b/core/src/test/java/jenkins/security/ConfidentialStoreRule.java
new file mode 100644
index 000000000..6f940b291
--- /dev/null
+++ b/core/src/test/java/jenkins/security/ConfidentialStoreRule.java
@@ -0,0 +1,37 @@
+package jenkins.security;
+
+import hudson.Util;
+import org.junit.rules.ExternalResource;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * Test rule that injects a temporary {@link DefaultConfidentialStore}
+ * @author Kohsuke Kawaguchi
+ */
+public class ConfidentialStoreRule extends ExternalResource {
+    public ConfidentialStore store;
+    public File tmp;
+
+    @Override
+    protected void before() throws Throwable {
+        tmp = Util.createTempDir();
+        store = new DefaultConfidentialStore(tmp);
+        ConfidentialStore.TEST.set(store);
+    }
+
+    @Override
+    protected void after() {
+        ConfidentialStore.TEST.set(null);
+        try {
+            Util.deleteRecursive(tmp);
+        } catch (IOException e) {
+            throw new Error(e);
+        }
+    }
+
+    static {
+        ConfidentialStore.TEST = new ThreadLocal<ConfidentialStore>();
+    }
+}
diff --git a/core/src/test/java/jenkins/security/CryptoConfidentialKeyTest.groovy b/core/src/test/java/jenkins/security/CryptoConfidentialKeyTest.groovy
new file mode 100644
index 000000000..e3816a342
--- /dev/null
+++ b/core/src/test/java/jenkins/security/CryptoConfidentialKeyTest.groovy
@@ -0,0 +1,37 @@
+package jenkins.security
+
+import org.junit.Rule
+import org.junit.Test
+
+/**
+ *
+ *
+ * @author Kohsuke Kawaguchi
+ */
+class CryptoConfidentialKeyTest {
+    @Rule
+    public ConfidentialStoreRule store = new ConfidentialStoreRule()
+
+    def key = new CryptoConfidentialKey("test")
+
+    @Test
+    void decryptGetsPlainTextBack() {
+        ["Hello world","","\u0000"].each { str ->
+            assert key.decrypt().doFinal(key.encrypt().doFinal(str.bytes))==str.bytes
+        }
+    }
+
+    @Test
+    void multipleEncryptsAreIdempotent() {
+        def str = "Hello world".bytes
+        assert key.encrypt().doFinal(str)==key.encrypt().doFinal(str)
+    }
+
+    @Test
+    void loadingExistingKey() {
+        def key2 = new CryptoConfidentialKey("test") // this will cause the key to be loaded from the disk
+        ["Hello world","","\u0000"].each { str ->
+            assert key2.decrypt().doFinal(key.encrypt().doFinal(str.bytes))==str.bytes
+        }
+    }
+}
diff --git a/core/src/test/java/jenkins/security/DefaultConfidentialStoreTest.groovy b/core/src/test/java/jenkins/security/DefaultConfidentialStoreTest.groovy
new file mode 100644
index 000000000..83cdaf218
--- /dev/null
+++ b/core/src/test/java/jenkins/security/DefaultConfidentialStoreTest.groovy
@@ -0,0 +1,54 @@
+package jenkins.security
+
+import hudson.FilePath
+import hudson.Functions
+import hudson.Util
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+
+/**
+ * @author Kohsuke Kawaguchi
+ */
+public class DefaultConfidentialStoreTest {
+
+    def tmp;
+
+    @Before
+    void setup() {
+        tmp = Util.createTempDir()
+    }
+
+    @After
+    void tearDown() {
+        Util.deleteRecursive(tmp)
+    }
+
+    @Test
+    void roundtrip() {
+        tmp.deleteDir()   // let ConfidentialStore create a directory
+
+        def store = new DefaultConfidentialStore(tmp);
+        def key = new ConfidentialKey("test") {};
+
+        // basic roundtrip
+        def str = "Hello world!"
+        store.store(key, str.bytes)
+        assert new String(store.load(key))==str
+
+        // data storage should have some stuff
+        assert new File(tmp,"test").exists()
+        assert new File(tmp,"master.key").exists()
+
+        assert !new File(tmp,"test").text.contains("Hello") // the data shouldn't be a plain text, obviously
+
+        if (!Functions.isWindows())
+            assert (new FilePath(tmp).mode()&0777) == 0700 // should be read only
+
+        // if the master key changes, we should gracefully fail to load the store
+        new File(tmp,"master.key").delete()
+        def store2 = new DefaultConfidentialStore(tmp)
+        assert new File(tmp,"master.key").exists()  // we should have a new key now
+        assert store2.load(key)==null;
+    }
+}
diff --git a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.groovy b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.groovy
new file mode 100644
index 000000000..118ff5b90
--- /dev/null
+++ b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.groovy
@@ -0,0 +1,39 @@
+package jenkins.security
+
+import org.junit.Rule
+import org.junit.Test
+
+/**
+ *
+ *
+ * @author Kohsuke Kawaguchi
+ */
+class HMACConfidentialKeyTest {
+    @Rule
+    public ConfidentialStoreRule store = new ConfidentialStoreRule()
+
+    def key = new HMACConfidentialKey("test",16)
+
+    @Test
+    void basics() {
+        def unique = [] as TreeSet
+        ["Hello world","","\u0000"].each { str ->
+            def mac = key.mac(str)
+            unique.add(mac)
+            assert mac =~ /[0-9A-Fa-f]{32}/
+            assert key.checkMac(str,mac)
+            assert !key.checkMac("garbage",mac)
+        }
+
+        assert unique.size()==3 // make sure all 3 MAC are different
+    }
+
+    @Test
+    void loadingExistingKey() {
+        // this second key of the same ID will cause it to load the key from the disk
+        def key2 = new HMACConfidentialKey("test",16)
+        ["Hello world","","\u0000"].each { str ->
+            assert key.mac(str)==key2.mac(str)
+        }
+    }
+}
diff --git a/core/src/test/java/jenkins/security/HexStringConfidentialKeyTest.groovy b/core/src/test/java/jenkins/security/HexStringConfidentialKeyTest.groovy
new file mode 100644
index 000000000..7062d2326
--- /dev/null
+++ b/core/src/test/java/jenkins/security/HexStringConfidentialKeyTest.groovy
@@ -0,0 +1,43 @@
+package jenkins.security
+
+import org.junit.Rule
+import org.junit.Test
+
+/**
+ *
+ *
+ * @author Kohsuke Kawaguchi
+ */
+class HexStringConfidentialKeyTest {
+    @Rule
+    public ConfidentialStoreRule store = new ConfidentialStoreRule()
+
+    @Test
+    void hexStringShouldProduceHexString() {
+        def key = new HexStringConfidentialKey("test",8)
+        assert key.get() =~ /[A-Fa-f0-9]{8}/
+    }
+
+    @Test
+    void multipleGetsAreIdempotent() {
+        def key = new HexStringConfidentialKey("test",8)
+        assert key.get()==key.get()
+    }
+
+    @Test
+    void specifyLengthAndMakeSureItTakesEffect() {
+        [8,16,32,256].each { n ->
+            new HexStringConfidentialKey("test"+n,n).get().length()==n
+        }
+    }
+
+    @Test
+    void loadingExistingKey() {
+        def key1 = new HexStringConfidentialKey("test",8)
+        key1.get() // this causes the ke to be generated
+
+        // this second key of the same ID will cause it to load the key from the disk
+        def key2 = new HexStringConfidentialKey("test",8)
+        assert key1.get()==key2.get()
+    }
+}
diff --git a/pom.xml b/pom.xml
index a0be8bf17..fe89506b1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -450,11 +450,11 @@ THE SOFTWARE.
         </plugin>
         <plugin>
           <!-- see http://maven-junit-plugin.kenai.com/ for more info -->
           <groupId>org.kohsuke</groupId>
           <artifactId>maven-junit-plugin</artifactId>
-          <version>1.10</version>
+          <version>1.11</version>
          </plugin>
         <plugin>
           <groupId>org.jvnet.localizer</groupId>
           <artifactId>maven-localizer-plugin</artifactId>
           <version>1.14</version>
diff --git a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
index 96c498837..b4d43cd3b 100644
--- a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
+++ b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
@@ -21,10 +21,11 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
 package hudson.bugs;
 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
 import com.gargoylesoftware.htmlunit.Page;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import com.gargoylesoftware.htmlunit.xml.XmlPage;
 import hudson.model.Node.Mode;
 import hudson.model.Slave;
@@ -37,10 +38,11 @@ import org.dom4j.io.DOMReader;
 import org.jvnet.hudson.test.Email;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.recipes.PresetData;
 import org.jvnet.hudson.test.recipes.PresetData.DataSet;
 
+import java.net.HttpURLConnection;
 import java.net.URL;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 
@@ -56,11 +58,11 @@ public class JnlpAccessWithSecuredHudsonTest extends HudsonTestCase {
      */
     protected Slave createNewJnlpSlave(String name) throws Exception {
         return new DumbSlave(name,"",System.getProperty("java.io.tmpdir")+'/'+name,"2", Mode.NORMAL, "", new JNLPLauncher(), RetentionStrategy.INSTANCE, Collections.EMPTY_LIST);
     }
 
-    @PresetData(DataSet.NO_ANONYMOUS_READACCESS)
+    @PresetData(DataSet.ANONYMOUS_READONLY)
     @Email("http://www.nabble.com/Launching-slave-by-JNLP-with-Active-Directory-plugin-and-matrix-security-problem-td18980323.html")
     public void test() throws Exception {
         jenkins.setNodes(Collections.singletonList(createNewJnlpSlave("test")));
         HudsonTestCase.WebClient wc = new WebClient();
         HtmlPage p = wc.login("alice").goTo("computer/test/");
@@ -78,7 +80,15 @@ public class JnlpAccessWithSecuredHudsonTest extends HudsonTestCase {
             
             // now make sure that these URLs are unprotected
             Page jarResource = jnlpAgent.getPage(url);
             assertTrue(jarResource.getWebResponse().getContentType().toLowerCase(Locale.ENGLISH).startsWith("application/"));
         }
+
+
+        try {
+            jnlp = (XmlPage) jnlpAgent.goTo("computer/test/slave-agent.jnlp", "application/x-java-jnlp-file");
+            fail("anonymous users must not be able to get secrets");
+        } catch (FailingHttpStatusCodeException x) {
+            assertEquals(HttpURLConnection.HTTP_FORBIDDEN, x.getStatusCode());
+        }
     }
 }

Vulnerable code:                     props.put("server-id", Util.getDigestOf(jenkins.getSecretKey()));
                tag(rsp,"server-id", Util.getDigestOf(jenkins.getSecretKey()));
                Cipher sym = Secret.getCipher("AES");
                sym.init(Cipher.DECRYPT_MODE, Jenkins.getInstance().getSecretKeyAsAES128());
        } catch (GeneralSecurityException e) {
            throw new IOException2(e);
        try {
            ByteArrayOutputStream baos = new ByteArrayOutputStream();
            Cipher sym = Secret.getCipher("AES");
            sym.init(Cipher.ENCRYPT_MODE, Jenkins.getInstance().getSecretKeyAsAES128());
            ObjectOutputStream oos = new ObjectOutputStream(new GZIPOutputStream(new CipherOutputStream(baos,sym)));
            oos.writeLong(System.currentTimeMillis()); // send timestamp to prevent a replay attack
            oos.writeObject(caw.getConsoleAnnotator());
            oos.close();
            StaplerResponse rsp = Stapler.getCurrentResponse();
            if (rsp!=null)
                rsp.setHeader("X-ConsoleAnnotator", new String(Base64.encode(baos.toByteArray())));
        } catch (GeneralSecurityException e) {
            throw new IOException2(e);
        }
        env.put("JENKINS_SERVER_COOKIE",Util.getDigestOf("ServerID:"+ Jenkins.getInstance().getSecretKey()));
        env.put("HUDSON_SERVER_COOKIE",Util.getDigestOf("ServerID:"+ Jenkins.getInstance().getSecretKey())); // Legacy compatibility
        Jenkins h = Jenkins.getInstance();
        o.put("install", Util.getDigestOf(h.getSecretKey()));
        o.put("servletContainer",h.servletContext.getServerInfo());
        for( Computer c : h.getComputers() ) {
            if(c.getNode()==h) {
            DescriptorImpl descriptor = h.getDescriptorByType(DescriptorImpl.class);
        for( PluginWrapper pw : h.getPluginManager().getPlugins() ) {
        List<TopLevelItem> items = h.getItems();
 * In Hudson, we need {@link jenkins.model.Jenkins} instance to perform remember-me service,
 * because it relies on {@link jenkins.model.Jenkins#getSecretKey()}. However, security
 * filters can be initialized before Hudson is initialized.
import org.apache.commons.codec.digest.DigestUtils;
        String expectedTokenSignature = DigestUtils.md5Hex(userDetails.getUsername() + ":" + tokenExpiryTime + ":"
            super(Jenkins.getInstance().getSecretKey(), System.getProperty("hudson.security.csrf.requestfield", ".crumb"));
 * Note that since the cryptography relies on {@link jenkins.model.Jenkins#getSecretKey()}, this is not meant as a protection
 * against code running in the same VM, nor against an attacker who has local file system access. 
     * Turns {@link jenkins.model.Jenkins#getSecretKey()} into an AES key.
    private static SecretKey getKey() throws UnsupportedEncodingException, GeneralSecurityException {
            Cipher cipher = getCipher("AES");
            cipher.init(Cipher.ENCRYPT_MODE, getKey());
            cipher.init(Cipher.DECRYPT_MODE, getKey());
            String plainText = new String(cipher.doFinal(Base64.decode(data.toCharArray())), "UTF-8");
            if(plainText.endsWith(MAGIC))
                return new Secret(plainText.substring(0,plainText.length()-MAGIC.length()));
            return null;
     * For testing only. Override the secret key so that we can test this class without {@link jenkins.model.Jenkins}.
import jenkins.model.Jenkins;
         * the initial API token value. So we take the seed by hasing the instance secret key + user ID.
            return new ApiTokenProperty(Util.getDigestOf(Jenkins.getInstance().getSecretKey() + ":" + user.getId()));
 * The approach here is to have {@link Jenkins#getSecretKey() a secret key} on the master.
 * This key is sent to the slave inside the <tt>.jnlp</tt> file
 * we use everywhere else in Hudson), and the slave sends this
            if(!getSecretKey().equals(in.readUTF())) {
            final String nodeName = in.readUTF();
        protected String getSecretKey() {
            return Jenkins.getInstance().getSecretKey();
        }

            if(!getSecretKey().equals(request.getProperty("Secret-Key"))) {
            final String nodeName = request.getProperty("Node-Name");
  <l:isAdminOrTest test="true">
        <argument>${app.secretKey}</argument>
  </l:isAdminOrTest>
/*
 * The MIT License
 * 
 * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
 * 
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 * 
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 * 
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 * THE SOFTWARE.
 */
package hudson.util;

import jenkins.model.Jenkins;
import junit.framework.TestCase;

import java.security.SecureRandom;

import hudson.Util;

/**
 * @author Kohsuke Kawaguchi
 */
public class SecretTest extends TestCase {
    @Override protected void setUp() throws Exception {
        SecureRandom sr = new SecureRandom();
        byte[] random = new byte[32];
        sr.nextBytes(random);
        Secret.SECRET = Util.toHexString(random);

    }

    @Override protected void tearDown() throws Exception {
        Secret.SECRET = null;
    }

    public void testEncrypt() {
        Secret secret = Secret.fromString("abc");
        assertEquals("abc",secret.getPlainText());

        // make sure we got some encryption going
        System.out.println(secret.getEncryptedValue());
        assertTrue(!"abc".equals(secret.getEncryptedValue()));

        // can we round trip?
        assertEquals(secret,Secret.fromString(secret.getEncryptedValue()));
    }

    public void testDecrypt() {
        assertEquals("abc",Secret.toString(Secret.fromString("abc")));
    }

    public void testSerialization() {
        Secret s = Secret.fromString("Mr.Hudson");
        String xml = Jenkins.XSTREAM.toXML(s);
        assertTrue(xml, !xml.contains(s.getPlainText()));
        assertTrue(xml, xml.contains(s.getEncryptedValue()));
        Object o = Jenkins.XSTREAM.fromXML(xml);
        assertEquals(xml, o, s);
    }

    public static class Foo {
        Secret password;
    }

    /**
     * Makes sure the serialization form is backward compatible with String.
     */
    public void testCompatibilityFromString() {
        String tagName = Foo.class.getName().replace("$","_-");
        String xml = "<"+tagName+"><password>secret</password></"+tagName+">";
        Foo foo = new Foo();
        Jenkins.XSTREAM.fromXML(xml, foo);
        assertEquals("secret",Secret.toString(foo.password));
    }
}
          <version>1.10</version>
    @PresetData(DataSet.NO_ANONYMOUS_READACCESS)
Fixed code:       <plugin><!-- run unit test in src/test/java -->
        <groupId>org.kohsuke.gmaven</groupId>
        <artifactId>gmaven-plugin</artifactId>
        <!-- version specified in grandparent pom -->
        <executions>
          <execution>
            <goals>
              <goal>testCompile</goal>
            </goals>
            <configuration>
              <sources>
                <fileset>
                  <directory>${project.basedir}/src/test/java</directory>
                  <includes>
                    <include>**/*.groovy</include>
                  </includes>
                </fileset>
              </sources>
            </configuration>
          </execution>
        </executions>
        <dependencies>
          <dependency>
            <groupId>org.codehaus.groovy</groupId>
            <artifactId>groovy-all</artifactId>
            <version>1.8.5</version>
          </dependency>
        </dependencies>
      </plugin>
                    props.put("server-id", jenkins.getLegacyInstanceId());
                tag(rsp,"server-id", jenkins.getLegacyInstanceId());
import jenkins.security.CryptoConfidentialKey;
                Cipher sym = PASSING_ANNOTATOR.decrypt();
        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        Cipher sym = PASSING_ANNOTATOR.encrypt();
        ObjectOutputStream oos = new ObjectOutputStream(new GZIPOutputStream(new CipherOutputStream(baos,sym)));
        oos.writeLong(System.currentTimeMillis()); // send timestamp to prevent a replay attack
        oos.writeObject(caw.getConsoleAnnotator());
        oos.close();
        StaplerResponse rsp = Stapler.getCurrentResponse();
        if (rsp!=null)
            rsp.setHeader("X-ConsoleAnnotator", new String(Base64.encode(baos.toByteArray())));
    /**
     * Used for sending the state of ConsoleAnnotator to the client, because we are deserializing this object later.
     */
    private static final CryptoConfidentialKey PASSING_ANNOTATOR = new CryptoConfidentialKey(AnnotatedLargeText.class,"consoleAnnotator");
import jenkins.security.HexStringConfidentialKey;
        env.put("JENKINS_SERVER_COOKIE",SERVER_COOKIE.get());
        env.put("HUDSON_SERVER_COOKIE",SERVER_COOKIE.get()); // Legacy compatibility

    private final static HexStringConfidentialKey SERVER_COOKIE = new HexStringConfidentialKey(Job.class,"serverCookie",16);
        Jenkins j = Jenkins.getInstance();
        o.put("install", j.getLegacyInstanceId());
        o.put("servletContainer", j.servletContext.getServerInfo());
        for( Computer c : j.getComputers() ) {
            if(c.getNode()==j) {
            DescriptorImpl descriptor = j.getDescriptorByType(DescriptorImpl.class);
        for( PluginWrapper pw : j.getPluginManager().getPlugins() ) {
        List<TopLevelItem> items = j.getItems();
import jenkins.model.Jenkins;
import jenkins.security.ConfidentialStore;
 * In Jenkins, we need {@link Jenkins} instance to perform remember-me service,
 * because it relies on {@link ConfidentialStore}. However, security
 * filters can be initialized before Jenkins is initialized.
        @SuppressWarnings("deprecation")
            /*
                TokenBasedRememberMeServices needs to be used in conjunction with RememberMeAuthenticationProvider,
                and both needs to use the same key (this is a reflection of a poor design in AcgeiSecurity, if you ask me)
                and various security plugins have its own groovy script that configures them.

                So if we change this, it creates a painful situation for those plugins by forcing them to choose
                to work with earlier version of Jenkins or newer version of Jenkins, and not both.

                So we keep this here.
             */
import jenkins.security.HMACConfidentialKey;
        String expectedTokenSignature = MAC.mac(userDetails.getUsername() + ":" + tokenExpiryTime + ":"

    /**
     * Used to compute the token signature securely.
     */
    private static final HMACConfidentialKey MAC = new HMACConfidentialKey(TokenBasedRememberMeServices.class,"mac");
            // salt just needs to be unique, and it doesn't have to be a secret
            super(Jenkins.getInstance().getLegacyInstanceId(), System.getProperty("hudson.security.csrf.requestfield", ".crumb"));
import jenkins.slaves.JnlpSlaveAgentProtocol;
    public String getJnlpMac() {
        return JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(getName());
    }

import jenkins.security.CryptoConfidentialKey;
import javax.crypto.BadPaddingException;
import javax.crypto.IllegalBlockSizeException;
 * This is not meant as a protection against code running in the same VM, nor against an attacker
 * who has local file system access on Jenkins master.
     * Turns {@link Jenkins#getSecretKey()} into an AES key.
     *
     * @deprecated
     * This is no longer the key we use to encrypt new information, but we still need this
     * to be able to decrypt what's already persisted.
    /*package*/ static SecretKey getLegacyKey() throws UnsupportedEncodingException, GeneralSecurityException {
            Cipher cipher = KEY.encrypt();
            byte[] in = Base64.decode(data.toCharArray());
            Secret s = tryDecrypt(KEY.decrypt(), in);
            if (s!=null)    return s;

            // try our historical key for backward compatibility
            cipher.init(Cipher.DECRYPT_MODE, getLegacyKey());
            return tryDecrypt(cipher, in);
    private static Secret tryDecrypt(Cipher cipher, byte[] in) throws UnsupportedEncodingException {
        try {
            String plainText = new String(cipher.doFinal(in), "UTF-8");
            if(plainText.endsWith(MAGIC))
                return new Secret(plainText.substring(0,plainText.length()-MAGIC.length()));
            return null;
        } catch (GeneralSecurityException e) {
            return null;
        }
    }

     * For testing only. Override the secret key so that we can test this class without {@link Jenkins}.
    /**
     * The key that encrypts the data on disk.
     */
    private static final CryptoConfidentialKey KEY = new CryptoConfidentialKey(Secret.class.getName());

import jenkins.security.ConfidentialKey;
import jenkins.security.ConfidentialStore;
     *
     * @deprecated
     *      Due to the past security advisory, this value should not be used any more to protect sensitive information.
     *      See {@link ConfidentialStore} and {@link ConfidentialKey} for how to store secrets.
     * @deprecated
     *       See {@link #getSecretKey()}.
    /**
     * Returns the unique identifier of this Jenkins that has been historically used to identify
     * this Jenkins to the outside world.
     *
     * <p>
     * This form of identifier is weak in that it can be impersonated by others. See
     * https://wiki.jenkins-ci.org/display/JENKINS/Instance+Identity for more modern form of instance ID
     * that can be challenged and verified.
     *
     * @since 1.498
     */
    @SuppressWarnings("deprecation")
    public String getLegacyInstanceId() {
        return Util.getDigestOf(getSecretKey());
    }

         * the initial API token value. So we take the seed by hashing the secret + user ID.
            return new ApiTokenProperty(API_KEY_SEED.mac(user.getId()));

    /**
     * We don't want an API key that's too long, so cut the length to 16 (which produces 32-letter MAC code in hexdump)
     */
    private static final HMACConfidentialKey API_KEY_SEED = new HMACConfidentialKey(ApiTokenProperty.class,"seed",16);
package jenkins.security;

import hudson.scm.SCM;
import hudson.tasks.Builder;
import hudson.util.Secret;
import jenkins.slaves.JnlpSlaveAgentProtocol;

import javax.annotation.CheckForNull;
import java.io.IOException;

/**
 * Confidential information that gets stored as a singleton in Jenkins, mostly some random token value.
 *
 * <p>
 * The actual value is persisted via {@link ConfidentialStore}, but each use case that requires
 * a secret like this should use a separate {@link ConfidentialKey} instance so that one compromised
 * {@link ConfidentialKey} (say through incorrect usage and failure to protect it) shouldn't compromise
 * all the others.
 *
 * <p>
 * {@link ConfidentialKey} is ultimately a sequence of bytes,
 * but for convenience, a family of subclasses are provided to represent the secret in different formats.
 * See {@link HexStringConfidentialKey} and {@link HMACConfidentialKey} for example. In addition to the programming
 * ease, these use case specific subtypes make it harder for vulnerability to creep in by making it harder
 * for the secret to leak.
 *
 * <p>
 * The {@link ConfidentialKey} subtypes are expected to be used as a singleton, like {@link JnlpSlaveAgentProtocol#SLAVE_SECRET}.
 * For code that relies on XStream for persistence (such as {@link Builder}s, {@link SCM}s, and other fragment objects
 * around builds and jobs), {@link Secret} provides more convenient way of storing secrets.
 *
 * @author Kohsuke Kawaguchi
 * @see Secret
 * @since 1.498
 */
public abstract class ConfidentialKey {
    /**
     * Name of the key. This is used as the file name.
     */
    private final String id;

    protected ConfidentialKey(String id) {
        this.id = id;
    }

    protected @CheckForNull byte[] load() throws IOException {
        return ConfidentialStore.get().load(this);
    }

    protected void store(byte[] payload) throws IOException {
        ConfidentialStore.get().store(this,payload);
    }

    public String getId() {
        return id;
    }
}
package jenkins.security;

import hudson.Extension;
import jenkins.model.Jenkins;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import java.io.IOException;
import java.security.SecureRandom;

/**
 * The actual storage for the data held by {@link ConfidentialKey}s, and the holder
 * of the master secret.
 *
 * <p>
 * This class is only relevant for the implementers of {@link ConfidentialKey}s.
 * Most plugin code should interact with {@link ConfidentialKey}s.
 *
 * <p>
 * OEM distributions of Jenkins can provide a custom {@link ConfidentialStore} implementation
 * by writing a subclass, mark it with {@link Extension} annotation, package it as a Jenkins module,
 * and bundling it with the war file.
 *
 * @author Kohsuke Kawaguchi
 * @since 1.498
 */
public abstract class ConfidentialStore {
    /**
     * Persists the payload of {@link ConfidentialKey} to a persisted storage (such as disk.)
     * The expectation is that the persisted form is secure.
     */
    protected abstract void store(ConfidentialKey key, byte[] payload) throws IOException;

    /**
     * Reverse operation of {@link #store(ConfidentialKey, byte[])}
     *
     * @return
     *      null the data has not been previously persisted, or if the data was tampered.
     */
    protected abstract @CheckForNull byte[] load(ConfidentialKey key) throws IOException;

    /**
     * Works like {@link SecureRandom#nextBytes(byte[])}.
     *
     * This enables implementations to consult other entropy sources, if it's available.
     */
    public abstract byte[] randomBytes(int size);

    /**
     * Retrieves the currently active singleton instance of {@link ConfidentialStore}.
     */
    public static @Nonnull ConfidentialStore get() {
        if (TEST!=null) return TEST.get();
        return Jenkins.getInstance().getExtensionList(ConfidentialStore.class).get(0);
    }

    /**
     * Testing only. Used for testing {@link ConfidentialKey} without {@link Jenkins}
     */
    /*package*/ static ThreadLocal<ConfidentialStore> TEST = null;
}
package jenkins.security;

import hudson.util.Secret;

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;
import java.io.IOException;
import java.security.GeneralSecurityException;

/**
 * {@link ConfidentialKey} that stores a {@link SecretKey} for shared-secret cryptography (AES).
 *
 * @author Kohsuke Kawaguchi
 * @since 1.498
 */
public class CryptoConfidentialKey extends ConfidentialKey {
    private volatile SecretKey secret;
    public CryptoConfidentialKey(String id) {
        super(id);
    }

    public CryptoConfidentialKey(Class owner, String shortName) {
        this(owner.getName()+'.'+shortName);
    }

    private SecretKey getKey() {
        try {
            if (secret==null) {
                synchronized (this) {
                    if (secret==null) {
                        byte[] payload = load();
                        if (payload==null) {
                            payload = ConfidentialStore.get().randomBytes(256);
                            store(payload);
                        }
                        // Due to the stupid US export restriction JDK only ships 128bit version.
                        secret = new SecretKeySpec(payload,0,128/8, ALGORITHM);
                    }
                }
            }
            return secret;
        } catch (IOException e) {
            throw new Error("Failed to load the key: "+getId(),e);
        }
    }

    /**
     * Returns a {@link Cipher} object for encrypting with this key.
     */
    public Cipher encrypt() {
        try {
            Cipher cipher = Secret.getCipher(ALGORITHM);
            cipher.init(Cipher.ENCRYPT_MODE, getKey());
            return cipher;
        } catch (GeneralSecurityException e) {
            throw new AssertionError(e);
        }
    }

    /**
     * Returns a {@link Cipher} object for decrypting with this key.
     */
    public Cipher decrypt() {
        try {
            Cipher cipher = Secret.getCipher(ALGORITHM);
            cipher.init(Cipher.DECRYPT_MODE, getKey());
            return cipher;
        } catch (GeneralSecurityException e) {
            throw new AssertionError(e);
        }
    }


    private static final String ALGORITHM = "AES";
}
package jenkins.security;

import hudson.Extension;
import hudson.FilePath;
import hudson.Util;
import hudson.util.IOException2;
import hudson.util.IOUtils;
import hudson.util.Secret;
import hudson.util.TextFile;
import jenkins.model.Jenkins;

import javax.crypto.Cipher;
import javax.crypto.CipherInputStream;
import javax.crypto.CipherOutputStream;
import javax.crypto.SecretKey;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.security.GeneralSecurityException;
import java.security.SecureRandom;

/**
 * Default portable implementation of {@link ConfidentialStore} that uses
 * a directory inside $JENKINS_HOME.
 *
 * The master key is also stored in this same directory.
 *
 * @author Kohsuke Kawaguchi
 */
@Extension(ordinal=-99999) // small ordinal value to allow other higher ones to take over
public class DefaultConfidentialStore extends ConfidentialStore {
    private final SecureRandom sr = new SecureRandom();

    /**
     * Directory that stores individual keys.
     */
    private final File rootDir;

    /**
     * The master key.
     *
     * The sole purpose of the master key is to encrypt individual keys on the disk.
     * Because leaking this master key compromises all the individual keys, we must not let
     * this master key used for any other purpose, hence the protected access.
     */
    private final SecretKey masterKey;

    public DefaultConfidentialStore() throws IOException, InterruptedException {
        this(new File(Jenkins.getInstance().getRootDir(),"secrets"));
    }

    public DefaultConfidentialStore(File rootDir) throws IOException, InterruptedException {
        this.rootDir = rootDir;
        if (rootDir.mkdirs()) {
            // protect this directory. but don't change the permission of the existing directory
            // in case the administrator changed this.
            new FilePath(rootDir).chmod(0700);
        }

        TextFile masterSecret = new TextFile(new File(rootDir,"master.key"));
        if (!masterSecret.exists()) {
            // we are only going to use small number of bits (since export control limits AES key length)
            // but let's generate a long enough key anyway
            masterSecret.write(Util.toHexString(randomBytes(128)));
        }
        this.masterKey = Util.toAes128Key(masterSecret.readTrim());
    }

    /**
     * Persists the payload of {@link ConfidentialKey} to the disk.
     */
    @Override
    protected void store(ConfidentialKey key, byte[] payload) throws IOException {
        CipherOutputStream cos=null;
        FileOutputStream fos=null;
        try {
            Cipher sym = Secret.getCipher("AES");
            sym.init(Cipher.ENCRYPT_MODE, masterKey);
            cos = new CipherOutputStream(fos=new FileOutputStream(getFileFor(key)), sym);
            cos.write(payload);
            cos.write(MAGIC);
        } catch (GeneralSecurityException e) {
            throw new IOException2("Failed to persist the key: "+key.getId(),e);
        } finally {
            IOUtils.closeQuietly(cos);
            IOUtils.closeQuietly(fos);
        }
    }

    /**
     * Reverse operation of {@link #store(ConfidentialKey, byte[])}
     *
     * @return
     *      null the data has not been previously persisted.
     */
    @Override
    protected byte[] load(ConfidentialKey key) throws IOException {
        CipherInputStream cis=null;
        FileInputStream fis=null;
        try {
            File f = getFileFor(key);
            if (!f.exists())    return null;

            Cipher sym = Secret.getCipher("AES");
            sym.init(Cipher.DECRYPT_MODE, masterKey);
            cis = new CipherInputStream(fis=new FileInputStream(f), sym);
            byte[] bytes = IOUtils.toByteArray(cis);
            return verifyMagic(bytes);
        } catch (GeneralSecurityException e) {
            throw new IOException2("Failed to persist the key: "+key.getId(),e);
        } finally {
            IOUtils.closeQuietly(cis);
            IOUtils.closeQuietly(fis);
        }
    }

    /**
     * Verifies that the given byte[] has the MAGIC trailer, to verify the integrity of the decryption process.
     */
    private byte[] verifyMagic(byte[] payload) {
        int payloadLen = payload.length-MAGIC.length;
        if (payloadLen<0)   return null;    // obviously broken

        for (int i=0; i<MAGIC.length; i++) {
            if (payload[payloadLen+i]!=MAGIC[i])
                return null;    // broken
        }
        byte[] truncated = new byte[payloadLen];
        System.arraycopy(payload,0,truncated,0,truncated.length);
        return truncated;
    }

    private File getFileFor(ConfidentialKey key) {
        return new File(rootDir, key.getId());
    }

    public byte[] randomBytes(int size) {
        byte[] random = new byte[size];
        sr.nextBytes(random);
        return random;
    }

    private static final byte[] MAGIC = "::::MAGIC::::".getBytes();
}
package jenkins.security;

import hudson.Util;

import javax.crypto.KeyGenerator;
import javax.crypto.Mac;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.security.GeneralSecurityException;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;

/**
 * {@link ConfidentialKey} that's used for creating a token by hashing some information with secret
 * (such as <tt>hash(msg|secret)</tt>).
 *
 * <p>
 * This provides more secure version of it by using HMAC.
 * See http://rdist.root.org/2009/10/29/stop-using-unsafe-keyed-hashes-use-hmac/ for background.
 * This implementation also never leaks the secret value to outside, so it makes it impossible
 * for the careless caller to misuse the key (thus protecting ourselves from our own stupidity!)
 *
 * @author Kohsuke Kawaguchi
 * @since 1.498
 */
public class HMACConfidentialKey extends ConfidentialKey {
    private volatile SecretKey key;
    private final int length;

    /**
     * @param length
     *      Byte length of the HMAC code.
     *      By default we use HMAC-SHA256, which produces 256bit (=32bytes) HMAC,
     *      but if different use cases requires a shorter HMAC, specify the desired length here.
     *      Note that when using {@link #mac(String)}, string encoding causes the length to double.
     *      So if you want to get 16-letter HMAC, you specify 8 here.
     */
    public HMACConfidentialKey(String id, int length) {
        super(id);
        this.length = length;
    }

    /**
     * Calls into {@link #HMACConfidentialKey(String, int)} with the longest possible HMAC length.
     */
    public HMACConfidentialKey(String id) {
        this(id,Integer.MAX_VALUE);
    }

    /**
     * Calls into {@link #HMACConfidentialKey(String, int)} by combining the class name and the shortName
     * as the ID.
     */
    public HMACConfidentialKey(Class owner, String shortName, int length) {
        this(owner.getName()+'.'+shortName,length);
    }

    public HMACConfidentialKey(Class owner, String shortName) {
        this(owner,shortName,Integer.MAX_VALUE);
    }


    /**
     * Computes the message authentication code for the specified byte sequence.
     */
    public byte[] mac(byte[] message) {
        return chop(createMac().doFinal(message));
    }

    /**
     * Convenience method for verifying the MAC code.
     */
    public boolean checkMac(byte[] message, byte[] mac) {
        return Arrays.equals(mac(message),mac);
    }

    /**
     * Computes the message authentication code and return it as a string.
     * While redundant, often convenient.
     */
    public String mac(String message) {
        try {
            return Util.toHexString(mac(message.getBytes("UTF-8")));
        } catch (UnsupportedEncodingException e) {
            throw new AssertionError(e);
        }
    }

    /**
     * Verifies MAC constructed from {@link #mac(String)}
     */
    public boolean checkMac(String message, String mac) {
        return mac(message).equals(mac);
    }

    private byte[] chop(byte[] mac) {
        if (mac.length<=length)  return mac; // already too short

        byte[] b = new byte[length];
        System.arraycopy(mac,0,b,0,b.length);
        return b;
    }

    /**
     * Creates a new {@link Mac} object.
     */
    public Mac createMac() {
        try {
            Mac mac = Mac.getInstance(ALGORITHM);
            mac.init(getKey());
            return mac;
        } catch (GeneralSecurityException e) {
            // Javadoc says HmacSHA256 must be supported by every Java implementation.
            throw new Error(ALGORITHM+" not supported?",e);
        }
    }

    private SecretKey getKey() {
        if (key==null) {
            synchronized (this) {
                if (key==null) {
                    try {
                        byte[] encoded = load();
                        if (encoded==null) {
                            KeyGenerator kg = KeyGenerator.getInstance(ALGORITHM);
                            SecretKey key = kg.generateKey();
                            store(encoded=key.getEncoded());
                        }
                        key = new SecretKeySpec(encoded,ALGORITHM);
                    } catch (IOException e) {
                        throw new Error("Failed to load the key: "+getId(),e);
                    } catch (NoSuchAlgorithmException e) {
                        throw new Error("Failed to load the key: "+getId(),e);
                    }
                }
            }
        }
        return key;
    }

    private static final String ALGORITHM = "HmacSHA256";
}
package jenkins.security;

import hudson.Util;

import java.io.IOException;

/**
 * {@link ConfidentialKey} that is the random hexadecimal string of length N.
 *
 * <p>
 * This is typically used as a unique ID, as a hex dump is suitable for printing, copy-pasting,
 * as well as use as an identifier.
 *
 * @author Kohsuke Kawaguchi
 * @since 1.498
 */
public class HexStringConfidentialKey extends ConfidentialKey {
    private final int length;

    private volatile String secret;

    /**
     * @param length
     *      Length of the hexadecimal string.
     */
    public HexStringConfidentialKey(String id, int length) {
        super(id);
        if (length%2!=0)
            throw new IllegalArgumentException("length must be even: "+length);
        this.length = length;
    }

    public HexStringConfidentialKey(Class owner, String shortName, int length) {
        this(owner.getName()+'.'+shortName,length);
    }

    /**
     * Returns the persisted hex string value.
     *
     * If the value isn't persisted, a new random value is created.
     *
     * @throws Error
     *      If the secret fails to load. Not throwing a checked exception is for the convenience
     *      of the caller.
     */
    public String get() {
        try {
            if (secret==null) {
                synchronized (this) {
                    if (secret==null) {
                        byte[] payload = load();
                        if (payload==null) {
                            payload = ConfidentialStore.get().randomBytes(length/2);
                            store(payload);
                        }
                        secret = Util.toHexString(payload).substring(0,length);
                    }
                }
            }
            return secret;
        } catch (IOException e) {
            throw new Error("Failed to load the key: "+getId(),e);
        }
    }
}
import jenkins.security.HMACConfidentialKey;
 * We do this by computing HMAC of the slave name.
 * This code is sent to the slave inside the <tt>.jnlp</tt> file
 * we use everywhere else in Jenkins), and the slave sends this
            final String secret = in.readUTF();
            final String nodeName = in.readUTF();

            if(!SLAVE_SECRET.mac(nodeName).equals(secret)) {


    /**
     * This secret value is used as a seed for slaves.
     */
    public static final HMACConfidentialKey SLAVE_SECRET = new HMACConfidentialKey(JnlpSlaveAgentProtocol.class,"secret");
            final String nodeName = request.getProperty("Node-Name");

            if(!SLAVE_SECRET.mac(nodeName).equals(request.getProperty("Secret-Key"))) {
  <j:getStatic var="connect" className="hudson.model.Computer" field="CONNECT"/>
  ${it.checkPermission(connect)}

        <argument>${it.jnlpMac}</argument>
/*
 * The MIT License
 * 
 * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
 * 
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 * 
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 * 
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 * THE SOFTWARE.
 */
package hudson.util

import com.trilead.ssh2.crypto.Base64;
import jenkins.model.Jenkins
import jenkins.security.ConfidentialStoreRule;
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test

import javax.crypto.Cipher;
import java.security.SecureRandom;

import hudson.Util;

/**
 * @author Kohsuke Kawaguchi
 */
public class SecretTest {
    @Rule
    public ConfidentialStoreRule confidentialStore = new ConfidentialStoreRule()

    @Test @Before
    void setUp() {
        def sr = new SecureRandom();
        byte[] random = new byte[32];
        sr.nextBytes(random);
        Secret.SECRET = Util.toHexString(random);
    }

    @Test @After
    void tearDown() {
        Secret.SECRET = null;
    }

    @Test
    void testEncrypt() {
        def secret = Secret.fromString("abc");
        assert "abc"==secret.plainText;

        // make sure we got some encryption going
        println secret.encryptedValue;
        assert !"abc".equals(secret.encryptedValue);

        // can we round trip?
        assert secret==Secret.fromString(secret.encryptedValue);
    }

    @Test
    void testDecrypt() {
        assert "abc"==Secret.toString(Secret.fromString("abc"))
    }

    @Test
    void testSerialization() {
        def s = Secret.fromString("Mr.Jenkins");
        def xml = Jenkins.XSTREAM.toXML(s);
        assert !xml.contains(s.plainText)
        assert xml.contains(s.encryptedValue)

        def o = Jenkins.XSTREAM.fromXML(xml);
        assert o==s : xml;
    }

    public static class Foo {
        Secret password;
    }

    /**
     * Makes sure the serialization form is backward compatible with String.
     */
    @Test
    void testCompatibilityFromString() {
        def tagName = Foo.class.name.replace("\$","_-");
        def xml = "<$tagName><password>secret</password></$tagName>";
        def foo = new Foo();
        Jenkins.XSTREAM.fromXML(xml, foo);
        assert "secret"==Secret.toString(foo.password)
    }

    /**
     * Secret persisted with Jenkins.getSecretKey() should still decrypt OK.
     */
    @Test
    void migrationFromLegacyKeyToConfidentialStore() {
        def legacy = Secret.legacyKey
        ["Hello world","","\u0000unprintable"].each { str ->
            def cipher = Secret.getCipher("AES");
            cipher.init(Cipher.ENCRYPT_MODE, legacy);
            def old = new String(Base64.encode(cipher.doFinal((str + Secret.MAGIC).getBytes("UTF-8"))))
            def s = Secret.fromString(old)
            assert s.plainText==str : "secret by the old key should decrypt"
            assert s.encryptedValue!=old : "but when encrypting, ConfidentialKey should be in use"
        }
    }
}
package jenkins.security;

import hudson.Util;
import org.junit.rules.ExternalResource;

import java.io.File;
import java.io.IOException;

/**
 * Test rule that injects a temporary {@link DefaultConfidentialStore}
 * @author Kohsuke Kawaguchi
 */
public class ConfidentialStoreRule extends ExternalResource {
    public ConfidentialStore store;
    public File tmp;

    @Override
    protected void before() throws Throwable {
        tmp = Util.createTempDir();
        store = new DefaultConfidentialStore(tmp);
        ConfidentialStore.TEST.set(store);
    }

    @Override
    protected void after() {
        ConfidentialStore.TEST.set(null);
        try {
            Util.deleteRecursive(tmp);
        } catch (IOException e) {
            throw new Error(e);
        }
    }

    static {
        ConfidentialStore.TEST = new ThreadLocal<ConfidentialStore>();
    }
}
package jenkins.security

import org.junit.Rule
import org.junit.Test

/**
 *
 *
 * @author Kohsuke Kawaguchi
 */
class CryptoConfidentialKeyTest {
    @Rule
    public ConfidentialStoreRule store = new ConfidentialStoreRule()

    def key = new CryptoConfidentialKey("test")

    @Test
    void decryptGetsPlainTextBack() {
        ["Hello world","","\u0000"].each { str ->
            assert key.decrypt().doFinal(key.encrypt().doFinal(str.bytes))==str.bytes
        }
    }

    @Test
    void multipleEncryptsAreIdempotent() {
        def str = "Hello world".bytes
        assert key.encrypt().doFinal(str)==key.encrypt().doFinal(str)
    }

    @Test
    void loadingExistingKey() {
        def key2 = new CryptoConfidentialKey("test") // this will cause the key to be loaded from the disk
        ["Hello world","","\u0000"].each { str ->
            assert key2.decrypt().doFinal(key.encrypt().doFinal(str.bytes))==str.bytes
        }
    }
}
package jenkins.security

import hudson.FilePath
import hudson.Functions
import hudson.Util
import org.junit.After
import org.junit.Before
import org.junit.Test

/**
 * @author Kohsuke Kawaguchi
 */
public class DefaultConfidentialStoreTest {

    def tmp;

    @Before
    void setup() {
        tmp = Util.createTempDir()
    }

    @After
    void tearDown() {
        Util.deleteRecursive(tmp)
    }

    @Test
    void roundtrip() {
        tmp.deleteDir()   // let ConfidentialStore create a directory

        def store = new DefaultConfidentialStore(tmp);
        def key = new ConfidentialKey("test") {};

        // basic roundtrip
        def str = "Hello world!"
        store.store(key, str.bytes)
        assert new String(store.load(key))==str

        // data storage should have some stuff
        assert new File(tmp,"test").exists()
        assert new File(tmp,"master.key").exists()

        assert !new File(tmp,"test").text.contains("Hello") // the data shouldn't be a plain text, obviously

        if (!Functions.isWindows())
            assert (new FilePath(tmp).mode()&0777) == 0700 // should be read only

        // if the master key changes, we should gracefully fail to load the store
        new File(tmp,"master.key").delete()
        def store2 = new DefaultConfidentialStore(tmp)
        assert new File(tmp,"master.key").exists()  // we should have a new key now
        assert store2.load(key)==null;
    }
}
package jenkins.security

import org.junit.Rule
import org.junit.Test

/**
 *
 *
 * @author Kohsuke Kawaguchi
 */
class HMACConfidentialKeyTest {
    @Rule
    public ConfidentialStoreRule store = new ConfidentialStoreRule()

    def key = new HMACConfidentialKey("test",16)

    @Test
    void basics() {
        def unique = [] as TreeSet
        ["Hello world","","\u0000"].each { str ->
            def mac = key.mac(str)
            unique.add(mac)
            assert mac =~ /[0-9A-Fa-f]{32}/
            assert key.checkMac(str,mac)
            assert !key.checkMac("garbage",mac)
        }

        assert unique.size()==3 // make sure all 3 MAC are different
    }

    @Test
    void loadingExistingKey() {
        // this second key of the same ID will cause it to load the key from the disk
        def key2 = new HMACConfidentialKey("test",16)
        ["Hello world","","\u0000"].each { str ->
            assert key.mac(str)==key2.mac(str)
        }
    }
}
package jenkins.security

import org.junit.Rule
import org.junit.Test

/**
 *
 *
 * @author Kohsuke Kawaguchi
 */
class HexStringConfidentialKeyTest {
    @Rule
    public ConfidentialStoreRule store = new ConfidentialStoreRule()

    @Test
    void hexStringShouldProduceHexString() {
        def key = new HexStringConfidentialKey("test",8)
        assert key.get() =~ /[A-Fa-f0-9]{8}/
    }

    @Test
    void multipleGetsAreIdempotent() {
        def key = new HexStringConfidentialKey("test",8)
        assert key.get()==key.get()
    }

    @Test
    void specifyLengthAndMakeSureItTakesEffect() {
        [8,16,32,256].each { n ->
            new HexStringConfidentialKey("test"+n,n).get().length()==n
        }
    }

    @Test
    void loadingExistingKey() {
        def key1 = new HexStringConfidentialKey("test",8)
        key1.get() // this causes the ke to be generated

        // this second key of the same ID will cause it to load the key from the disk
        def key2 = new HexStringConfidentialKey("test",8)
        assert key1.get()==key2.get()
    }
}
          <version>1.11</version>
import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import java.net.HttpURLConnection;
    @PresetData(DataSet.ANONYMOUS_READONLY)


        try {
            jnlp = (XmlPage) jnlpAgent.goTo("computer/test/slave-agent.jnlp", "application/x-java-jnlp-file");
            fail("anonymous users must not be able to get secrets");
        } catch (FailingHttpStatusCodeException x) {
            assertEquals(HttpURLConnection.HTTP_FORBIDDEN, x.getStatusCode());
        }