From 86e1f895a7419710a3fb699e5301a8bcc0074722 Mon Sep 17 00:00:00 2001 From: Nidhi Date: Mon, 6 May 2024 18:25:32 +0530 Subject: [PATCH] fix: Start using SHA2 instead of SHA1 algorithms for signing during git operations (#33166) --- app/client/cypress/support/Pages/GitSync.ts | 1 - app/server/appsmith-git/pom.xml | 13 +- .../helpers/SshTransportConfigCallback.java | 114 +++++++++++++++--- app/server/appsmith-interfaces/pom.xml | 6 + .../external/git/constants/SSHConstants.java | 12 ++ .../external/git/utils/CryptoUtil.java | 42 +++++++ .../server/helpers/GitDeployKeyGenerator.java | 77 +++++++----- 7 files changed, 213 insertions(+), 52 deletions(-) create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java diff --git a/app/client/cypress/support/Pages/GitSync.ts b/app/client/cypress/support/Pages/GitSync.ts index b40cb9b335b..7dbbc5eed5a 100644 --- a/app/client/cypress/support/Pages/GitSync.ts +++ b/app/client/cypress/support/Pages/GitSync.ts @@ -157,7 +157,6 @@ export class GitSync { cy.get("@guid").then((uid) => { cy.wait(`@generateKey-${repoName}`).then((result: any) => { let generatedKey = result.response.body.data.publicKey; - generatedKey = generatedKey.slice(0, generatedKey.length - 1); // fetch the generated key and post to the github repo cy.request({ method: "POST", diff --git a/app/server/appsmith-git/pom.xml b/app/server/appsmith-git/pom.xml index 32caa1558ea..877477edc14 100644 --- a/app/server/appsmith-git/pom.xml +++ b/app/server/appsmith-git/pom.xml @@ -23,13 +23,6 @@ provided - - - org.eclipse.jgit - org.eclipse.jgit.ssh.jsch - 6.6.1.202309021850-r - - io.projectreactor reactor-test @@ -47,6 +40,12 @@ interfaces 1.0-SNAPSHOT + + + org.eclipse.jgit + org.eclipse.jgit.ssh.apache + 6.6.1.202309021850-r + org.springframework spring-test diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java index bb74d5ad6e2..25ab61c26dd 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java @@ -1,26 +1,51 @@ package com.appsmith.git.helpers; -import com.jcraft.jsch.JSch; -import com.jcraft.jsch.JSchException; -import com.jcraft.jsch.Session; +import com.appsmith.external.git.utils.CryptoUtil; +import lombok.extern.slf4j.Slf4j; +import org.bouncycastle.crypto.params.AsymmetricKeyParameter; +import org.bouncycastle.crypto.util.OpenSSHPublicKeyUtil; +import org.bouncycastle.jcajce.spec.OpenSSHPrivateKeySpec; +import org.bouncycastle.jcajce.spec.OpenSSHPublicKeySpec; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.bouncycastle.util.io.pem.PemReader; import org.eclipse.jgit.api.TransportConfigCallback; +import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.SshSessionFactory; import org.eclipse.jgit.transport.SshTransport; import org.eclipse.jgit.transport.Transport; -import org.eclipse.jgit.transport.ssh.jsch.JschConfigSessionFactory; -import org.eclipse.jgit.transport.ssh.jsch.OpenSshConfig; -import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.transport.sshd.ServerKeyDatabase; +import org.eclipse.jgit.transport.sshd.SshdSessionFactory; + +import java.io.File; +import java.io.IOException; +import java.io.StringReader; +import java.net.InetSocketAddress; +import java.security.KeyFactory; +import java.security.KeyPair; +import java.security.NoSuchAlgorithmException; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.spec.EncodedKeySpec; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.PKCS8EncodedKeySpec; +import java.util.Base64; +import java.util.List; + +import static com.appsmith.external.git.constants.SSHConstants.ECDSA_KEY_FACTORY_IDENTIFIER_BC; +import static com.appsmith.external.git.constants.SSHConstants.RSA_KEY_FACTORY_IDENTIFIER; +import static com.appsmith.external.git.constants.SSHConstants.RSA_TYPE; /** * A custom TransportConfigCallback class that loads private key and public key from the provided strings in constructor. * An instance of this class will be used as follows: - * + *

* TransportConfigCallback transportConfigCallback = new SshTransportConfigCallback(PVT_KEY_STRING, PUB_KEY_STRING); * Git.open(gitRepoDirFile) // gitRepoDirFile is an instance of File - * .push() - * .setTransportConfigCallback(transportConfigCallback) - * .call(); + * .push() + * .setTransportConfigCallback(transportConfigCallback) + * .call(); */ +@Slf4j public class SshTransportConfigCallback implements TransportConfigCallback { private String privateKey; private String publicKey; @@ -30,17 +55,72 @@ public SshTransportConfigCallback(String privateKey, String publicKey) { this.publicKey = publicKey; } - private final SshSessionFactory sshSessionFactory = new JschConfigSessionFactory() { + private final SshSessionFactory sshSessionFactory = new SshdSessionFactory() { + @Override - protected void configure(OpenSshConfig.Host hc, Session session) { - session.setConfig("StrictHostKeyChecking", "no"); + protected ServerKeyDatabase getServerKeyDatabase(File homeDir, File sshDir) { + return new ServerKeyDatabase() { + @Override + public List lookup( + String connectAddress, InetSocketAddress remoteAddress, Configuration config) { + return List.of(); + } + + @Override + public boolean accept( + String connectAddress, + InetSocketAddress remoteAddress, + PublicKey serverKey, + Configuration config, + CredentialsProvider provider) { + return true; + } + }; } @Override - protected JSch createDefaultJSch(FS fs) throws JSchException { - JSch jSch = super.createDefaultJSch(fs); - jSch.addIdentity("id_rsa", privateKey.getBytes(), publicKey.getBytes(), "".getBytes()); - return jSch; + protected Iterable getDefaultKeys(File sshDir) { + + try { + KeyPair keyPair; + KeyFactory keyFactory; + PublicKey generatedPublicKey; + + if (publicKey.startsWith(RSA_TYPE)) { + keyFactory = KeyFactory.getInstance(RSA_KEY_FACTORY_IDENTIFIER); + + generatedPublicKey = keyFactory.generatePublic(CryptoUtil.decodeOpenSSHRSA(publicKey.getBytes())); + + } else { + keyFactory = KeyFactory.getInstance(ECDSA_KEY_FACTORY_IDENTIFIER_BC, new BouncyCastleProvider()); + String[] fields = publicKey.split(" "); + AsymmetricKeyParameter keyParameter = OpenSSHPublicKeyUtil.parsePublicKey( + Base64.getDecoder().decode(fields[1].getBytes())); + OpenSSHPublicKeySpec keySpec = + new OpenSSHPublicKeySpec(OpenSSHPublicKeyUtil.encodePublicKey(keyParameter)); + + generatedPublicKey = keyFactory.generatePublic(keySpec); + } + + EncodedKeySpec privateKeySpec; + String[] splitKeys = privateKey.split("-----.*-----\n"); + if (splitKeys.length > 1) { + byte[] content = new PemReader(new StringReader(privateKey)) + .readPemObject() + .getContent(); + privateKeySpec = new OpenSSHPrivateKeySpec(content); + } else { + privateKeySpec = new PKCS8EncodedKeySpec(Base64.getDecoder().decode(privateKey)); + } + + PrivateKey generatedPrivateKey = keyFactory.generatePrivate(privateKeySpec); + + keyPair = new KeyPair(generatedPublicKey, generatedPrivateKey); + return List.of(keyPair); + } catch (NoSuchAlgorithmException | InvalidKeySpecException | IOException e) { + log.debug("Error while associating keys for signing: ", e); + throw new RuntimeException(e); + } } }; diff --git a/app/server/appsmith-interfaces/pom.xml b/app/server/appsmith-interfaces/pom.xml index bcd2728a8dd..b75fe868302 100644 --- a/app/server/appsmith-interfaces/pom.xml +++ b/app/server/appsmith-interfaces/pom.xml @@ -22,6 +22,12 @@ com.hierynomus sshj 0.35.0 + + + org.bouncycastle + bcprov-jdk15on + + diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java new file mode 100644 index 00000000000..14602b251cc --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java @@ -0,0 +1,12 @@ +package com.appsmith.external.git.constants; + +public class SSHConstants { + public static final String RSA_TYPE = "ssh-rsa"; + public static final String RSA_TYPE_PREFIX = RSA_TYPE + " "; + private static final String ECDSA_TYPE = "ecdsa-sha2-nistp256"; + public static final String ECDSA_TYPE_PREFIX = ECDSA_TYPE + " "; + + public static final String RSA_KEY_FACTORY_IDENTIFIER = "RSA"; + public static final String ECDSA_KEY_FACTORY_IDENTIFIER = "EC"; + public static final String ECDSA_KEY_FACTORY_IDENTIFIER_BC = "ECDSA"; +} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java new file mode 100644 index 00000000000..02e0f78c27a --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java @@ -0,0 +1,42 @@ +package com.appsmith.external.git.utils; + +import java.math.BigInteger; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.spec.RSAPublicKeySpec; +import java.util.Base64; + +import static com.appsmith.external.git.constants.SSHConstants.RSA_TYPE; + +public class CryptoUtil { + public static RSAPublicKeySpec decodeOpenSSHRSA(byte[] input) { + String[] fields = new String(input, StandardCharsets.US_ASCII).split(" "); + if ((fields.length < 2) || (!fields[0].equals(RSA_TYPE))) + throw new IllegalArgumentException("Unsupported type"); + byte[] std = Base64.getDecoder().decode(fields[1]); + return decodeRSAPublicSSH(std); + } + + static RSAPublicKeySpec decodeRSAPublicSSH(byte[] encoded) { + ByteBuffer input = ByteBuffer.wrap(encoded); + String type = asString(input); + if (!RSA_TYPE.equals(type)) throw new IllegalArgumentException("Unsupported type"); + BigInteger exp = sshInt(input); + BigInteger mod = sshInt(input); + return new RSAPublicKeySpec(mod, exp); + } + + private static String asString(ByteBuffer buf) { + return new String(lenVal(buf), StandardCharsets.US_ASCII); + } + + private static BigInteger sshInt(ByteBuffer buf) { + return new BigInteger(+1, lenVal(buf)); + } + + private static byte[] lenVal(ByteBuffer buf) { + byte[] copy = new byte[buf.getInt()]; + buf.get(copy); + return copy; + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java index d75697cdad6..a8f0bd844e2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java @@ -1,22 +1,32 @@ package com.appsmith.server.helpers; -import com.appsmith.git.helpers.StringOutputStream; import com.appsmith.server.constants.Assets; import com.appsmith.server.domains.GitAuth; import com.appsmith.server.dtos.GitDeployKeyDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; -import com.jcraft.jsch.JSch; -import com.jcraft.jsch.JSchException; -import com.jcraft.jsch.KeyPair; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; +import org.bouncycastle.crypto.params.AsymmetricKeyParameter; +import org.bouncycastle.crypto.util.OpenSSHPublicKeyUtil; +import org.bouncycastle.crypto.util.PublicKeyFactory; +import java.io.IOException; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; +import java.security.PublicKey; import java.time.Instant; import java.util.ArrayList; +import java.util.Base64; import java.util.List; -import static org.reflections.Reflections.log; +import static com.appsmith.external.git.constants.SSHConstants.ECDSA_KEY_FACTORY_IDENTIFIER; +import static com.appsmith.external.git.constants.SSHConstants.ECDSA_TYPE_PREFIX; +import static com.appsmith.external.git.constants.SSHConstants.RSA_KEY_FACTORY_IDENTIFIER; +import static com.appsmith.external.git.constants.SSHConstants.RSA_TYPE_PREFIX; +@Slf4j public class GitDeployKeyGenerator { public enum supportedProtocols { ECDSA(256, ""), @@ -41,44 +51,57 @@ public GitDeployKeyDTO getProtocolDetails() { } public static GitAuth generateSSHKey(String keyType) { - JSch jsch = new JSch(); - KeyPair kpair; - int key; + String alg; int keySize; - if (!StringUtils.isEmpty(keyType) && keyType.equals(supportedProtocols.RSA.name())) { - key = KeyPair.RSA; - keySize = supportedProtocols.RSA.key_size; - } else { - key = KeyPair.ECDSA; - keySize = supportedProtocols.ECDSA.key_size; - } + KeyPair keyPair; + String publicKey; try { - kpair = KeyPair.genKeyPair(jsch, key, keySize); - } catch (JSchException e) { - log.error("failed to generate ECDSA key pair", e); - throw new AppsmithException(AppsmithError.SSH_KEY_GENERATION_ERROR); + if (!StringUtils.isEmpty(keyType) && keyType.equals(supportedProtocols.RSA.name())) { + alg = RSA_KEY_FACTORY_IDENTIFIER; + keySize = supportedProtocols.RSA.key_size; + keyPair = getKeyPair(alg, keySize); + publicKey = writeJavaPublicKeyToSSH2(keyPair.getPublic(), RSA_TYPE_PREFIX); + } else { + alg = ECDSA_KEY_FACTORY_IDENTIFIER; + keySize = supportedProtocols.ECDSA.key_size; + keyPair = getKeyPair(alg, keySize); + publicKey = writeJavaPublicKeyToSSH2(keyPair.getPublic(), ECDSA_TYPE_PREFIX); + } + } catch (NoSuchAlgorithmException | IOException e) { + log.debug("Error while creating key pair", e); + throw new AppsmithException(AppsmithError.SSH_KEY_GENERATION_ERROR, e); } - StringOutputStream privateKeyOutput = new StringOutputStream(); - StringOutputStream publicKeyOutput = new StringOutputStream(); - - kpair.writePrivateKey(privateKeyOutput); - kpair.writePublicKey(publicKeyOutput, "appsmith"); - GitAuth gitAuth = new GitAuth(); - gitAuth.setPublicKey(publicKeyOutput.toString()); - gitAuth.setPrivateKey(privateKeyOutput.toString()); + gitAuth.setPublicKey(publicKey); + byte[] encodedPrivateKey = keyPair.getPrivate().getEncoded(); + gitAuth.setPrivateKey(Base64.getEncoder().encodeToString(encodedPrivateKey)); gitAuth.setGeneratedAt(Instant.now()); gitAuth.setDocUrl(Assets.GIT_DEPLOY_KEY_DOC_URL); return gitAuth; } + private static KeyPair getKeyPair(String alg, int keySize) throws NoSuchAlgorithmException { + KeyPair keyPair; + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(alg); + keyPairGenerator.initialize(keySize); + keyPair = keyPairGenerator.generateKeyPair(); + + return keyPair; + } + public static List getSupportedProtocols() { List protocolList = new ArrayList<>(); protocolList.add(supportedProtocols.ECDSA.getProtocolDetails()); protocolList.add(supportedProtocols.RSA.getProtocolDetails()); return protocolList; } + + private static String writeJavaPublicKeyToSSH2(final PublicKey publicKey, String prefix) throws IOException { + AsymmetricKeyParameter key = PublicKeyFactory.createKey(publicKey.getEncoded()); + final byte[] sshKey = OpenSSHPublicKeyUtil.encodePublicKey(key); + return prefix + Base64.getEncoder().encodeToString(sshKey) + " appsmith"; + } }