Implement Secure Event Input for password fields on macOS

[saf-dmitry]

Overview

Keyboard monitor active when entering password to unlock database

Steps to Reproduce

Keyboard monitor status can be seen on menu bar icon of txt expansion tool (e.g. Typinator)

Expected Behavior

Keyboard monitor should be temporarily deactivated when cursor in master password field

Actual Behavior

Keyboard monitor active when cursor in master password field

Context

KeePassXC - Version 2.5.3 Revision: f8c962b

Operating System: macOS 10.14.6

[phoerious]

Is that some third-party application?

[droidmonkey]

This has nothing to do with our application.

[saf-dmitry]

This has nothing to do with our application.

Sorry, what I actually mean is, that the Secure Event Input is not activated when entering master password in KeePassXC password field.

EnableSecureEventInput was implemented in Mac OS X 10.3, to provide a secure means for a process to protect keyboard input to a custom data entry field. This function protects keyboard entry so that keyboard events cannot be intercepted by a keyboard intercept process. A Security Update implemented for Mac OS X 10.4 changes how the system affects keyboard intercept processes when a process uses EnableSecureEventInput.

[droidmonkey]

See #3307

[saf-dmitry]

Well, you are right, the Technical Note is too old. Nevertheless, text expansion software on macOS can read keystrokes and perform text expansion when entering password in KeePassXC password field, which is a safety issue. Other password managers like KeePassX or MacPass do inhibit this by activating Secure Input.

[droidmonkey]

This is something that should be implemented by Qt, to be honest, for all obscured text edit fields. I doubt that KeePassX implements this feature, we are a direct fork of KeePassX and they did not have this capability implemented.

[varjolintu]

https://github.com/varjolintu/keepassxc/tree/feature/macos_secure_line_edit - I started this almost year ago, but didn't finish it. Maybe we could try to get this to work.

[saf-dmitry]

I doubt that KeePassX implements this feature, we are a direct fork of KeePassX and they did not have this capability implemented.

Yes, it is interesting, because in my tests KeePassX v2.0.3 (using Qt 4.8.7 and libcrypt 1.7.3) does activate Secure Input on macOS 10.14.6.

[ShikiSuen]

This is an advice: If implementing this, please make sure it gets disabled immediately right after finishing using it. Otherwise, all 3rd-party input methods will be prohibited from being activated by user.

[whispy]

How is this not a high priority on the roadmap? As it currently stands, ANY application with the correct permissions on a Mac can read your password as you enter it. This seems like an incredible security flaw.

One argument against this is that if your Mac has a keylogger, you are already pwned. However, I don't think that is the case — it is far better to be keylogged and have your master password still be protected, than to be keylogged and have your master password be breached. Defense in depth is better than no defense at all.

What am I missing here? Why is this not a top priority?

[whispy]

I would be willing to fund a bounty on this. I am not sure the level of effort required to implement it, so am not sure what would be a reasonable bounty. Say, $150?

[droidmonkey]

As discussed above, this has some nasty side effects if it is not handled carefully. It is also unknown if this will work with Qt. Arguably, Qt should be providing this as an option. I don't even know how to test if this is even effective once implemented.

[droidmonkey]

I just spent the past half hour trying to find any documentation for this feature. It doesn't exist. I'm closing this unless someone can produce documentation that indicates how this is implemented or if its even still a feature of macOS.

[varjolintu]

This is all there is: https://developer.apple.com/documentation/appkit/nssecuretextfield

And the test implementation (which doesn't work correctly): https://github.com/varjolintu/keepassxc/blob/feature/macos_secure_line_edit/src/gui/macutils/SecureLineEdit.mm https://github.com/varjolintu/keepassxc/blob/feature/macos_secure_line_edit/src/gui/macutils/SecureLineEdit.h

[droidmonkey]

NSSecureTextField would require us to completely diverge from Qt which is a non starter.

[whispy]

An idea (with the caveat that I am not very familiar with Qt or C++ OR the structuring of KeepassXC, and so the below could be trash):

1. Can we create an empty Qt widget, then create an Objective-C++ wrapper class (which would allow us to mix Obj C and C++) for NSSecureTextField, and then add the wrapper to the newly created widget? Then we could use the widget like any other widget.

Since Qt does not natively support Objective-C++, we'd need to create a macOS-specific view containing the NSSecureTextField and then embed using a Qt QWidget.

Some really rough code that may be wrong:

// SecureTextFieldWrapper.mm
#import <Cocoa/Cocoa.h>

@interface SecureTextFieldWrapper : NSView
{
    NSSecureTextField* secureTextField;
}

- (instancetype)initWithFrame:(NSRect)frameRect;
- (void)setStringValue:(NSString*)value;
- (NSString*)stringValue;

@end

@implementation SecureTextFieldWrapper

- (instancetype)initWithFrame:(NSRect)frameRect
{
    self = [super initWithFrame:frameRect];
    if (self) {
        secureTextField = [[NSSecureTextField alloc] initWithFrame:frameRect];
        [self addSubview:secureTextField];
    }
    return self;
}

- (void)setStringValue:(NSString*)value
{
    [secureTextField setStringValue:value];
}

- (NSString*)stringValue
{
    return [secureTextField stringValue];
}

@end

// QtWidget.cpp
#import "SecureTextFieldWrapper.h"

// ...

SecureTextFieldWrapper* secureTextFieldWrapper = new SecureTextFieldWrapper();
QVBoxLayout* layout = new QVBoxLayout;
layout->addWidget(secureTextFieldWrapper);
setLayout(layout);

[whispy]

A seprate idea — here is how Chromium handles secure inputs when a password field is focused: https://github.com/chromium/chromium/blob/3baf23064ea29859cc32fa3c750c2cfca3cf9999/ui/base/cocoa/secure_password_input.mm and https://github.com/chromium/chromium/blob/e304e2fb226255a159ad75931a70f6f834f70340/ui/base/cocoa/secure_password_input.h

It looks like they are manually calling EnableSecureEventInput(); and DisableSecureEventInput();. Perhaps this can be done in Qt, without actually changing the structure of password fields in KeepassXC? If it's secure enough for Chrome, after all...

[droidmonkey]

Oh good, that points to where this function is located.

import <Carbon/Carbon.h>

I'll see if we can add this to our PasswordWidget

[ShikiSuen]

Just make sure DisableSecureEventInput() is called immediately as soon as any of the following conditions are met:

1. the input field loses focus.
2. the application loses focus.
3. User starts interacting with another application.

Otherwise, all 3rd-party input methods installed in the system will be hindered by the SecureEventInput.

[ShikiSuen]

(Seriously, this feature needs a toggle.)

[phoerious]

You should be able to just include the Cocoa headers normally without having to write anything in ObjectiveC. We have a bunch of ObjectiveC code in OSUtils, but for the most part it's not necessary. At least it wasn't for the CoreFramework headers I've been using recently.