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 .local pseduo TLD support to URL Match accuracy domain overrides #35

Open
tinuzz opened this issue May 2, 2018 · 6 comments
Open

Comments

@tinuzz
Copy link

tinuzz commented May 2, 2018

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

luckyrat commented May 2, 2018

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

@luckyrat luckyrat changed the title URL Match accuracy domain override check too restrictive Add .local pseduo TLD support to URL Match accuracy domain overrides May 2, 2018
@tinuzz
Copy link
Author

tinuzz commented May 2, 2018

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

luckyrat commented May 2, 2018

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

tinuzz commented May 2, 2018

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

luckyrat commented May 4, 2018

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

rjt commented Oct 6, 2018

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.

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

3 participants