Enhance config validation for bad regex in CWAgent (#459)

* Check if regex is valid in GO when applying rules for log filter and timestamp. * Add test case for log filtering validation. * fmt changes. * Remove redundant test and duplicate logging * make fmt && make fmt-sh * Add test case for invalid timestamp format.
aws · May 3, 2022 · 3a85fd6 · 3a85fd6
1 parent c4e07d7
commit 3a85fd6
Show file tree

Hide file tree

Showing 18 changed files with 127 additions and 74 deletions.
diff --git a/integration/clean/clean_dedicated_host/clean_dedicated_host.go b/integration/clean/clean_dedicated_host/clean_dedicated_host.go
@@ -8,12 +8,13 @@ package main
 
 import (
 	"context"
+	"log"
+	"time"
+
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
 	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
-	"log"
-	"time"
 )
 
 // Can't release a host if it was being used within the last 24 hr add 2 hr as a buffer

diff --git a/integration/pkg/tools/create_pkg.sh b/integration/pkg/tools/create_pkg.sh
@@ -11,8 +11,8 @@ COMMON_CONFIG_PATH=/tmp/AmazonCWAgentPackage/opt/aws/amazon-cloudwatch-agent/etc
 SAMPLE_SUFFIX=SAMPLE_DO_NOT_MODIFY
 mv ${COMMON_CONFIG_PATH} ${COMMON_CONFIG_PATH}.${SAMPLE_SUFFIX}
 if [ $? -ne 0 ]; then
-    echo "Failed to mv common-config.toml"
-    exit 1
+     echo "Failed to mv common-config.toml"
+     exit 1
 fi
 
 mkdir /tmp/AmazonAgentScripts
@@ -32,4 +32,4 @@ aws s3 cp ./artifact/amazon-cloudwatch-agent.pkg "s3://$1/integration-test/packa
 #tar -cvzf  package.tar.gz manifest.yaml artifact.gz
 #
 ##upload the .pkg file created
-#/usr/local/bin/aws s3 cp /tmp/package.tar.gz "s3://macos-cwagent-binaries/$AGENT_VERSION/pre-signed/package.tar.gz" --acl public-read
+#/usr/local/bin/aws s3 cp /tmp/package.tar.gz "s3://macos-cwagent-binaries/$AGENT_VERSION/pre-signed/package.tar.gz" --acl public-read
diff --git a/integration/pkg/tools/postinstall.sh b/integration/pkg/tools/postinstall.sh
@@ -4,15 +4,15 @@ echo "Creating a default user cwagent"
 COMMON_CONFIG_PATH=/opt/aws/amazon-cloudwatch-agent/etc/common-config.toml
 SAMPLE_SUFFIX=SAMPLE_DO_NOT_MODIFY
 if [ ! -f ${COMMON_CONFIG_PATH} ]; then
-    cp ${COMMON_CONFIG_PATH}.${SAMPLE_SUFFIX} ${COMMON_CONFIG_PATH}
+     cp ${COMMON_CONFIG_PATH}.${SAMPLE_SUFFIX} ${COMMON_CONFIG_PATH}
 fi
 
-if [[ cwagent == `sudo dscl . -list /Users UniqueID | awk '{print $1}' | grep -w cwagent` ]]; then
-    echo "User already exists!"
-    exit 0
+if [[ cwagent == $(sudo dscl . -list /Users UniqueID | awk '{print $1}' | grep -w cwagent) ]]; then
+     echo "User already exists!"
+     exit 0
 fi
 
-LastID=`sudo dscl . -list /Users UniqueID | awk '{print $2}' | sort -n | tail -1`
+LastID=$(sudo dscl . -list /Users UniqueID | awk '{print $2}' | sort -n | tail -1)
 NextID=$((LastID + 1))
 
 . /etc/rc.common
@@ -27,4 +27,4 @@ sudo dscl . create /Users/cwagent NFSHomeDirectory /Users/cwagent
 sudo createhomedir -u cwagent -c
 
 echo " "
-echo "New user `sudo dscl . -list /Users UniqueID | awk '{print $1}' | grep -w cwagent` has been created with unique ID `sudo dscl . -list /Users UniqueID | grep -w cwagent | awk '{print $2}'`"
+echo "New user $(sudo dscl . -list /Users UniqueID | awk '{print $1}' | grep -w cwagent) has been created with unique ID $(sudo dscl . -list /Users UniqueID | grep -w cwagent | awk '{print $2}')"
diff --git a/integration/pkg/tools/preinstall.sh b/integration/pkg/tools/preinstall.sh
@@ -3,16 +3,16 @@
 COMMON_CONFIG_PATH=/opt/aws/amazon-cloudwatch-agent/etc/common-config.toml
 SAMPLE_SUFFIX=SAMPLE_DO_NOT_MODIFY
 if [ -e ${COMMON_CONFIG_PATH}.${SAMPLE_SUFFIX} -a -e ${COMMON_CONFIG_PATH} ]; then
-    diff ${COMMON_CONFIG_PATH}.${SAMPLE_SUFFIX} ${COMMON_CONFIG_PATH} >/dev/null 2>&1
-    if [ $? -eq 0 ]; then
-        rm -r ${COMMON_CONFIG_PATH}
-    fi
+     diff ${COMMON_CONFIG_PATH}.${SAMPLE_SUFFIX} ${COMMON_CONFIG_PATH} >/dev/null 2>&1
+     if [ $? -eq 0 ]; then
+          rm -r ${COMMON_CONFIG_PATH}
+     fi
 fi
 
 launchctl list com.amazon.cloudwatch.agent >/dev/null 2>&1
 if [ $? -eq 0 ]; then
-    echo "Agent is running in the instance"
-    echo "Stopping the agent"
-    launchctl unload /Library/LaunchDaemons/com.amazon.cloudwatch.agent.plist
-    echo "Agent stopped"
-fi
+     echo "Agent is running in the instance"
+     echo "Stopping the agent"
+     launchctl unload /Library/LaunchDaemons/com.amazon.cloudwatch.agent.plist
+     echo "Agent stopped"
+fi
diff --git a/integration/test/agent_util.go b/integration/test/agent_util.go
@@ -9,13 +9,14 @@ package test
 import (
 	"context"
 	"fmt"
-	"github.com/aws/aws-sdk-go-v2/config"
-	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
-	"github.com/aws/aws-sdk-go-v2/service/cloudwatch"
 	"log"
 	"os/exec"
 	"path/filepath"
 	"time"
+
+	"github.com/aws/aws-sdk-go-v2/config"
+	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
+	"github.com/aws/aws-sdk-go-v2/service/cloudwatch"
 )
 
 func CopyFile(pathIn string, pathOut string) {

diff --git a/integration/test/cloudwatchlogs/publish_logs_test.go b/integration/test/cloudwatchlogs/publish_logs_test.go
@@ -8,11 +8,12 @@ package cloudwatchlogs
 
 import (
 	"fmt"
-	"github.com/aws/amazon-cloudwatch-agent/integration/test"
 	"log"
 	"os"
 	"strings"
 
+	"github.com/aws/amazon-cloudwatch-agent/integration/test"
+
 	"testing"
 	"time"
 )

diff --git a/integration/test/metrics_number_dimension/metrics_number_dimension_test.go b/integration/test/metrics_number_dimension/metrics_number_dimension_test.go
@@ -9,14 +9,15 @@ package metrics_number_dimension
 import (
 	"context"
 	"fmt"
-	"github.com/aws/amazon-cloudwatch-agent/integration/test"
-	"github.com/aws/aws-sdk-go-v2/aws"
-	"github.com/aws/aws-sdk-go-v2/service/cloudwatch"
-	"github.com/aws/aws-sdk-go-v2/service/cloudwatch/types"
 	"log"
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/aws/amazon-cloudwatch-agent/integration/test"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/cloudwatch"
+	"github.com/aws/aws-sdk-go-v2/service/cloudwatch/types"
 )
 
 const configOutputPath = "/opt/aws/amazon-cloudwatch-agent/bin/config.json"

diff --git a/integration/test/sanity/resources/verifyLinuxCtlScript.sh b/integration/test/sanity/resources/verifyLinuxCtlScript.sh
@@ -4,43 +4,44 @@
 # SPDX-License-Identifier: MIT
 
 assertStatus() {
-  keyToCheck="${1:-}"
-  expectedVal="${2:-}"
-
-  grepKey='unknown'
-  case "${keyToCheck}" in
-    cwa_running_status)
-        grepKey="\"status\""
-        ;;
-    cwa_config_status)
-        grepKey="\"configstatus\""
-        ;;
-    cwoc_running_status)
-        grepKey="\"cwoc_status\""
-        ;;
-    cwoc_config_status)
-        grepKey="\"cwoc_configstatus\""
-        ;;
-    *)  echo "Invalid Key To Check: ${keyToCheck}" >&2
-        exit 1
-        ;;
-  esac
-
-  result=$(/usr/bin/amazon-cloudwatch-agent-ctl -a status | grep "${grepKey}" | awk -F: '{print $2}' | sed 's/ "//; s/",//')
-
-  if [ "${result}" = "${expectedVal}" ]; then
-      echo "In step ${step}, ${keyToCheck} is expected"
-  else
-      echo "In step ${step}, ${keyToCheck} is NOT expected. (actual="${result}"; expected="${expectedVal}")"
-      exit 1
-  fi
+     keyToCheck="${1:-}"
+     expectedVal="${2:-}"
+
+     grepKey='unknown'
+     case "${keyToCheck}" in
+     cwa_running_status)
+          grepKey="\"status\""
+          ;;
+     cwa_config_status)
+          grepKey="\"configstatus\""
+          ;;
+     cwoc_running_status)
+          grepKey="\"cwoc_status\""
+          ;;
+     cwoc_config_status)
+          grepKey="\"cwoc_configstatus\""
+          ;;
+     *)
+          echo "Invalid Key To Check: ${keyToCheck}" >&2
+          exit 1
+          ;;
+     esac
+
+     result=$(/usr/bin/amazon-cloudwatch-agent-ctl -a status | grep "${grepKey}" | awk -F: '{print $2}' | sed 's/ "//; s/",//')
+
+     if [ "${result}" = "${expectedVal}" ]; then
+          echo "In step ${step}, ${keyToCheck} is expected"
+     else
+          echo "In step ${step}, ${keyToCheck} is NOT expected. (actual="${result}"; expected="${expectedVal}")"
+          exit 1
+     fi
 }
 
 # init
 step=0
 aoc_user=$(cat /etc/passwd | grep aoc)
 if [ "${aoc_user}" = "" ]; then
-  echo 'aoc:x:995:991:AOC Agent:/home/aoc:/sbin/nologin' | sudo tee -a /etc/passwd
+     echo 'aoc:x:995:991:AOC Agent:/home/aoc:/sbin/nologin' | sudo tee -a /etc/passwd
 fi
 /usr/bin/amazon-cloudwatch-agent-ctl -a remove-config -c all -o all
 /usr/bin/amazon-cloudwatch-agent-ctl -a stop
@@ -129,4 +130,4 @@ step=12
 assertStatus "cwa_running_status" "stopped"
 assertStatus "cwoc_running_status" "stopped"
 assertStatus "cwa_config_status" "not configured"
-assertStatus "cwoc_config_status" "configured"
+assertStatus "cwoc_config_status" "configured"
diff --git a/packaging/debian/preinst b/packaging/debian/preinst
@@ -3,8 +3,8 @@
 # SPDX-License-Identifier: MIT
 
 if [ "$1" = "upgrade" ]; then
-    /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a prep-restart
-    /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a stop
+     /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a prep-restart
+     /opt/aws/amazon-cloudwatch-agent/bin/amazon-cloudwatch-agent-ctl -a stop
 fi
 
 if ! grep "^cwagent:" /etc/group >/dev/null 2>&1; then
@@ -18,11 +18,11 @@ if ! id cwagent >/dev/null 2>&1; then
 fi
 
 if ! grep "^aoc:" /etc/group >/dev/null 2>&1; then
-    groupadd -r aoc >/dev/null 2>&1
-    echo "create group aoc, result: $?"
+     groupadd -r aoc >/dev/null 2>&1
+     echo "create group aoc, result: $?"
 fi
 
 if ! id aoc >/dev/null 2>&1; then
-    useradd -r -M aoc -d /home/aoc -g aoc >/dev/null 2>&1
-    echo "create user aoc, result: $?"
+     useradd -r -M aoc -d /home/aoc -g aoc >/dev/null 2>&1
+     echo "create user aoc, result: $?"
 fi
diff --git a/translator/totomlconfig/toTomlConfig.go b/translator/totomlconfig/toTomlConfig.go
@@ -4,8 +4,8 @@
 package totomlconfig
 
 import (
-	"log"
 	"bytes"
+	"log"
 
 	"github.com/aws/amazon-cloudwatch-agent/translator/translate"
 	_ "github.com/aws/amazon-cloudwatch-agent/translator/translate/agent"
@@ -61,7 +61,7 @@ func ToTomlConfig(c interface{}) string {
 	enc := toml.NewEncoder(&buf)
 	err := enc.Encode(val)
 	if err != nil {
-		log.Panicf("Encode to a valid TOML config fails because of %v",err)
+		log.Panicf("Encode to a valid TOML config fails because of %v", err)
 	}
 	return buf.String()
 }
diff --git a/translator/translate/logs/logs_collected/files/collect_list/collect_list.go b/translator/translate/logs/logs_collected/files/collect_list/collect_list.go
@@ -5,16 +5,17 @@ package collect_list
 
 import (
 	"encoding/json"
+	"io/ioutil"
+	"path/filepath"
+	"sort"
+
 	"github.com/aws/amazon-cloudwatch-agent/translator"
 	"github.com/aws/amazon-cloudwatch-agent/translator/context"
 	"github.com/aws/amazon-cloudwatch-agent/translator/jsonconfig/mergeJsonRule"
 	"github.com/aws/amazon-cloudwatch-agent/translator/jsonconfig/mergeJsonUtil"
 	"github.com/aws/amazon-cloudwatch-agent/translator/translate/agent"
 	parent "github.com/aws/amazon-cloudwatch-agent/translator/translate/logs/logs_collected/files"
 	logUtil "github.com/aws/amazon-cloudwatch-agent/translator/translate/logs/util"
-	"io/ioutil"
-	"path/filepath"
-	"sort"
 )
 
 type Rule translator.Rule

diff --git a/translator/translate/logs/logs_collected/files/collect_list/collect_list_test.go b/translator/translate/logs/logs_collected/files/collect_list/collect_list_test.go
@@ -310,6 +310,22 @@ func TestTimestampFormat_Template(t *testing.T) {
 	assert.Equal(t, time.Date(0, 8, 9, 20, 45, 51, 0, time.Local), parsedTime)
 }
 
+func TestTimestampFormat_InvalidRegex(t *testing.T) {
+	translator.ResetMessages()
+	r := new(TimestampRegax)
+	var input interface{}
+	e := json.Unmarshal([]byte(`{
+		"timestamp_format":"%Y-%m-%dT%H:%M%S+00:00"
+	}`), &input)
+	assert.Nil(t, e)
+
+	retKey, retVal := r.ApplyRule(input)
+	assert.Equal(t, "timestamp_regex", retKey)
+	assert.Nil(t, retVal)
+	assert.Len(t, translator.ErrorMessages, 1)
+
+}
+
 func TestMultiLineStartPattern(t *testing.T) {
 	f := new(FileConfig)
 	var input interface{}
@@ -371,6 +387,7 @@ func TestEncoding(t *testing.T) {
 }
 
 func TestEncoding_Invalid(t *testing.T) {
+	translator.ResetMessages()
 	f := new(FileConfig)
 	var input interface{}
 	e := json.Unmarshal([]byte(`{
@@ -687,6 +704,7 @@ func TestDuplicateRetention(t *testing.T) {
 }
 
 func TestConflictingRetention(t *testing.T) {
+	translator.ResetMessages()
 	f := new(FileConfig)
 	var input interface{}
 	e := json.Unmarshal([]byte(`{

diff --git a/translator/translate/logs/logs_collected/files/collect_list/ruleLogFilters.go b/translator/translate/logs/logs_collected/files/collect_list/ruleLogFilters.go
@@ -5,6 +5,7 @@ package collect_list
 
 import (
 	"fmt"
+	"regexp"
 
 	"github.com/aws/amazon-cloudwatch-agent/translator"
 )
@@ -37,6 +38,10 @@ func (lf *LogFilter) ApplyRule(input interface{}) (returnKey string, returnVal i
 				translator.AddErrorMessages(GetCurPath()+FiltersSectionKey, fmt.Sprintf("Filter %s is invalid", filter))
 				continue
 			}
+			if _, err := regexp.Compile(filterVal.(string)); err != nil {
+				translator.AddErrorMessages(GetCurPath()+FiltersSectionKey, fmt.Sprintf("Filter expression %s is invalid", filter))
+				continue
+			}
 			filterMap[FiltersExpressionSectionKey] = filterVal
 			res = append(res, filterMap)
 		}

diff --git a/translator/translate/logs/logs_collected/files/collect_list/ruleLogFilters_test.go b/translator/translate/logs/logs_collected/files/collect_list/ruleLogFilters_test.go
@@ -62,3 +62,18 @@ func TestApplyLogFiltersRuleMissingConfigInsideFilters(t *testing.T) {
 	assert.Nil(t, retVal)
 	assert.Len(t, translator.ErrorMessages, 2)
 }
+
+func TestApplyLogFiltersRuleInvalidRegex(t *testing.T) {
+	translator.ResetMessages()
+	r := new(LogFilter)
+	var input interface{}
+	e := json.Unmarshal([]byte(`{
+		"filters": [
+			{"type": "exclude", "expression": "(?!re)"}
+		]
+	}`), &input)
+	assert.Nil(t, e)
+	_, retVal := r.ApplyRule(input)
+	assert.Nil(t, retVal)
+	assert.Len(t, translator.ErrorMessages, 1)
+}
diff --git a/translator/translate/logs/logs_collected/files/collect_list/ruleTimestampFormat.go b/translator/translate/logs/logs_collected/files/collect_list/ruleTimestampFormat.go
@@ -4,7 +4,11 @@
 package collect_list
 
 import (
+	"fmt"
+	"regexp"
 	"strings"
+
+	"github.com/aws/amazon-cloudwatch-agent/translator"
 )
 
 /*
@@ -155,6 +159,10 @@ func (t *TimestampRegax) ApplyRule(input interface{}) (returnKey string, returnV
 		res = strings.TrimPrefix(res, "\\s{0,1}")
 		res = "(" + res + ")"
 		returnKey = "timestamp_regex"
+		if _, err := regexp.Compile(res); err != nil {
+			translator.AddErrorMessages(GetCurPath()+"timestamp_format", fmt.Sprintf("Timestamp format %s is invalid", val))
+			return
+		}
 		returnVal = res
 	}
 	return

diff --git a/translator/translate/logs/logs_collected/windows_events/collect_list/collectlist.go b/translator/translate/logs/logs_collected/windows_events/collect_list/collectlist.go
@@ -12,7 +12,6 @@ import (
 	parent "github.com/aws/amazon-cloudwatch-agent/translator/translate/logs/logs_collected/windows_events"
 	logUtil "github.com/aws/amazon-cloudwatch-agent/translator/translate/logs/util"
 	"github.com/aws/amazon-cloudwatch-agent/translator/util"
-
 )
 
 type Rule translator.Rule