Skip to content

Commit

Permalink
OpenSSH key parsing: minor clean-ups in new AEAD code paths
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tomaswolf committed Apr 6, 2023
1 parent 752faa5 commit f887414
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public Collection<KeyPair> 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;
Expand All @@ -164,6 +164,13 @@ public Collection<KeyPair> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit f887414

Please sign in to comment.