CVE-2017-2598: Inadequate Encryption Strength (CWE-326)
value: Jenkins before versions 2.44, 2.32.2 uses AES ECB block cipher mode without IV for encrypting secrets which makes Jenkins and the stored secrets vulnerable to unnecessary risks (SECURITY-304).
CWE description: The product stores or transmits sensitive data using an encryption scheme that is theoretically sound, but is not strong enough for the level of protection required.
CVSS v3 base score: 4.3
Published: 2018-05-23T13:29Z
Repository: https://github.com/jenkinsci/jenkins
Language: Java
Files changed: core/src/main/java/hudson/model/Item.java, core/src/main/java/hudson/util/HistoricalSecrets.java, core/src/main/java/hudson/util/Secret.java, core/src/main/java/hudson/util/SecretRewriter.java, core/src/main/java/jenkins/security/CryptoConfidentialKey.java, core/src/test/groovy/hudson/util/SecretRewriterTest.groovy, core/src/test/groovy/hudson/util/SecretTest.groovy, test/src/test/java/hudson/util/SecretCompatTest.java, test/src/test/java/jenkins/security/RekeySecretAdminMonitorTest.java, test/src/test/java/lib/form/PasswordTest.java, test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/config.xml, test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/jobs/OldSecret/config.xml, test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/master.key
Commit date: 2017-01-23 12:55:54 -0500
Commit message: Merge pull request #105 from jenkinsci-cert/SECURITY-304-t3

[SECURITY-304] Encrypt new secrets with CBC and random IV instead of ECB
Diff: diff --git a/core/src/main/java/hudson/model/Item.java b/core/src/main/java/hudson/model/Item.java
index 834cf97b485..7c2aeba5e29 100644
--- a/core/src/main/java/hudson/model/Item.java
+++ b/core/src/main/java/hudson/model/Item.java
@@ -227,11 +227,11 @@ public interface Item extends PersistenceRoot, SearchableModelObject, AccessCont
     Permission CONFIGURE = new Permission(PERMISSIONS, "Configure", Messages._Item_CONFIGURE_description(), Permission.CONFIGURE, PermissionScope.ITEM);
     Permission READ = new Permission(PERMISSIONS, "Read", Messages._Item_READ_description(), Permission.READ, PermissionScope.ITEM);
     Permission DISCOVER = new Permission(PERMISSIONS, "Discover", Messages._AbstractProject_DiscoverPermission_Description(), READ, PermissionScope.ITEM);
     /**
      * Ability to view configuration details.
-     * If the user lacks {@link CONFIGURE} then any {@link Secret}s must be masked out, even in encrypted form.
+     * If the user lacks {@link #CONFIGURE} then any {@link Secret}s must be masked out, even in encrypted form.
      * @see Secret#ENCRYPTED_VALUE_PATTERN
      */
     Permission EXTENDED_READ = new Permission(PERMISSIONS,"ExtendedRead", Messages._AbstractProject_ExtendedReadPermission_Description(), CONFIGURE, Boolean.getBoolean("hudson.security.ExtendedReadPermission"), new PermissionScope[]{PermissionScope.ITEM});
     // TODO the following really belong in Job, not Item, but too late to move since the owner.name is encoded in the ID:
     Permission BUILD = new Permission(PERMISSIONS, "Build", Messages._AbstractProject_BuildPermission_Description(),  Permission.UPDATE, PermissionScope.ITEM);
diff --git a/core/src/main/java/hudson/util/HistoricalSecrets.java b/core/src/main/java/hudson/util/HistoricalSecrets.java
new file mode 100644
index 00000000000..37a6fa39e17
--- /dev/null
+++ b/core/src/main/java/hudson/util/HistoricalSecrets.java
@@ -0,0 +1,84 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
+ * Copyright (c) 2016, CloudBees Inc.
+ *
+ * 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 hudson.Util;
+import jenkins.model.Jenkins;
+import jenkins.security.CryptoConfidentialKey;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Historical algorithms for decrypting {@link Secret}s.
+ */
+@Restricted(NoExternalUse.class)
+public class HistoricalSecrets {
+
+    /*package*/ static Secret decrypt(String data, CryptoConfidentialKey key) throws IOException, GeneralSecurityException {
+        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 = Secret.getCipher("AES");
+        cipher.init(Cipher.DECRYPT_MODE, getLegacyKey());
+        return tryDecrypt(cipher, in);
+    }
+
+    /*package*/ static Secret tryDecrypt(Cipher cipher, byte[] in) {
+        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; // if the key doesn't match with the bytes, it can result in BadPaddingException
+        }
+    }
+
+    /**
+     * 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.
+     */
+    @Deprecated
+    /*package*/ static SecretKey getLegacyKey() throws GeneralSecurityException {
+        String secret = Secret.SECRET;
+        if(secret==null)    return Jenkins.getInstance().getSecretKeyAsAES128();
+        return Util.toAes128Key(secret);
+    }
+
+    private static final String MAGIC = "::::MAGIC::::";
+}
diff --git a/core/src/main/java/hudson/util/Secret.java b/core/src/main/java/hudson/util/Secret.java
index 9a4dcb750a0..0ea02d7d471 100644
--- a/core/src/main/java/hudson/util/Secret.java
+++ b/core/src/main/java/hudson/util/Secret.java
@@ -1,9 +1,10 @@
 /*
  * The MIT License
  * 
  * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
+ * Copyright (c) 2016, CloudBees Inc.
  * 
  * 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
@@ -27,25 +28,27 @@ import com.thoughtworks.xstream.converters.Converter;
 import com.thoughtworks.xstream.converters.MarshallingContext;
 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 java.util.Arrays;
 import jenkins.model.Jenkins;
 import hudson.Util;
 import jenkins.security.CryptoConfidentialKey;
 import org.kohsuke.stapler.Stapler;
 
-import javax.crypto.SecretKey;
 import javax.crypto.Cipher;
 import java.io.Serializable;
 import java.io.UnsupportedEncodingException;
 import java.io.IOException;
 import java.security.GeneralSecurityException;
 import java.util.regex.Pattern;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  * Glorified {@link String} that uses encryption in the persisted form, to avoid accidental exposure of a secret.
  *
  * <p>
  * This is not meant as a protection against code running in the same VM, nor against an attacker
@@ -56,17 +59,24 @@ import org.kohsuke.accmod.restrictions.NoExternalUse;
  * String field to be updated to {@link Secret}.
  *
  * @author Kohsuke Kawaguchi
  */
 public final class Secret implements Serializable {
+    private static final byte PAYLOAD_V1 = 1;
     /**
      * Unencrypted secret text.
      */
     private final String value;
+    private byte[] iv;
+
+    /*package*/ Secret(String value) {
+        this.value = value;
+    }
 
-    private Secret(String value) {
+    /*package*/ Secret(String value, byte[] iv) {
         this.value = value;
+        this.iv = iv;
     }
 
     /**
      * Obtains the secret in a plain text.
      *
@@ -98,81 +108,106 @@ public final class Secret implements Serializable {
     @Override
     public int hashCode() {
         return value.hashCode();
     }
 
-    /**
-     * 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.
-     */
-    @Deprecated
-    /*package*/ static SecretKey getLegacyKey() throws GeneralSecurityException {
-        String secret = SECRET;
-        if(secret==null)    return Jenkins.getInstance().getSecretKeyAsAES128();
-        return Util.toAes128Key(secret);
-    }
-
     /**
      * Encrypts {@link #value} and returns it in an encoded printable form.
      *
      * @see #toString() 
      */
     public String getEncryptedValue() {
         try {
-            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"))));
+            synchronized (this) {
+                if (iv == null) { //if we were created from plain text or other reason without iv
+                    iv = KEY.newIv();
+                }
+            }
+            Cipher cipher = KEY.encrypt(iv);
+            byte[] encrypted = cipher.doFinal(this.value.getBytes(UTF_8));
+            byte[] payload = new byte[1 + 8 + iv.length + encrypted.length];
+            int pos = 0;
+            // For PAYLOAD_V1 we use this byte shifting model, V2 probably will need DataOutput
+            payload[pos++] = PAYLOAD_V1;
+            payload[pos++] = (byte)(iv.length >> 24);
+            payload[pos++] = (byte)(iv.length >> 16);
+            payload[pos++] = (byte)(iv.length >> 8);
+            payload[pos++] = (byte)(iv.length);
+            payload[pos++] = (byte)(encrypted.length >> 24);
+            payload[pos++] = (byte)(encrypted.length >> 16);
+            payload[pos++] = (byte)(encrypted.length >> 8);
+            payload[pos++] = (byte)(encrypted.length);
+            System.arraycopy(iv, 0, payload, pos, iv.length);
+            pos+=iv.length;
+            System.arraycopy(encrypted, 0, payload, pos, encrypted.length);
+            return "{"+new String(Base64.encode(payload))+"}";
         } catch (GeneralSecurityException e) {
             throw new Error(e); // impossible
-        } catch (UnsupportedEncodingException e) {
-            throw new Error(e); // impossible
         }
     }
 
     /**
-     * Pattern matching a possible output of {@link #getEncryptedValue}.
-     * Basically, any Base64-encoded value.
-     * You must then call {@link #decrypt} to eliminate false positives.
+     * Pattern matching a possible output of {@link #getEncryptedValue}
+     * Basically, any Base64-encoded value optionally wrapped by {@code {}}.
+     * You must then call {@link #decrypt(String)} to eliminate false positives.
+     * @see #ENCRYPTED_VALUE_PATTERN
      */
     @Restricted(NoExternalUse.class)
-    public static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("[A-Za-z0-9+/]+={0,2}");
+    public static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("\\{?[A-Za-z0-9+/]+={0,2}}?");
 
     /**
      * Reverse operation of {@link #getEncryptedValue()}. Returns null
      * 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;
+        if (data == null) return null;
 
-            // try our historical key for backward compatibility
-            Cipher cipher = getCipher("AES");
-            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;
-        }
-    }
-
-    /*package*/ 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; // if the key doesn't match with the bytes, it can result in BadPaddingException
+        if (data.startsWith("{") && data.endsWith("}")) { //likely CBC encrypted/containing metadata but could be plain text
+            byte[] payload;
+            try {
+                payload = Base64.decode(data.substring(1, data.length()-1).toCharArray());
+            } catch (IOException e) {
+                return null;
+            }
+            switch (payload[0]) {
+                case PAYLOAD_V1:
+                    // For PAYLOAD_V1 we use this byte shifting model, V2 probably will need DataOutput
+                    int ivLength = ((payload[1] & 0xff) << 24)
+                            | ((payload[2] & 0xff) << 16)
+                            | ((payload[3] & 0xff) << 8)
+                            | (payload[4] & 0xff);
+                    int dataLength = ((payload[5] & 0xff) << 24)
+                            | ((payload[6] & 0xff) << 16)
+                            | ((payload[7] & 0xff) << 8)
+                            | (payload[8] & 0xff);
+                    if (payload.length != 1 + 8 + ivLength + dataLength) {
+                        // not valid v1
+                        return null;
+                    }
+                    byte[] iv = Arrays.copyOfRange(payload, 9, 9 + ivLength);
+                    byte[] code = Arrays.copyOfRange(payload, 9+ivLength, payload.length);
+                    String text;
+                    try {
+                        text = new String(KEY.decrypt(iv).doFinal(code), UTF_8);
+                    } catch (GeneralSecurityException e) {
+                        // it's v1 which cannot be historical, but not decrypting
+                        return null;
+                    }
+                    return new Secret(text, iv);
+                default:
+                    return null;
+            }
+        } else {
+            try {
+                return HistoricalSecrets.decrypt(data, KEY);
+            } catch (GeneralSecurityException e) {
+                return null;
+            } catch (UnsupportedEncodingException e) {
+                throw new Error(e); // impossible
+            } catch (IOException e) {
+                return null;
+            }
         }
     }
 
     /**
      * Workaround for JENKINS-6459 / http://java.net/jira/browse/GLASSFISH-11862
@@ -226,12 +261,10 @@ public final class Secret implements Serializable {
         public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingContext context) {
             return fromString(reader.getValue());
         }
     }
 
-    private static final String MAGIC = "::::MAGIC::::";
-
     /**
      * Workaround for JENKINS-6459 / http://java.net/jira/browse/GLASSFISH-11862
      * @see #getCipher(String)
      */
     private static final String PROVIDER = System.getProperty(Secret.class.getName()+".provider");
@@ -244,10 +277,18 @@ public final class Secret implements Serializable {
     /**
      * The key that encrypts the data on disk.
      */
     private static final CryptoConfidentialKey KEY = new CryptoConfidentialKey(Secret.class.getName());
 
+    /**
+     * Reset the internal secret key for testing.
+     */
+    @Restricted(NoExternalUse.class)
+    /*package*/ static void resetKeyForTest() {
+        KEY.resetForTest();
+    }
+
     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/hudson/util/SecretRewriter.java b/core/src/main/java/hudson/util/SecretRewriter.java
index 6350adf0f3a..8b8a574e6ad 100644
--- a/core/src/main/java/hudson/util/SecretRewriter.java
+++ b/core/src/main/java/hudson/util/SecretRewriter.java
@@ -38,11 +38,11 @@ public class SecretRewriter {
      */
     private Set<String> callstack = new HashSet<String>();
 
     public SecretRewriter() throws GeneralSecurityException {
         cipher = Secret.getCipher("AES");
-        key = Secret.getLegacyKey();
+        key = HistoricalSecrets.getLegacyKey();
     }
 
     /** @deprecated SECURITY-376: {@code backupDirectory} is ignored */
     @Deprecated
     public SecretRewriter(File backupDirectory) throws GeneralSecurityException {
@@ -60,11 +60,11 @@ public class SecretRewriter {
             in = Base64.decode(s.toCharArray());
         } catch (IOException e) {
             return s;   // not a valid base64
         }
         cipher.init(Cipher.DECRYPT_MODE, key);
-        Secret sec = Secret.tryDecrypt(cipher, in);
+        Secret sec = HistoricalSecrets.tryDecrypt(cipher, in);
         if(sec!=null) // matched
             return sec.getEncryptedValue(); // replace by the new encrypted value
         else // not encrypted with the legacy key. leave it unmodified
             return s;
     }
diff --git a/core/src/main/java/jenkins/security/CryptoConfidentialKey.java b/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
index dd1dad9e37b..f4027060131 100644
--- a/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
+++ b/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
@@ -1,11 +1,17 @@
 package jenkins.security;
 
+import hudson.Main;
 import hudson.util.Secret;
+import jenkins.model.Jenkins;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 import javax.crypto.Cipher;
 import javax.crypto.SecretKey;
+import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 import java.io.IOException;
 import java.security.GeneralSecurityException;
 
 /**
@@ -13,10 +19,13 @@ import java.security.GeneralSecurityException;
  *
  * @author Kohsuke Kawaguchi
  * @since 1.498
  */
 public class CryptoConfidentialKey extends ConfidentialKey {
+    @Restricted(NoExternalUse.class) //TODO remove when in mainline
+    public static final int DEFAULT_IV_LENGTH = 16;
+
     private volatile SecretKey secret;
     public CryptoConfidentialKey(String id) {
         super(id);
     }
 
@@ -33,11 +42,11 @@ public class CryptoConfidentialKey extends ConfidentialKey {
                         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);
+                        secret = new SecretKeySpec(payload,0,128/8, KEY_ALGORITHM);
                     }
                 }
             }
             return secret;
         } catch (IOException e) {
@@ -45,32 +54,103 @@ public class CryptoConfidentialKey extends ConfidentialKey {
         }
     }
 
     /**
      * Returns a {@link Cipher} object for encrypting with this key.
+     * @deprecated use {@link #encrypt(byte[])}
      */
+    @Deprecated
     public Cipher encrypt() {
         try {
-            Cipher cipher = Secret.getCipher(ALGORITHM);
+            Cipher cipher = Secret.getCipher(KEY_ALGORITHM);
             cipher.init(Cipher.ENCRYPT_MODE, getKey());
             return cipher;
         } catch (GeneralSecurityException e) {
             throw new AssertionError(e);
         }
     }
 
+    /**
+     * Returns a {@link Cipher} object for encrypting with this key using the provided initialization vector.
+     * @param iv the initialization vector
+     * @return the cipher
+     */
+    @Restricted(NoExternalUse.class) //TODO remove when in mainline
+    public Cipher encrypt(byte[] iv) {
+        try {
+            Cipher cipher = Secret.getCipher(ALGORITHM);
+            cipher.init(Cipher.ENCRYPT_MODE, getKey(), new IvParameterSpec(iv));
+            return cipher;
+        } catch (GeneralSecurityException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    /**
+     * Returns a {@link Cipher} object for decrypting with this key using the provided initialization vector.
+     * @param iv the initialization vector
+     * @return the cipher
+     */
+    @Restricted(NoExternalUse.class) //TODO remove when in mainline
+    public Cipher decrypt(byte[] iv) {
+        try {
+            Cipher cipher = Secret.getCipher(ALGORITHM);
+            cipher.init(Cipher.DECRYPT_MODE, getKey(), new IvParameterSpec(iv));
+            return cipher;
+        } catch (GeneralSecurityException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    /**
+     * Generates a new Initialization Vector.
+     * @param length the length of the salt
+     * @return some random bytes
+     * @see #encrypt(byte[])
+     */
+    @Restricted(NoExternalUse.class) //TODO remove when in mainline
+    public byte[] newIv(int length) {
+        return ConfidentialStore.get().randomBytes(length);
+    }
+
+    /**
+     * Generates a new Initialization Vector of default length.
+     * @return some random bytes
+     * @see #newIv(int)
+     * @see #encrypt(byte[])
+     */
+    @Restricted(NoExternalUse.class) //TODO remove when in mainline
+    public byte[] newIv() {
+        return newIv(DEFAULT_IV_LENGTH);
+    }
+
     /**
      * Returns a {@link Cipher} object for decrypting with this key.
+     * @deprecated use {@link #decrypt(byte[])}
      */
+    @Deprecated
     public Cipher decrypt() {
         try {
-            Cipher cipher = Secret.getCipher(ALGORITHM);
+            Cipher cipher = Secret.getCipher(KEY_ALGORITHM);
             cipher.init(Cipher.DECRYPT_MODE, getKey());
             return cipher;
         } catch (GeneralSecurityException e) {
             throw new AssertionError(e);
         }
     }
 
 
-    private static final String ALGORITHM = "AES";
+    private static final String KEY_ALGORITHM = "AES";
+    private static final String ALGORITHM = "AES/CBC/PKCS5Padding";
+
+    /**
+     * Reset the internal secret key for testing.
+     */
+    @Restricted(NoExternalUse.class)
+    public void resetForTest() {
+        if (Main.isUnitTest) {
+            this.secret = null;
+        } else {
+            throw new IllegalStateException("Only for testing");
+        }
+    }
 }
diff --git a/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy b/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy
index f198bdd8a39..b748703ac17 100644
--- a/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy
+++ b/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy
@@ -21,46 +21,53 @@ class SecretRewriterTest {
     @Rule
     public ConfidentialStoreRule confidentialStoreRule = new ConfidentialStoreRule();
 
     @Rule public TemporaryFolder tmp = new TemporaryFolder()
 
+    def FOO_PATTERN = /<foo>\{[A-Za-z0-9+\/]+={0,2}}<\/foo>/
+    def MSG_PATTERN = /<msg>\{[A-Za-z0-9+\/]+={0,2}}<\/msg>/
+    def FOO_PATTERN2 = /(<foo>\{[A-Za-z0-9+\/]+={0,2}}<\/foo>){2}/
+    def ABC_FOO_PATTERN = /<abc>\s<foo>\{[A-Za-z0-9+\/]+={0,2}}<\/foo>\s<\/abc>/
+
     @Test
     void singleFileRewrite() {
         def o = encryptOld('foobar') // old
         def n = encryptNew('foobar') // new
         roundtrip "<foo>${o}</foo>",
-                  "<foo>${n}</foo>"
+                {assert it ==~ FOO_PATTERN}
+
 
         roundtrip "<foo>${o}</foo><foo>${o}</foo>",
-                  "<foo>${n}</foo><foo>${n}</foo>"
+                {assert it ==~ FOO_PATTERN2}
 
         roundtrip "<foo>${n}</foo>",
-                  "<foo>${n}</foo>"
+                {assert it == "<foo>${n}</foo>"}
 
         roundtrip "  <foo>thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret</foo>  ",
-                  "  <foo>thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret</foo>  "
+                {assert it == "<foo>thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret</foo>"}
 
         // to be rewritten, it needs to be between a tag
-        roundtrip "<foo>$o", "<foo>$o"
-        roundtrip "$o</foo>", "$o</foo>"
+        roundtrip "<foo>$o", {assert it == "<foo>$o"}
+        roundtrip "$o</foo>", {assert it == "$o</foo>"}
 
         //
-        roundtrip "<abc>\n<foo>$o</foo>\n</abc>", "<abc>\n<foo>$n</foo>\n</abc>"
+        roundtrip "<abc>\n<foo>$o</foo>\n</abc>", {assert it ==~ ABC_FOO_PATTERN}
     }
 
-    void roundtrip(String before, String after) {
+    void roundtrip(String before, Closure check) {
         def sr = new SecretRewriter(null);
         def f = File.createTempFile("test", "xml", tmp.root)
         f.text = before
         sr.rewrite(f,null)
-        assert after.replaceAll(System.getProperty("line.separator"), "\n").trim()==f.text.replaceAll(System.getProperty("line.separator"), "\n").trim()
+        check(f.text.replaceAll(System.getProperty("line.separator"), "\n").trim())
+        //assert after.replaceAll(System.getProperty("line.separator"), "\n").trim()==f.text.replaceAll(System.getProperty("line.separator"), "\n").trim()
     }
 
     String encryptOld(str) {
         def cipher = Secret.getCipher("AES");
-        cipher.init(Cipher.ENCRYPT_MODE, Secret.legacyKey);
-        return new String(Base64.encode(cipher.doFinal((str + Secret.MAGIC).getBytes("UTF-8"))))
+        cipher.init(Cipher.ENCRYPT_MODE, HistoricalSecrets.legacyKey);
+        return new String(Base64.encode(cipher.doFinal((str + HistoricalSecrets.MAGIC).getBytes("UTF-8"))))
     }
 
     String encryptNew(str) {
         return Secret.fromString(str).encryptedValue
     }
@@ -97,13 +104,13 @@ class SecretRewriterTest {
         new FilePath(t).child("a/symlink").symlinkTo(t2.absolutePath,st)
 
         assert 6==sw.rewriteRecursive(t, st)
 
         dirs.each { p->
-            assert new File(t,"$p/foo.xml").text.trim()==answer
+            assert new File(t,"$p/foo.xml").text.trim() ==~ MSG_PATTERN
         }
 
         // t2 is only reachable by following a symlink. this should be covered, too
-        assert new File(t2,"foo.xml").text.trim()==answer.trim();
+        assert new File(t2,"foo.xml").text.trim() ==~ MSG_PATTERN
     }
 
 }
diff --git a/core/src/test/groovy/hudson/util/SecretTest.groovy b/core/src/test/groovy/hudson/util/SecretTest.groovy
index f7a0ba3ad00..8f39ae7ec02 100644
--- a/core/src/test/groovy/hudson/util/SecretTest.groovy
+++ b/core/src/test/groovy/hudson/util/SecretTest.groovy
@@ -29,11 +29,12 @@ import jenkins.security.ConfidentialStoreRule;
 import org.apache.commons.lang.RandomStringUtils;
 import org.junit.Rule
 import org.junit.Test
 
 import java.util.Random;
-import javax.crypto.Cipher;
+import javax.crypto.Cipher
+import java.util.regex.Pattern;
 
 /**
  * @author Kohsuke Kawaguchi
  */
 public class SecretTest {
@@ -41,10 +42,12 @@ public class SecretTest {
     public ConfidentialStoreRule confidentialStore = new ConfidentialStoreRule()
 
     @Rule
     public MockSecretRule mockSecretRule = new MockSecretRule()
 
+    static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("\\{?[A-Za-z0-9+/]+={0,2}}?");
+
     @Test
     void testEncrypt() {
         def secret = Secret.fromString("abc");
         assert "abc"==secret.plainText;
 
@@ -52,21 +55,33 @@ public class SecretTest {
         println secret.encryptedValue;
         assert !"abc".equals(secret.encryptedValue);
 
         // can we round trip?
         assert secret==Secret.fromString(secret.encryptedValue);
+
+        //Two consecutive encryption requests of the same object should result in the same encrypted value - SECURITY-304
+        assert secret.encryptedValue == secret.encryptedValue
+        //Two consecutive encryption requests of different objects with the same value should not result in the same encrypted value - SECURITY-304
+        assert secret.encryptedValue != Secret.fromString(secret.plainText).encryptedValue
     }
 
     @Test
     void testEncryptedValuePattern() {
         for (int i = 1; i < 100; i++) {
             String plaintext = RandomStringUtils.random(new Random().nextInt(i));
             String ciphertext = Secret.fromString(plaintext).getEncryptedValue();
             //println "${plaintext} → ${ciphertext}"
-            assert Secret.ENCRYPTED_VALUE_PATTERN.matcher(ciphertext).matches();
+            assert ENCRYPTED_VALUE_PATTERN.matcher(ciphertext).matches();
         }
-        assert !Secret.ENCRYPTED_VALUE_PATTERN.matcher("hello world").matches();
+        //Not "plain" text
+        assert !ENCRYPTED_VALUE_PATTERN.matcher("hello world").matches();
+        //Not "plain" text
+        assert !ENCRYPTED_VALUE_PATTERN.matcher("helloworld!").matches();
+        //legacy key
+        assert ENCRYPTED_VALUE_PATTERN.matcher("abcdefghijklmnopqr0123456789").matches();
+        //legacy key
+        assert ENCRYPTED_VALUE_PATTERN.matcher("abcdefghijklmnopqr012345678==").matches();
     }
 
     @Test
     void testDecrypt() {
         assert "abc"==Secret.toString(Secret.fromString("abc"))
@@ -75,11 +90,11 @@ public class SecretTest {
     @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)
+        assert xml ==~ /<hudson\.util\.Secret>\{[A-Za-z0-9+\/]+={0,2}}<\/hudson\.util\.Secret>/
 
         def o = Jenkins.XSTREAM.fromXML(xml);
         assert o==s : xml;
     }
 
@@ -102,15 +117,15 @@ public class SecretTest {
     /**
      * Secret persisted with Jenkins.getSecretKey() should still decrypt OK.
      */
     @Test
     void migrationFromLegacyKeyToConfidentialStore() {
-        def legacy = Secret.legacyKey
+        def legacy = HistoricalSecrets.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 old = new String(Base64.encode(cipher.doFinal((str + HistoricalSecrets.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/test/src/test/java/hudson/util/SecretCompatTest.java b/test/src/test/java/hudson/util/SecretCompatTest.java
new file mode 100644
index 00000000000..8c440c4ff9c
--- /dev/null
+++ b/test/src/test/java/hudson/util/SecretCompatTest.java
@@ -0,0 +1,121 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
+ * Copyright (c) 2016, CloudBees Inc.
+ *
+ * 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 hudson.model.FreeStyleProject;
+import hudson.model.ParameterDefinition;
+import hudson.model.ParametersDefinitionProperty;
+import hudson.model.PasswordParameterDefinition;
+import org.hamcrest.core.Is;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.recipes.LocalData;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import static org.hamcrest.core.Is.isA;
+import static org.hamcrest.core.IsNot.not;
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.*;
+
+/**
+ * Tests {@link Secret}.
+ */
+public class SecretCompatTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule() {
+        @Override
+        public void before() throws Throwable {
+            Secret.resetKeyForTest();  //As early as possible
+            super.before();
+        }
+    };
+
+    @After
+    public void after() {
+        Secret.resetKeyForTest();
+    }
+
+
+    @Test
+    @Issue("SECURITY-304")
+    public void encryptedValueStaysTheSameAfterRoundtrip() throws Exception {
+        FreeStyleProject project = j.createFreeStyleProject();
+        project.addProperty(new ParametersDefinitionProperty(new PasswordParameterDefinition("p", "s3cr37", "Keep this a secret")));
+        project = j.configRoundtrip(project);
+        String round1 = project.getConfigFile().asString();
+        project = j.configRoundtrip(project);
+        String round2 = project.getConfigFile().asString();
+        assertEquals(round1, round2);
+
+
+        //But reconfiguring will make it a new value
+        project = j.jenkins.getItemByFullName(project.getFullName(), FreeStyleProject.class);
+        project.removeProperty(ParametersDefinitionProperty.class);
+        project.addProperty(new ParametersDefinitionProperty(new PasswordParameterDefinition("p", "s3cr37", "Keep this a secret")));
+        project = j.configRoundtrip(project);
+        String round3 = project.getConfigFile().asString();
+        assertNotEquals(round2, round3);
+        //Saving again will produce the same
+        project = j.configRoundtrip(project);
+        String round4 = project.getConfigFile().asString();
+        assertEquals(round3, round4);
+    }
+
+    @Test
+    @Issue("SECURITY-304")
+    @LocalData
+    public void canReadPreSec304Secrets() throws Exception {
+        FreeStyleProject project = j.jenkins.getItemByFullName("OldSecret", FreeStyleProject.class);
+        String oldxml = project.getConfigFile().asString();
+        //It should be unchanged on disk
+        assertThat(oldxml, containsString("<defaultValue>z/Dd3qrHdQ6/C5lR7uEafM/jD3nQDrGprw3XsfZ/0vo=</defaultValue>"));
+        ParametersDefinitionProperty property = project.getProperty(ParametersDefinitionProperty.class);
+        ParameterDefinition definition = property.getParameterDefinitions().get(0);
+        assertTrue(definition instanceof PasswordParameterDefinition);
+        Secret secret = ((PasswordParameterDefinition) definition).getDefaultValueAsSecret();
+        assertEquals("theSecret", secret.getPlainText());
+
+        //OK it was read correctly from disk, now the first roundtrip should update the encrypted value
+
+        project = j.configRoundtrip(project);
+        String newXml = project.getConfigFile().asString();
+        assertNotEquals(oldxml, newXml); //This could have changed because Jenkins has moved on, so not really a good check
+        assertThat(newXml, not(containsString("<defaultValue>z/Dd3qrHdQ6/C5lR7uEafM/jD3nQDrGprw3XsfZ/0vo=</defaultValue>")));
+        Pattern p = Pattern.compile("<defaultValue>\\{[A-Za-z0-9+/]+={0,2}}</defaultValue>");
+        assertTrue(p.matcher(newXml).find());
+
+        //But the next roundtrip should result in the same data
+        project = j.configRoundtrip(project);
+        String round2 = project.getConfigFile().asString();
+        assertEquals(newXml, round2);
+    }
+}
diff --git a/test/src/test/java/jenkins/security/RekeySecretAdminMonitorTest.java b/test/src/test/java/jenkins/security/RekeySecretAdminMonitorTest.java
index ea7a08ba458..413b4a6eed3 100644
--- a/test/src/test/java/jenkins/security/RekeySecretAdminMonitorTest.java
+++ b/test/src/test/java/jenkins/security/RekeySecretAdminMonitorTest.java
@@ -9,27 +9,33 @@ import com.trilead.ssh2.crypto.Base64;
 import hudson.FilePath;
 import hudson.Util;
 import hudson.util.Secret;
 import hudson.util.SecretHelper;
 import org.apache.commons.io.FileUtils;
+import org.hamcrest.CoreMatchers;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.recipes.Recipe.Runner;
 import org.xml.sax.SAXException;
 
 import javax.crypto.Cipher;
 import javax.inject.Inject;
 import java.io.File;
 import java.io.IOException;
 import java.lang.annotation.Annotation;
+import java.util.regex.Pattern;
+
+import static org.junit.Assert.assertThat;
 
 /**
  * @author Kohsuke Kawaguchi
  */
 public class RekeySecretAdminMonitorTest extends HudsonTestCase {
     @Inject
     RekeySecretAdminMonitor monitor;
 
+    final String plain_regex_match = ".*\\{[A-Za-z0-9+/]+={0,2}}.*";
+
     @Override
     protected void setUp() throws Exception {
         SecretHelper.set(TEST_KEY);
         super.setUp();
         monitor.setNeeded();
@@ -74,12 +80,12 @@ public class RekeySecretAdminMonitorTest extends HudsonTestCase {
         FileUtils.writeStringToFile(xml,"<foo>" + encryptOld(TEST_KEY) + "</foo>");
     }
 
     private void verifyRewrite(File dir) throws Exception {
         File xml = new File(dir, "foo.xml");
-        assertEquals("<foo>" + encryptNew(TEST_KEY) + "</foo>".trim(),
-                FileUtils.readFileToString(xml).trim());
+        Pattern pattern = Pattern.compile("<foo>"+plain_regex_match+"</foo>");
+        assertTrue(pattern.matcher(FileUtils.readFileToString(xml).trim()).matches());
     }
 
     // TODO sometimes fails: "Invalid request submission: {json=[Ljava.lang.String;@2c46358e, .crumb=[Ljava.lang.String;@35661457}"
     public void _testBasicWorkflow() throws Exception {
         putSomeOldData(jenkins.getRootDir());
diff --git a/test/src/test/java/lib/form/PasswordTest.java b/test/src/test/java/lib/form/PasswordTest.java
index 595b0e7e851..0a9f5eb1495 100644
--- a/test/src/test/java/lib/form/PasswordTest.java
+++ b/test/src/test/java/lib/form/PasswordTest.java
@@ -41,14 +41,17 @@ import hudson.util.Secret;
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Locale;
+import java.util.regex.Pattern;
+
 import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.TestExtension;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -76,14 +79,23 @@ public class PasswordTest extends HudsonTestCase implements Describable<Password
         public String getDisplayName() {
             return null;
         }
     }
 
-    @Issue("SECURITY-266")
+    // TODO in trunk switch to @Issue({"SECURITY-266","SECURITY-304"})
+    @Issue("SECURITY-266, SECURITY-304")
     public void testExposedCiphertext() throws Exception {
         boolean saveEnabled = Item.EXTENDED_READ.getEnabled();
         try {
+
+            //final String plain_regex_match = ".*\\{[A-Za-z0-9+/]+={0,2}}.*";
+            final String xml_regex_match = "\\{[A-Za-z0-9+/]+={0,2}}";
+            final Pattern xml_regex_pattern = Pattern.compile(xml_regex_match);
+            final String staticTest = "\n\nvalue=\"{AQAAABAAAAAgXhXgopokysZkduhl+v1gm0UhUBBbjKDVpKz7bGk3mIO53cNTRdlu7LC4jZYEc+vF}\"\n";
+            //Just a quick verification on what could be on the page and that the regexp is correctly set up
+            assertThat(xml_regex_pattern.matcher(staticTest).find(), is(true));
+
             jenkins.setSecurityRealm(createDummySecurityRealm());
             // TODO 1.645+ use MockAuthorizationStrategy
             GlobalMatrixAuthorizationStrategy pmas = new GlobalMatrixAuthorizationStrategy();
             pmas.add(Jenkins.ADMINISTER, "admin");
             pmas.add(Jenkins.READ, "dev");
@@ -91,27 +103,28 @@ public class PasswordTest extends HudsonTestCase implements Describable<Password
             Item.EXTENDED_READ.setEnabled(true);
             pmas.add(Item.EXTENDED_READ, "dev");
             pmas.add(Item.CREATE, "dev"); // so we can show CopyJobCommand would barf; more realistic would be to grant it only in a subfolder
             jenkins.setAuthorizationStrategy(pmas);
             Secret s = Secret.fromString("s3cr3t");
-            String sEnc = s.getEncryptedValue();
+            //String sEnc = s.getEncryptedValue();
             FreeStyleProject p = createFreeStyleProject("p");
             p.setDisplayName("Unicode here ←");
             p.setDescription("This+looks+like+Base64+but+is+not+a+secret");
             p.addProperty(new VulnerableProperty(s));
             WebClient wc = createWebClient();
             // Control case: an administrator can read and write configuration freely.
             wc.login("admin");
             HtmlPage configure = wc.getPage(p, "configure");
-            assertThat(configure.getWebResponse().getContentAsString(), containsString(sEnc));
+            assertThat(xml_regex_pattern.matcher(configure.getWebResponse().getContentAsString()).find(), is(true));
             submit(configure.getFormByName("config"));
             VulnerableProperty vp = p.getProperty(VulnerableProperty.class);
             assertNotNull(vp);
             assertEquals(s, vp.secret);
             Page configXml = wc.goTo(p.getUrl() + "config.xml", "application/xml");
             String xmlAdmin = configXml.getWebResponse().getContentAsString();
-            assertThat(xmlAdmin, containsString("<secret>" + sEnc + "</secret>"));
+
+            assertThat(Pattern.compile("<secret>" + xml_regex_match + "</secret>").matcher(xmlAdmin).find(), is(true));
             assertThat(xmlAdmin, containsString("<displayName>" + p.getDisplayName() + "</displayName>"));
             assertThat(xmlAdmin, containsString("<description>" + p.getDescription() + "</description>"));
             // CLICommandInvoker does not work here, as it sets up its own SecurityRealm + AuthorizationStrategy.
             GetJobCommand getJobCommand = new GetJobCommand();
             Authentication adminAuth = User.get("admin").impersonate();
@@ -129,15 +142,15 @@ public class PasswordTest extends HudsonTestCase implements Describable<Password
             pAdmin.setDisplayName(p.getDisplayName()); // counteract DisplayNameListener
             assertEquals(p.getConfigFile().asString(), pAdmin.getConfigFile().asString());
             // Test case: another user with EXTENDED_READ but not CONFIGURE should not get access even to encrypted secrets.
             wc.login("dev");
             configure = wc.getPage(p, "configure");
-            assertThat(configure.getWebResponse().getContentAsString(), not(containsString(sEnc)));
+            assertThat(xml_regex_pattern.matcher(configure.getWebResponse().getContentAsString()).find(), is(false));
             configXml = wc.goTo(p.getUrl() + "config.xml", "application/xml");
             String xmlDev = configXml.getWebResponse().getContentAsString();
-            assertThat(xmlDev, not(containsString(sEnc)));
-            assertEquals(xmlAdmin.replace(sEnc, "********"), xmlDev);
+            assertThat(xml_regex_pattern.matcher(xmlDev).find(), is(false));
+            assertEquals(xmlAdmin.replaceAll(xml_regex_match, "********"), xmlDev);
             getJobCommand = new GetJobCommand();
             Authentication devAuth = User.get("dev").impersonate();
             getJobCommand.setTransportAuth(devAuth);
             baos = new ByteArrayOutputStream();
             getJobCommand.main(Collections.singletonList(pName), Locale.ENGLISH, System.in, new PrintStream(baos), System.err);
diff --git a/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/config.xml b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/config.xml
new file mode 100644
index 00000000000..f47b1c32c9a
--- /dev/null
+++ b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/config.xml
@@ -0,0 +1,34 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<hudson>
+    <disabledAdministrativeMonitors/>
+    <version>1.625.4-SNAPSHOT (private-12/16/2016 18:04 GMT-rsandell)</version>
+    <numExecutors>2</numExecutors>
+    <mode>NORMAL</mode>
+    <useSecurity>true</useSecurity>
+    <authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
+    <securityRealm class="hudson.security.SecurityRealm$None"/>
+    <disableRememberMe>false</disableRememberMe>
+    <projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
+    <workspaceDir>${ITEM_ROOTDIR}/workspace</workspaceDir>
+    <buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
+    <jdks/>
+    <viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
+    <myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
+    <clouds/>
+    <quietPeriod>5</quietPeriod>
+    <scmCheckoutRetryCount>0</scmCheckoutRetryCount>
+    <views>
+        <hudson.model.AllView>
+            <owner class="hudson" reference="../../.."/>
+            <name>All</name>
+            <filterExecutors>false</filterExecutors>
+            <filterQueue>false</filterQueue>
+            <properties class="hudson.model.View$PropertyList"/>
+        </hudson.model.AllView>
+    </views>
+    <primaryView>All</primaryView>
+    <slaveAgentPort>0</slaveAgentPort>
+    <label></label>
+    <nodeProperties/>
+    <globalNodeProperties/>
+</hudson>
\ No newline at end of file
diff --git a/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/jobs/OldSecret/config.xml b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/jobs/OldSecret/config.xml
new file mode 100644
index 00000000000..e094ad77fd4
--- /dev/null
+++ b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/jobs/OldSecret/config.xml
@@ -0,0 +1,27 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<project>
+  <actions/>
+  <description></description>
+  <keepDependencies>false</keepDependencies>
+  <properties>
+    <hudson.model.ParametersDefinitionProperty>
+      <parameterDefinitions>
+        <hudson.model.PasswordParameterDefinition>
+          <name>alice</name>
+          <description>theSecret</description>
+          <defaultValue>z/Dd3qrHdQ6/C5lR7uEafM/jD3nQDrGprw3XsfZ/0vo=</defaultValue>
+        </hudson.model.PasswordParameterDefinition>
+      </parameterDefinitions>
+    </hudson.model.ParametersDefinitionProperty>
+  </properties>
+  <scm class="hudson.scm.NullSCM"/>
+  <canRoam>true</canRoam>
+  <disabled>false</disabled>
+  <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
+  <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
+  <triggers/>
+  <concurrentBuild>false</concurrentBuild>
+  <builders/>
+  <publishers/>
+  <buildWrappers/>
+</project>
\ No newline at end of file
diff --git a/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/hudson.util.Secret b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/hudson.util.Secret
new file mode 100644
index 00000000000..cdec3920fbc
Binary files /dev/null and b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/hudson.util.Secret differ
diff --git a/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/master.key b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/master.key
new file mode 100644
index 00000000000..2572203a97b
--- /dev/null
+++ b/test/src/test/resources/hudson/util/SecretCompatTest/canReadPreSec304Secrets/secrets/master.key
@@ -0,0 +1 @@
+4311ab5e95e3da7b9b1360b52cac6f0f666db7b48f9f701296cc07c8f00612b451f1874e584d49560810619e8a6ff6b19f8f58ae1305c515fc62a7b60ea3a69e6058cad16b2c8df317952b749fdaaecab013431da55bb4ea4b8eee754fa043261b51a99a2b537fd57f867cdcb1e209f3bba735a8672dbfc3f10b0e2209a81683
\ No newline at end of file

Vulnerable code:      * If the user lacks {@link CONFIGURE} then any {@link Secret}s must be masked out, even in encrypted form.
import javax.crypto.SecretKey;
    private Secret(String value) {
    /**
     * 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.
     */
    @Deprecated
    /*package*/ static SecretKey getLegacyKey() throws GeneralSecurityException {
        String secret = SECRET;
        if(secret==null)    return Jenkins.getInstance().getSecretKeyAsAES128();
        return Util.toAes128Key(secret);
    }

            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 (UnsupportedEncodingException e) {
            throw new Error(e); // impossible
     * Pattern matching a possible output of {@link #getEncryptedValue}.
     * Basically, any Base64-encoded value.
     * You must then call {@link #decrypt} to eliminate false positives.
    public static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("[A-Za-z0-9+/]+={0,2}");
        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, getLegacyKey());
            return tryDecrypt(cipher, in);
        } catch (GeneralSecurityException e) {
            return null;
        } catch (UnsupportedEncodingException e) {
            throw new Error(e); // impossible
        } catch (IOException e) {
            return null;
        }
    }

    /*package*/ 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; // if the key doesn't match with the bytes, it can result in BadPaddingException
    private static final String MAGIC = "::::MAGIC::::";

        key = Secret.getLegacyKey();
        Secret sec = Secret.tryDecrypt(cipher, in);
                        secret = new SecretKeySpec(payload,0,128/8, ALGORITHM);
            Cipher cipher = Secret.getCipher(ALGORITHM);
            Cipher cipher = Secret.getCipher(ALGORITHM);
    private static final String ALGORITHM = "AES";
                  "<foo>${n}</foo>"
                  "<foo>${n}</foo><foo>${n}</foo>"
                  "<foo>${n}</foo>"
                  "  <foo>thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret</foo>  "
        roundtrip "<foo>$o", "<foo>$o"
        roundtrip "$o</foo>", "$o</foo>"
        roundtrip "<abc>\n<foo>$o</foo>\n</abc>", "<abc>\n<foo>$n</foo>\n</abc>"
    void roundtrip(String before, String after) {
        assert after.replaceAll(System.getProperty("line.separator"), "\n").trim()==f.text.replaceAll(System.getProperty("line.separator"), "\n").trim()
        cipher.init(Cipher.ENCRYPT_MODE, Secret.legacyKey);
        return new String(Base64.encode(cipher.doFinal((str + Secret.MAGIC).getBytes("UTF-8"))))
            assert new File(t,"$p/foo.xml").text.trim()==answer
        assert new File(t2,"foo.xml").text.trim()==answer.trim();
import javax.crypto.Cipher;
            assert Secret.ENCRYPTED_VALUE_PATTERN.matcher(ciphertext).matches();
        assert !Secret.ENCRYPTED_VALUE_PATTERN.matcher("hello world").matches();
        assert xml.contains(s.encryptedValue)
        def legacy = Secret.legacyKey
            def old = new String(Base64.encode(cipher.doFinal((str + Secret.MAGIC).getBytes("UTF-8"))))
        assertEquals("<foo>" + encryptNew(TEST_KEY) + "</foo>".trim(),
                FileUtils.readFileToString(xml).trim());
    @Issue("SECURITY-266")
            String sEnc = s.getEncryptedValue();
            assertThat(configure.getWebResponse().getContentAsString(), containsString(sEnc));
            assertThat(xmlAdmin, containsString("<secret>" + sEnc + "</secret>"));
            assertThat(configure.getWebResponse().getContentAsString(), not(containsString(sEnc)));
            assertThat(xmlDev, not(containsString(sEnc)));
            assertEquals(xmlAdmin.replace(sEnc, "********"), xmlDev);
Fixed code:      * If the user lacks {@link #CONFIGURE} then any {@link Secret}s must be masked out, even in encrypted form.
/*
 * The MIT License
 *
 * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
 * Copyright (c) 2016, CloudBees Inc.
 *
 * 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 hudson.Util;
import jenkins.model.Jenkins;
import jenkins.security.CryptoConfidentialKey;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

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

import static java.nio.charset.StandardCharsets.UTF_8;

/**
 * Historical algorithms for decrypting {@link Secret}s.
 */
@Restricted(NoExternalUse.class)
public class HistoricalSecrets {

    /*package*/ static Secret decrypt(String data, CryptoConfidentialKey key) throws IOException, GeneralSecurityException {
        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 = Secret.getCipher("AES");
        cipher.init(Cipher.DECRYPT_MODE, getLegacyKey());
        return tryDecrypt(cipher, in);
    }

    /*package*/ static Secret tryDecrypt(Cipher cipher, byte[] in) {
        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; // if the key doesn't match with the bytes, it can result in BadPaddingException
        }
    }

    /**
     * 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.
     */
    @Deprecated
    /*package*/ static SecretKey getLegacyKey() throws GeneralSecurityException {
        String secret = Secret.SECRET;
        if(secret==null)    return Jenkins.getInstance().getSecretKeyAsAES128();
        return Util.toAes128Key(secret);
    }

    private static final String MAGIC = "::::MAGIC::::";
}
 * Copyright (c) 2016, CloudBees Inc.
import java.util.Arrays;
import static java.nio.charset.StandardCharsets.UTF_8;

    private static final byte PAYLOAD_V1 = 1;
    private byte[] iv;

    /*package*/ Secret(String value) {
        this.value = value;
    }
    /*package*/ Secret(String value, byte[] iv) {
        this.iv = iv;
            synchronized (this) {
                if (iv == null) { //if we were created from plain text or other reason without iv
                    iv = KEY.newIv();
                }
            }
            Cipher cipher = KEY.encrypt(iv);
            byte[] encrypted = cipher.doFinal(this.value.getBytes(UTF_8));
            byte[] payload = new byte[1 + 8 + iv.length + encrypted.length];
            int pos = 0;
            // For PAYLOAD_V1 we use this byte shifting model, V2 probably will need DataOutput
            payload[pos++] = PAYLOAD_V1;
            payload[pos++] = (byte)(iv.length >> 24);
            payload[pos++] = (byte)(iv.length >> 16);
            payload[pos++] = (byte)(iv.length >> 8);
            payload[pos++] = (byte)(iv.length);
            payload[pos++] = (byte)(encrypted.length >> 24);
            payload[pos++] = (byte)(encrypted.length >> 16);
            payload[pos++] = (byte)(encrypted.length >> 8);
            payload[pos++] = (byte)(encrypted.length);
            System.arraycopy(iv, 0, payload, pos, iv.length);
            pos+=iv.length;
            System.arraycopy(encrypted, 0, payload, pos, encrypted.length);
            return "{"+new String(Base64.encode(payload))+"}";
     * Pattern matching a possible output of {@link #getEncryptedValue}
     * Basically, any Base64-encoded value optionally wrapped by {@code {}}.
     * You must then call {@link #decrypt(String)} to eliminate false positives.
     * @see #ENCRYPTED_VALUE_PATTERN
    public static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("\\{?[A-Za-z0-9+/]+={0,2}}?");
        if (data == null) return null;
        if (data.startsWith("{") && data.endsWith("}")) { //likely CBC encrypted/containing metadata but could be plain text
            byte[] payload;
            try {
                payload = Base64.decode(data.substring(1, data.length()-1).toCharArray());
            } catch (IOException e) {
                return null;
            }
            switch (payload[0]) {
                case PAYLOAD_V1:
                    // For PAYLOAD_V1 we use this byte shifting model, V2 probably will need DataOutput
                    int ivLength = ((payload[1] & 0xff) << 24)
                            | ((payload[2] & 0xff) << 16)
                            | ((payload[3] & 0xff) << 8)
                            | (payload[4] & 0xff);
                    int dataLength = ((payload[5] & 0xff) << 24)
                            | ((payload[6] & 0xff) << 16)
                            | ((payload[7] & 0xff) << 8)
                            | (payload[8] & 0xff);
                    if (payload.length != 1 + 8 + ivLength + dataLength) {
                        // not valid v1
                        return null;
                    }
                    byte[] iv = Arrays.copyOfRange(payload, 9, 9 + ivLength);
                    byte[] code = Arrays.copyOfRange(payload, 9+ivLength, payload.length);
                    String text;
                    try {
                        text = new String(KEY.decrypt(iv).doFinal(code), UTF_8);
                    } catch (GeneralSecurityException e) {
                        // it's v1 which cannot be historical, but not decrypting
                        return null;
                    }
                    return new Secret(text, iv);
                default:
                    return null;
            }
        } else {
            try {
                return HistoricalSecrets.decrypt(data, KEY);
            } catch (GeneralSecurityException e) {
                return null;
            } catch (UnsupportedEncodingException e) {
                throw new Error(e); // impossible
            } catch (IOException e) {
                return null;
            }
    /**
     * Reset the internal secret key for testing.
     */
    @Restricted(NoExternalUse.class)
    /*package*/ static void resetKeyForTest() {
        KEY.resetForTest();
    }

        key = HistoricalSecrets.getLegacyKey();
        Secret sec = HistoricalSecrets.tryDecrypt(cipher, in);
import hudson.Main;
import jenkins.model.Jenkins;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import javax.crypto.spec.IvParameterSpec;
    @Restricted(NoExternalUse.class) //TODO remove when in mainline
    public static final int DEFAULT_IV_LENGTH = 16;

                        secret = new SecretKeySpec(payload,0,128/8, KEY_ALGORITHM);
     * @deprecated use {@link #encrypt(byte[])}
    @Deprecated
            Cipher cipher = Secret.getCipher(KEY_ALGORITHM);
    /**
     * Returns a {@link Cipher} object for encrypting with this key using the provided initialization vector.
     * @param iv the initialization vector
     * @return the cipher
     */
    @Restricted(NoExternalUse.class) //TODO remove when in mainline
    public Cipher encrypt(byte[] iv) {
        try {
            Cipher cipher = Secret.getCipher(ALGORITHM);
            cipher.init(Cipher.ENCRYPT_MODE, getKey(), new IvParameterSpec(iv));
            return cipher;
        } catch (GeneralSecurityException e) {
            throw new AssertionError(e);
        }
    }

    /**
     * Returns a {@link Cipher} object for decrypting with this key using the provided initialization vector.
     * @param iv the initialization vector
     * @return the cipher
     */
    @Restricted(NoExternalUse.class) //TODO remove when in mainline
    public Cipher decrypt(byte[] iv) {
        try {
            Cipher cipher = Secret.getCipher(ALGORITHM);
            cipher.init(Cipher.DECRYPT_MODE, getKey(), new IvParameterSpec(iv));
            return cipher;
        } catch (GeneralSecurityException e) {
            throw new AssertionError(e);
        }
    }

    /**
     * Generates a new Initialization Vector.
     * @param length the length of the salt
     * @return some random bytes
     * @see #encrypt(byte[])
     */
    @Restricted(NoExternalUse.class) //TODO remove when in mainline
    public byte[] newIv(int length) {
        return ConfidentialStore.get().randomBytes(length);
    }

    /**
     * Generates a new Initialization Vector of default length.
     * @return some random bytes
     * @see #newIv(int)
     * @see #encrypt(byte[])
     */
    @Restricted(NoExternalUse.class) //TODO remove when in mainline
    public byte[] newIv() {
        return newIv(DEFAULT_IV_LENGTH);
    }

     * @deprecated use {@link #decrypt(byte[])}
    @Deprecated
            Cipher cipher = Secret.getCipher(KEY_ALGORITHM);
    private static final String KEY_ALGORITHM = "AES";
    private static final String ALGORITHM = "AES/CBC/PKCS5Padding";

    /**
     * Reset the internal secret key for testing.
     */
    @Restricted(NoExternalUse.class)
    public void resetForTest() {
        if (Main.isUnitTest) {
            this.secret = null;
        } else {
            throw new IllegalStateException("Only for testing");
        }
    }
    def FOO_PATTERN = /<foo>\{[A-Za-z0-9+\/]+={0,2}}<\/foo>/
    def MSG_PATTERN = /<msg>\{[A-Za-z0-9+\/]+={0,2}}<\/msg>/
    def FOO_PATTERN2 = /(<foo>\{[A-Za-z0-9+\/]+={0,2}}<\/foo>){2}/
    def ABC_FOO_PATTERN = /<abc>\s<foo>\{[A-Za-z0-9+\/]+={0,2}}<\/foo>\s<\/abc>/

                {assert it ==~ FOO_PATTERN}

                {assert it ==~ FOO_PATTERN2}
                {assert it == "<foo>${n}</foo>"}
                {assert it == "<foo>thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret</foo>"}
        roundtrip "<foo>$o", {assert it == "<foo>$o"}
        roundtrip "$o</foo>", {assert it == "$o</foo>"}
        roundtrip "<abc>\n<foo>$o</foo>\n</abc>", {assert it ==~ ABC_FOO_PATTERN}
    void roundtrip(String before, Closure check) {
        check(f.text.replaceAll(System.getProperty("line.separator"), "\n").trim())
        //assert after.replaceAll(System.getProperty("line.separator"), "\n").trim()==f.text.replaceAll(System.getProperty("line.separator"), "\n").trim()
        cipher.init(Cipher.ENCRYPT_MODE, HistoricalSecrets.legacyKey);
        return new String(Base64.encode(cipher.doFinal((str + HistoricalSecrets.MAGIC).getBytes("UTF-8"))))
            assert new File(t,"$p/foo.xml").text.trim() ==~ MSG_PATTERN
        assert new File(t2,"foo.xml").text.trim() ==~ MSG_PATTERN
import javax.crypto.Cipher
import java.util.regex.Pattern;
    static final Pattern ENCRYPTED_VALUE_PATTERN = Pattern.compile("\\{?[A-Za-z0-9+/]+={0,2}}?");


        //Two consecutive encryption requests of the same object should result in the same encrypted value - SECURITY-304
        assert secret.encryptedValue == secret.encryptedValue
        //Two consecutive encryption requests of different objects with the same value should not result in the same encrypted value - SECURITY-304
        assert secret.encryptedValue != Secret.fromString(secret.plainText).encryptedValue
            assert ENCRYPTED_VALUE_PATTERN.matcher(ciphertext).matches();
        //Not "plain" text
        assert !ENCRYPTED_VALUE_PATTERN.matcher("hello world").matches();
        //Not "plain" text
        assert !ENCRYPTED_VALUE_PATTERN.matcher("helloworld!").matches();
        //legacy key
        assert ENCRYPTED_VALUE_PATTERN.matcher("abcdefghijklmnopqr0123456789").matches();
        //legacy key
        assert ENCRYPTED_VALUE_PATTERN.matcher("abcdefghijklmnopqr012345678==").matches();
        assert xml ==~ /<hudson\.util\.Secret>\{[A-Za-z0-9+\/]+={0,2}}<\/hudson\.util\.Secret>/
        def legacy = HistoricalSecrets.legacyKey
            def old = new String(Base64.encode(cipher.doFinal((str + HistoricalSecrets.MAGIC).getBytes("UTF-8"))))
/*
 * The MIT License
 *
 * Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi
 * Copyright (c) 2016, CloudBees Inc.
 *
 * 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 hudson.model.FreeStyleProject;
import hudson.model.ParameterDefinition;
import hudson.model.ParametersDefinitionProperty;
import hudson.model.PasswordParameterDefinition;
import org.hamcrest.core.Is;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.recipes.LocalData;

import java.io.IOException;
import java.util.regex.Pattern;

import static org.hamcrest.core.Is.isA;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.*;

/**
 * Tests {@link Secret}.
 */
public class SecretCompatTest {

    @Rule
    public JenkinsRule j = new JenkinsRule() {
        @Override
        public void before() throws Throwable {
            Secret.resetKeyForTest();  //As early as possible
            super.before();
        }
    };

    @After
    public void after() {
        Secret.resetKeyForTest();
    }


    @Test
    @Issue("SECURITY-304")
    public void encryptedValueStaysTheSameAfterRoundtrip() throws Exception {
        FreeStyleProject project = j.createFreeStyleProject();
        project.addProperty(new ParametersDefinitionProperty(new PasswordParameterDefinition("p", "s3cr37", "Keep this a secret")));
        project = j.configRoundtrip(project);
        String round1 = project.getConfigFile().asString();
        project = j.configRoundtrip(project);
        String round2 = project.getConfigFile().asString();
        assertEquals(round1, round2);


        //But reconfiguring will make it a new value
        project = j.jenkins.getItemByFullName(project.getFullName(), FreeStyleProject.class);
        project.removeProperty(ParametersDefinitionProperty.class);
        project.addProperty(new ParametersDefinitionProperty(new PasswordParameterDefinition("p", "s3cr37", "Keep this a secret")));
        project = j.configRoundtrip(project);
        String round3 = project.getConfigFile().asString();
        assertNotEquals(round2, round3);
        //Saving again will produce the same
        project = j.configRoundtrip(project);
        String round4 = project.getConfigFile().asString();
        assertEquals(round3, round4);
    }

    @Test
    @Issue("SECURITY-304")
    @LocalData
    public void canReadPreSec304Secrets() throws Exception {
        FreeStyleProject project = j.jenkins.getItemByFullName("OldSecret", FreeStyleProject.class);
        String oldxml = project.getConfigFile().asString();
        //It should be unchanged on disk
        assertThat(oldxml, containsString("<defaultValue>z/Dd3qrHdQ6/C5lR7uEafM/jD3nQDrGprw3XsfZ/0vo=</defaultValue>"));
        ParametersDefinitionProperty property = project.getProperty(ParametersDefinitionProperty.class);
        ParameterDefinition definition = property.getParameterDefinitions().get(0);
        assertTrue(definition instanceof PasswordParameterDefinition);
        Secret secret = ((PasswordParameterDefinition) definition).getDefaultValueAsSecret();
        assertEquals("theSecret", secret.getPlainText());

        //OK it was read correctly from disk, now the first roundtrip should update the encrypted value

        project = j.configRoundtrip(project);
        String newXml = project.getConfigFile().asString();
        assertNotEquals(oldxml, newXml); //This could have changed because Jenkins has moved on, so not really a good check
        assertThat(newXml, not(containsString("<defaultValue>z/Dd3qrHdQ6/C5lR7uEafM/jD3nQDrGprw3XsfZ/0vo=</defaultValue>")));
        Pattern p = Pattern.compile("<defaultValue>\\{[A-Za-z0-9+/]+={0,2}}</defaultValue>");
        assertTrue(p.matcher(newXml).find());

        //But the next roundtrip should result in the same data
        project = j.configRoundtrip(project);
        String round2 = project.getConfigFile().asString();
        assertEquals(newXml, round2);
    }
}
import org.hamcrest.CoreMatchers;
import java.util.regex.Pattern;

import static org.junit.Assert.assertThat;
    final String plain_regex_match = ".*\\{[A-Za-z0-9+/]+={0,2}}.*";

        Pattern pattern = Pattern.compile("<foo>"+plain_regex_match+"</foo>");
        assertTrue(pattern.matcher(FileUtils.readFileToString(xml).trim()).matches());
import java.util.regex.Pattern;

import static org.hamcrest.core.Is.is;
    // TODO in trunk switch to @Issue({"SECURITY-266","SECURITY-304"})
    @Issue("SECURITY-266, SECURITY-304")

            //final String plain_regex_match = ".*\\{[A-Za-z0-9+/]+={0,2}}.*";
            final String xml_regex_match = "\\{[A-Za-z0-9+/]+={0,2}}";
            final Pattern xml_regex_pattern = Pattern.compile(xml_regex_match);
            final String staticTest = "\n\nvalue=\"{AQAAABAAAAAgXhXgopokysZkduhl+v1gm0UhUBBbjKDVpKz7bGk3mIO53cNTRdlu7LC4jZYEc+vF}\"\n";
            //Just a quick verification on what could be on the page and that the regexp is correctly set up
            assertThat(xml_regex_pattern.matcher(staticTest).find(), is(true));

            //String sEnc = s.getEncryptedValue();
            assertThat(xml_regex_pattern.matcher(configure.getWebResponse().getContentAsString()).find(), is(true));

            assertThat(Pattern.compile("<secret>" + xml_regex_match + "</secret>").matcher(xmlAdmin).find(), is(true));
            assertThat(xml_regex_pattern.matcher(configure.getWebResponse().getContentAsString()).find(), is(false));
            assertThat(xml_regex_pattern.matcher(xmlDev).find(), is(false));
            assertEquals(xmlAdmin.replaceAll(xml_regex_match, "********"), xmlDev);
<?xml version='1.0' encoding='UTF-8'?>
<hudson>
    <disabledAdministrativeMonitors/>
    <version>1.625.4-SNAPSHOT (private-12/16/2016 18:04 GMT-rsandell)</version>
    <numExecutors>2</numExecutors>
    <mode>NORMAL</mode>
    <useSecurity>true</useSecurity>
    <authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
    <securityRealm class="hudson.security.SecurityRealm$None"/>
    <disableRememberMe>false</disableRememberMe>
    <projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
    <workspaceDir>${ITEM_ROOTDIR}/workspace</workspaceDir>
    <buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
    <jdks/>
    <viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
    <myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
    <clouds/>
    <quietPeriod>5</quietPeriod>
    <scmCheckoutRetryCount>0</scmCheckoutRetryCount>
    <views>
        <hudson.model.AllView>
            <owner class="hudson" reference="../../.."/>
            <name>All</name>
            <filterExecutors>false</filterExecutors>
            <filterQueue>false</filterQueue>
            <properties class="hudson.model.View$PropertyList"/>
        </hudson.model.AllView>
    </views>
    <primaryView>All</primaryView>
    <slaveAgentPort>0</slaveAgentPort>
    <label></label>
    <nodeProperties/>
    <globalNodeProperties/>
</hudson>
<?xml version='1.0' encoding='UTF-8'?>
<project>
  <actions/>
  <description></description>
  <keepDependencies>false</keepDependencies>
  <properties>
    <hudson.model.ParametersDefinitionProperty>
      <parameterDefinitions>
        <hudson.model.PasswordParameterDefinition>
          <name>alice</name>
          <description>theSecret</description>
          <defaultValue>z/Dd3qrHdQ6/C5lR7uEafM/jD3nQDrGprw3XsfZ/0vo=</defaultValue>
        </hudson.model.PasswordParameterDefinition>
      </parameterDefinitions>
    </hudson.model.ParametersDefinitionProperty>
  </properties>
  <scm class="hudson.scm.NullSCM"/>
  <canRoam>true</canRoam>
  <disabled>false</disabled>
  <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
  <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
  <triggers/>
  <concurrentBuild>false</concurrentBuild>
  <builders/>
  <publishers/>
  <buildWrappers/>
</project>
4311ab5e95e3da7b9b1360b52cac6f0f666db7b48f9f701296cc07c8f00612b451f1874e584d49560810619e8a6ff6b19f8f58ae1305c515fc62a7b60ea3a69e6058cad16b2c8df317952b749fdaaecab013431da55bb4ea4b8eee754fa043261b51a99a2b537fd57f867cdcb1e209f3bba735a8672dbfc3f10b0e2209a81683