From 1c5abac229621a986d03543502ff350f6bb62e8f Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 27 Dec 2024 13:39:52 +0100 Subject: [PATCH 1/8] add express-session instrumentation --- .../src/express-session.js | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 packages/datadog-instrumentations/src/express-session.js diff --git a/packages/datadog-instrumentations/src/express-session.js b/packages/datadog-instrumentations/src/express-session.js new file mode 100644 index 00000000000..12e41ae7847 --- /dev/null +++ b/packages/datadog-instrumentations/src/express-session.js @@ -0,0 +1,52 @@ +'use strict' + +const { addHook } = require('./helpers/instrument') +const shimmer = require('../../datadog-shimmer') +const { channel } = require('./helpers/instrument') + +const sessionMiddlewareFinishCh = channel('datadog:express-session:middleware:finish') + +function wrapSessionMiddleware (sessionMiddleware) { + return function wrappedSessionMiddleware (req, res, next) { + shimmer.wrap(arguments, 2, function wrapNext (next) { + return function wrappedNext () { + if (sessionMiddlewareFinishCh.hasSubscribers) { + const abortController = new AbortController() + + sessionMiddlewareFinishCh.publish({ req, res, sessionId: req.sessionID, abortController }) + + if (abortController.signal.aborted) return + } + + return next.apply(this, arguments) + } + }) + + return sessionMiddleware.apply(this, arguments) + } +} + +function wrapSession (session) { + return function wrappedSession () { + const sessionMiddleware = session.apply(this, arguments) + + return shimmer.wrapFunction(sessionMiddleware, wrapSessionMiddleware) + } +} + +addHook({ + name: 'express-session', + versions: ['>=0.3.0'] // TODO +}, session => { + return shimmer.wrapFunction(session, wrapSession) +}) + + + return shimmer.wrapFunction(session, function wrapSession { + const queryMiddleware = query.apply(this, arguments) + + return shimmer.wrapFunction(queryMiddleware, queryMiddleware => function (req, res, next) { + arguments[2] = publishQueryParsedAndNext(req, res, next) + return queryMiddleware.apply(this, arguments) + }) + }) \ No newline at end of file From b61edcd78e8b7206d855f8faeb86672c909878cf Mon Sep 17 00:00:00 2001 From: simon-id Date: Mon, 13 Jan 2025 12:09:25 +0100 Subject: [PATCH 2/8] add rc capability --- packages/dd-trace/src/appsec/remote_config/capabilities.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/dd-trace/src/appsec/remote_config/capabilities.js b/packages/dd-trace/src/appsec/remote_config/capabilities.js index 5057d38de43..2cde1a751df 100644 --- a/packages/dd-trace/src/appsec/remote_config/capabilities.js +++ b/packages/dd-trace/src/appsec/remote_config/capabilities.js @@ -24,6 +24,7 @@ module.exports = { APM_TRACING_SAMPLE_RULES: 1n << 29n, ASM_AUTO_USER_INSTRUM_MODE: 1n << 31n, ASM_ENDPOINT_FINGERPRINT: 1n << 32n, + ASM_SESSION_FINGERPRINT: 1n << 33n, // should we only send this if app uses express-session ? ASM_NETWORK_FINGERPRINT: 1n << 34n, ASM_HEADER_FINGERPRINT: 1n << 35n, ASM_RASP_CMDI: 1n << 37n From 721574dd3447ba34edb5d35e824c3e12d18d5e34 Mon Sep 17 00:00:00 2001 From: simon-id Date: Wed, 15 Jan 2025 14:29:33 +0100 Subject: [PATCH 3/8] add session id tracking in sdk --- packages/dd-trace/src/appsec/sdk/set_user.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/dd-trace/src/appsec/sdk/set_user.js b/packages/dd-trace/src/appsec/sdk/set_user.js index 6efe44ebd41..09e521121ee 100644 --- a/packages/dd-trace/src/appsec/sdk/set_user.js +++ b/packages/dd-trace/src/appsec/sdk/set_user.js @@ -21,6 +21,10 @@ function setUser (tracer, user) { return } + if (user.session_id && typeof user.session_id === 'string') { + persistent['usr.session_id'] = user.session_id + } + setUserTags(user, rootSpan) } From a79fdb9df519705441e751b99f956b843af04487 Mon Sep 17 00:00:00 2001 From: simon-id Date: Wed, 15 Jan 2025 16:15:31 +0100 Subject: [PATCH 4/8] add comment --- packages/dd-trace/src/appsec/sdk/set_user.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/dd-trace/src/appsec/sdk/set_user.js b/packages/dd-trace/src/appsec/sdk/set_user.js index 09e521121ee..49d9afa9447 100644 --- a/packages/dd-trace/src/appsec/sdk/set_user.js +++ b/packages/dd-trace/src/appsec/sdk/set_user.js @@ -21,6 +21,11 @@ function setUser (tracer, user) { return } + /* + When a user provides their own session ID through the use of the SDK using the session_id key: + If libddwaf hasn’t already been called with the usr.session_id address, it should be called with the provided session_id and further calls through the automated collection method should be inhibited. + If libddwaf has already been called with the usr.session_id address, it should be called again. + */ if (user.session_id && typeof user.session_id === 'string') { persistent['usr.session_id'] = user.session_id } From a022df511161bef0155726103d3f06ea5a730a1e Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 17 Jan 2025 16:53:51 +0100 Subject: [PATCH 5/8] add instrum hook --- packages/datadog-instrumentations/src/helpers/hooks.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/datadog-instrumentations/src/helpers/hooks.js b/packages/datadog-instrumentations/src/helpers/hooks.js index 4ea35f50218..ba86e2b516c 100644 --- a/packages/datadog-instrumentations/src/helpers/hooks.js +++ b/packages/datadog-instrumentations/src/helpers/hooks.js @@ -47,6 +47,7 @@ module.exports = { elasticsearch: () => require('../elasticsearch'), express: () => require('../express'), 'express-mongo-sanitize': () => require('../express-mongo-sanitize'), + 'express-session': () => require('../express-session'), fastify: () => require('../fastify'), 'find-my-way': () => require('../find-my-way'), fs: () => require('../fs'), From e487811a33d45aa8d30cc596525c8d6f8e5076e2 Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 17 Jan 2025 17:00:00 +0100 Subject: [PATCH 6/8] cleanup instrum --- .../src/express-session.js | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/packages/datadog-instrumentations/src/express-session.js b/packages/datadog-instrumentations/src/express-session.js index 12e41ae7847..c7e810415e3 100644 --- a/packages/datadog-instrumentations/src/express-session.js +++ b/packages/datadog-instrumentations/src/express-session.js @@ -1,8 +1,7 @@ 'use strict' -const { addHook } = require('./helpers/instrument') const shimmer = require('../../datadog-shimmer') -const { channel } = require('./helpers/instrument') +const { channel, addHook } = require('./helpers/instrument') const sessionMiddlewareFinishCh = channel('datadog:express-session:middleware:finish') @@ -17,7 +16,7 @@ function wrapSessionMiddleware (sessionMiddleware) { if (abortController.signal.aborted) return } - + return next.apply(this, arguments) } }) @@ -36,17 +35,7 @@ function wrapSession (session) { addHook({ name: 'express-session', - versions: ['>=0.3.0'] // TODO + versions: ['>=1.0.1'] }, session => { return shimmer.wrapFunction(session, wrapSession) }) - - - return shimmer.wrapFunction(session, function wrapSession { - const queryMiddleware = query.apply(this, arguments) - - return shimmer.wrapFunction(queryMiddleware, queryMiddleware => function (req, res, next) { - arguments[2] = publishQueryParsedAndNext(req, res, next) - return queryMiddleware.apply(this, arguments) - }) - }) \ No newline at end of file From 1914a35b48483d6c00dc723faf22d38c0dd7e6a3 Mon Sep 17 00:00:00 2001 From: simon-id Date: Sat, 18 Jan 2025 11:40:11 +0100 Subject: [PATCH 7/8] add session blocking --- packages/dd-trace/src/appsec/addresses.js | 1 + packages/dd-trace/src/appsec/channels.js | 1 + packages/dd-trace/src/appsec/index.js | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/packages/dd-trace/src/appsec/addresses.js b/packages/dd-trace/src/appsec/addresses.js index 20290baf9c4..f7073af3fd0 100644 --- a/packages/dd-trace/src/appsec/addresses.js +++ b/packages/dd-trace/src/appsec/addresses.js @@ -22,6 +22,7 @@ module.exports = { USER_ID: 'usr.id', USER_LOGIN: 'usr.login', + USER_SESSION_ID: 'usr.session_id', WAF_CONTEXT_PROCESSOR: 'waf.context.processor', diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index 1368e937dc9..6163a4f311a 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -14,6 +14,7 @@ module.exports = { incomingHttpRequestStart: dc.channel('dd-trace:incomingHttpRequestStart'), incomingHttpRequestEnd: dc.channel('dd-trace:incomingHttpRequestEnd'), passportVerify: dc.channel('datadog:passport:verify:finish'), + expressSession: dc.channel('datadog:express-session:middleware:finish'), queryParser: dc.channel('datadog:query:read:finish'), setCookieChannel: dc.channel('datadog:iast:set-cookie'), nextBodyParsed: dc.channel('apm:next:body-parsed'), diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index db089a61dca..d0c888b4c0e 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -10,6 +10,7 @@ const { incomingHttpRequestStart, incomingHttpRequestEnd, passportVerify, + expressSession, queryParser, nextBodyParsed, nextQueryParsed, @@ -67,6 +68,7 @@ function enable (_config) { incomingHttpRequestStart.subscribe(incomingHttpStartTranslator) incomingHttpRequestEnd.subscribe(incomingHttpEndTranslator) passportVerify.subscribe(onPassportVerify) // possible optimization: only subscribe if collection mode is enabled + expressSession.subscribe(onExpressSession) queryParser.subscribe(onRequestQueryParsed) nextBodyParsed.subscribe(onRequestBodyParsed) nextQueryParsed.subscribe(onRequestQueryParsed) @@ -197,6 +199,22 @@ function onPassportVerify ({ framework, login, user, success, abortController }) handleResults(results, store.req, store.req.res, rootSpan, abortController) } +function onExpressSession ({ req, res, sessionId, abortController }) { + const rootSpan = web.root(req) + if (!rootSpan) { + log.warn('[ASM] No rootSpan found in onExpressSession') + return + } + + const results = waf.run({ + persistent: { + [addresses.USER_SESSION_ID]: sessionId + } + }, req) + + handleResults(results, req, res, rootSpan, abortController) +} + function onRequestQueryParsed ({ req, res, query, abortController }) { if (!query || typeof query !== 'object') return @@ -310,6 +328,7 @@ function disable () { if (incomingHttpRequestStart.hasSubscribers) incomingHttpRequestStart.unsubscribe(incomingHttpStartTranslator) if (incomingHttpRequestEnd.hasSubscribers) incomingHttpRequestEnd.unsubscribe(incomingHttpEndTranslator) if (passportVerify.hasSubscribers) passportVerify.unsubscribe(onPassportVerify) + if (expressSession.hasSubscribers) expressSession.unsubscribe(onExpressSession) if (queryParser.hasSubscribers) queryParser.unsubscribe(onRequestQueryParsed) if (nextBodyParsed.hasSubscribers) nextBodyParsed.unsubscribe(onRequestBodyParsed) if (nextQueryParsed.hasSubscribers) nextQueryParsed.unsubscribe(onRequestQueryParsed) From 2e652bb97d2d8165174b1ca84ba16c89e60804f7 Mon Sep 17 00:00:00 2001 From: simon-id Date: Sat, 18 Jan 2025 15:52:01 +0100 Subject: [PATCH 8/8] cleanup --- packages/dd-trace/src/appsec/index.js | 1 + packages/dd-trace/src/appsec/sdk/set_user.js | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index d0c888b4c0e..bc741af6877 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -206,6 +206,7 @@ function onExpressSession ({ req, res, sessionId, abortController }) { return } + // do not call this if the SDK already called it const results = waf.run({ persistent: { [addresses.USER_SESSION_ID]: sessionId diff --git a/packages/dd-trace/src/appsec/sdk/set_user.js b/packages/dd-trace/src/appsec/sdk/set_user.js index 49d9afa9447..09e521121ee 100644 --- a/packages/dd-trace/src/appsec/sdk/set_user.js +++ b/packages/dd-trace/src/appsec/sdk/set_user.js @@ -21,11 +21,6 @@ function setUser (tracer, user) { return } - /* - When a user provides their own session ID through the use of the SDK using the session_id key: - If libddwaf hasn’t already been called with the usr.session_id address, it should be called with the provided session_id and further calls through the automated collection method should be inhibited. - If libddwaf has already been called with the usr.session_id address, it should be called again. - */ if (user.session_id && typeof user.session_id === 'string') { persistent['usr.session_id'] = user.session_id }