-
Notifications
You must be signed in to change notification settings - Fork 292
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 session rewriting detection #6692
Add session rewriting detection #6692
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 49 metrics, 14 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.33.0-SNAPSHOT~98443e8224, baseline=1.33.0-SNAPSHOT~734e3c5998
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.081 s) : 0, 1081099
Total [baseline] (10.384 s) : 0, 10383695
Agent [candidate] (1.078 s) : 0, 1078127
Total [candidate] (10.424 s) : 0, 10423686
section appsec
Agent [baseline] (1.196 s) : 0, 1196386
Total [baseline] (10.528 s) : 0, 10528049
Agent [candidate] (1.198 s) : 0, 1198101
Total [candidate] (10.636 s) : 0, 10636342
section iast
Agent [baseline] (1.198 s) : 0, 1198118
Total [baseline] (10.773 s) : 0, 10772780
Agent [candidate] (1.207 s) : 0, 1207483
Total [candidate] (10.801 s) : 0, 10801245
section profiling
Agent [baseline] (1.268 s) : 0, 1267810
Total [baseline] (10.654 s) : 0, 10654332
Agent [candidate] (1.286 s) : 0, 1286403
Total [candidate] (10.698 s) : 0, 10697639
gantt
title petclinic - break down per module: candidate=1.33.0-SNAPSHOT~98443e8224, baseline=1.33.0-SNAPSHOT~734e3c5998
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (677.315 ms) : 0, 677315
BytebuddyAgent [candidate] (674.947 ms) : 0, 674947
GlobalTracer [baseline] (310.882 ms) : 0, 310882
GlobalTracer [candidate] (310.777 ms) : 0, 310777
AppSec [baseline] (49.889 ms) : 0, 49889
AppSec [candidate] (49.733 ms) : 0, 49733
Remote Config [baseline] (669.513 µs) : 0, 670
Remote Config [candidate] (665.277 µs) : 0, 665
Telemetry [baseline] (7.701 ms) : 0, 7701
Telemetry [candidate] (7.638 ms) : 0, 7638
section appsec
BytebuddyAgent [baseline] (694.609 ms) : 0, 694609
BytebuddyAgent [candidate] (694.967 ms) : 0, 694967
GlobalTracer [baseline] (290.589 ms) : 0, 290589
GlobalTracer [candidate] (292.205 ms) : 0, 292205
AppSec [baseline] (149.901 ms) : 0, 149901
AppSec [candidate] (150.186 ms) : 0, 150186
IAST [baseline] (18.803 ms) : 0, 18803
IAST [candidate] (18.907 ms) : 0, 18907
Remote Config [baseline] (610.102 µs) : 0, 610
Remote Config [candidate] (605.35 µs) : 0, 605
Telemetry [baseline] (7.435 ms) : 0, 7435
Telemetry [candidate] (6.799 ms) : 0, 6799
section iast
BytebuddyAgent [baseline] (794.185 ms) : 0, 794185
BytebuddyAgent [candidate] (800.854 ms) : 0, 800854
GlobalTracer [baseline] (287.623 ms) : 0, 287623
GlobalTracer [candidate] (290.453 ms) : 0, 290453
AppSec [baseline] (50.315 ms) : 0, 50315
AppSec [candidate] (50.482 ms) : 0, 50482
IAST [baseline] (23.669 ms) : 0, 23669
IAST [candidate] (22.344 ms) : 0, 22344
Remote Config [baseline] (572.313 µs) : 0, 572
Remote Config [candidate] (580.213 µs) : 0, 580
Telemetry [baseline] (7.425 ms) : 0, 7425
Telemetry [candidate] (8.185 ms) : 0, 8185
section profiling
BytebuddyAgent [baseline] (677.278 ms) : 0, 677278
BytebuddyAgent [candidate] (686.79 ms) : 0, 686790
GlobalTracer [baseline] (380.474 ms) : 0, 380474
GlobalTracer [candidate] (386.017 ms) : 0, 386017
AppSec [baseline] (50.226 ms) : 0, 50226
AppSec [candidate] (51.01 ms) : 0, 51010
Remote Config [baseline] (723.259 µs) : 0, 723
Remote Config [candidate] (725.48 µs) : 0, 725
Telemetry [baseline] (7.434 ms) : 0, 7434
Telemetry [candidate] (7.561 ms) : 0, 7561
ProfilingAgent [baseline] (95.475 ms) : 0, 95475
ProfilingAgent [candidate] (97.253 ms) : 0, 97253
Profiling [baseline] (95.501 ms) : 0, 95501
Profiling [candidate] (97.277 ms) : 0, 97277
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.33.0-SNAPSHOT~98443e8224, baseline=1.33.0-SNAPSHOT~734e3c5998
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.072 s) : 0, 1072446
Total [baseline] (8.582 s) : 0, 8581613
Agent [candidate] (1.086 s) : 0, 1086255
Total [candidate] (8.639 s) : 0, 8639294
section iast
Agent [baseline] (1.194 s) : 0, 1194053
Total [baseline] (9.001 s) : 0, 9001295
Agent [candidate] (1.201 s) : 0, 1200876
Total [candidate] (9.038 s) : 0, 9038288
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.208 s) : 0, 1207981
Total [baseline] (9.007 s) : 0, 9007133
Agent [candidate] (1.201 s) : 0, 1200929
Total [candidate] (8.988 s) : 0, 8987869
section iast_TELEMETRY_OFF
Agent [baseline] (1.201 s) : 0, 1200930
Total [baseline] (9.041 s) : 0, 9041467
Agent [candidate] (1.196 s) : 0, 1196081
Total [candidate] (9.006 s) : 0, 9006119
gantt
title insecure-bank - break down per module: candidate=1.33.0-SNAPSHOT~98443e8224, baseline=1.33.0-SNAPSHOT~734e3c5998
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (672.092 ms) : 0, 672092
BytebuddyAgent [candidate] (680.394 ms) : 0, 680394
GlobalTracer [baseline] (308.195 ms) : 0, 308195
GlobalTracer [candidate] (312.932 ms) : 0, 312932
AppSec [baseline] (49.723 ms) : 0, 49723
AppSec [candidate] (50.02 ms) : 0, 50020
Remote Config [baseline] (661.505 µs) : 0, 662
Remote Config [candidate] (681.15 µs) : 0, 681
Telemetry [baseline] (7.508 ms) : 0, 7508
Telemetry [candidate] (7.639 ms) : 0, 7639
section iast
BytebuddyAgent [baseline] (791.53 ms) : 0, 791530
BytebuddyAgent [candidate] (795.967 ms) : 0, 795967
GlobalTracer [baseline] (287.068 ms) : 0, 287068
GlobalTracer [candidate] (288.519 ms) : 0, 288519
AppSec [baseline] (50.806 ms) : 0, 50806
AppSec [candidate] (48.941 ms) : 0, 48941
IAST [baseline] (23.2 ms) : 0, 23200
IAST [candidate] (25.151 ms) : 0, 25151
Remote Config [baseline] (580.079 µs) : 0, 580
Remote Config [candidate] (571.204 µs) : 0, 571
Telemetry [baseline] (6.618 ms) : 0, 6618
Telemetry [candidate] (7.348 ms) : 0, 7348
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (801.209 ms) : 0, 801209
BytebuddyAgent [candidate] (794.754 ms) : 0, 794754
GlobalTracer [baseline] (289.712 ms) : 0, 289712
GlobalTracer [candidate] (289.585 ms) : 0, 289585
AppSec [baseline] (51.283 ms) : 0, 51283
AppSec [candidate] (51.013 ms) : 0, 51013
IAST [baseline] (23.058 ms) : 0, 23058
IAST [candidate] (21.463 ms) : 0, 21463
Remote Config [baseline] (602.236 µs) : 0, 602
Remote Config [candidate] (597.924 µs) : 0, 598
Telemetry [baseline] (7.458 ms) : 0, 7458
Telemetry [candidate] (9.062 ms) : 0, 9062
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (795.637 ms) : 0, 795637
BytebuddyAgent [candidate] (792.059 ms) : 0, 792059
GlobalTracer [baseline] (289.381 ms) : 0, 289381
GlobalTracer [candidate] (288.552 ms) : 0, 288552
AppSec [baseline] (49.765 ms) : 0, 49765
AppSec [candidate] (50.213 ms) : 0, 50213
IAST [baseline] (23.881 ms) : 0, 23881
IAST [candidate] (23.074 ms) : 0, 23074
Remote Config [baseline] (577.569 µs) : 0, 578
Remote Config [candidate] (582.062 µs) : 0, 582
Telemetry [baseline] (7.285 ms) : 0, 7285
Telemetry [candidate] (7.354 ms) : 0, 7354
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 17 unstable metrics. Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.33.0-SNAPSHOT~98443e8224, baseline=1.33.0-SNAPSHOT~734e3c5998
dateFormat X
axisFormat %s
section baseline
no_agent (1.329 ms) : 1310, 1349
. : milestone, 1329,
appsec (1.731 ms) : 1706, 1756
. : milestone, 1731,
appsec_no_iast (1.747 ms) : 1723, 1771
. : milestone, 1747,
iast (1.485 ms) : 1462, 1508
. : milestone, 1485,
profiling (1.548 ms) : 1524, 1573
. : milestone, 1548,
tracing (1.5 ms) : 1476, 1524
. : milestone, 1500,
section candidate
no_agent (1.348 ms) : 1329, 1366
. : milestone, 1348,
appsec (1.733 ms) : 1709, 1756
. : milestone, 1733,
appsec_no_iast (1.734 ms) : 1710, 1758
. : milestone, 1734,
iast (1.493 ms) : 1470, 1515
. : milestone, 1493,
profiling (1.496 ms) : 1471, 1521
. : milestone, 1496,
tracing (1.474 ms) : 1451, 1497
. : milestone, 1474,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.33.0-SNAPSHOT~98443e8224, baseline=1.33.0-SNAPSHOT~734e3c5998
dateFormat X
axisFormat %s
section baseline
no_agent (365.917 µs) : 347, 385
. : milestone, 366,
iast (478.996 µs) : 458, 500
. : milestone, 479,
iast_FULL (543.586 µs) : 523, 565
. : milestone, 544,
iast_GLOBAL (500.515 µs) : 479, 522
. : milestone, 501,
iast_HARDCODED_SECRET_DISABLED (481.373 µs) : 460, 503
. : milestone, 481,
iast_INACTIVE (449.54 µs) : 429, 470
. : milestone, 450,
iast_TELEMETRY_OFF (475.873 µs) : 455, 497
. : milestone, 476,
tracing (444.193 µs) : 424, 465
. : milestone, 444,
section candidate
no_agent (365.48 µs) : 346, 385
. : milestone, 365,
iast (474.379 µs) : 453, 496
. : milestone, 474,
iast_FULL (541.313 µs) : 520, 562
. : milestone, 541,
iast_GLOBAL (502.458 µs) : 480, 525
. : milestone, 502,
iast_HARDCODED_SECRET_DISABLED (485.345 µs) : 464, 507
. : milestone, 485,
iast_INACTIVE (455.321 µs) : 434, 477
. : milestone, 455,
iast_TELEMETRY_OFF (474.795 µs) : 453, 497
. : milestone, 475,
tracing (453.178 µs) : 432, 475
. : milestone, 453,
|
7c37c84
to
541a643
Compare
Hi! 👋 Looks like you updated a Git Submodule.
|
71f572e
to
94b8763
Compare
public class SessionRewritingModuleImpl extends SinkModuleBase implements SessionRewritingModule { | ||
|
||
static final String EVIDENCE_VALUE = | ||
"URL rewriting may be used by the container for session tracking"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this belongs to the UI remediation text, not the evidence value.
cc @manuel-alvarez-alvarez @anderruiz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something shorter like "URL session tracking mode"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could even include Servlet URL Session Tracking Mode. We don't need the sentence as we can do it in the UI
...rvlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/IastServlet2Advice.java
Outdated
Show resolved
Hide resolved
We decide to split this PR as many changes belong to application vulnerabilities instead of session rewriting. There is no need to review it right now |
@@ -86,6 +86,9 @@ public interface VulnerabilityType { | |||
new VulnerabilityTypeImpl( | |||
VulnerabilityTypes.REFLECTION_INJECTION, VulnerabilityMarks.REFLECTION_INJECTION_MARK); | |||
|
|||
VulnerabilityType SESSION_REWRITING = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using class
and line
for the hash makes no sense for this vulnerability, perhaps vulnerability type
and service name
is closer to what we need.
7456e4f
to
ea303b5
Compare
|
||
final IastContext ctx = IastContext.Provider.get(); | ||
if (ctx == null) { | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you checking this?, you don't need access to the tainted map right?
import java.util.Set; | ||
import javax.annotation.Nonnull; | ||
|
||
public interface SessionRewritingModule extends IastModule { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if it makes more sense to add this logic to the application module (since it deals with servlet miss configuration issues)
What Does This Do
Add session rewriting detection in servlet3 and servlet.
The main condition to report the vulnerability is that ServletContext#getEffectiveSessionTrackingModes contains SessionTrackingMode.URL
We will take advantage of the current IastServletInstrumentation
Motivation
Add session rewriting detection
Additional Notes
Jira ticket: APPSEC-17164