matrix-org / matrix-user-verification-service

Service to verify details of a user based on a Open ID token.
Other
22 stars 21 forks source link

discoverHomeserverUrl Edgecase #31

Closed rwjack closed 9 months ago

rwjack commented 1 year ago

Edge case somewhere in matrixUtils.js:async function discoverHomeserverUrl(serverName)

My synapse server has delegation with the port 443, and the wellknown entry returns:

{ "m.server": "matrix.domain.tld:443" }

yet matrixUtils.js:54 somehow returns domain.tld:8448.

the .env entry also being: UVS_OPENID_VERIFY_SERVER_NAME=domain.tld

rwjack commented 1 year ago

Actually, the code somehow falls through all the way to the end:

This bypass fixes it for me, but the point is it should not fall through, because well-known delegation can be reached

@@ -198,7 +198,7 @@ async function discoverHomeserverUrl(serverName) {
      * The target server must present a valid certificate for <hostname>.
      */
     return {
-        homeserverUrl: `https://${hostname}:8448`,
+        homeserverUrl: `https://${hostname}:443`,
         serverName: hostname,
     };
 }
rwjack commented 1 year ago

Basically I'd say that the matrixUtils.js:98 code should pass, but it doesn't for some reason

rwjack commented 1 year ago

This is the final code edit that worked for me, I'll state again that this is not a solution, just the steps to fix my edgecase. It includes 2 commits from the following fork . I also had to proxy domain.tld/(_synapse|_matrix)/ to the synapse instance.

diff --git a/src/matrixUtils.js b/src/matrixUtils.js
index a652c40..bd5a2d0 100644
--- a/src/matrixUtils.js
+++ b/src/matrixUtils.js
@@ -55,7 +55,7 @@ async function discoverHomeserverUrl(serverName) {
     let {hostname, port, defaultPort} = parseHostnameAndPort(serverName);

     // Don't continue if we consider the hostname part to resolve to our blacklisted IP ranges
-    if (utils.isBlacklisted(await utils.resolveDomain(hostname))) {
+    if (!process.env.UVS_DISABLE_IP_BLACKLIST && utils.isBlacklisted(await utils.resolveDomain(hostname))) {
         throw Error('Hostname resolves to a blacklisted IP range.');
     }

@@ -104,6 +104,7 @@ async function discoverHomeserverUrl(serverName) {
     if (delegatedHostname) {
         const parsed = parseHostnameAndPort(delegatedHostname);

+        // maybe we need this here as well? !process.env.UVS_DISABLE_IP_BLACKLIST &&
          // Don't continue if we consider the hostname part to resolve to our blacklisted IP ranges
         if (utils.isBlacklisted(await utils.resolveDomain(parsed.hostname))) {
             throw Error('Delegated hostname resolves to a blacklisted IP range.');
@@ -197,7 +198,7 @@ async function discoverHomeserverUrl(serverName) {
      * The target server must present a valid certificate for <hostname>.
      */
     return {
-        homeserverUrl: `https://${hostname}:8448`,
+        homeserverUrl: `https://${hostname}:443`,
         serverName: hostname,
     };
 }
diff --git a/src/utils.js b/src/utils.js
index fa18820..f657e5a 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -92,7 +92,6 @@ function requestLogger(req) {

 const ip4RangeBlacklist = [
     '127.0.0.0/8',
-    '10.0.0.0/8',
     '172.16.0.0/12',
     '192.168.0.0/16',
     '100.64.0.0/10',
diff --git a/src/verify.js b/src/verify.js
index 7168735..aa574fe 100644
--- a/src/verify.js
+++ b/src/verify.js
@@ -85,7 +85,8 @@ async function verifyOpenIDToken(req) {
     let response;
     let serverName;
     if (process.env.UVS_OPENID_VERIFY_SERVER_NAME) {
-        if (req.body.matrix_server_name !== process.env.UVS_OPENID_VERIFY_SERVER_NAME) {
+        if (req.body.matrix_server_name !== process.env.UVS_OPENID_VERIFY_SERVER_NAME &&
+            req.body.matrix_server_name !== "matrix.cthd.icu") {
             // Refuse to check token against any other servers
             logger.log(
                 'warn',
@@ -120,6 +121,7 @@ async function verifyOpenIDToken(req) {
             {
                 Host: homeserver.serverName,
             },
+            process.env.UVS_DISABLE_IP_BLACKLIST === 'true',
         );
     } catch (error) {
         utils.errorLogger(error, req);
afr4283 commented 1 year ago

Hi, the main problem occurs because in the function verifyOpenIDToken process.env.UVS_HOMESERVER_URL is ignored, url is made from matrtix_server_name, unlike the correct code in getRoomPowerLevels or verifyRoomMembership. matrix_server_name should not be used when UVS_HOMESERVER_URL is set. Adam.