-
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
Move SSRF support for IAST to HttpClientDecorator #7792
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 52 metrics, 11 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.084 s) : 0, 1084156
Total [baseline] (8.652 s) : 0, 8652058
Agent [candidate] (1.077 s) : 0, 1077170
Total [candidate] (8.554 s) : 0, 8554270
section iast
Agent [baseline] (1.204 s) : 0, 1204207
Total [baseline] (9.108 s) : 0, 9108006
Agent [candidate] (1.203 s) : 0, 1203355
Total [candidate] (9.109 s) : 0, 9109404
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.213 s) : 0, 1212969
Total [baseline] (9.158 s) : 0, 9158497
Agent [candidate] (1.21 s) : 0, 1210430
Total [candidate] (9.093 s) : 0, 9092759
section iast_TELEMETRY_OFF
Agent [baseline] (1.202 s) : 0, 1201814
Total [baseline] (9.125 s) : 0, 9124683
Agent [candidate] (1.2 s) : 0, 1199601
Total [candidate] (9.098 s) : 0, 9097966
gantt
title insecure-bank - break down per module: candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (688.056 ms) : 0, 688056
BytebuddyAgent [candidate] (686.072 ms) : 0, 686072
GlobalTracer [baseline] (315.754 ms) : 0, 315754
GlobalTracer [candidate] (314.397 ms) : 0, 314397
AppSec [baseline] (54.685 ms) : 0, 54685
AppSec [candidate] (53.99 ms) : 0, 53990
Remote Config [baseline] (665.941 µs) : 0, 666
Remote Config [candidate] (674.577 µs) : 0, 675
Telemetry [baseline] (11.302 ms) : 0, 11302
Telemetry [candidate] (8.366 ms) : 0, 8366
section iast
BytebuddyAgent [baseline] (801.838 ms) : 0, 801838
BytebuddyAgent [candidate] (800.941 ms) : 0, 800941
GlobalTracer [baseline] (303.084 ms) : 0, 303084
GlobalTracer [candidate] (303.74 ms) : 0, 303740
AppSec [baseline] (56.23 ms) : 0, 56230
AppSec [candidate] (57.41 ms) : 0, 57410
IAST [baseline] (20.584 ms) : 0, 20584
IAST [candidate] (19.612 ms) : 0, 19612
Remote Config [baseline] (603.137 µs) : 0, 603
Remote Config [candidate] (594.422 µs) : 0, 594
Telemetry [baseline] (8.205 ms) : 0, 8205
Telemetry [candidate] (7.356 ms) : 0, 7356
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (808.074 ms) : 0, 808074
BytebuddyAgent [candidate] (806.341 ms) : 0, 806341
GlobalTracer [baseline] (305.888 ms) : 0, 305888
GlobalTracer [candidate] (304.448 ms) : 0, 304448
AppSec [baseline] (57.465 ms) : 0, 57465
AppSec [candidate] (57.238 ms) : 0, 57238
IAST [baseline] (19.731 ms) : 0, 19731
IAST [candidate] (20.649 ms) : 0, 20649
Remote Config [baseline] (606.716 µs) : 0, 607
Remote Config [candidate] (593.887 µs) : 0, 594
Telemetry [baseline] (7.42 ms) : 0, 7420
Telemetry [candidate] (7.362 ms) : 0, 7362
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (799.678 ms) : 0, 799678
BytebuddyAgent [candidate] (797.856 ms) : 0, 797856
GlobalTracer [baseline] (303.581 ms) : 0, 303581
GlobalTracer [candidate] (303.467 ms) : 0, 303467
AppSec [baseline] (57.37 ms) : 0, 57370
AppSec [candidate] (55.811 ms) : 0, 55811
IAST [baseline] (19.543 ms) : 0, 19543
IAST [candidate] (20.93 ms) : 0, 20930
Remote Config [baseline] (597.628 µs) : 0, 598
Remote Config [candidate] (588.968 µs) : 0, 589
Telemetry [baseline] (7.364 ms) : 0, 7364
Telemetry [candidate] (7.235 ms) : 0, 7235
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.076 s) : 0, 1076133
Total [baseline] (10.469 s) : 0, 10468553
Agent [candidate] (1.078 s) : 0, 1078211
Total [candidate] (10.437 s) : 0, 10436914
section appsec
Agent [baseline] (1.213 s) : 0, 1213378
Total [baseline] (10.711 s) : 0, 10710605
Agent [candidate] (1.216 s) : 0, 1215779
Total [candidate] (10.653 s) : 0, 10653113
section iast
Agent [baseline] (1.203 s) : 0, 1203474
Total [baseline] (10.873 s) : 0, 10872954
Agent [candidate] (1.206 s) : 0, 1206125
Total [candidate] (10.894 s) : 0, 10893662
section profiling
Agent [baseline] (1.273 s) : 0, 1273062
Total [baseline] (10.735 s) : 0, 10734660
Agent [candidate] (1.283 s) : 0, 1282954
Total [candidate] (10.685 s) : 0, 10684933
gantt
title petclinic - break down per module: candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (684.67 ms) : 0, 684670
BytebuddyAgent [candidate] (686.145 ms) : 0, 686145
GlobalTracer [baseline] (314.324 ms) : 0, 314324
GlobalTracer [candidate] (314.753 ms) : 0, 314753
AppSec [baseline] (53.806 ms) : 0, 53806
AppSec [candidate] (53.868 ms) : 0, 53868
Remote Config [baseline] (659.449 µs) : 0, 659
Remote Config [candidate] (662.561 µs) : 0, 663
Telemetry [baseline] (9.027 ms) : 0, 9027
Telemetry [candidate] (9.091 ms) : 0, 9091
section appsec
BytebuddyAgent [baseline] (704.303 ms) : 0, 704303
BytebuddyAgent [candidate] (705.45 ms) : 0, 705450
GlobalTracer [baseline] (312.403 ms) : 0, 312403
GlobalTracer [candidate] (313.163 ms) : 0, 313163
AppSec [baseline] (164.985 ms) : 0, 164985
AppSec [candidate] (165.008 ms) : 0, 165008
Remote Config [baseline] (644.509 µs) : 0, 645
Remote Config [candidate] (637.792 µs) : 0, 638
Telemetry [baseline] (7.733 ms) : 0, 7733
Telemetry [candidate] (8.086 ms) : 0, 8086
IAST [baseline] (19.274 ms) : 0, 19274
IAST [candidate] (19.427 ms) : 0, 19427
section iast
BytebuddyAgent [baseline] (801.334 ms) : 0, 801334
BytebuddyAgent [candidate] (802.466 ms) : 0, 802466
GlobalTracer [baseline] (303.181 ms) : 0, 303181
GlobalTracer [candidate] (304.393 ms) : 0, 304393
AppSec [baseline] (57.419 ms) : 0, 57419
AppSec [candidate] (56.734 ms) : 0, 56734
Remote Config [baseline] (601.265 µs) : 0, 601
Remote Config [candidate] (607.362 µs) : 0, 607
Telemetry [baseline] (7.366 ms) : 0, 7366
Telemetry [candidate] (7.44 ms) : 0, 7440
IAST [baseline] (19.931 ms) : 0, 19931
IAST [candidate] (20.786 ms) : 0, 20786
section profiling
BytebuddyAgent [baseline] (679.304 ms) : 0, 679304
BytebuddyAgent [candidate] (684.325 ms) : 0, 684325
GlobalTracer [baseline] (397.252 ms) : 0, 397252
GlobalTracer [candidate] (400.317 ms) : 0, 400317
AppSec [baseline] (54.116 ms) : 0, 54116
AppSec [candidate] (54.62 ms) : 0, 54620
Remote Config [baseline] (654.709 µs) : 0, 655
Remote Config [candidate] (673.901 µs) : 0, 674
Telemetry [baseline] (13.293 ms) : 0, 13293
Telemetry [candidate] (14.133 ms) : 0, 14133
ProfilingAgent [baseline] (89.775 ms) : 0, 89775
ProfilingAgent [candidate] (89.885 ms) : 0, 89885
Profiling [baseline] (89.798 ms) : 0, 89798
Profiling [candidate] (89.909 ms) : 0, 89909
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 6 metrics, 21 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section baseline
no_agent (459.305 µs) : 429, 490
. : milestone, 459,
iast (586.675 µs) : 554, 619
. : milestone, 587,
iast_FULL (829.938 µs) : 797, 863
. : milestone, 830,
iast_GLOBAL (622.269 µs) : 591, 654
. : milestone, 622,
iast_HARDCODED_SECRET_DISABLED (585.842 µs) : 553, 619
. : milestone, 586,
iast_INACTIVE (543.369 µs) : 513, 574
. : milestone, 543,
iast_TELEMETRY_OFF (572.941 µs) : 541, 605
. : milestone, 573,
tracing (538.206 µs) : 508, 568
. : milestone, 538,
section candidate
no_agent (449.952 µs) : 421, 479
. : milestone, 450,
iast (587.61 µs) : 555, 620
. : milestone, 588,
iast_FULL (834.987 µs) : 803, 867
. : milestone, 835,
iast_GLOBAL (629.453 µs) : 597, 662
. : milestone, 629,
iast_HARDCODED_SECRET_DISABLED (589.247 µs) : 557, 622
. : milestone, 589,
iast_INACTIVE (538.032 µs) : 507, 569
. : milestone, 538,
iast_TELEMETRY_OFF (582.249 µs) : 550, 614
. : milestone, 582,
tracing (535.474 µs) : 505, 566
. : milestone, 535,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section baseline
no_agent (1.701 ms) : 1676, 1726
. : milestone, 1701,
appsec (2.191 ms) : 2161, 2220
. : milestone, 2191,
appsec_no_iast (2.198 ms) : 2166, 2230
. : milestone, 2198,
iast (1.864 ms) : 1834, 1894
. : milestone, 1864,
profiling (1.962 ms) : 1928, 1996
. : milestone, 1962,
tracing (1.849 ms) : 1818, 1880
. : milestone, 1849,
section candidate
no_agent (1.717 ms) : 1693, 1742
. : milestone, 1717,
appsec (2.195 ms) : 2166, 2224
. : milestone, 2195,
appsec_no_iast (2.203 ms) : 2173, 2234
. : milestone, 2203,
iast (1.846 ms) : 1816, 1876
. : milestone, 1846,
profiling (1.854 ms) : 1824, 1884
. : milestone, 1854,
tracing (1.844 ms) : 1812, 1875
. : milestone, 1844,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section baseline
no_agent (15.081 s) : 15081000, 15081000
. : milestone, 15081000,
appsec (15.288 s) : 15288000, 15288000
. : milestone, 15288000,
iast (18.922 s) : 18922000, 18922000
. : milestone, 18922000,
iast_GLOBAL (18.39 s) : 18390000, 18390000
. : milestone, 18390000,
profiling (15.349 s) : 15349000, 15349000
. : milestone, 15349000,
tracing (15.109 s) : 15109000, 15109000
. : milestone, 15109000,
section candidate
no_agent (15.02 s) : 15020000, 15020000
. : milestone, 15020000,
appsec (15.296 s) : 15296000, 15296000
. : milestone, 15296000,
iast (19.171 s) : 19171000, 19171000
. : milestone, 19171000,
iast_GLOBAL (18.285 s) : 18285000, 18285000
. : milestone, 18285000,
profiling (15.063 s) : 15063000, 15063000
. : milestone, 15063000,
tracing (15.106 s) : 15106000, 15106000
. : milestone, 15106000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.42.0-SNAPSHOT~8a51bbbb9c, baseline=1.42.0-SNAPSHOT~104a441d0a
dateFormat X
axisFormat %s
section baseline
no_agent (1.466 ms) : 1454, 1477
. : milestone, 1466,
appsec (2.332 ms) : 2290, 2373
. : milestone, 2332,
iast (2.069 ms) : 2017, 2121
. : milestone, 2069,
iast_GLOBAL (2.127 ms) : 2074, 2180
. : milestone, 2127,
profiling (1.936 ms) : 1894, 1977
. : milestone, 1936,
tracing (1.912 ms) : 1873, 1951
. : milestone, 1912,
section candidate
no_agent (1.461 ms) : 1449, 1472
. : milestone, 1461,
appsec (2.341 ms) : 2299, 2382
. : milestone, 2341,
iast (2.076 ms) : 2023, 2129
. : milestone, 2076,
iast_GLOBAL (2.118 ms) : 2065, 2171
. : milestone, 2118,
profiling (1.942 ms) : 1900, 1984
. : milestone, 1942,
tracing (1.916 ms) : 1876, 1956
. : milestone, 1916,
|
...rc/main/java/datadog/trace/instrumentation/commonshttpclient/CommonsHttpClientDecorator.java
Outdated
Show resolved
Hide resolved
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientDecorator.java
Outdated
Show resolved
Hide resolved
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java
Outdated
Show resolved
Hide resolved
...test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecoratorTest.groovy
Outdated
Show resolved
Hide resolved
I have removed the method and added an exclusion for the coverage. It seems that method was the only one that tested the |
...rap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java
Show resolved
Hide resolved
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/SsrfModuleImpl.java
Show resolved
Hide resolved
...-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy
Outdated
Show resolved
Hide resolved
internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/URIUtils.java
Outdated
Show resolved
Hide resolved
LGTM! but please check the build, it seems that datadog.trace.api.iast.util.PropagationUtils is not passing the test coverage job |
What Does This Do
This changes the way of detecting an SSRF in the http client. In this PR we centralize inside the
HttpClientDecorator
the detection of the SSRF vulnerability. For now, we only have swapped the libraries that we were supporting with the previous approach (commons-httpclient
,apache-httpclient
andokHttp
). The objective is to implement with this approach the rest of the clients supported by theHttpClientDecorator
.Even after centralizing the detection inside the
HttpClientDecorator
we needed to make some instrumentation to ensure the propagation.Motivation
With this change we want to expand the support for SSRF in the different clients supported by the
HttpClientDecorator
.Additional Notes
There are some cases where we cannot use this approach, so we need to maintain the previous approach and instrument the required methods to cover those cases.
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: APPSEC-55237