Add ChaCha20-Poly1305 Support for OpenSSH Keys (#904)

* Add ChaCha20-Poly1305 Support for OpenSSH Keys - Updated ChachaPolyCipher to support decryption without Additional Authenticated Data * Added test for ChachaPolyCipher without AAD * Streamlined ChachaPolyCipher.update() method
hierynomus · Oct 16, 2023 · 542bb35 · 542bb35
1 parent 3b67d2b
commit 542bb35
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 17 deletions.
diff --git a/src/main/java/com/hierynomus/sshj/transport/cipher/ChachaPolyCipher.java b/src/main/java/com/hierynomus/sshj/transport/cipher/ChachaPolyCipher.java
@@ -16,9 +16,8 @@
 package com.hierynomus.sshj.transport.cipher;
 
 import java.security.GeneralSecurityException;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
 
+import java.security.MessageDigest;
 import java.security.spec.AlgorithmParameterSpec;
 import java.util.Arrays;
 import javax.crypto.spec.IvParameterSpec;
@@ -82,8 +81,7 @@ public void setSequenceNumber(long seq) {
     }
 
     @Override
-    protected void initCipher(javax.crypto.Cipher cipher, Mode mode, byte[] key, byte[] iv)
-            throws InvalidKeyException, InvalidAlgorithmParameterException {
+    protected void initCipher(javax.crypto.Cipher cipher, Mode mode, byte[] key, byte[] iv) {
         this.mode = mode;
 
         cipherKey = getKeySpec(Arrays.copyOfRange(key, 0, CHACHA_KEY_SIZE));
@@ -127,28 +125,34 @@ public void updateAAD(byte[] data) {
 
     @Override
     public void update(byte[] input, int inputOffset, int inputLen) {
-        if (inputOffset != AAD_LENGTH) {
+        if (inputOffset != 0 && inputOffset != AAD_LENGTH) {
             throw new IllegalArgumentException("updateAAD called with inputOffset " + inputOffset);
         }
 
-        final int macInputLength = AAD_LENGTH + inputLen;
-
+        final int macInputLength = inputOffset + inputLen;
         if (mode == Mode.Decrypt) {
-            byte[] macInput = new byte[macInputLength];
-            System.arraycopy(encryptedAad, 0, macInput, 0, AAD_LENGTH);
-            System.arraycopy(input, AAD_LENGTH, macInput, AAD_LENGTH, inputLen);
+            final byte[] macInput = new byte[macInputLength];
+
+            if (inputOffset == 0) {
+                // Handle decryption without AAD
+                System.arraycopy(input, 0, macInput, 0, inputLen);
+            } else {
+                // Handle decryption with previous AAD from updateAAD()
+                System.arraycopy(encryptedAad, 0, macInput, 0, AAD_LENGTH);
+                System.arraycopy(input, AAD_LENGTH, macInput, AAD_LENGTH, inputLen);
+            }
 
-            byte[] expectedPolyTag = mac.doFinal(macInput);
-            byte[] actualPolyTag = Arrays.copyOfRange(input, macInputLength, macInputLength + POLY_TAG_LENGTH);
-            if (!Arrays.equals(actualPolyTag, expectedPolyTag)) {
+            final byte[] expectedPolyTag = mac.doFinal(macInput);
+            final byte[] actualPolyTag = Arrays.copyOfRange(input, macInputLength, macInputLength + POLY_TAG_LENGTH);
+            if (!MessageDigest.isEqual(actualPolyTag, expectedPolyTag)) {
                 throw new SSHRuntimeException("MAC Error");
             }
         }
 
         try {
-            cipher.update(input, AAD_LENGTH, inputLen, input, AAD_LENGTH);
+            cipher.update(input, inputOffset, inputLen, input, inputOffset);
         } catch (GeneralSecurityException e) {
-            throw new SSHRuntimeException("Error updating data through cipher", e);
+            throw new SSHRuntimeException("ChaCha20 cipher processing failed", e);
         }
 
         if (mode == Mode.Encrypt) {

diff --git a/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java b/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java
@@ -18,6 +18,7 @@
 import com.hierynomus.sshj.common.KeyAlgorithm;
 import com.hierynomus.sshj.common.KeyDecryptionFailedException;
 import com.hierynomus.sshj.transport.cipher.BlockCiphers;
+import com.hierynomus.sshj.transport.cipher.ChachaPolyCiphers;
 import com.hierynomus.sshj.transport.cipher.GcmCiphers;
 import net.i2p.crypto.eddsa.EdDSAPrivateKey;
 import net.i2p.crypto.eddsa.spec.EdDSANamedCurveTable;
@@ -83,6 +84,7 @@ public class OpenSSHKeyV1KeyFile extends BaseFileKeyProvider {
         SUPPORTED_CIPHERS.put(BlockCiphers.AES256CTR().getName(), BlockCiphers.AES256CTR());
         SUPPORTED_CIPHERS.put(GcmCiphers.AES256GCM().getName(), GcmCiphers.AES256GCM());
         SUPPORTED_CIPHERS.put(GcmCiphers.AES128GCM().getName(), GcmCiphers.AES128GCM());
+        SUPPORTED_CIPHERS.put(ChachaPolyCiphers.CHACHA_POLY_OPENSSH().getName(), ChachaPolyCiphers.CHACHA_POLY_OPENSSH());
     }
 
     private PublicKey pubKey;
@@ -192,7 +194,7 @@ private byte[] readEncryptedPrivateKey(final byte[] privateKeyEncoded, final Pla
         if (bufferRemaining == 0) {
             encryptedPrivateKey = privateKeyEncoded;
         } else {
-            // Read Authentication Tag for AES-GCM
+            // Read Authentication Tag for AES-GCM or ChaCha20-Poly1305
             final byte[] authenticationTag = new byte[bufferRemaining];
             inputBuffer.readRawBytes(authenticationTag);
 
@@ -314,7 +316,7 @@ private KeyPair readUnencrypted(final PlainBuffer keyBuffer, final PublicKey pub
         int checkInt1 = keyBuffer.readUInt32AsInt(); // uint32 checkint1
         int checkInt2 = keyBuffer.readUInt32AsInt(); // uint32 checkint2
         if (checkInt1 != checkInt2) {
-            throw new KeyDecryptionFailedException();
+            throw new KeyDecryptionFailedException(new EncryptionException("OpenSSH Private Key integer comparison failed"));
         }
         // The private key section contains both the public key and the private key
         String keyType = keyBuffer.readString(); // string keytype

diff --git a/src/test/java/com/hierynomus/sshj/transport/ChachaPolyCipherTest.java b/src/test/java/com/hierynomus/sshj/transport/ChachaPolyCipherTest.java
@@ -70,6 +70,28 @@ public void testEncryptDecrypt() {
         }
     }
 
+    @Test
+    public void testEncryptDecryptWithoutAAD() {
+        final Cipher encryptionCipher = FACTORY.create();
+        final byte[] key = new byte[encryptionCipher.getBlockSize()];
+        Arrays.fill(key, (byte) 1);
+        encryptionCipher.init(Cipher.Mode.Encrypt, key, new byte[0]);
+
+        final byte[] plaintextBytes = PLAINTEXT.getBytes(StandardCharsets.UTF_8);
+        final byte[] message = new byte[plaintextBytes.length + POLY_TAG_LENGTH];
+        System.arraycopy(plaintextBytes, 0, message, 0, plaintextBytes.length);
+
+        encryptionCipher.update(message, 0, plaintextBytes.length);
+
+        final Cipher decryptionCipher = FACTORY.create();
+        decryptionCipher.init(Cipher.Mode.Decrypt, key, new byte[0]);
+        decryptionCipher.update(message, 0, plaintextBytes.length);
+
+        final byte[] decrypted = Arrays.copyOfRange(message, 0, plaintextBytes.length);
+        final String decoded = new String(decrypted, StandardCharsets.UTF_8);
+        assertEquals(PLAINTEXT, decoded);
+    }
+
     @Test
     public void testCheckOnUpdateParameters() {
         Cipher cipher = FACTORY.create();

diff --git a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java
@@ -265,6 +265,11 @@ public void shouldLoadProtectedEd25519PrivateKeyAES256GCM() throws IOException {
         checkOpenSSHKeyV1("src/test/resources/keytypes/ed25519_aes256-gcm", "sshjtest", true);
     }
 
+    @Test
+    public void shouldLoadProtectedEd25519PrivateKeyChaCha20Poly1305() throws IOException {
+        checkOpenSSHKeyV1("src/test/resources/keytypes/ed25519_chacha20-poly1305", "sshjtest", false);
+    }
+
     @Test
     public void shouldFailOnIncorrectPassphraseAfterRetries() {
         assertThrows(KeyDecryptionFailedException.class, () -> {

diff --git a/src/test/resources/keytypes/ed25519_chacha20-poly1305 b/src/test/resources/keytypes/ed25519_chacha20-poly1305
@@ -0,0 +1,9 @@
+-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAAHWNoYWNoYTIwLXBvbHkxMzA1QG9wZW5zc2guY29tAAAABm
+JjcnlwdAAAABgAAAAQilqbRL4q3X9kEqWdTsD5/gAAABAAAAABAAAAMwAAAAtzc2gtZWQy
+NTUxOQAAACCpvtXUZPONb1XDjLkHmP5mQrGryGaQsA68Nb+OAjaaEgAAAJh+Repmt76g31
+jlD1ITaJU298ZU3rFWgA/Hs3xnOTNPjhMMu9nzfoZAu0fraE1MBVaEgNKRpw7SG+2eDBOo
+3fvN3lF15i7Q8YHZd9alfcUg3FrvBzjd0Edx4AQxbSueibPFaqnwmVk/YzDiQHwlyWfA1x
+HbqxrbJf1S0i8Bt5OjLK6woGk0/lfWJmy82xIa1sa3ONkPVjaJncm/f2SKV7t2k1UP9/jx
+dLA=
+-----END OPENSSH PRIVATE KEY-----
diff --git a/src/test/resources/keytypes/ed25519_chacha20-poly1305.pub b/src/test/resources/keytypes/ed25519_chacha20-poly1305.pub
@@ -0,0 +1 @@
+ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKm+1dRk841vVcOMuQeY/mZCsavIZpCwDrw1v44CNpoS
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKm+1dRk841vVcOMuQeY/mZCsavIZpCwDrw1v44CNpoS