This is an automated email from the ASF dual-hosted git repository. twolf pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push: new f88741409 OpenSSH key parsing: minor clean-ups in new AEAD code paths f88741409 is described below commit f88741409b1473b3e63a91d2f1d52e689527067a Author: Thomas Wolf <tw...@apache.org> AuthorDate: Thu Apr 6 22:57:30 2023 +0200 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. --- .../config/keys/loader/openssh/OpenSSHKeyPairResourceParser.java | 9 ++++++++- .../common/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 class OpenSSHKeyPairResourceParser extends AbstractKeyPairResourceParser 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 class OpenSSHKeyPairResourceParser extends AbstractKeyPairResourceParser // 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 class BCryptKdfOptions implements OpenSSHKdfOptions { 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);