From 2b4ff57bd4b9dfe75b51bf7729594b8dcb1d1038 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Mon, 4 Sep 2017 12:27:53 +0200
Subject: [PATCH] [enh] #1084 WS2P: allow requests only when strict
 bidirectionnal connection is established

---
 app/lib/ws2p/WS2PConnection.ts      | 44 +++++++++++++++-----
 test/integration/ws2p_connection.ts | 62 +++++++++++++++++++++--------
 2 files changed, 81 insertions(+), 25 deletions(-)

diff --git a/app/lib/ws2p/WS2PConnection.ts b/app/lib/ws2p/WS2PConnection.ts
index 04a5a7663..fcce6a1f2 100644
--- a/app/lib/ws2p/WS2PConnection.ts
+++ b/app/lib/ws2p/WS2PConnection.ts
@@ -24,19 +24,20 @@ enum WS2P_ERR {
 
 export interface WS2PAuth {
   isAuthorizedPubkey(pub:string): Promise<boolean>
+  authenticationIsDone(): Promise<void>
 }
 
 export interface WS2PRemoteAuth extends WS2PAuth {
-  sendACK(ws:any): Promise<void>
   registerCONNECT(challenge:string, sig: string, pub: string): Promise<boolean>
+  sendACK(ws:any): Promise<void>
   registerOK(sig: string): Promise<boolean>
-  isAuthenticated(): boolean
   isAuthenticatedByRemote(): boolean
 }
 
 export interface WS2PLocalAuth extends WS2PAuth {
   sendCONNECT(ws:any): Promise<void>
   registerACK(sig: string, pub: string): Promise<boolean>
+  sendOK(ws:any): Promise<void>
   isRemoteAuthenticated(): boolean
 }
 
@@ -46,7 +47,6 @@ export interface WS2PLocalAuth extends WS2PAuth {
 export class WS2PPubkeyRemoteAuth implements WS2PRemoteAuth {
 
   protected challenge:string
-  protected authenticated = false
   protected authenticatedByRemote = false
   protected remotePub = ""
   protected serverAuth:Promise<void>
@@ -88,13 +88,14 @@ export class WS2PPubkeyRemoteAuth implements WS2PRemoteAuth {
   async registerOK(sig: string): Promise<boolean> {
     const challengeMessage = `WS2P:OK:${this.remotePub}:${this.challenge}`
     this.authenticatedByRemote = verify(challengeMessage, sig, this.remotePub)
+    if (!this.authenticatedByRemote) {
+      this.serverAuthReject("Wrong signature from remote OK")
+    } else {
+      this.serverAuthResolve()
+    }
     return this.authenticatedByRemote
   }
 
-  isAuthenticated(): boolean {
-    return this.authenticated
-  }
-
   isAuthenticatedByRemote(): boolean {
     return this.authenticatedByRemote
   }
@@ -102,6 +103,10 @@ export class WS2PPubkeyRemoteAuth implements WS2PRemoteAuth {
   async isAuthorizedPubkey(pub: string): Promise<boolean> {
     return true
   }
+
+  authenticationIsDone(): Promise<void> {
+    return this.serverAuth
+  }
 }
 
 /**
@@ -150,10 +155,24 @@ export class WS2PPubkeyLocalAuth implements WS2PLocalAuth {
     return this.authenticated
   }
 
+  async sendOK(ws:any): Promise<void> {
+    const challengeMessage = `WS2P:OK:${this.pair.pub}:${this.challenge}`
+    const sig = this.pair.signSync(challengeMessage)
+    await ws.send(JSON.stringify({
+      auth: 'OK',
+      sig
+    }))
+    return this.serverAuth
+  }
+
   isRemoteAuthenticated(): boolean {
     return this.authenticated
   }
 
+  authenticationIsDone(): Promise<void> {
+    return this.serverAuth
+  }
+
   async isAuthorizedPubkey(pub: string): Promise<boolean> {
     return true
   }
@@ -271,6 +290,10 @@ export class WS2PConnection {
             await this.onWsOpened
             try {
               await this.localAuth.sendCONNECT(this.ws)
+              await Promise.all([
+                this.localAuth.authenticationIsDone(),
+                this.remoteAuth.authenticationIsDone()
+              ])
               resolve()
             } catch (e) {
               reject(e)
@@ -296,7 +319,7 @@ export class WS2PConnection {
               if (data.auth && typeof data.auth === "string") {
 
                 if (data.auth === "CONNECT") {
-                  if (this.remoteAuth.isAuthenticated()) {
+                  if (this.remoteAuth.isAuthenticatedByRemote()) {
                     return this.errorDetected(WS2P_ERR.ALREADY_AUTHENTICATED_BY_REMOTE)
                   }
                   else if (
@@ -327,7 +350,10 @@ export class WS2PConnection {
                       await this.errorDetected(WS2P_ERR.INCORRECT_PUBKEY_FOR_REMOTE)
                     } else {
                       try {
-                        await this.localAuth.registerACK(data.sig, data.pub)
+                        const valid = await this.localAuth.registerACK(data.sig, data.pub)
+                        if (valid) {
+                          await this.localAuth.sendOK(this.ws)
+                        }
                       } catch (e) {
                         await this.errorDetected(WS2P_ERR.INCORRECT_ACK_SIGNATURE_FROM_REMOTE)
                       }
diff --git a/test/integration/ws2p_connection.ts b/test/integration/ws2p_connection.ts
index 86d671465..9075ac71d 100644
--- a/test/integration/ws2p_connection.ts
+++ b/test/integration/ws2p_connection.ts
@@ -114,7 +114,7 @@ describe('WS2P', () => {
 
       it('should accept the connection if the server answers with a good signature', async () => {
         const keypair = new Key('HgTTJLAQ5sqfknMq7yLPZbehtuLSsKj9CxWN7k8QvYJd', '51w4fEShBk1jCMauWu4mLpmDVfHksKmWcygpxriqCEZizbtERA6de4STKRkQBpxmMUwsKXRjSzuQ8ECwmqN1u2DP')
-        const ws2p = WS2PConnection.newConnectionToAddress('localhost:20903', () => {}, new WS2PPubkeyLocalAuth(keypair), new WS2PPubkeyRemoteAuth(keypair), {
+        const ws2p = WS2PConnection.newConnectionToAddress('localhost:20903', () => {}, new WS2PPubkeyLocalAuth(keypair), new WS2PNoRemoteAuth(), {
           connectionTimeout: 1000,
           requestTimeout: 1000
         })
@@ -201,11 +201,13 @@ describe('WS2P', () => {
       let resolveS3:any
       let resolveS4:any
       let resolveS5:any
+      let resolveS6:any
       let s1p:Promise<WS2PConnection> = new Promise(res => resolveS1 = res)
       let s2p:Promise<WS2PConnection> = new Promise(res => resolveS2 = res)
       let s3p:Promise<WS2PConnection> = new Promise(res => resolveS3 = res)
       let s4p:Promise<WS2PConnection> = new Promise(res => resolveS4 = res)
       let s5p:Promise<WS2PConnection> = new Promise(res => resolveS5 = res)
+      let s6p:Promise<WS2PConnection> = new Promise(res => resolveS6 = res)
 
       before(async () => {
         let i = 1
@@ -215,8 +217,8 @@ describe('WS2P', () => {
           switch (i) {
             case 1:
               resolveS1(WS2PConnection.newConnectionFromWebSocketServer(ws, () => {}, new WS2PPubkeyLocalAuth(serverKeypair), new WS2PPubkeyRemoteAuth(serverKeypair), {
-                connectionTimeout: 1000,
-                requestTimeout: 1000
+                connectionTimeout: 100,
+                requestTimeout: 100
               }));
               (await s1p).connect().catch((e:any) => console.error('WS2P: newConnectionFromWebSocketServer connection error'))
               break
@@ -229,24 +231,24 @@ describe('WS2P', () => {
             }
 
               resolveS2(WS2PConnection.newConnectionFromWebSocketServer(ws, () => {}, new WS2PPubkeyLocalAuth(serverKeypair), new WS2PPubkeyNotAnsweringWithOKAuth(serverKeypair), {
-                connectionTimeout: 1000,
-                requestTimeout: 1000
+                connectionTimeout: 100,
+                requestTimeout: 100
               }));
               (await s2p).connect().catch((e:any) => console.error('WS2P: newConnectionFromWebSocketServer connection error'))
               break
             case 3:
 
               resolveS3(WS2PConnection.newConnectionFromWebSocketServer(ws, () => {}, new WS2PPubkeyLocalAuth(serverKeypair), new WS2PPubkeyRemoteAuth(serverKeypair), {
-                connectionTimeout: 1000,
-                requestTimeout: 1000
+                connectionTimeout: 100,
+                requestTimeout: 100
               }));
               (await s3p).connect().catch((e:any) => console.error('WS2P: newConnectionFromWebSocketServer connection error'))
               break
             case 4:
 
               resolveS4(WS2PConnection.newConnectionFromWebSocketServer(ws, () => {}, new WS2PPubkeyLocalAuth(serverKeypair), new WS2PPubkeyRemoteAuth(serverKeypair), {
-                connectionTimeout: 1000,
-                requestTimeout: 1000
+                connectionTimeout: 100,
+                requestTimeout: 100
               }));
               (await s4p).connect().catch((e:any) => console.error('WS2P: newConnectionFromWebSocketServer connection error'))
               break
@@ -255,6 +257,15 @@ describe('WS2P', () => {
               resolveS5(WS2PConnection.newConnectionFromWebSocketServer(ws, () => {}, new WS2PPubkeyLocalAuth(serverKeypair), new WS2PPubkeyRemoteAuth(serverKeypair)));
               (await s5p).connect().catch((e:any) => console.error('WS2P: newConnectionFromWebSocketServer connection error'))
               break
+
+            case 6:
+
+              resolveS6(WS2PConnection.newConnectionFromWebSocketServer(ws, () => {}, new WS2PPubkeyLocalAuth(serverKeypair), new WS2PPubkeyRemoteAuth(serverKeypair), {
+                connectionTimeout: 100,
+                requestTimeout: 100
+              }));
+              (await s6p).connect().catch((e:any) => console.error('WS2P: newConnectionFromWebSocketServer connection error'))
+              break
           }
           i++
         })
@@ -264,7 +275,7 @@ describe('WS2P', () => {
         wss.close(done)
       })
 
-      it('should refuse the connection if the client does not sendCONNECT', async () => {
+      it('should refuse the connection if the client does not send ACK', async () => {
 
         class WS2PPubkeyNotAnsweringWithACKAuth extends WS2PPubkeyRemoteAuth {
           async sendACK(ws: any): Promise<void> {
@@ -274,7 +285,7 @@ describe('WS2P', () => {
 
         const keypair = new Key('HgTTJLAQ5sqfknMq7yLPZbehtuLSsKj9CxWN7k8QvYJd', '51w4fEShBk1jCMauWu4mLpmDVfHksKmWcygpxriqCEZizbtERA6de4STKRkQBpxmMUwsKXRjSzuQ8ECwmqN1u2DP')
         const c1 = WS2PConnection.newConnectionToAddress('localhost:20903', () => {}, new WS2PPubkeyLocalAuth(keypair), new WS2PPubkeyNotAnsweringWithACKAuth(keypair))
-        await c1.connect()
+        c1.connect()
         const s1 = await s1p
         await assertThrows(s1.request({ message: 'something' }), "WS2P connection timeout")
       })
@@ -302,7 +313,7 @@ describe('WS2P', () => {
 
         const keypair = new Key('HgTTJLAQ5sqfknMq7yLPZbehtuLSsKj9CxWN7k8QvYJd', '51w4fEShBk1jCMauWu4mLpmDVfHksKmWcygpxriqCEZizbtERA6de4STKRkQBpxmMUwsKXRjSzuQ8ECwmqN1u2DP')
         const c3 = WS2PConnection.newConnectionToAddress('localhost:20903', () => {}, new WS2PPubkeyLocalAuth(keypair), new WS2PPubkeyAnsweringWithWrongSigForACK(keypair))
-        await c3.connect()
+        c3.connect()
         const s3 = await s3p
         await assertThrows(s3.request({ message: 'something' }), "Wrong signature from server ACK")
       })
@@ -340,6 +351,20 @@ describe('WS2P', () => {
         const s5 = await s5p
         assert.deepEqual({ answer: 'success!' }, await s5.request({ message: 'connection?'} ))
       })
+
+      it('should refuse the connection if the client does not send OK', async () => {
+
+        class WS2PPubkeyNotAnsweringWithOKAuth extends WS2PPubkeyLocalAuth {
+          async sendOK(ws: any): Promise<void> {
+          }
+        }
+
+        const keypair = new Key('HgTTJLAQ5sqfknMq7yLPZbehtuLSsKj9CxWN7k8QvYJd', '51w4fEShBk1jCMauWu4mLpmDVfHksKmWcygpxriqCEZizbtERA6de4STKRkQBpxmMUwsKXRjSzuQ8ECwmqN1u2DP')
+        const c6 = WS2PConnection.newConnectionToAddress('localhost:20903', () => {}, new WS2PPubkeyNotAnsweringWithOKAuth(keypair), new WS2PPubkeyRemoteAuth(keypair))
+        c6.connect()
+        const s6 = await s6p
+        await assertThrows(s6.request({ message: 'something' }), "WS2P connection timeout")
+      })
     })
   })
 })
@@ -364,6 +389,12 @@ class WS2PNoLocalAuth implements WS2PLocalAuth {
   async isAuthorizedPubkey(pub: string): Promise<boolean> {
     return true
   }
+
+  async sendOK(ws: any): Promise<void> {
+  }
+
+  async authenticationIsDone(): Promise<void> {
+  }
 }
 
 class WS2PNoRemoteAuth implements WS2PRemoteAuth {
@@ -379,10 +410,6 @@ class WS2PNoRemoteAuth implements WS2PRemoteAuth {
     return true
   }
 
-  isAuthenticated(): boolean {
-    return true
-  }
-
   isAuthenticatedByRemote(): boolean {
     return true
   }
@@ -390,4 +417,7 @@ class WS2PNoRemoteAuth implements WS2PRemoteAuth {
   async isAuthorizedPubkey(pub: string): Promise<boolean> {
     return true
   }
+
+  async authenticationIsDone(): Promise<void> {
+  }
 }
-- 
GitLab