use central method to setup Jackson in locked down setup

This is based on @pserwlyo's work.  The App and Apk classes currently
need just the public instance variables auto-filled by Jackson, so
everything else is considered opt-in, via @JsonProperty declarations.
This is currently only used for setLocalized(), setUsesPermission(),
and setUsesPermissionSdk23().

# Conflicts:
#	app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
This commit is contained in:
Hans-Christoph Steiner 2017-03-22 21:47:47 +01:00 committed by Peter Serwylo
parent 2a2e475bdc
commit 28bcbc548a
2 changed files with 45 additions and 12 deletions

View File

@ -9,6 +9,8 @@ import android.support.v4.content.LocalBroadcastManager;
import android.text.TextUtils; import android.text.TextUtils;
import android.util.Log; import android.util.Log;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.PropertyAccessor;
import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.core.type.TypeReference;
@ -46,6 +48,12 @@ import java.util.jar.JarFile;
* by Jackson. This is possible but not wise to do with {@link Repo} since that * by Jackson. This is possible but not wise to do with {@link Repo} since that
* class has many fields that are related to security components of the * class has many fields that are related to security components of the
* implementation internal to this app. * implementation internal to this app.
* <p>
* All non-{@code public} fields and fields tagged with {@code @JsonIgnore} are
* ignored. All methods are ignored unless they are tagged with {@code @JsonProperty}.
* This setup prevents the situation where future developers add variables to the
* App/Apk classes, resulting in malicious servers being able to populate those
* variables.
*/ */
public class IndexV1Updater extends RepoUpdater { public class IndexV1Updater extends RepoUpdater {
public static final String TAG = "IndexV1Updater"; public static final String TAG = "IndexV1Updater";
@ -121,6 +129,22 @@ public class IndexV1Updater extends RepoUpdater {
return true; return true;
} }
/**
* Get the standard {@link ObjectMapper} instance used for parsing {@code index-v1.json}.
* This ignores unknown properties so that old releases won't crash when new things are
* added to {@code index-v1.json}. This is required for both forward compatibility,
* but also because ignoring such properties when coming from a malicious server seems
* reasonable anyway.
*/
public static ObjectMapper getObjectMapperInstance(long repoId) {
ObjectMapper mapper = new ObjectMapper();
mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
mapper.setInjectableValues(new InjectableValues.Std().addValue("repoId", repoId));
mapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE);
mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.PUBLIC_ONLY);
return mapper;
}
/** /**
* Parses the index and feeds it to the database via {@link Repo}, {@link App}, * Parses the index and feeds it to the database via {@link Repo}, {@link App},
* and {@link Apk} instances. * and {@link Apk} instances.
@ -132,9 +156,7 @@ public class IndexV1Updater extends RepoUpdater {
*/ */
public void processIndexV1(InputStream indexInputStream, JarEntry indexEntry, String cacheTag) public void processIndexV1(InputStream indexInputStream, JarEntry indexEntry, String cacheTag)
throws IOException, UpdateException { throws IOException, UpdateException {
ObjectMapper mapper = new ObjectMapper(); ObjectMapper mapper = getObjectMapperInstance(repo.getId());
mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
mapper.setInjectableValues(new InjectableValues.Std().addValue("repoId", repo.getId()));
JsonFactory f = mapper.getFactory(); JsonFactory f = mapper.getFactory();
JsonParser parser = f.createParser(indexInputStream); JsonParser parser = f.createParser(indexInputStream);
HashMap<String, Object> repoMap = null; HashMap<String, Object> repoMap = null;

View File

@ -7,7 +7,6 @@ import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.ObjectReader;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
@ -129,10 +128,9 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
@Test @Test
public void testJacksonParsing() throws IOException { public void testJacksonParsing() throws IOException {
ObjectMapper mapper = new ObjectMapper(); ObjectMapper mapper = IndexV1Updater.getObjectMapperInstance(FAKE_REPO_ID);
// the app ignores all unknown fields when complete, do not ignore during dev to catch mistakes // the app ignores all unknown fields when complete, do not ignore during dev to catch mistakes
mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES); mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
mapper.setInjectableValues(new InjectableValues.Std().addValue("repoId", FAKE_REPO_ID));
JsonFactory f = mapper.getFactory(); JsonFactory f = mapper.getFactory();
JsonParser parser = f.createParser(TestUtils.copyResourceToTempFile("guardianproject_index-v1.json")); JsonParser parser = f.createParser(TestUtils.copyResourceToTempFile("guardianproject_index-v1.json"));
@ -199,7 +197,10 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
* index parsing, Jackson is always reading JSON into {@link App} and {@link Apk} * index parsing, Jackson is always reading JSON into {@link App} and {@link Apk}
* instances. {@code @JsonIgnoreProperties} applies to both directions. * instances. {@code @JsonIgnoreProperties} applies to both directions.
* <p> * <p>
* The test sets come from the top of {@link App} and {@link Apk}. * If this test fails for you, then it probably means you are adding a new instance
* variable to {@link App}. In that case, you need to add it to the appropriate
* list here. If it is allowed, make sure this new instance variable is in sync with
* {@code index-v1.json} and {@code fdroidserver}.
*/ */
@Test @Test
public void testJsonIgnoreApp() throws JsonProcessingException { public void testJsonIgnoreApp() throws JsonProcessingException {
@ -256,6 +257,19 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
runJsonIgnoreTest(new App(), allowedInApp, ignoredInApp); runJsonIgnoreTest(new App(), allowedInApp, ignoredInApp);
} }
/**
* Test that all the fields are properly marked as whether Jackson should ignore them
* or not. Technically this test goes the opposite direction of how its being used:
* it writes out {@link App} and {@link Apk} instances to JSON using Jackson. With the
* index parsing, Jackson is always reading JSON into {@link App} and {@link Apk}
* instances. {@code @JsonIgnoreProperties} applies to both directions.
* <p>
* If this test fails for you, then it probably means you are adding a new instance
* variable to {@link Apk}. In that case, you need to add it to the appropriate
* list here. If it is allowed, make sure this new instance variable is in sync with
* {@code index-v1.json} and {@code fdroidserver}.
*/
@Test @Test
public void testJsonIgnoreApk() throws JsonProcessingException { public void testJsonIgnoreApk() throws JsonProcessingException {
String[] allowedInApk = new String[]{ String[] allowedInApk = new String[]{
@ -299,7 +313,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
private void runJsonIgnoreTest(Object instance, String[] allowed, String[] ignored) private void runJsonIgnoreTest(Object instance, String[] allowed, String[] ignored)
throws JsonProcessingException { throws JsonProcessingException {
ObjectMapper mapper = new ObjectMapper(); ObjectMapper mapper = IndexV1Updater.getObjectMapperInstance(FAKE_REPO_ID);
String objectAsString = mapper.writeValueAsString(instance); String objectAsString = mapper.writeValueAsString(instance);
Set<String> fields = getFields(instance); Set<String> fields = getFields(instance);
for (String field : ignored) { for (String field : ignored) {
@ -321,10 +335,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
@Test @Test
public void testInstanceVariablesAreProperlyMarked() throws IOException { public void testInstanceVariablesAreProperlyMarked() throws IOException {
ObjectMapper mapper = new ObjectMapper(); ObjectMapper mapper = IndexV1Updater.getObjectMapperInstance(FAKE_REPO_ID);
// testing with unknown metadata in it
mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
mapper.setInjectableValues(new InjectableValues.Std().addValue("repoId", FAKE_REPO_ID));
JsonFactory f = mapper.getFactory(); JsonFactory f = mapper.getFactory();
JsonParser parser = f.createParser(TestUtils.copyResourceToTempFile("all_fields_index-v1.json")); JsonParser parser = f.createParser(TestUtils.copyResourceToTempFile("all_fields_index-v1.json"));