Fix and improve ICE candidate censoring

This adds more fine-granular sanitising of the IP address:

- for IPv4, we censor the last two bytes,
- for IPv6, we censor all bytes but the first two bytes,
- for mDNS concealed candidates, there's no need to censor anything.

src/helpers/confidential.ts

@@ -15,6 +15,8 @@
  * along with Threema Web. If not, see <http://www.gnu.org/licenses/>.
  */
 
+import * as SDPUtils from 'sdp';
+
 /**
  * Recursively sanitises `value` in the following way:
  *
@@ -157,3 +159,56 @@ export class ConfidentialWireMessage extends BaseConfidential<threema.WireMessag
         return message;
     }
 }
+
+/**
+ * Wraps an ICE candidate's SDP attribute.
+ *
+ * When sanitising, this returns all attributes unchanged apart from the
+ * candidate's IP address which will be partially sanitised.
+ */
+export class ConfidentialIceCandidate extends BaseConfidential<string, string> {
+    public readonly uncensored: string;
+
+    constructor(candidateSdp: string) {
+        super();
+        this.uncensored = candidateSdp;
+    }
+
+    public censored(): string {
+        try {
+            // Parse into candidate object
+            const candidate = SDPUtils.parseCandidate(this.uncensored);
+
+            // Sanitise IP and port
+            if (candidate.type !== 'relay') {
+                candidate.address = candidate.ip = ConfidentialIceCandidate.censorIp(candidate.address);
+            }
+            if (candidate.relatedAddress !== undefined) {
+                candidate.relatedAddress = ConfidentialIceCandidate.censorIp(candidate.relatedAddress);
+            }
+
+            // Return as SDP
+            return SDPUtils.writeCandidate(candidate);
+        } catch (error) {
+            return this.uncensored;
+        }
+    }
+
+    private static censorIp(ip: string): string {
+        // Handle UUID (mDNS)
+        if (ip.includes('-')) {
+            return ip;
+        }
+
+        // Handle IPv4 address
+        const ipv4 = ip.split('.');
+        if (ipv4.length > 1) {
+            return `${ipv4.slice(0, 2).join('.')}.*.*`;
+        }
+
+        // Handle IPv6 address (catch-all)
+        const ipv6 = ip.split(':');
+        const head = ipv6.shift();
+        return `${head}:${ipv6.map((item) => item.length > 0 ? '*' : item).join(':')}`;
+    }
+}

src/services/peerconnection.ts

@@ -15,10 +15,10 @@
  * along with Threema Web. If not, see <http://www.gnu.org/licenses/>.
  */
 
-import * as SDPUtils from 'sdp';
-
 import TaskConnectionState = threema.TaskConnectionState;
 import {Logger} from 'ts-log';
+
+import {ConfidentialIceCandidate} from '../helpers/confidential';
 import {LogService} from './log';
 
 /**
@@ -39,13 +39,8 @@ export class PeerConnectionHelper {
     public connectionState: TaskConnectionState = TaskConnectionState.New;
     public onConnectionStateChange: (state: TaskConnectionState) => void = null;
 
-    // Debugging
-    private censorCandidates: boolean;
-
     constructor($q: ng.IQService, $timeout: ng.ITimeoutService, $rootScope: ng.IRootScopeService,
-                logService: LogService, webrtcTask: saltyrtc.tasks.webrtc.WebRTCTask,
-                iceServers: RTCIceServer[],
-                censorCandidates: boolean = true) {
+                logService: LogService, webrtcTask: saltyrtc.tasks.webrtc.WebRTCTask, iceServers: RTCIceServer[]) {
         this.log = logService.getLogger('PeerConnection', 'color: #fff; background-color: #3333ff');
         this.log.info('Initialize WebRTC PeerConnection');
         this.log.debug('ICE servers used:', [].concat(...iceServers.map((c) => c.urls)));
@@ -55,8 +50,6 @@ export class PeerConnectionHelper {
 
         this.webrtcTask = webrtcTask;
 
-        this.censorCandidates = censorCandidates;
-
         // Set up peer connection
         this.pc = new RTCPeerConnection({iceServers: iceServers});
         this.pc.onnegotiationneeded = (e: Event) => {
@@ -97,8 +90,7 @@ export class PeerConnectionHelper {
         this.log.debug('Setting up ICE candidate handling');
         this.pc.onicecandidate = (e: RTCPeerConnectionIceEvent) => {
             if (e.candidate) {
-                this.log.debug('Gathered local ICE candidate:',
-                    this.censorCandidate(e.candidate.candidate));
+                this.log.debug('Gathered local ICE candidate:', new ConfidentialIceCandidate(e.candidate.candidate));
                 this.webrtcTask.sendCandidate({
                     candidate: e.candidate.candidate,
                     sdpMid: e.candidate.sdpMid,
@@ -143,7 +135,7 @@ export class PeerConnectionHelper {
             for (const candidateInit of e.data) {
                 if (candidateInit) {
                     this.log.debug('Adding remote ICE candidate:',
-                        this.censorCandidate(candidateInit.candidate));
+                        new ConfidentialIceCandidate(candidateInit.candidate));
                 } else {
                     this.log.debug('No more remote ICE candidates');
                 }
@@ -208,22 +200,4 @@ export class PeerConnectionHelper {
         this.pc.ondatachannel = null;
         this.pc.close();
     }
-
-    /**
-     * Censor an ICE candidate's address and port (unless censoring is disabled).
-     *
-     * Return the censored ICE candidate.
-     */
-    private censorCandidate(candidateInit: string): string {
-        const candidate = SDPUtils.parseCandidate(candidateInit);
-        if (this.censorCandidates) {
-            if (candidate.type !== 'relay') {
-                candidate.ip = '***';
-                candidate.port = 1;
-            }
-            candidate.relatedAddress = '***';
-            candidate.relatedPort = 2;
-        }
-        return SDPUtils.writeCandidate(candidate);
-    }
 }

src/services/webclient.ts

@@ -870,8 +870,7 @@ export class WebClientService {
                 this.$q, this.$timeout, this.$rootScope,
                 this.logService,
                 this.webrtcTask,
-                this.config.ICE_SERVERS,
-                !this.config.ARP_LOG_TRACE);
+                this.config.ICE_SERVERS);
 
             // On state changes in the PeerConnectionHelper class, let state service know about it
             this.pcHelper.onConnectionStateChange = (state: threema.TaskConnectionState) => {

tests/ts/confidential_helpers.ts

@@ -20,6 +20,7 @@ import {
     censor,
     BaseConfidential,
     ConfidentialArray,
+    ConfidentialIceCandidate,
     ConfidentialObjectValues,
     ConfidentialWireMessage
 } from '../../src/helpers/confidential';
@@ -240,4 +241,63 @@ describe('Confidential Helpers', () => {
             });
         });
     });
+
+    describe('ConfidentialIceCandidate', () => {
+        it('subclass of BaseConfidential', () => {
+            expect(ConfidentialIceCandidate.prototype instanceof BaseConfidential).toBeTruthy();
+        });
+
+        it('returns underlying ICE candidate directly when unveiling', () => {
+            const input = 'cannot be bothered to use valid SDP here';
+            const confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.uncensored).toBe(input);
+        });
+
+        it('returns underlying ICE candidate directly if it cannot be parsed', () => {
+            const input = 'certainly invalid';
+            const confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(input);
+        });
+
+        it('does not censor mDNS concealed candidates', () => {
+            const input = 'candidate:1 1 UDP 1234 aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee.local 1337 typ host';
+            const confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(input);
+        });
+
+        it('censors host candidates', () => {
+            // IPv4
+            let input = 'candidate:1 1 UDP 1234 192.168.0.42 1337 typ host';
+            let expected = 'candidate:1 1 UDP 1234 192.168.*.* 1337 typ host';
+            let confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(expected);
+
+            // IPv6
+            input = 'candidate:1 1 UDP 1234 fe80::1 1337 typ host';
+            expected = 'candidate:1 1 UDP 1234 fe80::* 1337 typ host';
+            confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(expected);
+        });
+
+        it('censors srflx candidates', () => {
+            const input = 'candidate:1 1 UDP 1234 1.2.3.4 42 typ srflx raddr 192.168.0.42 rport 1337';
+            const expected = 'candidate:1 1 UDP 1234 1.2.*.* 42 typ srflx raddr 192.168.*.* rport 1337';
+            const confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(expected);
+        });
+
+        it('censors relay candidates', () => {
+            // IPv4
+            let input = 'candidate:1 1 UDP 1234 1.2.3.4 42 typ relay raddr 192.168.0.42 rport 1337';
+            let expected = 'candidate:1 1 UDP 1234 1.2.3.4 42 typ relay raddr 192.168.*.* rport 1337';
+            let confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(expected);
+
+            // IPv6
+            input = 'candidate:1 1 UDP 1234 2a02:1:2::3 42 typ relay raddr 2a02:dead:beef::1 rport 1337';
+            expected = 'candidate:1 1 UDP 1234 2a02:1:2::3 42 typ relay raddr 2a02:*:*::* rport 1337';
+            confidential = new ConfidentialIceCandidate(input);
+            expect(confidential.censored()).toBe(expected);
+        });
+    });
 });