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

KeePassHTTP hotfix, resolves #147 #196

Merged
merged 5 commits into from
Jan 25, 2017
Merged

Conversation

phoerious
Copy link
Member

@phoerious phoerious commented Jan 24, 2017

This pull request resolves #147. Although KeePassHTTP is still a weak protocol and its main flaws remain, this implementation should be safe enough for releasing it as part of KeePassXC.

Description

KeePassXC strictly limits communication to the local loopback interface at 127.0.0.1. This address is hardcoded and cannot be changed. The plugin is now also disabled by default at runtime and needs to be explicitly enabled before KeePassXC listens to any requests from PassIFox or chromeIPass.

Additional changes in this PR:

  • Warn user when trying to bind to a port below 1024 and fall back to default port
  • Update heavily outdated README.md and add security notice about KeePassHTTP
  • Only require libmicrohttpd build-time dependency when compiling with HTTP support.

How Has This Been Tested?

There are still no unit tests for the HTTP plugin, but all functionality was tested manually.
The patch was tested both on my Arch system as well as in an Ubuntu 16.04 virtual machine.

Types of changes

  • ✅ Bug fix (non-breaking change which fixes an issue) [Security Fix]

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]

Copy link
Contributor

@TheZ3ro TheZ3ro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested but code looks fine.

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

Successfully merging this pull request may close these issues.

4 participants