From 40aa92c5f769d454fc15ae66d0a285fad2a8d4bc Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 24 Jan 2017 00:08:48 +0100 Subject: [PATCH] Only listen to local loopback instead of user-configured host as discussed in issue #147 Also issue warning when trying to bind to a port below 1024 and use default port in that case --- src/http/HttpSettings.cpp | 12 ------ src/http/HttpSettings.h | 2 - src/http/OptionDialog.cpp | 13 +++++-- src/http/OptionDialog.ui | 74 ++++++++++++++++++------------------ src/http/Server.cpp | 80 +++++++++++---------------------------- 5 files changed, 71 insertions(+), 110 deletions(-) diff --git a/src/http/HttpSettings.cpp b/src/http/HttpSettings.cpp index 0d6b6f1f1..c935ba925 100644 --- a/src/http/HttpSettings.cpp +++ b/src/http/HttpSettings.cpp @@ -126,18 +126,6 @@ void HttpSettings::setSupportKphFields(bool supportKphFields) config()->set("Http/SupportKphFields", supportKphFields); } -QString HttpSettings::httpHost() -{ - static const QString host = "localhost"; - - return config()->get("Http/Host", host).toString().toUtf8(); -} - -void HttpSettings::setHttpHost(QString host) -{ - config()->set("Http/Host", host); -} - int HttpSettings::httpPort() { static const int PORT = 19455; diff --git a/src/http/HttpSettings.h b/src/http/HttpSettings.h index c1987f7ea..bea5648c9 100644 --- a/src/http/HttpSettings.h +++ b/src/http/HttpSettings.h @@ -42,8 +42,6 @@ public: static void setSearchInAllDatabases(bool searchInAllDatabases); static bool supportKphFields(); static void setSupportKphFields(bool supportKphFields); - static QString httpHost(); - static void setHttpHost(QString host); static int httpPort(); static void setHttpPort(int port); diff --git a/src/http/OptionDialog.cpp b/src/http/OptionDialog.cpp index e92c6e1a5..5245d333b 100644 --- a/src/http/OptionDialog.cpp +++ b/src/http/OptionDialog.cpp @@ -15,6 +15,8 @@ #include "ui_OptionDialog.h" #include "HttpSettings.h" +#include + OptionDialog::OptionDialog(QWidget *parent) : QWidget(parent), ui(new Ui::OptionDialog()) @@ -41,7 +43,6 @@ void OptionDialog::loadSettings() ui->sortByUsername->setChecked(true); else ui->sortByTitle->setChecked(true); - ui->httpHost->setText(settings.httpHost()); ui->httpPort->setText(QString::number(settings.httpPort())); /* @@ -70,8 +71,14 @@ void OptionDialog::saveSettings() settings.setUnlockDatabase(ui->unlockDatabase->isChecked()); settings.setMatchUrlScheme(ui->matchUrlScheme->isChecked()); settings.setSortByUsername(ui->sortByUsername->isChecked()); - settings.setHttpHost(ui->httpHost->text()); - settings.setHttpPort(ui->httpPort->text().toInt()); + + int port = ui->httpPort->text().toInt(); + if (port < 1024) { + QMessageBox::warning(this, tr("Cannot bind to privileged ports"), + tr("Cannot bind to privileged ports below 1024!\nUsing default port 19455.")); + port = 19455; + } + settings.setHttpPort(port); /* settings.setPasswordUseLowercase(ui->checkBoxLower->isChecked()); diff --git a/src/http/OptionDialog.ui b/src/http/OptionDialog.ui index ee09f9537..ea3ec508a 100644 --- a/src/http/OptionDialog.ui +++ b/src/http/OptionDialog.ui @@ -7,7 +7,7 @@ 0 0 605 - 389 + 429 @@ -17,7 +17,7 @@ - Enable KeepassXC Http protocol + Enable KeepassXC HTTP protocol This is required for accessing your databases from ChromeIPass or PassIFox @@ -28,7 +28,7 @@ This is required for accessing your databases from ChromeIPass or PassIFoxQTabWidget::Rounded - 0 + 2 @@ -201,32 +201,41 @@ Only entries with the same scheme (http://, https://, ftp://, ...) are returned< - - - - - - 0 - 0 - - - - HTTP Host: - - - - - - - Default host: localhost - - - - + + + Qt::Vertical + + + QSizePolicy::Fixed + + + + 20 + 20 + + + - - + + + + + d0000 + + + Default port: 19455 + + + + + + + KeePassXC will listen to this port on localhost + + + + @@ -237,15 +246,8 @@ Only entries with the same scheme (http://, https://, ftp://, ...) are returned< HTTP Port: - - - - - - d0000 - - - Default port: 19455 + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter diff --git a/src/http/Server.cpp b/src/http/Server.cpp index 16423d8cf..f09493829 100644 --- a/src/http/Server.cpp +++ b/src/http/Server.cpp @@ -331,68 +331,34 @@ void Server::start(void) if (m_started) return; - bool nohost = true; + // local loopback hardcoded, since KeePassHTTP handshake + // is not safe against interception + QHostAddress address("127.0.0.1"); int port = HttpSettings::httpPort(); + + void* addrx = NULL; + unsigned int flags = MHD_USE_SELECT_INTERNALLY; - QHostInfo info = QHostInfo::fromName(HttpSettings::httpHost()); - if (!info.addresses().isEmpty()) { - void* addrx = NULL; - unsigned int flags = MHD_USE_SELECT_INTERNALLY; - QHostAddress address = info.addresses().first(); + struct sockaddr_in *addr = static_cast(calloc(1, sizeof(struct sockaddr_in))); + addrx = static_cast(addr); + addr->sin_family = AF_INET; + addr->sin_port = htons(port); + addr->sin_addr.s_addr = htonl(address.toIPv4Address()); - if (address.protocol() == QAbstractSocket::IPv4Protocol) { - struct sockaddr_in *addr = static_cast(calloc(1, sizeof(struct sockaddr_in))); - addrx = static_cast(addr); - addr->sin_family = AF_INET; - addr->sin_port = htons(HttpSettings::httpPort()); - addr->sin_addr.s_addr = htonl(address.toIPv4Address()); - nohost = false; - } else { - struct sockaddr_in6 *addr = static_cast(calloc(1, sizeof(struct sockaddr_in6))); - addrx = static_cast(addr); - addr->sin6_family = AF_INET6; - addr->sin6_port = htons(HttpSettings::httpPort()); - memcpy(&addr->sin6_addr, address.toIPv6Address().c, 16); - nohost = false; - flags |= MHD_USE_IPv6; - } - - if (nohost) { - qWarning("HTTPPlugin: Faled to get configured host!"); - } else { - if (NULL == (daemon = MHD_start_daemon(flags, port, NULL, NULL, - &this->request_handler_wrapper, this, - MHD_OPTION_NOTIFY_COMPLETED, - this->request_completed, NULL, - MHD_OPTION_SOCK_ADDR, - addrx, - MHD_OPTION_END))) { - nohost = true; - qWarning("HTTPPlugin: Failed to bind to configured host!"); - } else { - nohost = false; - //qWarning("HTTPPlugin: Binded to configured host."); - } - - } - - if (addrx != NULL) - free(addrx); + if (NULL == (daemon = MHD_start_daemon(flags, port, NULL, NULL, + &this->request_handler_wrapper, this, + MHD_OPTION_NOTIFY_COMPLETED, + this->request_completed, NULL, + MHD_OPTION_SOCK_ADDR, + addrx, + MHD_OPTION_END))) { + qWarning("HTTPPlugin: Failed to bind to localhost!"); + } else { + m_started = true; } - if (nohost) { - if (NULL == (daemon = MHD_start_daemon(MHD_USE_SELECT_INTERNALLY, port, NULL, NULL, - &this->request_handler_wrapper, this, - MHD_OPTION_NOTIFY_COMPLETED, - this->request_completed, NULL, - MHD_OPTION_END))) { - qWarning("HTTPPlugin: Fatal! Failed to bind to both configured and default hosts!"); - } else { - qWarning("HTTPPlugin: Bound to fallback address 0.0.0.0/:::!"); - } - } - - m_started = true; + if (addrx != NULL) + free(addrx); }