org.kohsuke.stapler
diff --git a/core/src/main/java/hudson/console/ConsoleNote.java b/core/src/main/java/hudson/console/ConsoleNote.java
index c27fec6364..f44b7b6db6 100644
--- a/core/src/main/java/hudson/console/ConsoleNote.java
+++ b/core/src/main/java/hudson/console/ConsoleNote.java
@@ -51,6 +51,8 @@ import java.util.Collection;
import java.util.List;
import com.jcraft.jzlib.GZIPInputStream;
import com.jcraft.jzlib.GZIPOutputStream;
+import hudson.remoting.ClassFilter;
+import jenkins.security.HMACConfidentialKey;
/**
* Data that hangs off from a console output.
@@ -94,8 +96,6 @@ import com.jcraft.jzlib.GZIPOutputStream;
* {@link ConsoleNote} always sticks to a particular point in the console output.
*
*
- * This design allows descendant processes of Hudson to emit {@link ConsoleNote}s. For example, Ant forked
- * by a shell forked by Hudson can put an encoded note in its stdout, and Hudson will correctly understands that.
* The preamble and postamble includes a certain ANSI escape sequence designed in such a way to minimize garbage
* if this output is observed by a human being directly.
*
@@ -121,6 +121,15 @@ import com.jcraft.jzlib.GZIPOutputStream;
* @since 1.349
*/
public abstract class ConsoleNote implements Serializable, Describable>, ExtensionPoint {
+
+ private static final HMACConfidentialKey MAC = new HMACConfidentialKey(ConsoleNote.class, "MAC");
+ /**
+ * Allows historical build records with unsigned console notes to be displayed, at the expense of any security.
+ * Disables checking of {@link #MAC} so do not set this flag unless you completely trust all users capable of affecting build output,
+ * which in practice means that all SCM committers as well as all Jenkins users with any non-read-only access are consider administrators.
+ */
+ static /* nonfinal for tests & script console */ boolean INSECURE = Boolean.getBoolean(ConsoleNote.class.getName() + ".INSECURE"); // TODO 2.x use SystemProperties
+
/**
* When the line of a console output that this annotation is attached is read by someone,
* a new {@link ConsoleNote} is de-serialized and this method is invoked to annotate that line.
@@ -182,6 +191,9 @@ public abstract class ConsoleNote implements Serializable, Describable implements Serializable, Describable 0) { // new format
+ mac = new byte[macSz];
+ decoded.readFully(mac);
+ sz = decoded.readInt();
+ } else {
+ mac = null;
+ sz = - macSz;
+ }
byte[] buf = new byte[sz];
decoded.readFully(buf);
@@ -223,12 +245,19 @@ public abstract class ConsoleNote implements Serializable, Describable implements Serializable, Describable 0) { // new format
+ IOUtils.skip(decoded, macSz);
+ int sz = decoded.readInt();
+ IOUtils.skip(decoded, sz);
+ } else { // old format
+ int sz = -macSz;
+ IOUtils.skip(decoded, sz);
+ }
byte[] postamble = new byte[POSTAMBLE.length];
in.readFully(postamble);
diff --git a/core/src/main/java/hudson/model/Item.java b/core/src/main/java/hudson/model/Item.java
index 6706f2b492..6b8f66aa14 100644
--- a/core/src/main/java/hudson/model/Item.java
+++ b/core/src/main/java/hudson/model/Item.java
@@ -230,7 +230,7 @@ public interface Item extends PersistenceRoot, SearchableModelObject, AccessCont
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, SystemProperties.getBoolean("hudson.security.ExtendedReadPermission"), new PermissionScope[]{PermissionScope.ITEM});
diff --git a/core/src/main/java/hudson/slaves/OfflineCause.java b/core/src/main/java/hudson/slaves/OfflineCause.java
index 7f7704d712..e2a902f1d7 100644
--- a/core/src/main/java/hudson/slaves/OfflineCause.java
+++ b/core/src/main/java/hudson/slaves/OfflineCause.java
@@ -24,7 +24,6 @@
package hudson.slaves;
-import jenkins.model.Jenkins;
import hudson.Functions;
import hudson.model.Computer;
import hudson.model.User;
@@ -33,7 +32,10 @@ import org.jvnet.localizer.Localizable;
import org.kohsuke.stapler.export.ExportedBean;
import org.kohsuke.stapler.export.Exported;
+import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
+import java.io.ObjectStreamException;
+import java.util.Collections;
import java.util.Date;
/**
@@ -128,21 +130,49 @@ public abstract class OfflineCause {
/**
* Taken offline by user.
+ *
* @since 1.551
*/
public static class UserCause extends SimpleOfflineCause {
- private final User user;
+ @Deprecated
+ private transient User user;
+ // null when unknown
+ private /*final*/ @CheckForNull String userId;
- public UserCause(User user, String message) {
- super(hudson.slaves.Messages._SlaveComputer_DisconnectedBy(
- user!=null ? user.getId() : Jenkins.ANONYMOUS.getName(),
+ public UserCause(@CheckForNull User user, @CheckForNull String message) {
+ this(
+ user != null ? user.getId() : null,
message != null ? " : " + message : ""
- ));
- this.user = user;
+ );
+ }
+
+ private UserCause(String userId, String message) {
+ super(hudson.slaves.Messages._SlaveComputer_DisconnectedBy(userId, message));
+ this.userId = userId;
}
public User getUser() {
- return user;
+ return userId == null
+ ? User.getUnknown()
+ : User.getById(userId, true)
+ ;
+ }
+
+ // Storing the User in a filed was a mistake, switch to userId
+ @SuppressWarnings("deprecation")
+ private Object readResolve() throws ObjectStreamException {
+ if (user != null) {
+ String id = user.getId();
+ if (id != null) {
+ userId = id;
+ } else {
+ // The user field is not properly deserialized so id may be missing. Look the user up by fullname
+ User user = User.get(this.user.getFullName(), true, Collections.emptyMap());
+ userId = user.getId();
+ }
+ this.user = null;
+ }
+ return this;
}
}
diff --git a/core/src/main/java/hudson/tools/JDKInstaller.java b/core/src/main/java/hudson/tools/JDKInstaller.java
index 8a42e7e1ca..e313462f11 100644
--- a/core/src/main/java/hudson/tools/JDKInstaller.java
+++ b/core/src/main/java/hudson/tools/JDKInstaller.java
@@ -80,6 +80,7 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static hudson.tools.JDKInstaller.Preference.*;
+import org.kohsuke.stapler.interceptor.RequirePOST;
/**
* Install JDKs from java.sun.com.
@@ -794,7 +795,9 @@ public class JDKInstaller extends ToolInstaller {
/**
* Submits the Oracle account username/password.
*/
+ @RequirePOST
public HttpResponse doPostCredential(@QueryParameter String username, @QueryParameter String password) throws IOException, ServletException {
+ Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
this.username = username;
this.password = Secret.fromString(password);
save();
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 0000000000..37a6fa39e1
--- /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 0cfd9171a9..3c3d4e7980 100644
--- a/core/src/main/java/hudson/util/Secret.java
+++ b/core/src/main/java/hudson/util/Secret.java
@@ -2,6 +2,7 @@
* 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
@@ -30,12 +31,12 @@ import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
import com.trilead.ssh2.crypto.Base64;
import jenkins.util.SystemProperties;
+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;
@@ -45,6 +46,8 @@ 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.
*
@@ -59,15 +62,22 @@ import org.kohsuke.accmod.restrictions.NoExternalUse;
* @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;
- private Secret(String value) {
+ /*package*/ Secret(String value) {
this.value = value;
}
+ /*package*/ Secret(String value, byte[] iv) {
+ this.value = value;
+ this.iv = iv;
+ }
+
/**
* Obtains the secret in a plain text.
*
@@ -101,20 +111,6 @@ public final class Secret implements Serializable {
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.
*
@@ -122,56 +118,95 @@ public final class Secret implements Serializable {
*/
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;
+ }
}
}
@@ -229,8 +264,6 @@ public final class Secret implements Serializable {
}
}
- private static final String MAGIC = "::::MAGIC::::";
-
/**
* Workaround for JENKINS-6459 / http://java.net/jira/browse/GLASSFISH-11862
* @see #getCipher(String)
@@ -247,6 +280,14 @@ public final class Secret implements Serializable {
*/
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 {
diff --git a/core/src/main/java/hudson/util/SecretRewriter.java b/core/src/main/java/hudson/util/SecretRewriter.java
index 0ee05dc869..ac78b66b85 100644
--- a/core/src/main/java/hudson/util/SecretRewriter.java
+++ b/core/src/main/java/hudson/util/SecretRewriter.java
@@ -2,7 +2,6 @@ package hudson.util;
import com.trilead.ssh2.crypto.Base64;
import hudson.model.TaskListener;
-import org.apache.commons.io.FileUtils;
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
@@ -34,21 +33,21 @@ public class SecretRewriter {
*/
private int count;
- /**
- * If non-null the original file before rewrite gets in here.
- */
- private final File backupDirectory;
-
/**
* Canonical paths of the directories we are recursing to protect
* against symlink induced cycles.
*/
private Set callstack = new HashSet();
- public SecretRewriter(File backupDirectory) throws GeneralSecurityException {
+ public SecretRewriter() throws GeneralSecurityException {
cipher = Secret.getCipher("AES");
- key = Secret.getLegacyKey();
- this.backupDirectory = backupDirectory;
+ key = HistoricalSecrets.getLegacyKey();
+ }
+
+ /** @deprecated SECURITY-376: {@code backupDirectory} is ignored */
+ @Deprecated
+ public SecretRewriter(File backupDirectory) throws GeneralSecurityException {
+ this();
}
private String tryRewrite(String s) throws IOException, InvalidKeyException {
@@ -64,19 +63,21 @@ public class SecretRewriter {
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;
}
- /**
- * @param backup
- * if non-null, the original file will be copied here before rewriting.
- * if the rewrite doesn't happen, no copying.
- */
+ /** @deprecated SECURITY-376: {@code backup} is ignored */
+ @Deprecated
public boolean rewrite(File f, File backup) throws InvalidKeyException, IOException {
+ return rewrite(f);
+ }
+
+ public boolean rewrite(File f) throws InvalidKeyException, IOException {
+
AtomicFileWriter w = new AtomicFileWriter(f, "UTF-8");
try {
PrintWriter out = new PrintWriter(new BufferedWriter(w));
@@ -118,10 +119,6 @@ public class SecretRewriter {
}
if (modified) {
- if (backup!=null) {
- backup.getParentFile().mkdirs();
- FileUtils.copyFile(f,backup);
- }
w.commit();
}
return modified;
@@ -166,11 +163,7 @@ public class SecretRewriter {
if ((count++)%100==0)
listener.getLogger().println("Scanning "+child);
try {
- File backup = null;
- if (backupDirectory!=null) backup = new File(backupDirectory,relative+'/'+ cn);
- if (rewrite(child,backup)) {
- if (backup!=null)
- listener.getLogger().println("Copied "+child+" to "+backup+" as a backup");
+ if (rewrite(child)) {
listener.getLogger().println("Rewritten "+child);
rewritten++;
}
@@ -200,7 +193,6 @@ public class SecretRewriter {
String n = dir.getName();
return n.equals("workspace") || n.equals("artifacts")
|| n.equals("plugins") // no mutable data here
- || n.equals("jenkins.security.RekeySecretAdminMonitor") // we don't want to rewrite backups
|| n.equals(".") || n.equals("..");
}
diff --git a/core/src/main/java/jenkins/security/ConfidentialStore.java b/core/src/main/java/jenkins/security/ConfidentialStore.java
index 9434035d2a..78ff2565cf 100644
--- a/core/src/main/java/jenkins/security/ConfidentialStore.java
+++ b/core/src/main/java/jenkins/security/ConfidentialStore.java
@@ -59,7 +59,9 @@ public abstract class ConfidentialStore {
* Retrieves the currently active singleton instance of {@link ConfidentialStore}.
*/
public static @Nonnull ConfidentialStore get() {
- if (TEST!=null) return TEST.get();
+ if (TEST != null) {
+ return TEST;
+ }
Jenkins j = Jenkins.getInstance();
Lookup lookup = j.lookup;
@@ -90,7 +92,7 @@ public abstract class ConfidentialStore {
/**
* Testing only. Used for testing {@link ConfidentialKey} without {@link Jenkins}
*/
- /*package*/ static ThreadLocal TEST = null;
+ /*package*/ static ConfidentialStore TEST = null;
private static final Logger LOGGER = Logger.getLogger(ConfidentialStore.class.getName());
}
diff --git a/core/src/main/java/jenkins/security/CryptoConfidentialKey.java b/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
index dd1dad9e37..f402706013 100644
--- a/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
+++ b/core/src/main/java/jenkins/security/CryptoConfidentialKey.java
@@ -1,9 +1,15 @@
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;
@@ -15,6 +21,9 @@ import java.security.GeneralSecurityException;
* @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);
@@ -35,7 +44,7 @@ public class CryptoConfidentialKey extends ConfidentialKey {
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);
}
}
}
@@ -47,10 +56,12 @@ 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) {
@@ -59,11 +70,67 @@ public class CryptoConfidentialKey extends ConfidentialKey {
}
/**
- * Returns a {@link Cipher} object for decrypting with this key.
+ * Returns a {@link Cipher} object for encrypting with this key using the provided initialization vector.
+ * @param iv the initialization vector
+ * @return the cipher
*/
- public Cipher decrypt() {
+ @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(KEY_ALGORITHM);
cipher.init(Cipher.DECRYPT_MODE, getKey());
return cipher;
} catch (GeneralSecurityException e) {
@@ -72,5 +139,18 @@ public class CryptoConfidentialKey extends ConfidentialKey {
}
- 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/main/java/jenkins/security/RekeySecretAdminMonitor.java b/core/src/main/java/jenkins/security/RekeySecretAdminMonitor.java
index f68268efb8..e29676cf7e 100644
--- a/core/src/main/java/jenkins/security/RekeySecretAdminMonitor.java
+++ b/core/src/main/java/jenkins/security/RekeySecretAdminMonitor.java
@@ -1,6 +1,7 @@
package jenkins.security;
import hudson.Extension;
+import hudson.Util;
import hudson.init.InitMilestone;
import hudson.init.Initializer;
import hudson.model.TaskListener;
@@ -51,6 +52,7 @@ public class RekeySecretAdminMonitor extends AsynchronousAdministrativeMonitor {
*/
private final FileBoolean scanOnBoot = state("scanOnBoot");
+ @SuppressWarnings("OverridableMethodCallInConstructor") // should have been final
public RekeySecretAdminMonitor() throws IOException {
// if JENKINS_HOME existed <1.497, we need to offer rewrite
// this computation needs to be done and the value be captured,
@@ -60,6 +62,7 @@ public class RekeySecretAdminMonitor extends AsynchronousAdministrativeMonitor {
if (j.isUpgradedFromBefore(new VersionNumber("1.496.*"))
&& new FileBoolean(new File(j.getRootDir(),"secret.key.not-so-secret")).isOff())
needed.on();
+ Util.deleteRecursive(new File(getBaseDir(), "backups")); // SECURITY-376: no longer used
}
@Override
@@ -132,7 +135,7 @@ public class RekeySecretAdminMonitor extends AsynchronousAdministrativeMonitor {
protected void fix(TaskListener listener) throws Exception {
LOGGER.info("Initiating a re-keying of secrets. See "+getLogFile());
- SecretRewriter rewriter = new SecretRewriter(new File(getBaseDir(),"backups"));
+ SecretRewriter rewriter = new SecretRewriter();
try {
PrintStream log = listener.getLogger();
diff --git a/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy b/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy
index 9285f0bcdd..b748703ac1 100644
--- a/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy
+++ b/core/src/test/groovy/hudson/util/SecretRewriterTest.groovy
@@ -23,42 +23,49 @@ class SecretRewriterTest {
@Rule public TemporaryFolder tmp = new TemporaryFolder()
+ def FOO_PATTERN = /\{[A-Za-z0-9+\/]+={0,2}}<\/foo>/
+ def MSG_PATTERN = /\{[A-Za-z0-9+\/]+={0,2}}<\/msg>/
+ def FOO_PATTERN2 = /(\{[A-Za-z0-9+\/]+={0,2}}<\/foo>){2}/
+ def ABC_FOO_PATTERN = /\s\{[A-Za-z0-9+\/]+={0,2}}<\/foo>\s<\/abc>/
+
@Test
void singleFileRewrite() {
def o = encryptOld('foobar') // old
def n = encryptNew('foobar') // new
roundtrip "${o}",
- "${n}"
+ {assert it ==~ FOO_PATTERN}
+
roundtrip "${o}${o}",
- "${n}${n}"
+ {assert it ==~ FOO_PATTERN2}
roundtrip "${n}",
- "${n}"
+ {assert it == "${n}"}
roundtrip " thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret ",
- " thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret "
+ {assert it == "thisIsLegalBase64AndLongEnoughThatItCouldLookLikeSecret"}
// to be rewritten, it needs to be between a tag
- roundtrip "$o", "$o"
- roundtrip "$o", "$o"
+ roundtrip "$o", {assert it == "$o"}
+ roundtrip "$o", {assert it == "$o"}
//
- roundtrip "\n$o\n", "\n$n\n"
+ roundtrip "\n$o\n", {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) {
@@ -70,8 +77,7 @@ class SecretRewriterTest {
*/
@Test
void recursionDetection() {
- def backup = tmp.newFolder("backup")
- def sw = new SecretRewriter(backup);
+ def sw = new SecretRewriter();
def st = StreamTaskListener.fromStdout()
def o = encryptOld("Hello world")
@@ -100,12 +106,11 @@ class SecretRewriterTest {
assert 6==sw.rewriteRecursive(t, st)
dirs.each { p->
- assert new File(t,"$p/foo.xml").text.trim()==answer
- assert new File(backup,"$p/foo.xml").text.trim()==payload
+ 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 f7a0ba3ad0..8f39ae7ec0 100644
--- a/core/src/test/groovy/hudson/util/SecretTest.groovy
+++ b/core/src/test/groovy/hudson/util/SecretTest.groovy
@@ -31,7 +31,8 @@ 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
@@ -43,6 +44,8 @@ public class SecretTest {
@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");
@@ -54,6 +57,11 @@ public class SecretTest {
// 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
@@ -62,9 +70,16 @@ public class SecretTest {
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
@@ -77,7 +92,7 @@ public class SecretTest {
def s = Secret.fromString("Mr.Jenkins");
def xml = Jenkins.XSTREAM.toXML(s);
assert !xml.contains(s.plainText)
- assert xml.contains(s.encryptedValue)
+ assert xml ==~ /\{[A-Za-z0-9+\/]+={0,2}}<\/hudson\.util\.Secret>/
def o = Jenkins.XSTREAM.fromXML(xml);
assert o==s : xml;
@@ -104,11 +119,11 @@ public class SecretTest {
*/
@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/core/src/test/java/hudson/console/AnnotatedLargeTextTest.java b/core/src/test/java/hudson/console/AnnotatedLargeTextTest.java
new file mode 100644
index 0000000000..9456b1f398
--- /dev/null
+++ b/core/src/test/java/hudson/console/AnnotatedLargeTextTest.java
@@ -0,0 +1,125 @@
+/*
+ * The MIT License
+ *
+ * Copyright 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.console;
+
+import hudson.MarkupText;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.io.StringWriter;
+import jenkins.security.ConfidentialStoreRule;
+import org.apache.commons.io.Charsets;
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+import org.junit.Test;
+import org.junit.Rule;
+import org.jvnet.hudson.test.For;
+import org.jvnet.hudson.test.Issue;
+import org.kohsuke.stapler.framework.io.ByteBuffer;
+
+@For({AnnotatedLargeText.class, ConsoleNote.class, ConsoleAnnotationOutputStream.class, PlainTextConsoleOutputStream.class})
+public class AnnotatedLargeTextTest {
+
+ @Rule
+ public ConfidentialStoreRule confidentialStoreRule = new ConfidentialStoreRule();
+
+ /* TODO in trunk:
+ @Rule
+ public LoggerRule logging = new LoggerRule().record(ConsoleAnnotationOutputStream.class, Level.FINE).capture(100);
+ */
+
+ @Test
+ public void smokes() throws Exception {
+ ByteBuffer buf = new ByteBuffer();
+ PrintStream ps = new PrintStream(buf, true);
+ ps.print("Some text.\n");
+ ps.print("Go back to " + TestNote.encodeTo("/root", "your home") + ".\n");
+ ps.print("More text.\n");
+ AnnotatedLargeText text = new AnnotatedLargeText<>(buf, Charsets.UTF_8, true, null);
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ text.writeLogTo(0, baos);
+ assertEquals("Some text.\nGo back to your home.\nMore text.\n", baos.toString());
+ StringWriter w = new StringWriter();
+ text.writeHtmlTo(0, w);
+ assertEquals("Some text.\nGo back to your home.\nMore text.\n", w.toString());
+ }
+
+ @Issue("SECURITY-382")
+ @Test
+ public void oldDeserialization() throws Exception {
+ ByteBuffer buf = new ByteBuffer();
+ buf.write(("hello" + ConsoleNote.PREAMBLE_STR + "AAAAwR+LCAAAAAAAAP9dzLEOwVAUxvHThtiNprYxsGiMQhiwNSIhMR/tSZXr3Lr3oJPwPt7FM5hM3gFh8i3/5Bt+1yeUrYH6ap9Yza1Ys9WKWuMiR05wqWhEgpmyEy306Jxvwb19ccGNoBJjLplmgWq0xgOGCjkNZ2IyTrsRlFayVTs4gVMYqP3pw28/JnznuABF/rYWyIyeJfLQe1vxZiDQ7NnYZLn0UZGRRjA9MiV+0OyFv3+utadQyH8B+aJxVM4AAAA=" + ConsoleNote.POSTAMBLE_STR + "there\n").getBytes());
+ AnnotatedLargeText text = new AnnotatedLargeText<>(buf, Charsets.UTF_8, true, null);
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ text.writeLogTo(0, baos);
+ assertEquals("hellothere\n", baos.toString());
+ StringWriter w = new StringWriter();
+ text.writeHtmlTo(0, w);
+ assertEquals("hellothere\n", w.toString());
+ // TODO expect log record with message "Failed to resurrect annotation" and IOException with message "Refusing to deserialize unsigned note from an old log."
+ ConsoleNote.INSECURE = true;
+ try {
+ w = new StringWriter();
+ text.writeHtmlTo(0, w);
+ assertThat(w.toString(), containsString("