From a64e55d3a824712d0e074ed20da5b961753dc3e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Thu, 21 Jul 2022 12:15:29 +0200 Subject: [PATCH 1/4] chore: drops process request into an own package. --- http/http.go | 70 +++++++++++++++++++++++++++++++++ http/http_test.go | 60 ++++++++++++++++++++++++++++ rule.go | 2 +- seclang/parser.go | 2 +- transaction.go | 49 +---------------------- transaction_test.go | 38 ------------------ transformations/base64decode.go | 2 +- 7 files changed, 134 insertions(+), 89 deletions(-) create mode 100644 http/http.go create mode 100644 http/http_test.go diff --git a/http/http.go b/http/http.go new file mode 100644 index 000000000..666e8780c --- /dev/null +++ b/http/http.go @@ -0,0 +1,70 @@ +// Copyright 2022 Juan Pablo Tosso +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package http + +import ( + "io" + "net/http" + "strconv" + "strings" + + "github.com/corazawaf/coraza/v3/types" +) + +// ProcessRequest fills all transaction variables from an http.Request object +// Most implementations of Coraza will probably use http.Request objects +// so this will implement all phase 0, 1 and 2 variables +// Note: This function will stop after an interruption +// Note: Do not manually fill any request variables +func ProcessRequest(tx *Transaction, req *http.Request) (*types.Interruption, error) { + var client string + cport := 0 + // IMPORTANT: Some http.Request.RemoteAddr implementations will not contain port or contain IPV6: [2001:db8::1]:8080 + spl := strings.Split(req.RemoteAddr, ":") + if len(spl) > 1 { + client = strings.Join(spl[0:len(spl)-1], "") + cport, _ = strconv.Atoi(spl[len(spl)-1]) + } + var in *types.Interruption + // There is no socket access in the request object so we don't know the server client or port + tx.ProcessConnection(client, cport, "", 0) + tx.ProcessURI(req.URL.String(), req.Method, req.Proto) + for k, vr := range req.Header { + for _, v := range vr { + tx.AddRequestHeader(k, v) + } + } + // Host will always be removed from req.Headers(), so we manually add it + if req.Host != "" { + tx.AddRequestHeader("Host", req.Host) + } + + in = tx.ProcessRequestHeaders() + if in != nil { + return in, nil + } + if req.Body != nil { + _, err := io.Copy(tx.RequestBodyBuffer, req.Body) + if err != nil { + return tx.Interruption, err + } + reader, err := tx.RequestBodyBuffer.Reader() + if err != nil { + return tx.Interruption, err + } + req.Body = io.NopCloser(reader) + } + return tx.ProcessRequestBody() +} diff --git a/http/http_test.go b/http/http_test.go new file mode 100644 index 000000000..b31c9dd74 --- /dev/null +++ b/http/http_test.go @@ -0,0 +1,60 @@ +// Copyright 2022 Juan Pablo Tosso +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package http + +import ( + "bufio" + "context" + "net/http" + "strings" + "testing" +) + +func TestRequestExtractionSuccess(t *testing.T) { + req, _ := http.NewRequest("POST", "https://www.coraza.io/test", strings.NewReader("test=456")) + waf := NewWaf() + tx := waf.NewTransaction(context.Background()) + if _, err := tx.ProcessRequest(req); err != nil { + t.Fatal(err) + } + if tx.Variables.RequestMethod.String() != "POST" { + t.Fatal("failed to set request from request object") + } + if err := tx.Clean(); err != nil { + t.Fatal(err) + } +} + +func TestProcessRequestMultipart(t *testing.T) { + req, _ := http.NewRequest("POST", "/some", nil) + if err := multipartRequest(req); err != nil { + t.Fatal(err) + } + tx := makeTransaction() + tx.RequestBodyAccess = true + if _, err := tx.ProcessRequest(req); err != nil { + t.Fatal(err) + } + if req.Body == nil { + t.Error("failed to process multipart request") + } + reader := bufio.NewReader(req.Body) + if _, err := reader.ReadString('\n'); err != nil { + t.Error("failed to read multipart request", err) + } + if err := tx.Clean(); err != nil { + t.Error(err) + } +} diff --git a/rule.go b/rule.go index ab6d626c8..ea3d355d4 100644 --- a/rule.go +++ b/rule.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http:// www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, diff --git a/seclang/parser.go b/seclang/parser.go index 19678446d..03ad298bc 100644 --- a/seclang/parser.go +++ b/seclang/parser.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http:// www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, diff --git a/transaction.go b/transaction.go index 5c24ae61d..3101791e7 100644 --- a/transaction.go +++ b/transaction.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http:// www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -19,7 +19,6 @@ import ( "fmt" "io" "mime" - "net/http" "net/url" "path/filepath" "strconv" @@ -412,52 +411,6 @@ func (tx *Transaction) RemoveRuleByID(id int) { tx.ruleRemoveByID = append(tx.ruleRemoveByID, id) } -// ProcessRequest fills all transaction variables from an http.Request object -// Most implementations of Coraza will probably use http.Request objects -// so this will implement all phase 0, 1 and 2 variables -// Note: This function will stop after an interruption -// Note: Do not manually fill any request variables -func (tx *Transaction) ProcessRequest(req *http.Request) (*types.Interruption, error) { - var client string - cport := 0 - // IMPORTANT: Some http.Request.RemoteAddr implementations will not contain port or contain IPV6: [2001:db8::1]:8080 - spl := strings.Split(req.RemoteAddr, ":") - if len(spl) > 1 { - client = strings.Join(spl[0:len(spl)-1], "") - cport, _ = strconv.Atoi(spl[len(spl)-1]) - } - var in *types.Interruption - // There is no socket access in the request object so we don't know the server client or port - tx.ProcessConnection(client, cport, "", 0) - tx.ProcessURI(req.URL.String(), req.Method, req.Proto) - for k, vr := range req.Header { - for _, v := range vr { - tx.AddRequestHeader(k, v) - } - } - // Host will always be removed from req.Headers(), so we manually add it - if req.Host != "" { - tx.AddRequestHeader("Host", req.Host) - } - - in = tx.ProcessRequestHeaders() - if in != nil { - return in, nil - } - if req.Body != nil { - _, err := io.Copy(tx.RequestBodyBuffer, req.Body) - if err != nil { - return tx.Interruption, err - } - reader, err := tx.RequestBodyBuffer.Reader() - if err != nil { - return tx.Interruption, err - } - req.Body = io.NopCloser(reader) - } - return tx.ProcessRequestBody() -} - // ProcessConnection should be called at very beginning of a request process, it is // expected to be executed prior to the virtual host resolution, when the // connection arrives on the server. diff --git a/transaction_test.go b/transaction_test.go index ba1085f8a..c8d559887 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -15,7 +15,6 @@ package coraza import ( - "bufio" "bytes" "context" "fmt" @@ -227,21 +226,6 @@ func TestAuditLogFields(t *testing.T) { } } -func TestRequestStruct(t *testing.T) { - req, _ := http.NewRequest("POST", "https://www.coraza.io/test", strings.NewReader("test=456")) - waf := NewWaf() - tx := waf.NewTransaction(context.Background()) - if _, err := tx.ProcessRequest(req); err != nil { - t.Error(err) - } - if tx.Variables.RequestMethod.String() != "POST" { - t.Error("failed to set request from request object") - } - if err := tx.Clean(); err != nil { - t.Error(err) - } -} - func TestResetCapture(t *testing.T) { tx := makeTransaction() tx.Capture = true @@ -413,28 +397,6 @@ func TestAuditLogMessages(t *testing.T) { } -func TestProcessRequestMultipart(t *testing.T) { - req, _ := http.NewRequest("POST", "/some", nil) - if err := multipartRequest(req); err != nil { - t.Fatal(err) - } - tx := makeTransaction() - tx.RequestBodyAccess = true - if _, err := tx.ProcessRequest(req); err != nil { - t.Error(err) - } - if req.Body == nil { - t.Error("failed to process multipart request") - } - reader := bufio.NewReader(req.Body) - if _, err := reader.ReadString('\n'); err != nil { - t.Error("failed to read multipart request", err) - } - if err := tx.Clean(); err != nil { - t.Error(err) - } -} - func TestTransactionSyncPool(t *testing.T) { waf := NewWaf() tx := waf.NewTransaction(context.Background()) diff --git a/transformations/base64decode.go b/transformations/base64decode.go index 21e8868d9..114a86cba 100644 --- a/transformations/base64decode.go +++ b/transformations/base64decode.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http:// www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, From 0e842362ce26bae6f534514970b5a86c193fbc8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Thu, 21 Jul 2022 12:17:34 +0200 Subject: [PATCH 2/4] fix: moves missing function. --- http/http_test.go | 35 +++++++++++++++++++++++++++++++++++ transaction_test.go | 36 ------------------------------------ 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/http/http_test.go b/http/http_test.go index b31c9dd74..27ae6c5f7 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -16,8 +16,13 @@ package http import ( "bufio" + "bytes" "context" + "io" + "io/ioutil" + "mime/multipart" "net/http" + "os" "strings" "testing" ) @@ -58,3 +63,33 @@ func TestProcessRequestMultipart(t *testing.T) { t.Error(err) } } + +func multipartRequest(req *http.Request) error { + var b bytes.Buffer + w := multipart.NewWriter(&b) + tempfile, err := os.CreateTemp("/tmp", "tmpfile*") + if err != nil { + return err + } + defer os.Remove(tempfile.Name()) + for i := 0; i < 1024*5; i++ { + // this should create a 5mb file + if _, err := tempfile.Write([]byte(strings.Repeat("A", 1024))); err != nil { + return err + } + } + var fw io.Writer + if fw, err = w.CreateFormFile("fupload", tempfile.Name()); err != nil { + return err + } + if _, err := tempfile.Seek(0, 0); err != nil { + return err + } + if _, err = io.Copy(fw, tempfile); err != nil { + return err + } + req.Body = ioutil.NopCloser(&b) + req.Header.Set("Content-Type", w.FormDataContentType()) + req.Method = "POST" + return nil +} diff --git a/transaction_test.go b/transaction_test.go index c8d559887..ebca76626 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -15,14 +15,8 @@ package coraza import ( - "bytes" "context" "fmt" - "io" - "io/ioutil" - "mime/multipart" - "net/http" - "os" "regexp" "runtime/debug" "strconv" @@ -539,36 +533,6 @@ func TestTXProcessURI(t *testing.T) { } } -func multipartRequest(req *http.Request) error { - var b bytes.Buffer - w := multipart.NewWriter(&b) - tempfile, err := os.CreateTemp("/tmp", "tmpfile*") - if err != nil { - return err - } - defer os.Remove(tempfile.Name()) - for i := 0; i < 1024*5; i++ { - // this should create a 5mb file - if _, err := tempfile.Write([]byte(strings.Repeat("A", 1024))); err != nil { - return err - } - } - var fw io.Writer - if fw, err = w.CreateFormFile("fupload", tempfile.Name()); err != nil { - return err - } - if _, err := tempfile.Seek(0, 0); err != nil { - return err - } - if _, err = io.Copy(fw, tempfile); err != nil { - return err - } - req.Body = ioutil.NopCloser(&b) - req.Header.Set("Content-Type", w.FormDataContentType()) - req.Method = "POST" - return nil -} - func BenchmarkTransactionCreation(b *testing.B) { waf := NewWaf() for i := 0; i < b.N; i++ { From 14cf5d6a5b30e465c789df5cae06009f7ff1c658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Thu, 21 Jul 2022 12:39:53 +0200 Subject: [PATCH 3/4] fix: makes lint happy. --- http/http.go | 3 ++- http/http_test.go | 29 +++++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/http/http.go b/http/http.go index 666e8780c..8cd210a92 100644 --- a/http/http.go +++ b/http/http.go @@ -20,6 +20,7 @@ import ( "strconv" "strings" + "github.com/corazawaf/coraza/v3" "github.com/corazawaf/coraza/v3/types" ) @@ -28,7 +29,7 @@ import ( // so this will implement all phase 0, 1 and 2 variables // Note: This function will stop after an interruption // Note: Do not manually fill any request variables -func ProcessRequest(tx *Transaction, req *http.Request) (*types.Interruption, error) { +func ProcessRequest(tx *coraza.Transaction, req *http.Request) (*types.Interruption, error) { var client string cport := 0 // IMPORTANT: Some http.Request.RemoteAddr implementations will not contain port or contain IPV6: [2001:db8::1]:8080 diff --git a/http/http_test.go b/http/http_test.go index 27ae6c5f7..e278c937f 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -25,13 +25,15 @@ import ( "os" "strings" "testing" + + "github.com/corazawaf/coraza/v3" ) func TestRequestExtractionSuccess(t *testing.T) { req, _ := http.NewRequest("POST", "https://www.coraza.io/test", strings.NewReader("test=456")) - waf := NewWaf() + waf := coraza.NewWaf() tx := waf.NewTransaction(context.Background()) - if _, err := tx.ProcessRequest(req); err != nil { + if _, err := ProcessRequest(tx, req); err != nil { t.Fatal(err) } if tx.Variables.RequestMethod.String() != "POST" { @@ -47,9 +49,9 @@ func TestProcessRequestMultipart(t *testing.T) { if err := multipartRequest(req); err != nil { t.Fatal(err) } - tx := makeTransaction() + tx := makeTransaction(t) tx.RequestBodyAccess = true - if _, err := tx.ProcessRequest(req); err != nil { + if _, err := ProcessRequest(tx, req); err != nil { t.Fatal(err) } if req.Body == nil { @@ -93,3 +95,22 @@ func multipartRequest(req *http.Request) error { req.Method = "POST" return nil } + +func makeTransaction(t *testing.T) *coraza.Transaction { + t.Helper() + tx := coraza.NewWaf().NewTransaction(context.Background()) + tx.RequestBodyAccess = true + ht := []string{ + "POST /testurl.php?id=123&b=456 HTTP/1.1", + "Host: www.test.com:80", + "Cookie: test=123", + "Content-Type: application/x-www-form-urlencoded", + "X-Test-Header: test456", + "Content-Length: 13", + "", + "testfield=456", + } + data := strings.Join(ht, "\r\n") + _, _ = tx.ParseRequestReader(strings.NewReader(data)) + return tx +} From e20116890eae96aecc99ef3e271f71f4562aef8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Thu, 21 Jul 2022 13:39:43 +0200 Subject: [PATCH 4/4] fix: seclang tests. --- seclang/rules_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/seclang/rules_test.go b/seclang/rules_test.go index 777242e53..33a7cf5a0 100644 --- a/seclang/rules_test.go +++ b/seclang/rules_test.go @@ -26,6 +26,7 @@ import ( "testing" "github.com/corazawaf/coraza/v3" + txhttp "github.com/corazawaf/coraza/v3/http" "github.com/corazawaf/coraza/v3/types" ) @@ -394,7 +395,7 @@ func TestDirectiveSecAuditLog(t *testing.T) { if err != nil { t.Errorf("Description HTTP request parsing failed") } - _, err = tx.ProcessRequest(req) + _, err = txhttp.ProcessRequest(tx, req) if err != nil { t.Errorf("Failed to load the HTTP request") }