Skip to content
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

Changed certificate pinning to be optional #1117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ChatSecure.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,8 @@
D9A429D01F31169F00BD2545 /* UIAlertController+ChatSecure.swift in Sources */ = {isa = PBXBuildFile; fileRef = D9A429CF1F31169F00BD2545 /* UIAlertController+ChatSecure.swift */; };
D9A7756F1E43F8A200027864 /* ProxyXMPPStream.h in Headers */ = {isa = PBXBuildFile; fileRef = D9A7756D1E43F8A200027864 /* ProxyXMPPStream.h */; };
D9A775701E43F8A200027864 /* ProxyXMPPStream.m in Sources */ = {isa = PBXBuildFile; fileRef = D9A7756E1E43F8A200027864 /* ProxyXMPPStream.m */; };
D9A7BCE71E4554E200888A8E /* OTRXMPPStream.h in Headers */ = {isa = PBXBuildFile; fileRef = D9A7BCE51E4554E200888A8E /* OTRXMPPStream.h */; };

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy

D9A7BCE81E4554E200888A8E /* OTRXMPPStream.m in Sources */ = {isa = PBXBuildFile; fileRef = D9A7BCE61E4554E200888A8E /* OTRXMPPStream.m */; };
D9ABD71E1ED787EE00219A9C /* OTRHTMLItem.h in Headers */ = {isa = PBXBuildFile; fileRef = D9ABD71C1ED787EE00219A9C /* OTRHTMLItem.h */; };
D9ABD71F1ED787EE00219A9C /* OTRHTMLItem.m in Sources */ = {isa = PBXBuildFile; fileRef = D9ABD71D1ED787EE00219A9C /* OTRHTMLItem.m */; };
D9ABD72B1ED7886100219A9C /* OTRTextItem.h in Headers */ = {isa = PBXBuildFile; fileRef = D9ABD7291ED7886100219A9C /* OTRTextItem.h */; };
Expand Down Expand Up @@ -1116,6 +1118,8 @@
D9A429CF1F31169F00BD2545 /* UIAlertController+ChatSecure.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIAlertController+ChatSecure.swift"; sourceTree = "<group>"; };
D9A7756D1E43F8A200027864 /* ProxyXMPPStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ProxyXMPPStream.h; sourceTree = "<group>"; };
D9A7756E1E43F8A200027864 /* ProxyXMPPStream.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ProxyXMPPStream.m; sourceTree = "<group>"; };
D9A7BCE51E4554E200888A8E /* OTRXMPPStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OTRXMPPStream.h; sourceTree = "<group>"; };
D9A7BCE61E4554E200888A8E /* OTRXMPPStream.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OTRXMPPStream.m; sourceTree = "<group>"; };
D9ABD71C1ED787EE00219A9C /* OTRHTMLItem.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OTRHTMLItem.h; sourceTree = "<group>"; };
D9ABD71D1ED787EE00219A9C /* OTRHTMLItem.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OTRHTMLItem.m; sourceTree = "<group>"; };
D9ABD7291ED7886100219A9C /* OTRTextItem.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OTRTextItem.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1418,6 +1422,8 @@
D9F8C3C11FBFD2CA00D4B857 /* RoomManager.swift */,
D98B8E301E4CF90400A713E1 /* OTRServerCapabilities.h */,
D98B8E311E4CF90400A713E1 /* OTRServerCapabilities.m */,
D9A7BCE51E4554E200888A8E /* OTRXMPPStream.h */,
D9A7BCE61E4554E200888A8E /* OTRXMPPStream.m */,
D9A7756D1E43F8A200027864 /* ProxyXMPPStream.h */,
D9A7756E1E43F8A200027864 /* ProxyXMPPStream.m */,
63D639E11D12124F002B4175 /* OTRStreamManagementDelegate.swift */,
Expand Down Expand Up @@ -2205,6 +2211,7 @@
D93DDB961BA79A9800CD8331 /* OTRChatDemo.h in Headers */,
D93DDB2E1BA79A7000CD8331 /* UIActivity+ChatSecure.h in Headers */,
D9BEF8E01DCE6E12009945D1 /* OTRXMPPManager_Private.h in Headers */,
D9A7BCE71E4554E200888A8E /* OTRXMPPStream.h in Headers */,
D93DDB4B1BA79A7300CD8331 /* OTRXMPPTorManager.h in Headers */,
D93DDB891BA79A8C00CD8331 /* OTRXMPPTorAccount.h in Headers */,
D93DDBA81BA79AAA00CD8331 /* OTRXMPPServerListViewController.h in Headers */,
Expand Down Expand Up @@ -3070,6 +3077,7 @@
D943AA421E6A0BA3007F3564 /* XMPPAccountCell.swift in Sources */,
D93DDA911BA79A2400CD8331 /* OTRStreamManagementYapStorage.m in Sources */,
D93DDA941BA79A2400CD8331 /* OTRXMPPBuddyTimers.m in Sources */,
D9A7BCE81E4554E200888A8E /* OTRXMPPStream.m in Sources */,
D9AE3A331BA8D9AB00255537 /* OTRConstants.m in Sources */,
D926C19B20FBDD9E0053C538 /* Compatibility.swift in Sources */,
63363FAE1CCAE29B00B0C720 /* OTRYapExtensions.swift in Sources */,
Expand Down
6 changes: 4 additions & 2 deletions ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ - (YapDatabaseConnection*) databaseConnection {
#pragma mark Private
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

- (XMPPStream*) newStream {
return [[XMPPStream alloc] init];
- (OTRXMPPStream*) newStream {
return [[OTRXMPPStream alloc] init];
}

- (void)setupStream
Expand Down Expand Up @@ -472,6 +472,8 @@ - (BOOL)startConnection
}

[self.xmppStream setHostPort:self.account.port];

[self.xmppStream setCertificatePinning:self.account.certificatePinning];


error = nil;
Expand Down
8 changes: 4 additions & 4 deletions ChatSecure/Classes/Controllers/XMPP/OTRXMPPManager_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
#import <ChatSecureCore/ChatSecureCore-Swift.h>
#import "OTRXMPPRoomManager.h"
#import "OTRXMPPBuddyTimers.h"
@import XMPPFramework;
#import "OTRXMPPStream.h"

NS_ASSUME_NONNULL_BEGIN
@interface OTRXMPPManager() <OTRCertificatePinningDelegate>

@property (nonatomic, strong, readonly) XMPPStream *xmppStream;
@property (nonatomic, strong, readonly) OTRXMPPStream *xmppStream;
@property (nonatomic, strong, readonly) XMPPReconnect *xmppReconnect;
@property (nonatomic, strong, readonly) XMPPvCardTempModule *xmppvCardTempModule;
@property (nonatomic, strong, readonly) XMPPvCardAvatarModule *xmppvCardAvatarModule;
Expand Down Expand Up @@ -56,8 +56,8 @@ NS_ASSUME_NONNULL_BEGIN
/** wtf. why isn't this being picked up by OTRProtocol */
- (void) connectUserInitiated:(BOOL)userInitiated;

/** Return a newly allocated stream object. This is overridden in OTRXMPPTorManager to use ProxyXMPPStream instead of XMPPStream */
- (XMPPStream*) newStream;
/** Return a newly allocated stream object. This is overridden in OTRXMPPTorManager to use ProxyXMPPStream instead of OTRXMPPStream */
- (OTRXMPPStream*) newStream;

@end
NS_ASSUME_NONNULL_END
18 changes: 18 additions & 0 deletions ChatSecure/Classes/Controllers/XMPP/OTRXMPPStream.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//
// OTRXMPPStream.h
// ChatSecure
//
// Created by Chris Ballinger on 2/3/17.
// Copyright © 2017 Chris Ballinger. All rights reserved.
//

@import Foundation;
@import XMPPFramework;

@interface OTRXMPPStream : XMPPStream

@property (nonatomic, readonly) BOOL certificatePinning;

- (void)setCertificatePinning:(BOOL)certificatePinning;

@end
18 changes: 18 additions & 0 deletions ChatSecure/Classes/Controllers/XMPP/OTRXMPPStream.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//
// OTRXMPPStream.m
// ChatSecure
//
// Created by Chris Ballinger on 2/3/17.
// Copyright © 2017 Chris Ballinger. All rights reserved.
//

#import "OTRXMPPStream.h"

@implementation OTRXMPPStream

- (void)setCertificatePinning:(BOOL)certificatePinning
{
_certificatePinning = certificatePinning;
}

@end
2 changes: 1 addition & 1 deletion ChatSecure/Classes/Controllers/XMPP/OTRXMPPTorManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ - (void) connectUserInitiated:(BOOL)userInitiated {
}

/** Override XMPPStream with XMPPProxyStream */
- (XMPPStream*) newStream {
- (OTRXMPPStream*) newStream {
return [[ProxyXMPPStream alloc] init];
}

Expand Down
4 changes: 2 additions & 2 deletions ChatSecure/Classes/Controllers/XMPP/ProxyXMPPStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
// Copyright © 2017 Chris Ballinger. All rights reserved.
//

@import XMPPFramework;
#import "OTRXMPPStream.h"
@import ProxyKit;

@interface ProxyXMPPStream : XMPPStream
@interface ProxyXMPPStream : OTRXMPPStream

/**
* Sets SOCKS proxy host and port
Expand Down
2 changes: 2 additions & 0 deletions ChatSecure/Classes/Model/Yap Storage/Accounts/OTRAccount.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ extern NSString *const OTRXMPPTorImageName;
/** Whether or not user would like to auto fetch media messages */
@property (nonatomic, readwrite) BOOL disableAutomaticURLFetching;

@property (nonatomic, readwrite) BOOL certificatePinning;

/**
* Setting this value does a comparison of against the previously value
* to invalidate the OTRImages cache.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ - (instancetype) initWithUsername:(NSString *)username accountType:(OTRAccountTy
_resource = [[self class] newResource];
self.autologin = YES;
self.rememberPassword = YES;
self.certificatePinning = NO;
}
return self;
}
Expand Down
13 changes: 10 additions & 3 deletions ChatSecure/Classes/Utilities/OTRCertificatePinning.m
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import "OTRConstants.h"
#import "OTRLog.h"
#import "OTRXMPPStream.h"


///////////////////////////////////////////////
Expand Down Expand Up @@ -296,23 +297,29 @@ + (id)publicKeyWithCertData:(NSData *)certData
**/
#pragma - mark GCDAsyncSockeTDelegate Methods

- (void)xmppStream:(XMPPStream *)sender didReceiveTrust:(SecTrustRef)trust completionHandler:(void (^)(BOOL))completionHandler
- (void)xmppStream:(OTRXMPPStream *)sender didReceiveTrust:(SecTrustRef)trust completionHandler:(void (^)(BOOL))completionHandler
{
BOOL certificatePinning = sender.certificatePinning;
NSString *hostName = sender.myJID.domain;
// We should have a hostName. If we don't, something is wrong.
NSParameterAssert(hostName.length > 0);
if (!hostName.length) {
completionHandler(NO);
}
BOOL trusted = [self isValidPinnedTrust:trust withHostName:hostName];
BOOL trusted = NO;
if (certificatePinning) {
trusted = [self isValidPinnedTrust:trust withHostName:hostName];
}
if (!trusted) {
//Delegate firing off for user to verify with status
SecTrustResultType result;
SecPolicyRef policy = SecPolicyCreateSSL(true, (__bridge CFStringRef)hostName);
SecTrustSetPolicies(trust, policy);
OSStatus status = SecTrustEvaluate(trust, &result);
CFRelease(policy);
if ([self.delegate respondsToSelector:@selector(newTrust:withHostName:systemTrustResult:)] && status == noErr) {
if (!certificatePinning && (result == kSecTrustResultProceed || result == kSecTrustResultUnspecified)) {
trusted = YES;
} else if ([self.delegate respondsToSelector:@selector(newTrust:withHostName:systemTrustResult:)] && status == noErr) {
[self.delegate newTrust:trust withHostName:hostName systemTrustResult:result];
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ extern NSString *const kOTRXLFormResourceTextFieldTag;
extern NSString *const kOTRXLFormXMPPServerTag;
extern NSString *const kOTRXLFormUseTorTag;
extern NSString *const kOTRXLFormAutomaticURLFetchTag;
extern NSString *const kOTRXLFormCertificatePinningTag;


@interface XLFormDescriptor (OTRAccount)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

NSString *const kOTRXLFormUseTorTag = @"kOTRXLFormUseTorTag";
NSString *const kOTRXLFormAutomaticURLFetchTag = @"kOTRXLFormAutomaticURLFetchTag";
NSString *const kOTRXLFormCertificatePinningTag = @"kOTRXLFormCertificatePinningTag";


@implementation XLFormDescriptor (OTRAccount)
Expand All @@ -59,6 +60,7 @@ + (instancetype) existingAccountFormWithAccount:(OTRAccount *)account
torRow.hidden = @YES;
}
[[descriptor formRowWithTag:kOTRXLFormAutomaticURLFetchTag] setValue:@(!xmppAccount.disableAutomaticURLFetching)];
[[descriptor formRowWithTag:kOTRXLFormCertificatePinningTag] setValue:@(xmppAccount.certificatePinning)];
}
if (account.accountType == OTRAccountTypeXMPPTor) {
XLFormRowDescriptor *torRow = [descriptor formRowWithTag:kOTRXLFormUseTorTag];
Expand Down Expand Up @@ -135,6 +137,7 @@ + (XLFormDescriptor *)formForAccountType:(OTRAccountType)accountType createAccou
otherSection.footerTitle = AUTO_URL_FETCH_WARNING_STRING();
otherSection.hidden = [NSString stringWithFormat:@"$%@==0", kOTRXLFormShowAdvancedTag];
[otherSection addFormRow:[self autoFetchRowDescriptorWithValue:YES]];
[otherSection addFormRow:[self certificatePinningRowDescriptorWithValue:NO]];

[descriptor addFormSection:basicSection];
[descriptor addFormSection:serverSection];
Expand Down Expand Up @@ -167,6 +170,7 @@ + (XLFormDescriptor *)formForAccountType:(OTRAccountType)accountType createAccou
[advancedSection addFormRow:[self torRowDescriptorWithValue:NO]];
}
[advancedSection addFormRow:[self autoFetchRowDescriptorWithValue:YES]];
[advancedSection addFormRow:[self certificatePinningRowDescriptorWithValue:NO]];

break;
}
Expand All @@ -179,6 +183,7 @@ + (XLFormDescriptor *)formForAccountType:(OTRAccountType)accountType createAccou

[advancedSection addFormRow:[self resourceRowDescriptorWithValue:nil]];
[advancedSection addFormRow:[self autoFetchRowDescriptorWithValue:YES]];
[advancedSection addFormRow:[self certificatePinningRowDescriptorWithValue:NO]];

break;
}
Expand Down Expand Up @@ -277,6 +282,12 @@ + (XLFormRowDescriptor*) torRowDescriptorWithValue:(BOOL)value {
return torRow;
}

+ (XLFormRowDescriptor*) certificatePinningRowDescriptorWithValue:(BOOL)value {
XLFormRowDescriptor *certificatePinningRow = [XLFormRowDescriptor formRowDescriptorWithTag:kOTRXLFormCertificatePinningTag rowType:XLFormRowDescriptorTypeBooleanSwitch title:CERTIFICATE_PINNING_STRING()];
certificatePinningRow.value = @(value);
return certificatePinningRow;
}

+ (XLFormRowDescriptor *)resourceRowDescriptorWithValue:(NSString *)value
{
XLFormRowDescriptor *resourceRowDescriptor = [XLFormRowDescriptor formRowDescriptorWithTag:kOTRXLFormResourceTextFieldTag rowType:XLFormRowDescriptorTypeText title:RESOURCE_STRING()];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ - (void)moveAccountValues:(OTRXMPPAccount *)account intoForm:(XLFormDescriptor *
XLFormRowDescriptor *autofetch = [form formRowWithTag:kOTRXLFormAutomaticURLFetchTag];
autofetch.value = @(!account.disableAutomaticURLFetching);

XLFormRowDescriptor *certificatePinning = [form formRowWithTag:kOTRXLFormCertificatePinningTag];
certificatePinning.value = @(account.certificatePinning);

[[form formRowWithTag:kOTRXLFormResourceTextFieldTag] setValue:account.resource];
}

Expand Down Expand Up @@ -145,6 +148,11 @@ - (OTRXMPPAccount *)moveValues:(XLFormDescriptor *)form intoAccount:(OTRXMPPAcco
account.disableAutomaticURLFetching = !autofetch.boolValue;
}

NSNumber *certificatePinning = [[form formRowWithTag:kOTRXLFormCertificatePinningTag] value];
if (certificatePinning) {
account.certificatePinning = [certificatePinning boolValue];
}

// Post-process values via XMPPJID for stringprep

if (!jidDomain.length) {
Expand Down
2 changes: 2 additions & 0 deletions OTRAssets/Strings/OTRStrings.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ FOUNDATION_EXPORT NSString* Basic_Setup_Hint();
FOUNDATION_EXPORT NSString* Best_Available();
/** "Cancel", Cancel an alert window */
FOUNDATION_EXPORT NSString* CANCEL_STRING();
/** "Certificate Pinning", */
FOUNDATION_EXPORT NSString* CERTIFICATE_PINNING_STRING();
/** "ChatSecure Push", Title for ChatSecure Push (this probably doesnt need to be translated) */
FOUNDATION_EXPORT NSString* CHATSECURE_PUSH_STRING();
/** "Chats", Title for chats view */
Expand Down
2 changes: 2 additions & 0 deletions OTRAssets/Strings/OTRStrings.m
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@
NSString* Best_Available() { return [OTRLanguageManager translatedString:@"Best Available"]; }
/** "Cancel", Cancel an alert window */
NSString* CANCEL_STRING() { return [OTRLanguageManager translatedString:@"Cancel"]; }
/** "Certificate Pinning", */
NSString* CERTIFICATE_PINNING_STRING() { return [OTRLanguageManager translatedString:@"Certificate Pinning"]; }
/** "ChatSecure Push", Title for ChatSecure Push (this probably doesnt need to be translated) */
NSString* CHATSECURE_PUSH_STRING() { return [OTRLanguageManager translatedString:@"ChatSecure Push"]; }
/** "Chats", Title for chats view */
Expand Down