cleanup/comments

SoftU2FTool/AppDelegate.swift

@@ -10,6 +10,7 @@ import Cocoa
 @NSApplicationMain
 class AppDelegate: NSObject, NSApplicationDelegate {
     func applicationDidFinishLaunching(_ aNotification: Notification) {
+        // Fix up legacy keychain items.
         U2FRegistration.repair()
 
         if CLI(CommandLine.arguments).run() {
@@ -27,7 +28,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
     }
 
     func applicationDidBecomeActive(_ notification: Notification) {
-        // Chrome gives ignores our U2F responses if it isn't active when we send them.
+        // Chrome ignores our U2F responses if it isn't active when we send them.
         // This hack should give focus back to Chrome immediately after the user interacts
         // with our notification.
         NSApplication.shared().hide(nil)

SoftU2FTool/CLI.swift

@@ -43,6 +43,12 @@ class CLI {
     }
 
     private func listRegistrations() {
+        let registrations = U2FRegistration.all
+        if registrations.count == 0 {
+            print("No registrations to list")
+            return
+        }
+
         print("The following is a list of U2F registrations stored in your keychain. Each key contains several fields:")
         print("  - Key handle: This is the key handle that we registered with a website. For Soft U2F, the key handle is simply a hash of the public key.")
         print("  - Application parameter: This is the sha256 of the app-id of the site.")
@@ -51,7 +57,7 @@ class CLI {
         print("  — In SEP: Whether this registration's private key is stored in the SEP.")
         print("")
 
-        U2FRegistration.all.forEach { reg in
+        registrations.forEach { reg in
             print("Key handle: ", reg.keyHandle.base64EncodedString())
             print("Application parameter: ", reg.applicationParameter.base64EncodedString())
 

SoftU2FTool/KeyPair.swift

@@ -8,6 +8,7 @@
 import Foundation
 
 class KeyPair {
+    // Fix up legacy keychain items.
     static func repair(label: String) {
         Keychain.repair(attrLabel: label as CFString)
     }

SoftU2FTool/Keychain.swift

@@ -124,23 +124,23 @@ class Keychain {
             print("Error from keychain: \(err)")
             return false
         }
-        
+
         return true
     }
-    
+
     // Get the raw data from the key.
     static func exportSecKey(_ key: SecKey) -> Data? {
         var err: Unmanaged<CFError>? = nil
         let data = SecKeyCopyExternalRepresentation(key, &err)
-        
+
         if err != nil {
             print("Error exporting key")
             return nil
         }
-        
+
         return data as Data?
     }
-    
+
     // Lookup all private keys with the given label.
     static func getPrivateSecKeys(attrLabel: CFString) -> [SecKey] {
         let query = makeCFDictionary(
@@ -151,22 +151,26 @@ class Keychain {
             (kSecReturnRef, kCFBooleanTrue),
             (kSecMatchLimit, 1000 as CFNumber)
         )
-        
+
         var optionalOpaqueResult: CFTypeRef? = nil
         let err = SecItemCopyMatching(query, &optionalOpaqueResult)
-
+
+        if err == errSecItemNotFound {
+            return []
+        }
+
         if err != errSecSuccess {
             print("Error from keychain: \(err)")
             return []
         }
-        
+
         guard let opaqueResult = optionalOpaqueResult else {
             print("Unexpected nil returned from keychain")
             return []
         }
-        
+
         let result = opaqueResult as! [SecKey]
-        
+
         return result
     }
 
@@ -294,9 +298,16 @@ class Keychain {
 
         return ret
     }
-
+
+    // Previously, we had been storing both the public and private key in the keychain and
+    // using the application tag attribute on the public key for smuggling the U2F
+    // registration's counter. When generating a private key in the SEP, the public key
+    // isn't persisted in the keychain. From now on, we're using the application tag
+    // attribute on the private key for storing the counter and just deriving the public
+    // key from the private key whenever we need it. This function makes legacy keys
+    // consistent by deleting the public key from the keychain and copying its application
+    // tag into the private key.
     static func repair(attrLabel: CFString) {
-        // Lookup public keys
         let query = makeCFDictionary(
             (kSecClass, kSecClassKey),
             (kSecAttrKeyType, kSecAttrKeyTypeEC),
@@ -305,20 +316,24 @@ class Keychain {
             (kSecReturnAttributes, kCFBooleanTrue),
             (kSecMatchLimit, 100 as CFNumber)
         )
-        
+
         var optionalOpaqueResult: CFTypeRef? = nil
         let err = SecItemCopyMatching(query, &optionalOpaqueResult)
-
+
+        if err == errSecItemNotFound {
+            return
+        }
+
         if err != errSecSuccess {
             print("Error from keychain: \(err)")
             return
         }
-        
+
         guard let opaqueResult = optionalOpaqueResult else {
             print("Unexpected nil returned from keychain")
             return
         }
-        
+
         let publicKeys = opaqueResult as! [[String:AnyObject]]
 
         publicKeys.forEach { publicKey in
@@ -328,12 +343,12 @@ class Keychain {
                 print("error getting kSecAttrApplicationTag for public key")
                 return
             }
-            
+
             guard let attrAppLabel = publicKey[kSecAttrApplicationLabel as String] as? Data else {
                 print("error getting kSecAttrApplicationLabel for public key")
                 return
             }
-            
+
             guard let _ = getPrivateSecKey(attrAppLabel: attrAppLabel as CFData, signPrompt: "" as CFString) else {
                 print("error getting private key for public key")
                 return
@@ -343,18 +358,18 @@ class Keychain {
                 print("Error copying kSecAttrApplicationTag to private key")
                 return
             }
-            
+
             let ok = delete(
                 (kSecClass, kSecClassKey),
                 (kSecAttrKeyClass, kSecAttrKeyClassPublic),
                 (kSecAttrApplicationLabel, attrAppLabel as CFData)
             )
-            
+
             if !ok {
                 print("Error deleting public keys")
                 return
             }
-            
+
             print("Success")
         }
     }

SoftU2FTool/U2FRegistration.swift

@@ -10,11 +10,11 @@ import Foundation
 class U2FRegistration {
     // Allow using separate keychain namespace for tests.
     static var namespace = "SoftU2F Security Key"
-    
+
     static var all: [U2FRegistration] {
         let kps = KeyPair.all(label: namespace)
         var regs: [U2FRegistration] = []
-        
+
         kps.forEach { kp in
             guard let reg = U2FRegistration(keyPair: kp) else {
                 print("Error initializing U2FRegistration")
@@ -23,15 +23,16 @@ class U2FRegistration {
 
             regs.append(reg)
         }
-        
+
         return regs
     }
 
     // The number of key pairs (keys/2) in the keychain.
     static var count: Int? {
         return KeyPair.count(label: namespace)
     }
-
+
+    // Fix up legacy keychain items.
     static func repair() {
         KeyPair.repair(label: namespace)
     }
@@ -49,7 +50,7 @@ class U2FRegistration {
     var keyHandle: Data {
         return padKeyHandle(keyPair.applicationLabel)
     }
-    
+
     var inSEP: Bool {
         return keyPair.inSEP
     }
@@ -96,25 +97,25 @@ class U2FRegistration {
             return nil
         }
     }
-    
+
     // Initialize a registration with all the necessary data.
     init?(keyPair kp: KeyPair) {
         keyPair = kp
-        
+
         // Read our application parameter from the keychain.
         guard let appTag = keyPair.applicationTag else { return nil }
-        
+
         let counterSize = MemoryLayout<UInt32>.size
         let appTagSize = Int(U2F_APPID_SIZE)
-        
+
         if appTag.count != counterSize + appTagSize {
             return nil
         }
-        
+
         counter = appTag.withUnsafeBytes { (ptr:UnsafePointer<UInt32>) -> UInt32 in
             return ptr.pointee.bigEndian
         }
-        
+
         applicationParameter = appTag.subdata(in: counterSize..<(counterSize + appTagSize))
     }