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

Add the context on Envoy requests to the audit logs #256

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
105 changes: 105 additions & 0 deletions wasmplugin/auditlogger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package wasmplugin

import (
"fmt"
"sync"

ctypes "github.com/corazawaf/coraza/v3/types"
"github.com/tetratelabs/proxy-wasm-go-sdk/proxywasm"
)

// Transaction context to store against a transaction ID
type TxnContext struct {
envoyRequestId string
}

// Logger that includes context from the request in the audit logs and owns the final formatting
type ContextualAuditLogger struct {
IncludeRequestContext bool
txnContextMap map[string]*TxnContext
lock sync.Mutex
}

// Get the global audit logger that can be used across all requests
func NewAppAuditLogger(includeRequestContext bool) *ContextualAuditLogger {
return &ContextualAuditLogger{
txnContextMap: make(map[string]*TxnContext),
IncludeRequestContext: includeRequestContext,
}
}

// Register a transaction with the logger
func (cal *ContextualAuditLogger) Register(txnId string, ctx *TxnContext) {
cal.lock.Lock()
defer cal.lock.Unlock()

cal.txnContextMap[txnId] = ctx
}

// Remove the transaction information from the context map
func (cal *ContextualAuditLogger) Unregister(txnId string) {
cal.lock.Lock()
defer cal.lock.Unlock()

delete(cal.txnContextMap, txnId)
}

// Emit log on the given rule and add the txn context if available
func (cal *ContextualAuditLogger) AuditLog(rule ctypes.MatchedRule) {
cal.lock.Lock()
defer cal.lock.Unlock()

txnId := rule.TransactionID()

logPrefix := ""
if ctx, ok := cal.txnContextMap[txnId]; ok {
if cal.IncludeRequestContext {
// If we have context, add it to the log
logPrefix = fmt.Sprintf("[request-id %q] ", ctx.envoyRequestId)
}
}

logError(rule, logPrefix)
}

func logError(error ctypes.MatchedRule, logPrefix string) {
msg := logPrefix + error.ErrorLog()
switch error.Rule().Severity() {
case ctypes.RuleSeverityEmergency:
proxywasm.LogCritical(msg)
case ctypes.RuleSeverityAlert:
proxywasm.LogCritical(msg)
case ctypes.RuleSeverityCritical:
proxywasm.LogCritical(msg)
case ctypes.RuleSeverityError:
proxywasm.LogError(msg)
case ctypes.RuleSeverityWarning:
proxywasm.LogWarn(msg)
case ctypes.RuleSeverityNotice:
proxywasm.LogInfo(msg)
case ctypes.RuleSeverityInfo:
proxywasm.LogInfo(msg)
case ctypes.RuleSeverityDebug:
proxywasm.LogDebug(msg)
}
}

const (
// This is the standard Envoy header for request IDs
envoyRequestIdHeader = "x-request-id"
)

// A convenience method to register the request information with the audit logger if available
// on the request (else ignores). Must be called in the request context.
func registerRequestContextWithLogger(auditLogger *ContextualAuditLogger, txnId string) {
if id, err := proxywasm.GetHttpRequestHeader(envoyRequestIdHeader); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a request header, shouldn't it be automatically outputted in the headers (B) part of audit logs?

Copy link
Contributor

@anuraaga anuraaga Feb 18, 2024

Choose a reason for hiding this comment

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

Ah I see in corazawaf/coraza#711 there is the issue that all headers are printed which may not be desired. That seems like a big issue with seclang syntax, I would first add a config to seclang to set an allow or deny list for logging headers as without it the B part seems implementation seems incomplete.

While generic selection of variables for logging may have a different use case, for headers it doesn't seem like it should be needed, we already have a part for it we should improve that.

Copy link
Author

Choose a reason for hiding this comment

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

I could be wrong but it seems like the WASM filter here can't really print all the parts and just a fixed set of params are printed. See this issue I opened: #255

Is this not correct? I didn't find anything in the code that might print anything but these:
https://github.com/corazawaf/coraza-proxy-wasm/blob/main/wasmplugin/plugin.go#L699

The ErrorLog() method is here:
https://github.com/corazawaf/coraza/blob/f68d1c208791d741581a7d413c32777fbd74c611/internal/corazarules/rule_match.go#L254

Copy link
Author

Choose a reason for hiding this comment

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

Again, I am fairly new to this so I could be wrong

auditLogger.Register(txnId, &TxnContext{
envoyRequestId: id,
})
}
}

// Remove context for the given transaction ID
func removeRequestContextFromLogger(auditLogger *ContextualAuditLogger, txnId string) {
auditLogger.Unregister(txnId)
}
14 changes: 10 additions & 4 deletions wasmplugin/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import (

// pluginConfiguration is a type to represent an example configuration for this wasm plugin.
type pluginConfiguration struct {
directivesMap DirectivesMap
metricLabels map[string]string
defaultDirectives string
perAuthorityDirectives map[string]string
directivesMap DirectivesMap
metricLabels map[string]string
defaultDirectives string
perAuthorityDirectives map[string]string
includeRequestIdInAuditLogs bool
}

type DirectivesMap map[string][]string
Expand All @@ -33,6 +34,11 @@ func parsePluginConfiguration(data []byte, infoLogger func(string)) (pluginConfi
}

jsonData := gjson.ParseBytes(data)
includeReqId := jsonData.Get("include_request_id_in_audit_logs")
if includeReqId.Exists() && includeReqId.IsBool() && includeReqId.Bool() {
config.includeRequestIdInAuditLogs = true
}

config.directivesMap = make(DirectivesMap)
jsonData.Get("directives_map").ForEach(func(key, value gjson.Result) bool {
directiveName := key.String()
Expand Down
37 changes: 13 additions & 24 deletions wasmplugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewVMContext() types.VMContext {
return &vmContext{}
}

func (*vmContext) NewPluginContext(contextID uint32) types.PluginContext {
func (vc *vmContext) NewPluginContext(contextID uint32) types.PluginContext {
return &corazaPlugin{}
}

Expand Down Expand Up @@ -80,6 +80,7 @@ type corazaPlugin struct {
perAuthorityWAFs wafMap
metricLabelsKV []string
metrics *wafMetrics
auditLogger *ContextualAuditLogger
}

func (ctx *corazaPlugin) OnPluginStart(pluginConfigurationSize int) types.OnPluginStartStatus {
Expand All @@ -94,6 +95,8 @@ func (ctx *corazaPlugin) OnPluginStart(pluginConfigurationSize int) types.OnPlug
return types.OnPluginStartStatusFailed
}

ctx.auditLogger = NewAppAuditLogger(config.includeRequestIdInAuditLogs)

// directivesAuthoritesMap is a map of directives name to the list of
// authorities that reference those directives. This is used to
// initialize the WAFs only for the directives that are referenced
Expand Down Expand Up @@ -123,7 +126,7 @@ func (ctx *corazaPlugin) OnPluginStart(pluginConfigurationSize int) types.OnPlug

// First we initialize our waf and our seclang parser
conf := coraza.NewWAFConfig().
WithErrorCallback(logError).
WithErrorCallback(ctx.auditLogger.AuditLog).
WithDebugLogger(debuglog.DefaultWithPrinterFactory(logPrinterFactory)).
// TODO(anuraaga): Make this configurable in plugin configuration.
// WithRequestBodyLimit(1024 * 1024 * 1024).
Expand Down Expand Up @@ -181,6 +184,7 @@ func (ctx *corazaPlugin) NewHttpContext(contextID uint32) types.HttpContext {
metrics: ctx.metrics,
metricLabelsKV: ctx.metricLabelsKV,
perAuthorityWAFs: ctx.perAuthorityWAFs,
auditLogger: ctx.auditLogger,
}
}

Expand Down Expand Up @@ -228,6 +232,7 @@ type httpContext struct {
interruptedAt interruptionPhase
logger debuglog.Logger
metricLabelsKV []string
auditLogger *ContextualAuditLogger
}

func (ctx *httpContext) OnHttpRequestHeaders(numHeaders int, endOfStream bool) types.Action {
Expand All @@ -245,6 +250,7 @@ func (ctx *httpContext) OnHttpRequestHeaders(numHeaders int, endOfStream bool) t
}
authority = string(propHostRaw)
}

if waf, isDefault, resolveWAFErr := ctx.perAuthorityWAFs.getWAFOrDefault(authority); resolveWAFErr == nil {
ctx.tx = waf.NewTransaction()

Expand All @@ -268,6 +274,9 @@ func (ctx *httpContext) OnHttpRequestHeaders(numHeaders int, endOfStream bool) t

tx := ctx.tx

// Register context with audit logging context
registerRequestContextWithLogger(ctx.auditLogger, ctx.tx.ID())

// This currently relies on Envoy's behavior of mapping all requests to HTTP/2 semantics
// and its request properties, but they may not be true of other proxies implementing
// proxy-wasm.
Expand Down Expand Up @@ -632,6 +641,8 @@ func (ctx *httpContext) OnHttpResponseBody(bodySize int, endOfStream bool) types
}

func (ctx *httpContext) OnHttpStreamDone() {
// Cleanup transaction ID from the audit logging context
defer removeRequestContextFromLogger(ctx.auditLogger, ctx.tx.ID())
defer logTime("OnHttpStreamDone", currentTime())
tx := ctx.tx

Expand Down Expand Up @@ -695,28 +706,6 @@ func (ctx *httpContext) handleInterruption(phase interruptionPhase, interruption
return types.ActionPause
}

func logError(error ctypes.MatchedRule) {
msg := error.ErrorLog()
switch error.Rule().Severity() {
case ctypes.RuleSeverityEmergency:
proxywasm.LogCritical(msg)
case ctypes.RuleSeverityAlert:
proxywasm.LogCritical(msg)
case ctypes.RuleSeverityCritical:
proxywasm.LogCritical(msg)
case ctypes.RuleSeverityError:
proxywasm.LogError(msg)
case ctypes.RuleSeverityWarning:
proxywasm.LogWarn(msg)
case ctypes.RuleSeverityNotice:
proxywasm.LogInfo(msg)
case ctypes.RuleSeverityInfo:
proxywasm.LogInfo(msg)
case ctypes.RuleSeverityDebug:
proxywasm.LogDebug(msg)
}
}

// retrieveAddressInfo retrieves address properties from the proxy
// Expected targets are "source" or "destination"
// Envoy ref: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes#connection-attributes
Expand Down
Loading