Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #1359, Fix #1153, Fix #1347: Improved modals for security keys (#…
Browse files Browse the repository at this point in the history 
…1362)
  • Loading branch information
@jumde @jhreis
jumde authored and jhreis committed Aug 12, 2019
1 parent 435b67a commit 7e5a281
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 66 deletions.
8 changes: 8 additions & 0 deletions Client/Frontend/Popup/AlertPopupView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ class AlertPopupView: PopupView {
setDialogColor(color: BraveUX.PopupDialogColorLight)
}

func update(title: String) {
titleLabel.text = title
}

func clearTextField() {
textField?.text = nil
}

func updateSubviews() {
titleLabel.adjustsFontSizeToFitWidth = false
messageLabel.adjustsFontSizeToFitWidth = false
Expand Down
181 changes: 117 additions & 64 deletions Client/U2FExtensions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,13 @@ class U2FExtensions: NSObject {
fileprivate var fidoSignRequests: [Int: [FIDOSignRequest]] = [:]

fileprivate static var observationContext = 0
fileprivate var popup: AlertPopupView

// Popup modals presented to the user in different session or key states
fileprivate let touchKeyPopup = AlertPopupView(image: #imageLiteral(resourceName: "browser_lock_popup"), title: Strings.keyTitle, message: Strings.touchKeyMessage)
fileprivate let insertKeyPopup = AlertPopupView(image: #imageLiteral(resourceName: "browser_lock_popup"), title: Strings.keyTitle, message: Strings.insertKeyMessage)
fileprivate var pinVerificationPopup = AlertPopupView(image: #imageLiteral(resourceName: "browser_lock_popup"), title: Strings.pinLabel, message: Strings.pinTitle, inputType: .default, secureInput: true, inputPlaceholder: Strings.pinPlaceholder)

fileprivate var u2fActive = false
fileprivate var currentMessageType = U2FMessageType.None
fileprivate var currentHandle = -1
fileprivate var currentTabId = ""
Expand Down Expand Up @@ -132,29 +138,31 @@ class U2FExtensions: NSObject {
observeKeyStateUpdates = true
}

popup = AlertPopupView(image: #imageLiteral(resourceName: "browser_lock_popup"), title: Strings.touchKeyTitle, message: Strings.touchKeyMessage)
super.init()

popup.addButton(title: Strings.touchKeyCancel) { [weak self] in
guard let self = self else {
return .flyDown
}
let handleCancelButton: () -> PopupViewDismissType = {
let handle = self.currentHandle
self.cleanupPinVerificationPopup()
switch self.currentMessageType {
case .FIDO2Create:
self.sendFIDO2RegistrationError(handle: handle)
case .FIDO2Get:
self.sendFIDO2AuthenticationError(handle: handle)
case.FIDORegister:
self.sendFIDORegistrationError(handle: handle, requestId: self.requestId[handle] ?? -1, errorCode: U2FErrorCodes.other_error)
case .FIDOSign:
self.sendFIDOAuthenticationError(handle: handle, requestId: self.requestId[handle] ?? -1, errorCode: U2FErrorCodes.other_error)
case .FIDOLowLevel, .None:
break
case .FIDO2Create:
self.sendFIDO2RegistrationError(handle: handle)
case .FIDO2Get:
self.sendFIDO2AuthenticationError(handle: handle)
case.FIDORegister:
self.sendFIDORegistrationError(handle: handle, requestId: self.requestId[handle] ?? -1, errorCode: U2FErrorCodes.other_error)
case .FIDOSign:
self.sendFIDOAuthenticationError(handle: handle, requestId: self.requestId[handle] ?? -1, errorCode: U2FErrorCodes.other_error)
case .FIDOLowLevel, .None:
break
}
return .flyDown
}


[touchKeyPopup, insertKeyPopup, pinVerificationPopup].forEach {
$0.addButton(title: Strings.keyCancel, tapped: handleCancelButton)
}

// Make sure the session is started
YubiKitManager.shared.keySession.startSession()
}
Expand Down Expand Up @@ -212,6 +220,10 @@ class U2FExtensions: NSObject {
}

override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) {
// We cancel all observers in ObserveValue to avoid excessive queuing on
// the main thread when the keys are disconnected continuosly.
disableObservers()

guard context == &U2FExtensions.observationContext else {
super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
return
Expand All @@ -221,10 +233,12 @@ class U2FExtensions: NSObject {
case #keyPath(YKFKeySession.sessionState):
ensureMainThread {
self.handleSessionStateChange()
self.enableObservers()
}
case #keyPath(YKFKeySession.u2fService.keyState), #keyPath(YKFKeySession.fido2Service.keyState):
ensureMainThread {
self.presentInteractWithKeyModal()
self.enableObservers()
}
default:
return
Expand Down Expand Up @@ -403,6 +417,7 @@ class U2FExtensions: NSObject {
guard let self = self else {
return
}
self.cleanupPinVerificationPopup()
if error == true {
self.sendFIDO2RegistrationError(handle: handle)
return
Expand All @@ -424,6 +439,7 @@ class U2FExtensions: NSObject {
}

private func cleanupFIDO2Registration(handle: Int) {
u2fActive = false
guard let index = fido2RegHandles.firstIndex(of: handle) else {
log.error(U2FErrorMessages.ErrorRegistration)
return
Expand Down Expand Up @@ -569,14 +585,15 @@ class U2FExtensions: NSObject {

guard getAssertionError.code == YKFKeyFIDO2ErrorCode.PIN_REQUIRED.rawValue else {
let errorDescription = error.localizedDescription
sendFIDO2RegistrationError(handle: handle, errorDescription: errorDescription)
sendFIDO2AuthenticationError(handle: handle, errorDescription: errorDescription)
return
}

handlePinVerificationRequired { [weak self] (error) in
guard let self = self else {
return
}
self.cleanupPinVerificationPopup()
if error == true {
self.sendFIDO2AuthenticationError(handle: handle)
return
Expand All @@ -589,22 +606,23 @@ class U2FExtensions: NSObject {
private func sendFIDO2AuthenticationError(handle: Int, errorName: String = FIDO2ErrorMessages.NotAllowedError.rawValue, errorDescription: String = Strings.U2FAuthenticationError) {
cleanupFIDO2Authentication(handle: handle)
ensureMainThread {
self.tab?.webView?.evaluateJavaScript("navigator.credentials.postGet('\(handle)', \(true), '', '', '', '', '\(errorName.toBase64())', '\(errorDescription.toBase64())')", completionHandler: { _, error in
self.tab?.webView?.evaluateJavaScript("navigator.credentials.postGet('\(handle)', \(true), '', '', '', '', '', '\(errorName.toBase64())', '\(errorDescription.toBase64())')", completionHandler: { _, error in
if error != nil {
let errorDescription = error?.localizedDescription ?? U2FErrorMessages.ErrorAuthentication.rawValue
log.error(errorDescription)
}
}) }
}

// This modal is presented when FIDO/FIDO2 APIs are waiting for the security key
private func presentInsertKeyModal() {
let currentURL = self.tab?.url?.host ?? ""
insertKeyPopup.update(title: Strings.keyTitle + currentURL)
insertKeyPopup.showWithType(showType: .flyUp)
}

// This modal is presented when the key bootstrap is complete
private func presentInteractWithKeyModal() {
observeSessionStateUpdates = false
observeKeyStateUpdates = false
defer {
observeSessionStateUpdates = true
observeKeyStateUpdates = true
}

guard let fido2Service = YubiKitManager.shared.keySession.fido2Service else {
return
}
Expand All @@ -613,46 +631,66 @@ class U2FExtensions: NSObject {
return
}

if tab?.id == currentTabId && (fido2Service.keyState == .touchKey || u2fService.keyState == .YKFKeyU2FServiceKeyStateTouchKey) {
popup.showWithType(showType: .flyUp)
// The modal should be visible for the tab where the U2F API is active
if u2fActive && tab?.id == currentTabId && (fido2Service.keyState == .touchKey || u2fService.keyState == .YKFKeyU2FServiceKeyStateTouchKey) {
let currentURL = self.tab?.url?.host ?? ""
touchKeyPopup.update(title: Strings.keyTitle + currentURL)
touchKeyPopup.showWithType(showType: .flyUp)
return
}
popup.dismissWithType(dismissType: .flyDown)
touchKeyPopup.dismissWithType(dismissType: .flyDown)
}

private func handlePinVerificationRequired(completion: @escaping (Bool) -> Void ) {
private func handlePinVerificationRequired(completion: @escaping (Bool) -> Void) {
ensureMainThread {
let alert = UIAlertController.userTextInputAlert(title: Strings.pinTitle, message: Strings.pinLabel, placeholder: Strings.pinPlaceholder) {
pin, _ in
if let pin = pin, !pin.isEmpty {
guard let fido2Service = YubiKitManager.shared.keySession.fido2Service else {
completion(true)
return
}
guard let verifyPinRequest = YKFKeyFIDO2VerifyPinRequest(pin: pin) else {
completion(true)
return
}

fido2Service.execute(verifyPinRequest) { (error) in
guard error == nil else {
completion(true)
return
}
completion(false)
return
}
} else {
completion(true)
return
}
self.pinVerificationPopup.addButton(title: Strings.confirmPin) { [weak self] in
self?.verifyPin(completion: completion) ?? .flyDown
}
self.pinVerificationPopup.showWithType(showType: .flyUp)
}
}

private func verifyPin(completion: @escaping (Bool) -> Void) -> PopupViewDismissType {
guard
let pin = pinVerificationPopup.text, !pin.isEmpty,
let fido2Service = YubiKitManager.shared.keySession.fido2Service,
let verifyPinRequest = YKFKeyFIDO2VerifyPinRequest(pin: pin) else {
completion(true)
return .flyDown
}

fido2Service.execute(verifyPinRequest) { (error) in
guard error == nil else {
completion(true)
return
}
alert.textFields?.first?.isSecureTextEntry = true
(UIApplication.shared.delegate as? AppDelegate)?.browserViewController.present(alert, animated: true)
completion(false)
return
}
return .flyDown
}

// Confirm key with action is populated at run-time, we clean the state
// when the dialog box is dismissed.
private func cleanupPinVerificationPopup() {
ensureMainThread {
self.pinVerificationPopup.removeButtonAtIndex(buttonIndex: 1)
self.pinVerificationPopup.clearTextField()
}
}

private func disableObservers() {
observeSessionStateUpdates = false
observeKeyStateUpdates = false
}

private func enableObservers() {
observeSessionStateUpdates = true
observeKeyStateUpdates = true
}

private func cleanupFIDO2Authentication(handle: Int) {
u2fActive = false
guard let index = fido2AuthHandles.firstIndex(of: handle) else {
log.error(U2FErrorMessages.ErrorRegistration)
return
Expand Down Expand Up @@ -764,6 +802,7 @@ class U2FExtensions: NSObject {
}

private func cleanupFIDORegistration(handle: Int) {
u2fActive = false
guard let index = fidoRegHandles.firstIndex(of: handle) else {
log.error(U2FErrorMessages.ErrorRegistration)
return
Expand Down Expand Up @@ -894,6 +933,7 @@ class U2FExtensions: NSObject {
}

private func cleanupFIDOAuthentication(handle: Int) {
u2fActive = false
guard let index = fidoSignHandles.firstIndex(of: handle) else {
log.error(U2FErrorMessages.ErrorRegistration)
return
Expand All @@ -904,14 +944,10 @@ class U2FExtensions: NSObject {
}

private func handleSessionStateChange() {
observeSessionStateUpdates = false
observeKeyStateUpdates = false
defer {
observeSessionStateUpdates = true
observeKeyStateUpdates = true
}
let sessionState = YubiKitManager.shared.keySession.sessionState
if sessionState == .open { // The key session is ready to be used.
insertKeyPopup.dismissWithType(dismissType: .flyDown)

if !fido2RegHandles.isEmpty {
guard let handle = fido2RegHandles.first else {
log.error(U2FErrorMessages.ErrorRegistration)
Expand Down Expand Up @@ -959,6 +995,13 @@ class U2FExtensions: NSObject {
}
handleFIDOAuthentication(handle: handle, keys: keys, requestId: requestId[handle] ?? -1)
}
} else {
if u2fActive == true {
presentInsertKeyModal()
}
cleanupPinVerificationPopup()
touchKeyPopup.dismissWithType(dismissType: .flyDown)
pinVerificationPopup.dismissWithType(dismissType: .flyDown)
}
}
}
Expand All @@ -981,6 +1024,14 @@ extension U2FExtensions: TabContentScript {
return
}

u2fActive = true
currentHandle = handle
currentMessageType = U2FMessageType(rawValue: name) ?? U2FMessageType.None

if YubiKitManager.shared.keySession.sessionState != .open {
presentInsertKeyModal()
}

switch name {
case U2FMessageType.FIDO2Create.rawValue:
// FIDO2 is the new webauthn API
Expand Down Expand Up @@ -1077,6 +1128,7 @@ extension U2FExtensions: TabContentScript {
log.error(error.localizedDescription)
}
default:
insertKeyPopup.dismissWithType(dismissType: .flyDown)
log.error(U2FErrorMessages.Error)
}
}
Expand All @@ -1090,9 +1142,10 @@ extension Strings {
public static let U2FAuthenticationError = NSLocalizedString("U2FAuthenticationError", tableName: "BraveShared", bundle: Bundle.braveShared, value: "Error authenticating your security key", comment: "Error handling U2F authentication.") + tryAgain

//Lightning Modals
public static let touchKeyTitle = NSLocalizedString("touchKeyTitle", bundle: Bundle.shared, value: "Use the security key", comment: "Title for touch key modal.")
public static let touchKeyMessage = NSLocalizedString("touchKeyMessage", bundle: Bundle.shared, value: "Insert your security key and touch it.", comment: "Message for touch key modal.")
public static let touchKeyCancel = NSLocalizedString("touchKeyCancel", bundle: Bundle.shared, value: "Cancel", comment: "Text for touch key modal button.")
public static let keyTitle = NSLocalizedString("touchKeyTitle", bundle: Bundle.shared, value: "Use the security key for ", comment: "Title for touch key modal.")
public static let touchKeyMessage = NSLocalizedString("touchKeyMessage", bundle: Bundle.shared, value: "Interact with the security key.", comment: "Message for touch key modal.")
public static let insertKeyMessage = NSLocalizedString("insertKeyMessage", bundle: Bundle.shared, value: "Insert your security key.", comment: "Message for touch key modal.")
public static let keyCancel = NSLocalizedString("touchKeyCancel", bundle: Bundle.shared, value: "Cancel", comment: "Text for touch key modal button.")

//PIN
public static let pinTitle = NSLocalizedString("pinTitle", bundle: Bundle.shared, value: "PIN Required", comment: "Title for the alert modal when a security key with PIN is inserted.")
Expand Down
4 changes: 2 additions & 2 deletions Client/WebAuthN/WebAuthnAuthenticateRequest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ extension WebAuthnAuthenticateRequest: Decodable {
rpID = try publicKeyDictionary.decodeIfPresent(String.self, forKey: .rpId)
challenge = try publicKeyDictionary.decode(String.self, forKey: .challenge)

// userPresence is the inverse of userVerification
let userVerifcationString = try publicKeyDictionary.decodeIfPresent(String.self, forKey: .userVerification) ?? ""
// userPresence is the inverse of userVerification, UP by default is true
let userVerifcationString = try publicKeyDictionary.decodeIfPresent(String.self, forKey: .userVerification) ?? "discouraged"
userPresence = userVerifcationString == "discouraged"

let allowCredentialsArray = try publicKeyDictionary.decode([AllowCredentials].self, forKey: .allowCredentials)
Expand Down

0 comments on commit 7e5a281

Please sign in to comment.