[FIXED SECURITY-107] When security-related fields in Jenkins cannot be...

[FIXED SECURITY-107] When security-related fields in Jenkins cannot be unmarshaled, it is best to halt startup.

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

@@ -45,6 +45,7 @@ import hudson.model.Saveable;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -73,6 +74,8 @@ public class RobustReflectionConverter implements Converter {
     protected transient SerializationMethodInvoker serializationMethodInvoker;
     private transient ReflectionProvider pureJavaReflectionProvider;
     private final @Nonnull XStream2.ClassOwnership classOwnership;
+    /** {@code pkg.Clazz#fieldName} */
+    private final Set<String> criticalFields = Collections.synchronizedSet(new HashSet<String>());
 
     public RobustReflectionConverter(Mapper mapper, ReflectionProvider reflectionProvider) {
         this(mapper, reflectionProvider, new XStream2().new PluginClassOwnership());
@@ -85,6 +88,10 @@ public class RobustReflectionConverter implements Converter {
         serializationMethodInvoker = new SerializationMethodInvoker();
     }
 
+    void addCriticalField(Class<?> clazz, String field) {
+        criticalFields.add(clazz.getName() + '#' + field);
+    }
+
     public boolean canConvert(Class type) {
         return true;
     }
@@ -261,8 +268,16 @@ public class RobustReflectionConverter implements Converter {
         while (reader.hasMoreChildren()) {
             reader.moveDown();
 
+            boolean critical = false;
             try {
                 String fieldName = mapper.realMember(result.getClass(), reader.getNodeName());
+                for (Class<?> concrete = result.getClass(); concrete != null; concrete = concrete.getSuperclass()) {
+                    // Not quite right since a subclass could shadow a field, but probably suffices:
+                    if (criticalFields.contains(concrete.getName() + '#' + fieldName)) {
+                        critical = true;
+                        break;
+                    }
+                }
                 boolean implicitCollectionHasSameName = mapper.getImplicitCollectionDefForFieldName(result.getClass(), reader.getNodeName()) != null;
 
                 Class classDefiningField = determineWhichClassDefinesField(reader);
@@ -294,12 +309,21 @@ public class RobustReflectionConverter implements Converter {
                     }
                 }
             } catch (MissingFieldException e) {
+                if (critical) {
+                    throw e;
+                }
                 LOGGER.log(WARNING,"Skipping a non-existent field "+e.getFieldName(),e);
                 addErrorInContext(context, e);
             } catch (CannotResolveClassException e) {
+                if (critical) {
+                    throw e;
+                }
                 LOGGER.log(WARNING,"Skipping a non-existent type",e);
                 addErrorInContext(context, e);
             } catch (LinkageError e) {
+                if (critical) {
+                    throw e;
+                }
                 LOGGER.log(WARNING,"Failed to resolve a type",e);
                 addErrorInContext(context, e);
             }

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

@@ -65,6 +65,8 @@ import java.nio.charset.Charset;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.CheckForNull;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * {@link XStream} enhanced for additional Java5 support and improved robustness.
@@ -121,6 +123,16 @@ public class XStream2 extends XStream {
         return reflectionConverter;
     }
 
+    /**
+     * Specifies that a given field of a given class should not be treated with laxity by {@link RobustCollectionConverter}.
+     * @param clazz a class which we expect to hold a non-{@code transient} field
+     * @param field a field name in that class
+     */
+    @Restricted(NoExternalUse.class) // TODO could be opened up later
+    public void addCriticalField(Class<?> clazz, String field) {
+        reflectionConverter.addCriticalField(clazz, field);
+    }
+
     static String trimVersion(String version) {
         // XXX seems like there should be some trick with VersionNumber to do this
         return version.replaceFirst(" .+$", "");

core/src/main/java/jenkins/model/Jenkins.java

@@ -3985,6 +3985,8 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
         // for backward compatibility with <1.75, recognize the tag name "view" as well.
         XSTREAM.alias("view", ListView.class);
         XSTREAM.alias("listView", ListView.class);
+        XSTREAM2.addCriticalField(Jenkins.class, "securityRealm");
+        XSTREAM2.addCriticalField(Jenkins.class, "authorizationStrategy");
         // this seems to be necessary to force registration of converter early enough
         Mode.class.getEnumConstants();