Skip to content

fix(ias): require verified provenance for IAS telemetry identity attributes - #368

Closed
tiagoek wants to merge 2 commits into
mainfrom
fix/ias-jwks-telemetry-verification
Closed

tiagoek wants to merge 2 commits into
mainfrom
fix/ias-jwks-telemetry-verification

Conversation

@tiagoek

@tiagoek tiagoek commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Summary

StarletteIASTelemetryMiddleware was calling parse_token() — a deliberately unverified JWT decoder — and promoting the decoded sap_gtid/user_uuid directly onto OTel span attributes (sap.tenancy.tenant_id, user.id). An attacker could forge a JWT carrying a victim tenant/user ID, submit it to any agent using this SDK, and permanently pollute telemetry attribution.

Root cause: no signature verification before stamping security-sensitive span attributes.

Fix (SDK-only — no consumer changes required):

  1. New IASVerifier class — JWKS-backed verifier, auto-configured from the SAP BTP Identity service binding (VCAP_SERVICES on CF, IAS_URL env var on Kubernetes). Caches signing keys internally and handles key rotation transparently.
  2. New VerifiedIASClaims frozen dataclass — the SDK's provenance marker. Instances can only come from a verifier that ran signature + issuer + algorithm + expiry checks.
  3. StarletteIASTelemetryMiddleware now auto-configures IASVerifier.from_env() at construction and uses it to verify every token before stamping identity attributes. No consumer code changes needed.

Behaviour change

Scenario Before After
IAS service binding present (CF or K8s) Unverified claims stamped Verified-only claims stamped automatically
No IAS binding / env var Unverified claims stamped No identity attrs + WARNING logged; app starts normally
Forged JWT (attacker-signed) Victim tenant_id/user.id stamped Nothing stamped (verifier raises, attrs omitted)
x-sap-origin header Always stamped Always stamped (unchanged — not JWT identity)

Zero-config usage

from starlette.applications import Starlette
from sap_cloud_sdk.core.telemetry import auto_instrument
from sap_cloud_sdk.core.telemetry.middleware import StarletteIASTelemetryMiddleware

app = Starlette(...)
# Auto-configures IASVerifier from VCAP_SERVICES (CF) or IAS_URL (K8s)
auto_instrument(middlewares=[StarletteIASTelemetryMiddleware(app=app)])

Agents that already have an IAS service binding get verified telemetry with zero code changes.

Apps without an IAS service binding

Identity span attributes will not be stamped and a WARNING is logged at startup — the app continues running normally. The previous behaviour (stamping unverified claims) was a security issue, so this is the correct safe default. Bind an SAP Identity service instance to restore them.

For advanced scenarios (e.g. Istio/Kyma already verified the token and double-verification is undesirable), a custom verifier can be passed via token_verifier=. See IAS user guide for details.

Scope

  • SDK only. No consumer app changes.
  • parse_token() is unchanged — still available as an unverified claim extractor (diagnostics, pre-auth inspection).
  • AuditClient is unchanged — already requires explicit common.tenant_id; no auto-fill exists.
  • cryptography>=44.0.0 promoted from dev-only to runtime dependency (required for RSA/EC key verification).

Files changed

File Change
src/sap_cloud_sdk/ias/_verifier.py NEW — IASVerifier + IASConfigError
src/sap_cloud_sdk/ias/_token.py Added VerifiedIASClaims, TokenVerifier
src/sap_cloud_sdk/ias/__init__.py Export IASVerifier, IASConfigError, VerifiedIASClaims, TokenVerifier
src/sap_cloud_sdk/core/telemetry/middleware/starlette_a2a.py Auto-configure IASVerifier; identity attrs only on verified path
pyproject.toml cryptography>=44.0.0 → runtime dep
tests/ias/unit/test_verifier.py NEW — 22 tests for IASVerifier
tests/ias/unit/test_token.py Added VerifiedIASClaims / TokenVerifier tests
tests/core/unit/telemetry/middleware/test_starlette_a2a.py Full rewrite with auto-config + regression tests
src/sap_cloud_sdk/ias/user-guide.md IASVerifier as primary approach
src/sap_cloud_sdk/core/telemetry/user-guide.md Zero-config usage; token_verifier parameter docs

…ibutes (HASI2026203-278)

StarletteIASTelemetryMiddleware previously called parse_token() (unverified
JWT decode) and promoted sap_gtid/user_uuid to sap.tenancy.tenant_id/user.id
span attributes. A remote sender could submit an attacker-signed JWT naming
a victim tenant/user to pollute telemetry attribution.

Fix: introduce VerifiedIASClaims (frozen dataclass) and TokenVerifier
(Callable type alias) as provenance markers in the IAS module. The
middleware now requires an explicit token_verifier parameter — if absent
it fails closed (no identity attributes stamped, one-time WARNING logged).
If the verifier raises for any reason, identity attributes are silently
omitted. x-sap-origin (trigger type, not JWT identity) is stamped
independently of token verification.

parse_token() and AuditClient are unchanged. No new runtime dependencies.
…fication

The middleware previously required consumers to implement their own verifier
or left identity attributes unverified. IASVerifier auto-configures from
VCAP_SERVICES (CF) or IAS_URL (K8s) so agents using the SDK get working
signature verification with no extra code.

- Add IASVerifier: JWKS-backed verifier, PyJWKClient with key caching,
  RS256/ES256 algorithm pinning, issuer/audience/exp/nbf enforcement
- Add IASVerifier.from_env(): resolves VCAP_SERVICES → identity → xsuaa,
  then IAS_URL/IAS_CLIENT_ID env vars; raises IASConfigError if nothing found
- StarletteIASTelemetryMiddleware auto-calls IASVerifier.from_env() when no
  token_verifier supplied; logs WARNING + disables identity attrs on failure
- Promote cryptography>=44.0.0 to runtime dep (required for RSA/EC key ops)
- Add IASConfigError, IASVerifier to sap_cloud_sdk.ias public API
- Add 22 unit tests for IASVerifier (from_env variants + __call__ scenarios)
- Rewrite middleware tests: add auto-config coverage + retain all regression
  tests for forged/invalid tokens
@tiagoek

tiagoek commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #382 (clean branch: fix/ias-telemetry-verified-claims)

@tiagoek tiagoek closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant