Skip to content

Commit

Permalink
Fix support for AES-256/GCM openssh keys (#8968)
Browse files Browse the repository at this point in the history
* 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).
  • Loading branch information
novasharper authored Jan 29, 2023
1 parent 03ad6c5 commit e221f89
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 5 deletions.
4 changes: 4 additions & 0 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5764,6 +5764,10 @@ We recommend you use the AppImage available on our downloads page.</source>
<source>Unexpected EOF when writing private key</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>AES-256/GCM is currently not supported</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>PasswordEditWidget</name>
Expand Down
19 changes: 19 additions & 0 deletions src/crypto/SymmetricCipher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
1 change: 1 addition & 0 deletions src/crypto/SymmetricCipher.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class SymmetricCipher
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);
Expand Down
18 changes: 13 additions & 5 deletions src/sshagent/OpenSSHKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);

Expand All @@ -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);
Expand All @@ -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");
Expand Down
1 change: 1 addition & 0 deletions src/sshagent/OpenSSHKey.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class OpenSSHKey : public QObject
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
Expand Down

0 comments on commit e221f89

Please sign in to comment.