Skip to content

Commit

Permalink
Merge pull request #33210 from appsmithorg/cp/20240506
Browse files Browse the repository at this point in the history 
fix: Start using SHA2 instead of SHA1 algorithms for signing during git operations (#33166)
  • Loading branch information
@nidhi-nair
nidhi-nair committed May 6, 2024
2 parents 4785670 + 86e1f89 commit 235f563
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 52 deletions.
1 change: 0 additions & 1 deletion app/client/cypress/support/Pages/GitSync.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 6 additions & 7 deletions app/server/appsmith-git/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@
<scope>provided</scope>
</dependency>

<!-- https://mvnrepository.com/artifact/org.eclipse.jgit/org.eclipse.jgit.junit.ssh -->
<dependency>
<groupId>org.eclipse.jgit</groupId>
<artifactId>org.eclipse.jgit.ssh.jsch</artifactId>
<version>6.6.1.202309021850-r</version>
</dependency>

<dependency>
<groupId>io.projectreactor</groupId>
<artifactId>reactor-test</artifactId>
Expand All @@ -47,6 +40,12 @@
<artifactId>interfaces</artifactId>
<version>1.0-SNAPSHOT</version>
</dependency>
<dependency>
<!-- SSH support for JGit based on Apache MINA sshd -->
<groupId>org.eclipse.jgit</groupId>
<artifactId>org.eclipse.jgit.ssh.apache</artifactId>
<version>6.6.1.202309021850-r</version>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-test</artifactId>
Expand Down
114 changes: 97 additions & 17 deletions ...erver/appsmith-git/src/main/java/com/appsmith/git/helpers/SshTransportConfigCallback.java
View file
Original file line number Diff line number Diff line change
@@ -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:
*
* <p>
* 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;
Expand All @@ -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<PublicKey> 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<KeyPair> 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);
}
}
};

Expand Down
6 changes: 6 additions & 0 deletions app/server/appsmith-interfaces/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
<groupId>com.hierynomus</groupId>
<artifactId>sshj</artifactId>
<version>0.35.0</version>
<exclusions>
<exclusion>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
</exclusion>
</exclusions>
</dependency>

<!-- Actual Junit5 implementation. Will transitively include junit-jupiter-api -->
Expand Down
12 changes: 12 additions & 0 deletions ...r/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/SSHConstants.java
View file
Original file line number Diff line number Diff line change
@@ -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";
}
42 changes: 42 additions & 0 deletions app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/utils/CryptoUtil.java
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
77 changes: 50 additions & 27 deletions ...rver/appsmith-server/src/main/java/com/appsmith/server/helpers/GitDeployKeyGenerator.java
View file
Original file line number Diff line number Diff line change
@@ -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, ""),
Expand All @@ -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<GitDeployKeyDTO> getSupportedProtocols() {
List<GitDeployKeyDTO> 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";
}
}

0 comments on commit 235f563

Please sign in to comment.