Skip to content

Commit

Permalink
Bug/fix for lock bruteforce (trustwallet#878)
Browse files Browse the repository at this point in the history
* Add UI fix for lock. Add fix for bruteforcing lock screen.

* Remove responsibility of displaying and hiding keyboard of LockController from Coordinators. Adjust wrok of lock screen.

* Add tests

* Adjust tests for LockEnterPasscodeViewController

* Remove optional type
  • Loading branch information
Taras Pasichnyk authored and vikmeup committed Aug 21, 2018
1 parent 0c716fd commit ec5c584
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 18 deletions.
12 changes: 12 additions & 0 deletions Trust.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@
61FC5ED11FCFBDEB00CCB12A /* EtherNumberFormatterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61FC5ED01FCFBDEB00CCB12A /* EtherNumberFormatterTests.swift */; };
664D11A12007D59F0041A0B0 /* EstimateGasRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 664D11A02007D59F0041A0B0 /* EstimateGasRequest.swift */; };
71A433702113860900985ADC /* ConfirmPaymentDetailsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 71A4336F2113860900985ADC /* ConfirmPaymentDetailsViewModelTests.swift */; };
71B1B5312114570A00EF23E5 /* LockEnterPasscodeViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 71B1B5302114570A00EF23E5 /* LockEnterPasscodeViewControllerTests.swift */; };
7301BA9220A3117000E1AFE5 /* AutoLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7301BA9120A3117000E1AFE5 /* AutoLock.swift */; };
7301BA9620AB1E5600E1AFE5 /* CookiesStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7301BA9520AB1E5600E1AFE5 /* CookiesStore.swift */; };
7302405C204C65DF00B327DF /* CollectibleTokenCategory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7302405B204C65DF00B327DF /* CollectibleTokenCategory.swift */; };
Expand Down Expand Up @@ -748,6 +749,7 @@
646C8C822C986358D7388602 /* Pods_Trust.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Trust.framework; sourceTree = BUILT_PRODUCTS_DIR; };
664D11A02007D59F0041A0B0 /* EstimateGasRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EstimateGasRequest.swift; sourceTree = "<group>"; };
71A4336F2113860900985ADC /* ConfirmPaymentDetailsViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfirmPaymentDetailsViewModelTests.swift; sourceTree = "<group>"; };
71B1B5302114570A00EF23E5 /* LockEnterPasscodeViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LockEnterPasscodeViewControllerTests.swift; sourceTree = "<group>"; };
7301BA9120A3117000E1AFE5 /* AutoLock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutoLock.swift; sourceTree = "<group>"; };
7301BA9520AB1E5600E1AFE5 /* CookiesStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CookiesStore.swift; sourceTree = "<group>"; };
7302405B204C65DF00B327DF /* CollectibleTokenCategory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CollectibleTokenCategory.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2009,6 +2011,14 @@
path = Views;
sourceTree = "<group>";
};
71B1B52F211456F700EF23E5 /* ViewControllers */ = {
isa = PBXGroup;
children = (
71B1B5302114570A00EF23E5 /* LockEnterPasscodeViewControllerTests.swift */,
);
path = ViewControllers;
sourceTree = "<group>";
};
73200B652044B21100118A82 /* Storage */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -2244,6 +2254,7 @@
772C6DBC210A8B8100FCE4F1 /* Lock */ = {
isa = PBXGroup;
children = (
71B1B52F211456F700EF23E5 /* ViewControllers */,
772C6DBD210A8B8600FCE4F1 /* Coordinators */,
);
path = Lock;
Expand Down Expand Up @@ -3247,6 +3258,7 @@
files = (
771A8480203240BB00528D28 /* PreferencesControllerTests.swift in Sources */,
29F1C85620036887003780D8 /* AppTrackerTests.swift in Sources */,
71B1B5312114570A00EF23E5 /* LockEnterPasscodeViewControllerTests.swift in Sources */,
29BDF19F1FEE51650023A45F /* GasLimitConfigurationTests.swift in Sources */,
73200B642040B9D700118A82 /* CryptoAddressValidatorTest.swift in Sources */,
733AFC7C210907DE00C0AE35 /* GasViewModelTest.swift in Sources */,
Expand Down
2 changes: 0 additions & 2 deletions Trust/Lock/Coordinators/AuthenticateUserCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ final class AuthenticateUserCoordinator: Coordinator {
}

func showAuthentication() {
lockEnterPasscodeViewController.showKeyboard()
lockEnterPasscodeViewController.showBioMerickAuth()
lockEnterPasscodeViewController.cleanUserInput()
}

Expand Down
2 changes: 0 additions & 2 deletions Trust/Lock/Coordinators/LockEnterPasscodeCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ final class LockEnterPasscodeCoordinator: Coordinator {
return
}

lockEnterPasscodeViewController.showKeyboard()
lockEnterPasscodeViewController.showBioMerickAuth()
lockEnterPasscodeViewController.cleanUserInput()
}

Expand Down
15 changes: 10 additions & 5 deletions Trust/Lock/Lock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ final class Lock: LockInterface {
private let maxAttemptTime = "maxAttemptTime"
private let autoLockType = "autoLockType"
private let autoLockTime = "autoLockTime"
private let keychain = KeychainSwift(keyPrefix: Constants.keychainKeyPrefix)
private let keychain: KeychainSwift

init(keychain: KeychainSwift = KeychainSwift(keyPrefix: Constants.keychainKeyPrefix)) {
self.keychain = keychain
}

func shouldShowProtection() -> Bool {
return isPasscodeSet() && autoLockTriggered()
Expand Down Expand Up @@ -90,10 +94,11 @@ final class Lock: LockInterface {
keychain.set(String(numberOfAttemptsSoFar), forKey: passcodeAttempts)
}

func recordedMaxAttemptTime() -> Date {
//This method is called only when we knew that maxAttemptTime is set. So no worries with !.
let timeString = keychain.get(maxAttemptTime)!
return dateFormatter().date(from: timeString)!
func recordedMaxAttemptTime() -> Date? {
guard let timeString = keychain.get(maxAttemptTime) else {
return nil
}
return dateFormatter().date(from: timeString)
}

func incorrectMaxAttemptTimeIsSet() -> Bool {
Expand Down
14 changes: 10 additions & 4 deletions Trust/Lock/ViewControllers/LockEnterPasscodeViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ final class LockEnterPasscodeViewController: LockPasscodeViewController {
if lock.incorrectMaxAttemptTimeIsSet() {
lockView.lockTitle.text = lockEnterPasscodeViewModel.tryAfterOneMinute
maxAttemptTimerValidation()
} else {
showBiometricAuth()
}
}
func showBioMerickAuth() {
private func showBiometricAuth() {
self.context = LAContext()
self.touchValidation()
}
Expand Down Expand Up @@ -54,8 +56,13 @@ final class LockEnterPasscodeViewController: LockPasscodeViewController {
self.hideKeyboard()
}
private func maxAttemptTimerValidation() {
// if there is no recordedMaxAttemptTime user has logged successfuly previous time
guard let maxAttemptTimer = lock.recordedMaxAttemptTime() else {
self.lockView.lockTitle.text = lockEnterPasscodeViewModel.initialLabelText
self.showKeyboard()
return
}
let now = Date()
let maxAttemptTimer = lock.recordedMaxAttemptTime()
let interval = now.timeIntervalSince(maxAttemptTimer)
//if interval is greater or equal 60 seconds we give 1 attempt.
if interval >= 60 {
Expand All @@ -71,7 +78,6 @@ final class LockEnterPasscodeViewController: LockPasscodeViewController {
if let unlock = unlockWithResult {
unlock(success, bioUnlock)
}

}
private func canEvaluatePolicy() -> Bool {
return context.canEvaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, error: nil)
Expand All @@ -90,7 +96,7 @@ final class LockEnterPasscodeViewController: LockPasscodeViewController {
self.lockView.lockTitle.text = self.lockEnterPasscodeViewModel.initialLabelText
self.unlock(withResult: true, bioUnlock: true)
} else {
self.showKeyboard()
self.maxAttemptTimerValidation()
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions Trust/Lock/ViewControllers/LockPasscodeViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ class LockPasscodeViewController: UIViewController {
var willFinishWithResult: ((_ success: Bool) -> Void)?
let model: LockViewModel
var lockView: LockView!
let lock = Lock()
let lock: Lock
private var invisiblePasscodeField = UITextField()
private var shouldIgnoreTextFieldDelegateCalls = false
init(model: LockViewModel) {
init(model: LockViewModel, lock: Lock = Lock()) {
self.model = model
self.lock = lock
super.init(nibName: nil, bundle: nil)
}
override func viewDidLoad() {
Expand All @@ -29,7 +30,7 @@ class LockPasscodeViewController: UIViewController {
invisiblePasscodeField.resignFirstResponder()
}
}
private func configureInvisiblePasscodeField() {
public func configureInvisiblePasscodeField() {
invisiblePasscodeField = UITextField()
invisiblePasscodeField.keyboardType = .numberPad
invisiblePasscodeField.isSecureTextEntry = true
Expand All @@ -38,7 +39,7 @@ class LockPasscodeViewController: UIViewController {
view.addSubview(invisiblePasscodeField)
}

private func configureLockView() {
public func configureLockView() {
lockView = LockView(model)
lockView.translatesAutoresizingMaskIntoConstraints = false
view.addSubview(lockView)
Expand Down
5 changes: 4 additions & 1 deletion Trust/Lock/ViewModels/LockViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import UIKit

class LockViewModel {
private let lock = Lock()
private let lock: Lock
init(lock: Lock = Lock()) {
self.lock = lock
}
func charCount() -> Int {
//This step is required for old clients to support 4 digit passcode.
var count = 0
Expand Down
1 change: 1 addition & 0 deletions Trust/Lock/Views/LockView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ final class LockView: UIView {
lockTitle.font = UIFont.systemFont(ofSize: 19)
lockTitle.textAlignment = .center
lockTitle.translatesAutoresizingMaskIntoConstraints = false
lockTitle.numberOfLines = 0
}
private func applyConstraints() {
characterView.centerXAnchor.constraint(equalTo: self.centerXAnchor).isActive = true
Expand Down
1 change: 1 addition & 0 deletions Trust/Settings/Types/Constants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public struct Constants {
public static let changellyRefferalID = "968d4f0f0bf9"
//
public static let keychainKeyPrefix = "trustwallet"
public static let keychainTestsKeyPrefix = "trustwallet-tests"

// social
public static let website = "https://trustwalletapp.com"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright DApps Platform Inc. All rights reserved.

import XCTest
import KeychainSwift
@testable import Trust

class LockEnterPasscodeViewControllerTests: XCTestCase {

let passcode = "123456"
let incorrectPasscode = "111111"

func testCorrectPasscodeInput() {
let lock = Lock(keychain: KeychainSwift(keyPrefix: Constants.keychainTestsKeyPrefix))
let lockViewModel = LockEnterPasscodeViewModel(lock: lock)
let vc = LockEnterPasscodeViewController(model: lockViewModel, lock: lock)
vc.configureLockView()
vc.configureInvisiblePasscodeField()

lock.setPasscode(passcode: passcode)

vc.enteredPasscode(passcode)

XCTAssertTrue(lock.numberOfAttempts() == 0)
}

func testIncorrectPasscodeInput() {
let lock = Lock(keychain: KeychainSwift(keyPrefix: Constants.keychainTestsKeyPrefix))
let lockViewModel = LockEnterPasscodeViewModel(lock: lock)
let vc = LockEnterPasscodeViewController(model: lockViewModel, lock: lock)
vc.configureLockView()
vc.configureInvisiblePasscodeField()

lock.setPasscode(passcode: passcode)

vc.enteredPasscode(incorrectPasscode)

XCTAssertTrue(lock.numberOfAttempts() > 0)
}

func testIncorrectPasscodeLimitInput() {
let lock = Lock(keychain: KeychainSwift(keyPrefix: Constants.keychainTestsKeyPrefix))

XCTAssertFalse(lock.incorrectMaxAttemptTimeIsSet())

let lockViewModel = LockEnterPasscodeViewModel(lock: lock)
let vc = LockEnterPasscodeViewController(model: lockViewModel, lock: lock)
vc.configureLockView()
vc.configureInvisiblePasscodeField()

lock.setPasscode(passcode: passcode)

while lock.numberOfAttempts() <= lockViewModel.passcodeAttemptLimit() {
vc.enteredPasscode(incorrectPasscode)
}

XCTAssertTrue(lock.incorrectMaxAttemptTimeIsSet())
}

}

0 comments on commit ec5c584

Please sign in to comment.