Add basic trigger system support (call scripts on certain events)

arigit commented 7 years ago

Expected Behavior

Keepass2 supports setting up complex triggers, where you can create arbitrary triggers that are fired upon selectable conditions, and can execute different actions.

These are described here: https://keepass.info/help/kb/trigger_examples.html#dbsync

I would find it very usefult to have at least very basic trigger support for a few commonly used conditions: e.g. starting with just two: Database Save and Database Open. The trigger could just then call an external script (perhaps with some parameters e.g. the current database path); the path of the script can be configured via the GUI. It could be a single script call, and the Triggering Condition could be passed as parameter as well, to simplify the GUI.

E.g: new GUI contents:

Checkbox to activate "Trigger Call to External Script"
Sub-checkboxes to select the conditions to trigger the call (DB Save, DB Open)
Button triggering File Chooser to choose the script to call
- sample script call: /yourpath/yourscript.py --trigger DBSave --currentdatabase /path/to/your/db

Why "Database Save": I would use this to call an external script that uploads the database to a server. I use one-way synchronization of just one file, and common cloud file sync tools like google drive don't fit the use case.

Why Database Open: I notice some ppl asked for Merge on open; calling a script that uses keepassxc CLI this merge could be triggered automatically

Current Behavior

Currently there is no support; the new behavior is as described above.

Possible Solution

Context

Why "Database Save": I would use this to call an external script that uploads the database to a server. I use one-way synchronization of just one file, and common cloud file sync tools like google drive don't fit the use case.

Why Database Open: I notice some ppl asked for Merge on open; calling a script that uses keepassxc CLI this merge could be triggered automatically

Operating system: Linux / Fedora 26 CPU architecture: x64

Enabled extensions:

none

albuic commented 6 years ago

I would just add that the trigger should allow calling a script in the Database. Using an external script is probably less secure and modifying the script inside the Database allow the user to modify the script on all synchronized laptops/computers/... Also, It could be useful to execute a script according to the operating system (ie: one script for a Linux laptop and another for a Windows laptop)

ksz16 commented 6 years ago

The lack of trigger mechanism is something I miss the most after switching from KeePass to KeePassXC.

Two cases would be enough: on the "Database Save" and on "Database Open". And necessarily with Field References and Placeholders to use in scripts.

I used this feature to backup my DB after saving the changes. Now I have to manually run the external script after each modification, which is a nuisance.

droidmonkey commented 6 years ago

We have a setting that saves a backup of the database after every save. I would also strongly encourage using an automatic versioning system instead.

ksz16 commented 6 years ago

Yes, I'm aware of these possibilities. But in my usage scenario, I did one backup locally (in the location I chose on the disk) and the second to the WebDav (using curl). It was a very simple solution, without any additional software. Simple, 100% automated and very secure, because my password was not in plain text (Field References).

Now I'm running the external script manually and additionally I need to enter the password. So I regret that I can't use triggers anymore.

ringerc commented 5 years ago

I'm a newbie to this codebase, but a moderately experienced C and C++ dev (mostly backend, less GUI).

I'm interested in adding functionality to trigger external commands and wait for a result on events. I'd like to sketch a design below for your evaluation, as I don't want to jump in and start coding anything without some idea it'd be likely to be accepted. I understand why #366 was rejected and don't want to add similar issues, while making it possible to correctly support integration with external cloud sync tools amongst other users.

I'm thinking a simple external interface, something like a single external hook script configured in KeePassXC that is responsible for managing whatever arbitrary complexity the user wants (multiple hooks, etc etc).

The hook script would be configured in keepassxc.ini and would just be something like

 event_hook_script=/path/to/command "$EVENT"

EVENT would be something like:

before-file-open
after-file-open
before-file-save
after-file-save

etc; after the event an arbitrary number of event-s pecific parameters may be passed, and KeePassXC would reserve the right to write to the script's stdin or expect some specific stdout for particular events in future.

An optional timeout parameter would also be provided.

When the proc returns we'd check its return code. For "before" events a nonzero return will cause KeePassXC to display a prompt with the stderr text and the option to continue the operation or cancel it. For other events the stderr will be displayed as an informational message.

For now keepass would not write to the hook command's stdin, and would discard any stdout, though I'd connect their pipes in case someone wanted to do something fancy later.

KeePassXC's contact surface with the external hooks would be limited to preparing its arguments and invoking it via execvp or CreateProcessEx() (or your frameworks's wrappers if appropriate), with the process's stdin, stdout and stderr connected to pipes for interaction with KeePassXC, though I'd only intend to initially use stderr. KeePass would just then need to wait for the child proc to complete, doing nonblocking reads from its pipes into a local buffer so we don't deadlock between KeePassXC's waiting for exit and the child's attempt to write to its stdout/stderr. So it's a very simple, very well established pattern.

Other than the hook call/wait/handle code and a small bit of GUI for the prompt/results, the rest would be simple macros, template functions, variadic functions, or whatever floats your boat as an interface for injecting hookable events. So the code intrusion would be low and the whole backend could be swapped out later if you wanted.

Thoughts?

ringerc commented 5 years ago

Note that personally I'd prefer a simple shared library plugin interface, so I can implement things more cleanly. The basic approach of dlopen(), dlsym() a magic compat key symbol, dlsym() a setup function and call it to populate an API struct of function pointers, then interact with that. That way I can avoid clunky interaction with external processes and shell scripts if what I want to do can be accomplished simply. And I can do things like load a Python3 interpreter in my shared library and use Python libraries.

But I get the impression a plugin interface isn't particularly desired, right?

droidmonkey commented 5 years ago

Good proposal start but there are significant security concerns to address. Most especially the definition of the callback hook in the global config file. I am not in favor of any solution that cannot be explicitly controlled by the user. This, to me, means the configuration of that solution is stored within the encrypted database itself.

Personally, I am more in favor of exploring the option of using Qt's script engine (a fork of v8) to provide limited scriptable hooks that can be cryptographically registered with KeePassXC.

ringerc commented 5 years ago

@droidmonkey There's a chicken/egg problem there, because one of the use cases for such hooks is to check that the current local copy of the DB is up to date with a synced master copy before opening it.

I'm sure people will later want to be able to enumerate extra database sources directly from cloud hosts etc, too.

Yet another reason I'd personally prefer a shared library / simple plugin interface, rather than scripts.

As far as I'm concerned, if an attacker can mess with KeePassXC's configuration file they're also going to be able to mess with LD_PRELOAD (e.g. via .bashrc, or directly) to intercept the program's I/O or completely wrap it. Or change the user's PATH. Or any number of other attacks.

But you could probably assuage your concerns somewhat by having a flag you can set in a database that KeePassXC will check after loading, then disable all hook invocations for. The before-load hook would still be called, but it won't do anything that an attacker who could modify it can't already do other ways. This way if you open your super-s3kr3t-db you'll get a warning about the local KeePassXC having hooks enabled and it'll refuse to load, or prompt you.

Would that help? I don't think it adds any meaningful safety against a malicious attacker but it'd help stop you accidentally uploading some particularly sensitive DB if you open it with an install that has a careless "sync everything on close" hook.

droidmonkey commented 5 years ago

Compiled plugins are not appropriate for our application because we are cross platform. As a plugin author you would have to compile for each operating system every time you release. That is unreasonable for most contributors (we do that heavy lifting out of necessity). A plugin interface based on scripting (in this case it would be JavaScript) is far more efficient, and its already "built-in" with Qt. Please read more here: https://doc.qt.io/qt-5/qtscript-index.html

As for your proposal, it has merit and I think it will be useful for many people.

seafox commented 4 years ago

Good proposal start but there are significant security concerns to address. Most especially the definition of the callback hook in the global config file. I am not in favor of any solution that cannot be explicitly controlled by the user. This, to me, means the configuration of that solution is stored within the encrypted database itself.

You are so right! I'd NOT suggest blindly copying KeePass trigger solution as it has severe drawbacks, first of all in security. Triggers in KeePass are clearly an afterthought solution. I understand they wanted to keep kdbx portability and base functionality for all devices and OS's so trigger system not affected inner kdbx format. But the price to pay is absence of scalability and pain of maintainability as the design flaws are obvious: Trigger system has indeed required access to active db field references but triggers are maintained outside db itself plain open to any observer having file read level access to the system! Also, they aren't portable due to the chosen design. KPXC as the successor could do much better as keeping kdbx format and customizations already possible give an excellent chance to re-implement. My proposition to KPXC dev team follows below.

Two-tier implementation. Not only trigger system, but also the entire config system! Means we have outer config for KPXC (the current one), where users have usual UI customization stuff AND also inter-db (outer) trigger system for general program automation tasks. But, with second inner (and optional) tier that inherits (like in OOP) and overrides outer config and trigger system when a db just becomes ready to render (BTW delay-able or cancellable when false returned from afterAuthorization() event handler after successful authorization.

What advantages the proposed design brings? Stronger security by encapsulating most sensitive metadata (including db-specific triggers and config settings) inside encrypted db. Also, much more flexibility as say general KPXC built-in handler would allow execution of custom commands / scripts and provide access to fields like OS env params to inner triggers. In-db triggers would be able to subscribe to general actions carried by outer KPXC layer (like on db/app/system open and close) with custom secure code, so it would be bi-directional. Imagine you never need to copy/paste your triggers between many installations/users anymore, that means extreme robustness and portability of once designed and tested automation logic that would be synchronized by design!

What does it bring to config? In most common use case when working with just a single db, that may bring KPXC UI customizations like adding relevant for the db custom toolbar buttons ("Hello KeePass!" style), menu items or even custom toolbars. Sure the simplest UI changes would be easy applied like enlarging the main window size and on screen position upon db opening etc. as well. And remember, all that metadata like button captions etc. would be securely encrypted within db file! Isn't that The awesome concept to implement ASAP?

TimOverboard commented 4 years ago

I feel bad sayin this but I just want a hook to sync my database, why make it complicated? You really need to pass anything? You really actually need anything from inside your password database for this?

I don't need hooks on open, you need hooks on open? No, but someone else might?? Is this why there is no save hook implemented yet? I know this is cross-platform n all but it still hurts to see features being requested without a cast-iron "I will use this"... this ain't no boardroom. Not quite yet*.

I'm just about to setup FS monitoring cos no hook. giev hook. KISS hook.

like dis

[ ] do this on database save:
I Would absolutely use this and it would deffo save me hours. FS monitor on android and win? I prob just go back to usin chrome for a few months... that kind of thing can take up my whole weekend (by experience).

Many thanks for writing this port. xxxx

*stack exchange.

seafox commented 4 years ago

@TimOverboard

I'm going to disappoint you - think again! Yes, we have to check for need to sync on open, as typical master - local machines setup requires that to ensure the local copy of db you start working with is up to date. And yes, I am extensively using triggers with KeePass just because of inability to have a thoroughly thought automation in KPXC yet!

manwe-pl commented 2 years ago

Let me dig up this issue. Any chance for? Each and every time after adding something to the database I have to launch my upload2cloud script manually.

droidmonkey commented 2 years ago

You might want to look into an external watchdog script/tool that just monitors the file for changes and executes your script. We are not going to build this into KeePassXC.

https://facebook.github.io/watchman/

cxandru commented 2 years ago

I use entr (a file watching tool: https://github.com/eradman/entr), and have written the following wrapper script (on linux): (my 'upload2cloud' script in this case being drive, a Google Drive cli tool: https://github.com/odeke-em/drive)

set -eu
drive pull --no-prompt "$1"
echo "$1" | entr -np drive push --no-prompt "$1" &
keepassxc "$1"; kill $!
#https://stackoverflow.com/questions/360201/how-do-i-kill-background-processes-jobs-when-my-shell-script-exits/22644006

droidmonkey commented 1 year ago

We will not be implementing this.

Mattie112 commented 1 year ago

(another reason why not to implement triggers https://nvd.nist.gov/vuln/detail/CVE-2023-24055)

ytr8 commented 1 year ago

Looks like you are just brilliantly ignored the proposition just here Congratulations!

mgscreativa commented 4 months ago

I use entr (a file watching tool: https://github.com/eradman/entr), and have written the following wrapper script (on linux): (my 'upload2cloud' script in this case being drive, a Google Drive cli tool: https://github.com/odeke-em/drive)
set -eu
drive pull --no-prompt "$1"
echo "$1" | entr -np drive push --no-prompt "$1" &
keepassxc "$1"; kill $!
#https://stackoverflow.com/questions/360201/how-do-i-kill-background-processes-jobs-when-my-shell-script-exits/22644006

Hi! thanks for the script! How did you implemented it? did you added it to cron at boot?

keepassxreboot / keepassxc