[WIP] JENKINS-70101 - testing reentrance

pom.xml

@@ -155,6 +155,33 @@
       <artifactId>workflow-basic-steps</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>job-dsl</artifactId>
+        <version>1.81</version>
+        <scope>test</scope>
+    </dependency>
+    <dependency>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>command-launcher</artifactId>
+        <version>1.6</version>
+        <scope>test</scope>
+    </dependency>
+    <dependency>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>credentials-binding</artifactId>
+        <version>523.vd859a_4b_122e6</version>
+        <scope>test</scope>
+    </dependency>
+    <dependency>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>http_request</artifactId>
+        <!-- Note: testCertHttpRequestOnNodeRemote() fails for 1.16
+             unless CertificateCredentialsSnapshotTaker is functional
+         -->
+        <version>1.16</version>
+        <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java

@@ -23,6 +23,7 @@
  */
 package com.cloudbees.plugins.credentials.impl;
 
+import com.cloudbees.plugins.credentials.CredentialsProvider;
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import com.cloudbees.plugins.credentials.SecretBytes;
 import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials;
@@ -34,6 +35,7 @@
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import hudson.model.Items;
+import hudson.remoting.Channel;
 import hudson.util.FormValidation;
 import hudson.util.Secret;
 import java.io.ByteArrayInputStream;
@@ -59,6 +61,7 @@
 import java.util.logging.Logger;
 
 import jenkins.model.Jenkins;
+import jenkins.util.JenkinsJVM;
 import net.jcip.annotations.GuardedBy;
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.lang.StringUtils;
@@ -138,6 +141,21 @@ private static char[] toCharArray(@NonNull Secret password) {
         return plainText == null ? null : plainText.toCharArray();
     }
 
+    /**
+     * When serializing over a {@link Channel} ensure that we send a self-contained version.
+     *
+     * @return the object instance to write to the stream.
+     */
+    private Object writeReplace() {
+        if (/* XStream */ Channel.current() == null
+        ||  /* already safe to serialize */ keyStoreSource
+                .isSnapshotSource()
+        ) {
+            return this;
+        }
+        return CredentialsProvider.snapshot(this);
+    }
+
     /**
      * Returns the {@link KeyStore} containing the certificate.
      *
@@ -414,17 +432,18 @@ public static class UploadedKeyStoreSource extends KeyStoreSource implements Ser
 
         /**
          * The old uploaded keystore.
+         * Still used for snapshot taking, with contents independent of Jenkins instance and JVM.
          */
         @CheckForNull
         @Deprecated
-        private transient Secret uploadedKeystore;
+        private Secret uploadedKeystore;
         /**
          * The uploaded keystore.
          *
          * @since 2.1.5
          */
         @CheckForNull
-        private final SecretBytes uploadedKeystoreBytes;
+        private SecretBytes uploadedKeystoreBytes;
 
         /**
          * Our constructor.
@@ -452,6 +471,19 @@ public UploadedKeyStoreSource(@CheckForNull SecretBytes uploadedKeystore) {
             this.uploadedKeystoreBytes = uploadedKeystore;
         }
 
+        /**
+         * Our constructor for serialization (e.g. to remote agents, whose SecretBytes
+         * in another JVM use a different static KEY); would re-encode.
+         *
+         * @param uploadedKeystore the keystore content.
+         * @deprecated
+         */
+        @SuppressWarnings("unused") // by stapler
+        @Deprecated
+        public UploadedKeyStoreSource(@CheckForNull Secret uploadedKeystore) {
+            this.uploadedKeystore = uploadedKeystore;
+        }
+
         /**
          * Constructor able to receive file directly
          * 
@@ -470,6 +502,23 @@ public UploadedKeyStoreSource(FileItem uploadedCertFile, @CheckForNull SecretByt
             this.uploadedKeystoreBytes = uploadedKeystore;
         }
 
+        /**
+         * Request that if the less-efficient but more-portable Secret
+         * is involved (e.g. to cross the remoting gap to another JVM),
+         * we use the more secure and efficient SecretBytes.
+         */
+        public void useSecretBytes() {
+            if (this.uploadedKeystore != null && this.uploadedKeystoreBytes == null) {
+                String msg = "[INFOLOG] UploadedKeyStoreSource instance was using Secret and fixed to use SecretBytes";
+                LOGGER.log(Level.SEVERE, msg);
+                LOGGER.log(Level.INFO, msg);
+                System.out.println (msg);
+                System.err.println (msg);
+                this.uploadedKeystoreBytes = SecretBytes.fromBytes(DescriptorImpl.toByteArray(this.uploadedKeystore));
+                this.uploadedKeystore = null;
+            }
+        }
+
         /**
          * Migrate to the new field.
          *
@@ -485,11 +534,28 @@ private Object readResolve() throws ObjectStreamException {
         }
 
         /**
-         * Returns the private key file name.
+         * Returns the private key + certificate file bytes.
          *
-         * @return the private key file name.
+         * @return the private key + certificate file bytes.
          */
         public SecretBytes getUploadedKeystore() {
+            if (uploadedKeystore != null && uploadedKeystoreBytes == null) {
+                String msg = "[INFOLOG] UploadedKeyStoreSource instance using Secret:" +
+                        " in Channel: " + (Channel.current() == null ? "false" : "true") +
+                        " on controller: " + (JenkinsJVM.isJenkinsJVM() ? "true" : "false");
+                LOGGER.log(Level.SEVERE, msg);
+                LOGGER.log(Level.INFO, msg);
+                System.out.println (msg);
+                System.err.println (msg);
+                if (/* XStream */ Channel.current() != null
+                &&  /* remote  */!(JenkinsJVM.isJenkinsJVM())
+                ) {
+                    // Force recoding on first read so the remote copy is secure
+                    useSecretBytes();
+                } else {
+                    return SecretBytes.fromBytes(DescriptorImpl.toByteArray(uploadedKeystore));
+                }
+            }
             return uploadedKeystoreBytes;
         }
 
@@ -499,6 +565,23 @@ public SecretBytes getUploadedKeystore() {
         @NonNull
         @Override
         public byte[] getKeyStoreBytes() {
+            if (uploadedKeystore != null && uploadedKeystoreBytes == null) {
+                String msg = "[INFOLOG] UploadedKeyStoreSource instance using Secret:" +
+                        " in Channel: " + (Channel.current() == null ? "false" : "true") +
+                        " on controller: " + (JenkinsJVM.isJenkinsJVM() ? "true" : "false");
+                LOGGER.log(Level.SEVERE, msg);
+                LOGGER.log(Level.INFO, msg);
+                System.err.println (msg);
+                System.out.println (msg);
+                if (/* XStream */ Channel.current() != null
+                &&  /* remote  */!(JenkinsJVM.isJenkinsJVM())
+                ) {
+                    // Force recoding on first read so the remote copy is secure
+                    useSecretBytes();
+                } else {
+                    return DescriptorImpl.toByteArray(uploadedKeystore);
+                }
+            }
             return SecretBytes.getPlainData(uploadedKeystoreBytes);
         }
 
@@ -515,7 +598,11 @@ public long getKeyStoreLastModified() {
          */
         @Override
         public boolean isSnapshotSource() {
-            return true;
+            //return this.snapshotSecretBytes;
+            // If context is local, clone SecretBytes directly (only
+            // usable in same JVM). Otherwise use Secret for transport
+            // (see {@link CertificateCredentialsSnapshotTaker}.
+            return (/* XStream */ Channel.current() == null);
         }
 
         /**

src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsSnapshotTaker.java

@@ -0,0 +1,97 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2011-2016, CloudBees, Inc., Stephen Connolly.
+ *
+ * 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 com.cloudbees.plugins.credentials.impl;
+
+import com.cloudbees.plugins.credentials.CredentialsSnapshotTaker;
+import com.cloudbees.plugins.credentials.SecretBytes;
+import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials;
+import hudson.Extension;
+import hudson.util.Secret;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.util.Arrays;
+
+/**
+ * The {@link CredentialsSnapshotTaker} for {@link StandardCertificateCredentials}.
+ * Taking a snapshot of the credential ensures that all the details are captured
+ * within the credential.
+ *
+ * @since 1.14
+ *
+ * Historic note: This code was dropped from {@link CertificateCredentialsImpl}
+ * codebase along with most of FileOnMasterKeyStoreSource (deprecated and headed
+ * towards eventual deletion) due to SECURITY-1322, see more details at
+ * https://www.jenkins.io/security/advisory/2019-05-21/
+ * In hind-sight, this snapshot taker was needed to let the
+ * {@link CertificateCredentialsImpl.UploadedKeyStoreSource} be used
+ * on remote agents.
+ */
+@Extension
+public class CertificateCredentialsSnapshotTaker extends CredentialsSnapshotTaker<StandardCertificateCredentials> {
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public Class<StandardCertificateCredentials> type() {
+        return StandardCertificateCredentials.class;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public StandardCertificateCredentials snapshot(StandardCertificateCredentials credentials) {
+        if (credentials instanceof CertificateCredentialsImpl) {
+            final CertificateCredentialsImpl.KeyStoreSource keyStoreSource = ((CertificateCredentialsImpl) credentials).getKeyStoreSource();
+            if (keyStoreSource.isSnapshotSource()) {
+                return credentials;
+            }
+            return new CertificateCredentialsImpl(credentials.getScope(), credentials.getId(),
+                    credentials.getDescription(), credentials.getPassword().getEncryptedValue(),
+                    new CertificateCredentialsImpl.UploadedKeyStoreSource(CertificateCredentialsImpl.UploadedKeyStoreSource.DescriptorImpl.toSecret(keyStoreSource.getKeyStoreBytes())));
+        }
+
+        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        final char[] password = credentials.getPassword().getPlainText().toCharArray();
+        try {
+            credentials.getKeyStore().store(bos, password);
+            bos.close();
+        } catch (KeyStoreException | IOException | NoSuchAlgorithmException | CertificateException e) {
+            return credentials; // as-is
+        } finally {
+            Arrays.fill(password, (char) 0);
+        }
+
+        return new CertificateCredentialsImpl(credentials.getScope(), credentials.getId(),
+                credentials.getDescription(), credentials.getPassword().getEncryptedValue(),
+                new CertificateCredentialsImpl.UploadedKeyStoreSource(CertificateCredentialsImpl.UploadedKeyStoreSource.DescriptorImpl.toSecret(bos.toByteArray())));
+    }
+}