fix: uri and request parsing; (#222)

Co-authored-by: m.gougam <[email protected]>

src/extensions/nginx-app-protect/monitoring/collector/nap.go

@@ -70,14 +70,14 @@ func newSyslogServer(logger *logrus.Entry, ip string, port int) (*syslogServer,
 	addr := fmt.Sprintf("%s:%d", ip, port)
 	err := server.ListenTCP(addr)
 	if err != nil {
-		msg := fmt.Sprintf("Error while configuring syslog server to listen on %s:\n %v", addr, err)
+		msg := fmt.Sprintf("error while configuring syslog server to listen on %s:\n %v", addr, err)
 		logger.Error(msg)
 		return nil, err
 	}
 
 	err = server.Boot()
 	if err != nil {
-		msg := fmt.Sprintf("Error while booting the syslog server at %s:\n %v ", addr, err)
+		msg := fmt.Sprintf("error while booting the syslog server at %s:\n %v ", addr, err)
 		logger.Error(msg)
 		return nil, err
 	}

src/extensions/nginx-app-protect/monitoring/collector/nap_test.go

@@ -16,6 +16,7 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/mcuadros/go-syslog.v2/format"
@@ -24,6 +25,53 @@ import (
 	"github.com/nginx/agent/v2/src/extensions/nginx-app-protect/monitoring/collector"
 )
 
+func TestNewNAPCollector(t *testing.T) {
+	testCases := []struct {
+		testName string
+		cfg      *collector.NAPConfig
+		expError bool
+	}{
+		{
+			testName: "Valid Config",
+			cfg: &collector.NAPConfig{
+				SyslogIP:   "127.0.0.1",
+				SyslogPort: 1234,
+				Logger:     logrus.NewEntry(logrus.New()),
+			},
+			expError: false,
+		},
+		{
+			testName: "Malformed IP address",
+			cfg: &collector.NAPConfig{
+				SyslogIP:   "not_an_ipaddress",
+				SyslogPort: 514,
+				Logger:     logrus.NewEntry(logrus.New()),
+			},
+			expError: true,
+		},
+		{
+			testName: "Bad port number",
+			cfg: &collector.NAPConfig{
+				SyslogIP:   "127.0.0.1",
+				SyslogPort: -1024,
+				Logger:     logrus.NewEntry(logrus.New()),
+			},
+			expError: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.testName, func(t *testing.T) {
+			_, err := collector.NewNAPCollector(tc.cfg)
+			if tc.expError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestNAPCollect(t *testing.T) {
 	var logwriter *syslog.Writer
 	var testPort = 22514

src/extensions/nginx-app-protect/monitoring/processor/nap.go

@@ -30,6 +30,9 @@ const (
 	cookieCtx               = "cookie"
 	defaultBlockedRespCode  = "0"
 	defaultBlockedRespValue = "Blocked"
+
+	decodedComma = ","
+	encodedComma = "%2C"
 )
 
 // NGINX App Protect Logging Directives
@@ -104,6 +107,7 @@ var (
 		clientApplicationVersion,
 		transportProtocol,
 		httpURI,
+		request,
 	}
 )
 
@@ -450,11 +454,6 @@ func parseNAP(logEntry string, logger *logrus.Entry) (*NAPConfig, error) {
 		}
 	}
 
-	err := setValue(&waf, request, strings.Join(values[len(logFormatKeys):], ","), logger)
-	if err != nil {
-		return &NAPConfig{}, err
-	}
-
 	return &waf, nil
 }
 
@@ -506,13 +505,13 @@ func setValue(napConfig *NAPConfig, key, value string, logger *logrus.Entry) err
 	case httpServerPort:
 		napConfig.HTTPServerPort = value
 	case httpURI:
-		napConfig.HTTPURI = value
+		napConfig.HTTPURI = strings.ReplaceAll(value, encodedComma, decodedComma)
 	case isTruncated:
 		napConfig.IsTruncated = value
 	case policyName:
 		napConfig.PolicyName = value
 	case request:
-		napConfig.Request = value
+		napConfig.Request = strings.ReplaceAll(value, encodedComma, decodedComma)
 	case requestOutcome:
 		napConfig.RequestOutcome = value
 	case requestOutcomeReason:

src/extensions/nginx-app-protect/monitoring/processor/nap_test.go

@@ -25,7 +25,7 @@ func TestParseNAPExpanded(t *testing.T) {
 	}{
 		{
 			testName: "ValidEntry",
-			logEntry: fmt.Sprintf(`"%s"`, func() string {
+			logEntry: fmt.Sprintf(`%s`, func() string {
 				input, _ := os.ReadFile("./testdata/expanded_nap_waf.log.txt")
 				return string(input)
 			}()),
@@ -46,3 +46,39 @@ func TestParseNAPExpanded(t *testing.T) {
 		})
 	}
 }
+
+func TestParseNAPCommaEncoding(t *testing.T) {
+	testCases := []struct {
+		testName     string
+		logEntry     string
+		expNAPConfig *NAPConfig
+		expError     error
+	}{
+		{
+			testName: "Valid Entry with comma in both URI and Request",
+			logEntry: fmt.Sprintf(`%s`, func() string {
+				input, _ := os.ReadFile("./testdata/uri_request_contain_escaped_comma.log.txt")
+				return string(input)
+			}()),
+			expNAPConfig: &NAPConfig{
+				HTTPURI: "/with,comma",
+				Request: "GET /with,comma HTTP/1.1\\r\\nHost: 10.146.183.68\\r\\nConnection: keep-alive\\r\\nCache-Control: max-age=0\\r\\nUpgrade-Insecure-Requests: 1\\r\\nUser-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36\\r\\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9\\r\\nAccept-Encoding: gzip, deflate\\r\\nAccept-Language: en-US,en;q=0.9\\r\\n\\r\\n",
+			},
+			expError: nil,
+		},
+	}
+
+	log := logrus.New()
+	log.SetLevel(logrus.DebugLevel)
+
+	for _, tc := range testCases {
+		t.Run(tc.testName, func(t *testing.T) {
+			result, err := parseNAP(tc.logEntry, log.WithFields(logrus.Fields{
+				"extension": "test",
+			}))
+			assert.Equal(t, tc.expNAPConfig.HTTPURI, result.HTTPURI)
+			assert.Equal(t, tc.expNAPConfig.Request, result.Request)
+			assert.Equal(t, tc.expError, err)
+		})
+	}
+}

src/extensions/nginx-app-protect/monitoring/processor/testdata/expanded_nap_waf.log.txt

@@ -1 +1 @@
-N/A,80,127.0.0.1,,GET,app_protect_default_policy,HTTP,blocked,0,Critical,::,{Cross Site Scripting Signatures;High Accuracy Signatures}::{Cross Site Scripting Signatures;High Accuracy Signatures},61478,HTTP protocol compliance failed:Host header contains IP address::HTTP protocol compliance failed:Evasion technique,4355056874564592513,campaign1::campaign2,5,1-localhost:1-/,N/A,REJECTED,SECURITY_WAF_VIOLATION,HTTP protocol compliance failed::Illegal meta character in value::Attack signature detected::Violation Rating Threat detected::Bot Client Detected,<?xml version='1.0' encoding='UTF-8'?><BAD_MSG><violation_masks><block>410000000200c00-3a03030c30000072-8000000000000000-0</block><alarm>477f0ffcbbd0fea-befbf35cb000007e-8000000000000000-0</alarm><learn>0-20-0-0</learn><staging>0-0-0-0</staging></violation_masks><request-violations><violation><viol_index>42</viol_index><viol_name>VIOL_ATTACK_SIGNATURE</viol_name><context>parameter</context><parameter_data><value_error/><enforcement_level>global</enforcement_level><name>YQ==</name><auto_detected_type>alpha-numeric</auto_detected_type><value>PHNjcmlwdD4=</value><location>query</location><param_name_pattern>*</param_name_pattern><staging>0</staging></parameter_data><staging>0</staging><sig_data><sig_id>200001475</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>YT08c2NyaXB0Pg==</buffer><offset>3</offset><length>7</length></kw_data></sig_data><sig_data><sig_id>200000098</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>YT08c2NyaXB0Pg==</buffer><offset>2</offset><length>7</length></kw_data></sig_data></violation><violation><viol_index>14</viol_index><viol_name>VIOL_HTTP_PROTOCOL</viol_name><http_sanity_checks_status>2048</http_sanity_checks_status><http_sub_violation_status>2048</http_sub_violation_status><http_sub_violation>SG9zdCBoZWFkZXIgd2l0aCBJUCB2YWx1ZTogMTAuMTQ2LjE3OS4xMTk=</http_sub_violation></violation><violation><viol_index>24</viol_index><viol_name>VIOL_PARAMETER_VALUE_METACHAR</viol_name><parameter_data><value_error/><enforcement_level>global</enforcement_level><name>YQ==</name><auto_detected_type>alpha-numeric</auto_detected_type><value>PHNjcmlwdD4=</value><location>query</location></parameter_data><wildcard_entity>*</wildcard_entity><staging>0</staging><language_type>4</language_type><metachar_index>60</metachar_index><metachar_index>62</metachar_index></violation></request-violations></BAD_MSG>,curl,HTTP Library,N/A,N/A,Untrusted Bot,N/A,N/A,/,GET /?a=<script> HTTP/1.1\r\nHost: 127.0.0.1\r\nUser-Agent: curl/7.64.1\r\nAccept: */*\r\n\r\n,HTTP/1.1
+N/A,80,127.0.0.1,,GET,app_protect_default_policy,HTTP,blocked,0,Critical,::,{Cross Site Scripting Signatures;High Accuracy Signatures}::{Cross Site Scripting Signatures;High Accuracy Signatures},61478,HTTP protocol compliance failed:Host header contains IP address::HTTP protocol compliance failed:Evasion technique,4355056874564592513,campaign1::campaign2,5,1-localhost:1-/,N/A,REJECTED,SECURITY_WAF_VIOLATION,HTTP protocol compliance failed::Illegal meta character in value::Attack signature detected::Violation Rating Threat detected::Bot Client Detected,<?xml version='1.0' encoding='UTF-8'?><BAD_MSG><violation_masks><block>410000000200c00-3a03030c30000072-8000000000000000-0</block><alarm>477f0ffcbbd0fea-befbf35cb000007e-8000000000000000-0</alarm><learn>0-20-0-0</learn><staging>0-0-0-0</staging></violation_masks><request-violations><violation><viol_index>42</viol_index><viol_name>VIOL_ATTACK_SIGNATURE</viol_name><context>parameter</context><parameter_data><value_error/><enforcement_level>global</enforcement_level><name>YQ==</name><auto_detected_type>alpha-numeric</auto_detected_type><value>PHNjcmlwdD4=</value><location>query</location><param_name_pattern>*</param_name_pattern><staging>0</staging></parameter_data><staging>0</staging><sig_data><sig_id>200001475</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>YT08c2NyaXB0Pg==</buffer><offset>3</offset><length>7</length></kw_data></sig_data><sig_data><sig_id>200000098</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>YT08c2NyaXB0Pg==</buffer><offset>2</offset><length>7</length></kw_data></sig_data></violation><violation><viol_index>14</viol_index><viol_name>VIOL_HTTP_PROTOCOL</viol_name><http_sanity_checks_status>2048</http_sanity_checks_status><http_sub_violation_status>2048</http_sub_violation_status><http_sub_violation>SG9zdCBoZWFkZXIgd2l0aCBJUCB2YWx1ZTogMTAuMTQ2LjE3OS4xMTk=</http_sub_violation></violation><violation><viol_index>24</viol_index><viol_name>VIOL_PARAMETER_VALUE_METACHAR</viol_name><parameter_data><value_error/><enforcement_level>global</enforcement_level><name>YQ==</name><auto_detected_type>alpha-numeric</auto_detected_type><value>PHNjcmlwdD4=</value><location>query</location></parameter_data><wildcard_entity>*</wildcard_entity><staging>0</staging><language_type>4</language_type><metachar_index>60</metachar_index><metachar_index>62</metachar_index></violation></request-violations></BAD_MSG>,curl,HTTP Library,N/A,N/A,Untrusted Bot,N/A,N/A,HTTP/1.1,/,GET /?a=<script> HTTP/1.1\r\nHost: 127.0.0.1\r\nUser-Agent: curl/7.64.1\r\nAccept: */*\r\n\r\n

src/extensions/nginx-app-protect/monitoring/processor/testdata/uri_request_contain_escaped_comma.log.txt

@@ -0,0 +1 @@
+N/A,80,127.0.0.1,,GET,app_protect_default_policy,HTTP,blocked,0,Critical,::,{Cross Site Scripting Signatures;High Accuracy Signatures}::{Cross Site Scripting Signatures;High Accuracy Signatures},61478,HTTP protocol compliance failed:Host header contains IP address::HTTP protocol compliance failed:Evasion technique,4355056874564592513,campaign1::campaign2,5,1-localhost:1-/,N/A,REJECTED,SECURITY_WAF_VIOLATION,HTTP protocol compliance failed::Illegal meta character in value::Attack signature detected::Violation Rating Threat detected::Bot Client Detected,<?xml version='1.0' encoding='UTF-8'?><BAD_MSG><violation_masks><block>410000000200c00-3a03030c30000072-8000000000000000-0</block><alarm>477f0ffcbbd0fea-befbf35cb000007e-8000000000000000-0</alarm><learn>0-20-0-0</learn><staging>0-0-0-0</staging></violation_masks><request-violations><violation><viol_index>42</viol_index><viol_name>VIOL_ATTACK_SIGNATURE</viol_name><context>parameter</context><parameter_data><value_error/><enforcement_level>global</enforcement_level><name>YQ==</name><auto_detected_type>alpha-numeric</auto_detected_type><value>PHNjcmlwdD4=</value><location>query</location><param_name_pattern>*</param_name_pattern><staging>0</staging></parameter_data><staging>0</staging><sig_data><sig_id>200001475</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>YT08c2NyaXB0Pg==</buffer><offset>3</offset><length>7</length></kw_data></sig_data><sig_data><sig_id>200000098</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>YT08c2NyaXB0Pg==</buffer><offset>2</offset><length>7</length></kw_data></sig_data></violation><violation><viol_index>14</viol_index><viol_name>VIOL_HTTP_PROTOCOL</viol_name><http_sanity_checks_status>2048</http_sanity_checks_status><http_sub_violation_status>2048</http_sub_violation_status><http_sub_violation>SG9zdCBoZWFkZXIgd2l0aCBJUCB2YWx1ZTogMTAuMTQ2LjE3OS4xMTk=</http_sub_violation></violation><violation><viol_index>24</viol_index><viol_name>VIOL_PARAMETER_VALUE_METACHAR</viol_name><parameter_data><value_error/><enforcement_level>global</enforcement_level><name>YQ==</name><auto_detected_type>alpha-numeric</auto_detected_type><value>PHNjcmlwdD4=</value><location>query</location></parameter_data><wildcard_entity>*</wildcard_entity><staging>0</staging><language_type>4</language_type><metachar_index>60</metachar_index><metachar_index>62</metachar_index></violation></request-violations></BAD_MSG>,curl,HTTP Library,N/A,N/A,Untrusted Bot,N/A,N/A,HTTP/1.1,/with%2Ccomma,GET /with%2Ccomma HTTP/1.1\r\nHost: 10.146.183.68\r\nConnection: keep-alive\r\nCache-Control: max-age=0\r\nUpgrade-Insecure-Requests: 1\r\nUser-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML%2C like Gecko) Chrome/104.0.0.0 Safari/537.36\r\nAccept: text/html%2Capplication/xhtml+xml%2Capplication/xml;q=0.9%2Cimage/avif%2Cimage/webp%2Cimage/apng%2C*/*;q=0.8%2Capplication/signed-exchange;v=b3;q=0.9\r\nAccept-Encoding: gzip%2C deflate\r\nAccept-Language: en-US%2Cen;q=0.9\r\n\r\n

src/plugins/nap_monitoring.go

@@ -55,7 +55,7 @@ func NewNAPMonitoring(env core.Environment, cfg *config.Config) (*NAPMonitoring,
 		log.Warnf("NAP Monitoring report count must be between %v and %v. Defaulting to %v",
 			minReportCountDelimiter,
 			maxReportCountDelimiter,
-			config.Defaults.NAPMonitoring.ReportInterval)
+			config.Defaults.NAPMonitoring.ReportCount)
 		cfg.NAPMonitoring.ReportCount = config.Defaults.NAPMonitoring.ReportCount
 	}
 

test/component/nginx-app-protect/monitoring/monitoring_test.go

@@ -179,6 +179,7 @@ func TestNAPMonitoring(t *testing.T) {
 }
 
 func assertEqualSecurityViolationEvents(t *testing.T, expectedEvent, resultEvent *events.Event) {
+	assert.Equal(t, expectedEvent.GetSecurityViolationEvent().SupportID, resultEvent.GetSecurityViolationEvent().SupportID)
 	assert.Equal(t, expectedEvent.GetSecurityViolationEvent().PolicyName, resultEvent.GetSecurityViolationEvent().PolicyName)
 	assert.Equal(t, expectedEvent.GetSecurityViolationEvent().Outcome, resultEvent.GetSecurityViolationEvent().Outcome)
 	assert.Equal(t, expectedEvent.GetSecurityViolationEvent().OutcomeReason, resultEvent.GetSecurityViolationEvent().OutcomeReason)

test/component/nginx-app-protect/monitoring/testData/events-out/expanded_nap_waf.log.txt.out

@@ -1,8 +1,5 @@
-
-j
-Agent$8b8e7050-655d-11ed-9d1d-5671f551701b4355056874564592513"ܭћ����*ERROR2Nginx:
-AppProtect�
-app_protect_default_policy4355056874564592513REJECTED"SECURITY_WAF_VIOLATION*N/A2GET:HTTPBN/AJ^GET /?a=<script> HTTP/1.1\r\nHost: 127.0.0.1\r\nUser-Agent: curl/7.64.1\r\nAccept: */*\r\n\r\nRHTTP/1.1bblockedjBlockedz1-localhost:1-/�
	127.0.0.1�
61478�
80�
�
HTTP protocol compliance failed,Illegal meta character in value,Attack signature detected,Violation Rating Threat detected,Bot Client Detected�
qHTTP protocol compliance failed:Host header contains IP address,HTTP protocol compliance failed:Evasion technique�

5�
u{Cross Site Scripting Signatures;High Accuracy Signatures},{Cross Site Scripting Signatures;High Accuracy Signatures}�

,�
Untrusted Bot�
N/A�
N/A�
critical�
campaign1,campaign2�
N/A�
HTTP Library�
N/A�curl�	parameter�u
+�
+app_protect_default_policy4355056874564592513REJECTED"SECURITY_WAF_VIOLATION2GET:HTTPJ
/R^GET /?a=<script> HTTP/1.1\r\nHost: 127.0.0.1\r\nUser-Agent: curl/7.64.1\r\nAccept: */*\r\n\r\nbblockedjBlockedz1-localhost:1-/�
	127.0.0.1�
61478�
80�
�
HTTP protocol compliance failed,Illegal meta character in value,Attack signature detected,Violation Rating Threat detected,Bot Client Detected�
Untrusted Bot�
critical�curl�u
 VIOL_ATTACK_SIGNATURE	parameter
 
a<script>" 
 	200001475
3