SSH Agent: Support AES-128-CBC encrypted keys #1463
Conversation
src/sshagent/OpenSSHKey.cpp
Outdated
| m_error = tr("Encrypted keys are not yet supported"); | ||
| return false; | ||
| if (pemOptions.value("Proc-Type") == "4,ENCRYPTED") { | ||
| m_kdfName = "md5"; |
src/sshagent/OpenSSHKey.cpp
Outdated
| @@ -308,12 +309,14 @@ bool OpenSSHKey::openPrivateKey(const QString& passphrase) | |||
| return false; | |||
| } | |||
|
|
|||
| if (m_cipherName == "aes256-cbc") { | |||
| if (m_cipherName == "AES-128-CBC") { | |||
There was a problem hiding this comment.
Why is this not all-lowercase like the others? Maybe use case-insensitive comparison?
There was a problem hiding this comment.
Both are using the raw values from the files and they are written a bit different depending on the file type (PEM vs OpenSSH).
There was a problem hiding this comment.
I assumed so. Are they guaranteed to be uppercase, though? If in doubt, just use case-insensitive comparison.
tests/TestSymmetricCipher.cpp
Outdated
| QCOMPARE(cipher.blockSize(), 16); | ||
|
|
||
| QCOMPARE(cipher.process(plainText, &ok), | ||
| cipherText); |
tests/TestSymmetricCipher.cpp
Outdated
| QCOMPARE(cipher.blockSize(), 16); | ||
|
|
||
| QCOMPARE(cipher.process(cipherText, &ok), | ||
| plainText); |
There was a problem hiding this comment.
Remove these line breaks, too.
tests/TestSymmetricCipher.cpp
Outdated
| QVERIFY(ok); | ||
|
|
||
| // padded with 16 0x16 bytes | ||
| QByteArray cipherTextPadded = cipherText + QByteArray::fromHex("3a3aa5e0213db1a9901f9036cf5102d2"); |
There was a problem hiding this comment.
You are trying to pad the cipher text here, which is incorrect and also makes no sense in general. cipherText + QByteArray::fromHex("3a3aa5e0213db1a9901f9036cf5102d2") is the cipher text for plainText + padding when encrypted with AES256 using the key 603deb1015ca71be2b73aef0857d77811f352c073b6108d72d9810a30914dff4. For AES128 with the key 2b7e151628aed2a6abf7158809cf4f3c, the cipher text is of course different.
The padding scheme seems to be PKCS#7, which means you need to add N padding bytes of value N to the plaintext. In case of a full block, this is 16 padding bytes of value 16 (which is 0x10 in hex, the comment is misleading).
hifi
force-pushed
the
feature/sshagent-encrypted-old-keys
branch
from
abb3000 to
2bb6fad
Compare
|
For now I removed the stream tests until I or someone else can add them properly without issues. Basic block processing is preserved to have some kind of test that the cipher works. Tried to clean up some of the string comparisons. It's not pretty but it works. The whole OpenSSHKey class is pending a refactor to integrate ASN1Key class into it and cleaning up some rough edges. |
hifi
force-pushed
the
feature/sshagent-encrypted-old-keys
branch
from
2bb6fad to
352d641
Compare
|
Fixed the padding test according to the information @phoerious provided on IRC. |
phoerious
force-pushed
the
feature/sshagent-encrypted-old-keys
branch
from
352d641 to
d58e3ca
Compare
- Add support for KDBX 4.0, Argon2 and ChaCha20 [#148, #1179, #1230, #1494] - Add SSH Agent feature [#1098, #1450, #1463] - Add preview panel with details of the selected entry [#879, #1338] - Add more and configurable columns to entry table and allow copying of values by double click [#1305] - Add KeePassXC-Browser API as a replacement for KeePassHTTP [#608] - Deprecate KeePassHTTP [#1392] - Add support for Steam one-time passwords [#1206] - Add support for multiple Auto-Type sequences for a single entry [#1390] - Adjust YubiKey HMAC-SHA1 challenge-response key generation for KDBX 4.0 [#1060] - Replace qHttp with cURL for website icon downloads [#1460] - Remove lock file [#1231] - Add option to create backup file before saving [#1385] - Ask to save a generated password before closing the entry password generator [#1499] - Resolve placeholders recursively [#1078] - Add Auto-Type button to the toolbar [#1056] - Improve window focus handling for Auto-Type dialogs [#1204, #1490] - Auto-Type dialog and password generator can now be exited with ESC [#1252, #1412] - Add optional dark tray icon [#1154] - Add new "Unsafe saving" option to work around saving problems with file sync services [#1385] - Add IBus support to AppImage and additional image formats to Windows builds [#1534, #1537] - Add diceware password generator to CLI [#1406] - Add --key-file option to CLI [#816, #824] - Add DBus interface for opening and closing KeePassXC databases [#283] - Add KDBX compression options to database settings [#1419] - Discourage use of old fixed-length key files in favor of arbitrary files [#1326, #1327] - Correct reference resolution in entry fields [#1486] - Fix window state and recent databases not being remembered on exit [#1453] - Correct history item generation when configuring TOTP for an entry [#1446] - Correct multiple TOTP bugs [#1414] - Automatic saving after every change is now a default [#279] - Allow creation of new entries during search [#1398] - Correct menu issues on macOS [#1335] - Allow compilation on OpenBSD [#1328] - Improve entry attachments view [#1139, #1298] - Fix auto lock for Gnome and Xfce [#910, #1249] - Don't remember key files in file dialogs when the setting is disabled [#1188] - Improve database merging and conflict resolution [#807, #1165] - Fix macOS pasteboard issues [#1202] - Improve startup times on some platforms [#1205] - Hide the notes field by default [#1124] - Toggle main window by clicking tray icon with the middle mouse button [#992] - Fix custom icons not copied over when databases are merged [#1008] - Allow use of DEL key to delete entries [#914] - Correct intermittent crash due to stale history items [#1527] - Sanitize newline characters in title, username and URL fields [#1502] - Reopen previously opened databases in correct order [#774] - Use system's zxcvbn library if available [#701] - Implement various i18n improvements [#690, #875, #1436]
Candidate for 2.3.0 as it makes the agent feature pretty much feature complete when it comes to common existing keys.
Description
Added required bits to existing OpenSSHKey implementation to decrypt this key type. Added tests for both AES-128-CBC and decrypting an RSA key with it.
Additionally fixed error message translation placeholders.
Motivation and context
RSA keys generated with
ssh-keygenuse AES-128-CBC encryption with MD5 used to hash the passphrase (with salt). This is currently still the default for OpenSSH and most likely represents the majority of encrypted SSH keys.How has this been tested?
New covering tests added, tested manually in the UI to verify.
Types of changes
Checklist: