fbshipit-source-id: b14273e883aba6de7b817801a1b04e54a29a6366

src/server.js

@@ -11,6 +11,7 @@ import type {SonarPlugin} from './plugin.js';
 import plugins from './plugins/index.js';
 import CertificateProvider from './utils/CertificateProvider';
 import type {SecureServerConfig} from './utils/CertificateProvider';
+import type Logger from './fb-stubs/Logger';
 
 import {RSocketServer, ReactiveSocket, PartialResponder} from 'rsocket-core';
 import RSocketTCPServer from 'rsocket-tcp-server';
@@ -315,6 +316,7 @@ export class Server extends EventEmitter {
   secureServer: RSocketServer;
   insecureServer: RSocketServer;
   certificateProvider: CertificateProvider;
+  connectionTracker: ConnectionTracker;
   app: App;
 
   constructor(app: App) {
@@ -322,6 +324,7 @@ export class Server extends EventEmitter {
     this.app = app;
     this.connections = new Map();
     this.certificateProvider = new CertificateProvider(this, app.logger);
+    this.connectionTracker = new ConnectionTracker(app.logger);
     this.init();
   }
 
@@ -389,7 +392,10 @@ export class Server extends EventEmitter {
   _trustedRequestHandler = (conn: RSocket, connectRequest: {data: string}) => {
     const server = this;
 
-    const client = this.addConnection(conn, connectRequest.data);
+    const clientData: ClientQuery = JSON.parse(connectRequest.data);
+    this.connectionTracker.logConnectionAttempt(clientData);
+
+    const client = this.addConnection(conn, clientData);
 
     conn.connectionStatus().subscribe({
       onNext(payload) {
@@ -413,7 +419,8 @@ export class Server extends EventEmitter {
     conn: RSocket,
     connectRequest: {data: string},
   ) => {
-    const connectionParameters = JSON.parse(connectRequest.data);
+    const clientData = JSON.parse(connectRequest.data);
+    this.connectionTracker.logConnectionAttempt(clientData);
 
     return {
       fireAndForget: (payload: {data: string}) => {
@@ -442,7 +449,7 @@ export class Server extends EventEmitter {
           const {csr, destination} = json;
           this.certificateProvider.processCertificateSigningRequest(
             csr,
-            connectionParameters.os,
+            clientData.os,
             destination,
           );
         }
@@ -459,13 +466,12 @@ export class Server extends EventEmitter {
     return null;
   }
 
-  addConnection(conn: ReactiveSocket, queryString: string): Client {
-    const query = JSON.parse(queryString);
+  addConnection(conn: ReactiveSocket, query: ClientQuery): Client {
     invariant(query, 'expected query');
 
-    this.app.logger.warn(`Device connected: ${queryString}`, 'connection');
-
     const id = `${query.app}-${query.os}-${query.device}`;
+    this.app.logger.warn(`Device connected: ${id}`, 'connection');
+
     const client = new Client(this.app, id, query, conn);
 
     const info = {
@@ -518,3 +524,34 @@ export class Server extends EventEmitter {
     }
   }
 }
+
+class ConnectionTracker {
+  timeWindowMillis = 20 * 1000;
+  connectionProblemThreshold = 4;
+
+  // "${device}.${app}" -> [timestamp1, timestamp2...]
+  connectionAttempts: Map<string, Array<number>> = new Map();
+  logger: Logger;
+
+  constructor(logger: Logger) {
+    this.logger = logger;
+  }
+
+  logConnectionAttempt(client: ClientQuery) {
+    const key = `${client.os}-${client.device}-${client.app}`;
+    const time = Date.now();
+    var entry = this.connectionAttempts.get(key) || [];
+    entry.push(time);
+    entry = entry.filter(t => t >= time - this.timeWindowMillis);
+
+    this.connectionAttempts.set(key, entry);
+    if (entry.length >= this.connectionProblemThreshold) {
+      this.logger.error(
+        `Connection loop detected with ${key}. Connected ${
+          entry.length
+        } times in ${(time - entry[0]) / 1000}s.`,
+        'ConnectionTracker',
+      );
+    }
+  }
+}

src/utils/CertificateProvider.js

@@ -33,6 +33,7 @@ const minCertExpiryWindowSeconds = 24 * 60 * 60;
 const appNotDebuggableRegex = /debuggable/;
 const allowedAppNameRegex = /^[a-zA-Z0-9.\-]+$/;
 const allowedAppDirectoryRegex = /^\/[ a-zA-Z0-9.\-\/]+$/;
+const logTag = 'CertificateProvider';
 
 export type SecureServerConfig = {|
   key: Buffer,
@@ -124,7 +125,7 @@ export default class CertificateProvider {
   }
 
   generateClientCertificate(csr: string): Promise<string> {
-    this.logger.warn('Creating new client cert', 'CertificateProvider');
+    this.logger.warn('Creating new client cert', logTag);
     const csrFile = this.writeToTempFile(csr);
     // Create a certificate for the client, using the details in the CSR.
     return openssl('x509', {
@@ -145,40 +146,66 @@ export default class CertificateProvider {
     contents: string,
     csr: string,
     os: string,
-  ) {
+  ): Promise<void> {
     if (os === 'Android') {
-      this.extractAppNameFromCSR(csr).then(app => {
-        const client = adb.createClient();
-        client.listDevices().then((devices: Array<{id: string}>) => {
-          devices.forEach(d =>
-            // To find out which device requested the cert, search them
-            // all for a matching csr file.
-            // It's not important to keep these secret from other apps.
-            // Just need to make sure each app can find it's own one.
-            this.androidDeviceHasMatchingCSR(destination, d.id, app, csr)
-              .catch(e =>
-                this.logger.error(
-                  `Unable to check for matching CSR in ${d.id}:${app}`,
-                  'CertificateProvider',
-                ),
-              )
-              .then(isMatch => {
-                if (isMatch) {
-                  this.pushFileToAndroidDevice(
-                    d.id,
-                    app,
-                    destination + filename,
-                    contents,
-                  );
-                }
-              }),
-          );
-        });
-      });
+      const appNamePromise = this.extractAppNameFromCSR(csr);
+      const deviceIdPromise = appNamePromise.then(app =>
+        this.getTargetDeviceId(app, destination, csr),
+      );
+      return Promise.all([deviceIdPromise, appNamePromise]).then(
+        ([deviceId, appName]) =>
+          this.pushFileToAndroidDevice(
+            deviceId,
+            appName,
+            destination + filename,
+            contents,
+          ),
+      );
     }
     if (os === 'iOS') {
       fs.writeFileSync(destination + filename, contents);
+      return Promise.resolve();
     }
+    return Promise.reject(new Error(`Unsupported device os: ${os}`));
+  }
+
+  getTargetDeviceId(
+    appName: string,
+    deviceCsrFilePath: string,
+    csr: string,
+  ): Promise<string> {
+    const client = adb.createClient();
+    return client.listDevices().then((devices: Array<{id: string}>) => {
+      const deviceMatchList = devices.map(device =>
+        // To find out which device requested the cert, search them
+        // all for a matching csr file.
+        // It's not important to keep these secret from other apps.
+        // Just need to make sure each app can find it's own one.
+        this.androidDeviceHasMatchingCSR(
+          deviceCsrFilePath,
+          device.id,
+          appName,
+          csr,
+        )
+          .then(isMatch => {
+            return {id: device.id, isMatch};
+          })
+          .catch(e => {
+            this.logger.error(
+              `Unable to check for matching CSR in ${device.id}:${appName}`,
+              logTag,
+            );
+            return {id: device.id, isMatch: false};
+          }),
+      );
+      return Promise.all(deviceMatchList).then(devices => {
+        const matchingIds = devices.filter(m => m.isMatch).map(m => m.id);
+        if (matchingIds.length == 0) {
+          throw new Error(`No matching device found for app: ${appName}`);
+        }
+        return matchingIds[0];
+      });
+    });
   }
 
   androidDeviceHasMatchingCSR(
@@ -191,14 +218,19 @@ export default class CertificateProvider {
       deviceId,
       processName,
       `cat ${directory + csrFileName}`,
-    ).then(deviceCsr => {
-      return (
-        deviceCsr
-          .toString()
-          .replace(/\r/g, '')
-          .trim() === csr.replace(/\r/g, '').trim()
-      );
-    });
+    )
+      .then(deviceCsr => {
+        return (
+          deviceCsr
+            .toString()
+            .replace(/\r/g, '')
+            .trim() === csr.replace(/\r/g, '').trim()
+        );
+      })
+      .catch(err => {
+        this.logger.error(err, logTag);
+        return false;
+      });
   }
 
   pushFileToAndroidDevice(
@@ -207,10 +239,7 @@ export default class CertificateProvider {
     filename: string,
     contents: string,
   ): Promise<void> {
-    this.logger.warn(
-      `Deploying sonar certificate to ${deviceId}:${app}`,
-      'CertificateProvider',
-    );
+    this.logger.warn(`Deploying ${filename} to ${deviceId}:${app}`, logTag);
     return this.executeCommandOnAndroid(
       deviceId,
       app,
@@ -245,6 +274,13 @@ export default class CertificateProvider {
           throw e;
         }
         return output;
+      })
+      .catch(err => {
+        this.logger.error(
+          `Error executing command on android device ${deviceId}:${user}. Command: ${command}`,
+          logTag,
+        );
+        this.logger.error(err, logTag);
       });
   }
 
@@ -306,10 +342,7 @@ export default class CertificateProvider {
     })
       .then(output => undefined)
       .catch(e => {
-        this.logger.warn(
-          `Certificate will expire soon: ${filename}`,
-          'CertificateProvider',
-        );
+        this.logger.warn(`Certificate will expire soon: ${filename}`, logTag);
         throw e;
       });
   }
@@ -331,7 +364,7 @@ export default class CertificateProvider {
     if (!fs.existsSync(getFilePath(''))) {
       fs.mkdirSync(getFilePath(''));
     }
-    this.logger.info('Generating new CA', 'CertificateProvider');
+    this.logger.info('Generating new CA', logTag);
     return openssl('genrsa', {out: caKey, '2048': false})
       .then(_ =>
         openssl('req', {
@@ -364,7 +397,7 @@ export default class CertificateProvider {
   generateServerCertificate(): Promise<void> {
     return this.ensureCertificateAuthorityExists()
       .then(_ => {
-        this.logger.warn('Creating new server cert', 'CertificateProvider');
+        this.logger.warn('Creating new server cert', logTag);
       })
       .then(_ => openssl('genrsa', {out: serverKey, '2048': false}))
       .then(_ =>