-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Secure Cell passphrase API: ObjCThemis #609
Changes from 2 commits
419a1b7
0f82553
d44c79a
f4867da
7769d8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -18,13 +18,38 @@ | |||||
#import <objcthemis/serror.h> | ||||||
#import <themis/themis.h> | ||||||
|
||||||
@interface TSCellSealWithPassphrase : TSCellSeal | ||||||
@end | ||||||
|
||||||
@implementation TSCellSeal | ||||||
|
||||||
- (nullable instancetype)initWithKey:(NSData *)key { | ||||||
self = [super initWithKey:key]; | ||||||
return self; | ||||||
} | ||||||
|
||||||
- (nullable instancetype)initWithPassphrase:(NSString *)passphrase | ||||||
{ | ||||||
return [self initWithPassphrase:passphrase usingEncoding:NSUTF8StringEncoding]; | ||||||
} | ||||||
|
||||||
- (nullable instancetype)initWithPassphrase:(NSString *)passphrase | ||||||
usingEncoding:(NSStringEncoding)encoding | ||||||
{ | ||||||
NSData *encoded = [passphrase dataUsingEncoding:encoding]; | ||||||
if (!encoded) { | ||||||
return nil; | ||||||
} | ||||||
return [self initWithPassphraseData:encoded]; | ||||||
} | ||||||
|
||||||
- (nullable instancetype)initWithPassphraseData:(NSData *)passphrase | ||||||
{ | ||||||
self = [TSCellSealWithPassphrase alloc]; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why we do this as two lines?
won't it work like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uh... I forgot why. Just tested it, it seems I did this to suppress Xcode warning about the initializer not calling an init method on "self". With this code self = [[TSCellSealWithPassphrase alloc] initWithPassphraseData:passphrase]]; the newly allocated object is not Should I add a comment in the code about that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes please put a code comment. something deeply triggers me here, but i don't really understand why and i don't remember all the specifics of ObjC coding 🤔 |
||||||
self = [self initWithPassphraseData:passphrase]; | ||||||
return self; | ||||||
} | ||||||
|
||||||
#pragma mark - Encryption | ||||||
|
||||||
- (nullable NSData *)encrypt:(NSData *)message | ||||||
|
@@ -161,3 +186,100 @@ - (nullable NSData *)unwrapData:(NSData *)message error:(NSError **)error | |||||
} | ||||||
|
||||||
@end | ||||||
|
||||||
@implementation TSCellSealWithPassphrase | ||||||
|
||||||
- (nullable instancetype)initWithPassphraseData:(NSData *)passphrase | ||||||
{ | ||||||
// Call grandparent TSCell initializer. We store the passphrase as a "key". | ||||||
self = [super initWithKey:passphrase]; | ||||||
return self; | ||||||
} | ||||||
|
||||||
- (nullable NSData *)encrypt:(NSData *)data | ||||||
context:(nullable NSData *)context | ||||||
error:(NSError **)error | ||||||
{ | ||||||
themis_status_t res = THEMIS_FAIL; | ||||||
|
||||||
size_t encryptedLengh = 0; | ||||||
res = themis_secure_cell_encrypt_seal_with_passphrase(self.key.bytes, | ||||||
self.key.length, | ||||||
context.bytes, | ||||||
context.length, | ||||||
data.bytes, | ||||||
data.length, | ||||||
NULL, | ||||||
&encryptedLengh); | ||||||
if (res != THEMIS_BUFFER_TOO_SMALL) { | ||||||
if (error) { | ||||||
*error = SCERROR(res, @"Secure Cell encryption failed"); | ||||||
} | ||||||
return nil; | ||||||
} | ||||||
|
||||||
NSMutableData *encryptedData = [NSMutableData dataWithLength:encryptedLengh]; | ||||||
|
||||||
res = themis_secure_cell_encrypt_seal_with_passphrase(self.key.bytes, | ||||||
self.key.length, | ||||||
context.bytes, | ||||||
context.length, | ||||||
data.bytes, | ||||||
data.length, | ||||||
encryptedData.mutableBytes, | ||||||
&encryptedLengh); | ||||||
if (res != THEMIS_SUCCESS) { | ||||||
if (error) { | ||||||
*error = SCERROR(res, @"Secure Cell encryption failed"); | ||||||
} | ||||||
return nil; | ||||||
} | ||||||
|
||||||
[encryptedData setLength:encryptedLengh]; | ||||||
return encryptedData; | ||||||
} | ||||||
|
||||||
- (nullable NSData *)decrypt:(NSData *)data | ||||||
context:(nullable NSData *)context | ||||||
error:(NSError **)error | ||||||
{ | ||||||
themis_status_t res = THEMIS_FAIL; | ||||||
|
||||||
size_t decryptedLengh = 0; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
and below There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (you see, i'm actually reading your PRs) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
res = themis_secure_cell_decrypt_seal_with_passphrase(self.key.bytes, | ||||||
self.key.length, | ||||||
context.bytes, | ||||||
context.length, | ||||||
data.bytes, | ||||||
data.length, | ||||||
NULL, | ||||||
&decryptedLengh); | ||||||
if (res != THEMIS_BUFFER_TOO_SMALL) { | ||||||
if (error) { | ||||||
*error = SCERROR(res, @"Secure Cell decryption failed"); | ||||||
} | ||||||
return nil; | ||||||
} | ||||||
|
||||||
NSMutableData *decryptedData = [NSMutableData dataWithLength:decryptedLengh]; | ||||||
|
||||||
res = themis_secure_cell_decrypt_seal_with_passphrase(self.key.bytes, | ||||||
self.key.length, | ||||||
context.bytes, | ||||||
context.length, | ||||||
data.bytes, | ||||||
data.length, | ||||||
decryptedData.mutableBytes, | ||||||
&decryptedLengh); | ||||||
if (res != THEMIS_SUCCESS) { | ||||||
if (error) { | ||||||
*error = SCERROR(res, @"Secure Cell decryption failed"); | ||||||
} | ||||||
return nil; | ||||||
} | ||||||
|
||||||
[decryptedData setLength:decryptedLengh]; | ||||||
return decryptedData; | ||||||
} | ||||||
|
||||||
@end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol poor users :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but won't it confuse to have this interface in objc, but not to have in swift? maybe we should remove it completely, as most users use swift anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunno... Normally, the users would not want to use this interface anyway, even in Objective-C, and if they really need to use a different encoding, they can always use the one that takes NSData. So by removing this interface we do not remove any features, only lower usability.
On the other hand, once we are able to ship Swift code with ObjCThemis (I imagine this should be possible when Swift 4 becomes obsolete), we can forward this interface to Swift with an extension. So we can leave it in Objective-C with this in mind, if this interface is useful.
“Help me, @Lagovas. You're my only hope.” Please be a tie-breaker.
The default decisions will be to remove it as we can extend the API in a future version, but we won't be able to remove it once it's published.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that removing this interface will lead to lower usability, but I do think that having interface-that-you-can't-use leads to frustration :)
My vote is to kill it 🔥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and done.