From 9a39eaa2372b74494050cf9a6ff33c4cb06a18a0 Mon Sep 17 00:00:00 2001 From: Albert Vaca Cintora Date: Sun, 19 May 2024 10:04:43 +0000 Subject: [PATCH] Use EC keys instead of RSA Use smaller and safer EC keys, replacing 2048 bit RSA. NID_X9_62_prime256v1 is roughly as secure as a 3072 bit RSA key, but way shorter. Since we have to embed the key in the identity packet that is sent over UDP and some stacks aren't happy with large UDP messages (notably: macos), I switched to EC instead of to a longer RSA key. This seems to be compatible with other clients even on older systems like Android 5.0. I did stick with NID_X9_62_prime256v1 because stronger EC like NID_secp384r1 failed the handshake (I didn't investigate why). We now store the kind of key in the config, so we can know which kind of key we are loading. --- core/backends/lan/lanlinkprovider.cpp | 9 +--- core/kdeconnectconfig.cpp | 22 +++++++++- core/kdeconnectconfig.h | 3 ++ core/sslhelper.cpp | 62 +++++++++++++++++++++++++-- core/sslhelper.h | 1 + 5 files changed, 84 insertions(+), 13 deletions(-) diff --git a/core/backends/lan/lanlinkprovider.cpp b/core/backends/lan/lanlinkprovider.cpp index e60abeb2a..b9a23c915 100644 --- a/core/backends/lan/lanlinkprovider.cpp +++ b/core/backends/lan/lanlinkprovider.cpp @@ -527,14 +527,7 @@ void LanLinkProvider::configureSslSocket(QSslSocket *socket, const QString &devi // Configure for ssl QSslConfiguration sslConfig; sslConfig.setLocalCertificate(KdeConnectConfig::instance().certificate()); - - QFile privateKeyFile(KdeConnectConfig::instance().privateKeyPath()); - QSslKey privateKey; - if (privateKeyFile.open(QIODevice::ReadOnly)) { - privateKey = QSslKey(privateKeyFile.readAll(), QSsl::Rsa); - } - privateKeyFile.close(); - sslConfig.setPrivateKey(privateKey); + sslConfig.setPrivateKey(KdeConnectConfig::instance().privateKey()); if (isDeviceTrusted) { QSslCertificate certificate = KdeConnectConfig::instance().getTrustedDeviceCertificate(deviceId); diff --git a/core/kdeconnectconfig.cpp b/core/kdeconnectconfig.cpp index c1f9dfc7d..39d37763a 100644 --- a/core/kdeconnectconfig.cpp +++ b/core/kdeconnectconfig.cpp @@ -117,6 +117,11 @@ QSslCertificate KdeConnectConfig::certificate() return d->m_certificate; } +QSslKey KdeConnectConfig::privateKey() +{ + return d->m_privateKey; +} + DeviceInfo KdeConnectConfig::deviceInfo() { const auto incoming = PluginLoader::instance()->incomingCapabilities(); @@ -130,6 +135,16 @@ DeviceInfo KdeConnectConfig::deviceInfo() QSet(outgoing.begin(), outgoing.end())); } +QSsl::KeyAlgorithm KdeConnectConfig::privateKeyAlgorithm() +{ + QString value = d->m_config->value(QStringLiteral("keyAlgorithm"), QStringLiteral("RSA")).toString(); + if (value == QLatin1String("EC")) { + return QSsl::KeyAlgorithm::Ec; + } else { + return QSsl::KeyAlgorithm::Rsa; + } +} + QDir KdeConnectConfig::baseConfigDir() { QString configPath = QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation); @@ -250,7 +265,7 @@ bool KdeConnectConfig::loadPrivateKey(const QString &keyPath) { QFile privKey(keyPath); if (privKey.exists() && privKey.open(QIODevice::ReadOnly)) { - d->m_privateKey = QSslKey(privKey.readAll(), QSsl::KeyAlgorithm::Rsa); + d->m_privateKey = QSslKey(privKey.readAll(), privateKeyAlgorithm()); if (d->m_privateKey.isNull()) { qCWarning(KDECONNECT_CORE) << "Private key from" << keyPath << "is not valid!"; } @@ -295,7 +310,7 @@ void KdeConnectConfig::generatePrivateKey(const QString &keyPath) { qCDebug(KDECONNECT_CORE) << "Generating private key"; - d->m_privateKey = SslHelper::generateRsaPrivateKey(); + d->m_privateKey = SslHelper::generateEcPrivateKey(); if (d->m_privateKey.isNull()) { qCritical() << "Could not generate the private key"; Daemon::instance()->reportError(i18n("KDE Connect failed to start"), i18n("Could not generate the private key.")); @@ -313,6 +328,9 @@ void KdeConnectConfig::generatePrivateKey(const QString &keyPath) } } + d->m_config->setValue(QStringLiteral("keyAlgorithm"), QStringLiteral("EC")); + d->m_config->sync(); + if (error) { Daemon::instance()->reportError(QStringLiteral("KDE Connect"), i18n("Could not store private key file: %1", keyPath)); } diff --git a/core/kdeconnectconfig.h b/core/kdeconnectconfig.h index c04d04a9d..3cb4cb100 100644 --- a/core/kdeconnectconfig.h +++ b/core/kdeconnectconfig.h @@ -8,6 +8,7 @@ #define KDECONNECTCONFIG_H #include +#include #include "deviceinfo.h" #include "kdeconnectcore_export.h" @@ -26,8 +27,10 @@ public: QString deviceId(); QString name(); DeviceType deviceType(); + QSslKey privateKey(); QSslCertificate certificate(); DeviceInfo deviceInfo(); + QSsl::KeyAlgorithm privateKeyAlgorithm(); QString privateKeyPath(); QString certificatePath(); diff --git a/core/sslhelper.cpp b/core/sslhelper.cpp index e7b3f6162..c41bed34e 100644 --- a/core/sslhelper.cpp +++ b/core/sslhelper.cpp @@ -26,6 +26,61 @@ QString getSslError() return QString::fromLatin1(buf); } +QSslKey generateEcPrivateKey() +{ + // Initialize context. + auto pctxRaw = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr); + auto pctx = std::unique_ptr(pctxRaw, ::EVP_PKEY_CTX_free); + if (!pctx) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed to allocate context " << getSslError(); + return QSslKey(); + } + + if (EVP_PKEY_keygen_init(pctx.get()) <= 0) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed to initialize context " << getSslError(); + return QSslKey(); + } + + // Set the curve. + if (EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx.get(), NID_X9_62_prime256v1) <= 0) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed to set curve " << getSslError(); + return QSslKey(); + } + + // Generate private key. + auto pkey = std::unique_ptr(EVP_PKEY_new(), ::EVP_PKEY_free); + if (!pkey) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed to allocate private key " << getSslError(); + return QSslKey(); + } + + auto pkey_raw = pkey.get(); + if (EVP_PKEY_keygen(pctx.get(), &pkey_raw) <= 0) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed to generate private key " << getSslError(); + return QSslKey(); + } + + // Convert private key format to PEM as required by QSslKey. + auto bio = std::unique_ptr(BIO_new(BIO_s_mem()), ::BIO_free_all); + if (!bio) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed to allocate I/O abstraction " << getSslError(); + return QSslKey(); + } + + if (!PEM_write_bio_PrivateKey(bio.get(), pkey_raw, nullptr, nullptr, 0, nullptr, nullptr)) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed write PEM format private key to BIO " << getSslError(); + return QSslKey(); + } + + BUF_MEM *mem = nullptr; + if (!BIO_get_mem_ptr(bio.get(), &mem)) { + qCWarning(KDECONNECT_CORE) << "Generate EC Private Key failed get PEM format address " << getSslError(); + return QSslKey(); + } + + return QSslKey(QByteArray(mem->data, mem->length), QSsl::KeyAlgorithm::Ec); +} + QSslKey generateRsaPrivateKey() { // Initialize context. @@ -86,11 +141,12 @@ QSslCertificate generateSelfSignedCertificate(const QSslKey &qtPrivateKey, const // Create certificate. auto x509 = std::unique_ptr(X509_new(), ::X509_free); if (!x509) { - qCWarning(KDECONNECT_CORE) << "Generate Self Signed Certificate failed to allocate certifcate " << getSslError(); + qCWarning(KDECONNECT_CORE) << "Generate Self Signed Certificate failed to allocate certificate " << getSslError(); return QSslCertificate(); } - if (!X509_set_version(x509.get(), 2)) { + constexpr int x509version = 3 - 1; // version is 0-indexed, so we need the -1 + if (!X509_set_version(x509.get(), x509version)) { qCWarning(KDECONNECT_CORE) << "Generate Self Signed Certificate failed to set version " << getSslError(); return QSslCertificate(); } @@ -169,7 +225,7 @@ QSslCertificate generateSelfSignedCertificate(const QSslKey &qtPrivateKey, const } // Sign the certificate with private key. - if (!X509_sign(x509.get(), pkey.get(), EVP_sha256())) { + if (!X509_sign(x509.get(), pkey.get(), EVP_sha512())) { qCWarning(KDECONNECT_CORE) << "Generate Self Signed Certificate failed to sign certificate " << getSslError(); return QSslCertificate(); } diff --git a/core/sslhelper.h b/core/sslhelper.h index 40c6db918..61dc20204 100644 --- a/core/sslhelper.h +++ b/core/sslhelper.h @@ -13,6 +13,7 @@ namespace SslHelper { +QSslKey generateEcPrivateKey(); QSslKey generateRsaPrivateKey(); QSslCertificate generateSelfSignedCertificate(const QSslKey &privateKey, const QString &commonName); }