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

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

Open
arigit opened this issue Oct 1, 2017 · 15 comments
Open

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

arigit opened this issue Oct 1, 2017 · 15 comments

Comments

@arigit
Copy link

arigit commented Oct 1, 2017

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
Copy link

albuic commented Jul 3, 2018

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
Copy link

ksz16 commented Oct 18, 2018

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
Copy link
Member

droidmonkey commented Oct 18, 2018

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
Copy link

ksz16 commented Oct 19, 2018

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
Copy link

ringerc commented Feb 24, 2019

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
Copy link

ringerc commented Feb 24, 2019

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
Copy link
Member

droidmonkey commented Feb 24, 2019

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
Copy link

ringerc commented Feb 25, 2019

@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
Copy link
Member

droidmonkey commented Feb 25, 2019

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.

@droidmonkey droidmonkey removed this from the Future milestone Mar 24, 2019
@seafox
Copy link

seafox commented Nov 6, 2019

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
Copy link

TimOverboard commented Nov 28, 2019

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
Copy link

seafox commented Dec 6, 2019

@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
Copy link

manwe-pl commented Feb 7, 2022

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
Copy link
Member

droidmonkey commented Feb 7, 2022

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
Copy link

cxandru commented Feb 9, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants