fix(announce): verify connection to Instana host agent via payload check

This is in a sense a follow up to commit
7d6a05b which removed the Server header
check after that was found unreliable in the presence of service meshes
and certain proxies like Envoy.

packages/collector/src/announceCycle/agentHostLookup.js

@@ -122,16 +122,18 @@ function checkHost(host, cb) {
       },
       res => {
         if (res.statusCode >= 200 && res.statusCode < 300) {
-          cb(null);
+          handleResponse(host, res, cb);
         } else {
+          // We are not interested in the body of a non-2xx HTTP response, but since we have added a response handler,
+          // we must consume the response body, see https://nodejs.org/api/http.html#class-httpclientrequest.
+          res.resume();
           cb(
             new Error(
               `The attempt to connect to the Instana host agent on ${host}:${agentOpts.port} has failed with an ` +
                 `unexpected status code. Expected HTTP 200 but received: ${res.statusCode}`
             )
           );
         }
-        res.resume();
       }
     );
   } catch (e) {
@@ -163,6 +165,53 @@ function checkHost(host, cb) {
   req.end();
 }
 
+/**
+ * Checks the response payload to determine whether we have actually connected to an Instana host agent.
+ *
+ * @param {string} host the target IP (will only be used for logging in this method)
+ * @param {import('http').IncomingMessage} res the HTTP response
+ * @param {(...args: *) => *} cb
+ */
+function handleResponse(host, res, cb) {
+  /* eslint-disable max-len */
+  // We inspect the payload, checking whether it is JSON and has the "version" property, to verify that we have
+  // actually connected to an Instana host agent. See
+  // https://github.ibm.com/instana/technical-documentation/blob/master/tracing/specification/README.md#phase-1--host-lookup
+  /* eslint-enable max-len */
+
+  res.setEncoding('utf8');
+  let responsePayload = '';
+  res.on('data', chunk => {
+    responsePayload += chunk;
+  });
+  res.on('end', () => {
+    let responseJson;
+    try {
+      responseJson = JSON.parse(responsePayload);
+    } catch (e) {
+      cb(
+        new Error(
+          `The attempt to connect to the Instana host agent on ${host}:${agentOpts.port} has failed, the response ` +
+            `cannot be parsed as JSON. The party listening on ${host}:${agentOpts.port} does not seem to be an ` +
+            `Instana host agent. Full response: ${responsePayload}`
+        )
+      );
+      return;
+    }
+    if (responseJson.version !== undefined) {
+      cb(null);
+    } else {
+      cb(
+        new Error(
+          `The attempt to connect to the Instana host agent on ${host}:${agentOpts.port} has failed, the response did` +
+            ` not have a "version" property. The party listening on ${host}:${agentOpts.port} does not seem to be an ` +
+            `Instana host agent. Full response: ${responsePayload}`
+        )
+      );
+    }
+  });
+}
+
 /**
  * @param {Error} error
  */

packages/collector/test/announceCycle/agentHostLookup_test.js

@@ -33,9 +33,21 @@ describe('agent host lookup state', () => {
   const hostAndPort = { host, port };
   const defaultGatewayIpPort = { host: defaultGatewayIp, port };
 
+  const responseOk = (req, cb) => {
+    const res = new FakeResponse(200, { version: '1.2.3' });
+    cb(res);
+    res.emitPayload();
+  };
+
+  const responseNotAnAgent = (req, cb) => {
+    const res = new FakeResponse(200, { message: 'this is not an Instana host agent' });
+    cb(res);
+    res.emitPayload();
+  };
+
   const configuredHostLookupOk = new FakeRequestHandler({
     when: hostAndPort,
-    then: (req, cb) => cb(new FakeResponse(200))
+    then: responseOk
   });
 
   const configuredHostLookupConnectionRefused = new FakeRequestHandler({
@@ -44,9 +56,15 @@ describe('agent host lookup state', () => {
     onlyOnce: true
   });
 
+  const configuredHostLookupNotAnAgent = new FakeRequestHandler({
+    when: hostAndPort,
+    then: responseNotAnAgent,
+    onlyOnce: true
+  });
+
   const defaultGatewayLookupOk = new FakeRequestHandler({
     when: defaultGatewayIpPort,
-    then: (req, cb) => cb(new FakeResponse(200))
+    then: responseOk
   });
 
   const defaultGatewayLookupConnectionRefused = new FakeRequestHandler({
@@ -145,6 +163,38 @@ describe('agent host lookup state', () => {
     });
   });
 
+  describe('via default gateway IP when something else listens on 127.0.0.1:42699', () => {
+    before(() => {
+      agentOptsMock.host = hostAndPort.host;
+      agentOptsMock.port = hostAndPort.port;
+      parseProcSelfNetRouteFileStub.callsFake(async () => defaultGatewayIp);
+      requestStub.callsFake((opt, cb) => {
+        fakeRequest = new FakeRequest(
+          [
+            //
+            configuredHostLookupNotAnAgent,
+            defaultGatewayLookupOk
+          ],
+          opt,
+          cb
+        );
+        return fakeRequest;
+      });
+    });
+
+    it('should not connect to 127.0.0.1:42699 but find the agent at the default gateway IP', done => {
+      transitionTo.callsFake(newState => {
+        expect(configuredHostLookupNotAnAgent.hasBeenCalled()).to.be.true;
+        expect(parseProcSelfNetRouteFileStub).to.have.been.called;
+        expect(defaultGatewayLookupOk.hasBeenCalled()).to.be.true;
+        expect(newState).to.equal('unannounced');
+        done();
+      });
+
+      agentHostLookupState.enter(ctxStub);
+    });
+  });
+
   describe('retry when default gateway IP cannot be determined', () => {
     before(() => {
       agentOptsMock.host = hostAndPort.host;

packages/collector/test/apps/agentStub.js

@@ -51,7 +51,7 @@ app.use(
 );
 
 app.get('/', (req, res) => {
-  res.send('OK');
+  res.json({ version: '1.1.999' });
 });
 
 app.put('/com.instana.plugin.nodejs.discovery', (req, res) => {

packages/collector/test/test_util/fake_http/FakeResponse.js

@@ -4,10 +4,25 @@
 
 'use strict';
 
-module.exports = class FakeResponse {
-  constructor(statusCode) {
+const EventEmitter = require('events');
+
+module.exports = class FakeResponse extends EventEmitter {
+  constructor(statusCode, payload) {
+    super();
     this.statusCode = statusCode;
+    if (payload) {
+      this.payload = Buffer.from(JSON.stringify(payload), 'utf8');
+    }
+  }
+
+  emitPayload() {
+    if (this.payload) {
+      this.emit('data', this.payload);
+    }
+    this.emit('end');
   }
 
+  setEncoding() {}
+
   resume() {}
 };

packages/collector/test/tracing/database/ioredis/app.js

@@ -89,8 +89,8 @@ app.get('/keepTracing', (req, res) => {
       // Execute another traced call to verify that we keep the tracing context.
       return request(`http://127.0.0.1:${agentPort}`);
     })
-    .then(httpRes => {
-      res.send(`${httpRes};${redisResponse}`);
+    .then(() => {
+      res.send(redisResponse);
     })
     .catch(err => {
       log('Unexpected error for key %s', key, err);
@@ -111,12 +111,12 @@ app.get('/keepTracingCallback', (req, res) => {
         return;
       }
       // Execute another traced call to verify that we keep the tracing context.
-      request(`http://127.0.0.1:${agentPort}`, (httpErr, httpRes) => {
+      request(`http://127.0.0.1:${agentPort}`, httpErr => {
         if (httpErr) {
           log('HTTP call failed', httpErr);
           return reject(httpErr);
         }
-        return resolve(`${httpRes.body};${redisRes}`);
+        return resolve(redisRes);
       });
     });
   })

packages/collector/test/tracing/database/ioredis/test.js

@@ -121,7 +121,7 @@ mochaSuiteFn('tracing/ioredis', function () {
         })
       )
       .then(response => {
-        expect(String(response)).to.equal('OK;13');
+        expect(String(response)).to.equal('13');
 
         return retry(() =>
           agentControls.getSpans().then(spans => {
@@ -201,7 +201,7 @@ mochaSuiteFn('tracing/ioredis', function () {
         })
       )
       .then(response => {
-        expect(String(response)).to.equal('OK;13');
+        expect(String(response)).to.equal('13');
 
         return retry(() =>
           agentControls.getSpans().then(spans => {