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.

Sign up for GitHub

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

[fix/bypass-lock-prevention] Bypass Passcode Prevention #1324

Closed
wants to merge 6 commits into from
Closed

[fix/bypass-lock-prevention] Bypass Passcode Prevention #1324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3f1c16
Prevent the user from changing the device time to bypass the lock pre…
hosy Feb 19, 2024
5bf4d2a
changed privacy reason and removed unneeded key/value pairs
hosy Feb 20, 2024
4e37782
maybe keys are needed
hosy Feb 20, 2024
853e803
fixed code review findings
hosy Mar 5, 2024
b21c0b6
Converted lockUntilDate to lockDelay. This is a better prevention if …
hosy Mar 15, 2024
5840f4d
converted code to significant time change notification
hosy Apr 16, 2024
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
4 changes: 4 additions & 0 deletions ownCloud/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate {

return true
}

func applicationSignificantTimeChange(_ application: UIApplication) {
AppLockManager.shared.significantTimeChangeOccurred()
}

func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey : Any] = [:]) -> Bool {
if !OCAuthenticationBrowserSessionCustomScheme.handleOpen(url), // No custom scheme URL handling for this URL
Expand Down
80 changes: 47 additions & 33 deletions ownCloudAppShared/AppLock/AppLockManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,22 @@ public class AppLockManager: NSObject {
self.userDefaults.set(newValue, forKey: "applock-failed-passcode-attempts")
}
}

private var lockedUntilDate: Date? {
get {
return userDefaults.object(forKey: "applock-locked-until-date") as? Date
}
set(newValue) {
self.userDefaults.set(newValue, forKey: "applock-locked-until-date")
}
}
private var lockDelay: Double? {
get {
return userDefaults.object(forKey: "applock-delay") as? Double
return userDefaults.object(forKey: "applock-lock-delay") as? Double
}
set(newValue) {
self.userDefaults.set(newValue, forKey: "applock-delay")
self.userDefaults.set(newValue, forKey: "applock-lock-delay")
}
}

private var biometricalAuthenticationSucceeded: Bool {
get {
return userDefaults.bool(forKey: "applock-biometrical-authentication-succeeded")
Expand Down Expand Up @@ -153,11 +159,13 @@ public class AppLockManager: NSObject {
userDefaults = OCAppIdentity.shared.userDefaults!

super.init()
significantTimeChangeOccurred()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By handling every initialization of the AppLockManager as a significant time change and resetting the countdown, users may be hit without trying to bypass the lock timer, f.ex. during regular usage like:

  • invoking the share extension from a share sheet
  • iOS quits the app in the background to free memory for apps launched after it
  • the user closes the app

Adding UIApplication.significantTimeChangeNotification as a signal to the mix of detecting a bypass attempt is a good idea, but if the notification is not delivered to the app and/or app extensions when they aren't running at the time the significant time change occurs, it can't be relied on alone.

Resetting the timer every time the AppLockManager is initialized in a new process closes the gaps that a UIApplication.significantTimeChangeNotification can leave, but also has negative effects on legitimate usage, unfortunately, which I think we should avoid.

Why not use ProcessInfo.processInfo.systemUptime as outlined above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felix-schwarz please go ahead if by implementing the prevention using ProcessInfo.processInfo.systemUptime if you think it is a better solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I implemented a solution based on ProcessInfo.processInfo.systemUptime at #1347 . If it passes testing on a real device, I'd suggest to close this PR and proceed with #1347 instead.


if AppLockManager.supportedOnDevice {
NotificationCenter.default.addObserver(self, selector: #selector(self.appDidEnterBackground), name: UIApplication.didEnterBackgroundNotification, object: nil)
NotificationCenter.default.addObserver(self, selector: #selector(self.appWillEnterForeground), name: UIApplication.willEnterForegroundNotification, object: nil)
NotificationCenter.default.addObserver(self, selector: #selector(self.updateLockscreens), name: ThemeWindow.themeWindowListChangedNotification, object: nil)
NotificationCenter.default.addObserver(self, selector: #selector(significantTimeChangeOccurred), name: UIApplication.significantTimeChangeNotification, object: nil)
}
}

Expand All @@ -166,6 +174,7 @@ public class AppLockManager: NSObject {
NotificationCenter.default.removeObserver(self, name: UIApplication.didEnterBackgroundNotification, object: nil)
NotificationCenter.default.removeObserver(self, name: UIApplication.willEnterForegroundNotification, object: nil)
NotificationCenter.default.removeObserver(self, name: ThemeWindow.themeWindowListChangedNotification, object: nil)
NotificationCenter.default.removeObserver(self, name: UIApplication.significantTimeChangeNotification, object: nil)
}
}

Expand All @@ -175,19 +184,19 @@ public class AppLockManager: NSObject {
lockscreenOpenForced = forceShow
lockscreenOpen = true

// The following code needs to be executed after a short delay, because in the share sheet the biometrical unlock UI can block adding the PasscodeViewController UI
var delay = 0.0
if self.passwordViewHostViewController != nil {
delay = 0.5
}
OnMainThread(after: delay) {
// Show biometrical
if !forceShow, !self.shouldDisplayCountdown, self.biometricalAuthenticationSucceeded {
self.showBiometricalAuthenticationInterface(context: context)
} else if setupMode {
self.showBiometricalAuthenticationInterface(context: context)
}
}
// The following code needs to be executed after a short delay, because in the share sheet the biometrical unlock UI can block adding the PasscodeViewController UI
var delay = 0.0
if self.passwordViewHostViewController != nil {
delay = 0.5
}
OnMainThread(after: delay) {
// Show biometrical
if !forceShow, !self.shouldDisplayCountdown, self.biometricalAuthenticationSucceeded {
self.showBiometricalAuthenticationInterface(context: context)
} else if setupMode {
self.showBiometricalAuthenticationInterface(context: context)
}
}
} else {
dismissLockscreen(animated: true)
}
Expand Down Expand Up @@ -357,14 +366,20 @@ public class AppLockManager: NSObject {
dismissLockscreen(animated: false)
}
}

@objc public func significantTimeChangeOccurred() {
if let lockDelay = lockDelay {
lockedUntilDate = Date().addingTimeInterval(lockDelay)
}
}

// MARK: - Unlock
func attemptUnlock(with testPasscode: String?, customErrorMessage: String? = nil, passcodeViewController: PasscodeViewController? = nil) {
if testPasscode == self.passcode {
unlocked = true
failedPasscodeAttempts = 0
lockedUntilDate = nil
lockDelay = nil

dismissLockscreen(animated: true)
} else {
unlocked = false
Expand All @@ -375,6 +390,8 @@ public class AppLockManager: NSObject {

if self.failedPasscodeAttempts >= self.maximumPasscodeAttempts {
let delayUntilNextAttempt = pow(powBaseDelay, Double(failedPasscodeAttempts))

lockedUntilDate = Date().addingTimeInterval(delayUntilNextAttempt)
lockDelay = delayUntilNextAttempt
startLockCountdown()
}
Expand Down Expand Up @@ -421,8 +438,8 @@ public class AppLockManager: NSObject {
}

private var shouldDisplayCountdown : Bool {
if let lockDelay = self.lockDelay, lockDelay > 0 {
return true
if let startLockBeforeDate = self.lockedUntilDate {
return startLockBeforeDate > Date()
}

return false
Expand All @@ -444,29 +461,26 @@ public class AppLockManager: NSObject {
}

@objc private func updateLockCountdown() {
if let lockDelay = self.lockDelay {
let hours = Int(lockDelay) / 3600
let minutes = Int(lockDelay) / 60 % 60
let seconds = Int(lockDelay) % 60

if lockDelay > 0 {
self.lockDelay = (lockDelay - 1)
}

if let date = self.lockedUntilDate {
let interval = Int(date.timeIntervalSinceNow)
let seconds = interval % 60
let minutes = (interval / 60) % 60
let hours = (interval / 3600)

let dateFormatted:String?
if hours > 0 {
dateFormatted = String(format: "%02d:%02d:%02d", hours, minutes, seconds)
} else {
dateFormatted = String(format: "%02d:%02d", minutes, seconds)
}

let timeoutMessage:String = NSString(format: "Please try again in %@".localized as NSString, dateFormatted!) as String

performPasscodeViewControllerUpdates { (passcodeViewController) in
passcodeViewController.timeoutMessage = timeoutMessage
}
if lockDelay <= 0 {

if date <= Date() {
// Time elapsed, allow entering passcode again
self.lockTimer?.invalidate()
performPasscodeViewControllerUpdates { (passcodeViewController) in
Expand Down
Loading