diff options
| author | David Lönnhager <david.l@mullvad.net> | 2021-06-18 13:54:22 +0200 |
|---|---|---|
| committer | David Lönnhager <david.l@mullvad.net> | 2021-06-18 13:54:22 +0200 |
| commit | 1c7f284c710f3b96f8849d9ad7de989c8f8a5fe0 (patch) | |
| tree | 8d31f554493b97e1e84d2e8dfc8f04bca7e0870b | |
| parent | 2749175102717c74ff945c49302cef8a1ac00bad (diff) | |
| parent | a51d01c9550a2c1710f46f724723a5a50cfdbe3a (diff) | |
| download | mullvadvpn-1c7f284c710f3b96f8849d9ad7de989c8f8a5fe0.tar.xz mullvadvpn-1c7f284c710f3b96f8849d9ad7de989c8f8a5fe0.zip | |
Merge branch 'remove-account-history'
33 files changed, 425 insertions, 518 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d071331587..d310dd6b89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ Line wrap the file at 100 chars. Th ## [Unreleased] +### Changed +- Only use the account history file to store the last used account. + ### Fixed - Fix link to download page not always using the beta URL when it should. diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Event.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Event.kt index faaba05d89..a5854232a8 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Event.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Event.kt @@ -17,7 +17,7 @@ sealed class Event : Message.EventMessage() { protected override val messageKey = MESSAGE_KEY @Parcelize - data class AccountHistory(val history: List<String>?) : Event() + data class AccountHistory(val history: String?) : Event() @Parcelize data class AppVersionInfo(val versionInfo: AppVersionInfoData?) : Event() diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Request.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Request.kt index 8093cac415..27a93b0a2d 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Request.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/Request.kt @@ -54,7 +54,7 @@ sealed class Request : Message.RequestMessage() { data class RegisterListener(val listener: Messenger) : Request() @Parcelize - data class RemoveAccountFromHistory(val account: String?) : Request() + object ClearAccountHistory : Request() @Parcelize data class RemoveCustomDnsServer(val address: InetAddress) : Request() diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt index bab7bcf3ce..4565844daa 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt @@ -52,7 +52,7 @@ class MullvadDaemon(val vpnService: MullvadVpnService) { return getAccountData(daemonInterfaceAddress, accountToken) } - fun getAccountHistory(): ArrayList<String>? { + fun getAccountHistory(): String? { return getAccountHistory(daemonInterfaceAddress) } @@ -92,8 +92,8 @@ class MullvadDaemon(val vpnService: MullvadVpnService) { reconnect(daemonInterfaceAddress) } - fun removeAccountFromHistory(accountToken: String) { - removeAccountFromHistory(daemonInterfaceAddress, accountToken) + fun clearAccountHistory() { + clearAccountHistory(daemonInterfaceAddress) } fun setAccount(accountToken: String?) { @@ -159,7 +159,7 @@ class MullvadDaemon(val vpnService: MullvadVpnService) { daemonInterfaceAddress: Long, accountToken: String ): GetAccountDataResult - private external fun getAccountHistory(daemonInterfaceAddress: Long): ArrayList<String>? + private external fun getAccountHistory(daemonInterfaceAddress: Long): String? private external fun getWwwAuthToken(daemonInterfaceAddress: Long): String? private external fun getCurrentLocation(daemonInterfaceAddress: Long): GeoIpLocation? private external fun getCurrentVersion(daemonInterfaceAddress: Long): String? @@ -169,10 +169,7 @@ class MullvadDaemon(val vpnService: MullvadVpnService) { private external fun getVersionInfo(daemonInterfaceAddress: Long): AppVersionInfo? private external fun getWireguardKey(daemonInterfaceAddress: Long): PublicKey? private external fun reconnect(daemonInterfaceAddress: Long) - private external fun removeAccountFromHistory( - daemonInterfaceAddress: Long, - accountToken: String - ) + private external fun clearAccountHistory(daemonInterfaceAddress: Long) private external fun setAccount(daemonInterfaceAddress: Long, accountToken: String?) private external fun setAllowLan(daemonInterfaceAddress: Long, allowLan: Boolean) private external fun setAutoConnect(daemonInterfaceAddress: Long, alwaysOn: Boolean) diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/AccountCache.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/AccountCache.kt index 5e6524e3a0..768b00e1b5 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/AccountCache.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/AccountCache.kt @@ -42,7 +42,7 @@ class AccountCache(private val endpoint: ServiceEndpoint) { val onAccountNumberChange = EventNotifier<String?>(null) val onAccountExpiryChange = EventNotifier<DateTime?>(null) - val onAccountHistoryChange = EventNotifier<List<String>>(listOf<String>()) + val onAccountHistoryChange = EventNotifier<String?>(null) val onLoginStatusChange = EventNotifier<LoginStatus?>(null) var newlyCreatedAccount = false @@ -104,10 +104,8 @@ class AccountCache(private val endpoint: ServiceEndpoint) { invalidateAccountExpiry(request.expiry) } - registerHandler(Request.RemoveAccountFromHistory::class) { request -> - request.account?.let { account -> - removeAccountFromHistory(account) - } + registerHandler(Request.ClearAccountHistory::class) { _ -> + clearAccountHistory() } } } @@ -162,9 +160,9 @@ class AccountCache(private val endpoint: ServiceEndpoint) { } } - private fun removeAccountFromHistory(accountToken: String) { - jobTracker.newBackgroundJob("removeAccountFromHistory $accountToken") { - daemon.await().removeAccountFromHistory(accountToken) + private fun clearAccountHistory() { + jobTracker.newBackgroundJob("clearAccountHistory") { + daemon.await().clearAccountHistory() fetchAccountHistory() } } @@ -227,7 +225,7 @@ class AccountCache(private val endpoint: ServiceEndpoint) { private fun fetchAccountHistory() { jobTracker.newBackgroundJob("fetchHistory") { - daemon.await().getAccountHistory()?.let { history -> + daemon.await().getAccountHistory().let { history -> accountHistory = history } } diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/LoginFragment.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/LoginFragment.kt index 83f0ecc3ad..399490f04b 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/LoginFragment.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/LoginFragment.kt @@ -51,7 +51,7 @@ class LoginFragment : ServiceDependentFragment(OnNoService.GoToLaunchScreen), Na accountLogin = view.findViewById<AccountLogin>(R.id.account_login).apply { onLogin = { accountToken -> login(accountToken) } - onRemoveFromHistory = { account -> accountCache.removeAccountFromHistory(account) } + onClearHistory = { -> accountCache.clearAccountHistory() } } view.findViewById<Button>(R.id.create_account) diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/AccountCache.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/AccountCache.kt index e2c344f63f..9bf5942f4c 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/AccountCache.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/AccountCache.kt @@ -11,7 +11,7 @@ import org.joda.time.DateTime class AccountCache(private val connection: Messenger, eventDispatcher: EventDispatcher) { val onAccountNumberChange = EventNotifier<String?>(null) val onAccountExpiryChange = EventNotifier<DateTime?>(null) - val onAccountHistoryChange = EventNotifier<List<String>>(listOf<String>()) + val onAccountHistoryChange = EventNotifier<String?>(null) val onLoginStatusChange = EventNotifier<LoginStatus?>(null) private var accountHistory by onAccountHistoryChange.notifiable() @@ -20,7 +20,7 @@ class AccountCache(private val connection: Messenger, eventDispatcher: EventDisp init { eventDispatcher.apply { registerHandler(Event.AccountHistory::class) { event -> - accountHistory = event.history ?: listOf<String>() + accountHistory = event.history } registerHandler(Event.LoginStatus::class) { event -> @@ -54,8 +54,8 @@ class AccountCache(private val connection: Messenger, eventDispatcher: EventDisp connection.send(request.message) } - fun removeAccountFromHistory(account: String) { - connection.send(Request.RemoveAccountFromHistory(account).message) + fun clearAccountHistory() { + connection.send(Request.ClearAccountHistory.message) } fun onDestroy() { diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountHistoryAdapter.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountHistoryAdapter.kt index caefcfe51f..48a05dd63e 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountHistoryAdapter.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountHistoryAdapter.kt @@ -14,12 +14,12 @@ class AccountHistoryAdapter : Adapter<AccountHistoryHolder>() { } } - var accountHistory by observable(listOf<String>()) { _, _, _ -> + var accountHistory by observable<String?>(null) { _, _, _ -> notifyDataSetChanged() } var onSelectEntry: ((String) -> Unit)? = null - var onRemoveEntry: ((String) -> Unit)? = null + var onRemoveEntry: (() -> Unit)? = null var onChildFocusChanged: ((String, Boolean) -> Unit)? = null override fun onCreateViewHolder(parentView: ViewGroup, type: Int): AccountHistoryHolder { @@ -28,14 +28,14 @@ class AccountHistoryAdapter : Adapter<AccountHistoryHolder>() { return AccountHistoryHolder(view, formatter).apply { onSelect = { account -> onSelectEntry?.invoke(account) } - onRemove = { account -> onRemoveEntry?.invoke(account) } + onRemove = { _ -> onRemoveEntry?.invoke() } onFocusChanged = { account, hasFocus -> onChildFocusChanged?.invoke(account, hasFocus) } } } override fun onBindViewHolder(holder: AccountHistoryHolder, position: Int) { - holder.accountToken = accountHistory[position] + holder.accountToken = accountHistory ?: "" } - override fun getItemCount() = accountHistory.size + override fun getItemCount() = if (accountHistory !== null) 1 else 0 } diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountLogin.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountLogin.kt index e4256a9dc5..aa3463eb94 100644 --- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountLogin.kt +++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/widget/AccountLogin.kt @@ -99,13 +99,12 @@ class AccountLogin : RelativeLayout { val hasFocus get() = focused - var accountHistory by observable<List<String>?>(null) { _, _, history -> - val entryCount = history?.size ?: 0 - - historyHeight = entryCount * (historyEntryHeight + dividerHeight) - + var accountHistory by observable<String?>(null) { _, _, history -> if (history != null) { + historyHeight = historyEntryHeight + dividerHeight historyAdapter.accountHistory = history + } else { + historyHeight = 0 } } @@ -123,7 +122,7 @@ class AccountLogin : RelativeLayout { get() = input.onLogin set(value) { input.onLogin = value } - var onRemoveFromHistory: ((String) -> Unit)? + var onClearHistory: (() -> Unit)? get() = historyAdapter.onRemoveEntry set(value) { historyAdapter.onRemoveEntry = value } diff --git a/dist-assets/linux/before-remove.sh b/dist-assets/linux/before-remove.sh index 0c2990b139..0871efab1e 100644 --- a/dist-assets/linux/before-remove.sh +++ b/dist-assets/linux/before-remove.sh @@ -24,4 +24,4 @@ elif /sbin/init --version | grep upstart &> /dev/null; then fi /opt/Mullvad\ VPN/resources/mullvad-setup reset-firewall || echo "Failed to reset firewall" -/opt/Mullvad\ VPN/resources/mullvad-setup clear-history || echo "Failed to remove leftover WireGuard keys" +/opt/Mullvad\ VPN/resources/mullvad-setup remove-wireguard-key || echo "Failed to remove leftover WireGuard key" diff --git a/dist-assets/uninstall_macos.sh b/dist-assets/uninstall_macos.sh index 4c02245a30..72b11e2e51 100755 --- a/dist-assets/uninstall_macos.sh +++ b/dist-assets/uninstall_macos.sh @@ -20,7 +20,7 @@ sudo rm -f "$DAEMON_PLIST_PATH" echo "Resetting firewall" sudo /Applications/Mullvad\ VPN.app/Contents/Resources/mullvad-setup reset-firewall -sudo /Applications/Mullvad\ VPN.app/Contents/Resources/mullvad-setup clear-history +sudo /Applications/Mullvad\ VPN.app/Contents/Resources/mullvad-setup remove-wireguard-key echo "Removing zsh shell completion symlink ..." sudo rm -f /usr/local/share/zsh/site-functions/_mullvad diff --git a/dist-assets/windows/installer.nsh b/dist-assets/windows/installer.nsh index 08907c73f6..4f967f0196 100644 --- a/dist-assets/windows/installer.nsh +++ b/dist-assets/windows/installer.nsh @@ -569,25 +569,25 @@ !define ClearFirewallRules '!insertmacro "ClearFirewallRules"' # -# ClearAccountHistory +# RemoveWireGuardKey # -# Removes account history and any associated keys +# Remove the WireGuard key from the account, if there is one # -!macro ClearAccountHistory +!macro RemoveWireGuardKey - log::Log "ClearAccountHistory()" + log::Log "RemoveWireGuardKey()" Push $0 Push $1 - nsExec::ExecToStack '"$TEMP\mullvad-setup.exe" clear-history' + nsExec::ExecToStack '"$TEMP\mullvad-setup.exe" remove-wireguard-key' Pop $0 Pop $1 ${If} $0 != ${MVSETUP_OK} - log::LogWithDetails "ClearAccountHistory() failed" $1 + log::LogWithDetails "RemoveWireGuardKey() failed" $1 ${Else} - log::Log "ClearAccountHistory() completed successfully" + log::Log "RemoveWireGuardKey() completed successfully" ${EndIf} Pop $1 @@ -595,7 +595,7 @@ !macroend -!define ClearAccountHistory '!insertmacro "ClearAccountHistory"' +!define RemoveWireGuardKey '!insertmacro "RemoveWireGuardKey"' # @@ -1000,7 +1000,7 @@ ${If} $FullUninstall == 1 ${ClearFirewallRules} - ${ClearAccountHistory} + ${RemoveWireGuardKey} ${ExtractWintun} ${RemoveWintun} diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts index 04332a4447..8ed75b8885 100644 --- a/gui/src/main/daemon-rpc.ts +++ b/gui/src/main/daemon-rpc.ts @@ -418,13 +418,13 @@ export class DaemonRpc { } } - public async getAccountHistory(): Promise<AccountToken[]> { + public async getAccountHistory(): Promise<AccountToken | undefined> { const response = await this.callEmpty<grpcTypes.AccountHistory>(this.client.getAccountHistory); - return response.toObject().tokenList; + return response.getToken()?.getValue(); } - public async removeAccountFromHistory(accountToken: AccountToken): Promise<void> { - await this.callString(this.client.removeAccountFromHistory, accountToken); + public async clearAccountHistory(): Promise<void> { + await this.callEmpty(this.client.clearAccountHistory); } public async getCurrentVersion(): Promise<string> { diff --git a/gui/src/main/index.ts b/gui/src/main/index.ts index 2b9bc74a0a..d86a6cba53 100644 --- a/gui/src/main/index.ts +++ b/gui/src/main/index.ts @@ -115,7 +115,7 @@ class ApplicationMain { private quitStage = AppQuitStage.unready; private accountData?: IAccountData = undefined; - private accountHistory: AccountToken[] = []; + private accountHistory?: AccountToken = undefined; private tunnelState: TunnelState = { state: 'disconnected' }; private settings: ISettings = { accountToken: undefined, @@ -665,7 +665,7 @@ class ApplicationMain { return daemonEventListener; } - private setAccountHistory(accountHistory: AccountToken[]) { + private setAccountHistory(accountHistory?: AccountToken) { this.accountHistory = accountHistory; if (this.windowController) { @@ -1128,8 +1128,8 @@ class ApplicationMain { this.daemonRpc.submitVoucher(voucherCode), ); - IpcMainEventChannel.accountHistory.handleRemoveItem(async (token: AccountToken) => { - await this.daemonRpc.removeAccountFromHistory(token); + IpcMainEventChannel.accountHistory.handleClear(async () => { + await this.daemonRpc.clearAccountHistory(); consumePromise(this.updateAccountHistory()); }); @@ -1162,8 +1162,8 @@ class ApplicationMain { const reportPath = this.getProblemReportPath(id); const executable = resolveBin('mullvad-problem-report'); const args = ['collect', '--output', reportPath]; - if (toRedact.length > 0) { - args.push('--redact', ...toRedact); + if (toRedact) { + args.push('--redact', toRedact); } return new Promise((resolve, reject) => { diff --git a/gui/src/renderer/app.tsx b/gui/src/renderer/app.tsx index 7627428059..9fa0e4a456 100644 --- a/gui/src/renderer/app.tsx +++ b/gui/src/renderer/app.tsx @@ -148,7 +148,7 @@ export default class AppRenderer { this.setAccountExpiry(newAccountData && newAccountData.expiry); }); - IpcRendererEventChannel.accountHistory.listen((newAccountHistory: AccountToken[]) => { + IpcRendererEventChannel.accountHistory.listen((newAccountHistory?: AccountToken) => { this.setAccountHistory(newAccountHistory); }); @@ -337,8 +337,8 @@ export default class AppRenderer { return IpcRendererEventChannel.settings.setDnsOptions(dns); } - public removeAccountFromHistory(accountToken: AccountToken): Promise<void> { - return IpcRendererEventChannel.accountHistory.removeItem(accountToken); + public clearAccountHistory(): Promise<void> { + return IpcRendererEventChannel.accountHistory.clear(); } public async openLinkWithAuth(link: string): Promise<void> { @@ -455,7 +455,7 @@ export default class AppRenderer { return IpcRendererEventChannel.splitTunneling.launchApplication(application); } - public collectProblemReport(toRedact: string[]): Promise<string> { + public collectProblemReport(toRedact?: string): Promise<string> { return IpcRendererEventChannel.problemReport.collectLogs(toRedact); } @@ -653,7 +653,7 @@ export default class AppRenderer { } } - private setAccountHistory(accountHistory: AccountToken[]) { + private setAccountHistory(accountHistory?: AccountToken) { this.reduxActions.account.updateAccountHistory(accountHistory); } diff --git a/gui/src/renderer/components/Login.tsx b/gui/src/renderer/components/Login.tsx index 6118f7ac8e..e672094f99 100644 --- a/gui/src/renderer/components/Login.tsx +++ b/gui/src/renderer/components/Login.tsx @@ -36,13 +36,13 @@ import { AriaControlGroup, AriaControlled, AriaControls } from './AriaGroup'; interface IProps { accountToken?: AccountToken; - accountHistory: AccountToken[]; + accountHistory?: AccountToken; loginState: LoginState; openExternalLink: (type: string) => void; login: (accountToken: AccountToken) => void; resetLoginError: () => void; updateAccountToken: (accountToken: AccountToken) => void; - removeAccountTokenFromHistory: (accountToken: AccountToken) => Promise<void>; + clearAccountHistory: () => Promise<void>; createNewAccount: () => void; } @@ -208,7 +208,9 @@ export default class Login extends React.Component<IProps, IState> { } private shouldShowAccountHistory() { - return this.allowInteraction() && this.state.isActive && this.props.accountHistory.length > 0; + return ( + this.allowInteraction() && this.state.isActive && this.props.accountHistory !== undefined + ); } private shouldShowFooter() { @@ -223,13 +225,13 @@ export default class Login extends React.Component<IProps, IState> { this.props.login(accountToken); }; - private onRemoveAccountFromHistory = (accountToken: string) => { - consumePromise(this.removeAccountFromHistory(accountToken)); + private onClearAccountHistory = () => { + consumePromise(this.clearAccountHistory()); }; - private async removeAccountFromHistory(accountToken: AccountToken) { + private async clearAccountHistory() { try { - await this.props.removeAccountTokenFromHistory(accountToken); + await this.props.clearAccountHistory(); // TODO: Remove account from memory } catch (error) { @@ -288,9 +290,9 @@ export default class Login extends React.Component<IProps, IState> { <Accordion expanded={this.shouldShowAccountHistory()}> <StyledAccountDropdownContainer> <AccountDropdown - items={this.props.accountHistory.slice().reverse()} + item={this.props.accountHistory} onSelect={this.onSelectAccountFromHistory} - onRemove={this.onRemoveAccountFromHistory} + onRemove={this.onClearAccountHistory} /> </StyledAccountDropdownContainer> </Accordion> @@ -316,28 +318,24 @@ export default class Login extends React.Component<IProps, IState> { } interface IAccountDropdownProps { - items: AccountToken[]; + item?: AccountToken; onSelect: (value: AccountToken) => void; onRemove: (value: AccountToken) => void; } function AccountDropdown(props: IAccountDropdownProps) { - const uniqueItems = [...new Set(props.items)]; + const token = props.item; + if (!token) { + return null; + } + const label = formatAccountToken(token); return ( - <> - {uniqueItems.map((token) => { - const label = formatAccountToken(token); - return ( - <AccountDropdownItem - key={token} - value={token} - label={label} - onSelect={props.onSelect} - onRemove={props.onRemove} - /> - ); - })} - </> + <AccountDropdownItem + value={token} + label={label} + onSelect={props.onSelect} + onRemove={props.onRemove} + /> ); } diff --git a/gui/src/renderer/components/Support.tsx b/gui/src/renderer/components/Support.tsx index e7960a30c7..d34e6b6b08 100644 --- a/gui/src/renderer/components/Support.tsx +++ b/gui/src/renderer/components/Support.tsx @@ -49,13 +49,13 @@ interface ISupportState { interface ISupportProps { defaultEmail: string; defaultMessage: string; - accountHistory: AccountToken[]; + accountHistory?: AccountToken; isOffline: boolean; onClose: () => void; viewLog: (path: string) => void; saveReportForm: (form: ISupportReportForm) => void; clearReportForm: () => void; - collectProblemReport: (accountsToRedact: string[]) => Promise<string>; + collectProblemReport: (accountToRedact?: string) => Promise<string>; sendProblemReport: (email: string, message: string, savedReportId: string) => Promise<void>; outdatedVersion: boolean; suggestedIsBeta: boolean; diff --git a/gui/src/renderer/containers/LoginPage.tsx b/gui/src/renderer/containers/LoginPage.tsx index f937beb590..a56e6ec04a 100644 --- a/gui/src/renderer/containers/LoginPage.tsx +++ b/gui/src/renderer/containers/LoginPage.tsx @@ -25,7 +25,7 @@ const mapDispatchToProps = (dispatch: ReduxDispatch, props: IAppContext) => { }, openExternalLink: (url: string) => props.app.openUrl(url), updateAccountToken, - removeAccountTokenFromHistory: (token: string) => props.app.removeAccountFromHistory(token), + clearAccountHistory: () => props.app.clearAccountHistory(), createNewAccount: () => consumePromise(props.app.createNewAccount()), }; }; diff --git a/gui/src/renderer/redux/account/actions.ts b/gui/src/renderer/redux/account/actions.ts index 3c0bad3c7b..b8fbe94d39 100644 --- a/gui/src/renderer/redux/account/actions.ts +++ b/gui/src/renderer/redux/account/actions.ts @@ -44,7 +44,7 @@ interface IUpdateAccountTokenAction { interface IUpdateAccountHistoryAction { type: 'UPDATE_ACCOUNT_HISTORY'; - accountHistory: AccountToken[]; + accountHistory?: AccountToken; } interface IUpdateAccountExpiryAction { @@ -125,7 +125,7 @@ function updateAccountToken(token: AccountToken): IUpdateAccountTokenAction { }; } -function updateAccountHistory(accountHistory: AccountToken[]): IUpdateAccountHistoryAction { +function updateAccountHistory(accountHistory?: AccountToken): IUpdateAccountHistoryAction { return { type: 'UPDATE_ACCOUNT_HISTORY', accountHistory, diff --git a/gui/src/renderer/redux/account/reducers.ts b/gui/src/renderer/redux/account/reducers.ts index 8de7b2d07e..53bc55db1b 100644 --- a/gui/src/renderer/redux/account/reducers.ts +++ b/gui/src/renderer/redux/account/reducers.ts @@ -8,14 +8,14 @@ export type LoginState = | { type: 'failed'; method: LoginMethod; error: Error }; export interface IAccountReduxState { accountToken?: AccountToken; - accountHistory: AccountToken[]; + accountHistory?: AccountToken; expiry?: string; // ISO8601 status: LoginState; } const initialState: IAccountReduxState = { accountToken: undefined, - accountHistory: [], + accountHistory: undefined, expiry: undefined, status: { type: 'none' }, }; diff --git a/gui/src/shared/ipc-schema.ts b/gui/src/shared/ipc-schema.ts index 5cc347e51e..01c5c53a95 100644 --- a/gui/src/shared/ipc-schema.ts +++ b/gui/src/shared/ipc-schema.ts @@ -44,7 +44,7 @@ export interface IAppStateSnapshot { isConnected: boolean; autoStart: boolean; accountData?: IAccountData; - accountHistory: AccountToken[]; + accountHistory?: AccountToken; tunnelState: TunnelState; settings: ISettings; location?: ILocation; @@ -165,8 +165,8 @@ export const ipcSchema = { submitVoucher: invoke<string, VoucherResponse>(), }, accountHistory: { - '': notifyRenderer<AccountToken[]>(), - removeItem: invoke<AccountToken, void>(), + '': notifyRenderer<AccountToken | undefined>(), + clear: invoke<void, void>(), }, autoStart: { '': notifyRenderer<boolean>(), @@ -183,7 +183,7 @@ export const ipcSchema = { launchApplication: invoke<ILinuxSplitTunnelingApplication | string, LaunchApplicationResult>(), }, problemReport: { - collectLogs: invoke<string[], string>(), + collectLogs: invoke<string | undefined, string>(), sendReport: invoke<{ email: string; message: string; savedReportId: string }, void>(), viewLog: invoke<string, string>(), }, diff --git a/mullvad-cli/src/cmds/account.rs b/mullvad-cli/src/cmds/account.rs index 803cabde00..af3af0debd 100644 --- a/mullvad-cli/src/cmds/account.rs +++ b/mullvad-cli/src/cmds/account.rs @@ -35,10 +35,6 @@ impl Command for Account { .about("Removes the account number from the settings"), ) .subcommand( - clap::SubCommand::with_name("clear-history") - .about("Clear account history, along with removing all associated keys"), - ) - .subcommand( clap::SubCommand::with_name("create") .about("Creates a new account and sets it as the active one"), ) @@ -75,8 +71,6 @@ impl Command for Account { self.get().await } else if let Some(_matches) = matches.subcommand_matches("unset") { self.set(None).await - } else if let Some(_matches) = matches.subcommand_matches("clear-history") { - self.clear_history().await } else if let Some(_matches) = matches.subcommand_matches("create") { self.create().await } else if let Some(matches) = matches.subcommand_matches("redeem") { @@ -174,11 +168,4 @@ impl Account { let utc = chrono::DateTime::<chrono::Utc>::from_utc(ndt, chrono::Utc); utc.with_timezone(&chrono::Local).to_string() } - - async fn clear_history(&self) -> Result<()> { - let mut rpc = new_rpc_client().await?; - rpc.clear_account_history(()).await?; - println!("Removed account history and all associated keys"); - Ok(()) - } } diff --git a/mullvad-daemon/src/account_history.rs b/mullvad-daemon/src/account_history.rs index 1c5095321f..4b83915ff7 100644 --- a/mullvad-daemon/src/account_history.rs +++ b/mullvad-daemon/src/account_history.rs @@ -1,10 +1,9 @@ -use mullvad_rpc::{rest::MullvadRestHandle, WireguardKeyProxy}; +use crate::settings::SettingsPersister; use mullvad_types::{account::AccountToken, wireguard::WireguardData}; +use regex::Regex; use std::{ - collections::VecDeque, fs, - future::Future, - io::{self, Seek, Write}, + io::{self, Read, Seek, Write}, path::Path, sync::{Arc, Mutex}, }; @@ -29,13 +28,14 @@ pub enum Error { } static ACCOUNT_HISTORY_FILE: &str = "account-history.json"; -static ACCOUNT_HISTORY_LIMIT: usize = 1; -/// A trivial MRU cache of account data pub struct AccountHistory { file: Arc<Mutex<io::BufWriter<fs::File>>>, - accounts: Arc<Mutex<VecDeque<AccountEntry>>>, - rpc_handle: MullvadRestHandle, + token: Option<AccountToken>, +} + +lazy_static::lazy_static! { + static ref ACCOUNT_REGEX: Regex = Regex::new(r"^[0-9]+$").unwrap(); } @@ -43,7 +43,7 @@ impl AccountHistory { pub async fn new( cache_dir: &Path, settings_dir: &Path, - rpc_handle: MullvadRestHandle, + settings: &mut SettingsPersister, ) -> Result<AccountHistory> { Self::migrate_from_old_file_location(cache_dir, settings_dir).await; @@ -60,7 +60,7 @@ impl AccountHistory { options.share_mode(0); } let path = settings_dir.join(ACCOUNT_HISTORY_FILE); - let (file, accounts) = if path.is_file() { + let (file, token) = if path.is_file() { log::info!("Opening account history file in {}", path.display()); let mut reader = options .write(true) @@ -69,24 +69,30 @@ impl AccountHistory { .map(io::BufReader::new) .map_err(Error::Read)?; - let accounts: VecDeque<AccountEntry> = match serde_json::from_reader(&mut reader) { - Err(e) => { - log::warn!( - "{}", - e.display_chain_with_msg("Failed to read+deserialize account history") - ); - Self::try_old_format(&mut reader)? - .into_iter() - .map(|account| AccountEntry { - account, - wireguard: None, - }) - .collect() + let mut buffer = String::new(); + let token: Option<AccountToken> = match reader.read_to_string(&mut buffer) { + Ok(0) => None, + Ok(_) if ACCOUNT_REGEX.is_match(&buffer) => Some(buffer), + Ok(_) | Err(_) => { + log::warn!("Failed to parse account history. Trying old formats",); + match Self::try_format_v2(&mut reader)? { + Some((token, migrated_data)) => { + if let Err(error) = settings.set_wireguard(migrated_data).await { + log::error!( + "{}", + error.display_chain_with_msg( + "Failed to migrate WireGuard key from account history" + ) + ); + } + Some(token) + } + None => Self::try_format_v1(&mut reader)?, + } } - Ok(accounts) => accounts, }; - (reader.into_inner(), accounts) + (reader.into_inner(), token) } else { log::info!("Creating account history file in {}", path.display()); ( @@ -95,14 +101,13 @@ impl AccountHistory { .create(true) .open(path) .map_err(Error::Read)?, - VecDeque::new(), + settings.get_account_token(), ) }; let file = io::BufWriter::new(file); let mut history = AccountHistory { file: Arc::new(Mutex::new(file)), - accounts: Arc::new(Mutex::new(accounts)), - rpc_handle, + token, }; if let Err(e) = history.save_to_disk().await { log::error!("Failed to save account cache after opening it: {}", e); @@ -129,175 +134,67 @@ impl AccountHistory { } } - fn try_old_format(reader: &mut io::BufReader<fs::File>) -> Result<Vec<AccountToken>> { + fn try_format_v1(reader: &mut io::BufReader<fs::File>) -> Result<Option<AccountToken>> { #[derive(Deserialize)] struct OldFormat { accounts: Vec<AccountToken>, } reader.seek(io::SeekFrom::Start(0)).map_err(Error::Read)?; Ok(serde_json::from_reader(reader) - .map(|old_format: OldFormat| old_format.accounts) - .unwrap_or_else(|_| Vec::new())) - } - - /// Gets account data for a certain account id and bumps it's entry to the top of the list if - /// it isn't there already. Returns None if the account entry is not available. - pub async fn get(&mut self, account: &AccountToken) -> Result<Option<AccountEntry>> { - let (idx, entry) = match self - .accounts - .lock() - .unwrap() - .iter() - .enumerate() - .find(|(_idx, entry)| &entry.account == account) - { - Some((idx, entry)) => (idx, entry.clone()), - None => { - return Ok(None); - } - }; - // this account is already on top - if idx == 0 { - return Ok(Some(entry)); - } - self.insert(entry.clone()).await?; - Ok(Some(entry)) + .map(|old_format: OldFormat| old_format.accounts.first().cloned()) + .unwrap_or_else(|_| None)) } - /// Bumps history of an account token. If the account token is not in history, it will be - /// added. - pub async fn bump_history(&mut self, account: &AccountToken) -> Result<()> { - if self.get(account).await?.is_none() { - let new_entry = AccountEntry { - account: account.to_string(), - wireguard: None, - }; - self.insert(new_entry).await?; + fn try_format_v2( + reader: &mut io::BufReader<fs::File>, + ) -> Result<Option<(AccountToken, Option<WireguardData>)>> { + #[derive(Serialize, Deserialize, Clone, Debug)] + pub struct AccountEntry { + pub account: AccountToken, + pub wireguard: Option<WireguardData>, } - Ok(()) - } - - fn create_remove_wg_key_rpc( - &self, - account: &str, - wg_data: &WireguardData, - ) -> impl Future<Output = ()> + 'static { - let mut rpc = WireguardKeyProxy::new(self.rpc_handle.clone()); - let pub_key = wg_data.private_key.public_key(); - let account = String::from(account); - - async move { - if let Err(err) = rpc.remove_wireguard_key(account, &pub_key).await { - log::error!("Failed to remove WireGuard key: {}", err); - } - } - } - - /// Always inserts a new entry at the start of the list - pub async fn insert(&mut self, new_entry: AccountEntry) -> Result<()> { - let mut accounts = self.accounts.lock().unwrap(); - accounts.retain(|entry| entry.account != new_entry.account); - accounts.push_front(new_entry); - - while accounts.len() > ACCOUNT_HISTORY_LIMIT { - let last_entry = accounts.pop_back().unwrap(); - if let Some(wg_data) = last_entry.wireguard { - tokio::spawn(self.create_remove_wg_key_rpc(&last_entry.account, &wg_data)); - } - } - - std::mem::drop(accounts); - self.save_to_disk().await + reader.seek(io::SeekFrom::Start(0)).map_err(Error::Read)?; + Ok(serde_json::from_reader(reader) + .map(|entries: Vec<AccountEntry>| { + entries + .first() + .map(|entry| (entry.account.clone(), entry.wireguard.clone())) + }) + .unwrap_or_else(|_| None)) } - /// Retrieve account history. - pub fn get_account_history(&self) -> Vec<AccountToken> { - self.accounts - .lock() - .unwrap() - .iter() - .map(|entry| entry.account.clone()) - .collect() + /// Gets the account token in the history + pub fn get(&self) -> Option<AccountToken> { + self.token.clone() } - /// Remove account data - pub async fn remove_account(&mut self, account: &str) -> Result<()> { - let entry = self.get(&String::from(account)).await?; - let entry = match entry { - Some(entry) => entry, - None => return Ok(()), - }; - - if let Some(wg_data) = entry.wireguard { - tokio::spawn(self.create_remove_wg_key_rpc(account, &wg_data)); - } - - let _ = self.accounts.lock().unwrap().pop_front(); + /// Replace the account token in the history + pub async fn set(&mut self, new_entry: AccountToken) -> Result<()> { + self.token = Some(new_entry); self.save_to_disk().await } /// Remove account history pub async fn clear(&mut self) -> Result<()> { - log::debug!("account_history::clear"); - - let rpc = WireguardKeyProxy::new(self.rpc_handle.clone()); - - let removal: Vec<_> = self - .accounts - .lock() - .unwrap() - .drain(0..) - .filter_map(move |entry| { - let account = entry.account.clone(); - let mut rpc = rpc.clone(); - entry.wireguard.map(move |wg_data| { - let public_key = wg_data.private_key.public_key(); - async move { - if let Err(err) = rpc.remove_wireguard_key(account, &public_key).await { - log::error!("Failed to remove WireGuard key: {}", err); - } - } - }) - }) - .collect(); - - - futures::future::join_all(removal).await; - - { - let mut accounts = self.accounts.lock().unwrap(); - *accounts = VecDeque::new(); - } + self.token = None; self.save_to_disk().await } async fn save_to_disk(&mut self) -> Result<()> { let file = self.file.clone(); - let accounts = self.accounts.clone(); + let token = self.token.clone(); tokio::task::spawn_blocking(move || { let mut file = file.lock().unwrap(); - let accounts = accounts.lock().unwrap(); - Self::save_to_disk_inner(&mut *file, &*accounts) + file.get_mut().set_len(0).map_err(Error::Write)?; + file.seek(io::SeekFrom::Start(0)).map_err(Error::Write)?; + if let Some(token) = token { + write!(&mut file, "{}", token).map_err(Error::Write)?; + } + file.flush().map_err(Error::Write)?; + file.get_mut().sync_all().map_err(Error::Write) }) .await .map_err(Error::WriteCancelled)? } - - fn save_to_disk_inner( - mut file: &mut io::BufWriter<fs::File>, - accounts: &VecDeque<AccountEntry>, - ) -> Result<()> { - file.get_mut().set_len(0).map_err(Error::Write)?; - file.seek(io::SeekFrom::Start(0)).map_err(Error::Write)?; - serde_json::to_writer_pretty(&mut file, accounts).map_err(Error::Serialize)?; - file.flush().map_err(Error::Write)?; - file.get_mut().sync_all().map_err(Error::Write) - } -} - -#[derive(Serialize, Deserialize, Clone, Debug)] -pub struct AccountEntry { - pub account: AccountToken, - pub wireguard: Option<WireguardData>, } diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 546f06647b..25afafe250 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -15,7 +15,7 @@ mod relays; #[cfg(not(target_os = "android"))] pub mod rpc_uniqueness_check; pub mod runtime; -mod settings; +pub mod settings; pub mod version; mod version_check; @@ -103,13 +103,16 @@ pub enum Error { #[error(display = "REST request failed")] RestError(#[error(source)] mullvad_rpc::rest::Error), - #[error(display = "Unable to load account history with wireguard key cache")] + #[error(display = "Unable to load account history")] LoadAccountHistory(#[error(source)] account_history::Error), #[cfg(target_os = "linux")] #[error(display = "Unable to initialize split tunneling")] InitSplitTunneling(#[error(source)] split_tunnel::Error), + #[error(display = "The account has too many wireguard keys")] + TooManyKeys, + #[error(display = "No wireguard private key available")] NoKeyAvailable, @@ -195,10 +198,8 @@ pub enum DaemonCommand { /// Submit voucher to add time to the current account. Returns time added in seconds SubmitVoucher(ResponseTx<VoucherSubmission, Error>, String), /// Request account history - GetAccountHistory(oneshot::Sender<Vec<AccountToken>>), - /// Request account history - RemoveAccountFromHistory(ResponseTx<(), Error>, AccountToken), - /// Clear account history + GetAccountHistory(oneshot::Sender<Option<AccountToken>>), + /// Remove the last used account, if there is one ClearAccountHistory(ResponseTx<(), Error>), /// Get the list of countries and cities where there are relays. GetRelayLocations(oneshot::Sender<RelayList>), @@ -580,7 +581,7 @@ where ); tokio::spawn(version_updater.run()); let account_history = - account_history::AccountHistory::new(&cache_dir, &settings_dir, rpc_handle.clone()) + account_history::AccountHistory::new(&cache_dir, &settings_dir, &mut settings) .await .map_err(Error::LoadAccountHistory)?; @@ -938,12 +939,7 @@ where &constraints, self.settings.get_bridge_state(), retry_attempt, - self.account_history - .get(&account_token) - .await - .unwrap_or(None) - .and_then(|entry| entry.wireguard) - .is_some(), + self.settings.get_wireguard().is_some(), ) .ok(); if let Some((relay, endpoint)) = endpoint { @@ -1087,13 +1083,7 @@ where let exit_peer = entry_peer.as_ref().map(|_| peer.clone()); let entry_peer = entry_peer.unwrap_or(peer); - let wg_data = self - .account_history - .get(&account_token) - .await - .map_err(Error::AccountHistory)? - .and_then(|entry| entry.wireguard) - .ok_or(Error::NoKeyAvailable)?; + let wg_data = self.settings.get_wireguard().ok_or(Error::NoKeyAvailable)?; let tunnel = wireguard::TunnelConfig { private_key: wg_data.private_key, addresses: vec![ @@ -1156,9 +1146,6 @@ where UpdateRelayLocations => self.on_update_relay_locations().await, SetAccount(tx, account_token) => self.on_set_account(tx, account_token).await, GetAccountHistory(tx) => self.on_get_account_history(tx), - RemoveAccountFromHistory(tx, account_token) => { - self.on_remove_account_from_history(tx, account_token).await - } ClearAccountHistory(tx) => self.on_clear_account_history(tx).await, UpdateRelaySettings(tx, update) => self.on_update_relay_settings(tx, update).await, SetAllowLan(tx, allow_lan) => self.on_set_allow_lan(tx, allow_lan).await, @@ -1225,27 +1212,15 @@ where match result { Ok(data) => { let public_key = data.get_public_key(); - let mut account_entry = self - .account_history - .get(&account) - .await - .ok() - .and_then(|entry| entry) - .unwrap_or_else(|| account_history::AccountEntry { - account: account.clone(), - wireguard: None, - }); - // if no key existed before - let first_key_for_account_on_host = account_entry.wireguard.is_none(); - account_entry.wireguard = Some(data); - match self.account_history.insert(account_entry).await { + let is_first_key = self.settings.get_wireguard().is_none(); + match self.settings.set_wireguard(Some(data)).await { Ok(_) => { if let Some(TunnelType::Wireguard) = self.get_connected_tunnel_type() { self.schedule_reconnect(WG_RECONNECT_DELAY).await; } self.event_listener .notify_key_event(KeygenEvent::NewKey(public_key)); - if first_key_for_account_on_host { + if is_first_key { self.ensure_key_rotation().await; } } @@ -1277,15 +1252,21 @@ where } async fn ensure_key_rotation(&mut self) { - if let Some(token) = self.settings.get_account_token() { - self.wireguard_key_manager - .set_rotation_interval( - &mut self.account_history, - token, - self.settings.tunnel_options.wireguard.rotation_interval, - ) - .await; - } + let token = match self.settings.get_account_token() { + Some(token) => token, + None => return, + }; + let public_key = match self.settings.get_wireguard() { + Some(data) => data.get_public_key(), + None => return, + }; + self.wireguard_key_manager + .set_rotation_interval( + public_key, + token, + self.settings.tunnel_options.wireguard.rotation_interval, + ) + .await; } async fn handle_new_account_event( @@ -1521,6 +1502,7 @@ where &mut self, account_token: Option<String>, ) -> Result<bool, settings::Error> { + let previous_token = self.settings.get_account_token(); let account_changed = self .settings .set_account_token(account_token.clone()) @@ -1529,55 +1511,93 @@ where self.event_listener .notify_settings(self.settings.to_settings()); - // Bump account history if a token was set - if let Some(token) = account_token.clone() { - if let Err(e) = self.account_history.bump_history(&token).await { - log::error!("Failed to bump account history: {}", e); - } + let history_token = match account_token { + Some(token) => token, + None => previous_token.clone().unwrap_or("".to_string()), + }; + if let Err(error) = self.account_history.set(history_token).await { + log::error!( + "{}", + error.display_chain_with_msg("Failed to update account history") + ); } + if let Some(previous_token) = previous_token { + if let Some(previous_key) = self + .settings + .get_wireguard() + .map(|data| data.private_key.public_key()) + { + let remove_key = self + .wireguard_key_manager + .remove_key(previous_token, previous_key); + tokio::spawn(async move { + if let Err(error) = remove_key.await { + log::error!( + "{}", + error.display_chain_with_msg( + "Failed to remove WireGuard key for previous account" + ) + ); + } + }); + } + } + if let Err(error) = self.settings.set_wireguard(None).await { + log::error!( + "{}", + error.display_chain_with_msg("Error resetting WireGuard key") + ); + } self.ensure_wireguard_keys_for_current_account().await; } Ok(account_changed) } - fn on_get_account_history(&mut self, tx: oneshot::Sender<Vec<AccountToken>>) { + fn on_get_account_history(&mut self, tx: oneshot::Sender<Option<AccountToken>>) { Self::oneshot_send( tx, - self.account_history.get_account_history(), + self.account_history.get(), "get_account_history response", ); } - async fn on_remove_account_from_history( - &mut self, - tx: ResponseTx<(), Error>, - account_token: AccountToken, - ) { + async fn on_clear_account_history(&mut self, tx: ResponseTx<(), Error>) { let result = self .account_history - .remove_account(&account_token) + .clear() .await .map_err(Error::AccountHistory); - Self::oneshot_send(tx, result, "remove_account_from_history response"); + Self::oneshot_send(tx, result, "clear_account_history response"); } - async fn on_clear_account_history(&mut self, tx: ResponseTx<(), Error>) { - match self.account_history.clear().await { - Ok(_) => { - self.set_target_state(TargetState::Unsecured).await; - Self::oneshot_send(tx, Ok(()), "clear_account_history response"); + // Remove the key associated with the current account, if there is one. + // This does not modify settings or account history. + #[cfg(not(target_os = "android"))] + fn remove_current_key_rpc(&self) -> impl std::future::Future<Output = Result<(), Error>> { + let remove_key = if let Some(token) = self.settings.get_account_token() { + if let Some(wg_data) = self.settings.get_wireguard() { + Some( + self.wireguard_key_manager + .remove_key(token, wg_data.private_key.public_key()), + ) + } else { + None } - Err(err) => { - log::error!( - "{}", - err.display_chain_with_msg("Failed to clear account history") - ); - Self::oneshot_send( - tx, - Err(Error::AccountHistory(err)), - "clear_account_history response", - ); + } else { + None + }; + + async move { + if let Some(task) = remove_key { + match task.await { + Err(wireguard::Error::RestError(error)) => Err(Error::RestError(error)), + // This result should never occur + Err(wireguard::Error::TooManyKeys) => Err(Error::TooManyKeys), + _ => Ok(()), + } + } else { + Ok(()) } } } @@ -1608,17 +1628,31 @@ where async fn on_factory_reset(&mut self, tx: ResponseTx<(), Error>) { let mut last_error = Ok(()); + let remove_key = self.remove_current_key_rpc(); + tokio::spawn(async move { + if let Err(error) = remove_key.await { + log::error!( + "{}", + error.display_chain_with_msg( + "Failed to remove WireGuard key for previous account" + ) + ); + } + }); + + if let Err(error) = self.account_history.clear().await { + log::error!( + "{}", + error.display_chain_with_msg("Failed to clear account history") + ); + last_error = Err(Error::ClearAccountHistoryError(error)); + } if let Err(e) = self.settings.reset().await { log::error!("Failed to reset settings - {}", e); last_error = Err(Error::ClearSettingsError(e)); } - if let Err(e) = self.account_history.clear().await { - log::error!("Failed to clear account history - {}", e); - last_error = Err(Error::ClearAccountHistoryError(e)); - } - // Shut the daemon down. self.trigger_shutdown_event(); @@ -1969,13 +2003,7 @@ where async fn ensure_wireguard_keys_for_current_account(&mut self) { if let Some(account) = self.settings.get_account_token() { - if self - .account_history - .get(&account) - .await - .map(|entry| entry.map(|e| e.wireguard.is_none()).unwrap_or(true)) - .unwrap_or(true) - { + if self.settings.get_wireguard().is_none() { log::info!("Generating new WireGuard key for account"); self.wireguard_key_manager .spawn_key_generation_task(account, Some(FIRST_KEY_PUSH_TIMEOUT)) @@ -2007,23 +2035,9 @@ where .settings .get_account_token() .ok_or(Error::NoAccountToken)?; + let wireguard_data = self.settings.get_wireguard(); - let mut account_entry = self - .account_history - .get(&account_token) - .await - .map_err(Error::AccountHistory) - .map(|data| { - data.unwrap_or_else(|| { - log::error!("Account token set in settings but not in account history"); - account_history::AccountEntry { - account: account_token.clone(), - wireguard: None, - } - }) - })?; - - let gen_result = match &account_entry.wireguard { + let gen_result = match &wireguard_data { Some(wireguard_data) => { self.wireguard_key_manager .replace_key(account_token.clone(), wireguard_data.get_public_key()) @@ -2039,21 +2053,20 @@ where match gen_result { Ok(new_data) => { let public_key = new_data.get_public_key(); - account_entry.wireguard = Some(new_data); - self.account_history - .insert(account_entry) + self.settings + .set_wireguard(Some(new_data)) .await - .map_err(Error::AccountHistory)?; + .map_err(Error::SettingsError)?; if let Some(TunnelType::Wireguard) = self.get_target_tunnel_type() { self.schedule_reconnect(WG_RECONNECT_DELAY).await; } - let keygen_event = KeygenEvent::NewKey(public_key); + let keygen_event = KeygenEvent::NewKey(public_key.clone()); self.event_listener.notify_key_event(keygen_event.clone()); // update automatic rotation self.wireguard_key_manager .set_rotation_interval( - &mut self.account_history, + public_key, account_token, self.settings.tunnel_options.wireguard.rotation_interval, ) @@ -2067,17 +2080,11 @@ where } async fn on_get_wireguard_key(&mut self, tx: ResponseTx<Option<wireguard::PublicKey>, Error>) { - let token = self.settings.get_account_token(); - let result = if let Some(token) = token { - let entry = self.account_history.get(&token).await; - match entry { - Ok(Some(entry)) => { - let key = entry.wireguard.map(|wg| wg.get_public_key()); - Ok(key) - } - Ok(None) => Err(Error::NoAccountTokenHistory), - Err(error) => Err(Error::AccountHistory(error)), - } + let result = if self.settings.get_account_token().is_some() { + Ok(self + .settings + .get_wireguard() + .map(|data| data.get_public_key())) } else { Err(Error::NoAccountToken) }; @@ -2092,28 +2099,12 @@ where return; } }; - - let key = self - .account_history - .get(&account) - .await - .map(|entry| entry.and_then(|e| e.wireguard.map(|wg| wg.private_key.public_key()))); - - let public_key = match key { - Ok(Some(public_key)) => public_key, - Ok(None) => { + let public_key = match self.settings.get_wireguard() { + Some(wg_data) => wg_data.private_key.public_key(), + None => { Self::oneshot_send(tx, Ok(false), "verify_wireguard_key response"); return; } - Err(e) => { - log::error!("Failed to read key data: {}", e); - Self::oneshot_send( - tx, - Err(Error::AccountHistory(e)), - "verify_wireguard_key response", - ); - return; - } }; let verification_rpc = self diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs index 2e1d8cbc82..2b8bf31a10 100644 --- a/mullvad-daemon/src/management_interface.rs +++ b/mullvad-daemon/src/management_interface.rs @@ -434,7 +434,6 @@ impl ManagementService for ManagementServiceImpl { } async fn get_account_history(&self, _: Request<()>) -> ServiceResult<types::AccountHistory> { - // TODO: this might be a stream log::debug!("get_account_history"); let (tx, rx) = oneshot::channel(); self.send_command_to_daemon(DaemonCommand::GetAccountHistory(tx))?; @@ -443,20 +442,6 @@ impl ManagementService for ManagementServiceImpl { .map(|history| Response::new(types::AccountHistory { token: history })) } - async fn remove_account_from_history( - &self, - request: Request<AccountToken>, - ) -> ServiceResult<()> { - log::debug!("remove_account_from_history"); - let account_token = request.into_inner(); - let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::RemoveAccountFromHistory(tx, account_token))?; - self.wait_for_result(rx) - .await? - .map(Response::new) - .map_err(map_daemon_error) - } - async fn clear_account_history(&self, _: Request<()>) -> ServiceResult<()> { log::debug!("clear_account_history"); let (tx, rx) = oneshot::channel(); @@ -863,7 +848,9 @@ fn map_rest_error(error: RestError) -> Status { /// Converts an instance of [`mullvad_daemon::settings::Error`] into a tonic status. fn map_settings_error(error: settings::Error) -> Status { match error { - settings::Error::DeleteError(..) | settings::Error::WriteError(..) => { + settings::Error::DeleteError(..) + | settings::Error::WriteError(..) + | settings::Error::SetPermissions(..) => { Status::new(Code::FailedPrecondition, error.to_string()) } settings::Error::SerializeError(..) => Status::new(Code::Internal, error.to_string()), diff --git a/mullvad-daemon/src/settings.rs b/mullvad-daemon/src/settings.rs index 7862e21cd6..0b75d0005c 100644 --- a/mullvad-daemon/src/settings.rs +++ b/mullvad-daemon/src/settings.rs @@ -4,14 +4,17 @@ use log::{debug, error, info}; use mullvad_types::{ relay_constraints::{BridgeSettings, BridgeState, RelaySettingsUpdate}, settings::{DnsOptions, Settings}, - wireguard::RotationInterval, + wireguard::{RotationInterval, WireguardData}, }; use std::{ ops::Deref, path::{Path, PathBuf}, }; use talpid_types::ErrorExt; -use tokio::{fs, io}; +use tokio::{ + fs, + io::{self, AsyncWriteExt}, +}; const SETTINGS_FILE: &str = "settings.json"; @@ -28,6 +31,9 @@ pub enum Error { #[error(display = "Unable to write settings to {}", _0)] WriteError(String, #[error(source)] io::Error), + + #[error(display = "Unable to set settings file permissions")] + SetPermissions(#[error(source)] io::Error), } #[derive(err_derive::Error, Debug)] @@ -138,9 +144,44 @@ impl SettingsPersister { debug!("Writing settings to {}", self.path.display()); let buffer = serde_json::to_string_pretty(&self.settings).map_err(Error::SerializeError)?; - fs::write(&self.path, &buffer) + let mut options = fs::OpenOptions::new(); + #[cfg(unix)] + { + use fs::os::unix::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options + .write(true) + .truncate(true) + .open(&self.path) + .await + .map_err(|e| Error::WriteError(self.path.display().to_string(), e))?; + file.write_all(&buffer.into_bytes()) + .await + .map_err(|e| Error::WriteError(self.path.display().to_string(), e))?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut permissions = file + .metadata() + .await + .map_err(Error::SetPermissions)? + .permissions(); + if permissions.mode() & 0o777 != 0o600 { + log::debug!("Updating file permissions"); + permissions.set_mode(0o600); + file.set_permissions(permissions) + .await + .map_err(Error::SetPermissions)?; + } + } + + file.sync_all() .await - .map_err(|e| Error::WriteError(self.path.display().to_string(), e)) + .map_err(|e| Error::WriteError(self.path.display().to_string(), e))?; + + Ok(()) } /// Resets default settings @@ -176,6 +217,11 @@ impl SettingsPersister { self.update(should_save).await } + pub async fn set_wireguard(&mut self, wireguard: Option<WireguardData>) -> Result<bool, Error> { + let should_save = self.settings.set_wireguard(wireguard); + self.update(should_save).await + } + pub async fn update_relay_settings( &mut self, update: RelaySettingsUpdate, diff --git a/mullvad-daemon/src/wireguard.rs b/mullvad-daemon/src/wireguard.rs index b12c61ec56..575ffa261c 100644 --- a/mullvad-daemon/src/wireguard.rs +++ b/mullvad-daemon/src/wireguard.rs @@ -1,4 +1,4 @@ -use crate::{account_history::AccountHistory, DaemonEventSender, InternalDaemonEvent}; +use crate::{DaemonEventSender, InternalDaemonEvent}; use chrono::offset::Utc; use mullvad_rpc::rest::{Error as RestError, MullvadRestHandle}; use mullvad_types::account::AccountToken; @@ -59,38 +59,21 @@ impl KeyManager { /// Reset key rotation, cancelling the current one and starting a new one for the specified /// account - pub async fn reset_rotation( - &mut self, - account_history: &mut AccountHistory, - account_token: AccountToken, - ) { - match account_history - .get(&account_token) + pub async fn reset_rotation(&mut self, current_key: PublicKey, account_token: AccountToken) { + self.run_automatic_rotation(account_token, current_key) .await - .map(|entry| entry.map(|entry| entry.wireguard.map(|wg| wg.get_public_key()))) - { - Ok(Some(Some(public_key))) => { - self.run_automatic_rotation(account_token, public_key).await - } - Ok(Some(None)) => { - log::error!("reset_rotation: failed to obtain public key for account entry.") - } - Ok(None) => log::error!("reset_rotation: account entry not found."), - Err(e) => log::error!("reset_rotation: failed to obtain account entry. {}", e), - }; } /// Update automatic key rotation interval /// Passing `None` for the interval will cause the default value to be used. pub async fn set_rotation_interval( &mut self, - account_history: &mut AccountHistory, + current_key: PublicKey, account_token: AccountToken, auto_rotation_interval: Option<RotationInterval>, ) { self.auto_rotation_interval = auto_rotation_interval.unwrap_or_default(); - - self.reset_rotation(account_history, account_token).await; + self.reset_rotation(current_key, account_token).await; } /// Stop current key generation @@ -143,6 +126,20 @@ impl KeyManager { } } + /// Removes a key from an account + pub fn remove_key( + &self, + account: AccountToken, + key: talpid_types::net::wireguard::PublicKey, + ) -> impl Future<Output = Result<()>> { + let mut rpc = mullvad_rpc::WireguardKeyProxy::new(self.http_handle.clone()); + async move { + rpc.remove_wireguard_key(account, &key) + .await + .map_err(Self::map_rpc_error) + } + } + fn should_retry(error: &RestError) -> bool { if let RestError::ApiError(_status, code) = &error { code != mullvad_rpc::INVALID_ACCOUNT && code != mullvad_rpc::KEY_LIMIT_REACHED @@ -256,7 +253,6 @@ impl KeyManager { Box::new(push_future) } - async fn replace_key_rpc( http_handle: MullvadRestHandle, account: AccountToken, diff --git a/mullvad-jni/src/daemon_interface.rs b/mullvad-jni/src/daemon_interface.rs index 777cee99c2..6f47fe21e8 100644 --- a/mullvad-jni/src/daemon_interface.rs +++ b/mullvad-jni/src/daemon_interface.rs @@ -1,7 +1,7 @@ use futures::{channel::oneshot, executor::block_on}; use mullvad_daemon::{DaemonCommand, DaemonCommandSender}; use mullvad_types::{ - account::{AccountData, VoucherSubmission}, + account::{AccountData, AccountToken, VoucherSubmission}, location::GeoIpLocation, relay_constraints::RelaySettingsUpdate, relay_list::RelayList, @@ -99,7 +99,7 @@ impl DaemonInterface { .map_err(Error::RpcError) } - pub fn get_account_history(&self) -> Result<Vec<String>> { + pub fn get_account_history(&self) -> Result<Option<AccountToken>> { let (tx, rx) = oneshot::channel(); self.send_command(DaemonCommand::GetAccountHistory(tx))?; @@ -175,10 +175,10 @@ impl DaemonInterface { Ok(()) } - pub fn remove_account_from_history(&self, account_token: String) -> Result<()> { + pub fn clear_account_history(&self) -> Result<()> { let (tx, rx) = oneshot::channel(); - self.send_command(DaemonCommand::RemoveAccountFromHistory(tx, account_token))?; + self.send_command(DaemonCommand::ClearAccountHistory(tx))?; block_on(rx) .map_err(|_| Error::NoResponse)? diff --git a/mullvad-jni/src/lib.rs b/mullvad-jni/src/lib.rs index 32d89a84cf..38c5e54423 100644 --- a/mullvad-jni/src/lib.rs +++ b/mullvad-jni/src/lib.rs @@ -510,13 +510,7 @@ pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_getAcco Some(daemon_interface) => daemon_interface .get_account_history() .map(|history| history.into_java(&env).forget()) - .unwrap_or_else(|err| { - log::error!( - "{}", - err.display_chain_with_msg("Failed to get account history") - ); - JObject::null() - }), + .unwrap_or(JObject::null()), None => JObject::null(), } } @@ -758,21 +752,16 @@ pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_reconne #[no_mangle] #[allow(non_snake_case)] -pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_removeAccountFromHistory( - env: JNIEnv<'_>, +pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_clearAccountHistory( + _: JNIEnv<'_>, _: JObject<'_>, daemon_interface_address: jlong, - accountToken: JString<'_>, ) { - let env = JnixEnv::from(env); - if let Some(daemon_interface) = get_daemon_interface(daemon_interface_address) { - let account = String::from_java(&env, accountToken); - - if let Err(error) = daemon_interface.remove_account_from_history(account) { + if let Err(error) = daemon_interface.clear_account_history() { log::error!( "{}", - error.display_chain_with_msg("Failed to remove account from history") + error.display_chain_with_msg("Failed to clear account history") ); } } diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index faa5a73ffd..636148c7d6 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -47,7 +47,6 @@ service ManagementService { rpc SetAccount(google.protobuf.StringValue) returns (google.protobuf.Empty) {} rpc GetAccountData(google.protobuf.StringValue) returns (AccountData) {} rpc GetAccountHistory(google.protobuf.Empty) returns (AccountHistory) {} - rpc RemoveAccountFromHistory(google.protobuf.StringValue) returns (google.protobuf.Empty) {} rpc ClearAccountHistory(google.protobuf.Empty) returns (google.protobuf.Empty) {} rpc GetWwwAuthToken(google.protobuf.Empty) returns (google.protobuf.StringValue) {} rpc SubmitVoucher(google.protobuf.StringValue) returns (VoucherSubmission) {} @@ -77,6 +76,10 @@ message AccountData { google.protobuf.Timestamp expiry = 1; } +message AccountHistory { + google.protobuf.StringValue token = 1; +} + message VoucherSubmission { uint64 seconds_added = 1; google.protobuf.Timestamp new_expiry = 2; @@ -201,10 +204,6 @@ message GeoIpLocation { string bridge_hostname = 9; } -message AccountHistory { - repeated string token = 1; -} - message BridgeSettings { message BridgeConstraints { RelayLocation location = 1; diff --git a/mullvad-setup/src/main.rs b/mullvad-setup/src/main.rs index 45186dd1a7..5f88e3456f 100644 --- a/mullvad-setup/src/main.rs +++ b/mullvad-setup/src/main.rs @@ -1,5 +1,4 @@ use clap::{crate_authors, crate_description, crate_name, SubCommand}; -use mullvad_daemon::account_history; use mullvad_management_interface::new_rpc_client; use mullvad_rpc::MullvadRpcRuntime; use mullvad_types::version::ParsedAppVersion; @@ -58,17 +57,17 @@ pub enum Error { #[error(display = "Failed to initialize mullvad RPC runtime")] RpcInitializationError(#[error(source)] mullvad_rpc::Error), + #[error(display = "Failed to remove WireGuard key for account")] + RemoveKeyError(#[error(source)] mullvad_rpc::rest::Error), + #[error(display = "Failed to obtain settings directory path")] SettingsPathError(#[error(source)] SettingsPathErrorType), #[error(display = "Failed to obtain cache directory path")] CachePathError(#[error(source)] mullvad_paths::Error), - #[error(display = "Failed to initialize account history")] - InitializeAccountHistoryError(#[error(source)] account_history::Error), - - #[error(display = "Failed to initialize account history")] - ClearAccountHistoryError(#[error(source)] account_history::Error), + #[error(display = "Failed to update the settings")] + SettingsError(#[error(source)] mullvad_daemon::settings::Error), #[error(display = "Cannot parse the version string")] ParseVersionStringError, @@ -83,7 +82,8 @@ async fn main() { .about("Move a running daemon into a blocking state and save its target state"), SubCommand::with_name("reset-firewall") .about("Remove any firewall rules introduced by the daemon"), - SubCommand::with_name("clear-history").about("Clear account history"), + SubCommand::with_name("remove-wireguard-key") + .about("Removes the WireGuard key from the active account"), SubCommand::with_name("is-older-version") .about("Checks whether the given version is older than the current version") .arg( @@ -108,7 +108,7 @@ async fn main() { let result = match matches.subcommand() { ("prepare-restart", _) => prepare_restart().await, ("reset-firewall", _) => reset_firewall().await, - ("clear-history", _) => clear_history().await, + ("remove-wireguard-key", _) => remove_wireguard_key().await, ("is-older-version", Some(sub_matches)) => { let old_version = sub_matches.value_of("OLDVERSION").unwrap(); match is_older_version(old_version).await { @@ -161,30 +161,34 @@ async fn reset_firewall() -> Result<(), Error> { firewall.reset_policy().map_err(Error::FirewallError) } -async fn clear_history() -> Result<(), Error> { +async fn remove_wireguard_key() -> Result<(), Error> { let (cache_path, settings_path) = get_paths()?; + let mut settings = mullvad_daemon::settings::SettingsPersister::load(&settings_path).await; - let mut rpc_runtime = MullvadRpcRuntime::with_cache( - tokio::runtime::Handle::current(), - None, - &cache_path, - false, - |_| Ok(()), - ) - .await - .map_err(Error::RpcInitializationError)?; + if let Some(token) = settings.get_account_token() { + if let Some(wg_data) = settings.get_wireguard() { + let mut rpc_runtime = MullvadRpcRuntime::with_cache( + tokio::runtime::Handle::current(), + None, + &cache_path, + false, + |_| Ok(()), + ) + .await + .map_err(Error::RpcInitializationError)?; + let mut key_proxy = + mullvad_rpc::WireguardKeyProxy::new(rpc_runtime.mullvad_rest_handle()); + key_proxy + .remove_wireguard_key(token, &wg_data.private_key.public_key()) + .await + .map_err(Error::RemoveKeyError)?; + settings + .set_wireguard(None) + .await + .map_err(Error::SettingsError)?; + } + } - let mut account_history = account_history::AccountHistory::new( - &cache_path, - &settings_path, - rpc_runtime.mullvad_rest_handle(), - ) - .await - .map_err(Error::InitializeAccountHistoryError)?; - account_history - .clear() - .await - .map_err(Error::ClearAccountHistoryError)?; Ok(()) } diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs index 0430f530ea..faddb9ad0c 100644 --- a/mullvad-types/src/settings/mod.rs +++ b/mullvad-types/src/settings/mod.rs @@ -38,6 +38,8 @@ pub enum Error { #[cfg_attr(target_os = "android", jnix(package = "net.mullvad.mullvadvpn.model"))] pub struct Settings { account_token: Option<String>, + #[cfg_attr(target_os = "android", jnix(skip))] + wireguard: Option<wireguard::WireguardData>, relay_settings: RelaySettings, #[cfg_attr(target_os = "android", jnix(skip))] pub bridge_settings: BridgeSettings, @@ -65,6 +67,7 @@ impl Default for Settings { fn default() -> Self { Settings { account_token: None, + wireguard: None, relay_settings: RelaySettings::Normal(RelayConstraints { location: Constraint::Only(LocationConstraint::Country("se".to_owned())), ..Default::default() @@ -120,6 +123,19 @@ impl Settings { } } + pub fn get_wireguard(&self) -> Option<wireguard::WireguardData> { + self.wireguard.clone() + } + + pub fn set_wireguard(&mut self, wireguard: Option<wireguard::WireguardData>) -> bool { + if wireguard != self.wireguard { + self.wireguard = wireguard; + true + } else { + false + } + } + pub fn get_relay_settings(&self) -> RelaySettings { self.relay_settings.clone() } diff --git a/mullvad-types/src/wireguard.rs b/mullvad-types/src/wireguard.rs index cbe05363ec..2991eb1a1d 100644 --- a/mullvad-types/src/wireguard.rs +++ b/mullvad-types/src/wireguard.rs @@ -14,7 +14,7 @@ pub const DEFAULT_ROTATION_INTERVAL: Duration = if cfg!(target_os = "android") { }; /// Contains account specific wireguard data -#[derive(Serialize, Deserialize, Clone, Debug)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] pub struct WireguardData { pub private_key: wireguard::PrivateKey, pub addresses: AssociatedAddresses, @@ -140,7 +140,7 @@ pub struct PublicKey { /// Contains a pair of local link addresses that are paired with a specific wireguard /// public/private keypair. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] pub struct AssociatedAddresses { pub ipv4_address: ipnetwork::Ipv4Network, pub ipv6_address: ipnetwork::Ipv6Network, |
