Skip to content

Commit

Permalink
feat(iast): include as iast telemetry metrics tags in the root span o…
Browse files Browse the repository at this point in the history
…f the trace (#7354)

For metrics that need to be correlated to traces, a small subset of IAST
telemetry metrics can be included as tags in the root span of the trace
following the next algorithm, this PR contains:
- `iast_metrics` into asm_request_context
- Span metrics with IAST `request.tainted` and `executed.skins.*` data

TODO: add integration tests with a flask server and retrieve telemetry
events from test agent but we need to refactor telemetry tests and move
testagent fixture from conftest to a upper level conftest


## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: Christophe Papazian <[email protected]>
  • Loading branch information
avara1986 and christophe-papazian authored Oct 31, 2023
1 parent ed5465d commit bf5fd17
Show file tree
Hide file tree
Showing 19 changed files with 163 additions and 32 deletions.
7 changes: 7 additions & 0 deletions ddtrace/appsec/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ class IAST(metaclass=Constant_Class):
SEP_MODULES = ","


class IAST_SPAN_TAGS(metaclass=Constant_Class):
"""Specific constants for IAST span tags"""

TELEMETRY_REQUEST_TAINTED = "_dd.iast.telemetry.request.tainted"
TELEMETRY_EXECUTED_SINK = "_dd.iast.telemetry.executed.sink"


class WAF_DATA_NAMES(metaclass=Constant_Class):
"""string names used by the waf library for requesting data from requests"""

Expand Down
47 changes: 44 additions & 3 deletions ddtrace/appsec/_iast/_metrics.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import os
from typing import Dict

from ddtrace.appsec._constants import IAST
from ddtrace.appsec._constants import IAST_SPAN_TAGS
from ddtrace.appsec._deduplications import deduplication
from ddtrace.internal.logger import get_logger
from ddtrace.internal.telemetry import telemetry_writer
Expand All @@ -26,6 +28,8 @@
(TELEMETRY_OFF_VERBOSITY, TELEMETRY_OFF_NAME),
)

_IAST_SPAN_METRICS: Dict[str, int] = {}


def get_iast_metrics_report_lvl(*args, **kwargs):
report_lvl_name = os.environ.get(IAST.TELEMETRY_REPORT_LVL, TELEMETRY_INFORMATION_NAME).upper()
Expand Down Expand Up @@ -98,10 +102,47 @@ def _set_metric_iast_executed_sink(vulnerability_type):
)


@metric_verbosity(TELEMETRY_INFORMATION_VERBOSITY)
def _set_metric_iast_request_tainted():
def _request_tainted():
from ._taint_tracking import num_objects_tainted

total_objects_tainted = num_objects_tainted()
return num_objects_tainted()


@metric_verbosity(TELEMETRY_INFORMATION_VERBOSITY)
def _set_metric_iast_request_tainted():
total_objects_tainted = _request_tainted()
if total_objects_tainted > 0:
telemetry_writer.add_count_metric(TELEMETRY_NAMESPACE_TAG_IAST, "request.tainted", total_objects_tainted)


def _set_span_tag_iast_request_tainted(span):
total_objects_tainted = _request_tainted()

if total_objects_tainted > 0:
span.set_tag(IAST_SPAN_TAGS.TELEMETRY_REQUEST_TAINTED, total_objects_tainted)


def _set_span_tag_iast_executed_sink(span):
data = get_iast_span_metrics()

if data is not None:
for key, value in data.items():
if key.startswith(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK):
span.set_tag(key, value)

reset_iast_span_metrics()


def increment_iast_span_metric(prefix: str, metric_key: str, counter: int = 1) -> None:
data = get_iast_span_metrics()
full_key = prefix + "." + metric_key.lower()
result = data.get(full_key, 0)
data[full_key] = result + counter


def get_iast_span_metrics() -> Dict:
return _IAST_SPAN_METRICS


def reset_iast_span_metrics() -> None:
_IAST_SPAN_METRICS.clear()
9 changes: 5 additions & 4 deletions ddtrace/appsec/_iast/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from .._trace_utils import _asm_manual_keep
from . import oce
from ._metrics import _set_metric_iast_request_tainted
from ._metrics import _set_span_tag_iast_executed_sink
from ._metrics import _set_span_tag_iast_request_tainted
from ._utils import _iast_report_to_str
from ._utils import _is_iast_enabled

Expand Down Expand Up @@ -57,13 +59,12 @@ def on_span_finish(self, span):
data = core.get_item(IAST.CONTEXT_KEY, span=span)

if data:
span.set_tag_str(
IAST.JSON,
_iast_report_to_str(data),
)
span.set_tag_str(IAST.JSON, _iast_report_to_str(data))
_asm_manual_keep(span)

_set_metric_iast_request_tainted()
_set_span_tag_iast_request_tainted(span)
_set_span_tag_iast_executed_sink(span)
reset_context()

if span.get_tag(ORIGIN_KEY) is None:
Expand Down
3 changes: 3 additions & 0 deletions ddtrace/appsec/_iast/taint_sinks/ast_taint.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from typing import TYPE_CHECKING

from ..._constants import IAST_SPAN_TAGS
from .._metrics import _set_metric_iast_executed_sink
from .._metrics import increment_iast_span_metric
from ..constants import DEFAULT_PATH_TRAVERSAL_FUNCTIONS
from ..constants import DEFAULT_WEAK_RANDOMNESS_FUNCTIONS
from .path_traversal import check_and_report_path_traversal
Expand Down Expand Up @@ -29,6 +31,7 @@ def ast_funcion(

if cls.__class__.__module__ == "random" and cls_name == "Random" and func_name in DEFAULT_WEAK_RANDOMNESS_FUNCTIONS:
# Weak, run the analyzer
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakRandomness.vulnerability_type)
_set_metric_iast_executed_sink(WeakRandomness.vulnerability_type)
WeakRandomness.report(evidence_value=cls_name + "." + func_name)
elif hasattr(func, "__module__") and DEFAULT_PATH_TRAVERSAL_FUNCTIONS.get(func.__module__):
Expand Down
3 changes: 3 additions & 0 deletions ddtrace/appsec/_iast/taint_sinks/command_injection.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
from ddtrace.internal.logger import get_logger
from ddtrace.settings.asm import config as asm_config

from ..._constants import IAST_SPAN_TAGS
from .. import oce
from .._metrics import increment_iast_span_metric
from .._utils import _has_to_scrub
from .._utils import _scrub
from .._utils import _scrub_get_tokens_positions
Expand Down Expand Up @@ -252,5 +254,6 @@ def _iast_report_cmdi(shell_args):
report_cmdi = shell_args

if report_cmdi:
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, CommandInjection.vulnerability_type)
_set_metric_iast_executed_sink(CommandInjection.vulnerability_type)
CommandInjection.report(evidence_value=report_cmdi)
7 changes: 6 additions & 1 deletion ddtrace/appsec/_iast/taint_sinks/insecure_cookie.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

from ddtrace.internal.compat import six

from ..._constants import IAST_SPAN_TAGS
from .. import oce
from .._metrics import _set_metric_iast_executed_sink
from .._metrics import increment_iast_span_metric
from ..constants import EVIDENCE_COOKIE
from ..constants import VULN_INSECURE_COOKIE
from ..constants import VULN_NO_HTTPONLY_COOKIE
Expand Down Expand Up @@ -47,11 +49,13 @@ def asm_check_cookies(cookies): # type: (Optional[Dict[str, str]]) -> None
evidence = "%s=%s" % (cookie_key, cookie_value)

if ";secure" not in lvalue:
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, InsecureCookie.vulnerability_type)
_set_metric_iast_executed_sink(InsecureCookie.vulnerability_type)
InsecureCookie.report(evidence_value=evidence)
return

if ";httponly" not in lvalue:
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, NoHttpOnlyCookie.vulnerability_type)
_set_metric_iast_executed_sink(NoHttpOnlyCookie.vulnerability_type)
NoHttpOnlyCookie.report(evidence_value=evidence)
return
Expand All @@ -68,5 +72,6 @@ def asm_check_cookies(cookies): # type: (Optional[Dict[str, str]]) -> None
report_samesite = True

if report_samesite:
NoSameSite.report(evidence_value=evidence)
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, NoSameSite.vulnerability_type)
_set_metric_iast_executed_sink(NoSameSite.vulnerability_type)
NoSameSite.report(evidence_value=evidence)
3 changes: 3 additions & 0 deletions ddtrace/appsec/_iast/taint_sinks/path_traversal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

from ddtrace.internal.logger import get_logger

from ..._constants import IAST_SPAN_TAGS
from .. import oce
from .._metrics import _set_metric_iast_instrumented_sink
from .._metrics import increment_iast_span_metric
from .._patch import set_and_check_module_is_patched
from .._patch import set_module_unpatched
from ..constants import EVIDENCE_PATH_TRAVERSAL
Expand Down Expand Up @@ -52,6 +54,7 @@ def check_and_report_path_traversal(*args: Any, **kwargs: Any) -> None:
from .._metrics import _set_metric_iast_executed_sink
from .._taint_tracking import is_pyobject_tainted

increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, PathTraversal.vulnerability_type)
_set_metric_iast_executed_sink(PathTraversal.vulnerability_type)
if is_pyobject_tainted(args[0]):
PathTraversal.report(evidence_value=args[0])
Expand Down
3 changes: 3 additions & 0 deletions ddtrace/appsec/_iast/taint_sinks/ssrf.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
from ddtrace.internal.logger import get_logger
from ddtrace.settings.asm import config as asm_config

from ..._constants import IAST_SPAN_TAGS
from .. import oce
from .._metrics import increment_iast_span_metric
from .._taint_tracking import taint_ranges_as_evidence_info
from .._utils import _has_to_scrub
from .._utils import _scrub
Expand Down Expand Up @@ -159,6 +161,7 @@ def _iast_report_ssrf(func: Callable, *args, **kwargs):
from .._metrics import _set_metric_iast_executed_sink

report_ssrf = kwargs.get("url", False)
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, SSRF.vulnerability_type)
_set_metric_iast_executed_sink(SSRF.vulnerability_type)
if report_ssrf:
if oce.request_has_quota and SSRF.has_quota():
Expand Down
5 changes: 5 additions & 0 deletions ddtrace/appsec/_iast/taint_sinks/weak_cipher.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

from ddtrace.internal.logger import get_logger

from ..._constants import IAST_SPAN_TAGS
from .. import oce
from .._metrics import _set_metric_iast_executed_sink
from .._metrics import _set_metric_iast_instrumented_sink
from .._metrics import increment_iast_span_metric
from .._patch import set_and_check_module_is_patched
from .._patch import set_module_unpatched
from .._patch import try_unwrap
Expand Down Expand Up @@ -129,6 +131,7 @@ def wrapped_aux_blowfish_function(wrapped, instance, args, kwargs):
@WeakCipher.wrap
def wrapped_rc4_function(wrapped, instance, args, kwargs):
# type: (Callable, Any, Any, Any) -> Any
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakCipher.vulnerability_type)
_set_metric_iast_executed_sink(WeakCipher.vulnerability_type)
WeakCipher.report(
evidence_value="RC4",
Expand All @@ -141,6 +144,7 @@ def wrapped_function(wrapped, instance, args, kwargs):
# type: (Callable, Any, Any, Any) -> Any
if hasattr(instance, "_dd_weakcipher_algorithm"):
evidence = instance._dd_weakcipher_algorithm + "_" + str(instance.__class__.__name__)
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakCipher.vulnerability_type)
_set_metric_iast_executed_sink(WeakCipher.vulnerability_type)
WeakCipher.report(
evidence_value=evidence,
Expand All @@ -154,6 +158,7 @@ def wrapped_cryptography_function(wrapped, instance, args, kwargs):
# type: (Callable, Any, Any, Any) -> Any
algorithm_name = instance.algorithm.name.lower()
if algorithm_name in get_weak_cipher_algorithms():
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakCipher.vulnerability_type)
_set_metric_iast_executed_sink(WeakCipher.vulnerability_type)
WeakCipher.report(
evidence_value=algorithm_name,
Expand Down
5 changes: 5 additions & 0 deletions ddtrace/appsec/_iast/taint_sinks/weak_hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

from ddtrace.internal.logger import get_logger

from ..._constants import IAST_SPAN_TAGS
from .. import oce
from .._metrics import _set_metric_iast_executed_sink
from .._metrics import _set_metric_iast_instrumented_sink
from .._metrics import increment_iast_span_metric
from .._patch import set_and_check_module_is_patched
from .._patch import set_module_unpatched
from .._patch import try_unwrap
Expand Down Expand Up @@ -127,6 +129,7 @@ def patch():
def wrapped_digest_function(wrapped, instance, args, kwargs):
# type: (Callable, Any, Any, Any) -> Any
if instance.name.lower() in get_weak_hash_algorithms():
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakHash.vulnerability_type)
_set_metric_iast_executed_sink(WeakHash.vulnerability_type)
WeakHash.report(
evidence_value=instance.name,
Expand All @@ -150,6 +153,7 @@ def wrapped_sha1_function(wrapped, instance, args, kwargs):
def wrapped_new_function(wrapped, instance, args, kwargs):
# type: (Callable, Any, Any, Any) -> Any
if args[0].lower() in get_weak_hash_algorithms():
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakHash.vulnerability_type)
_set_metric_iast_executed_sink(WeakHash.vulnerability_type)
WeakHash.report(
evidence_value=args[0].lower(),
Expand All @@ -159,6 +163,7 @@ def wrapped_new_function(wrapped, instance, args, kwargs):

def wrapped_function(wrapped, evidence, instance, args, kwargs):
# type: (Callable, str, Any, Any, Any) -> Any
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, WeakHash.vulnerability_type)
_set_metric_iast_executed_sink(WeakHash.vulnerability_type)
WeakHash.report(
evidence_value=evidence,
Expand Down
3 changes: 3 additions & 0 deletions ddtrace/contrib/dbapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from ddtrace.appsec._iast._utils import _is_iast_enabled
from ddtrace.internal.constants import COMPONENT

from ...appsec._constants import IAST_SPAN_TAGS
from ...appsec._iast._metrics import increment_iast_span_metric
from ...constants import ANALYTICS_SAMPLE_RATE_KEY
from ...constants import SPAN_KIND
from ...constants import SPAN_MEASURED_KEY
Expand Down Expand Up @@ -114,6 +116,7 @@ def _trace_method(self, method, name, resource, extra_tags, dbm_propagator, *arg
from ddtrace.appsec._iast._taint_utils import check_tainted_args
from ddtrace.appsec._iast.taint_sinks.sql_injection import SqlInjection

increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, SqlInjection.vulnerability_type)
_set_metric_iast_executed_sink(SqlInjection.vulnerability_type)
if check_tainted_args(args, kwargs, pin.tracer, self._self_config.integration_name, method):
SqlInjection.report(evidence_value=args[0])
Expand Down
3 changes: 3 additions & 0 deletions ddtrace/contrib/dbapi_async/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
from ddtrace.appsec._iast._utils import _is_iast_enabled
from ddtrace.internal.constants import COMPONENT

from ...appsec._constants import IAST_SPAN_TAGS
from ...appsec._iast._metrics import increment_iast_span_metric
from ...constants import ANALYTICS_SAMPLE_RATE_KEY
from ...constants import SPAN_KIND
from ...constants import SPAN_MEASURED_KEY
Expand Down Expand Up @@ -77,6 +79,7 @@ async def _trace_method(self, method, name, resource, extra_tags, dbm_propagator
from ddtrace.appsec._iast._taint_utils import check_tainted_args
from ddtrace.appsec._iast.taint_sinks.sql_injection import SqlInjection

increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, SqlInjection.vulnerability_type)
_set_metric_iast_executed_sink(SqlInjection.vulnerability_type)
if check_tainted_args(args, kwargs, pin.tracer, self._self_config.integration_name, method):
SqlInjection.report(evidence_value=args[0])
Expand Down
7 changes: 4 additions & 3 deletions tests/.suitespec.json
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,7 @@
"@tracing",
"@appsec",
"@appsec_iast",
"tests/appsec/appsec/*",
"tests/snapshots/tests.appsec.*"
"tests/appsec/*"
],
"appsec_iast": [
"@bootstrap",
Expand All @@ -411,7 +410,9 @@
"@tracing",
"@appsec",
"@appsec_iast",
"tests/appsec/iast_integrations/*"
"tests/appsec/*",
"tests/appsec/iast_integrations/*",
"tests/snapshots/tests.appsec.*"
],
"aws_lambda": [
"@bootstrap",
Expand Down
18 changes: 14 additions & 4 deletions tests/appsec/appsec_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,38 @@ def gunicorn_server(appsec_enabled="true", remote_configuration_enabled="true",


@contextmanager
def flask_server(appsec_enabled="true", remote_configuration_enabled="true", tracer_enabled="true", token=None):
def flask_server(
appsec_enabled="true", remote_configuration_enabled="true", iast_enabled="false", tracer_enabled="true", token=None
):
cmd = ["python", "tests/appsec/integrations/app.py", "--no-reload"]
yield from appsec_application_server(
cmd,
appsec_enabled=appsec_enabled,
remote_configuration_enabled=remote_configuration_enabled,
iast_enabled=iast_enabled,
tracer_enabled=tracer_enabled,
token=token,
)


def appsec_application_server(
cmd, appsec_enabled="true", remote_configuration_enabled="true", tracer_enabled="true", token=None
cmd,
appsec_enabled="true",
remote_configuration_enabled="true",
iast_enabled="false",
tracer_enabled="true",
token=None,
):
env = _build_env()
env["DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS"] = "0.5"
env["DD_REMOTE_CONFIGURATION_ENABLED"] = remote_configuration_enabled
if token:
env["_DD_REMOTE_CONFIGURATION_ADDITIONAL_HEADERS"] = "X-Datadog-Test-Session-Token:%s," % (token,)
if appsec_enabled:
if appsec_enabled is not None:
env["DD_APPSEC_ENABLED"] = appsec_enabled
if tracer_enabled:
if iast_enabled is not None and iast_enabled != "false":
env["DD_IAST_ENABLED"] = iast_enabled
if tracer_enabled is not None:
env["DD_TRACE_ENABLED"] = tracer_enabled
env["DD_TRACE_AGENT_URL"] = os.environ.get("DD_TRACE_AGENT_URL", "")

Expand Down
Empty file.
Loading

0 comments on commit bf5fd17

Please sign in to comment.