Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: TIME variables support #1223

Merged
merged 3 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions internal/corazawaf/rule_multiphase.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@
return types.PhaseResponseHeaders
case variables.UniqueID:
return types.PhaseRequestHeaders
case variables.Time:
return types.PhaseRequestHeaders
case variables.TimeDay:
return types.PhaseRequestHeaders
case variables.TimeEpoch:
return types.PhaseRequestHeaders
case variables.TimeHour:
return types.PhaseRequestHeaders
case variables.TimeMin:
return types.PhaseRequestHeaders
case variables.TimeMon:
return types.PhaseRequestHeaders
case variables.TimeSec:
return types.PhaseRequestHeaders
case variables.TimeWday:
return types.PhaseRequestHeaders
case variables.TimeYear:
return types.PhaseRequestHeaders

Check warning on line 67 in internal/corazawaf/rule_multiphase.go

View check run for this annotation

Codecov / codecov/patch

internal/corazawaf/rule_multiphase.go#L50-L67

Added lines #L50 - L67 were not covered by tests
case variables.ArgsCombinedSize:
// Size changes between phase 1 and 2 so evaluate both times
return types.PhaseRequestHeaders
Expand Down
77 changes: 77 additions & 0 deletions internal/corazawaf/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,24 @@ func (tx *Transaction) Collection(idx variables.RuleVariable) collection.Collect
return tx.variables.multipartPartHeaders
case variables.MultipartStrictError:
return tx.variables.multipartStrictError
case variables.Time:
return tx.variables.time
case variables.TimeDay:
return tx.variables.timeDay
case variables.TimeEpoch:
return tx.variables.timeEpoch
case variables.TimeHour:
return tx.variables.timeHour
case variables.TimeMin:
return tx.variables.timeMin
case variables.TimeMon:
return tx.variables.timeMon
case variables.TimeSec:
return tx.variables.timeSec
case variables.TimeWday:
return tx.variables.timeWday
case variables.TimeYear:
return tx.variables.timeYear
}

return collections.Noop
Expand Down Expand Up @@ -1591,6 +1609,20 @@ func (tx *Transaction) generateResponseBodyError(err error) {
tx.variables.resBodyProcessorErrorMsg.Set(err.Error())
}

// setTimeVariables sets all the time variables
func (tx *Transaction) setTimeVariables() {
timestamp := time.Unix(0, tx.Timestamp)
tx.variables.time.Set(timestamp.Format(time.TimeOnly))
tx.variables.timeDay.Set(strconv.Itoa(timestamp.Day()))
tx.variables.timeEpoch.Set(strconv.FormatInt(timestamp.Unix(), 10))
tx.variables.timeHour.Set(strconv.Itoa(timestamp.Hour()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am inclined to say we can come up with a map for this instead of doing the conversion since these values are deterministic. Still worth to write a benchmark

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you propose to use a TIME_MAP variable with keys such as hour, day, month, year, and so on? Did I get this right?

If so, this approach would further widen the compatibility gap between ModSecurity and Coraza, whereas the main goal of this PR is to reduce it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if I wasn't clear enough. I was advocating for having a map in memory e.g.

var hourConversion map[int]string = map[int]string{1: "1", 2: "2", ...}

instead of doing the strconv.Itoa.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of benchmarking, I run BenchmarkTransactionCreation - because all the values are set in the constructor.

Before changes:

cpu: Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz
BenchmarkTransactionCreation
BenchmarkTransactionCreation-8   	   53013	     21910 ns/op
PASS

After changes:

BenchmarkTransactionCreation
BenchmarkTransactionCreation-8   	   51748	     22462 ns/op
PASS

However, the results vary widely from run to run. Am I missing something? Maybe, you could explain in more detail, what should I benchmark?

tx.variables.timeMin.Set(strconv.Itoa(timestamp.Minute()))
tx.variables.timeSec.Set(strconv.Itoa(timestamp.Second()))
tx.variables.timeWday.Set(strconv.Itoa(int(timestamp.Weekday())))
tx.variables.timeMon.Set(strconv.Itoa(int(timestamp.Month())))
tx.variables.timeYear.Set(strconv.Itoa(timestamp.Year()))
}

// TransactionVariables has pointers to all the variables of the transaction
type TransactionVariables struct {
args *collections.ConcatKeyed
Expand Down Expand Up @@ -1669,6 +1701,15 @@ type TransactionVariables struct {
resBodyErrorMsg *collections.Single
resBodyProcessorError *collections.Single
resBodyProcessorErrorMsg *collections.Single
time *collections.Single
timeDay *collections.Single
timeEpoch *collections.Single
timeHour *collections.Single
timeMin *collections.Single
timeMon *collections.Single
timeSec *collections.Single
timeWday *collections.Single
timeYear *collections.Single
}

func NewTransactionVariables() *TransactionVariables {
Expand Down Expand Up @@ -1741,6 +1782,15 @@ func NewTransactionVariables() *TransactionVariables {
v.requestXML = collections.NewMap(variables.RequestXML)
v.multipartPartHeaders = collections.NewMap(variables.MultipartPartHeaders)
v.multipartStrictError = collections.NewSingle(variables.MultipartStrictError)
v.time = collections.NewSingle(variables.Time)
v.timeDay = collections.NewSingle(variables.TimeDay)
v.timeEpoch = collections.NewSingle(variables.TimeEpoch)
v.timeHour = collections.NewSingle(variables.TimeHour)
v.timeMin = collections.NewSingle(variables.TimeMin)
v.timeMon = collections.NewSingle(variables.TimeMon)
v.timeSec = collections.NewSingle(variables.TimeSec)
v.timeWday = collections.NewSingle(variables.TimeWday)
v.timeYear = collections.NewSingle(variables.TimeYear)

// XML is a pointer to RequestXML
v.xml = v.requestXML
Expand Down Expand Up @@ -2299,6 +2349,33 @@ func (v *TransactionVariables) All(f func(v variables.RuleVariable, col collecti
if !f(variables.XML, v.xml) {
return
}
if !f(variables.Time, v.time) {
return
}
if !f(variables.TimeDay, v.timeDay) {
return
}
if !f(variables.TimeEpoch, v.timeEpoch) {
return
}
if !f(variables.TimeHour, v.timeHour) {
return
}
if !f(variables.TimeMin, v.timeMin) {
return
}
if !f(variables.TimeMon, v.timeMon) {
return
}
if !f(variables.TimeSec, v.timeSec) {
return
}
if !f(variables.TimeWday, v.timeWday) {
return
}
if !f(variables.TimeYear, v.timeYear) {
return
}
}

type formattable interface {
Expand Down
30 changes: 30 additions & 0 deletions internal/corazawaf/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"testing"
"time"

"github.com/corazawaf/coraza/v3/collection"
"github.com/corazawaf/coraza/v3/debuglog"
Expand Down Expand Up @@ -67,6 +68,23 @@ func TestTxSetters(t *testing.T) {

validateMacroExpansion(exp, tx, t)
}

func TestTxTime(t *testing.T) {
tx := makeTransactionTimestamped(t)
exp := map[string]string{
"%{TIME}": "15:27:34",
"%{TIME_DAY}": "18",
"%{TIME_EPOCH}": fmt.Sprintf("%d", tx.Timestamp/1e9), // 1731943654 in UTC, may differ in local timezone
"%{TIME_HOUR}": "15",
"%{TIME_MIN}": "27",
"%{TIME_MON}": "11",
"%{TIME_SEC}": "34",
"%{TIME_WDAY}": "1",
"%{TIME_YEAR}": "2024",
}
validateMacroExpansion(exp, tx, t)
}

func TestTxMultipart(t *testing.T) {
tx := NewWAF().NewTransaction()
body := []string{
Expand Down Expand Up @@ -1360,6 +1378,18 @@ func makeTransaction(t testing.TB) *Transaction {
return tx
}

func makeTransactionTimestamped(t testing.TB) *Transaction {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder by testing.TB

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same interface as in makeTransaction and makeTransactionMultipart to ensure uniformity.

t.Helper()
tx := NewWAF().NewTransaction()
timestamp, err := time.ParseInLocation(time.DateTime, "2024-11-18 15:27:34", time.Local)
if err != nil {
panic(err)
}
tx.Timestamp = timestamp.UnixNano()
tx.setTimeVariables()
return tx
}

func makeTransactionMultipart(t *testing.T) *Transaction {
if t != nil {
t.Helper()
Expand Down
1 change: 1 addition & 0 deletions internal/corazawaf/waf.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ func (w *WAF) newTransaction(opts Options) *Transaction {
tx.variables.duration.Set("0")
tx.variables.highestSeverity.Set("0")
tx.variables.uniqueID.Set(tx.id)
tx.setTimeVariables()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't better to just remove this call and generate the value for each variable at access time instead? Kind of being more an init than set.

The lazy collections idea is generic and will work. I would like to see if we add extra memory/benchmarks.

Another option is to have specific time collections that receive the timestamp, and the function to apply when setting and only evaluate when getting. I guess in the end is more or less the same?


tx.debugLogger.Debug().Msg("Transaction started")

Expand Down
20 changes: 19 additions & 1 deletion internal/variables/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//go:generate go run generator/main.go

// Package variables contains the representation of the variables used in the rules
// Variables are created as bytes and they have a string representation
// Variables are created as bytes, and they have a string representation
package variables

// This internal file contains all variables supported by handling of SecLang, such as
Expand Down Expand Up @@ -236,4 +236,22 @@ const (
ResBodyProcessorError
// ResBodyProcessorErrorMsg
ResBodyProcessorErrorMsg
// Time holds a formatted string representing the time (hour:minute:second).
Time
// TimeDay holds the current day of the month (1-31)
TimeDay
// TimeEpoch holds the time in seconds since 1970
TimeEpoch
// TimeHour holds the current hour of the day (0-23)
TimeHour
// TimeMin holds the current minute of the hour (0-59)
TimeMin
// TimeMon holds the current month of the year (0-11)
TimeMon
// TimeSec holds the current second of the minute (0-59)
TimeSec
// TimeWday holds the current weekday value (1–7), where Monday is 1
TimeWday
// TimeYear the current four-digit year value
TimeYear
)
2 changes: 1 addition & 1 deletion internal/variables/variables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func TestNameToVariable(t *testing.T) {
vars := []string{"URLENCODED_ERROR", "RESPONSE_CONTENT_TYPE", "UNIQUE_ID", "ARGS_COMBINED_SIZE", "AUTH_TYPE", "FILES_COMBINED_SIZE", "FULL_REQUEST", "FULL_REQUEST_LENGTH", "INBOUND_DATA_ERROR", "MATCHED_VAR", "MATCHED_VAR_NAME", "MULTIPART_BOUNDARY_QUOTED", "MULTIPART_BOUNDARY_WHITESPACE", "MULTIPART_CRLF_LF_LINES", "MULTIPART_DATA_AFTER", "MULTIPART_DATA_BEFORE", "MULTIPART_FILE_LIMIT_EXCEEDED", "MULTIPART_HEADER_FOLDING", "MULTIPART_INVALID_HEADER_FOLDING", "MULTIPART_INVALID_PART", "MULTIPART_INVALID_QUOTING", "MULTIPART_LF_LINE", "MULTIPART_MISSING_SEMICOLON", "MULTIPART_STRICT_ERROR", "MULTIPART_UNMATCHED_BOUNDARY", "OUTBOUND_DATA_ERROR", "PATH_INFO", "QUERY_STRING", "REMOTE_ADDR", "REMOTE_HOST", "REMOTE_PORT", "REQBODY_ERROR", "REQBODY_ERROR_MSG", "REQBODY_PROCESSOR_ERROR", "REQBODY_PROCESSOR_ERROR_MSG", "REQBODY_PROCESSOR", "REQUEST_BASENAME", "REQUEST_BODY", "REQUEST_BODY_LENGTH", "REQUEST_FILENAME", "REQUEST_LINE", "REQUEST_METHOD", "REQUEST_PROTOCOL", "REQUEST_URI", "REQUEST_URI_RAW", "RESPONSE_BODY", "RESPONSE_CONTENT_LENGTH", "RESPONSE_PROTOCOL", "RESPONSE_STATUS", "SERVER_ADDR", "SERVER_NAME", "SERVER_PORT", "SESSIONID", "RESPONSE_HEADERS_NAMES", "REQUEST_HEADERS_NAMES", "USERID", "ARGS", "ARGS_GET", "ARGS_POST", "FILES_SIZES", "FILES_NAMES", "FILES_TMP_CONTENT", "MULTIPART_FILENAME", "MULTIPART_NAME", "MATCHED_VARS_NAMES", "MATCHED_VARS", "FILES", "REQUEST_COOKIES", "REQUEST_HEADERS", "RESPONSE_HEADERS", "GEO", "REQUEST_COOKIES_NAMES", "FILES_TMPNAMES", "ARGS_NAMES", "ARGS_GET_NAMES", "ARGS_POST_NAMES", "RULE", "XML", "TX", "DURATION"}
vars := []string{"URLENCODED_ERROR", "RESPONSE_CONTENT_TYPE", "UNIQUE_ID", "ARGS_COMBINED_SIZE", "AUTH_TYPE", "FILES_COMBINED_SIZE", "FULL_REQUEST", "FULL_REQUEST_LENGTH", "INBOUND_DATA_ERROR", "MATCHED_VAR", "MATCHED_VAR_NAME", "MULTIPART_BOUNDARY_QUOTED", "MULTIPART_BOUNDARY_WHITESPACE", "MULTIPART_CRLF_LF_LINES", "MULTIPART_DATA_AFTER", "MULTIPART_DATA_BEFORE", "MULTIPART_FILE_LIMIT_EXCEEDED", "MULTIPART_HEADER_FOLDING", "MULTIPART_INVALID_HEADER_FOLDING", "MULTIPART_INVALID_PART", "MULTIPART_INVALID_QUOTING", "MULTIPART_LF_LINE", "MULTIPART_MISSING_SEMICOLON", "MULTIPART_STRICT_ERROR", "MULTIPART_UNMATCHED_BOUNDARY", "OUTBOUND_DATA_ERROR", "PATH_INFO", "QUERY_STRING", "REMOTE_ADDR", "REMOTE_HOST", "REMOTE_PORT", "REQBODY_ERROR", "REQBODY_ERROR_MSG", "REQBODY_PROCESSOR_ERROR", "REQBODY_PROCESSOR_ERROR_MSG", "REQBODY_PROCESSOR", "REQUEST_BASENAME", "REQUEST_BODY", "REQUEST_BODY_LENGTH", "REQUEST_FILENAME", "REQUEST_LINE", "REQUEST_METHOD", "REQUEST_PROTOCOL", "REQUEST_URI", "REQUEST_URI_RAW", "RESPONSE_BODY", "RESPONSE_CONTENT_LENGTH", "RESPONSE_PROTOCOL", "RESPONSE_STATUS", "SERVER_ADDR", "SERVER_NAME", "SERVER_PORT", "SESSIONID", "RESPONSE_HEADERS_NAMES", "REQUEST_HEADERS_NAMES", "USERID", "ARGS", "ARGS_GET", "ARGS_POST", "FILES_SIZES", "FILES_NAMES", "FILES_TMP_CONTENT", "MULTIPART_FILENAME", "MULTIPART_NAME", "MATCHED_VARS_NAMES", "MATCHED_VARS", "FILES", "REQUEST_COOKIES", "REQUEST_HEADERS", "RESPONSE_HEADERS", "GEO", "REQUEST_COOKIES_NAMES", "FILES_TMPNAMES", "ARGS_NAMES", "ARGS_GET_NAMES", "ARGS_POST_NAMES", "RULE", "XML", "TX", "DURATION", "TIME", "TIME_DAY", "TIME_EPOCH", "TIME_HOUR", "TIME_MIN", "TIME_MON", "TIME_SEC", "TIME_WDAY", "TIME_YEAR"}
for _, v := range vars {
_, err := Parse(v)
if err != nil {
Expand Down
27 changes: 27 additions & 0 deletions internal/variables/variablesmap.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions types/variables/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,24 @@ const (
ResBodyProcessorErrorMsg = variables.ResBodyProcessorErrorMsg
// MultipartStrictError will be set to 1 when there is an error parsing multipart
MultipartStrictError = variables.MultipartStrictError
// Time holds a formatted string representing the time (hour:minute:second).
Time = variables.Time
// TimeDay holds the current day of the month (1-31)
TimeDay = variables.TimeDay
// TimeEpoch holds the time in seconds since 1970
TimeEpoch = variables.TimeEpoch
// TimeHour holds the current hour of the day (0-23)
TimeHour = variables.TimeHour
// TimeMin holds the current minute of the hour (0-59)
TimeMin = variables.TimeMin
// TimeMon holds the current month of the year (0-11)
TimeMon = variables.TimeMon
// TimeSec holds the current second of the minute (0-59)
TimeSec = variables.TimeSec
// TimeWday holds the current weekday value (1–7), where Monday is 1
TimeWday = variables.TimeWday
// TimeYear the current four-digit year value
TimeYear = variables.TimeYear
)

// Parse returns the byte interpretation
Expand Down
Loading