Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(client): make parsing async and config an opaque string #2324

Merged
merged 9 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions client/electron/go_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import {pathToEmbeddedTun2socksBinary} from './app_paths';
import {ChildProcessHelper} from './process';
import {TransportConfigJson} from '../src/www/app/outline_server_repository/config';

/**
* Verifies the UDP connectivity of the server specified in `config`.
Expand All @@ -34,7 +33,7 @@ import {TransportConfigJson} from '../src/www/app/outline_server_repository/conf
* @throws ProcessTerminatedExitCodeError if tun2socks failed to run.
*/
export async function checkUDPConnectivity(
config: TransportConfigJson,
config: string,
debugMode: boolean = false
): Promise<boolean> {
const tun2socks = new ChildProcessHelper(pathToEmbeddedTun2socksBinary());
Expand Down
10 changes: 3 additions & 7 deletions client/electron/go_vpn_tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {checkUDPConnectivity} from './go_helpers';
import {ChildProcessHelper, ProcessTerminatedSignalError} from './process';
import {RoutingDaemon} from './routing_service';
import {VpnTunnel} from './vpn_tunnel';
import {TransportConfigJson} from '../src/www/app/outline_server_repository/config';
import {TunnelStatus} from '../src/www/app/outline_server_repository/vpn';

const isLinux = platform() === 'linux';
Expand Down Expand Up @@ -64,7 +63,7 @@ export class GoVpnTunnel implements VpnTunnel {

constructor(
private readonly routing: RoutingDaemon,
readonly transportConfig: TransportConfigJson
readonly transportConfig: string
) {
this.tun2socks = new GoTun2socks();

Expand Down Expand Up @@ -248,10 +247,7 @@ class GoTun2socks {
* Otherwise, an error containing a JSON-formatted message will be thrown.
* @param isUdpEnabled Indicates whether the remote Outline server supports UDP.
*/
async start(
config: TransportConfigJson,
isUdpEnabled: boolean
): Promise<void> {
async start(transportConfig: string, isUdpEnabled: boolean): Promise<void> {
// ./tun2socks.exe \
// -tunName outline-tap0 -tunDNS 1.1.1.1,9.9.9.9 \
// -tunAddr 10.0.85.2 -tunGw 10.0.85.1 -tunMask 255.255.255.0 \
Expand All @@ -263,7 +259,7 @@ class GoTun2socks {
args.push('-tunGw', TUN2SOCKS_VIRTUAL_ROUTER_IP);
args.push('-tunMask', TUN2SOCKS_VIRTUAL_ROUTER_NETMASK);
args.push('-tunDNS', DNS_RESOLVERS.join(','));
args.push('-transport', JSON.stringify(config));
args.push('-transport', transportConfig);
args.push('-logLevel', this.process.isDebugModeEnabled ? 'debug' : 'info');
if (!isUdpEnabled) {
args.push('-dnsFallback');
Expand Down
12 changes: 7 additions & 5 deletions client/electron/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as path from 'path';
import * as process from 'process';
import * as url from 'url';

import * as net from '@outline/infrastructure/net';
import * as Sentry from '@sentry/electron/main';
import autoLaunch = require('auto-launch'); // tslint:disable-line
import {
Expand Down Expand Up @@ -352,17 +353,18 @@ async function createVpnTunnel(
// because startVpn will add a routing table entry that prefixed with this
// host (e.g. "<host>/32"), therefore <host> must be an IP address.
// TODO: make sure we resolve it in the native code
const host = tunnelConfig.firstHop.host;
fortuna marked this conversation as resolved.
Show resolved Hide resolved
const {host} = net.splitHostPort(tunnelConfig.firstHop);
if (!host) {
throw new errors.IllegalServerConfiguration('host is missing');
}
const hostIp = await lookupIp(host);
const routing = new RoutingDaemon(hostIp || '', isAutoConnect);
// Make sure the transport will use the IP we will allowlist.
const resolvedTransport =
config.setTransportConfigHost(tunnelConfig.transport, hostIp) ??
tunnelConfig.transport;
const tunnel = new GoVpnTunnel(routing, resolvedTransport);
// HACK: We do a simple string replacement in the config here. This may not always work with general configs
// but it works for simple configs.
// TODO: Remove the need to allowlisting the host IP.
tunnelConfig.transport = tunnelConfig.transport.replaceAll(host, hostIp);
const tunnel = new GoVpnTunnel(routing, tunnelConfig.transport);
routing.onNetworkChange = tunnel.networkChanged.bind(tunnel);
return tunnel;
}
Expand Down
6 changes: 4 additions & 2 deletions client/src/cordova/plugin/apple/src/OutlinePlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,18 @@ class OutlinePlugin: CDVPlugin {
guard let input = command.argument(at: 1) as? String else {
return sendError("Missing method input", callbackId: command.callbackId)
}
DDLogInfo("Invoking Method \(methodName) with input \(input)")
DDLogDebug("Invoking Method \(methodName) with input \(input)")
Task {
guard let result = OutlineInvokeMethod(methodName, input) else {
DDLogDebug("InvokeMethod \(methodName) got nil result")
return self.sendError("unexpected invoke error", callbackId: command.callbackId)
}
if result.error != nil {
let errorJson = marshalErrorJson(error: OutlineError.platformError(result.error!))
DDLogDebug("InvokeMethod \(methodName) failed with error \(errorJson)")
return self.sendError(errorJson, callbackId: command.callbackId)
}
DDLogInfo("InvokeMethod result: \(result.value)")
DDLogDebug("InvokeMethod result: \(result.value)")
self.sendSuccess(result.value, callbackId: command.callbackId)
}
}
Expand Down
46 changes: 24 additions & 22 deletions client/src/www/app/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {OperationTimedOut} from '@outline/infrastructure/timeout_promise';
import {Clipboard} from './clipboard';
import {EnvironmentVariables} from './environment';
import {localizeErrorCode} from './error_localizer';
import {OutlineServerRepository} from './outline_server_repository';
import * as config from './outline_server_repository/config';
import {Settings, SettingsKey} from './settings';
import {Updater} from './updater';
Expand All @@ -30,7 +29,7 @@ import {
PlatformError,
ROUTING_SERVICE_NOT_RUNNING,
} from '../model/platform_error';
import {Server} from '../model/server';
import {Server, ServerRepository} from '../model/server';
import {OutlineErrorReporter} from '../shared/error_reporter';
import {ServerConnectionState, ServerListItem} from '../views/servers_view';
import {SERVER_CONNECTION_INDICATOR_DURATION_MS} from '../views/servers_view/server_connection_indicator';
Expand Down Expand Up @@ -94,7 +93,7 @@ export class App {

constructor(
private eventQueue: events.EventQueue,
private serverRepo: OutlineServerRepository,
private serverRepo: ServerRepository,
private rootEl: polymer.Base,
private debugMode: boolean,
urlInterceptor: UrlInterceptor | undefined,
Expand Down Expand Up @@ -227,9 +226,11 @@ export class App {

this.eventQueue.startPublishing();

this.rootEl.$.addServerView.isValidAccessKey = (accessKey: string) => {
this.rootEl.$.addServerView.validateAccessKey = async (
accessKey: string
): Promise<boolean> => {
try {
config.parseAccessKey(accessKey);
await config.parseAccessKey(accessKey);
return true;
} catch {
return false;
Expand Down Expand Up @@ -363,7 +364,7 @@ export class App {
private async pullClipboardText() {
try {
const text = await this.clipboard.getContents();
this.handleClipboardText(text);
await this.handleClipboardText(text);
} catch (e) {
console.warn('cannot read clipboard, system may lack clipboard support');
}
Expand Down Expand Up @@ -416,13 +417,13 @@ export class App {
this.rootEl.changePage(event.detail.page);
}

private handleClipboardText(text: string) {
private async handleClipboardText(text: string) {
// Shorten, sanitise.
// Note that we always check the text, even if the contents are same as last time, because we
// keep an in-memory cache of user-ignored access keys.
text = text.substring(0, 1000).trim();
try {
this.confirmAddServer(text, true);
await this.confirmAddServer(text, true);
} catch (err) {
// Don't alert the user; high false positive rate.
}
Expand All @@ -444,28 +445,29 @@ export class App {
}

private requestAddServer(event: CustomEvent) {
try {
this.serverRepo.add(event.detail.accessKey);
} catch (err) {
this.changeToDefaultPage();
this.showLocalizedError(err);
} finally {
this.rootEl.$.addServerView.open = false;
}
this.serverRepo
.add(event.detail.accessKey)
.catch(err => {
jyyi1 marked this conversation as resolved.
Show resolved Hide resolved
this.changeToDefaultPage();
this.showLocalizedError(err);
})
.finally(() => {
this.rootEl.$.addServerView.open = false;
});
}

private requestAddServerConfirmation(event: CustomEvent) {
private async requestAddServerConfirmation(event: CustomEvent) {
const accessKey = event.detail.accessKey;
console.debug('Got add server confirmation request from UI');
try {
this.confirmAddServer(accessKey);
await this.confirmAddServer(accessKey);
} catch (err) {
console.error('Failed to confirm add sever.', err);
this.showLocalizedError(err);
}
}

private confirmAddServer(accessKey: string, fromClipboard = false) {
private async confirmAddServer(accessKey: string, fromClipboard = false) {
const addServerView = this.rootEl.$.addServerView;
accessKey = unwrapInvite(accessKey);
if (fromClipboard && !addServerView.open) {
Expand All @@ -478,7 +480,7 @@ export class App {
}
}
try {
config.validateAccessKey(accessKey);
await config.parseAccessKey(accessKey);
addServerView.accessKey = accessKey;
addServerView.open = true;
} catch (e) {
Expand Down Expand Up @@ -821,7 +823,7 @@ export class App {
}

private registerUrlInterceptionListener(urlInterceptor: UrlInterceptor) {
urlInterceptor.registerListener(url => {
urlInterceptor.registerListener(async url => {
if (!isOutlineAccessKey(unwrapInvite(url))) {
// This check is necessary to ignore empty and malformed install-referrer URLs in Android
// while allowing ss://, ssconf:// and invite URLs.
Expand All @@ -830,7 +832,7 @@ export class App {
}

try {
this.confirmAddServer(url);
await this.confirmAddServer(url);
} catch (err) {
this.showLocalizedErrorInDefaultPage(err);
}
Expand Down
7 changes: 6 additions & 1 deletion client/src/www/app/main.cordova.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ class CordovaErrorReporter extends SentryErrorReporter {

class CordovaMethodChannel implements MethodChannel {
invokeMethod(methodName: string, params: string): Promise<string> {
return pluginExecWithErrorCode('invokeMethod', methodName, params);
try {
return pluginExecWithErrorCode('invokeMethod', methodName, params);
} catch (e) {
console.debug('invokeMethod failed', methodName, e);
jyyi1 marked this conversation as resolved.
Show resolved Hide resolved
throw e;
}
}
}

Expand Down
16 changes: 10 additions & 6 deletions client/src/www/app/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {makeConfig, SIP002_URI} from 'ShadowsocksConfig';

import {App} from './app';
import {onceEnvVars} from './environment';
import {OutlineServerRepository} from './outline_server_repository';
import {newOutlineServerRepository} from './outline_server_repository';
import {
FAKE_BROKEN_HOSTNAME,
FAKE_UNREACHABLE_HOSTNAME,
Expand All @@ -28,6 +28,7 @@ import {
import {OutlinePlatform} from './platform';
import {Settings} from './settings';
import {EventQueue} from '../model/events';
import {ServerRepository} from '../model/server.js';

// Used to determine whether to use Polymer functionality on app initialization failure.
let webComponentsAreReady = false;
Expand All @@ -52,11 +53,14 @@ function getRootEl() {
return document.querySelector('app-root') as {} as polymer.Base;
}

function createServerRepo(platform: OutlinePlatform, eventQueue: EventQueue) {
async function createServerRepo(
platform: OutlinePlatform,
eventQueue: EventQueue
): Promise<ServerRepository> {
const localize = getLocalizationFunction();
const vpnApi = platform.getVpnApi();
if (vpnApi) {
return new OutlineServerRepository(
return await newOutlineServerRepository(
vpnApi,
eventQueue,
window.localStorage,
Expand All @@ -65,7 +69,7 @@ function createServerRepo(platform: OutlinePlatform, eventQueue: EventQueue) {
}

console.debug('Platform not supported, using fake servers.');
const repo = new OutlineServerRepository(
const repo = await newOutlineServerRepository(
new FakeVpnApi(),
eventQueue,
window.localStorage,
Expand Down Expand Up @@ -109,14 +113,14 @@ function createServerRepo(platform: OutlinePlatform, eventQueue: EventQueue) {

export function main(platform: OutlinePlatform) {
return Promise.all([onceEnvVars, oncePolymerIsReady]).then(
([environmentVars]) => {
async ([environmentVars]) => {
console.debug('running main() function');

const queryParams = new URL(document.URL).searchParams;
const debugMode = queryParams.get('debug') === 'true';

const eventQueue = new EventQueue();
const serverRepo = createServerRepo(platform, eventQueue);
const serverRepo = await createServerRepo(platform, eventQueue);
const settings = new Settings();
new App(
eventQueue,
Expand Down
Loading
Loading