Using Google gson vs FasterXML Jackson
Contributors
References
Project Recommendations for Package Upgrades
AAI R3 Security/Vulnerability Threat Analysis
Faster XML Jackson usage in Portal Code and replacing it with Gson
AAI-628: Review security issues: com.fasterxml.jackson.core (CVE-2017-7525)Closed
AAI-908: Security: CVE-2017-7525 jackson-mapper-asl:1.9.13Closed
AAI-910: Security: CVE-2017-7525 jackson-databind:2.2.3Closed
AAI-928: Security: CVE-2017-17485 jackson-databind 2.8.10Closed
AAI-1218: [Champ] Fix JSON serialization of properties in update-notification-raw payloadClosed
Seccom Recommendations
15th July 2019: "Out of Scope" as per Fixing Vulnerabilities in the ONAP Code Base
Jackson Replacement
Security subcommittee has recommended teams move away from jackson, and will be presenting alternatives and asking for an assessment from each project. Our team will need to do an analysis - this would not be trivial, especially given how many of our repos are impacted. As of now, this would be a very high LOE for the team, we need to understand what the recommendation from the SECCOM is before we can provide better details on what the LOE would be.
Three Areas of Concern
Direct usage of Jackson by ONAP code
Frameworks configured with Jackson like Spring Boot
Usage of Jackson by third-party tools like Cassandra
Survey of Replacement Options
Articles with comparisons and benchmarks:
Rationale for eliminating some options from the articles above (about 20 libraries in total):
Related to or derived from Jackson code
Requires change to compilers and compile-time processes
Counter-productive to CII Badging criteria, see also https://github.com/coreinfrastructure/best-practices-badge
Unmaintained in recent years
Vulnerabilities not addressed
"Bus factor" too low
Number of contributors and reviewers too low
Short-list of libraries as reasonable options to be explored, including:
Quick CVE comparison:
https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=fastjson+or+gson+or+moshi+or+genson
https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=fasterxml+or+jackson
Code Analysis
Search on AAI source code shows:
approx 611 hits in 227 files for "fasterxml", which includes pom.xml and Java imports
approx 978 hits in 215 files for "gson", which includes pom.xml and Java imports and initialising Java object
zero hits for "fastjson"
zero hits for "moshi"
zero hits for "genson"
Code Examples
aai\aai-common\aai-auth\src\main\java\org\onap\aaiauth\auth\AuthCore.java
aai\aai-common\aai-core\src\main\java\org\onap\aai\auth\AAIAuthCore.java
FasterXML Jackson example
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
public synchronized void loadUsers(String authFilename) throws Exception {
users = new HashMap<>();
mapper = new ObjectMapper(); // can reuse, share globally
JsonNode rootNode = mapper.readTree(new File(authFilename));
JsonNode rolesNode = rootNode.path(AuthConstants.ROLES_NODE_PATH);
for (JsonNode roleNode : rolesNode) {
String roleName = roleNode.path(AuthConstants.ROLE_NAME_PATH).asText();
AuthRole role = new AuthRole();
JsonNode usersNode = roleNode.path(AuthConstants.USERS_NODE_PATH);
JsonNode functionsNode = roleNode.path(AuthConstants.FUNCTIONS_NODE_PATH);
for (JsonNode functionNode : functionsNode) {
String function = functionNode.path(AuthConstants.FUNCTION_NAME_PATH).asText();
JsonNode methodsNode = functionNode.path(AuthConstants.METHODS_NODE_PATH);
boolean hasMethods = handleMethodNode(methodsNode, role, function);
if (!hasMethods) {
// iterate the list from HTTP_METHODS
for (HTTP_METHODS meth : HTTP_METHODS.values()) {
String thisFunction = meth.toString() + ":" + function;
role.addAllowedFunction(thisFunction);
}
}
}
handleUserNode(usersNode, roleName, role);
}
usersInitialized = true;
}gson example
import com.fasterxml.jackson.core.JsonProcessingException;
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
private synchronized void reloadUsers() {
Map<String, AAIUser> tempUsers = new HashMap<>();
try {
LOGGER.debug("Reading from " + globalAuthFileName);
String authFile = new String(Files.readAllBytes(Paths.get(globalAuthFileName)));
JsonParser parser = new JsonParser();
JsonObject authObject = parser.parse(authFile).getAsJsonObject();
if (authObject.has("roles")) {
JsonArray roles = authObject.getAsJsonArray("roles");
for (JsonElement role : roles) {
if (role.isJsonObject()) {
JsonObject roleObject = role.getAsJsonObject();
String roleName = roleObject.get("name").getAsString();
Map<String, Boolean> usrs = this.getUsernamesFromRole(roleObject);
List<String> aaiFunctions = this.getAAIFunctions(roleObject);
usrs.forEach((key, value) -> {
final AAIUser au = tempUsers.getOrDefault(key, new AAIUser(key, value));
au.addRole(roleName);
aaiFunctions.forEach(f -> {
List<String> httpMethods = this.getRoleHttpMethods(f, roleObject);
httpMethods.forEach(hm -> au.setUserAccess(f, hm));
this.validFunctions.add(f);
});
tempUsers.put(key, au);
});
}
}
if (!tempUsers.isEmpty()) {
users = tempUsers;
}
}
} catch (FileNotFoundException e) {
ErrorLogHelper.logError("AAI_4001", globalAuthFileName + ". Exception: " + e);
} catch (JsonProcessingException e) {
ErrorLogHelper.logError("AAI_4001", globalAuthFileName + ". Not valid JSON: " + e);
} catch (Exception e) {
ErrorLogHelper.logError("AAI_4001", globalAuthFileName + ". Exception caught: " + e);
}
}Side-by-side comparison
FasterXML Jackson version | Google gson version | Comments |
|---|---|---|
mapper = new ObjectMapper(); | JsonParser parser = new JsonParser(); |
|
JsonNode rootNode = mapper.readTree(new File(authFilename));
JsonNode rolesNode = rootNode.path(AuthConstants.ROLES_NODE_PATH); | JsonObject authObject = parser.parse(authFile).getAsJsonObject();
JsonArray roles = authObject.getAsJsonArray("roles"); | Jackson's JsonNode is a more abstract data structure, compared with Gson's more concrete data structures JsonObject and JsonArray. |
String function = functionNode.path(AuthConstants.FUNCTION_NAME_PATH).asText(); | String roleName = roleObject.get("name").getAsString(); | Code structure differs at this point (function name vs role name) but the general intent of the code is equivalent (get the element name as a string). |
public synchronized void loadUsers(String authFilename) throws Exception
(no exception handling in this method) | } catch (JsonProcessingException e) {
ErrorLogHelper.logError("AAI_4001", globalAuthFileName + ". Not valid JSON: " + e); | For some reason, this version still catches com.fasterxml.jackson.core.JsonProcessingException even though it uses Google gson for parsing. Not a good idea to defer exception handling to the caller since the caller has no idea why/how/when/where the parsing failed and might be left with an invalid data structure as well. |
boolean hasMethods = handleMethodNode(methodsNode, role, function); | usrs.forEach((key, value) -> {
...
}); | Method call vs Java lambda call is not really relevant to the Jackson replacement, but consistency of style could be an overall goal if the code is being re-factored anyway. |