From f08a54ed1e9b157c10ac05115ed69073305e3021 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 5 Dec 2018 18:00:09 -0700 Subject: [PATCH 1/2] Don't consider ACL'd servers as permalink candidates and fix the bug where it was picking 4 servers instead of 3. This was due to `<=` instead of `<` in the `MAX_SERVER_CANDIDATES` loop. Added tests for all the things. Fixes https://github.com/vector-im/riot-web/issues/7752 Fixes https://github.com/vector-im/riot-web/issues/7682 --- package.json | 1 + src/matrix-to.js | 61 +++++++++++++- test/matrix-to-test.js | 186 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 232 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 67d1f3ba1e..d444c15eab 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "gfm.css": "^1.1.1", "glob": "^5.0.14", "highlight.js": "^9.13.0", + "is-ip": "^2.0.0", "isomorphic-fetch": "^2.2.1", "linkifyjs": "^2.1.6", "lodash": "^4.13.1", diff --git a/src/matrix-to.js b/src/matrix-to.js index b5827f671a..fb2c8096d7 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -15,6 +15,8 @@ limitations under the License. */ import MatrixClientPeg from "./MatrixClientPeg"; +import isIp from "is-ip"; +import utils from 'matrix-js-sdk/lib/utils'; export const host = "matrix.to"; export const baseUrl = `https://${host}`; @@ -90,7 +92,9 @@ export function pickServerCandidates(roomId) { // Rationale for popular servers: It's hard to get rid of people when // they keep flocking in from a particular server. Sure, the server could // be ACL'd in the future or for some reason be evicted from the room - // however an event like that is unlikely the larger the room gets. + // however an event like that is unlikely the larger the room gets. If + // the server is ACL'd at the time of generating the link however, we + // shouldn't pick them. We also don't pick IP addresses. // Note: we don't pick the server the room was created on because the // homeserver should already be using that server as a last ditch attempt @@ -104,12 +108,29 @@ export function pickServerCandidates(roomId) { // The receiving user can then manually append the known-good server to // the list and magically have the link work. + const bannedHostsRegexps = []; + let allowedHostsRegexps = [new RegExp(".*")]; // default allow everyone + if (room.currentState) { + const aclEvent = room.currentState.getStateEvents("m.room.server_acl", ""); + if (aclEvent && aclEvent.getContent()) { + const getRegex = (hostname) => new RegExp("^" + utils.globToRegexp(hostname, false) + "$"); + + const denied = aclEvent.getContent().deny || []; + denied.forEach(h => bannedHostsRegexps.push(getRegex(h))); + + const allowed = aclEvent.getContent().allow || []; + allowedHostsRegexps = []; // we don't want to use the default rule here + allowed.forEach(h => allowedHostsRegexps.push(getRegex(h))); + } + } + const populationMap: {[server:string]:number} = {}; const highestPlUser = {userId: null, powerLevel: 0, serverName: null}; for (const member of room.getJoinedMembers()) { const serverName = member.userId.split(":").splice(1).join(":"); - if (member.powerLevel > highestPlUser.powerLevel) { + if (member.powerLevel > highestPlUser.powerLevel && !isHostnameIpAddress(serverName) + && !isHostInRegex(serverName, bannedHostsRegexps) && isHostInRegex(serverName, allowedHostsRegexps)) { highestPlUser.userId = member.userId; highestPlUser.powerLevel = member.powerLevel; highestPlUser.serverName = serverName; @@ -125,8 +146,9 @@ export function pickServerCandidates(roomId) { const beforePopulation = candidates.length; const serversByPopulation = Object.keys(populationMap) .sort((a, b) => populationMap[b] - populationMap[a]) - .filter(a => !candidates.includes(a)); - for (let i = beforePopulation; i <= MAX_SERVER_CANDIDATES; i++) { + .filter(a => !candidates.includes(a) && !isHostnameIpAddress(a) + && !isHostInRegex(a, bannedHostsRegexps) && isHostInRegex(a, allowedHostsRegexps)); + for (let i = beforePopulation; i < MAX_SERVER_CANDIDATES; i++) { const idx = i - beforePopulation; if (idx >= serversByPopulation.length) break; candidates.push(serversByPopulation[idx]); @@ -134,3 +156,34 @@ export function pickServerCandidates(roomId) { return candidates; } + +function getHostnameFromMatrixDomain(domain) { + if (!domain) return null; + + // The hostname might have a port, so we convert it to a URL and + // split out the real hostname. + const parser = document.createElement('a'); + parser.href = "https://" + domain; + return parser.hostname; +} + +function isHostInRegex(hostname, regexps) { + hostname = getHostnameFromMatrixDomain(hostname); + if (!hostname) return true; // assumed + if (regexps.length > 0 && !regexps[0].test) throw new Error(regexps[0]); + + return regexps.filter(h => h.test(hostname)).length > 0; +} + +function isHostnameIpAddress(hostname) { + hostname = getHostnameFromMatrixDomain(hostname); + if (!hostname) return false; + + // is-ip doesn't want IPv6 addresses surrounded by brackets, so + // take them off. + if (hostname.startsWith("[") && hostname.endsWith("]")) { + hostname = hostname.substring(1, hostname.length - 1); + } + + return isIp(hostname); +} \ No newline at end of file diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index 70533575c4..6392e326e9 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -150,7 +150,39 @@ describe('matrix-to', function() { expect(pickedServers[2]).toBe("third"); }); - it('should work with IPv4 hostnames', function() { + it('should pick a maximum of 3 candidate servers', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:alpha", + powerLevel: 100, + }, + { + userId: "@alice:bravo", + powerLevel: 0, + }, + { + userId: "@alice:charlie", + powerLevel: 0, + }, + { + userId: "@alice:delta", + powerLevel: 0, + }, + { + userId: "@alice:echo", + powerLevel: 0, + }, + ], + }; + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(3); + }); + + it('should not consider IPv4 hosts', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -163,11 +195,10 @@ describe('matrix-to', function() { }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); - expect(pickedServers.length).toBe(1); - expect(pickedServers[0]).toBe("127.0.0.1"); + expect(pickedServers.length).toBe(0); }); - it('should work with IPv6 hostnames', function() { + it('should not consider IPv6 hosts', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -180,11 +211,10 @@ describe('matrix-to', function() { }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); - expect(pickedServers.length).toBe(1); - expect(pickedServers[0]).toBe("[::1]"); + expect(pickedServers.length).toBe(0); }); - it('should work with IPv4 hostnames with ports', function() { + it('should not consider IPv4 hostnames with ports', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -197,11 +227,10 @@ describe('matrix-to', function() { }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); - expect(pickedServers.length).toBe(1); - expect(pickedServers[0]).toBe("127.0.0.1:8448"); + expect(pickedServers.length).toBe(0); }); - it('should work with IPv6 hostnames with ports', function() { + it('should not consider IPv6 hostnames with ports', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -214,8 +243,7 @@ describe('matrix-to', function() { }; const pickedServers = pickServerCandidates("!somewhere:example.org"); expect(pickedServers).toExist(); - expect(pickedServers.length).toBe(1); - expect(pickedServers[0]).toBe("[::1]:8448"); + expect(pickedServers.length).toBe(0); }); it('should work with hostnames with ports', function() { @@ -235,6 +263,140 @@ describe('matrix-to', function() { expect(pickedServers[0]).toBe("example.org:8448"); }); + it('should not consider servers explicitly denied by ACLs', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:evilcorp.com", + powerLevel: 100, + }, + { + userId: "@bob:chat.evilcorp.com", + powerLevel: 0, + }, + ], + currentState: { + getStateEvents: (type, key) => { + if (type !== "m.room.server_acl" || key !== "") return null; + return { + getContent: () => { + return { + deny: ["evilcorp.com", "*.evilcorp.com"], + allow: ["*"], + }; + }, + }; + }, + }, + }; + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(0); + }); + + it('should not consider servers not allowed by ACLs', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:evilcorp.com", + powerLevel: 100, + }, + { + userId: "@bob:chat.evilcorp.com", + powerLevel: 0, + }, + ], + currentState: { + getStateEvents: (type, key) => { + if (type !== "m.room.server_acl" || key !== "") return null; + return { + getContent: () => { + return { + deny: [], + allow: [], // implies "ban everyone" + }; + }, + }; + }, + }, + }; + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(0); + }); + + it('should consider servers not explicitly banned by ACLs', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:evilcorp.com", + powerLevel: 100, + }, + { + userId: "@bob:chat.evilcorp.com", + powerLevel: 0, + }, + ], + currentState: { + getStateEvents: (type, key) => { + if (type !== "m.room.server_acl" || key !== "") return null; + return { + getContent: () => { + return { + deny: ["*.evilcorp.com"], // evilcorp.com is still good though + allow: ["*"], + }; + }, + }; + }, + }, + }; + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toEqual("evilcorp.com"); + }); + + it('should consider servers not disallowed by ACLs', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:evilcorp.com", + powerLevel: 100, + }, + { + userId: "@bob:chat.evilcorp.com", + powerLevel: 0, + }, + ], + currentState: { + getStateEvents: (type, key) => { + if (type !== "m.room.server_acl" || key !== "") return null; + return { + getContent: () => { + return { + deny: [], + allow: ["evilcorp.com"], // implies "ban everyone else" + }; + }, + }; + }, + }, + }; + }; + const pickedServers = pickServerCandidates("!somewhere:example.org"); + expect(pickedServers).toExist(); + expect(pickedServers.length).toBe(1); + expect(pickedServers[0]).toEqual("evilcorp.com"); + }); + it('should generate an event permalink for room IDs with no candidate servers', function() { peg.get().getRoom = () => null; const result = makeEventPermalink("!somewhere:example.org", "$something:example.com"); From 45bc1f7dbdfe72e969e80890849426bcc5285703 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 5 Dec 2018 18:14:22 -0700 Subject: [PATCH 2/2] Appease the linter --- src/matrix-to.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index fb2c8096d7..b750dff6d6 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -186,4 +186,4 @@ function isHostnameIpAddress(hostname) { } return isIp(hostname); -} \ No newline at end of file +}