Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
#2979, #2437, #2247, #1836: Enable mysql cleartext auth without acces…
…s to keychain and with a warning to the user
  • Loading branch information
dmoagx committed Feb 24, 2018
1 parent 586312b commit 030eac5
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 22 deletions.
18 changes: 18 additions & 0 deletions Frameworks/SPMySQLFramework/Source/SPMySQLConnection.h
Expand Up @@ -61,6 +61,7 @@
pthread_t reconnectingThread;
uint64_t initialConnectTime;
unsigned long mysqlConnectionThreadId;
BOOL allowCleartextPlugin;

// Connection proxy
NSObject <SPMySQLConnectionProxy> *proxy;
Expand Down Expand Up @@ -175,6 +176,23 @@
- (void)addClientFlags:(SPMySQLClientFlags)opts;
- (void)removeClientFlags:(SPMySQLClientFlags)opts;

/**
* This tells the mysql client whether the cleartext auth plugin is permitted.
*
* If enabled, and requested by the server, the password will simply be transmitted
* in plaintext, instead of hashing it on the client first, thus this plugin is
* rather risky. It is mostly used when the server has to forward the password to another
* auth backend (which it can't do with the one-way hash).
*
* If it is not enabled, but requested by the server the connection will fail with
* error "plugin not enabled (2059)"
*
* WARNING: There are 2 ways to enable this plugin: Via this property or by setting
* the envvar LIBMYSQL_ENABLE_CLEARTEXT_PLUGIN to 1. Sadly ANY ONE of those two is
* sufficient to enable the plugin.
*/
@property (readwrite, assign, nonatomic) BOOL allowCleartextPlugin;

#pragma mark -
#pragma mark Connection and disconnection

Expand Down
10 changes: 8 additions & 2 deletions Frameworks/SPMySQLFramework/Source/SPMySQLConnection.m
Expand Up @@ -82,6 +82,7 @@ @implementation SPMySQLConnection
@synthesize delegateQueryLogging;
@synthesize lastQueryWasCancelled;
@synthesize clientFlags = clientFlags;
@synthesize allowCleartextPlugin = allowCleartextPlugin;

#pragma mark -
#pragma mark Getters and Setters
Expand Down Expand Up @@ -609,6 +610,9 @@ - (MYSQL *)_makeRawMySQLConnectionWithEncoding:(NSString *)encodingName isMaster
NSStringEncoding connectEncodingNS = [SPMySQLConnection stringEncodingForMySQLCharset:[encodingName UTF8String]];
mysql_options(theConnection, MYSQL_SET_CHARSET_NAME, [encodingName UTF8String]);

my_bool cleartextAllowed = [self allowCleartextPlugin] ? TRUE : FALSE;
mysql_options(theConnection, MYSQL_ENABLE_CLEARTEXT_PLUGIN, &cleartextAllowed);

// Set up the connection variables in the format MySQL needs, from the class-wide variables
const char *theHost = NULL;
const char *theUsername = "";
Expand Down Expand Up @@ -746,9 +750,11 @@ - (void)_mysqlConnection:(MYSQL *)connection wantsPassword:(void (^)(const char
passwd = [delegate passwordForConnection:self authPlugin:plugin];
}

// shortcut for empty/nil password
// shortcut for nil/empty passwords
if(![passwd length]) {
inBlock(NULL);
// nil means abort, "" is a valid password:
// only invoke the block when the password is @"", otherwise we'll skip it which will make mysql abort the connection
if(passwd) inBlock("");
return;
}

Expand Down
Expand Up @@ -68,6 +68,8 @@
* the secure store (Keychain), and the other connection details (user, host)
* can be used to look it up and supplied on demand.
*
* NOTE: This will be called on the thread SPMySQL is running on (which *MAY* be a background thread)!
*
* @param connection The connection instance to supply the password for
* @param pluginName The auth plugin libmysqlclients wants to use the password with
*/
Expand Down
53 changes: 51 additions & 2 deletions Interfaces/English.lproj/ConnectionView.xib
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="6751" systemVersion="13F1507" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none">
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="6751" systemVersion="13F1911" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none">
<dependencies>
<deployment identifier="macosx"/>
<development version="5100" identifier="xcode"/>
Expand All @@ -23,6 +23,9 @@
<outlet property="helpButton" destination="4829" id="5458"/>
<outlet property="progressIndicator" destination="5422" id="5426"/>
<outlet property="progressIndicatorText" destination="5423" id="5425"/>
<outlet property="requestPasswordAccessoryView" destination="eXu-WW-1SA" id="67S-wG-21A"/>
<outlet property="requestPasswordPasswordField" destination="iX8-1c-QGP" id="f2E-RB-7D1"/>
<outlet property="requestPasswordPluginNameField" destination="qYD-Yv-BoC" id="4Ps-j5-3U9"/>
<outlet property="saveFavoriteButton" destination="5869" id="5874"/>
<outlet property="socketColorField" destination="5896" id="5897"/>
<outlet property="socketConnectionFormContainer" destination="5162" id="5163"/>
Expand Down Expand Up @@ -2590,6 +2593,52 @@ DQ
</textField>
</subviews>
</customView>
<customView id="eXu-WW-1SA" userLabel="Request Password Accessory View">
<rect key="frame" x="-5" y="0.0" width="334" height="47"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<subviews>
<textField toolTip="The name of the authentication plugin MySQL wants to use" horizontalHuggingPriority="251" verticalHuggingPriority="750" id="kJN-ff-ksF">
<rect key="frame" x="-2" y="30" width="105" height="17"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" scrollable="YES" lineBreakMode="clipping" sendsActionOnEndEditing="YES" alignment="right" title="Plugin:" id="r9R-qm-igR">
<font key="font" metaFont="system"/>
<color key="textColor" name="controlTextColor" catalog="System" colorSpace="catalog"/>
<color key="backgroundColor" name="controlColor" catalog="System" colorSpace="catalog"/>
</textFieldCell>
</textField>
<textField horizontalHuggingPriority="251" verticalHuggingPriority="750" id="qYD-Yv-BoC">
<rect key="frame" x="107" y="30" width="229" height="17"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" scrollable="YES" lineBreakMode="clipping" sendsActionOnEndEditing="YES" title="$pluginName" id="6ge-00-IRy">
<font key="font" metaFont="systemBold"/>
<color key="textColor" name="controlTextColor" catalog="System" colorSpace="catalog"/>
<color key="backgroundColor" name="controlColor" catalog="System" colorSpace="catalog"/>
</textFieldCell>
</textField>
<secureTextField verticalHuggingPriority="750" id="iX8-1c-QGP">
<rect key="frame" x="109" y="0.0" width="225" height="22"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<secureTextFieldCell key="cell" scrollable="YES" lineBreakMode="clipping" selectable="YES" editable="YES" sendsActionOnEndEditing="YES" borderStyle="bezel" drawsBackground="YES" usesSingleLineMode="YES" id="de0-rh-heM">
<font key="font" metaFont="system"/>
<color key="textColor" name="textColor" catalog="System" colorSpace="catalog"/>
<color key="backgroundColor" name="textBackgroundColor" catalog="System" colorSpace="catalog"/>
<allowedInputSourceLocales>
<string>NSAllRomanInputSourcesLocaleIdentifier</string>
</allowedInputSourceLocales>
</secureTextFieldCell>
</secureTextField>
<textField horizontalHuggingPriority="251" verticalHuggingPriority="750" id="YdZ-UM-eof">
<rect key="frame" x="-2" y="2" width="105" height="17"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" scrollable="YES" lineBreakMode="clipping" sendsActionOnEndEditing="YES" alignment="right" title="Password:" id="afg-rd-MnW">
<font key="font" metaFont="system"/>
<color key="textColor" name="controlTextColor" catalog="System" colorSpace="catalog"/>
<color key="backgroundColor" name="controlColor" catalog="System" colorSpace="catalog"/>
</textFieldCell>
</textField>
</subviews>
<point key="canvasLocation" x="104" y="923.5"/>
</customView>
</objects>
<resources>
<image name="button_action" width="30" height="22"/>
Expand Down
19 changes: 19 additions & 0 deletions Source/SPConnectionController.h
Expand Up @@ -69,6 +69,8 @@
BOOL isConnecting;
BOOL isEditingConnection;
BOOL isTestingConnection;
NSString *agreedInsecurePlugin;
NSString *insecureOverridePassword;

// Standard details
NSInteger previousType;
Expand Down Expand Up @@ -164,6 +166,10 @@
IBOutlet NSMenuItem *favoritesSortByMenuItem;
IBOutlet NSView *exportPanelAccessoryView;
IBOutlet NSView *editButtonsView;

IBOutlet NSView *requestPasswordAccessoryView;
IBOutlet NSTextField *requestPasswordPluginNameField;
IBOutlet NSSecureTextField *requestPasswordPasswordField;

BOOL isEditingItemName;
BOOL reverseFavoritesSort;
Expand Down Expand Up @@ -214,6 +220,18 @@
@property (readwrite, retain) NSString *connectionSSHKeychainItemName;
@property (readwrite, retain) NSString *connectionSSHKeychainItemAccount;
@property (readwrite, assign) BOOL useCompression;
/**
* If the user was prompted to allow a connection with an insecure auth plugin,
* the name of that plugin will be stored here (not persisted) so that we
* don't have to ask again when duplicating a connection/reconnecting.
*/
@property (readwrite, copy, nonatomic) NSString *agreedInsecurePlugin;
/**
* If the user has given a password that is not the keychain password in
* the insecure auth plugin request, we will store it in memory and keep the
* other properties unchanged, since they are connected to GUI and/or backing stores
*/
@property (readwrite, copy, nonatomic) NSString *insecureOverridePassword;

#ifdef SP_CODA
@property (readwrite, assign) SPDatabaseDocument *dbDocument;
Expand All @@ -224,6 +242,7 @@

- (NSString *)keychainPassword;
- (NSString *)keychainPasswordForSSH;
- (NSString *)actualPasswordForAuthPlugin:(NSString *)pluginName;

// Connection processes
- (IBAction)initiateConnection:(id)sender;
Expand Down
153 changes: 146 additions & 7 deletions Source/SPConnectionController.m
Expand Up @@ -114,6 +114,9 @@ - (void)_startEditingConnection;

- (void)_documentWillClose:(NSNotification *)notification;

- (void)_beginRequestPasswordForInsecurePlugin:(NSString *)pluginName;
- (void)_insecurePasswordAlertDidEnd:(NSAlert *)alert returnCode:(NSInteger)returnCode contextInfo:(void *)contextInfo;

static NSComparisonResult _compareFavoritesUsingKey(id favorite1, id favorite2, void *key);
#endif

Expand Down Expand Up @@ -163,6 +166,8 @@ @implementation SPConnectionController
@synthesize sshKeyLocation;
@synthesize sshPort;
@synthesize useCompression;
@synthesize agreedInsecurePlugin = agreedInsecurePlugin;
@synthesize insecureOverridePassword = insecureOverridePassword;

#ifdef SP_CODA
@synthesize dbDocument;
Expand Down Expand Up @@ -199,6 +204,133 @@ - (NSString *)keychainPasswordForSSH
return kcSSHPassword;
}

/**
* This method is responsible for handing the user password to SPMySQL when it needs it.
* It will receive the name of the auth plugin mysql plans to use and with
* that it has to decide whether to fetch the password from keychain, or present
* additional prompts to the user (e.g. in case the auth plugin is insecure).
*
* Note: The 5.5 libmysqlclient on the first attempt ignores the requested plugin
* and tries to guess one instead. Only if that fails it will try to use
* the server's suggested one. Thus this method may be invoked multiple times
* during a single connection attempt.
*
* @return The actual user password to give to the mysql auth plugin.
* This may be empty (no password).
* This may be nil, which will be interpreted as "cancelled by user"
*
* This MUST be called on the UI thread!
*/
- (NSString *)actualPasswordForAuthPlugin:(NSString *)pluginName
{
NSArray *securePluginNames = @[
@"mysql_native_password", //default on 4.1+
//@"sha256_password", // not supported by the 5.5 client, only secure when used with TLS server cert checks!
// over SSL: password is transmitted in plaintext
// over plaintext: the mysql client uses assymetric crypto to encrypt the password and
// send the encrypted plaintext to the server (this requires the OpenSSL
// libs, so doesn't work with our client builds)
];
//INSECURE:
// mysql_clear_password; Plaintext password
// mysql_old_password; Deprecated, used with pre-4.1 servers. No longer supported in 5.7.5+ clients
//UNSUPPORTED:
// authentication_windows_client; Kerberos/NTLM auth. Not supported on UNIX by libmysqlclient
// authentication_ldap_sasl_client; MySQL Enterprise
// auth_test_plugin; Developer example only

// TODO incorporate SSL/TLS state in decision (cleartext over trusted SSL should be fine)
// This is not possible right now, because someone with access to the Mac could just swap the SSL CA cert
// and server ip to something he controls. So if Sequel Pro would just check that SSL was in use, it could
// still be attacked to easily give the password away.
// Instead we have to link all critical connection parameters (host+port+ssh-tunnel+ssl server or ca cert hash)
// to the password in keychain and make sure none of them changed before giving the password to libmysqlclient.

if(![securePluginNames containsObject:pluginName]) {
if(![pluginName isEqualToString:agreedInsecurePlugin]) {
// since the user will probably take longer to answer the dialog than the connection timeout is
// (and because the UI sheet is async anyway),
// we will do a little trick and disconnect mysql right now and retry, once we actually have the password
[self performSelector:@selector(_beginRequestPasswordForInsecurePlugin:) withObject:pluginName afterDelay:0.0];
cancellingConnection = YES;
return nil;
}
}

NSString *pass;
// override password always wins
if ((pass = [self insecureOverridePassword])) {
;
}
// Only set the password if there is no Keychain item set and the connection is not being tested.
// The connection will otherwise ask the delegate for passwords in the Keychain.
else if ((!connectionKeychainItemName || isTestingConnection) && (pass = [self password])) {
;
}
else {
pass = [self keychainPassword];
}

return (pass ? pass : @""); //returning nil would mean cancelled
}

- (void)_beginRequestPasswordForInsecurePlugin:(NSString *)pluginName
{
// if the user presses "Disconnect" in the dialog OR
// if this was only a test connection OR
// if the connection failed anyway
// -> agreedInsecurePlugin will be cleared again via -_restoreConnectionInterface
[self setAgreedInsecurePlugin:pluginName];

//show modal warning dialog
NSAlert *alert = [[NSAlert alloc] init]; //released in alert callback
[alert setAlertStyle:NSAlertStyleCritical];
[alert setMessageText:NSLocalizedString(@"Transmit password insecurely?",@"Connection dialog : password security error alert : title")];
[alert setInformativeText:NSLocalizedString(@"The MySQL server has requested the password to be transmitted in an insecure manner. This could be indicative of an attack on the server or your network connection!\n\nIf you still want to continue anyway, manually reenter your connection password.",@"Connection dialog : password security error alert : detail message")];
[alert setAccessoryView:requestPasswordAccessoryView];

[requestPasswordPluginNameField setStringValue:pluginName];

NSButton *connectAnywayButton = [alert addButtonWithTitle:NSLocalizedString(@"Connect Anyway",@"Connection dialog : password security error alert : confirm button")];
[connectAnywayButton setTag:NSAlertDefaultReturn];

NSButton *disconnectButton = [alert addButtonWithTitle:NSLocalizedString(@"Disconnect",@"Connection dialog : password security error alert : cancel button")];
[disconnectButton setTag:NSAlertAlternateReturn];

[alert beginSheetModalForWindow:[dbDocument parentWindow]
modalDelegate:self
didEndSelector:@selector(_insecurePasswordAlertDidEnd:returnCode:contextInfo:)
contextInfo:NULL];
}

- (void)_insecurePasswordAlertDidEnd:(NSAlert *)alert returnCode:(NSInteger)returnCode contextInfo:(void *)contextInfo
{
NSString *pass = nil;
if(returnCode == NSAlertDefaultReturn) {
pass = [NSString stringWithString:[requestPasswordPasswordField stringValue]];
}
[requestPasswordPasswordField setStringValue:@""];

[alert autorelease];

cancellingConnection = NO;

//cancelled by user?
if(!pass) {
[self _restoreConnectionInterface];
return;
}

// if the manually given password matches the keychain password there is no need to keep it in memory,
// otherwise we use the override password ivar in order to not affect the normal logic
if (!connectionKeychainItemName || ![pass isEqualToString:[self keychainPassword]]) {
[self setInsecureOverridePassword:pass];
}

[[pass retain] release]; //free it asap
[self initiateMySQLConnection]; //reconnect
}

#pragma mark -
#pragma mark Connection processes

Expand Down Expand Up @@ -765,6 +897,8 @@ - (void)updateFavoriteSelection:(id)sender
if (connectionKeychainItemAccount) SPClear(connectionKeychainItemAccount);
if (connectionSSHKeychainItemName) SPClear(connectionSSHKeychainItemName);
if (connectionSSHKeychainItemAccount) SPClear(connectionSSHKeychainItemAccount);
[self setAgreedInsecurePlugin:nil];
[self setInsecureOverridePassword:nil];

SPTreeNode *node = [self selectedFavoriteNode];
if ([node isGroup]) node = nil;
Expand Down Expand Up @@ -1764,7 +1898,11 @@ - (void)_restoreConnectionInterface
// Reset the window title
[dbDocument updateWindowTitle:self];
[[dbDocument parentTabViewItem] setLabel:[dbDocument displayName]];


//only store that if the connection attempt was successful
[self setAgreedInsecurePlugin:nil];
[self setInsecureOverridePassword:nil];

// Stop the current tab's progress indicator
[dbDocument setIsProcessing:NO];

Expand Down Expand Up @@ -2083,12 +2221,6 @@ - (void)initiateMySQLConnectionInBackground
}
}

// Only set the password if there is no Keychain item set and the connection is not being tested.
// The connection will otherwise ask the delegate for passwords in the Keychain.
if ((!connectionKeychainItemName || isTestingConnection) && [self password]) {
[mySQLConnection setPassword:[self password]];
}

// Enable SSL if set
if ([self useSSL]) {
[mySQLConnection setUseSSL:YES];
Expand Down Expand Up @@ -2130,6 +2262,11 @@ - (void)initiateMySQLConnectionInBackground
[mySQLConnection setUseKeepAlive:[[prefs objectForKey:SPUseKeepAlive] boolValue]];
[mySQLConnection setKeepAliveInterval:[[prefs objectForKey:SPKeepAliveInterval] floatValue]];

// We can always enable the cleartext plugin (this does not yet mean it will actually be used),
// since mysql will ask us for the password in -actualPasswordForAuthPlugin: at which point
// we get to decide what to do with the actual plugin.
[mySQLConnection setAllowCleartextPlugin:YES];

// Connect
[mySQLConnection connect];

Expand Down Expand Up @@ -3646,6 +3783,8 @@ - (void)dealloc
if (connectionKeychainItemAccount) SPClear(connectionKeychainItemAccount);
if (connectionSSHKeychainItemName) SPClear(connectionSSHKeychainItemName);
if (connectionSSHKeychainItemAccount) SPClear(connectionSSHKeychainItemAccount);
[self setAgreedInsecurePlugin:nil];
[self setInsecureOverridePassword:nil];

#ifndef SP_CODA
if (currentFavorite) SPClear(currentFavorite);
Expand Down

0 comments on commit 030eac5

Please sign in to comment.