matthiasplappert / Secure-NSUserDefaults

User defaults that cannot be modified or shared between devices.
203 stars 14 forks source link

setSecureDictionary not matching hash after app exits #4

Open M0000N opened 10 years ago

M0000N commented 10 years ago

Hello! Thanks for this great class. I am using it to try and set a secure Dictionary. It works as it should, but only as long as the app runs. After I close it and restart the simulator, the hash value does not match any more.

nader-eloshaiker commented 10 years ago

Most likely you are generating the hash for the current dictionary and then adding it to it. Once you do this, the hash value changes. What you you need to do is store a dictionary inside another dictionary.

Generate a hash for the secured dictionary and then store the hash and the secured dictionary in another dictionary that is not secured. When loading, retrieve the secuured dictionary and hash code, then generate a new hash for the secured dictionary to see if the hash codes match.

This is what I am doing and works really well.

M0000N commented 10 years ago

Hi! Thanks for the quick response! I am not sure if I follow what you say, but all that I am doing really is calling setSecureDictionary once, and then secureDictionaryForKey to retrieve it. The Dictionary doesn't even change. The hash works fine as long as the app runs, but comes up invalid when I exit the simulator and restart.

nader-eloshaiker commented 10 years ago

Sorry, I think I may have misinterpreted your question. I thought you where having issues with checking to see if the hash codes matched to determine if the settings had been tampered with. It appears that you are having issues with encryption and decryption of the settings, correct?

M0000N commented 10 years ago

Not really. I am simply trying to use this class as is.

nader-eloshaiker commented 10 years ago

To be honest, I haven't used this code, I only looked at it for reference and it is a very good reference. Perhaps this link will give you more luck http://terafactor.com/i/securing-data-in-iphoneipadipod-apps/

M0000N commented 10 years ago

Thanks for the link. I've seen this before I think but it's somewhat out of my comfort zone (I'm a newb), and I don't need encryption for what I am doing with this (no sensitive data is saved), just a way to deter all too easy manipulation of the NSUserDefaults plist (iExplorer etc.)

For the record, I am using this code for strings and arrays etc. and it works well, but there seems to be something about NSDictionaries that just doesn't want to work. I cross-posted the question to StackOverflow and someone there indicates that using NSKeyArchiver for a NSDictionary just won't work. So I guess the current implementation for Dictionaries in this code is plain broken at the moment.

nader-eloshaiker commented 10 years ago

I don't think this code is at fault, what is the stackoverflow url, lets take the conversation off this repo so that the author can close the issue.

M0000N commented 10 years ago

http://stackoverflow.com/questions/21664986/saving-nsdictionary-in-nsuserdefaults-hash-fails

CRD2 commented 10 years ago

It is the code, it makes an invalid assumption that if two instances of a type are semantically the same then NSKeyArchiver archives of them are semantically the same, this does not hold.

Try:

NS_INLINE NSString *Bool2Str(BOOL b) { return b ? @"YES" : @"NO"; } // convenience
...
NSDictionary *one = @{@"3" : @[@4, @5, @6], @"one" : @2};
NSData *oneData =[NSKeyedArchiver archivedDataWithRootObject:one];

NSDictionary *two = @{@"one" : @2, @"3" : @[@4, @5, @6]};
NSData *twoData =[NSKeyedArchiver archivedDataWithRootObject:two];

NSLog(@"dict: %@ | data : %@", Bool2Str([one isEqual:two]), Bool2Str([oneData isEqual:twoData]));

This will output:

dict: YES | data: NO

Given that dictionaries/arrays can contain arrays/dictionaries the approach taken by the code is doomed.

M0000N commented 10 years ago

So this seems to work for me. Thoughts?

- (NSString *)_hashObject:(id)object
{
    if (_secretData == nil) {
        // Use if statement in case asserts are disabled
        NSAssert(NO, @"Provide a secret before using any secure writing or reading methods!");
        return nil;
}

// Copy object to make sure it is immutable (thanks Stephen)
    object = [object copy];

    //added check for array or dictionary
    if ([NSJSONSerialization isValidJSONObject:object]) {
        NSMutableData *archivedData = [[NSJSONSerialization dataWithJSONObject:object options:0 error:nil] mutableCopy];
        [archivedData appendData:_secretData];
        if (_deviceIdentifierData != nil) {
            [archivedData appendData:_deviceIdentifierData];
        }
        NSString *hash = [self _hashData:archivedData];
        return hash;
    }

    // Archive & hash
    NSMutableData *archivedData = [[NSKeyedArchiver archivedDataWithRootObject:object] mutableCopy];
    [archivedData appendData:_secretData];
    if (_deviceIdentifierData != nil) {
        [archivedData appendData:_deviceIdentifierData];
    }
    NSString *hash = [self _hashData:archivedData];
    ////[archivedData release];
    return hash;
}
M0000N commented 10 years ago

@CRD2, thanks for posting your test. I have run it using NSJSONSerialization and it fails, as well. However, when I changed

NSDictionary *one = @{@"3" : @[@4, @5, @6], @"one" : @2};
NSDictionary *two = @{@"one" : @2, @"3" : @[@4, @5, @6]};

to

NSDictionary *one = @{@"3" : @[@4, @5, @6]};
NSDictionary *two = @{@"3" : @[@4, @5, @6]};

(which more closely resembles the way I have used this when I ran into the issue), both NSKeyedArchiver and NSJSONSerialization return YES.

So it seems that something else is at work here. In my code, I have switched from using secureDictionary to using secureArray. This works but not for arrays containing arrays, where the exact same problem appears as with using secureDictionary, but only for NSKeyedArchiver. Since arrays are ordered, and the code I posted above works so far for retrieving the NSUserDefaults between app starts, I guess my issue is solved, although I would definitely like to understand what the problem is in the first place.

CRD2 commented 10 years ago

There isn't really "something else at work".

Both NSKeyed(Un)Archiver & NSJSONSerialization encode and decode structures such that the decoded result is semantically equivalent to the original. They make no guarantees as such that for two semantically equivalent inputs the encoded versions are the same.

As JSON is a text format and doesn't concern itself with issues such as sharing - where the same object is referenced more than once in the input - then there is a higher probability that on particular inputs the encoded versions match. But is it still only a probability.

NSKeyed(Un)Archiver concerns itself with sharing - it archives the object graph and restores the same graph - and the extra information it needs to store in the archive to achieve this decreases the probability that different objects that match semantically encode the same.

The code in this project is built on the assumption that given any two objects which match semantically that the encoded byte streams will be identical. Neither of these classes will guarantee that.