Skip to content

Commit a955194

Browse files
committed
fix(exporters): keep the Datadog API key off cleartext for agentless intake (#8847)
* fix(exporters): send agentless spans to the https intake, not the agent URL The agentless APM exporter took its intake from config.url, which defaults to the local agent URL over plain http. Agentless traffic carries the Datadog API key, so the key was pointed at the agent's cleartext endpoint instead of the intake. The exporter now always builds the intake from the configured site over https and never consults the agent URL; DD_SITE stays the knob for redirecting it. Refs: https://datadoghq.atlassian.net/browse/APMSP-3483 * fix(exporters): never send the Datadog API key over cleartext http A misconfigured or default intake URL could point agentless traffic at a non-loopback host over plain http, putting the dd-api-key on the wire in the clear where any on-path observer could read it. Strip the dd-api-key header (both casings) and log when a request targets a non-loopback host over http. Loopback hosts (localhost, 127.0.0.0/8, ::1) stay exempt so the local agent and dev proxies keep working. Stripping rather than dropping the request preserves agent-proxied telemetry, which authenticates with its own key rather than the header.
1 parent 3ecfbcf commit a955194

4 files changed

Lines changed: 128 additions & 16 deletions

File tree

packages/dd-trace/src/exporters/agentless/index.js

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,21 @@ class AgentlessExporter {
1818

1919
/**
2020
* @param {object} config - Configuration object
21-
* @param {string} [config.site] - The Datadog site. Defaults to 'datadoghq.com'.
22-
* @param {string} [config.url] - Override intake URL
21+
* @param {string} [config.site] - The Datadog site. Defaults to 'datadoghq.com'.
2322
* @param {number} [config.flushInterval] - Batch flush interval in ms
2423
* @param {string} [config.env] - Environment name
2524
* @param {object} [config.tags] - Tags including runtime-id
2625
*/
2726
constructor (config) {
2827
this._config = config
29-
const { site = 'datadoghq.com', url } = config
28+
const site = config.site ?? 'datadoghq.com'
3029

3130
try {
32-
this._url = url ? new URL(url) : new URL(`https://public-trace-http-intake.logs.${site}`)
31+
// Agentless traffic carries the Datadog API key, so the intake is always the public https
32+
// endpoint; never derive it from config.url (the agent's cleartext http) or the key leaks.
33+
this._url = new URL(`https://public-trace-http-intake.logs.${site}`)
3334
} catch (err) {
34-
log.error(
35-
'Invalid URL configuration for agentless exporter. url=%s, site=%s. Error: %s',
36-
url || 'not set',
37-
site,
38-
err.message
39-
)
35+
log.error('Invalid site for agentless exporter. site=%s. Error: %s', site, err.message)
4036
this._url = null
4137
}
4238

packages/dd-trace/src/exporters/common/request.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
const { Readable } = require('stream')
77
const http = require('http')
88
const https = require('https')
9+
const net = require('net')
910
const zlib = require('zlib')
1011

1112
const { storage } = require('../../../../datadog-core')
@@ -45,6 +46,17 @@ function parseUrl (urlObjOrString) {
4546
return url
4647
}
4748

49+
/**
50+
* @param {string} hostname Host as resolved by {@link parseUrl}; IPv6 is unbracketed (`::1`).
51+
*/
52+
function isLoopbackHost (hostname) {
53+
// The 127.0.0.0/8 block is loopback, but only when the host is an actual IPv4 literal: a
54+
// hostname like `127.evil.com` shares the prefix yet resolves anywhere, so net.isIPv4 gates it.
55+
return hostname === 'localhost' ||
56+
hostname === '::1' ||
57+
(hostname.startsWith('127.') && net.isIPv4(hostname))
58+
}
59+
4860
/**
4961
* @param {Buffer|string|Readable|Array<Buffer|string>} data
5062
* @param {object} options
@@ -67,6 +79,20 @@ function request (data, options, callback) {
6779
}
6880
}
6981

82+
// Never put the Datadog API key on a cleartext connection to a non-loopback host; that would
83+
// expose it on the wire. Loopback (local agent, dev proxy, tests) is exempt. Strip the key
84+
// rather than drop the request: the agent proxies telemetry with its own key, while an https
85+
// intake URL is required to authenticate agentless traffic.
86+
const hasApiKey = options.headers['dd-api-key'] !== undefined || options.headers['DD-API-KEY'] !== undefined
87+
if (hasApiKey && options.protocol === 'http:' && !isLoopbackHost(options.hostname)) {
88+
log.error(
89+
'Not sending the Datadog API key over a non-TLS connection to %s. Configure an https intake URL.',
90+
options.hostname
91+
)
92+
delete options.headers['dd-api-key']
93+
delete options.headers['DD-API-KEY']
94+
}
95+
7096
if (data instanceof Readable) {
7197
const chunks = []
7298

packages/dd-trace/test/exporters/agentless/exporter.spec.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,10 @@ describe('AgentlessExporter', () => {
5454
sinon.assert.match(exporter._url.href, expectedUrl.href)
5555
})
5656

57-
it('should use provided URL', () => {
58-
const customUrl = 'https://custom-intake.example.com'
59-
exporter = new Exporter({ url: customUrl, site: 'datadoghq.com' })
57+
it('should send to the https intake and ignore the agent URL (config.url)', () => {
58+
exporter = new Exporter({ url: 'http://127.0.0.1:8126', site: 'datadoghq.com' })
6059

61-
sinon.assert.match(exporter._url.href, customUrl)
60+
assert.strictEqual(exporter._url.href, 'https://public-trace-http-intake.logs.datadoghq.com/')
6261
})
6362

6463
it('should default to datadoghq.com site', () => {
@@ -77,15 +76,15 @@ describe('AgentlessExporter', () => {
7776
)
7877
})
7978

80-
it('should handle invalid URL gracefully', () => {
79+
it('should handle an invalid site gracefully', () => {
8180
const log = { error: sinon.spy() }
8281

8382
Exporter = proxyquire('../../../src/exporters/agentless', {
8483
'./writer': function () { return writer },
8584
'../../log': log,
8685
})
8786

88-
exporter = new Exporter({ url: 'not-a-valid-url' })
87+
exporter = new Exporter({ site: 'bad host' })
8988

9089
sinon.assert.calledOnce(log.error)
9190
assert.strictEqual(exporter._url, null)

packages/dd-trace/test/exporters/common/request.spec.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,4 +605,95 @@ describe('request', function () {
605605
})
606606
}
607607
})
608+
609+
describe('stripping the Datadog API key from a non-TLS connection', () => {
610+
// `badheaders` only matches when the key is absent, so a passing request proves it was
611+
// stripped; a regression that left the key on would miss the interceptor and surface here.
612+
it('strips dd-api-key when sending over http to a non-loopback host', (done) => {
613+
nock('http://intake.example.com', { badheaders: ['dd-api-key'] })
614+
.post('/v1/input')
615+
.reply(200, 'OK')
616+
617+
request(Buffer.from(''), {
618+
method: 'POST',
619+
url: new URL('http://intake.example.com/v1/input'),
620+
headers: { 'dd-api-key': 'secret-key' },
621+
}, (err, res) => {
622+
assert.strictEqual(res, 'OK')
623+
sinon.assert.calledOnce(log.error)
624+
assert.match(log.error.getCall(0).args[0], /non-TLS connection/)
625+
done(err)
626+
})
627+
})
628+
629+
it('strips the DD-API-KEY header casing as well', (done) => {
630+
nock('http://intake.example.com', { badheaders: ['dd-api-key'] })
631+
.post('/v1/input')
632+
.reply(200, 'OK')
633+
634+
request(Buffer.from(''), {
635+
method: 'POST',
636+
url: new URL('http://intake.example.com/v1/input'),
637+
headers: { 'DD-API-KEY': 'secret-key' },
638+
}, (err, res) => {
639+
assert.strictEqual(res, 'OK')
640+
sinon.assert.calledOnce(log.error)
641+
done(err)
642+
})
643+
})
644+
645+
it('strips dd-api-key for a non-loopback host that merely starts with "127."', (done) => {
646+
nock('http://127.evil.com', { badheaders: ['dd-api-key'] })
647+
.post('/v1/input')
648+
.reply(200, 'OK')
649+
650+
request(Buffer.from(''), {
651+
method: 'POST',
652+
url: new URL('http://127.evil.com/v1/input'),
653+
headers: { 'dd-api-key': 'secret-key' },
654+
}, (err, res) => {
655+
assert.strictEqual(res, 'OK')
656+
sinon.assert.calledOnce(log.error)
657+
done(err)
658+
})
659+
})
660+
661+
for (const loopbackHost of ['127.0.0.1', '127.1.2.3', 'localhost', '[::1]']) {
662+
it(`keeps dd-api-key over http to the loopback host ${loopbackHost}`, (done) => {
663+
nock(`http://${loopbackHost}:9999`, {
664+
reqheaders: { 'dd-api-key': 'secret-key' },
665+
})
666+
.post('/v1/input')
667+
.reply(200, 'OK')
668+
669+
request(Buffer.from(''), {
670+
method: 'POST',
671+
url: new URL(`http://${loopbackHost}:9999/v1/input`),
672+
headers: { 'dd-api-key': 'secret-key' },
673+
}, (err, res) => {
674+
assert.strictEqual(res, 'OK')
675+
sinon.assert.notCalled(log.error)
676+
done(err)
677+
})
678+
})
679+
}
680+
681+
it('keeps dd-api-key over https to a non-loopback host', (done) => {
682+
nock('https://intake.example.com', {
683+
reqheaders: { 'dd-api-key': 'secret-key' },
684+
})
685+
.post('/v1/input')
686+
.reply(200, 'OK')
687+
688+
request(Buffer.from(''), {
689+
method: 'POST',
690+
url: new URL('https://intake.example.com/v1/input'),
691+
headers: { 'dd-api-key': 'secret-key' },
692+
}, (err, res) => {
693+
assert.strictEqual(res, 'OK')
694+
sinon.assert.notCalled(log.error)
695+
done(err)
696+
})
697+
})
698+
})
608699
})

0 commit comments

Comments
 (0)