aboutsummaryrefslogtreecommitdiffstats
path: root/policy-management
diff options
context:
space:
mode:
authorjhh <jorge.hernandez-herrero@att.com>2021-05-12 15:08:04 -0500
committerjhh <jorge.hernandez-herrero@att.com>2021-05-12 15:08:04 -0500
commit20241c950c0e69ecbe10eb26f89cebb2d0ef91e9 (patch)
tree47e8d60db9fbd9b863569cf941acacc3159250d4 /policy-management
parent61b27e147b5d8d816488b3a5a5dc1d57a1539186 (diff)
fix telemetry related sonar security issues
Issue-ID: POLICY-3257 Signed-off-by: jhh <jorge.hernandez-herrero@att.com> Change-Id: Ic599b593abbc999c4e6a6fd4bc72acd5ec6e09f9
Diffstat (limited to 'policy-management')
-rw-r--r--policy-management/src/main/java/org/onap/policy/drools/protocol/coders/GenericEventProtocolCoder.java28
-rw-r--r--policy-management/src/main/java/org/onap/policy/drools/protocol/coders/MultiplexorEventProtocolCoder.java26
-rw-r--r--policy-management/src/main/java/org/onap/policy/drools/server/restful/RestManager.java17
3 files changed, 56 insertions, 15 deletions
diff --git a/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/GenericEventProtocolCoder.java b/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/GenericEventProtocolCoder.java
index b16186d6..5faf8a86 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/GenericEventProtocolCoder.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/GenericEventProtocolCoder.java
@@ -2,7 +2,7 @@
* ============LICENSE_START=======================================================
* ONAP
* ================================================================================
- * Copyright (C) 2019-2020 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2019-2020, 2021 AT&T Intellectual Property. All rights reserved.
* ================================================================================
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -43,7 +43,7 @@ abstract class GenericEventProtocolCoder {
private static final String UNSUPPORTED_EX_MSG = "Unsupported:";
private static final String MISSING_CLASS = "class must be provided";
- private static Logger logger = LoggerFactory.getLogger(GenericEventProtocolCoder.class);
+ private static final Logger logger = LoggerFactory.getLogger(GenericEventProtocolCoder.class);
/**
* Mapping topic:controller-id -> /<protocol-decoder-toolset/> where protocol-coder-toolset contains
@@ -64,9 +64,6 @@ abstract class GenericEventProtocolCoder {
/**
* Index a new coder.
- *
- * @param eventProtocolParams parameter object for event encoder
- * @throw IllegalArgumentException if an invalid parameter is passed
*/
public void add(EventProtocolParams eventProtocolParams) {
if (eventProtocolParams.getGroupId() == null || eventProtocolParams.getGroupId().isEmpty()) {
@@ -117,8 +114,7 @@ abstract class GenericEventProtocolCoder {
return;
}
- GsonProtocolCoderToolset coderTools =
- new GsonProtocolCoderToolset(eventProtocolParams, key);
+ var coderTools = new GsonProtocolCoderToolset(eventProtocolParams, key);
logger.info("{}: adding coders for new {}: {}", this, key, coderTools);
@@ -135,7 +131,7 @@ abstract class GenericEventProtocolCoder {
List<ProtocolCoderToolset> toolsets =
reverseCoders.get(reverseKey);
- boolean present = false;
+ var present = false;
for (ProtocolCoderToolset parserSet : toolsets) {
// just doublecheck
present = parserSet.getControllerId().equals(key);
@@ -402,7 +398,19 @@ abstract class GenericEventProtocolCoder {
*/
protected String encodeInternal(String key, Object event) {
- logger.debug("{}: encode for {}: {}", this, key, event);
+ logger.debug("{}: encode for {}: {}", this, key, event); // NOSONAR
+
+ /*
+ * It seems that sonar declares the previous logging line as a security vulnerability
+ * when logging the topic variable. The static code analysis indicates that
+ * the path starts in org.onap.policy.drools.server.restful.RestManager::decode(),
+ * but the request is rejected if the topic contains invalid characters (the sonar description
+ * mentions "/r/n/t" characters) all of which are validated against in the checkValidNameInput(topic).
+ * Furthermore production instances are assumed not to have debug enabled, nor the REST telemetry API
+ * should be published externally. An additional note is that Path URLs containing spaces and newlines
+ * will be rejected earlier in the HTTP protocol libraries (jetty) so an URL of the form
+ * "https://../to\npic" won't even make it here.
+ */
ProtocolCoderToolset coderTools = coders.get(key);
try {
@@ -476,7 +484,7 @@ abstract class GenericEventProtocolCoder {
List<CoderFilters> coderFilters = encoderSet.getCoders();
for (CoderFilters coder : coderFilters) {
if (coder.getCodedClass().equals(encodedClass.getClass().getName())) {
- DroolsController droolsController =
+ var droolsController =
DroolsControllerConstants.getFactory().get(groupId, artifactId, "");
if (droolsController.ownsCoder(
encodedClass.getClass(), coder.getModelClassLoaderHash())) {
diff --git a/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/MultiplexorEventProtocolCoder.java b/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/MultiplexorEventProtocolCoder.java
index a8f3c3a3..d341024a 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/MultiplexorEventProtocolCoder.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/protocol/coders/MultiplexorEventProtocolCoder.java
@@ -2,7 +2,7 @@
* ============LICENSE_START=======================================================
* ONAP
* ================================================================================
- * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2019, 2021 AT&T Intellectual Property. All rights reserved.
* ================================================================================
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ class MultiplexorEventProtocolCoder implements EventProtocolCoder {
/**
* Logger.
*/
- private static Logger logger = LoggerFactory.getLogger(MultiplexorEventProtocolCoder.class);
+ private static final Logger logger = LoggerFactory.getLogger(MultiplexorEventProtocolCoder.class);
/**
* Decoders.
@@ -124,7 +124,19 @@ class MultiplexorEventProtocolCoder implements EventProtocolCoder {
*/
@Override
public Object decode(String groupId, String artifactId, String topic, String json) {
- logger.debug("{}: decode {}:{}:{}:{}", this, groupId, artifactId, topic, json);
+ logger.debug("{}: decode {}:{}:{}:{}", this, groupId, artifactId, topic, json); // NOSONAR
+
+ /*
+ * It seems that sonar declares the previous logging line as a security vulnerability
+ * when logging the topic variable. The static code analysis indicates that
+ * the path starts in org.onap.policy.drools.server.restful.RestManager::decode(),
+ * but the request is rejected if the topic contains invalid characters (the sonar description
+ * mentions "/r/n/t" characters) which are validated against in the checkValidNameInput(topic).
+ * Furthermore production instances are assumed not to have debug enabled, nor the REST telemetry API
+ * should be published externally. An additional note is that Path URLs containing spaces and newlines
+ * will be failed earlier at the HTTP protocol libraries (jetty, etc ..) so an URL of the form
+ * "https://../to\npic" won't even make it here.
+ */
return this.decoders.decode(groupId, artifactId, topic, json);
}
@@ -142,7 +154,13 @@ class MultiplexorEventProtocolCoder implements EventProtocolCoder {
*/
@Override
public String encode(String topic, Object event) {
- logger.debug("{}: encode {}:{}", this, topic, event);
+ logger.debug("{}: encode {}:{}", this, topic, event); // NOSONAR
+
+ /*
+ * See explanation for decode(String groupId, String artifactId, String topic, String json).
+ * The same applies here as it is called from
+ * org.onap.policy.drools.server.restful.RestManager::encode(),
+ */
return this.encoders.encode(topic, event);
}
diff --git a/policy-management/src/main/java/org/onap/policy/drools/server/restful/RestManager.java b/policy-management/src/main/java/org/onap/policy/drools/server/restful/RestManager.java
index e704f0b0..ca2618c1 100644
--- a/policy-management/src/main/java/org/onap/policy/drools/server/restful/RestManager.java
+++ b/policy-management/src/main/java/org/onap/policy/drools/server/restful/RestManager.java
@@ -32,6 +32,7 @@ import io.swagger.annotations.SwaggerDefinition;
import io.swagger.annotations.Tag;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -129,7 +130,7 @@ public class RestManager {
/**
* Feed Ports into Resources.
*/
- private static final List<String> INPUTS = Arrays.asList("configuration");
+ private static final List<String> INPUTS = Collections.singletonList("configuration");
/**
* Resource Toggles.
@@ -1626,6 +1627,13 @@ public class RestManager {
public Response commSources(
@ApiParam(value = "Communication Mechanism", required = true) @PathParam("comm") String comm
) {
+ if (!checkValidNameInput(comm)) {
+ return Response
+ .status(Response.Status.NOT_ACCEPTABLE)
+ .entity(new Error("source communication mechanism contains whitespaces " + NOT_ACCEPTABLE_MSG))
+ .build();
+ }
+
List<TopicSource> sources = new ArrayList<>();
var status = Status.OK;
switch (CommInfrastructure.valueOf(comm.toUpperCase())) {
@@ -1656,6 +1664,13 @@ public class RestManager {
public Response commSinks(
@ApiParam(value = "Communication Mechanism", required = true) @PathParam("comm") String comm
) {
+ if (!checkValidNameInput(comm)) {
+ return Response
+ .status(Response.Status.NOT_ACCEPTABLE)
+ .entity(new Error("sink communication mechanism contains whitespaces " + NOT_ACCEPTABLE_MSG))
+ .build();
+ }
+
List<TopicSink> sinks = new ArrayList<>();
var status = Status.OK;
switch (CommInfrastructure.valueOf(comm.toUpperCase())) {