From 8463fcaac3b288182c208f933888ecbb95048b23 Mon Sep 17 00:00:00 2001 From: sdikov Date: Wed, 20 Oct 2021 22:22:48 +0300 Subject: [PATCH] - Fix keystore loading for PKCS12 files; Update JSch to 0.1.55 - Clarifications in FileTransferClient --- .../ats/action/ActionLibraryConfigurator.java | 2 +- .../filetransfer/FileTransferClient.java | 8 +-- .../ats/core/filetransfer/SftpClient.java | 4 +- .../com/axway/ats/core/utils/SslUtils.java | 63 ++++++++++++------- pom.xml | 2 +- 5 files changed, 48 insertions(+), 31 deletions(-) diff --git a/actionlibrary/src/main/java/com/axway/ats/action/ActionLibraryConfigurator.java b/actionlibrary/src/main/java/com/axway/ats/action/ActionLibraryConfigurator.java index 8ac383f93..1e0e26ac5 100644 --- a/actionlibrary/src/main/java/com/axway/ats/action/ActionLibraryConfigurator.java +++ b/actionlibrary/src/main/java/com/axway/ats/action/ActionLibraryConfigurator.java @@ -147,7 +147,7 @@ public boolean getFileTransferVerboseMode() { /** * Set the file transfer verbose mode * - * @param verbose mode the file transfer verbose mode + * @param verboseMode if true, then log verbosity is increased */ @PublicAtsApi public void setFileTransferVerboseMode( diff --git a/actionlibrary/src/main/java/com/axway/ats/action/filetransfer/FileTransferClient.java b/actionlibrary/src/main/java/com/axway/ats/action/filetransfer/FileTransferClient.java index 023807502..ca0b24f74 100644 --- a/actionlibrary/src/main/java/com/axway/ats/action/filetransfer/FileTransferClient.java +++ b/actionlibrary/src/main/java/com/axway/ats/action/filetransfer/FileTransferClient.java @@ -313,8 +313,7 @@ public void addCustomProperty( String key, Object value ) throws IllegalArgument } /** - * Uploads a the file to the specified directory and with the specified file - * name + * Uploads a local file to the specified remote directory, with the specified remote file name * * @param localFile * the local file to upload @@ -336,8 +335,7 @@ public void uploadFile( @Validate( name = "localFile", type = ValidationType.STR } /** - * Upload a the file to the specified directory and with the specified file - * name + * Upload a local file to the specified remote directory, with the same name as local name * * @param localFile * the local file to upload @@ -352,7 +350,7 @@ public void uploadFile( String localFile, String remoteDir ) { } /** - * Download a file from the specified directory and with the specified file + * Download a file from the specified directory with the specified local file name * name * * @param localDir diff --git a/corelibrary/src/main/java/com/axway/ats/core/filetransfer/SftpClient.java b/corelibrary/src/main/java/com/axway/ats/core/filetransfer/SftpClient.java index 370e019cc..093d82fe0 100644 --- a/corelibrary/src/main/java/com/axway/ats/core/filetransfer/SftpClient.java +++ b/corelibrary/src/main/java/com/axway/ats/core/filetransfer/SftpClient.java @@ -126,7 +126,7 @@ public void connect( String hostname, String userName, String password ) throws if( !StringUtils.isNullOrEmpty( this.keyStoreFile ) ) { log.info( "Keystore location set to '" + this.keyStoreFile + "'" ); } - + // TODO: comment/remove as disconnect() is invoked in doConnect() if( this.channel != null ) { this.channel.disconnect(); } @@ -179,7 +179,7 @@ private void doConnect() throws FileTransferException { this.jsch.addIdentity( "client_prvkey", privateKeyContent, null, null ); } - //The internally used client version 'SSH-2.0-JSCH-0.1.54' needs to be changed to 'SSH-2.0-OpenSSH_2.5.3' + //The internally used client version 'SSH-2.0-JSCH-0.1.54' (even for v0.1.55) needs to be changed to 'SSH-2.0-OpenSSH_2.5.3' this.session.setClientVersion( "SSH-2.0-OpenSSH_2.5.3" ); /** if no trust server certificate or trust store are provided, do not check if hostname is in known hosts **/ diff --git a/corelibrary/src/main/java/com/axway/ats/core/utils/SslUtils.java b/corelibrary/src/main/java/com/axway/ats/core/utils/SslUtils.java index f556530b9..8bd93b0e4 100644 --- a/corelibrary/src/main/java/com/axway/ats/core/utils/SslUtils.java +++ b/corelibrary/src/main/java/com/axway/ats/core/utils/SslUtils.java @@ -168,8 +168,8 @@ public static SSLContext getSSLContext( String certFileName, String certPassword * Trust-all SSL context. * Optionally specify certificate file to create the keystore from. * - * @param certFileName - * @param certPassword + * @param certFileName - PEM file to import key + * @param certPassword - password for the cert file * @param protocol e.g. TLS, TLSv1.2 * @return */ @@ -394,32 +394,51 @@ public static KeyStore loadKeystore( String keystoreFile, String keystorePasswor log.debug("Load keystore '" + keystoreFile + "' file with '" + keystorePassword + "' password"); } + Exception cause = null; KeyStore keystore = null; - try (FileInputStream fis = new FileInputStream(new File(keystoreFile))) { - keystore = KeyStore.getInstance("JKS"); - keystore.load(fis, keystorePassword.toCharArray()); - return keystore; - } catch (Exception e) { - if (log.isDebugEnabled()) { - log.debug("Error loading JKS keystore '" + keystoreFile + "' file with '" + keystorePassword - + "' password. Maybe its type is not JKS:\n" + e); + /* Add explicit check for keystore type (JKS/PKCS`1) by checking extension. + Recent versions (Java 11) support both JKS and PKCS12 but reading .p12 keystore with JKS instance produces + not clear NPE exception: + Caused by: java.security.UnrecoverableKeyException: Get Key failed: null + at java.base/sun.security.pkcs12.PKCS12KeyStore.engineGetKey(PKCS12KeyStore.java:446) + at java.base/sun.security.util.KeyStoreDelegator.engineGetKey(KeyStoreDelegator.java:90) + at java.base/java.security.KeyStore.getKey(KeyStore.java:1057) + at com.axway.ats.core.filetransfer.SftpClient.getPrivateKeyContent(SftpClient.java:326) + ... 29 more + Caused by: java.lang.NullPointerException + at java.base/sun.security.pkcs12.PKCS12KeyStore$RetryWithZero.run(PKCS12KeyStore.java:278) + at java.base/sun.security.pkcs12.PKCS12KeyStore.engineGetKey(PKCS12KeyStore.java:381) + ... 32 more + */ + if (keystoreFile.endsWith(".jks")) { // old JKS format; Newer JVMs support by default PKCS12 + try (FileInputStream fis = new FileInputStream(new File(keystoreFile))) { + keystore = KeyStore.getInstance("JKS"); + keystore.load(fis, keystorePassword.toCharArray()); + return keystore; + } catch (Exception e) { + cause = e; + if (log.isDebugEnabled()) { + log.debug("Error loading JKS keystore '" + keystoreFile + "' file with '" + keystorePassword + + "' password. Maybe its type is not JKS:\n" + e); + } } - } - - try (FileInputStream fis = new FileInputStream(new File(keystoreFile))) { - keystore = KeyStore.getInstance("PKCS12"); - keystore.load(fis, keystorePassword.toCharArray()); - return keystore; - } catch (Exception e) { - if (log.isDebugEnabled()) { - log.debug("Error loading PKCS12 keystore '" + keystoreFile + "' file with '" - + keystorePassword + "' password. Maybe its type is not PKCS12:\n" - + e); + } else { // assume P12, PKCS12 format + try (FileInputStream fis = new FileInputStream(new File(keystoreFile))) { + keystore = KeyStore.getInstance("PKCS12"); + keystore.load(fis, keystorePassword.toCharArray()); + return keystore; + } catch (Exception e) { + cause = e; + if (log.isDebugEnabled()) { + log.debug("Error loading PKCS12 keystore '" + keystoreFile + "' file with '" + + keystorePassword + "' password. Maybe its type is not PKCS12:\n" + + e); + } } } throw new RuntimeException("Error loading keystore '" + keystoreFile + "' file with '" - + keystorePassword + "' password"); + + keystorePassword + "' password", cause); } /** diff --git a/pom.xml b/pom.xml index ec276d6ad..522c410f1 100644 --- a/pom.xml +++ b/pom.xml @@ -102,7 +102,7 @@ 2.0.1 - 0.1.54 + 0.1.55 2.1.3