Merge "fix telemetry related sonar security issues"

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())) {