Add support for verifying reply addresses

lib/builder/linux.js

@@ -22,6 +22,7 @@ var defaultConfig = {
     v6: false,
     sourceAddr: '',
     packetSize: 56,
+    ignoreDifferentAddressReply: false,
     extra: [],
 };
 
@@ -40,7 +41,7 @@ builder.getCommandArguments = function (target, config) {
 
     // Make every key in config has been setup properly
     var keys = ['numeric', 'timeout', 'deadline', 'min_reply', 'v6',
-        'sourceAddr', 'extra', 'packetSize'];
+        'sourceAddr', 'extra', 'packetSize', 'ignoreDifferentAddressReply'];
     keys.forEach(function (k) {
         // Falsy value will be overridden without below checking
         if (typeof (_config[k]) !== 'boolean') {

lib/index.js

@@ -18,7 +18,14 @@
  * @property {string} [sourceAddr] - source address for sending the ping
  * @property {number} [packetSize] - Specifies the number of data bytes to be sent
  *                                 Default: Linux / MAC: 56 Bytes,
- *                                          Window: 32 Bytes
+ *                                          Windows: 32 Bytes
+ * @property {boolean} [ignoreDifferentAddressReply] - Enables ignoring replies from different addresses
+ *                                 Default: Linux: false
+ *                                 Other platforms do not need this option.
+ * If set to true and a reply is received from an address which does not match the parsed
+ * `PingResponse.numeric_host`, the ping response is ignored.
+ * This helps against detecting a host as alive by accident when another host replies to the ping.
+ * This workaround addresses an imperfection in the ping implementation within the package iputils.*
  * @property {string[]} [extra] - Optional options does not provided
  */
 

lib/parser/mac.js

@@ -19,6 +19,16 @@ function MacParser(addr, config) {
 
 util.inherits(MacParser, base);
 
+/**
+ * Check whether a reply comes from the target address
+ * @param {string} line - A line from system ping
+ * @param {string} numericHost - Target IP address
+ * @returns {boolean} Whether the reply address matches the target
+ */
+function isReplyFromTarget(line, numericHost) {
+    return line.includes(` ${numericHost}: `) || line.includes(` (${numericHost}): `);
+}
+
 /**
  * Process output's header
  * @param {string} line - A line from system ping
@@ -43,7 +53,14 @@ MacParser.prototype._processBody = function (line) {
     if (count >= 3) {
         var regExp = /([0-9.]+)[ ]*ms/;
         var match = regExp.exec(line);
-        this._times.push(getFloatOrUnknown(match[1]));
+
+        // XXX: This option is only provided in linux builder, but the parser is shared.
+        // This option should have no effect on macOS,
+        // as ping on macOS does not accept replies from a different address
+        if (!this._pingConfig.ignoreDifferentAddressReply ||
+            isReplyFromTarget(line, this._response.numeric_host)) {
+            this._times.push(getFloatOrUnknown(match[1]));
+        }
     }
 
     // Change state if it see a '---'
@@ -57,6 +74,13 @@ MacParser.prototype._processBody = function (line) {
  * @param {string} line - A line from system ping
  */
 MacParser.prototype._processFooter = function (line) {
+    // The footer includes ignored replies, so its statistics are invalid when none were accepted.
+    if (this._pingConfig.ignoreDifferentAddressReply && this._times.length === 0) {
+        this._response.packetLoss = 100;
+        this._changeState(base.STATES.END);
+        return;
+    }
+
     var packetLoss = line.match(/ ([\d.]+(\.?[\d]*))%/);
     if (packetLoss) {
         this._response.packetLoss = getFloatOrUnknown(packetLoss[1]);

test/fixture/answer.json

@@ -107,6 +107,25 @@
     "packetLoss": "100.000",
     "stddev": "unknown"
   },
+  "linux_en_sample_reply_from_different_address": {
+    "inputHost": "whatever",
+    "host": "192.168.178.1",
+    "numeric_host": "192.168.178.1",
+    "alive": true,
+    "output": "PING 192.168.178.1 (192.168.178.1) 56(84) bytes of data.\n64 bytes from 192.168.178.8: icmp_seq=1 ttl=64 time=0.578 ms (DIFFERENT ADDRESS!)\n64 bytes from 192.168.178.8: icmp_seq=2 ttl=64 time=0.348 ms (DIFFERENT ADDRESS!)\n64 bytes from 192.168.178.8: icmp_seq=3 ttl=64 time=0.994 ms (DIFFERENT ADDRESS!)\n64 bytes from 192.168.178.8: icmp_seq=4 ttl=64 time=0.349 ms (DIFFERENT ADDRESS!)\n\n--- 192.168.178.1 ping statistics ---\n4 packets transmitted, 4 received, 0% packet loss, time 3004ms\nrtt min/avg/max/mdev = 0.348/0.567/0.994/0.263 ms",
+    "time": 0.578,
+    "times": [
+      0.578,
+      0.348,
+      0.994,
+      0.349
+    ],
+    "min": "0.348",
+    "max": "0.994",
+    "avg": "0.567",
+    "stddev": "0.263",
+    "packetLoss": "0.000"
+  },
   "linux_en_sample4": {
     "inputHost": "whatever",
     "host": "google.com",

test/fixture/ignore-address/reply_from_18.8.8.8.txt

@@ -0,0 +1,6 @@
+PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
+64 bytes from 18.8.8.8: icmp_seq=1 ttl=64 time=1.000 ms
+
+--- 8.8.8.8 ping statistics ---
+1 packets transmitted, 1 received, 0% packet loss, time 0ms
+rtt min/avg/max/mdev = 1.000/1.000/1.000/0.000 ms

test/fixture/ignore-address/reply_from_8.8.8.80.txt

@@ -0,0 +1,6 @@
+PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
+64 bytes from 8.8.8.80: icmp_seq=1 ttl=64 time=1.000 ms
+
+--- 8.8.8.8 ping statistics ---
+1 packets transmitted, 1 received, 0% packet loss, time 0ms
+rtt min/avg/max/mdev = 1.000/1.000/1.000/0.000 ms

test/fixture/linux/en/sample_reply_from_different_address.txt

@@ -0,0 +1,9 @@
+PING 192.168.178.1 (192.168.178.1) 56(84) bytes of data.
+64 bytes from 192.168.178.8: icmp_seq=1 ttl=64 time=0.578 ms (DIFFERENT ADDRESS!)
+64 bytes from 192.168.178.8: icmp_seq=2 ttl=64 time=0.348 ms (DIFFERENT ADDRESS!)
+64 bytes from 192.168.178.8: icmp_seq=3 ttl=64 time=0.994 ms (DIFFERENT ADDRESS!)
+64 bytes from 192.168.178.8: icmp_seq=4 ttl=64 time=0.349 ms (DIFFERENT ADDRESS!)
+
+--- 192.168.178.1 ping statistics ---
+4 packets transmitted, 4 received, 0% packet loss, time 3004ms
+rtt min/avg/max/mdev = 0.348/0.567/0.994/0.263 ms

test/test-ping.js

@@ -124,6 +124,102 @@ var createTestCase = function (platform, pingExecution) {
     });
 };
 
+describe('ping reply from a different address', function () {
+    describe('on linux platform', function() {
+        beforeEach(function() {
+            this.platformStub = sinon.stub(os, 'platform').callsFake(function() {
+                return 'linux';
+            });
+            const fixturePath = path.join(__dirname, 'fixture', 'linux', 'en', 'sample_reply_from_different_address.txt');
+            this.spawnStub = sinon.stub(cp, 'spawn').callsFake(mockOutSpawn(fixturePath));
+        });
+
+        afterEach(function() {
+            this.platformStub.restore();
+            this.spawnStub.restore();
+        });
+
+        it('host is not considered alive if ignoreDifferentAddressReply is true', async function() {
+            const res = await ping.promise
+                .probe('whatever', {
+                    ignoreDifferentAddressReply: true,
+                });
+            expect(res.alive).to.be.false;
+        });
+
+        it('host is considered alive if ignoreDifferentAddressReply is false', async function() {
+            const res = await ping.promise
+                .probe('whatever', {
+                    ignoreDifferentAddressReply: false,
+                });
+            expect(res.alive).to.be.true;
+        });
+
+        it('does not keep footer stats when every reply address is ignored', async function() {
+            const res = await ping.promise
+                .probe('whatever', {
+                    ignoreDifferentAddressReply: true,
+                });
+            expect(res.alive).to.be.false;
+            expect(res.packetLoss).to.equal('100.000');
+            expect(res.time).to.equal('unknown');
+            expect(res.times).to.deep.equal([]);
+            expect(res.min).to.equal('unknown');
+            expect(res.avg).to.equal('unknown');
+            expect(res.max).to.equal('unknown');
+            expect(res.stddev).to.equal('unknown');
+        });
+
+        ['sample1', 'sample2', 'v6_sample1', 'v6_sample2'].forEach(function (fixtureName) {
+            it(`keeps replies and stats from the target address using ${fixtureName}`, async function () {
+                const fixturePath = path.join(__dirname, 'fixture', 'linux', 'en', `${fixtureName}.txt`);
+                this.spawnStub.callsFake(mockOutSpawn(fixturePath));
+
+                const res = await ping.promise.probe('whatever', {
+                    ignoreDifferentAddressReply: true,
+                    v6: fixtureName.startsWith('v6'),
+                });
+                const expected = ANSWER[`linux_en_${fixtureName}`];
+                expect({...res, output: res.output.trim()}).to.deep.equal({
+                    ...expected,
+                    output: expected.output.trim(),
+                });
+            });
+        });
+    });
+
+    describe('on linux platform when the reply IP only contains the target as a substring', function() {
+        afterEach(function() {
+            this.platformStub.restore();
+            this.spawnStub.restore();
+        });
+
+        var stubFixture = function (fixtureName) {
+            this.platformStub = sinon.stub(os, 'platform').callsFake(function() {
+                return 'linux';
+            });
+            const fixturePath = path.join(__dirname, 'fixture', 'ignore-address', fixtureName);
+            this.spawnStub = sinon.stub(cp, 'spawn').callsFake(mockOutSpawn(fixturePath));
+        };
+
+        it('does not treat 18.8.8.8 as a reply for 8.8.8.8', async function() {
+            stubFixture.call(this, 'reply_from_18.8.8.8.txt');
+            const res = await ping.promise.probe('8.8.8.8', {
+                ignoreDifferentAddressReply: true,
+            });
+            expect(res.alive).to.be.false;
+        });
+
+        it('does not treat 8.8.8.80 as a reply for 8.8.8.8', async function() {
+            stubFixture.call(this, 'reply_from_8.8.8.80.txt');
+            const res = await ping.promise.probe('8.8.8.8', {
+                ignoreDifferentAddressReply: true,
+            });
+            expect(res.alive).to.be.false;
+        });
+    });
+});
+
 describe('ping timeout and deadline options', function () {
     describe('on linux platform', function () {
         beforeEach(function () {

types/index.d.ts

@@ -38,9 +38,19 @@ export type PingConfig = {
     /**
      * - Specifies the number of data bytes to be sent
      *  Default: Linux / MAC: 56 Bytes,
-     *           Window: 32 Bytes
+     *           Windows: 32 Bytes
      */
     packetSize?: number;
+    /**
+     * - Enables ignoring replies from different addresses
+     * Default: Linux: false
+     * Other platforms do not need this option.
+     * If set to true and a reply is received from an address which does not match the parsed
+     * `PingResponse.numeric_host`, the ping response is ignored.
+     * This helps against detecting a host as alive by accident when another host replies to the ping.
+     * This workaround addresses an imperfection in the ping implementation within the package iputils.*
+     */
+    ignoreDifferentAddressReply?: boolean;
     /**
      * - Optional options does not provided
      */

types/index.test-d.ts

@@ -10,6 +10,7 @@ import { PingConfig } from ".";
 const pingConfig: PingConfig = {
     numeric: true,
     timeout: 10,
+    ignoreDifferentAddressReply: true,
 };
 
 expectType<Promise<PingResponse>>(promiseProbe('localhost', pingConfig));