Issues related to the creation of new clients : Extending JsonRPC DB handling capabilities.

[Gugli]

Hello,

I have started to make a new client for KeepassRPC.
The client is an Ansible "lookup" plugin, to allow automation of server deployment with secrets read from Keepass, and new secrets generated and stored to Keepass. This plugin will be published opensource soon. SRP and JsonRPC already work well.

First very small issue I encountered : the schema part of the "origin" URL in websocket handshake is restricted. I know it can be changed in the client via advanced config, but it is not very convenient, and I don't think it offers any security improvement. I have chosen to piggyback "moz-extension://" for user convenience.

But the bigger issue is the list of JSonRPC callable functions : some are missing to allow for new clients types.
From my understanding, some of those calls are "low-level" or "plumbing" calls, directly accessing the DB : GetDatabaseFileName; GetRoot, GetChildGroups, GetAllChildEntries, AddGroup, RemoveEntry, etc... : they use the Database UniqueIDs and the changes requested correspond 1-to-1 to changes in the database.
Other calls are more "convenient"/"porcelain", and seem to be specific to browser-extension clients : AddLogin, UpdateLogin, FindLogins. They rely more on "Keefox"-oriented notions, such as URLs and Fields.
The issue when writing a plugin such as mine, is that there are no "plumbing" functions for Entry edition (apart from RemoveEntry). Such functions should rely on Keepass-oriented notions like "properties", and allow access to attached files and so on.

If I were to make a Pull Request adding such functions, do you think it will be able to be merged one day ? Or is it a direction you don't want this plugin to explore ?
Such functions would include AddEntry, AddEntryProperty, GetEntryProperty, GetEntryAttachement, etc...

Thanks for your help.

[Gugli]

This may be related to this issue, but I'm not sure : #101

[luckyrat]

Hi,

Thanks for your message. In general the answer to your main question is yes, I would be happy to merge in PRs which add additional functions.

The feature flag configuration approach should make it easy to add these functions in a backwards compatible fashion but the main issue would be the necessary changes to the data exchange model. Obviously we can just leave the existing ones unchanged for any clients that don't add the relevant new feature flag in their list of supported features but given that #101 would require some changes too, it would be best if any changes you make can also align with the ideas evolving in there. Perhaps the "form field" is the most complex area of overlap since in some cases (i.e. Username/Password) it relies on a standard KDBX property and additional metadata, and in others there is no corresponding Property.

Maybe we can switch to a more generic "Field" data type as a base with only something like name and value being required? Then if any of the additional metadata around form fields and Kee configuration (or your client configuration) are present we can follow the appropriate path (sometimes creating a KDBX Property and sometimes not). Maybe we could take a different path depending on whether we are exchanging data with the convenience methods like AddLogin or the less-specific methods you propose like AddEntry. I think in the long-run I would prefer to focus on just one method for adding entries (we don't even refer to "Login" within Kee apart from for legacy reasons) but having both available is a reasonable intermediate point to get you up and running more quickly.

We could take a similar approach with "Entry" - some basic required fields like uuid and others optional for the varying use cases.

Another example of overlap with #101 is that I'd like to add support for TOTP and this is something which has since been added to KeePass and therefore might no longer require the use of supplementary configuration specific to KeePassRPC. Likewise, you may find a feature that your client requires which is not natively supported as part of KeePass (some sort of cache of ansible metadata? I'm not sure) and storing this within the KeePassRPC configuration JSON (or its replacement in #101) would be a reasonable way to accommodate this.

It might be apparent that I've not fully thought through all of the implications about changes to the data exchange format yet, and I have lots of old notes that I will need to dig out as we go along and I refresh my memory about what direction the plugin has to go in to support Kee in the future. It's not likely to be a major consideration but maybe worth noting that I would want to be able to implement the changes in #101 in the Kee Vault kprpc code too so I'll be bearing that in mind when considering potential designs.

https://forum.kee.pm/t/configuring-keepassrpc-permitted-websocket-origins/3041 documents the origin configuration requirement - it's security benefit comes from preventing the vulnerability discovered in 2020 which relied upon arbitrary connections from http(s) origins. If you have a custom scheme that you'd like added to the defaults for a new version of the plugin, we can check that it is safe and then add it. In the mean time you'll have to override the configuration as you already discovered.