From f88741409b1473b3e63a91d2f1d52e689527067a Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 6 Apr 2023 22:57:30 +0200 Subject: [PATCH] OpenSSH key parsing: minor clean-ups in new AEAD code paths Guard against corrupted key files with excessive or negative key data lengths (AEAD encryption only; other code paths were already guarding against this.) Avoid an extra copy of decrypted key data if no MAC is used by the cipher. --- .../loader/openssh/OpenSSHKeyPairResourceParser.java | 9 ++++++++- .../config/keys/loader/openssh/kdf/BCryptKdfOptions.java | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHKeyPairResourceParser.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHKeyPairResourceParser.java index 09e8d61bc..01c8fd35d 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHKeyPairResourceParser.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/OpenSSHKeyPairResourceParser.java @@ -154,7 +154,7 @@ public Collection extractKeyPairs( CipherFactory cipherSpec = BuiltinCiphers.resolveFactory(cipher); if (cipherSpec == null || !cipherSpec.isSupported()) { - throw new NoSuchAlgorithmException("Unsupported cipher: " + cipher); + throw new NoSuchAlgorithmException("Unsupported cipher: " + cipher + " for encrypted key in " + resourceKey); } byte[] encryptedData; @@ -164,6 +164,13 @@ public Collection extractKeyPairs( // to follow the payload data. int authTokenLength = cipherSpec.getAuthenticationTagSize(); int encryptedLength = KeyEntryResolver.decodeInt(stream); + if (encryptedLength < 0) { + throw new StreamCorruptedException( + "Key length " + encryptedLength + " negative for encrypted key in " + resourceKey); + } else if (encryptedLength > MAX_PRIVATE_KEY_DATA_SIZE) { + throw new StreamCorruptedException("Key length " + encryptedLength + " > allowed maximum " + + MAX_PRIVATE_KEY_DATA_SIZE + " for encrypted key in " + resourceKey); + } encryptedData = new byte[encryptedLength + authTokenLength]; IoUtils.readFully(stream, encryptedData); } else { diff --git a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java index 8c62b021e..3838e0d10 100644 --- a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java +++ b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/loader/openssh/kdf/BCryptKdfOptions.java @@ -137,6 +137,12 @@ public byte[] decodePrivateKeyBytes( int macLength = cipherSpec.getAuthenticationTagSize(); cipher.init(Cipher.Mode.Decrypt, kv, iv); cipher.update(sensitive, 0, sensitive.length - macLength); + if (macLength == 0) { + // Avoid an extra copy + byte[] result = sensitive; + sensitive = null; // Don't clear in finalization + return result; + } return Arrays.copyOf(sensitive, sensitive.length - macLength); } catch (RuntimeException e) { Throwable t = ExceptionUtils.peelException(e);