Skip to content

Instantly share code, notes, and snippets.

@gonecoding
Created February 22, 2011 12:42
Show Gist options
  • Save gonecoding/838614 to your computer and use it in GitHub Desktop.
Save gonecoding/838614 to your computer and use it in GitHub Desktop.
Adding methods to NSData and NSString using categories to provide AES256 encryption on iOS

Important notice

I took down this Gist due to concerns about the security of the encryption/decryption part of this code (see comments below).

Rob Napier (@rnapier) has created a publicly available class that provides similar AES encryption/decryption functionality at https://github.com/rnapier/RNCryptor.

@alexshemesh
Copy link

Looks like good and solid work.
I would also want to know whats dangerous about it.

@Kastet
Copy link

Kastet commented Dec 13, 2011

Will Apple approve this code?

@jworley
Copy link

jworley commented Dec 15, 2011

I would also like to know what is dangerous about this code.

@gonecoding
Copy link
Author

Yes, Kastet, Apple will approve this code.

@gonecoding
Copy link
Author

The only thing 'dangerous' about this code (from my point of view) is, that's it's using a symmetric-key algorithm (AES).

If, in your use case, you don't let users enter a password that is used as the symmetric key the creation/storage of the symmetric key is problematic. See http://en.wikipedia.org/wiki/Symmetric_encryption for more details. Use public-key cryptography whenever applicable.

@tudormunteanu
Copy link

So the code itself is not dangerous, but the idea of using AES? Let's assume the dev who decides to use AES has a good reason.

@gonecoding
Copy link
Author

You nailed it, Tudor. :-)

@tudormunteanu
Copy link

Also, I'm not really sure why, but I have the feeling that the key sent to CCCrypt() is not in the right format. For example it's not compatible with a key generated by openSSL (assuming both IVs are 0). My guess is that the code above works as encrypting and decryption, because the same mistake is done in both cases, but this might not be the right way of doing things. This is my observation after 2 days of trying to decrypt files encrypted with openSSL.

@radhar
Copy link

radhar commented Mar 12, 2012

Can any one help me......why do i get a memory leak at the line " NSString *plainString = [[NSString alloc] initWithData:plainData encoding:NSUTF8StringEncoding];
" which is from the above code......while running on the device.

Thanks Inadvance :)

@tszming
Copy link

tszming commented Mar 13, 2012

@tudormunteanu, OpenSSL's key must be in hexadecimal, which is different from the above method which accept an ASCII key
@radhar, you need to release when you have called alloc to create an object

@radhar
Copy link

radhar commented Mar 14, 2012

@tszming: Thanks for your response.
here is the code plz go through the comment in the code.

  • (NSString *)AES256DecryptWithKey:(NSString *)key
    {
    NSData *encryptedData = [NSData dataWithBase64EncodedString:self];
    NSData *plainData = [encryptedData AES256DecryptWithKey:key];

    NSString *plainString = [[NSString alloc] initWithData:plainData encoding:NSUTF8StringEncoding]; //getting memory leak at this line.

    return [plainString autorelease]; //here the plainString is getting autoreleased....then y should we release it again
    }
    Can you give detailed explaination now where to release the object.....if required..

waiting for response...

@tszming
Copy link

tszming commented Mar 14, 2012

@radhar, your code is fine and I can't see a memory leak there.

@gonecoding
Copy link
Author

@radhar, you could also try this

NSString *plainString = [[[NSString alloc] initWithData:plainData encoding:NSUTF8StringEncoding] autorelease];
return plainString;

which is basically the same. Either way there's no memory leak in this method AFAIK.

@rnapier
Copy link

rnapier commented Mar 21, 2012

To @tqbf's comment, yes, this is terribly insecure. For a full discussion on the problems with this code, see http://robnapier.net/blog/aes-commoncrypto-564. I'm worried that this code seems to be very popular, leading to serious encryption problems within the iOS community. It's easy to use, which has led to broad usage, but it doesn't correctly generate the key (no reasonable KDF, and no salt), and it doesn't provide an IV.

It's good to see an easy-to-use AES implementation, but this code is not something people should use without careful understanding of what it does. It dramatically constricts the keyspace, and has significant vulnerabilities in the first block due to the lack of IV.

To @tudormunteanu's comment about OpenSSL, the issue is that there is no standard for AES data format. OpenSSL uses EVP_BytesToKey as their KDF. AES256Encrypt doesn't use a KDF at all (which is why the keyspace is so constricted). The standard is PBKDF2, but there is no standard for the number of rounds. Some implementations use bcrypt or scrypt, but there isn't any standard there. OpenSSL also has its own format for providing the IV that will not be compatible with this code.

To put code where my mouth is, I offer https://github.com/rnapier/RNCryptor, which I intend to address the issues I've raised here, and I hope is as easy to use as this category. I'm still cleaning up some of the error handling, but the main encryption/decryption + HMAC code should be solid and stable.

@tudormunteanu
Copy link

@rnapier, thank you for the explanation.

@gonecoding
Copy link
Author

Rob, thanks for your clarification what the problem with this code is. I took down the Gist (or the code files in it) and added a link to your class. I hope you're OK with that.

When I find the time I will fork your repository and add "encryptString" / "decryptString" methods using BASE64 encoding to it — something that was provided by this code.

Again, thanks for pointing out the weaknesses in this code collection.

– Michael

@rnapier
Copy link

rnapier commented Mar 23, 2012

@gonecoding, what kind of API are you seeing for Base-64 encoding? From its name, it's not clear what encryptString: or decryptString: would take and return. My expectation would be that encryptString: would take an NSString and return an NSData. decryptString: is unclear. Perhaps decryptBase64:, which would take Base64 data and return an NSData. In any case, though it's challenging to see how that expands to the stream functions. If the Base64 data is in a file, can you use decryptFromURL:...? I would be thoughtful of what belongs in the cryptor and what doesn't. It's very easy to provide a function that does NSData <-> Base64 to wrap around the cryptor. And an NSStream that converts data to/from Base64 might be much more flexible than putting it directly into the cryptor.

Do you have a use specific case in mind? Fork and code is great; also feel free to open issues.

@evaristoyok
Copy link

Some body could help me, I get two errors

Undefined symbols for architecture i386:
"_STAssertTrue", referenced from:
-[BrowserController testStream] in SomeFile.o
"_STAssertEqualObjects", referenced from:
-[BrowserController testStream] in SomeFile.o
ld: symbol(s) not found for architecture i386
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@rnapier
Copy link

rnapier commented Apr 4, 2012

Have you pulled the entire project, or just dropped the files into another project? The failed symbols are from the unit tests. You don't need those in a real project. If you want to build the unit tests, you need to link with SenTestingKit.framework (this comes with Xcode).

@evaristoyok
Copy link

ok thanks. Now when I try to build in iOS 4.3 i got this error
dyld: lazy symbol binding failed: Symbol not found: _CCKeyDerivationPBKDF
and
dyld: Symbol not found: _CCKeyDerivationPBKDF
but it works fine in simulator 5.0

@rnapier
Copy link

rnapier commented Apr 10, 2012

See the discussion here: RNCryptor/RNCryptor#22. RNCryptor is currently designed to be iOS5 only. I'm considering whether to modify that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment