diff --git a/client/electron/index.ts b/client/electron/index.ts index 90f49fb44c..8282d2bd8c 100644 --- a/client/electron/index.ts +++ b/client/electron/index.ts @@ -351,7 +351,7 @@ async function createVpnTunnel( // because startVpn will add a routing table entry that prefixed with this // host (e.g. "/32"), therefore must be an IP address. // TODO: make sure we resolve it in the native code - const host = config.getHostFromTransportConfig(tunnelConfig.transport); + const host = tunnelConfig.firstHop.host; if (!host) { throw new errors.IllegalServerConfiguration('host is missing'); } diff --git a/client/src/www/app/outline_server_repository/config.spec.ts b/client/src/www/app/outline_server_repository/config.spec.ts index a6b2d416a6..efcff0f378 100644 --- a/client/src/www/app/outline_server_repository/config.spec.ts +++ b/client/src/www/app/outline_server_repository/config.spec.ts @@ -19,36 +19,27 @@ import * as config from './config'; describe('getAddressFromTransport', () => { it('extracts address', () => { expect( - config.getAddressFromTransportConfig({host: 'example.com', port: '443'}) - ).toEqual('example.com:443'); + config.TEST_ONLY.getAddressFromTransportConfig({ + host: 'example.com', + port: 443, + }) + ).toEqual({host: 'example.com', port: 443}); expect( - config.getAddressFromTransportConfig({host: '1:2::3', port: '443'}) - ).toEqual('[1:2::3]:443'); - expect(config.getAddressFromTransportConfig({host: 'example.com'})).toEqual( - 'example.com' - ); - expect(config.getAddressFromTransportConfig({host: '1:2::3'})).toEqual( - '1:2::3' - ); - }); - - it('fails on invalid config', () => { - expect(config.getAddressFromTransportConfig({})).toBeUndefined(); - }); -}); - -describe('getHostFromTransport', () => { - it('extracts host', () => { + config.TEST_ONLY.getAddressFromTransportConfig({ + host: '1:2::3', + port: 443, + }) + ).toEqual({host: '1:2::3', port: 443}); expect( - config.getHostFromTransportConfig({host: 'example.com', port: '443'}) - ).toEqual('example.com'); + config.TEST_ONLY.getAddressFromTransportConfig({host: 'example.com'}) + ).toEqual({host: 'example.com', port: undefined}); expect( - config.getHostFromTransportConfig({host: '1:2::3', port: '443'}) - ).toEqual('1:2::3'); + config.TEST_ONLY.getAddressFromTransportConfig({host: '1:2::3'}) + ).toEqual({host: '1:2::3', port: undefined}); }); it('fails on invalid config', () => { - expect(config.getHostFromTransportConfig({})).toBeUndefined(); + expect(config.TEST_ONLY.getAddressFromTransportConfig({})).toBeUndefined(); }); }); @@ -57,24 +48,24 @@ describe('setTransportHost', () => { expect( JSON.stringify( config.setTransportConfigHost( - {host: 'example.com', port: '443'}, + {host: 'example.com', port: 443}, '1.2.3.4' ) ) - ).toEqual('{"host":"1.2.3.4","port":"443"}'); + ).toEqual('{"host":"1.2.3.4","port":443}'); expect( JSON.stringify( config.setTransportConfigHost( - {host: 'example.com', port: '443'}, + {host: 'example.com', port: 443}, '1:2::3' ) ) - ).toEqual('{"host":"1:2::3","port":"443"}'); + ).toEqual('{"host":"1:2::3","port":443}'); expect( JSON.stringify( - config.setTransportConfigHost({host: '1.2.3.4', port: '443'}, '1:2::3') + config.setTransportConfigHost({host: '1.2.3.4', port: 443}, '1:2::3') ) - ).toEqual('{"host":"1:2::3","port":"443"}'); + ).toEqual('{"host":"1:2::3","port":443}'); }); it('fails on invalid config', () => { @@ -89,6 +80,10 @@ describe('parseTunnelConfig', () => { '{"server": "example.com", "server_port": 443, "method": "METHOD", "password": "PASSWORD"}' ) ).toEqual({ + firstHop: { + host: 'example.com', + port: 443, + }, transport: { host: 'example.com', port: 443, @@ -104,6 +99,10 @@ describe('parseTunnelConfig', () => { '{"server": "example.com", "server_port": 443, "method": "METHOD", "password": "PASSWORD", "prefix": "POST "}' ) ).toEqual({ + firstHop: { + host: 'example.com', + port: 443, + }, transport: { host: 'example.com', port: 443, @@ -124,6 +123,10 @@ describe('parseTunnelConfig', () => { }) ); expect(config.parseTunnelConfig(ssUrl)).toEqual({ + firstHop: { + host: 'example.com', + port: 443, + }, transport: { host: 'example.com', port: 443, @@ -133,3 +136,23 @@ describe('parseTunnelConfig', () => { }); }); }); + +describe('serviceNameFromAccessKey', () => { + it('extracts name from ss:// key', () => { + expect( + config.TEST_ONLY.serviceNameFromAccessKey('ss://anything#My%20Server') + ).toEqual('My Server'); + }); + it('extracts name from ssconf:// key', () => { + expect( + config.TEST_ONLY.serviceNameFromAccessKey('ssconf://anything#My%20Server') + ).toEqual('My Server'); + }); + it('ignores parameters', () => { + expect( + config.TEST_ONLY.serviceNameFromAccessKey( + 'ss://anything#foo=bar&My%20Server&baz=boo' + ) + ).toEqual('My Server'); + }); +}); diff --git a/client/src/www/app/outline_server_repository/config.ts b/client/src/www/app/outline_server_repository/config.ts index 907af931f6..7770cad56f 100644 --- a/client/src/www/app/outline_server_repository/config.ts +++ b/client/src/www/app/outline_server_repository/config.ts @@ -12,16 +12,44 @@ // See the License for the specific language governing permissions and // limitations under the License. -import * as net from '@outline/infrastructure/net'; import {SHADOWSOCKS_URI} from 'ShadowsocksConfig'; import * as errors from '../../model/errors'; +export const TEST_ONLY = { + getAddressFromTransportConfig: getAddressFromTransportConfig, + serviceNameFromAccessKey: serviceNameFromAccessKey, +}; + +export type ServiceConfig = { + readonly name: string; +} & (StaticServiceConfig | DynamicServiceConfig); + +export class StaticServiceConfig { + constructor( + readonly name: string, + readonly tunnelconfig: TunnelConfigJson + ) {} +} + +export class DynamicServiceConfig { + constructor( + readonly name: string, + readonly transportConfigLocation: URL + ) {} +} + +class EndpointAddress { + readonly host: string; + readonly port: number | undefined; +} + // Transport configuration. Application code should treat it as opaque, as it's handled by the networking layer. export type TransportConfigJson = object; /** TunnelConfigJson represents the configuration to set up a tunnel. */ export interface TunnelConfigJson { + firstHop: EndpointAddress | undefined; /** transport describes how to establish connections to the destinations. * See https://github.com/Jigsaw-Code/outline-apps/blob/master/client/go/outline/config.go for format. */ transport: TransportConfigJson; @@ -31,34 +59,23 @@ export interface TunnelConfigJson { * getAddressFromTransportConfig returns the address of the tunnel server, if there's a meaningful one. * This is used to show the server address in the UI when connected. */ -export function getAddressFromTransportConfig( +function getAddressFromTransportConfig( transport: TransportConfigJson -): string | undefined { - const hostConfig: {host?: string; port?: string} = transport; - if (hostConfig.host && hostConfig.port) { - return net.joinHostPort(hostConfig.host, hostConfig.port); - } else if (hostConfig.host) { - return hostConfig.host; +): EndpointAddress | undefined { + const hostConfig: {host?: string; port?: number} = transport; + if (hostConfig.host) { + return {host: hostConfig.host, port: hostConfig?.port}; } else { return undefined; } } -/** - * getHostFromTransportConfig returns the host of the tunnel server, if there's a meaningful one. - * This is used by the proxy resolution in Electron. - */ -export function getHostFromTransportConfig( - transport: TransportConfigJson -): string | undefined { - return (transport as unknown as {host: string | undefined}).host; -} - /** * setTransportConfigHost returns a new TransportConfigJson with the given host as the tunnel server. * Should only be set if getHostFromTransportConfig returns one. * This is used by the proxy resolution in Electron. */ +// TODO(fortuna): Move config parsing to Go and do the DNS resolution and IP injection for Electron there. export function setTransportConfigHost( transport: TransportConfigJson, newHost: string @@ -90,6 +107,9 @@ export function parseTunnelConfig( ); } + // TODO(fortuna): stop converting to the Go format. Let the Go code convert. + // We don't validate the method because that's already done in the Go code as + // part of the Dynamic Key connection flow. const transport: TransportConfigJson = { host: responseJson.server, port: responseJson.server_port, @@ -99,57 +119,70 @@ export function parseTunnelConfig( if (responseJson.prefix) { (transport as {prefix?: string}).prefix = responseJson.prefix; } - return {transport}; + return { + transport, + firstHop: getAddressFromTransportConfig(transport), + }; } /** Parses an access key string into a TunnelConfig object. */ -export function staticKeyToTunnelConfig(staticKey: string): TunnelConfigJson { +function staticKeyToTunnelConfig(staticKey: string): TunnelConfigJson { + const config = SHADOWSOCKS_URI.parse(staticKey); + if (!isShadowsocksCipherSupported(config.method.data)) { + throw new errors.ShadowsocksUnsupportedCipher( + config.method.data || 'unknown' + ); + } + const transport: TransportConfigJson = { + host: config.host.data, + port: config.port.data, + method: config.method.data, + password: config.password.data, + }; + if (config.extra?.['prefix']) { + (transport as {prefix?: string}).prefix = config.extra?.['prefix']; + } + return { + transport, + firstHop: getAddressFromTransportConfig(transport), + }; +} + +export function parseAccessKey(accessKey: string): ServiceConfig { try { - const config = SHADOWSOCKS_URI.parse(staticKey); - const transport: TransportConfigJson = { - host: config.host.data, - port: config.port.data, - method: config.method.data, - password: config.password.data, - }; - if (config.extra?.['prefix']) { - (transport as {prefix?: string}).prefix = config.extra?.['prefix']; + accessKey = accessKey.trim(); + + // The default service name is extracted from the URL fragment of the access key. + const name = serviceNameFromAccessKey(accessKey); + + // Static ss:// keys. It encodes the full service config. + if (accessKey.startsWith('ss://')) { + return new StaticServiceConfig(name, parseTunnelConfig(accessKey)); + } + + // Dynamic ssconf:// keys. It encodes the location of the service config. + if (accessKey.startsWith('ssconf://') || accessKey.startsWith('https://')) { + try { + // URL does not parse the hostname (treats as opaque string) if the protocol is non-standard (e.g. non-http). + const configLocation = new URL( + accessKey.replace(/^ssconf:\/\//, 'https://') + ); + return new DynamicServiceConfig(name, configLocation); + } catch (error) { + throw new errors.ServerUrlInvalid(error.message); + } } - return {transport}; - } catch (cause) { + + throw new TypeError('Access Key is not a ss:// or ssconf:// URL'); + } catch (e) { throw new errors.ServerAccessKeyInvalid('Invalid static access key.', { - cause, + cause: e, }); } } export function validateAccessKey(accessKey: string) { - if (!isDynamicAccessKey(accessKey)) { - return validateStaticKey(accessKey); - } - - try { - // URL does not parse the hostname if the protocol is non-standard (e.g. non-http) - new URL(accessKey.replace(/^ssconf:\/\//, 'https://')); - } catch (error) { - throw new errors.ServerUrlInvalid(error.message); - } -} - -function validateStaticKey(staticKey: string) { - let config = null; - try { - config = SHADOWSOCKS_URI.parse(staticKey); - } catch (error) { - throw new errors.ServerUrlInvalid( - error.message || 'failed to parse access key' - ); - } - if (!isShadowsocksCipherSupported(config.method.data)) { - throw new errors.ShadowsocksUnsupportedCipher( - config.method.data || 'unknown' - ); - } + parseAccessKey(accessKey); } // We only support AEAD ciphers for Shadowsocks. @@ -165,21 +198,13 @@ function isShadowsocksCipherSupported(cipher?: string): boolean { return SUPPORTED_SHADOWSOCKS_CIPHERS.includes(cipher); } -// TODO(daniellacosse): write unit tests for these functions -// Determines if the key is expected to be a url pointing to an ephemeral session config. -export function isDynamicAccessKey(accessKey: string): boolean { - return accessKey.startsWith('ssconf://') || accessKey.startsWith('https://'); -} - /** * serviceNameFromAccessKey extracts the service name from the access key. * This is done by getting parsing the fragment hash in the URL and returning the * entry that is not a key=value pair. * This is used to name the service card in the UI when the service is added. */ -export function serviceNameFromAccessKey( - accessKey: string -): string | undefined { +function serviceNameFromAccessKey(accessKey: string): string | undefined { const {hash} = new URL(accessKey.replace(/^ss(?:conf)?:\/\//, 'https://')); if (!hash) return; diff --git a/client/src/www/app/outline_server_repository/index.ts b/client/src/www/app/outline_server_repository/index.ts index c4dd94cf59..1658d183cf 100644 --- a/client/src/www/app/outline_server_repository/index.ts +++ b/client/src/www/app/outline_server_repository/index.ts @@ -16,12 +16,11 @@ import {Localizer} from '@outline/infrastructure/i18n'; import {makeConfig, SIP002_URI} from 'ShadowsocksConfig'; import uuidv4 from 'uuidv4'; -import * as config from './config'; import {OutlineServer} from './server'; import {TunnelStatus, VpnApi} from './vpn'; import * as errors from '../../model/errors'; import * as events from '../../model/events'; -import {ServerRepository, ServerType} from '../../model/server'; +import {ServerRepository} from '../../model/server'; import {ResourceFetcher} from '../resource_fetcher'; // DEPRECATED: V0 server persistence format. @@ -120,11 +119,7 @@ export class OutlineServerRepository implements ServerRepository { if (alreadyAddedServer) { throw new errors.ServerAlreadyAdded(alreadyAddedServer); } - config.validateAccessKey(accessKey); - - // Note that serverNameFromAccessKey depends on the fact that the Access Key is a URL. - const serverName = config.serviceNameFromAccessKey(accessKey); - const server = this.createServer(uuidv4(), accessKey, serverName); + const server = this.createServer(uuidv4(), accessKey, undefined); this.serverById.set(server.id, server); this.storeServers(); @@ -278,28 +273,13 @@ export class OutlineServerRepository implements ServerRepository { accessKey: string, name?: string ): OutlineServer { - const server = new OutlineServer( + return new OutlineServer( this.vpnApi, this.urlFetcher, id, name, accessKey, - config.isDynamicAccessKey(accessKey) - ? ServerType.DYNAMIC_CONNECTION - : ServerType.STATIC_CONNECTION, this.localize ); - - try { - config.validateAccessKey(accessKey); - } catch (e) { - if (e instanceof errors.ShadowsocksUnsupportedCipher) { - // Don't throw for backward-compatibility. - server.errorMessageId = 'unsupported-cipher'; - } else { - throw e; - } - } - return server; } } diff --git a/client/src/www/app/outline_server_repository/outline_server_repository.spec.ts b/client/src/www/app/outline_server_repository/outline_server_repository.spec.ts index f448e5cb2f..c401c0dfd8 100644 --- a/client/src/www/app/outline_server_repository/outline_server_repository.spec.ts +++ b/client/src/www/app/outline_server_repository/outline_server_repository.spec.ts @@ -25,6 +25,7 @@ import * as config from './config'; import {OutlineServer} from './server'; import {FakeVpnApi} from './vpn.fake'; import { + ServerAccessKeyInvalid, ServerUrlInvalid, ShadowsocksUnsupportedCipher, } from '../../model/errors'; @@ -193,8 +194,8 @@ describe('OutlineServerRepository', () => { it('add throws on invalid access keys', () => { const repo = newTestRepo(new EventQueue(), new InMemoryStorage()); - expect(() => repo.add('ss://invalid')).toThrowError(ServerUrlInvalid); - expect(() => repo.add('')).toThrowError(ServerUrlInvalid); + expect(() => repo.add('ss://invalid')).toThrowError(ServerAccessKeyInvalid); + expect(() => repo.add('')).toThrowError(ServerAccessKeyInvalid); }); it('getAll returns added servers', () => { @@ -220,9 +221,9 @@ describe('OutlineServerRepository', () => { repo.add(accessKey); const serverId = repo.getAll()[0].id; const server = repo.getById(serverId); - expect(server.id).toEqual(serverId); - expect(server.accessKey).toEqual(accessKey); - expect(server.name).toEqual(CONFIG_0_V0.name); + expect(server?.id).toEqual(serverId); + expect(server?.accessKey).toEqual(accessKey); + expect(server?.name).toEqual(CONFIG_0_V0.name); }); it('getById returns undefined for nonexistent servers', () => { @@ -308,7 +309,7 @@ describe('OutlineServerRepository', () => { repo.forget(forgottenServerId); repo.undoForget(forgottenServerId); const forgottenServer = repo.getById(forgottenServerId); - expect(forgottenServer.id).toEqual(forgottenServerId); + expect(forgottenServer?.id).toEqual(forgottenServerId); const serverIds = repo.getAll().map(s => s.id); expect(serverIds.length).toEqual(2); expect(serverIds).toContain(forgottenServerId); @@ -341,9 +342,11 @@ describe('OutlineServerRepository', () => { it('validates static access keys', () => { // Invalid access keys. - expect(() => config.validateAccessKey('')).toThrowError(ServerUrlInvalid); + expect(() => config.validateAccessKey('')).toThrowError( + ServerAccessKeyInvalid + ); expect(() => config.validateAccessKey('ss://invalid')).toThrowError( - ServerUrlInvalid + ServerAccessKeyInvalid ); // IPv6 host. expect(() => @@ -370,7 +373,7 @@ describe('OutlineServerRepository', () => { }) ) ) - ).toThrowError(ShadowsocksUnsupportedCipher); + ).toThrowError(ServerAccessKeyInvalid); expect(() => config.validateAccessKey( SIP002_URI.stringify( @@ -382,7 +385,7 @@ describe('OutlineServerRepository', () => { }) ) ) - ).toThrowError(ShadowsocksUnsupportedCipher); + ).toThrowError(ServerAccessKeyInvalid); }); }); diff --git a/client/src/www/app/outline_server_repository/server.ts b/client/src/www/app/outline_server_repository/server.ts index 384e3b2024..7bdea3721f 100644 --- a/client/src/www/app/outline_server_repository/server.ts +++ b/client/src/www/app/outline_server_repository/server.ts @@ -16,10 +16,11 @@ import {Localizer} from '@outline/infrastructure/i18n'; import * as net from '@outline/infrastructure/net'; import { - staticKeyToTunnelConfig, parseTunnelConfig, - getAddressFromTransportConfig, TunnelConfigJson, + DynamicServiceConfig, + StaticServiceConfig, + parseAccessKey, } from './config'; import {StartRequestJson, VpnApi} from './vpn'; import * as errors from '../../model/errors'; @@ -30,10 +31,11 @@ import {ResourceFetcher} from '../resource_fetcher'; // PLEASE DON'T use this class outside of this `outline_server_repository` folder! export class OutlineServer implements Server { - errorMessageId?: string; + public readonly type: ServerType; readonly tunnelConfigLocation: URL; - private _address: string; + private displayAddress: string; private readonly staticTunnelConfig?: TunnelConfigJson; + errorMessageId?: string; constructor( private vpnApi: VpnApi, @@ -41,47 +43,46 @@ export class OutlineServer implements Server { readonly id: string, public name: string, readonly accessKey: string, - readonly type: ServerType, localize: Localizer ) { - switch (this.type) { - case ServerType.DYNAMIC_CONNECTION: - this.tunnelConfigLocation = new URL( - accessKey.replace(/^ssconf:\/\//, 'https://') - ); - this._address = ''; - - if (!name) { - this.name = - this.tunnelConfigLocation.port === '443' - ? this.tunnelConfigLocation.hostname - : net.joinHostPort( - this.tunnelConfigLocation.hostname, - this.tunnelConfigLocation.port - ); - } - break; - - case ServerType.STATIC_CONNECTION: - default: - this.staticTunnelConfig = staticKeyToTunnelConfig(accessKey); - this._address = getAddressFromTransportConfig( - this.staticTunnelConfig.transport - ); + const serviceConfig = parseAccessKey(accessKey); + this.name = name ?? serviceConfig.name; + + if (serviceConfig instanceof DynamicServiceConfig) { + this.type = ServerType.DYNAMIC_CONNECTION; + this.tunnelConfigLocation = serviceConfig.transportConfigLocation; + this.displayAddress = ''; + + if (!serviceConfig.name) { + this.name = + this.tunnelConfigLocation.port === '443' + ? this.tunnelConfigLocation.hostname + : net.joinHostPort( + this.tunnelConfigLocation.hostname, + this.tunnelConfigLocation.port + ); + } + } else if (serviceConfig instanceof StaticServiceConfig) { + this.type = ServerType.STATIC_CONNECTION; + this.staticTunnelConfig = serviceConfig.tunnelconfig; + const firstHop = serviceConfig.tunnelconfig.firstHop; + this.displayAddress = net.joinHostPort( + firstHop.host, + firstHop.port.toString() + ); - if (!name) { - this.name = localize( - accessKey.includes('outline=1') - ? 'server-default-name-outline' - : 'server-default-name' - ); - } - break; + if (!this.name) { + this.name = localize( + accessKey.includes('outline=1') + ? 'server-default-name-outline' + : 'server-default-name' + ); + } } } get address() { - return this._address; + return this.displayAddress; } async connect() { @@ -91,7 +92,10 @@ export class OutlineServer implements Server { this.urlFetcher, this.tunnelConfigLocation ); - this._address = getAddressFromTransportConfig(tunnelConfig.transport); + this.displayAddress = net.joinHostPort( + tunnelConfig.firstHop.host, + tunnelConfig.firstHop.port.toString() + ); } else { tunnelConfig = this.staticTunnelConfig; } @@ -127,7 +131,7 @@ export class OutlineServer implements Server { await this.vpnApi.stop(this.id); if (this.type === ServerType.DYNAMIC_CONNECTION) { - this._address = ''; + this.displayAddress = ''; } } catch (e) { // All the plugins treat disconnection errors as ErrorCode.UNEXPECTED. diff --git a/client/src/www/app/outline_server_repository/vpn.fake.ts b/client/src/www/app/outline_server_repository/vpn.fake.ts index ebcad9942f..19c019e75d 100644 --- a/client/src/www/app/outline_server_repository/vpn.fake.ts +++ b/client/src/www/app/outline_server_repository/vpn.fake.ts @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {getHostFromTransportConfig} from './config'; import {VpnApi, TunnelStatus, StartRequestJson} from './vpn'; import * as errors from '../../model/errors'; @@ -40,7 +39,7 @@ export class FakeVpnApi implements VpnApi { return; } - const host = getHostFromTransportConfig(request.config.transport); + const host = request.config.firstHop.host; if (this.playUnreachable(host)) { throw new errors.OutlinePluginError(errors.ErrorCode.SERVER_UNREACHABLE); } else if (this.playBroken(host)) {