search

kee-org / keepassrpc

The KeePassRPC plugin that needs to be installed inside KeePass in order for Kee to be able to connect your browser to your passwords
GNU General Public License v2.0
635 stars 35 forks source link

Add .local pseduo TLD support to URL Match accuracy domain overrides #35

Closed tinuzz closed 4 months ago

tinuzz commented 6 years ago

Hi,

I'm trying to add some domain overrides in for URL match accuracy in the database settings. Our internal domain's TLD is '.local' (yes.. I know...) Unfortunately, Kee doesn't let met add a .local domain as an override, but tells me "Invalid domain name".

It seems that the domain name check in Forms/KeeMAMOverride.cs is too restrictive.

Best regards, Martijn.

luckyrat commented 6 years ago

Hi Martijn,

Technically .local is not a TLD, rather a pseudo TLD. Not that this helps you of course. The domain override feature requires domain names to function so the restriction in the KeeMAMOverride form is necessary in order to avoid bugs cropping up in other parts of the plugin.

To support .local would mean rewriting parts of the override feature to no longer expect a domain name. I'm not sure how feasible that is and won't have time to investigate myself in the foreseeable future but if someone else wants to look into it, bear in mind that enabling IP address support for this feature is another idea worth considering when developing an improvement. I expect there to be a fair bit of overlap in the two features although IP addresses would need to go a little further with more complex planning for the use of CIDR notation and/or wildcards.

It would also most likley make sense to add support for https://en.wikipedia.org/wiki/Top-level_domain#Reserved_domains at the same time.

As long as changes to this feature don't impact on the security, performance or usability of the current public domain name support, I'd accept a PR.

Thanks, Chris

tinuzz commented 6 years ago

Unfortunately, I wouldn't know where to begin, so I can't offer any help.

I am curious though: what does "requires domain names to function" mean, exactly? Hoe does a domain name function?

luckyrat commented 6 years ago

Sorry, that could have been clearer. What I mean is that the override feature is designed to compare against domain names, as oppose to hostnames or generic strings of text.

tinuzz commented 6 years ago

I assume that your own domain-public-suffix library is used to check the validity of the domain, is that correct?

Wouldn't that be the most logical place to fix this?

luckyrat commented 6 years ago

I assume that your own domain-public-suffix library is used to check the validity of the domain, is that correct?

Yes

Wouldn't that be the most logical place to fix this?

It's not really a thing to be fixed though. The library is correctly reporting the status of the string being supplied to it (i.e. it is not a registrable domain).

To add support for unregistrable domains would be something that would stretch the boundaries of what that library should be responsible for so I think it would be better to put the enhancement into KeePassRPC, rather than the PSL library. Whether we still use the PSL library for this after making the change is debatable - it depends upon how complex and bespoke we have to make the checks for different variants of hostnames/IPs/registrable domains/ports.

rjt commented 6 years ago

What about having a special keepass entry of end user customizable domain names such as 

.local
.localdomain
.zendesk.com
.internalonly.domainname.corp

that are appended to the public list and would follow users to different machines.

luckyrat commented 4 months ago

I'm closing this since I don't think it's an approach we will take.

There is hope that web browsers will improve our access to the PSL which could significantly change how we need to handle it ourselves. Potentially leaving this KeePassRPC Domain override feature the only direct consumer of the PSL. In that circumstance, we may even remove the feature but certainly wouldn't want to spend a lot of time further developing it with additional configuration options. Perhaps some similar feature based on the KeePassRPC client would be a better way forward.

Maybe in a year or two when the PSL support situation is clearer we can revisit exactly what to do with the Domain override feature but it's not likely to be implemented in exactly this way so I think it's not worth leaving this issue around open for even longer than it already has been.