Skip to content

Commit

Permalink
Fix some fundamental flaws in how the keychain is handled.
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Morris committed Oct 29, 2013
1 parent 7adbf4f commit c3bc7b5
Show file tree
Hide file tree
Showing 9 changed files with 210 additions and 184 deletions.
26 changes: 20 additions & 6 deletions Classes/Dialogs/TDCServerSheet.m
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,11 @@ - (void)load
self.autoReconnectCheck.state = self.config.autoReconnect;
self.connectionUsesSSLCheck.state = self.config.connectionUsesSSL;
self.serverNameField.stringValue = self.config.clientName;
self.serverPasswordField.stringValue = self.config.serverPassword;

if (self.config.serverPasswordIsSet) {
self.serverPasswordField.stringValue = self.config.serverPassword;
}

self.serverPortField.stringValue = [NSString stringWithInteger:self.config.serverPort];

#ifdef TEXTUAL_BUILT_WITH_ICLOUD_SUPPORT
Expand Down Expand Up @@ -358,9 +362,11 @@ - (void)load
} else {
self.alternateNicknamesField.stringValue = NSStringEmptyPlaceholder;
}

self.nicknamePasswordField.stringValue = self.config.nicknamePassword;


if (self.config.nicknamePasswordIsSet) {
self.nicknamePasswordField.stringValue = self.config.nicknamePassword;
}

/* Messages */
self.sleepModeQuitMessageField.stringValue = self.config.sleepModeLeavingComment;
self.normalLeavingCommentField.stringValue = self.config.normalLeavingComment;
Expand All @@ -377,7 +383,11 @@ - (void)load

self.proxyAddressField.stringValue = self.config.proxyAddress;
self.proxyUsernameField.stringValue = self.config.proxyUsername;
self.proxyPasswordField.stringValue = self.config.proxyPassword;

if (self.config.proxyPasswordIsSet) {
self.proxyPasswordField.stringValue = self.config.proxyPassword;
}

self.proxyPortField.stringValue = [NSString stringWithInteger:self.config.proxyPort];

/* Connect Commands */
Expand Down Expand Up @@ -1035,7 +1045,11 @@ - (id)tableView:(NSTableView *)sender objectValueForTableColumn:(NSTableColumn *
if ([columnId isEqualToString:@"name"]) {
return c.channelName;
} else if ([columnId isEqualToString:@"pass"]) {
return c.secretKey;
if (c.secretKeyIsSet) {
return c.secretKey;
} else {
return NSStringEmptyPlaceholder;
}
} else if ([columnId isEqualToString:@"join"]) {
return @(c.autoJoin);
}
Expand Down
12 changes: 9 additions & 3 deletions Classes/Dialogs/TDChannelSheet.m
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,15 @@ - (void)load
self.channelNameField.stringValue = self.config.channelName;
self.defaultModesField.stringValue = self.config.defaultModes;
self.defaultTopicField.stringValue = self.config.defaultTopic;
self.encryptionKeyField.stringValue = self.config.encryptionKey;
self.secretKeyField.stringValue = self.config.secretKey;


if (self.config.encryptionKeyIsSet) {
self.encryptionKeyField.stringValue = self.config.encryptionKey;
}

if (self.config.secretKeyIsSet) {
self.secretKeyField.stringValue = self.config.secretKey;
}

self.autoJoinCheck.state = self.config.autoJoin;
self.ignoreHighlightsCheck.state = self.config.ignoreHighlights;
self.pushNotificationsCheck.state = self.config.pushNotifications;
Expand Down
3 changes: 3 additions & 0 deletions Classes/Headers/IRCChannelConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ typedef enum IRCChannelType : NSInteger {
@property (nonatomic, assign) BOOL ignoreHighlights;
@property (nonatomic, assign) BOOL ignoreJPQActivity;

@property (nonatomic, assign) BOOL encryptionKeyIsSet;
@property (nonatomic, assign) BOOL secretKeyIsSet;

- (void)destroyKeychains;

- (id)initWithDictionary:(NSDictionary *)dic;
Expand Down
4 changes: 4 additions & 0 deletions Classes/Headers/IRCClientConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ NSComparisonResult IRCChannelDataSort(IRCChannel *s1, IRCChannel *s2, void *cont
@property (nonatomic, strong) NSString *normalLeavingComment;
@property (nonatomic, strong) NSString *sleepModeLeavingComment;

@property (nonatomic, assign) BOOL serverPasswordIsSet;
@property (nonatomic, assign) BOOL nicknamePasswordIsSet;
@property (nonatomic, assign) BOOL proxyPasswordIsSet;

- (id)initWithDictionary:(NSDictionary *)dic;
- (NSMutableDictionary *)dictionaryValue;

Expand Down
118 changes: 46 additions & 72 deletions Classes/IRC/IRCChannelConfig.m
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@

@implementation IRCChannelConfig

@synthesize secretKey = _secretKey;
@synthesize encryptionKey = _encryptionKey;

- (id)init
{
if ((self = [super init])) {
Expand Down Expand Up @@ -77,87 +74,57 @@ - (void)dealloc

- (NSString *)encryptionKey
{
NSString *kcPassword = NSStringEmptyPlaceholder;

/* Only read from keychain if our value is nil. Let the set command
handle any changes to the actual property after that. */
if (_encryptionKey == nil) {
kcPassword = [AGKeychain getPasswordFromKeychainItem:@"Textual (Blowfish Encryption)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cblowfish.%@", self.itemUUID]];

if (kcPassword) {
if ([kcPassword isEqualToString:_encryptionKey] == NO) {
_encryptionKey = nil;
_encryptionKey = kcPassword;
}
}
}
NSString *kcPassword = [AGKeychain getPasswordFromKeychainItem:@"Textual (Blowfish Encryption)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cblowfish.%@", self.itemUUID]];

return _encryptionKey;
return kcPassword;
}

- (NSString *)secretKey
{
NSString *kcPassword = NSStringEmptyPlaceholder;

if (_secretKey == nil) {
kcPassword = [AGKeychain getPasswordFromKeychainItem:@"Textual (Channel JOIN Key)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];

if (kcPassword) {
if ([kcPassword isEqualToString:_secretKey] == NO) {
_secretKey = nil;
_secretKey = kcPassword;
}
}
}
NSString *kcPassword = [AGKeychain getPasswordFromKeychainItem:@"Textual (Channel JOIN Key)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];

return _secretKey;
return kcPassword;
}

- (void)setEncryptionKey:(NSString *)pass
{
if ([_encryptionKey isEqualToString:pass] == NO) {
if (NSObjectIsEmpty(pass)) {
[AGKeychain deleteKeychainItem:@"Textual (Blowfish Encryption)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cblowfish.%@", self.itemUUID]];
} else {
[AGKeychain modifyOrAddKeychainItem:@"Textual (Blowfish Encryption)"
withItemKind:@"application password"
forUsername:nil
withNewPassword:pass
serviceName:[NSString stringWithFormat:@"textual.cblowfish.%@", self.itemUUID]];
}

_encryptionKey = nil;
_encryptionKey = pass;
self.encryptionKeyIsSet = NSObjectIsNotEmpty(pass);

if (self.encryptionKeyIsSet == NO) {
[AGKeychain deleteKeychainItem:@"Textual (Blowfish Encryption)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cblowfish.%@", self.itemUUID]];
} else {
[AGKeychain modifyOrAddKeychainItem:@"Textual (Blowfish Encryption)"
withItemKind:@"application password"
forUsername:nil
withNewPassword:pass
serviceName:[NSString stringWithFormat:@"textual.cblowfish.%@", self.itemUUID]];
}
}

- (void)setSecretKey:(NSString *)pass
{
if ([_secretKey isEqualToString:pass] == NO) {
if (NSObjectIsEmpty(pass)) {
[AGKeychain deleteKeychainItem:@"Textual (Channel JOIN Key)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];
} else {
[AGKeychain modifyOrAddKeychainItem:@"Textual (Channel JOIN Key)"
withItemKind:@"application password"
forUsername:nil
withNewPassword:pass
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];
}

_secretKey = nil;
_secretKey = pass;
self.secretKeyIsSet = NSObjectIsNotEmpty(pass);

if (self.secretKeyIsSet == NO) {
[AGKeychain deleteKeychainItem:@"Textual (Channel JOIN Key)"
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];
} else {
[AGKeychain modifyOrAddKeychainItem:@"Textual (Channel JOIN Key)"
withItemKind:@"application password"
forUsername:nil
withNewPassword:pass
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];
}
}

Expand All @@ -172,6 +139,9 @@ - (void)destroyKeychains
withItemKind:@"application password"
forUsername:nil
serviceName:[NSString stringWithFormat:@"textual.cjoinkey.%@", self.itemUUID]];

self.secretKeyIsSet = NO;
self.encryptionKeyIsSet = NO;
}

#pragma mark -
Expand All @@ -191,6 +161,7 @@ + (NSDictionary *)seedDictionary:(NSString *)channelName
- (id)initWithDictionary:(NSDictionary *)dic
{
if ((self = [self init])) {
/* General preferences. */
self.type = (IRCChannelType)NSDictionaryIntegerKeyValueCompare(dic, @"channelType", self.type);

self.itemUUID = NSDictionaryObjectKeyValueCompare(dic, @"uniqueIdentifier", self.itemUUID);
Expand All @@ -206,8 +177,7 @@ - (id)initWithDictionary:(NSDictionary *)dic
self.defaultModes = NSDictionaryObjectKeyValueCompare(dic, @"defaultMode", self.defaultModes);
self.defaultTopic = NSDictionaryObjectKeyValueCompare(dic, @"defaultTopic", self.defaultTopic);

// ---- // Migrate to keychain.

/* Migrate to keychain.*/
NSString *oldEncKey = [dic stringForKey:@"encryptionKey"];
NSString *oldJoinKey = [dic stringForKey:@"secretJoinKey"];

Expand All @@ -218,7 +188,11 @@ - (id)initWithDictionary:(NSDictionary *)dic
if (NSObjectIsNotEmpty(oldJoinKey)) {
[self setSecretKey:oldJoinKey];
}


/* Establish state. */
self.secretKeyIsSet = NSObjectIsNotEmpty(self.secretKey);
self.encryptionKeyIsSet = NSObjectIsNotEmpty(self.encryptionKey);

return self;
}

Expand Down
33 changes: 18 additions & 15 deletions Classes/IRC/IRCClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ - (BOOL)isSupportedMessageEncryptionFormat:(NSString *)message channel:(IRCChann
NSObjectIsEmptyAssertReturn(message, NO);

if (channel && (channel.isChannel || channel.isPrivateMessage)) {
return NSObjectIsNotEmpty(channel.config.encryptionKey);
return channel.config.encryptionKeyIsSet;
}

return NO;
Expand Down Expand Up @@ -3066,7 +3066,7 @@ - (void)ircConnectionDidConnect:(IRCConnection *)sender

[self send:IRCPrivateCommandIndex("cap"), @"LS", nil];

if (NSObjectIsNotEmpty(self.config.serverPassword)) {
if (self.config.serverPasswordIsSet) {
[self send:IRCPrivateCommandIndex("pass"), self.config.serverPassword, nil];
}

Expand Down Expand Up @@ -3590,7 +3590,7 @@ - (void)receiveText:(IRCMessage *)m command:(NSString *)command text:(NSString *
if ([cleanedText containsIgnoringCase:token]) {
continueNickServScan = NO;

NSObjectIsEmptyAssertLoopBreak(self.config.nicknamePassword);
NSAssertReturnLoopContinue(self.config.nicknamePasswordIsSet);

NSString *IDMessage = [NSString stringWithFormat:@"IDENTIFY %@", self.config.nicknamePassword];

Expand Down Expand Up @@ -3879,7 +3879,7 @@ - (void)receiveJoin:(IRCMessage *)m
}
}

if (NSObjectIsNotEmpty(c.config.encryptionKey)) {
if (c.config.encryptionKeyIsSet) {
[c.client printDebugInformation:TXTLS(@"BlowfishEncryptionStarted") channel:c];
}
}
Expand Down Expand Up @@ -4337,7 +4337,7 @@ - (void)receiveMode:(IRCMessage *)m
if (saveKey) {
[c.config setSecretKey:newSecretKeyActual];
}
} else if (oldSecretKey.modeIsSet == YES && newSecretKey.modeIsSet == NO && NSObjectIsNotEmpty(c.secretKey)) {
} else if (oldSecretKey.modeIsSet == YES && newSecretKey.modeIsSet == NO && c.config.secretKeyIsSet) {
BOOL saveKey = [TLOPopupPrompts dialogWindowWithQuestion:TXTFLS(@"ChannelKeyRemovalDetectedDialogMessage", c.name)
title:TXTLS(@"ChannelKeyRemovalDetectedDialogTitle")
defaultButton:TXTLS(@"YesButton")
Expand Down Expand Up @@ -4504,7 +4504,7 @@ - (BOOL)isCapAvailable:(NSString *)cap
[cap isEqualIgnoringCase:@"server-time"] ||
[cap isEqualIgnoringCase:@"znc.in/server-time"] ||
[cap isEqualIgnoringCase:@"znc.in/server-time-iso"] ||
([cap isEqualIgnoringCase:@"sasl"] && NSObjectIsNotEmpty(self.config.nicknamePassword)));
([cap isEqualIgnoringCase:@"sasl"] && self.config.nicknamePasswordIsSet));
}

- (void)cap:(NSString *)cap result:(BOOL)supported
Expand Down Expand Up @@ -5072,7 +5072,7 @@ - (void)receiveNumericReply:(IRCMessage *)m

NSString *secretKey = [c.modeInfo modeInfoFor:@"k"].modeParamater;

if (NSObjectIsEmpty(c.secretKey) && NSObjectIsNotEmpty(secretKey)) {
if (c.config.secretKeyIsSet == NO && NSObjectIsNotEmpty(secretKey)) {
/* We offer to remember the key when we found one and our configuration does not
already have one. */

Expand All @@ -5086,7 +5086,7 @@ - (void)receiveNumericReply:(IRCMessage *)m
if (saveKey) {
[c.config setSecretKey:secretKey];
}
} else if (NSObjectIsEmpty(secretKey) && NSObjectIsNotEmpty(c.secretKey)) {
} else if (NSObjectIsEmpty(secretKey) && c.config.secretKeyIsSet) {
/* We have a key set on a channel that no longer has one. */
BOOL saveKey = [TLOPopupPrompts dialogWindowWithQuestion:TXTFLS(@"ChannelKeyRemovalDetectedDialogMessage", c.name)
title:TXTLS(@"ChannelKeyRemovalDetectedDialogTitle")
Expand Down Expand Up @@ -6595,11 +6595,11 @@ - (void)joinChannel:(IRCChannel *)channel password:(NSString *)password
channel.status = IRCChannelJoining;

if (NSObjectIsEmpty(password)) {
password = channel.secretKey;
}

if (NSObjectIsEmpty(password)) {
password = nil;
if (channel.config.secretKey) {
password = channel.secretKey;
} else {
password = nil;
}
}

[self forceJoinChannel:channel.name password:password];
Expand Down Expand Up @@ -6708,7 +6708,7 @@ - (void)quickJoin:(NSArray *)chans withKeys:(BOOL)passKeys

c.status = IRCChannelJoining;

if (NSObjectIsNotEmpty(c.secretKey)) {
if (c.config.secretKeyIsSet) {
if (passKeys == NO) {
continue;
}
Expand Down Expand Up @@ -6738,7 +6738,10 @@ - (void)quickJoin:(NSArray *)chans withKeys:(BOOL)passKeys
}

[channelList setString:c.name];
[passwordList setString:c.secretKey];

if (c.config.secretKeyIsSet) {
[passwordList setString:c.secretKey];
}

channelCount = 1; // To match setString: statements up above.
} else {
Expand Down
Loading

0 comments on commit c3bc7b5

Please sign in to comment.