From d724f218d4d868cf1cdf9e8c85fba40d63b67523 Mon Sep 17 00:00:00 2001 From: Thomas Hunter II Date: Thu, 7 Nov 2024 03:24:13 -0800 Subject: [PATCH] Revert "always enable tracing header injection for AWS requests (#4717)" (#4867) - this reverts commit 1d2543c90a405971776963f5d57a187af36c000c. - reverts a change that would automatically inject tracing headers into AWS requests - this appears to break S3 requests (and DynamoDB?) when using AWS SDK v2 - we don't have any reports of other services or of AWS SDK v3 breaking - for follow up work we need to make this a configurable environment variable instead of just an init setting - this is because folks using the lambda layer need to configure the tracer via env vars - alternatively we only block s3 and dynamo? however there could be other services that fail... - alternatively we only block aws sdk v2? however it seems that a bunch of the services are fine... - internal stuff: APMS-13694, APMS-13713 - more discussion in #4717 --- docs/test.ts | 3 + index.d.ts | 8 + .../test/aws-sdk.spec.js | 22 --- .../datadog-plugin-fetch/test/index.spec.js | 96 +++++++++++ packages/datadog-plugin-http/src/client.js | 43 ++++- .../datadog-plugin-http/test/client.spec.js | 154 ++++++++++++++++++ packages/datadog-plugin-http2/src/client.js | 27 ++- .../datadog-plugin-http2/test/client.spec.js | 125 ++++++++++++++ 8 files changed, 454 insertions(+), 24 deletions(-) diff --git a/docs/test.ts b/docs/test.ts index 6c3f54c2598..37342718c2a 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -324,6 +324,9 @@ tracer.use('http', { tracer.use('http', { client: httpClientOptions }); +tracer.use('http', { + enablePropagationWithAmazonHeaders: true +}); tracer.use('http2'); tracer.use('http2', { server: http2ServerOptions diff --git a/index.d.ts b/index.d.ts index 86673bd2c98..f8f06cacbae 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1043,6 +1043,14 @@ declare namespace tracer { * @default code => code < 500 */ validateStatus?: (code: number) => boolean; + + /** + * Enable injection of tracing headers into requests signed with AWS IAM headers. + * Disable this if you get AWS signature errors (HTTP 403). + * + * @default false + */ + enablePropagationWithAmazonHeaders?: boolean; } /** @hidden */ diff --git a/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js b/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js index 4f68f5fbf94..848b00855d4 100644 --- a/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js +++ b/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js @@ -114,28 +114,6 @@ describe('Plugin', () => { s3.listBuckets({}, e => e && done(e)) }) - // different versions of aws-sdk use different casings and different AWS headers - it('should include tracing headers and not cause a 403 error', (done) => { - const HttpClientPlugin = require('../../datadog-plugin-http/src/client.js') - const spy = sinon.spy(HttpClientPlugin.prototype, 'bindStart') - agent.use(traces => { - const headers = new Set( - Object.keys(spy.firstCall.firstArg.args.options.headers) - .map(x => x.toLowerCase()) - ) - spy.restore() - - expect(headers).to.include('authorization') - expect(headers).to.include('x-amz-date') - expect(headers).to.include('x-datadog-trace-id') - expect(headers).to.include('x-datadog-parent-id') - expect(headers).to.include('x-datadog-sampling-priority') - expect(headers).to.include('x-datadog-tags') - }).then(done, done) - - s3.listBuckets({}, e => e && done(e)) - }) - it('should mark error responses', (done) => { let error diff --git a/packages/datadog-plugin-fetch/test/index.spec.js b/packages/datadog-plugin-fetch/test/index.spec.js index b469f4a9722..1d322de04a4 100644 --- a/packages/datadog-plugin-fetch/test/index.spec.js +++ b/packages/datadog-plugin-fetch/test/index.spec.js @@ -215,6 +215,102 @@ describe('Plugin', () => { }) }) + it('should skip injecting if the Authorization header contains an AWS signature', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + fetch(`http://localhost:${port}/`, { + headers: { + Authorization: 'AWS4-HMAC-SHA256 ...' + } + }) + }) + }) + + it('should skip injecting if one of the Authorization headers contains an AWS signature', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + fetch(`http://localhost:${port}/`, { + headers: { + Authorization: ['AWS4-HMAC-SHA256 ...'] + } + }) + }) + }) + + it('should skip injecting if the X-Amz-Signature header is set', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + fetch(`http://localhost:${port}/`, { + headers: { + 'X-Amz-Signature': 'abc123' + } + }) + }) + }) + + it('should skip injecting if the X-Amz-Signature query param is set', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + fetch(`http://localhost:${port}/?X-Amz-Signature=abc123`) + }) + }) + it('should handle connection errors', done => { let error diff --git a/packages/datadog-plugin-http/src/client.js b/packages/datadog-plugin-http/src/client.js index 55a025f4970..d4c105d2508 100644 --- a/packages/datadog-plugin-http/src/client.js +++ b/packages/datadog-plugin-http/src/client.js @@ -58,7 +58,7 @@ class HttpClientPlugin extends ClientPlugin { span._spanContext._trace.record = false } - if (this.config.propagationFilter(uri)) { + if (this.shouldInjectTraceHeaders(options, uri)) { this.tracer.inject(span, HTTP_HEADERS, options.headers) } @@ -71,6 +71,18 @@ class HttpClientPlugin extends ClientPlugin { return message.currentStore } + shouldInjectTraceHeaders (options, uri) { + if (hasAmazonSignature(options) && !this.config.enablePropagationWithAmazonHeaders) { + return false + } + + if (!this.config.propagationFilter(uri)) { + return false + } + + return true + } + bindAsyncStart ({ parentStore }) { return parentStore } @@ -200,6 +212,31 @@ function getHooks (config) { return { request } } +function hasAmazonSignature (options) { + if (!options) { + return false + } + + if (options.headers) { + const headers = Object.keys(options.headers) + .reduce((prev, next) => Object.assign(prev, { + [next.toLowerCase()]: options.headers[next] + }), {}) + + if (headers['x-amz-signature']) { + return true + } + + if ([].concat(headers.authorization).some(startsWith('AWS4-HMAC-SHA256'))) { + return true + } + } + + const search = options.search || options.path + + return search && search.toLowerCase().indexOf('x-amz-signature=') !== -1 +} + function extractSessionDetails (options) { if (typeof options === 'string') { return new URL(options).host @@ -211,4 +248,8 @@ function extractSessionDetails (options) { return { host, port } } +function startsWith (searchString) { + return value => String(value).startsWith(searchString) +} + module.exports = HttpClientPlugin diff --git a/packages/datadog-plugin-http/test/client.spec.js b/packages/datadog-plugin-http/test/client.spec.js index 268aff9b238..42f4c8436f8 100644 --- a/packages/datadog-plugin-http/test/client.spec.js +++ b/packages/datadog-plugin-http/test/client.spec.js @@ -446,6 +446,116 @@ describe('Plugin', () => { }) }) + it('should skip injecting if the Authorization header contains an AWS signature', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + const req = http.request({ + port, + headers: { + Authorization: 'AWS4-HMAC-SHA256 ...' + } + }) + + req.end() + }) + }) + + it('should skip injecting if one of the Authorization headers contains an AWS signature', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + const req = http.request({ + port, + headers: { + Authorization: ['AWS4-HMAC-SHA256 ...'] + } + }) + + req.end() + }) + }) + + it('should skip injecting if the X-Amz-Signature header is set', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + const req = http.request({ + port, + headers: { + 'X-Amz-Signature': 'abc123' + } + }) + + req.end() + }) + }) + + it('should skip injecting if the X-Amz-Signature query param is set', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.undefined + expect(req.get('x-datadog-parent-id')).to.be.undefined + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + const req = http.request({ + port, + path: '/?X-Amz-Signature=abc123' + }) + + req.end() + }) + }) + it('should run the callback in the parent context', done => { const app = express() @@ -983,6 +1093,50 @@ describe('Plugin', () => { }) }) + describe('with config enablePropagationWithAmazonHeaders enabled', () => { + let config + + beforeEach(() => { + config = { + enablePropagationWithAmazonHeaders: true + } + + return agent.load('http', config) + .then(() => { + http = require(pluginToBeLoaded) + express = require('express') + }) + }) + + it('should inject tracing header into AWS signed request', done => { + const app = express() + + app.get('/', (req, res) => { + try { + expect(req.get('x-datadog-trace-id')).to.be.a('string') + expect(req.get('x-datadog-parent-id')).to.be.a('string') + + res.status(200).send() + + done() + } catch (e) { + done(e) + } + }) + + appListener = server(app, port => { + const req = http.request({ + port, + headers: { + Authorization: 'AWS4-HMAC-SHA256 ...' + } + }) + + req.end() + }) + }) + }) + describe('with validateStatus configuration', () => { let config diff --git a/packages/datadog-plugin-http2/src/client.js b/packages/datadog-plugin-http2/src/client.js index 3f8d996fcd3..296f1161e59 100644 --- a/packages/datadog-plugin-http2/src/client.js +++ b/packages/datadog-plugin-http2/src/client.js @@ -62,7 +62,9 @@ class Http2ClientPlugin extends ClientPlugin { addHeaderTags(span, headers, HTTP_REQUEST_HEADERS, this.config) - this.tracer.inject(span, HTTP_HEADERS, headers) + if (!hasAmazonSignature(headers, path)) { + this.tracer.inject(span, HTTP_HEADERS, headers) + } message.parentStore = store message.currentStore = { ...store, span } @@ -132,6 +134,29 @@ function extractSessionDetails (authority, options) { return { protocol, port, host } } +function hasAmazonSignature (headers, path) { + if (headers) { + headers = Object.keys(headers) + .reduce((prev, next) => Object.assign(prev, { + [next.toLowerCase()]: headers[next] + }), {}) + + if (headers['x-amz-signature']) { + return true + } + + if ([].concat(headers.authorization).some(startsWith('AWS4-HMAC-SHA256'))) { + return true + } + } + + return path && path.toLowerCase().indexOf('x-amz-signature=') !== -1 +} + +function startsWith (searchString) { + return value => String(value).startsWith(searchString) +} + function getStatusValidator (config) { if (typeof config.validateStatus === 'function') { return config.validateStatus diff --git a/packages/datadog-plugin-http2/test/client.spec.js b/packages/datadog-plugin-http2/test/client.spec.js index cfdedcde489..f8d44f3ac0b 100644 --- a/packages/datadog-plugin-http2/test/client.spec.js +++ b/packages/datadog-plugin-http2/test/client.spec.js @@ -365,6 +365,131 @@ describe('Plugin', () => { }) }) + it('should skip injecting if the Authorization header contains an AWS signature', done => { + const app = (stream, headers) => { + try { + expect(headers['x-datadog-trace-id']).to.be.undefined + expect(headers['x-datadog-parent-id']).to.be.undefined + + stream.respond({ + ':status': 200 + }) + stream.end() + + done() + } catch (e) { + done(e) + } + } + + appListener = server(app, port => { + const headers = { + Authorization: 'AWS4-HMAC-SHA256 ...' + } + const client = http2 + .connect(`${protocol}://localhost:${port}`) + .on('error', done) + + const req = client.request(headers) + req.on('error', done) + + req.end() + }) + }) + + it('should skip injecting if one of the Authorization headers contains an AWS signature', done => { + const app = (stream, headers) => { + try { + expect(headers['x-datadog-trace-id']).to.be.undefined + expect(headers['x-datadog-parent-id']).to.be.undefined + + stream.respond({ + ':status': 200 + }) + stream.end() + + done() + } catch (e) { + done(e) + } + } + + appListener = server(app, port => { + const headers = { + Authorization: ['AWS4-HMAC-SHA256 ...'] + } + const client = http2 + .connect(`${protocol}://localhost:${port}`) + .on('error', done) + + const req = client.request(headers) + req.on('error', done) + + req.end() + }) + }) + + it('should skip injecting if the X-Amz-Signature header is set', done => { + const app = (stream, headers) => { + try { + expect(headers['x-datadog-trace-id']).to.be.undefined + expect(headers['x-datadog-parent-id']).to.be.undefined + + stream.respond({ + ':status': 200 + }) + stream.end() + + done() + } catch (e) { + done(e) + } + } + + appListener = server(app, port => { + const headers = { + 'X-Amz-Signature': 'abc123' + } + const client = http2 + .connect(`${protocol}://localhost:${port}`) + .on('error', done) + + const req = client.request(headers) + req.on('error', done) + + req.end() + }) + }) + + it('should skip injecting if the X-Amz-Signature query param is set', done => { + const app = (stream, headers) => { + try { + expect(headers['x-datadog-trace-id']).to.be.undefined + expect(headers['x-datadog-parent-id']).to.be.undefined + + stream.respond({ + ':status': 200 + }) + stream.end() + + done() + } catch (e) { + done(e) + } + } + + appListener = server(app, port => { + const client = http2 + .connect(`${protocol}://localhost:${port}`) + .on('error', done) + + const req = client.request({ ':path': '/?X-Amz-Signature=abc123' }) + req.on('error', done) + + req.end() + }) + }) + it('should run the callback in the parent context', done => { const app = (stream, headers) => { stream.respond({