From f3b867725ddeeca3e5f5f815d34784fc8d97e9da Mon Sep 17 00:00:00 2001 From: lovesh-ap Date: Tue, 1 Oct 2024 11:51:02 +0530 Subject: [PATCH] Fix header key generation, use set to store parsed params, disable entity expansion --- .../utils/RestrictionUtility.java | 84 ++++++++++--------- .../agent/security/schema/HttpRequest.java | 24 +++--- 2 files changed, 58 insertions(+), 50 deletions(-) diff --git a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/intcodeagent/utils/RestrictionUtility.java b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/intcodeagent/utils/RestrictionUtility.java index b53d39ced..f0a0c4518 100644 --- a/newrelic-security-agent/src/main/java/com/newrelic/agent/security/intcodeagent/utils/RestrictionUtility.java +++ b/newrelic-security-agent/src/main/java/com/newrelic/agent/security/intcodeagent/utils/RestrictionUtility.java @@ -72,13 +72,13 @@ public static boolean hasValidAccountId(RestrictionCriteria restrictionCriteria, } if(restrictionCriteria.getMappingParameters().getHeader().isEnabled()) { - List headerValues = getHeaderParameters(restrictionCriteria.getMappingParameters().getHeader().getLocations(), request.getRequestHeaderParameters()); + Set headerValues = getHeaderParameters(restrictionCriteria.getMappingParameters().getHeader().getLocations(), request.getRequestHeaderParameters()); if(matcher(accountIds, headerValues)){ return true; } } if(restrictionCriteria.getMappingParameters().getQuery().isEnabled()) { - List queryValues = getQueryString(restrictionCriteria.getMappingParameters().getHeader().getLocations(), request.getQueryParameters()); + Set queryValues = getQueryString(restrictionCriteria.getMappingParameters().getHeader().getLocations(), request.getQueryParameters()); if(matcher(accountIds, queryValues)){ return true; } @@ -89,19 +89,19 @@ public static boolean hasValidAccountId(RestrictionCriteria restrictionCriteria, } } if(restrictionCriteria.getMappingParameters().getBody().isEnabled()) { - List bodyValues = getBodyParameters(restrictionCriteria.getMappingParameters().getBody().getLocations(), request.getRequestBodyParameters()); + Set bodyValues = getBodyParameters(restrictionCriteria.getMappingParameters().getBody().getLocations(), request.getRequestBodyParameters()); return matcher(accountIds, bodyValues); } return false; } - private static List getBodyParameters(List accountIds, Map> requestBodyParameters) { + private static Set getBodyParameters(List accountIds, Map> requestBodyParameters) { if (requestBodyParameters == null || requestBodyParameters.isEmpty()) { - return Collections.emptyList(); + return Collections.emptySet(); } - List values = new ArrayList<>(); + Set values = new HashSet<>(); for (String accountId : accountIds) { String lowerCaseAccountId = accountId.toLowerCase(); if(requestBodyParameters.containsKey(lowerCaseAccountId)) { @@ -112,11 +112,11 @@ private static List getBodyParameters(List accountIds, Map getHeaderParameters(List accountIds, Map> requestHeaderParameters) { + private static Set getHeaderParameters(List accountIds, Map> requestHeaderParameters) { if (requestHeaderParameters == null || requestHeaderParameters.isEmpty()) { - return Collections.emptyList(); + return Collections.emptySet(); } - List values = new ArrayList<>(); + Set values = new HashSet<>(); for (String accountId : accountIds) { String lowerCaseAccountId = accountId.toLowerCase(); if(requestHeaderParameters.containsKey(lowerCaseAccountId)) { @@ -126,11 +126,11 @@ private static List getHeaderParameters(List accountIds, Map getQueryString(List accountIds, Map> queryParameters) { + private static Set getQueryString(List accountIds, Map> queryParameters) { if(queryParameters == null || queryParameters.isEmpty()) { - return Collections.emptyList(); + return Collections.emptySet(); } - List values = new ArrayList<>(); + Set values = new HashSet<>(); for (String accountId : accountIds) { String lowerCaseAccountId = accountId.toLowerCase(); if(queryParameters.containsKey(lowerCaseAccountId)) { @@ -140,7 +140,7 @@ private static List getQueryString(List accountIds, Map accountIds, List values) { + private static boolean matcher(List accountIds, Set values) { for (String accountId : accountIds) { if(values == null || values.isEmpty() || StringUtils.isBlank(accountId)) { continue; @@ -165,10 +165,10 @@ private static void parseHttpRequestParameters(HttpRequest request) { logger.log(LogLevel.WARNING, String.format("Request Body parsing failed reason %s", e.getMessage()), RestrictionUtility.class.getName()); } request.setRequestBodyParameters(parseRequestParameterMap(request.getParameterMap(), request.getRequestBodyParameters())); - request.setRequestParsed(true); + request.setRequestParametersParsed(true); } - private static Map> parseRequestParameterMap(Map parameterMap, Map> requestBodyParameters) { + private static Map> parseRequestParameterMap(Map parameterMap, Map> requestBodyParameters) { if(parameterMap == null) { return requestBodyParameters; } @@ -179,7 +179,7 @@ private static Map> parseRequestParameterMap(Map entry : parameterMap.entrySet()) { String key = entry.getKey(); String[] values = entry.getValue(); - List valuesList = new ArrayList<>(); + Set valuesList = new HashSet<>(); for (String value : values) { valuesList.add(StringUtils.lowerCase(value)); } @@ -192,7 +192,7 @@ private static Map> parseRequestParameterMap(Map> parseRequestBody(StringBuilder body, String contentType, Map> requestBodyParameters) throws RestrictionModeException { + private static Map> parseRequestBody(StringBuilder body, String contentType, Map> requestBodyParameters) throws RestrictionModeException { if(StringUtils.isBlank(body.toString())) { return requestBodyParameters; } @@ -220,11 +220,12 @@ private static Map> parseRequestBody(StringBuilder body, St } - private static Map> parseXmlRequestBody(String body) throws RestrictionModeException { + private static Map> parseXmlRequestBody(String body) throws RestrictionModeException { //write logic to xml parsing - Map> requestBodyParameters = new HashMap<>(); + Map> requestBodyParameters = new HashMap<>(); try { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setExpandEntityReferences(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document document = builder.parse(new InputSource(new StringReader(body))); document.getDocumentElement().normalize(); @@ -237,7 +238,7 @@ private static Map> parseXmlRequestBody(String bod return requestBodyParameters; } - private static void parseXmlNode(Node node, String baseKey, Map> requestBodyParameters) { + private static void parseXmlNode(Node node, String baseKey, Map> requestBodyParameters) { if (node.getNodeType() == Node.ELEMENT_NODE) { Element element = (Element) node; NodeList children = element.getChildNodes(); @@ -245,7 +246,7 @@ private static void parseXmlNode(Node node, String baseKey, Map new ArrayList<>()).add(value); + requestBodyParameters.computeIfAbsent(key, k -> new HashSet<>()).add(value); } } else { for (int i = 0; i < children.getLength(); i++) { @@ -255,12 +256,12 @@ private static void parseXmlNode(Node node, String baseKey, Map> parseJsonRequestBody(String body) throws RestrictionModeException { + private static Map> parseJsonRequestBody(String body) throws RestrictionModeException { JsonNode node; ObjectMapper mapper = new ObjectMapper(); try { node = mapper.readValue(body, JsonNode.class); - Map> requestBodyParameters = new HashMap<>(); + Map> requestBodyParameters = new HashMap<>(); return parseJsonNode(node, StringUtils.EMPTY, requestBodyParameters); } catch (JsonProcessingException e) { logger.log(LogLevel.FINER, String.format("JSON Request Body parsing failed for %s : reason %s", body, e.getMessage()), RestrictionUtility.class.getName()); @@ -268,7 +269,7 @@ private static Map> parseJsonRequestBody(String bo } } - private static Map> parseJsonNode(JsonNode node, String baseKey, Map> requestBodyParameters) { + private static Map> parseJsonNode(JsonNode node, String baseKey, Map> requestBodyParameters) { if (node.isObject()) { Iterator> iterator = node.fields(); while (iterator.hasNext()) { @@ -280,7 +281,7 @@ private static Map> parseJsonNode(JsonNode node, String bas parseJsonNode(value, base, requestBodyParameters); } else if (StringUtils.isNotBlank(value.asText())) { if(!requestBodyParameters.containsKey(base)){ - requestBodyParameters.put(base, new ArrayList<>()); + requestBodyParameters.put(base, new HashSet<>()); } requestBodyParameters.get(base).add(value.asText()); } @@ -294,7 +295,7 @@ private static Map> parseJsonNode(JsonNode node, String bas parseJsonNode(jsonNode, base, requestBodyParameters); } else if (StringUtils.isNotBlank(jsonNode.asText())) { if(!requestBodyParameters.containsKey(base)){ - requestBodyParameters.put(base, new ArrayList<>()); + requestBodyParameters.put(base, new HashSet<>()); } requestBodyParameters.get(base).add(jsonNode.asText()); } @@ -317,8 +318,8 @@ private static Map> parseJsonNode(JsonNode node, String bas return String.format("%s[]", baseKey); } - private static Map> parseRequestHeaders(Map headers) { - Map> requestHeaderParameters = new HashMap<>(); + private static Map> parseRequestHeaders(Map headers) { + Map> requestHeaderParameters = new HashMap<>(); for (Map.Entry header : headers.entrySet()) { String key = header.getKey(); String value = header.getValue(); @@ -330,7 +331,7 @@ private static Map> parseRequestHeaders(Map && !StringUtils.endsWith(headerKeyValues[i], EQUAL)) { String headerKey = StringUtils.substringBefore(headerKeyValues[i], EQUAL).trim(); String headerValue = StringUtils.substringAfter(headerKeyValues[i], EQUAL).trim(); - putHeaderParameter(headerKey, headerValue, requestHeaderParameters); + putHeaderParameter(key, headerKey, headerValue, requestHeaderParameters); } else { putHeaderParameter(key, headerKeyValues[i], requestHeaderParameters); } @@ -340,18 +341,25 @@ private static Map> parseRequestHeaders(Map return requestHeaderParameters; } - private static void putHeaderParameter(String key, String value, Map> requestHeaderParameters) { - List headerValues = requestHeaderParameters.get(key); + private static void putHeaderParameter(String baseKey, String headerKey, String headerValue, Map> requestHeaderParameters) { + if(StringUtils.isBlank(baseKey)){ + putHeaderParameter(headerKey, headerValue, requestHeaderParameters); + } + putHeaderParameter(String.format("%s.%s", baseKey, headerKey), headerValue, requestHeaderParameters); + } + + private static void putHeaderParameter(String key, String value, Map> requestHeaderParameters) { + Set headerValues = requestHeaderParameters.get(key); if (headerValues == null) { - headerValues = new ArrayList<>(); + headerValues = new HashSet<>(); } headerValues.add(StringUtils.lowerCase(value)); headerValues.add(StringUtils.lowerCase(ServletHelper.urlDecode(value))); requestHeaderParameters.put(StringUtils.lowerCase(key), headerValues); } - private static Map> parseQueryParameters(String url) { - Map> queryParameters = new HashMap<>(); + private static Map> parseQueryParameters(String url) { + Map> queryParameters = new HashMap<>(); String query = StringUtils.substringAfter(url, SEPARATOR_CHARS_QUESTION_MARK); if (StringUtils.isNotBlank(query)) { queryParamKeyValueGenerator(query, queryParameters); @@ -364,13 +372,13 @@ private static Map> parseQueryParameters(String url) { return queryParameters; } - private static Map> queryParamKeyValueGenerator(String query, Map> queryParameters) { + private static Map> queryParamKeyValueGenerator(String query, Map> queryParameters) { String[] queryParams = StringUtils.split(query, AND); for (String queryParam : queryParams) { String key, value; key = StringUtils.substringBefore(queryParam, SEPARATOR_EQUALS); value = StringUtils.substringAfter(queryParam, SEPARATOR_EQUALS); - List values = new ArrayList<>(); + Set values = new HashSet<>(); values.add(StringUtils.lowerCase(value)); values.add(StringUtils.lowerCase(ServletHelper.urlDecode(value))); queryParameters.put(StringUtils.lowerCase(key), values); @@ -378,8 +386,8 @@ private static Map> queryParamKeyValueGenerator(String quer return queryParameters; } - private static List parsePathParameters(String uri) { - List pathParameters = new ArrayList<>(); + private static Set parsePathParameters(String uri) { + Set pathParameters = new HashSet<>(); String requestPath = StringUtils.substringBefore(uri, SEPARATOR_CHARS_SEMICOLON); if(StringUtils.isNotBlank(requestPath)) { diff --git a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/HttpRequest.java b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/HttpRequest.java index 1910758bd..db73b983d 100644 --- a/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/HttpRequest.java +++ b/newrelic-security-api/src/main/java/com/newrelic/api/agent/security/schema/HttpRequest.java @@ -44,16 +44,16 @@ public class HttpRequest { private Map customDataType; @JsonIgnore - private List pathParameters; + private Set pathParameters; @JsonIgnore - private Map> queryParameters; + private Map> queryParameters; @JsonIgnore - private Map> requestHeaderParameters; + private Map> requestHeaderParameters; @JsonIgnore - private Map> requestBodyParameters; + private Map> requestBodyParameters; @JsonIgnore private boolean isRequestParametersParsed = false; @@ -260,35 +260,35 @@ public void setRoute(String segment, boolean isAlreadyServlet) { } } - public List getPathParameters() { + public Set getPathParameters() { return pathParameters; } - public void setPathParameters(List pathParameters) { + public void setPathParameters(Set pathParameters) { this.pathParameters = pathParameters; } - public Map> getQueryParameters() { + public Map> getQueryParameters() { return queryParameters; } - public void setQueryParameters(Map> queryParameters) { + public void setQueryParameters(Map> queryParameters) { this.queryParameters = queryParameters; } - public Map> getRequestHeaderParameters() { + public Map> getRequestHeaderParameters() { return requestHeaderParameters; } - public void setRequestHeaderParameters(Map> requestHeaderParameters) { + public void setRequestHeaderParameters(Map> requestHeaderParameters) { this.requestHeaderParameters = requestHeaderParameters; } - public Map> getRequestBodyParameters() { + public Map> getRequestBodyParameters() { return requestBodyParameters; } - public void setRequestBodyParameters(Map> requestBodyParameters) { + public void setRequestBodyParameters(Map> requestBodyParameters) { this.requestBodyParameters = requestBodyParameters; }