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

Collect WAF headers on user sdk events #7014

Merged
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ public class GatewayBridge {
private static final Pattern QUERY_PARAM_SPLITTER = Pattern.compile("&");
private static final Map<String, List<String>> EMPTY_QUERY_PARAMS = Collections.emptyMap();

/** User tracking tags that will force the collection of request headers */
private static final String[] USER_TRACKING_TAGS = {
"appsec.events.users.login.success.track", "appsec.events.users.login.failure.track"
};

private final SubscriptionService subscriptionService;
private final EventProducerService producerService;
private final RateLimiter rateLimiter;
Expand Down Expand Up @@ -155,6 +160,9 @@ public void init() {
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
writeResponseHeaders(
traceSeg, RESPONSE_HEADERS_ALLOW_LIST, ctx.getResponseHeaders());
} else if (hasUserTrackingEvent(traceSeg)) {
// Report all collected request headers on user tracking event
writeRequestHeaders(traceSeg, REQUEST_HEADERS_ALLOW_LIST, ctx.getRequestHeaders());
manuel-alvarez-alvarez marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Report minimum set of collected request headers
writeRequestHeaders(
Expand Down Expand Up @@ -411,6 +419,16 @@ public void stop() {
subscriptionService.reset();
}

private static boolean hasUserTrackingEvent(final TraceSegment traceSeg) {
for (String tagName : USER_TRACKING_TAGS) {
final Object value = traceSeg.getTagTop(tagName);
if (value != null && "true".equalsIgnoreCase(value.toString())) {
return true;
}
}
return false;
}

private static void writeRequestHeaders(
final TraceSegment traceSeg,
final Set<String> allowed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -804,4 +804,32 @@ class GatewayBridgeSpecification extends DDSpecification {
1 * traceSegment.setTagTop('http.request.headers.x-sigsci-tags', 'SQLI, XSS')
1 * traceSegment.setTagTop('http.request.headers.akamai-user-risk', 'uuid=913c4545-757b-4d8d-859d-e1361a828361;status=0')
}

void 'request headers are always set when there are user tracking events'() {
given:
final mockAppSecCtx = Stub(AppSecRequestContext) {
transferCollectedEvents() >> []
getRequestHeaders() >> [
'host': ['localhost']
]
}
final mockCtx = Stub(RequestContext) {
getData(RequestContextSlot.APPSEC) >> mockAppSecCtx
getTraceSegment() >> traceSegment
}
final spanInfo = Stub(AgentSpan)
traceSegment.getTagTop(tag) >> true

when:
requestEndedCB.apply(mockCtx, spanInfo)

then:
(userTracking ? 1 : 0) * traceSegment.setTagTop('http.request.headers.host', 'localhost')

where:
tag | userTracking
'appsec.events.users.login.success.track' | true
'appsec.events.users.login.failure.track' | true
'appsec.another.unrelated.tag' | false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,18 @@ abstract class AgentTestRunner extends DDSpecification implements AgentBuilder.L
delegate.setTagCurrent(key, value, sanitize)
}

@Override
Object getTagTop(String key, boolean sanitize) {
check()
return delegate.getTagTop(key, sanitize)
}

@Override
Object getTagCurrent(String key, boolean sanitize) {
check()
return delegate.getTagCurrent(key, sanitize)
}

@Override
void setDataTop(String key, Object value) {
check()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,23 @@ default void setTagTop(String key, Object value) {
*/
void setTagTop(String key, Object value, boolean sanitize);

/**
* Get the tag value from the top of this {@code TraceSegment}.
*
* @param key key of the tag
*/
default Object getTagTop(String key) {
return getTagTop(key, false);
}

/**
* Get the tag value from the top of this {@code TraceSegment} with optional key sanitization.
*
* @param key key of the tag
* @param sanitize indicates is key need to be sanitized
*/
Object getTagTop(String key, boolean sanitize);

/**
* Add a tag to the current span in this {@code TraceSegment}.
*
Expand All @@ -46,6 +63,24 @@ default void setTagCurrent(String key, Object value) {
*/
void setTagCurrent(String key, Object value, boolean sanitize);

/**
* Get the tag value from the current span in this {@code TraceSegment}.
*
* @param key key of the tag
*/
default Object getTagCurrent(String key) {
return getTagCurrent(key, false);
}

/**
* Get the tag value from the current span in this {@code TraceSegment}. with optional key
* sanitization.
*
* @param key key of the tag
* @param sanitize indicates is key need to be sanitized
*/
Object getTagCurrent(String key, boolean sanitize);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have doubts about getters in TraceSegment. If I recall correctly, it was designed only to write to isolate writing to local root span

Copy link
Member Author

Choose a reason for hiding this comment

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

There are already other getters in the TraceSegment like Object getDataCurrent(String key) and they seem to be working fine. I did test this PR with the related tests for this functionality and it seems to be working fine.

Are there any other mechanism to access the local root span and extract the tags?


/**
* Add data to the top of this {@code TraceSegment}. The {@code toString} representation of the
* {@code value} must be valid top level JSON, i.e. an {@code Object} or an {@code Array}.
Expand Down Expand Up @@ -94,6 +129,16 @@ public void setTagTop(String key, Object value, boolean sanitize) {}
@Override
public void setTagCurrent(String key, Object value, boolean sanitize) {}

@Override
public Object getTagTop(String key, boolean sanitize) {
return null;
}

@Override
public Object getTagCurrent(String key, boolean sanitize) {
return null;
}

@Override
public void setDataTop(String key, Object value) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,11 @@ public void setTagTop(String key, Object value, boolean sanitize) {
getRootSpanContextOrThis().setTagCurrent(key, value, sanitize);
}

@Override
public Object getTagTop(String key, boolean sanitize) {
return getRootSpanContextOrThis().getTagCurrent(key, sanitize);
}

@Override
public void setTagCurrent(String key, Object value, boolean sanitize) {
if (sanitize) {
Expand All @@ -916,6 +921,14 @@ public void setTagCurrent(String key, Object value, boolean sanitize) {
this.setTag(key, value);
}

@Override
public Object getTagCurrent(String key, boolean sanitize) {
if (sanitize) {
key = TagsHelper.sanitize(key);
}
return this.getTag(key);
}

@Override
public void setDataTop(String key, Object value) {
getRootSpanContextOrThis().setDataCurrent(key, value);
Expand Down
Loading