Open Radar

 
Summary:

kSecAttrApplicationTag is an attribute of SecKey items with the following documentation:

	 kSecAttrApplicationTag
             Specifies a dictionary key whose value is a CFDataRef containing private tag data.

In practice, most (but not all) functions in the Keychain Services API treat it as a CFStringRef.  Functions that output this attribute seem to always use CFStringRef.  Functions that accept this attribute in an input parameter sometimes take CFStringRefs, sometimes CFDataRefs, but never both. In either case, the arguments are usually not adequately type checked, resulting in crashes and unexpected Objective-C exceptions when we accidentally supply a value of the wrong type.

The type of this attribute should be consistent across all SecKey* and SecItem* functions. All input arguments should be type checked, and paramErr returned on type errors.

A description of the erroneous behavior of individual functions is given below:

(1) SecKeyGenerateSymmetric reports paramErr when given a kSecAttrApplicationTag with a CFDataRef value. It does accept CFStringRef values. The documentation should be amended to reflect that it accepts kSecAttrApplicationTag keys in its parameter dictionary, and the implementation should be fixed to allow CFDataRefs.

(2) SecKeyGeneratePair is documented to accept kSecAttrApplicationTag, and works correctly when given a CFDataRef value. With CFStringRef, it should fail with paramErr, but instead crashes with the following call stack:

    #0	0x00007fff8ff06590 in longcopy ()
    #1	0x00007fff8fef1f8f in memmove$VARIANT$sse3x ()
    #2	0x00007fff8b26712f in Security::CssmDbAttributeData::set(cssm_db_attribute_info const&, Security::CssmPolyData const&, Security::Allocator&) ()
    #3	0x00007fff8b3ae6a2 in Security::CssmAutoDbRecordAttributeData::add(cssm_db_attribute_info const&, Security::CssmPolyData const&) ()
    #4	0x00007fff8b2e8469 in Security::KeychainCore::ItemImpl::modifyAttributesAndData(SecKeychainAttributeList const*, unsigned int, void const*) ()
    #5	0x00007fff8b306f80 in SecKeychainItemModifyAttributesAndData ()
    #6	0x00007fff8b300aef in SetKeyLabelAndTag(OpaqueSecKeyRef*, void const*, __CFData const*) ()
    #7	0x00007fff8b30207f in SecKeyGeneratePair ()

(3) SecItemCopyMatching always returns kSecAttrApplicationTag values as CFStringRefs. It should instead return these values as CFDataRefs.

(4) SecItemUpdate can't modify kSecAttrApplicationTag. When given a CFStringRef value, it reports success, but modifies kSecAttrLabel instead of kSecAttrApplicationTag. When given a CFDataRef value, it throws an Objective-C exception with the call stack given below. It should accept CFDataRefs and update the correct attribute.

  #0  0x00007fff88983d45 in objc_exception_throw ()
  #1  0x00007fff9242b4ce in -[NSObject doesNotRecognizeSelector:] ()
  #2  0x00007fff9238c133 in ___forwarding___ ()
  #3  0x00007fff9238bf48 in __forwarding_prep_0___ ()
  #4  0x00007fff92306096 in CFStringGetCharactersPtr ()
  #5  0x00007fff92304e04 in __CFStringEncodeByteStream ()
  #6  0x00007fff8dd40387 in _CFStringCreateAttribute ()
  #7  0x00007fff8dd40ca5 in _CreateSecKeychainKeyAttributeListFromDictionary ()
  #8  0x00007fff8dd4006d in _UpdateKeychainItem ()
  #9  0x00007fff8dd45234 in SecItemUpdate ()


Steps to Reproduce:

  I attached a sample command-line tool that creates a temporary keychain and generates a 3DES key and an RSA key pair into it, setting initial values for the application tag. 
  It then queries the kSecApplicationTag attributes for all three keys and displays their value and type on the console.
  Finally, it tries to modify the application tag of the 3DES key using SecItemUpdate, then verifies the result.

  The code demonstrating the four issues above is marked with the corresponding parenthesized number.

Expected Results:

  See above.

Actual Results:
  
  See above.

Regression:

  Unknown.

Notes:


//
//  main.c
//  KeychainBugs
//
//  Created by Károly Lőrentey on 2011-12-22.
//  Copyright (c) 2011 __MyCompanyName__. All rights reserved.
//

#import <Foundation/Foundation.h>
#import <Security/Security.h>

static void ReportError(OSStatus status, NSString *message, ...) __attribute__((noreturn));

static void 
ReportError(OSStatus status, NSString *message, ...)
{
    va_list args;
    va_start(args, message);
    NSString *bakedMessage = [[[NSString alloc] initWithFormat:message arguments:args] autorelease];
    va_end(args);
    CFStringRef errorString = SecCopyErrorMessageString(status, NULL);
    NSLog(@"%@: %@ (%d)", bakedMessage, (errorString ? (NSString *)errorString : @"unknown error"),  (int)status);
    if (errorString)
        CFRelease(errorString);
    exit(1);
}

static SecKeychainRef
MakeKeychain(NSString *path)
{
    if ([[NSFileManager defaultManager] fileExistsAtPath:path]) {
        [[NSFileManager defaultManager] removeItemAtPath:path error:NULL];
    }
    
    SecKeychainRef keychain = NULL;
    OSStatus status = SecKeychainCreate([path fileSystemRepresentation], 6, "foobar", NO, NULL, &keychain);
    if (status)
        ReportError(status, @"Can't create keychain");
    [(id)keychain autorelease];
    return keychain;
}

struct Keypair {
    SecKeyRef publicKey;
    SecKeyRef privateKey;
};

static struct Keypair
MakeRSAKeyPair(id tag, SecKeychainRef keychain)
{
    OSStatus status;
    
    NSMutableDictionary *parameters = [NSMutableDictionary dictionary];
    
    [parameters setObject:kSecAttrKeyTypeRSA forKey:kSecAttrKeyType];
    [parameters setObject:[NSNumber numberWithInt:2048] forKey:kSecAttrKeySizeInBits];
    [parameters setObject:(id)keychain forKey:kSecUseKeychain];
    [parameters setObject:(id)kCFBooleanTrue forKey:kSecAttrIsPermanent];
    [parameters setObject:@"Test RSA Key" forKey:kSecAttrLabel];
    [parameters setObject:tag forKey:kSecAttrApplicationTag];
    
    SecKeyRef spublicKey = NULL;
    SecKeyRef sprivateKey = NULL;
    status = SecKeyGeneratePair((CFDictionaryRef)parameters, &spublicKey, &sprivateKey);
    if (status)
        ReportError(status, @"Can't generate key pair");
    [(id)spublicKey autorelease];
    [(id)sprivateKey autorelease];
    
    return (struct Keypair){ .publicKey = spublicKey, .privateKey = sprivateKey };
}

static SecKeyRef
Make3DESKey(id tag, SecKeychainRef keychain)
{
    NSMutableDictionary *parameters = [NSMutableDictionary dictionary];
    [parameters setObject:[NSNumber numberWithInt:192] forKey:kSecAttrKeySizeInBits];
    [parameters setObject:kSecAttrKeyType3DES forKey:kSecAttrKeyType];
    [parameters setObject:@"Test key" forKey:kSecAttrLabel];
    [parameters setObject:(id)keychain forKey:kSecUseKeychain];
    [parameters setObject:(id)kCFBooleanTrue forKey:kSecAttrIsPermanent];
    [parameters setObject:tag forKey:kSecAttrApplicationTag];
    
    CFErrorRef cferror = NULL;
    SecKeyRef skey = SecKeyGenerateSymmetric((CFDictionaryRef)parameters, &cferror);
    if (skey == NULL) {
        ReportError((OSStatus)CFErrorGetCode(cferror), @"Can't generate symmetric key");
        CFRelease(cferror);
    }
    [(id)skey autorelease];
    return skey;
}

static NSDictionary *
AttributesForKey(SecKeyRef skey)
{
    NSDictionary *query = [NSDictionary dictionaryWithObjectsAndKeys:
                           kSecClassKey, kSecClass,
                           [NSArray arrayWithObject:(id)skey], kSecMatchItemList,
                           kCFBooleanTrue, kSecReturnAttributes,
                           kSecMatchLimitOne, kSecMatchLimit,
                           nil];
    NSDictionary *attrs = nil;
    OSStatus status = SecItemCopyMatching((CFDictionaryRef)query, (CFTypeRef *)&attrs);
    if (status)
        ReportError(status, @"Can't get item attributes");
    [(id)attrs autorelease];
    return attrs;
}

static void
ChangeAttribute(SecKeyRef skey, id attribute, id value)
{
    NSDictionary *updatedAttrs = [NSDictionary dictionaryWithObject:value forKey:attribute];
    
    NSDictionary *query = [NSDictionary dictionaryWithObjectsAndKeys:
                           kSecClassKey, kSecClass,
                           [NSArray arrayWithObject:(id)skey], kSecMatchItemList,
                           nil];
    
    OSStatus status = SecItemUpdate((CFDictionaryRef)query, (CFDictionaryRef)updatedAttrs);
    if (status)
        ReportError(status, @"Can't update item attributes");
}

int 
main (int argc, const char *argv[])
{
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

    // Create an empty keychain.
    NSString *keychainPath = [NSTemporaryDirectory() stringByAppendingPathComponent:@"Test Keychain.keychain"];
    SecKeychainRef keychain = MakeKeychain(keychainPath);

    NSString *stringTag = @"Test value";
    NSData *dataTag = [stringTag dataUsingEncoding:NSUTF8StringEncoding];
    
    // Create a symmetric key and a pair of assymetric keys.
    
    // (1) Using dataTag instead of stringTag here would make SecKeyGenerateSymmetric fail with paramErr:
    SecKeyRef tripleDESKey = Make3DESKey(stringTag, keychain);
    
    // (2) Using stringTag instead of dataTag here would crash SecKeyGeneratePair:
    struct Keypair keypair = MakeRSAKeyPair(dataTag, keychain);
    
    NSLog(@"Keys successfully created.");

    // (3) SecItemCopyMatching should return kSecAttrApplicationTag values as CFDataRefs.
    id tripleDESTag = [AttributesForKey(tripleDESKey) objectForKey:kSecAttrApplicationTag];
    id publicTag = [AttributesForKey(keypair.publicKey) objectForKey:kSecAttrApplicationTag];;
    id privateTag = [AttributesForKey(keypair.privateKey) objectForKey:kSecAttrApplicationTag];;
    NSLog(@"3DES application tag: '%@' (%@, should be __NSCFData)", tripleDESTag, [tripleDESTag class]);
    NSLog(@"RSA public key application tag: '%@' (%@, should be __NSCFData)", publicTag, [publicTag class]);
    NSLog(@"RSA private key application tag: '%@' (%@, should be __NSCFData)", privateTag, [privateTag class]);
    
    // (4) SecItemUpdate should be able to modify kSecAttrApplicationTag. It modifies kSecAttrLabel instead.
    CFStringRef newStringTag = CFSTR("New value");
    CFDataRef newDataTag = CFDataCreate(kCFAllocatorDefault, (const UInt8 *)"New value", 9);
    
    ChangeAttribute(tripleDESKey, kSecAttrApplicationTag, (id)newDataTag);
    
    id tripleDESTag2 = [AttributesForKey(tripleDESKey) objectForKey:kSecAttrApplicationTag];
    if (![(id)newStringTag isEqualTo:tripleDESTag2]) {
        NSLog(@"FAIL: SecItemUpdate did not modify kSecAttrApplicationTag (value is '%@', should be '%@')",
              tripleDESTag2, newStringTag);
    }
    else {
        NSLog(@"SUCCESS: SecItemUpdate could modify the value of the kSecAttrApplicationTag attribute");
    }
    [pool drain];
    return 0;
}