From 6b2cb4ae793078151e1a4f9bae0a744e2a069783 Mon Sep 17 00:00:00 2001 From: Matthias Wiedemann Date: Thu, 4 Mar 2021 09:45:01 +0100 Subject: [PATCH 1/2] #34 throwing IllegalstateException when encrypted openssh v1 key is used without password --- .../java/com/jcraft/jsch/KeyPairDeferred.java | 36 ++++++++++-------- .../java/com/jcraft/jsch/KeyPairTest.java | 37 +++++++++++++------ 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/jcraft/jsch/KeyPairDeferred.java b/src/main/java/com/jcraft/jsch/KeyPairDeferred.java index a2a5c2d4..b0dabaff 100644 --- a/src/main/java/com/jcraft/jsch/KeyPairDeferred.java +++ b/src/main/java/com/jcraft/jsch/KeyPairDeferred.java @@ -5,7 +5,7 @@ import java.util.Arrays; /** - * A {@link KeyPair} which can only reveal its type and content after it was decrypted using KeyPairDeferred{@link #decrypt(byte[])}. + * A {@link KeyPair} which can only reveal its type and content after it was decrypted using {@link com.jcraft.jsch.KeyPairDeferred#decrypt(byte[])}. * This is needed for openssh-v1-private-key format. */ public class KeyPairDeferred extends KeyPair { @@ -72,62 +72,62 @@ void generate(int key_size) throws JSchException { @Override byte[] getBegin() { - return delegate.getBegin(); + return requireDecrypted(delegate).getBegin(); } @Override byte[] getEnd() { - return delegate.getEnd(); + return requireDecrypted(delegate).getEnd(); } @Override int getKeySize() { - return delegate.getKeySize(); + return requireDecrypted(delegate).getKeySize(); } @Override public byte[] getSignature(byte[] data) { - return delegate.getSignature(data); + return requireDecrypted(delegate).getSignature(data); } @Override public byte[] getSignature(byte[] data, String alg) { - return delegate.getSignature(data, alg); + return requireDecrypted(delegate).getSignature(data, alg); } @Override public Signature getVerifier() { - return delegate.getVerifier(); + return requireDecrypted(delegate).getVerifier(); } @Override public Signature getVerifier(String alg) { - return delegate.getVerifier(alg); + return requireDecrypted(delegate).getVerifier(alg); } @Override public byte[] forSSHAgent() throws JSchException { - return delegate.forSSHAgent(); + return requireDecrypted(delegate).forSSHAgent(); } @Override byte[] getPrivateKey() { - return delegate.getPrivateKey(); + return requireDecrypted(delegate).getPrivateKey(); } @Override byte[] getKeyTypeName() { - return delegate.getKeyTypeName(); + return requireDecrypted(delegate).getKeyTypeName(); } @Override public int getKeyType() { - return delegate.getKeyType(); + return requireDecrypted(delegate).getKeyType(); } @Override boolean parse(byte[] data) { - return delegate.parse(data); + return requireDecrypted(delegate).parse(data); } @Override @@ -137,16 +137,22 @@ public byte[] getPublicKeyBlob() { @Override public String getPublicKeyComment() { - return delegate.getPublicKeyComment(); + return requireDecrypted(delegate).getPublicKeyComment(); } @Override public String getFingerPrint() { - return delegate.getFingerPrint(); + return requireDecrypted(delegate).getFingerPrint(); } @Override public boolean isEncrypted() { return delegate != null ? delegate.isEncrypted() : super.isEncrypted(); } + + private T requireDecrypted(T obj) { + if (obj == null) + throw new IllegalStateException("encrypted key has not been decrypted yet."); + return obj; + } } diff --git a/src/test/java/com/jcraft/jsch/KeyPairTest.java b/src/test/java/com/jcraft/jsch/KeyPairTest.java index 296c3a85..8687e9a1 100644 --- a/src/test/java/com/jcraft/jsch/KeyPairTest.java +++ b/src/test/java/com/jcraft/jsch/KeyPairTest.java @@ -11,15 +11,16 @@ import java.net.URISyntaxException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Objects; import java.util.stream.Stream; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertTrue; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.*; class KeyPairTest { - @TempDir public Path tmpDir; + @TempDir + public Path tmpDir; @BeforeAll static void init() { @@ -41,21 +42,20 @@ static Stream keyArgs() { // encrypted dsa Arguments.of("encrypted_openssh_private_key_dsa", "secret123", "ssh-dss"), // ecdsa EC private key format - Arguments.of("docker/id_ecdsa256", null, null), // - Arguments.of("docker/id_ecdsa384", null, null), // - Arguments.of("docker/id_ecdsa521", null, null), - Arguments.of("docker/ssh_host_ecdsa256_key", null, null), - Arguments.of("docker/ssh_host_ecdsa384_key", null, null), - Arguments.of("docker/ssh_host_ecdsa521_key", null, null), + Arguments.of("docker/id_ecdsa256", null, "ecdsa-sha2-nistp256"), // + Arguments.of("docker/id_ecdsa384", null, "ecdsa-sha2-nistp384"), // + Arguments.of("docker/id_ecdsa521", null, "ecdsa-sha2-nistp521"), + Arguments.of("docker/ssh_host_ecdsa256_key", null, "ecdsa-sha2-nistp256"), + Arguments.of("docker/ssh_host_ecdsa384_key", null, "ecdsa-sha2-nistp384"), + Arguments.of("docker/ssh_host_ecdsa521_key", null, "ecdsa-sha2-nistp521"), // encrypted ecdsa - Arguments.of("encrypted_openssh_private_key_ecdsa", "secret123", null) - + Arguments.of("encrypted_openssh_private_key_ecdsa", "secret123", "ecdsa-sha2-nistp256") ); } @ParameterizedTest @MethodSource("keyArgs") - void loadKey(String path, String password, String publicKeyType) throws URISyntaxException, JSchException { + void loadKey(String path, String password, String keyType) throws URISyntaxException, JSchException { final JSch jSch = new JSch(); final String prvkey = Paths.get(ClassLoader.getSystemResource(path).toURI()).toFile().getAbsolutePath(); assertTrue(new File(prvkey).exists()); @@ -66,6 +66,7 @@ void loadKey(String path, String password, String publicKeyType) throws URISynta jSch.addIdentity(prvkey); } }); + assertEquals(keyType, jSch.getIdentityRepository().getIdentities().get(0).getAlgName()); } @Test @@ -86,4 +87,16 @@ void genKeypairEncrypted() { }); } + @Test + void loadAndAcessEncryptedKeyWithoutPassword() throws URISyntaxException, JSchException { + final JSch jSch = new JSch(); + final String prvkey = Paths.get(ClassLoader.getSystemResource("encrypted_openssh_private_key_dsa").toURI()).toFile().getAbsolutePath(); + assertTrue(new File(prvkey).exists()); + jSch.addIdentity(prvkey); + + assertThrows(IllegalStateException.class, () -> { + jSch.getIdentityRepository().getIdentities().get(0).getAlgName(); + }); + } + } From 7439f7a073dcdd154f61ad83b91c7568f2bdb394 Mon Sep 17 00:00:00 2001 From: Matthias Wiedemann Date: Thu, 4 Mar 2021 12:36:45 +0000 Subject: [PATCH 2/2] moving up key decryption to handle openssh v1 format --- .../com/jcraft/jsch/UserAuthPublicKey.java | 77 ++++++++++--------- src/test/java/com/jcraft/jsch/KeyPairIT.java | 53 +++++++++++++ .../java/com/jcraft/jsch/KeyPairTest.java | 1 - 3 files changed, 94 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/jcraft/jsch/UserAuthPublicKey.java b/src/main/java/com/jcraft/jsch/UserAuthPublicKey.java index a84c349d..a0527a47 100644 --- a/src/main/java/com/jcraft/jsch/UserAuthPublicKey.java +++ b/src/main/java/com/jcraft/jsch/UserAuthPublicKey.java @@ -39,7 +39,6 @@ public boolean start(Session session) throws Exception{ Vector identities=session.getIdentityRepository().getIdentities(); - byte[] passphrase=null; byte[] _username=null; int command; @@ -89,6 +88,10 @@ public boolean start(Session session) throws Exception{ Identity identity=identities.elementAt(i); + //System.err.println("UserAuthPublicKey: identity.isEncrypted()="+identity.isEncrypted()); + decryptKey(session, identity); + //System.err.println("UserAuthPublicKey: identity.isEncrypted()="+identity.isEncrypted()); + String ipkmethod=identity.getAlgName(); String[] ipkmethoda=null; if(ipkmethod.equals("ssh-rsa")){ @@ -182,42 +185,7 @@ else if(command==SSH_MSG_USERAUTH_BANNER){ } } -//System.err.println("UserAuthPublicKey: identity.isEncrypted()="+identity.isEncrypted()); - - int count=5; - while(true){ - if((identity.isEncrypted() && passphrase==null)){ - if(userinfo==null) throw new JSchException("USERAUTH fail"); - if(identity.isEncrypted() && - !userinfo.promptPassphrase("Passphrase for "+identity.getName())){ - throw new JSchAuthCancelException("publickey"); - //throw new JSchException("USERAUTH cancel"); - //break; - } - String _passphrase=userinfo.getPassphrase(); - if(_passphrase!=null){ - passphrase=Util.str2byte(_passphrase); - } - } - - if(!identity.isEncrypted() || passphrase!=null){ - if(identity.setPassphrase(passphrase)){ - if(passphrase!=null && - (session.getIdentityRepository() instanceof IdentityRepository.Wrapper)){ - ((IdentityRepository.Wrapper)session.getIdentityRepository()).check(); - } - break; - } - } - Util.bzero(passphrase); - passphrase=null; - count--; - if(count==0)break; - } - Util.bzero(passphrase); - passphrase=null; -//System.err.println("UserAuthPublicKey: identity.isEncrypted()="+identity.isEncrypted()); if(identity.isEncrypted()) continue; if(pubkeyblob==null) pubkeyblob=identity.getPublicKeyBlob(); @@ -322,4 +290,41 @@ else if(command==SSH_MSG_USERAUTH_FAILURE){ } return false; } + + private void decryptKey(Session session, Identity identity) throws JSchException { + byte[] passphrase=null; + int count=5; + while(true){ + if((identity.isEncrypted() && passphrase==null)){ + if(userinfo==null) throw new JSchException("USERAUTH fail"); + if(identity.isEncrypted() && + !userinfo.promptPassphrase("Passphrase for "+identity.getName())){ + throw new JSchAuthCancelException("publickey"); + //throw new JSchException("USERAUTH cancel"); + //break; + } + String _passphrase=userinfo.getPassphrase(); + if(_passphrase!=null){ + passphrase= Util.str2byte(_passphrase); + } + } + + if(!identity.isEncrypted() || passphrase!=null){ + if(identity.setPassphrase(passphrase)){ + if(passphrase!=null && + (session.getIdentityRepository() instanceof IdentityRepository.Wrapper)){ + ((IdentityRepository.Wrapper)session.getIdentityRepository()).check(); + } + break; + } + } + Util.bzero(passphrase); + passphrase=null; + count--; + if(count==0)break; + } + + Util.bzero(passphrase); + passphrase=null; + } } diff --git a/src/test/java/com/jcraft/jsch/KeyPairIT.java b/src/test/java/com/jcraft/jsch/KeyPairIT.java index 2d0d820b..bbe79cb8 100644 --- a/src/test/java/com/jcraft/jsch/KeyPairIT.java +++ b/src/test/java/com/jcraft/jsch/KeyPairIT.java @@ -47,6 +47,59 @@ void connectWithPublicKey(String path, String password, String keyType) throws E } + @ParameterizedTest + @MethodSource("com.jcraft.jsch.KeyPairTest#keyArgs") + void connectWithPublicKeyAndUserInfo(String path, String password, String keyType) throws Exception { + + final JSch jSch = new JSch(); + + jSch.addIdentity(Paths.get(ClassLoader.getSystemResource(path).toURI()).toFile().getAbsolutePath()); + + Session session = createSession(jSch); + session.setUserInfo(new UserInfo() { + @Override + public String getPassphrase() { + return password; + } + + @Override + public String getPassword() { + return null; + } + + @Override + public boolean promptPassword(String message) { + return false; + } + + @Override + public boolean promptPassphrase(String message) { + return true; + } + + @Override + public boolean promptYesNo(String message) { + return false; + } + + @Override + public void showMessage(String message) { + + } + }); + + if (keyType != null) { + session.setConfig("PubkeyAcceptedKeyTypes", keyType); + } + try { + session.connect(2000); + assertTrue(session.isConnected()); + } finally { + session.disconnect(); + } + + } + private JSch createIdentity(String path, String password) throws JSchException, URISyntaxException { JSch ssh = new JSch(); if (password != null) { diff --git a/src/test/java/com/jcraft/jsch/KeyPairTest.java b/src/test/java/com/jcraft/jsch/KeyPairTest.java index 8687e9a1..ee90cf66 100644 --- a/src/test/java/com/jcraft/jsch/KeyPairTest.java +++ b/src/test/java/com/jcraft/jsch/KeyPairTest.java @@ -11,7 +11,6 @@ import java.net.URISyntaxException; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Objects; import java.util.stream.Stream; import static java.nio.charset.StandardCharsets.UTF_8;