From 7bdcf05fc30c75d4a2f0c917d5ba5fd6cb94437d Mon Sep 17 00:00:00 2001 From: Pat Long Date: Sun, 29 Jan 2023 10:57:09 -0500 Subject: [PATCH] Fix support for AES-256/GCM openssh keys (#8968) * Fix detecting AES-256/GCM cipher, fixes #8964 When you generate a ssh key using the aes-256/gcm cipher, the cipher name in the keyfile includes an @openssh.com at the end. * Use separate iv length for getting iv data, the assumption that the block size and iv size are equal does not hold for every cipher mode (e.g., GCM) * Disable AES-256/GCM for now in ssh keys Currently, the granularity for the botan gcm implementation is too large. To fix a problem with another algorithm in the library, they are multiplying the blocksize, so by default the granularity is 64. This causes issues since the encrypted data in the key is only guaranteed to have a length that is a multiple of the block size (16). --- share/translations/keepassxc_en.ts | 4 ++++ src/crypto/SymmetricCipher.cpp | 19 +++++++++++++++++++ src/crypto/SymmetricCipher.h | 1 + src/sshagent/OpenSSHKey.cpp | 18 +++++++++++++----- src/sshagent/OpenSSHKey.h | 1 + 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 5c78cf98c..a7acf8e9e 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -5827,6 +5827,10 @@ We recommend you use the AppImage available on our downloads page. Unexpected EOF when writing private key + + AES-256/GCM is currently not supported + + PasswordEditWidget diff --git a/src/crypto/SymmetricCipher.cpp b/src/crypto/SymmetricCipher.cpp index 4d3a7bdfe..1ed05bb33 100644 --- a/src/crypto/SymmetricCipher.cpp +++ b/src/crypto/SymmetricCipher.cpp @@ -267,3 +267,22 @@ int SymmetricCipher::blockSize(Mode mode) return 0; } } + +int SymmetricCipher::ivSize(Mode mode) +{ + switch (mode) { + case Aes128_CBC: + case Aes256_CBC: + case Aes128_CTR: + case Aes256_CTR: + case Twofish_CBC: + return 16; + case Aes256_GCM: + return 12; + case Salsa20: + case ChaCha20: + return 8; + default: + return 0; + } +} diff --git a/src/crypto/SymmetricCipher.h b/src/crypto/SymmetricCipher.h index 83b54658f..e18f623f3 100644 --- a/src/crypto/SymmetricCipher.h +++ b/src/crypto/SymmetricCipher.h @@ -70,6 +70,7 @@ public: static int defaultIvSize(Mode mode); static int keySize(Mode mode); static int blockSize(Mode mode); + static int ivSize(Mode mode); private: static QString modeToString(const Mode mode); diff --git a/src/sshagent/OpenSSHKey.cpp b/src/sshagent/OpenSSHKey.cpp index e7a2a9b85..e6b21c863 100644 --- a/src/sshagent/OpenSSHKey.cpp +++ b/src/sshagent/OpenSSHKey.cpp @@ -30,6 +30,7 @@ const QString OpenSSHKey::TYPE_DSA_PRIVATE = "DSA PRIVATE KEY"; const QString OpenSSHKey::TYPE_RSA_PRIVATE = "RSA PRIVATE KEY"; const QString OpenSSHKey::TYPE_OPENSSH_PRIVATE = "OPENSSH PRIVATE KEY"; +const QString OpenSSHKey::OPENSSH_CIPHER_SUFFIX = "@openssh.com"; OpenSSHKey::OpenSSHKey(QObject* parent) : QObject(parent) @@ -310,9 +311,16 @@ bool OpenSSHKey::openKey(const QString& passphrase) QByteArray rawData = m_rawData; if (m_cipherName != "none") { - auto cipherMode = SymmetricCipher::stringToMode(m_cipherName); + QString l_cipherName(m_cipherName); + if (l_cipherName.endsWith(OPENSSH_CIPHER_SUFFIX)) { + l_cipherName.remove(OPENSSH_CIPHER_SUFFIX); + } + auto cipherMode = SymmetricCipher::stringToMode(l_cipherName); if (cipherMode == SymmetricCipher::InvalidMode) { - m_error = tr("Unknown cipher: %1").arg(m_cipherName); + m_error = tr("Unknown cipher: %1").arg(l_cipherName); + return false; + } else if (cipherMode == SymmetricCipher::Aes256_GCM) { + m_error = tr("AES-256/GCM is currently not supported"); return false; } @@ -325,7 +333,7 @@ bool OpenSSHKey::openKey(const QString& passphrase) } int keySize = cipher->keySize(cipherMode); - int blockSize = 16; + int ivSize = cipher->ivSize(cipherMode); BinaryStream optionStream(&m_kdfOptions); @@ -335,7 +343,7 @@ bool OpenSSHKey::openKey(const QString& passphrase) optionStream.readString(salt); optionStream.read(rounds); - QByteArray decryptKey(keySize + blockSize, '\0'); + QByteArray decryptKey(keySize + ivSize, '\0'); try { auto baPass = passphrase.toUtf8(); auto pwhash = Botan::PasswordHashFamily::create_or_throw("Bcrypt-PBKDF")->from_iterations(rounds); @@ -351,7 +359,7 @@ bool OpenSSHKey::openKey(const QString& passphrase) } keyData = decryptKey.left(keySize); - ivData = decryptKey.right(blockSize); + ivData = decryptKey.right(ivSize); } else if (m_kdfName == "md5") { if (m_cipherIV.length() < 8) { m_error = tr("Cipher IV is too short for MD5 kdf"); diff --git a/src/sshagent/OpenSSHKey.h b/src/sshagent/OpenSSHKey.h index 78ccf7192..a42e433de 100644 --- a/src/sshagent/OpenSSHKey.h +++ b/src/sshagent/OpenSSHKey.h @@ -58,6 +58,7 @@ public: static const QString TYPE_DSA_PRIVATE; static const QString TYPE_RSA_PRIVATE; static const QString TYPE_OPENSSH_PRIVATE; + static const QString OPENSSH_CIPHER_SUFFIX; private: enum KeyPart