From c9ec915d7d16b88f53493c85928d463d070df472 Mon Sep 17 00:00:00 2001
From: lukegleeson <luke.gleeson@est.tech>
Date: Wed, 7 Sep 2022 14:21:36 +0100
Subject: Fix security bug in logs

When a method with signature containing "AuthPassword" is passed, the value returned is changed to *********** in logs
e.g...  Execution time of : DmiProperties.getAuthPassword() with argument[s] = *********** ...

Legacy code cleaning also included

Issue-ID: CPS-1226
Signed-off-by: lukegleeson <luke.gleeson@est.tech>
Change-Id: Ic4914eae7e5ed6d361287413d17abfe50a3788ae
---
 .../org/onap/cps/aop/CpsLoggingAspectService.java  | 30 ++++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

(limited to 'cps-service/src/main/java')

diff --git a/cps-service/src/main/java/org/onap/cps/aop/CpsLoggingAspectService.java b/cps-service/src/main/java/org/onap/cps/aop/CpsLoggingAspectService.java
index b5fe0abae..6956d8518 100644
--- a/cps-service/src/main/java/org/onap/cps/aop/CpsLoggingAspectService.java
+++ b/cps-service/src/main/java/org/onap/cps/aop/CpsLoggingAspectService.java
@@ -39,33 +39,47 @@ public class CpsLoggingAspectService {
 
     private static final String CPS_PACKAGE_NAME = "org.onap.cps";
     private static final String ALL_CPS_METHODS = "execution(* " + CPS_PACKAGE_NAME + "..*(..)))";
+    private static final String METHOD_RETURNING_SENSITIVE_DATA = "AuthPassword";
+    private static final String SENSITIVE_DATA_MASK = "***********";
 
     /**
-     * To measure method execution time as a logging.
+     * Intercept methods to measure and log execution details when debug level logging enabled.
      *
      * @param proceedingJoinPoint exposes the proceed(..) method in order to support around advice.
      * @return empty in case of void otherwise an object of return type
      */
     @Around(ALL_CPS_METHODS)
     @SneakyThrows
-    public Object logMethodExecutionTime(final ProceedingJoinPoint proceedingJoinPoint) {
+    public Object interceptMethodCall(final ProceedingJoinPoint proceedingJoinPoint) {
         if (isSlf4JDebugEnabled()) {
             final StopWatch stopWatch = new StopWatch();
-            //Calculate method execution time
             stopWatch.start();
             final Object returnValue = proceedingJoinPoint.proceed();
             stopWatch.stop();
             final MethodSignature methodSignature = (MethodSignature) proceedingJoinPoint.getSignature();
-            //Log method execution time
-            log.debug("Execution time of : {}.{}() with argument[s] = {} having result = {} :: {} ms",
-                    methodSignature.getDeclaringType().getSimpleName(),
-                    methodSignature.getName(), Arrays.toString(proceedingJoinPoint.getArgs()), returnValue,
-                    stopWatch.getTotalTimeMillis());
+
+            final Object logValue;
+            if (methodSignature.getName().contains(METHOD_RETURNING_SENSITIVE_DATA)) {
+                logValue = SENSITIVE_DATA_MASK;
+            } else {
+                logValue = returnValue;
+            }
+            logMethodCall(methodSignature, proceedingJoinPoint, stopWatch, logValue);
             return returnValue;
         }
         return proceedingJoinPoint.proceed();
     }
 
+    void logMethodCall(final MethodSignature methodSignature,
+                       final ProceedingJoinPoint proceedingJoinPoint,
+                       final StopWatch stopWatch,
+                       final Object logValue) {
+        log.debug("Execution time of : {}.{}() with argument[s] = {} having result = {} :: {} ms",
+                methodSignature.getDeclaringType().getSimpleName(),
+                methodSignature.getName(), Arrays.toString(proceedingJoinPoint.getArgs()), logValue,
+                stopWatch.getTotalTimeMillis());
+    }
+
     private static boolean isSlf4JDebugEnabled() {
         return Logger.getLogger(CPS_PACKAGE_NAME).isLoggable(Level.FINE);
     }
-- 
cgit 1.2.3-korg