Refactor the ios keychain add command.

This commit removes the --key flag and adds the --account and
--service flags.

Fixes #350

agent/src/ios/keychain.ts

@@ -161,19 +161,26 @@ export namespace ioskeychain {
   };
 
   // add a string entry to the keychain
-  export const add = (key: string, data: string): boolean => {
-    // Convert the key and data to NSData
-    const dataString: NSStringType = NSString.stringWithString_(data).dataUsingEncoding_(NSUTF8StringEncoding);
-    const dataKey: NSStringType = NSString.stringWithString_(key).dataUsingEncoding_(NSUTF8StringEncoding);
-    const itemDict: NSMutableDictionaryType = NSMutableDictionary.alloc().init();
+  export const add = (account: string, service: string, data: string): boolean => {
 
+    // prepare the dictionary for SecItemAdd()
+    const itemDict: NSMutableDictionaryType = NSMutableDictionary.alloc().init();
     itemDict.setObject_forKey_(kSec.kSecClassGenericPassword, kSec.kSecClass);
-    itemDict.setObject_forKey_(dataKey, kSec.kSecAttrService);
-    itemDict.setObject_forKey_(dataString, kSec.kSecValueData);
+
+    [
+      { "type": "account", "value": account, "ksec": kSec.kSecAttrAccount },
+      { "type": "service", "value": service, "ksec": kSec.kSecAttrService },
+      { "type": "data", "value": data, "ksec": kSec.kSecValueData }
+    ].forEach(e => {
+      if (e.value == null) return;
+      const v: NSStringType = NSString.stringWithString_(e.value)
+        .dataUsingEncoding_(NSUTF8StringEncoding);
+
+      itemDict.setObject_forKey_(v, e.ksec);
+    });
 
     // Add the keychain entry
     const result: any = libObjc.SecItemAdd(itemDict, NULL);
-
     return result.isNull();
   };
 

agent/src/rpc/ios.ts

@@ -83,7 +83,8 @@ export const ios = {
   iosBundlesGetFrameworks: (): IFramework[] => bundles.getBundles(BundleType.NSBundleFramework),
 
   // ios keychain
-  iosKeychainAdd: (key: string, data: string): boolean => ioskeychain.add(key, data),
+  iosKeychainAdd: (account: string, service: string, data: string): boolean => 
+    ioskeychain.add(account, service, data),
   iosKeychainEmpty: (): void => ioskeychain.empty(),
   iosKeychainList: (smartDecode): IKeychainItem[] => ioskeychain.list(smartDecode),
   iosKeychainListRaw: (): void => ioskeychain.listRaw(),

objection/commands/ios/keychain.py

@@ -30,17 +30,19 @@ def _should_do_smart_decode(args: list) -> bool:
     return len(args) > 0 and '--smart' in args
 
 
-def _has_minimum_flags_to_add_item(args: list) -> bool:
+def _data_flag_has_identifier(args: list) -> bool:
     """
-        Ensure that all of the flags are present for a keychain
-        entry item. At the same time, ensure that each flag has a value.
+        Checks that if the data flag is specified, an identifier
+        is also passed.
 
         :param args:
         :return:
     """
 
-    return all(i in args for i in ['--key', '--data']) and len([
-        x for x in args if '--' not in x]) == len([x for x in args if '--' in x])
+    if '--data' in args:
+        return any(x in args for x in ['--service', '--account'])
+
+    return True
 
 
 def _get_flag_value(args: list, flag: str) -> str:
@@ -52,7 +54,7 @@ def _get_flag_value(args: list, flag: str) -> str:
         :return:
     """
 
-    return args[args.index(flag) + 1]
+    return args[args.index(flag) + 1] if flag in args else None
 
 
 def dump(args: list = None) -> None:
@@ -138,25 +140,28 @@ def clear(args: list = None) -> None:
 
 def add(args: list) -> None:
     """
-        Adds a new keychain entry to the keychain
+        Adds a new kSecClassGenericPassword keychain entry to the keychain
 
         :param args:
         :return:
     """
 
-    if not _has_minimum_flags_to_add_item(args):
-        click.secho('Usage: ios keychain add --key <key name> --data <entry data>', bold=True)
+    if not _data_flag_has_identifier(args):
+        click.secho('When specifying the --data flag, either --account or '
+                    '--server should also be added', fg='red')
         return
 
-    key = _get_flag_value(args, '--key')
-    value = _get_flag_value(args, '--data')
+    account = _get_flag_value(args, '--account')
+    service = _get_flag_value(args, '--service')
+    data = _get_flag_value(args, '--data')
 
     click.secho('Adding a new entry to the iOS keychain...', dim=True)
-    click.secho('Key:       {0}'.format(key), dim=True)
-    click.secho('Value:     {0}'.format(value), dim=True)
+    click.secho('Account:  {0}'.format(account), dim=True)
+    click.secho('Service:  {0}'.format(service), dim=True)
+    click.secho('Data:     {0}'.format(data), dim=True)
 
     api = state_connection.get_api()
-    if api.ios_keychain_add(key, value):
+    if api.ios_keychain_add(account, service, data):
         click.secho('Successfully added the keychain item', fg='green')
         return
 

objection/console/commands.py

@@ -514,7 +514,7 @@
                     },
                     'add': {
                         'meta': 'Add an entry to the iOS keychain',
-                        'flags': ['--key', '--data'],
+                        'flags': ['--account', '--service', '--data'],
                         'exec': keychain.add
                     }
                 }

objection/console/helpfiles/ios.keychain.add.txt

@@ -1,11 +1,12 @@
 Command: ios keychain add
 
-Usage: ios keychain add --key <key name> --data <entry data>
+Usage: ios keychain add --account <account> --service <service> --data <entry data>
 
 Adds a new entry to the iOS keychain using SecItemAdd.
 
 The new keychain entry class would be kSecClassGenericPassword with no extra 
 kSecAttrAccessControl set.
 
 Examples:
-   ios keychain add --key token --data 1122-33344-55122-55512
+   ios keychain add --account token --data 1122-33344-55122-55512
+   ios keychain add --service foo --data bar

objection/utils/patchers/base.py

@@ -12,6 +12,7 @@
 objection_path = os.path.join(os.path.expanduser('~'), '.objection')
 gadget_versions = os.path.join(objection_path, 'gadget_versions')
 
+
 def list2posix_cmdline(seq):
     """
         Translate a sequence of arguments into a command line
@@ -20,7 +21,7 @@ def list2posix_cmdline(seq):
         Implemented using shlex.quote because 
         subprocess.list2cmdline doesn't work with POSIX
     """
-        
+
     return ' '.join(map(shlex.quote, seq))
 
 
@@ -116,7 +117,7 @@ def __init__(self):
             self.list2cmdline = list2cmdline
         else:
             self.list2cmdline = list2posix_cmdline
-    
+
     def _check_commands(self) -> bool:
         """
             Check if the shell commands in required_commands are

tests/commands/ios/test_keychain.py

@@ -2,7 +2,7 @@
 from unittest import mock
 
 from objection.commands.ios.keychain import _should_output_json, dump, dump_raw, clear, add, \
-    _has_minimum_flags_to_add_item, _get_flag_value, _should_do_smart_decode
+    _data_flag_has_identifier, _get_flag_value, _should_do_smart_decode
 from ...helpers import capture
 
 
@@ -28,13 +28,18 @@ def test_dump_validates_arguments_if_json_output_is_wanted(self):
 
         self.assertEqual(output, 'Usage: ios keychain dump (--json <local destination>)\n')
 
-    def test_has_minimum_flags_to_add_item_returns_true(self):
-        result = _has_minimum_flags_to_add_item(['--key', 'test_key', '--data', 'test_data'])
+    def test_data_flag_check_ignored_without_data_flag(self):
+        result = _data_flag_has_identifier(['--key', 'test_key'])
 
         self.assertTrue(result)
 
-    def test_has_minumum_flags_to_add_item_returns_false(self):
-        result = _has_minimum_flags_to_add_item(['--key', 'test_key'])
+    def test_data_flag_is_checked_when_flag_is_specified(self):
+        result = _data_flag_has_identifier(['--key', 'test_key', '--data', 'foo'])
+
+        self.assertFalse(result)
+
+    def test_data_flag_is_checked_when_only_data_flag_is_specified_without_key(self):
+        result = _data_flag_has_identifier(['--data', 'foo'])
 
         self.assertFalse(result)
 
@@ -132,28 +137,34 @@ def test_clear(self, mock_confirm, mock_api):
         self.assertEqual(output, 'Clearing the keychain...\nKeychain cleared\n')
         self.assertTrue(mock_api.return_value.ios_keychain_empty.called)
 
-    def test_adds_item_validates_arguments(self):
-        with capture(add, ['--key', 'test_key']) as o:
+    def test_adds_item_validates_data_key_to_need_identifier(self):
+        with capture(add, ['--data', 'test_data']) as o:
             output = o
 
-        self.assertEqual(output, 'Usage: ios keychain add --key <key name> --data <entry data>\n')
+        self.assertEqual(output, 'When specifying the --data flag, either --account or --server should also be added\n')
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_adds_item_successfully(self, mock_api):
         mock_api.return_value.keychain_add.return_value = True
 
-        with capture(add, ['--key', 'test_key', '--data', 'test_data']) as o:
+        with capture(add, ['--account', 'test_key', '--data', 'test_data']) as o:
             output = o
 
-        self.assertEqual(output, 'Adding a new entry to the iOS keychain...\nKey:       test_key\n'
-                                 'Value:     test_data\nSuccessfully added the keychain item\n')
+        self.assertEqual(output, """Adding a new entry to the iOS keychain...
+Account:  test_key
+Service:  None
+Data:     test_data
+Successfully added the keychain item\n""")
 
     @mock.patch('objection.state.connection.state_connection.get_api')
     def test_adds_item_with_failure(self, mock_api):
         mock_api.return_value.ios_keychain_add.return_value = False
 
-        with capture(add, ['--key', 'test_key', '--data', 'test_data']) as o:
+        with capture(add, ['--service', 'test_key', '--data', 'test_data']) as o:
             output = o
 
-        self.assertEqual(output, 'Adding a new entry to the iOS keychain...\nKey:       test_key\n'
-                                 'Value:     test_data\nFailed to add the keychain item\n')
+        self.assertEqual(output, """Adding a new entry to the iOS keychain...
+Account:  None
+Service:  test_key
+Data:     test_data
+Failed to add the keychain item\n""")