From 1d2543c90a405971776963f5d57a187af36c000c Mon Sep 17 00:00:00 2001 From: Thomas Hunter II Date: Wed, 25 Sep 2024 11:58:31 -0700 Subject: [PATCH] always enable tracing header injection for AWS requests (#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, 24 insertions(+), 454 deletions(-) diff --git a/docs/test.ts b/docs/test.ts index 07b96a01673..e37177e0898 100644 --- a/docs/test.ts +++ b/docs/test.ts @@ -320,9 +320,6 @@ 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 1d0e2473d01..02c84fb47d3 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1030,14 +1030,6 @@ 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 848b00855d4..4f68f5fbf94 100644 --- a/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js +++ b/packages/datadog-plugin-aws-sdk/test/aws-sdk.spec.js @@ -114,6 +114,28 @@ 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 1d322de04a4..b469f4a9722 100644 --- a/packages/datadog-plugin-fetch/test/index.spec.js +++ b/packages/datadog-plugin-fetch/test/index.spec.js @@ -215,102 +215,6 @@ 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 d4c105d2508..55a025f4970 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.shouldInjectTraceHeaders(options, uri)) { + if (this.config.propagationFilter(uri)) { this.tracer.inject(span, HTTP_HEADERS, options.headers) } @@ -71,18 +71,6 @@ 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 } @@ -212,31 +200,6 @@ 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 @@ -248,8 +211,4 @@ 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 42f4c8436f8..268aff9b238 100644 --- a/packages/datadog-plugin-http/test/client.spec.js +++ b/packages/datadog-plugin-http/test/client.spec.js @@ -446,116 +446,6 @@ 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() @@ -1093,50 +983,6 @@ 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 296f1161e59..3f8d996fcd3 100644 --- a/packages/datadog-plugin-http2/src/client.js +++ b/packages/datadog-plugin-http2/src/client.js @@ -62,9 +62,7 @@ class Http2ClientPlugin extends ClientPlugin { addHeaderTags(span, headers, HTTP_REQUEST_HEADERS, this.config) - if (!hasAmazonSignature(headers, path)) { - this.tracer.inject(span, HTTP_HEADERS, headers) - } + this.tracer.inject(span, HTTP_HEADERS, headers) message.parentStore = store message.currentStore = { ...store, span } @@ -134,29 +132,6 @@ 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 f8d44f3ac0b..cfdedcde489 100644 --- a/packages/datadog-plugin-http2/test/client.spec.js +++ b/packages/datadog-plugin-http2/test/client.spec.js @@ -365,131 +365,6 @@ 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({