Skip to content

Commit

Permalink
Replaced transient with ephemeral addresses
Browse files Browse the repository at this point in the history
  • Loading branch information
ValentinZakharov committed Feb 23, 2024
1 parent 6d93153 commit 28afe26
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.datadog.appsec.event.data.Address;
import com.datadog.appsec.event.data.DataBundle;
import com.datadog.appsec.event.data.KnownAddresses;
import com.datadog.appsec.report.AppSecEvent;
import com.datadog.appsec.util.StandardizedLogging;
import datadog.trace.api.http.StoredBodySupplier;
Expand Down Expand Up @@ -74,6 +73,7 @@ public class AppSecRequestContext implements DataBundle, Closeable {
private boolean rawReqBodyPublished;
private boolean convertedReqBodyPublished;
private boolean respDataPublished;
private boolean pathParamsPublished;
private Map<String, String> apiSchemas;

// should be guarded by this
Expand Down Expand Up @@ -327,7 +327,11 @@ public void setReqDataPublished(boolean reqDataPublished) {
}

public boolean isPathParamsPublished() {
return persistentData.containsKey(KnownAddresses.REQUEST_PATH_PARAMS);
return pathParamsPublished;
}

public void setPathParamsPublished(boolean pathParamsPublished) {
this.pathParamsPublished = pathParamsPublished;
}

public boolean isRawReqBodyPublished() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,10 @@ public void init() {
EVENTS.requestPathParams(),
(ctx_, data) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return NoopFlow.INSTANCE;
}

if (ctx.isPathParamsPublished()) {
log.debug("Second or subsequent publication of request params");
if (ctx == null || ctx.isPathParamsPublished()) {
return NoopFlow.INSTANCE;
}
ctx.setPathParamsPublished(true);

while (true) {
DataSubscriberInfo subInfo = pathParamsSubInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,7 @@ private Powerwaf.ResultWithData doRunPowerwaf(
PowerwafMetrics metrics = reqCtx.getWafMetrics();

if (isTransient) {
DataBundle bundle = DataBundle.unionOf(newData, reqCtx);
return runPowerwafTransient(metrics, bundle, ctxAndAddr);
return runPowerwafTransient(additive, metrics, newData, ctxAndAddr);
} else {
return runPowerwafAdditive(additive, metrics, newData, ctxAndAddr);
}
Expand All @@ -527,9 +526,9 @@ private Powerwaf.ResultWithData runPowerwafAdditive(
}

private Powerwaf.ResultWithData runPowerwafTransient(
PowerwafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr)
Additive additive, PowerwafMetrics metrics, DataBundle bundle, CtxAndAddresses ctxAndAddr)
throws AbstractPowerwafException {
return ctxAndAddr.ctx.runRules(
return additive.runEphemeral(
new DataBundleMapWrapper(ctxAndAddr.addressesOfInterest, bundle), LIMITS, metrics);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,10 @@ class GatewayBridgeSpecification extends DDSpecification {
void 'path params is not published twice'() {
Flow flow

setup:
pathParamsCB.apply(ctx, [a: 'b'])

when:
arCtx.addAll(new SingletonDataBundle(KnownAddresses.REQUEST_PATH_PARAMS, [a: 'b']))
flow = pathParamsCB.apply(ctx, [c: 'd'])

then:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,44 @@ class PowerWAFModuleSpecification extends DDSpecification {
ret.isEmpty()
}

void 'ephemeral and persistent addresses'() {
setupWithStubConfigService()
ChangeableFlow flow = Mock()

when:
def transientBundle = MapDataBundle.of(
KnownAddresses.REQUEST_BODY_OBJECT,
'/cybercop'
)
dataListener.onDataAvailable(flow, ctx, transientBundle, false)

then:
1 * ctx.getOrCreateAdditive(_, true) >> {
pwafAdditive = it[0].openAdditive() }
1 * ctx.reportEvents(_ as Collection<AppSecEvent>) >> {
it[0].iterator().next().ruleMatches[0].parameters[0].value == '/cybercop'
}
1 * ctx.getWafMetrics()
1 * flow.isBlocking()
0 * _

when:
dataListener.onDataAvailable(flow, ctx, ATTACK_BUNDLE, false)
ctx.closeAdditive()

then:
1 * ctx.getOrCreateAdditive(_, true) >> {
pwafAdditive }
1 * flow.setAction({ it.blocking })
1 * ctx.reportEvents(_ as Collection<AppSecEvent>) >> {
it[0].iterator().next().ruleMatches[0].parameters[0].value == 'user-to-block-1'
}
1 * ctx.getWafMetrics()
1 * ctx.closeAdditive()
1 * flow.isBlocking()
0 * _
}

/**
* This test simulates double REQUEST_END with increasing interval
* The race condition shouldn't happen when closing Additive
Expand Down

0 comments on commit 28afe26

Please sign in to comment.