Skip to content

Commit

Permalink
Upgrade from SpongyCastle 1.58 to BouncyCastle 1.70
Browse files Browse the repository at this point in the history 
SpongyCastle was a fork of BouncyCastle needed before Android 3.0 because
of a conflict with Android's own version of BC. It's no longer needed and
rarely receives updates anymore [1]. Furthermore the version we were using
was from 2015 and had security issues (although I'm not sure we were
affected by them since we only use it to generate certificates).

With this change we now also use Java's standard library to read the certs
from a byte[] since the standard CertificateFactory can already do that.


[1] rtyley/spongycastle#34
  • Loading branch information
@albertvaka
albertvaka committed Mar 19, 2023
1 parent 51dfa2d commit 33e8e04
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 35 deletions.
2 changes: 1 addition & 1 deletion build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ dependencies {
implementation 'org.apache.mina:mina-core:2.0.19' //For some reason, makes sshd-core:0.14.0 work without NIO, which isn't available until Android 8 (api 26)

//implementation('com.github.bright:slf4android:0.1.6') { transitive = true } // For org.apache.sshd debugging
implementation 'com.madgag.spongycastle:bcpkix-jdk15on:1.58.0.0' //For SSL certificate generation
implementation 'org.bouncycastle:bcpkix-jdk15on:1.70' //For SSL certificate generation

implementation 'org.atteo.classindex:classindex:3.13'
annotationProcessor 'org.atteo.classindex:classindex:3.13'
Expand Down
63 changes: 32 additions & 31 deletions src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,20 @@
import android.util.Base64;
import android.util.Log;

import org.apache.commons.lang3.ArrayUtils;
import org.kde.kdeconnect.Helpers.DeviceHelper;
import org.kde.kdeconnect.Helpers.RandomHelper;
import org.spongycastle.asn1.x500.RDN;
import org.spongycastle.asn1.x500.X500Name;
import org.spongycastle.asn1.x500.X500NameBuilder;
import org.spongycastle.asn1.x500.style.BCStyle;
import org.spongycastle.asn1.x500.style.IETFUtils;
import org.spongycastle.cert.X509CertificateHolder;
import org.spongycastle.cert.X509v3CertificateBuilder;
import org.spongycastle.cert.jcajce.JcaX509CertificateConverter;
import org.spongycastle.cert.jcajce.JcaX509v3CertificateBuilder;
import org.spongycastle.jce.provider.BouncyCastleProvider;
import org.spongycastle.operator.ContentSigner;
import org.spongycastle.operator.jcajce.JcaContentSignerBuilder;
import org.spongycastle.util.Arrays;

import org.bouncycastle.asn1.x500.RDN;
import org.bouncycastle.asn1.x500.X500Name;
import org.bouncycastle.asn1.x500.X500NameBuilder;
import org.bouncycastle.asn1.x500.style.BCStyle;
import org.bouncycastle.asn1.x500.style.IETFUtils;
import org.bouncycastle.cert.X509v3CertificateBuilder;
import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder;
import org.bouncycastle.operator.ContentSigner;
import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
import org.bouncycastle.util.Arrays;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.math.BigInteger;
import java.net.Socket;
Expand All @@ -43,11 +40,11 @@
import java.security.cert.Certificate;
import java.security.cert.CertificateEncodingException;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.time.Instant;
import java.time.LocalDate;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Date;
import java.util.Formatter;
import java.util.Locale;
Expand All @@ -63,9 +60,15 @@

public class SslHelper {

public static X509Certificate certificate; //my device's certificate

public final static BouncyCastleProvider BC = new BouncyCastleProvider();
public static Certificate certificate; //my device's certificate
private static CertificateFactory factory;
static {
try {
factory = CertificateFactory.getInstance("X.509");
} catch (CertificateException e) {
throw new RuntimeException(e);
}
}

private final static TrustManager[] trustAllCerts = new TrustManager[]{new X509TrustManager() {
public java.security.cert.X509Certificate[] getAcceptedIssuers() {
Expand Down Expand Up @@ -103,8 +106,7 @@ public static void initialiseCertificate(Context context) {
try {
SharedPreferences globalSettings = PreferenceManager.getDefaultSharedPreferences(context);
byte[] certificateBytes = Base64.decode(globalSettings.getString("certificate", ""), 0);
X509CertificateHolder certificateHolder = new X509CertificateHolder(certificateBytes);
X509Certificate cert = new JcaX509CertificateConverter().setProvider(BC).getCertificate(certificateHolder);
X509Certificate cert = (X509Certificate) parseCertificate(certificateBytes);

String certDeviceId = getCommonNameFromCertificate(cert);
if (!certDeviceId.equals(deviceId)) {
Expand Down Expand Up @@ -145,11 +147,12 @@ public static void initialiseCertificate(Context context) {
nameBuilder.build(),
publicKey
);
ContentSigner contentSigner = new JcaContentSignerBuilder("SHA256WithRSAEncryption").setProvider(BC).build(privateKey);
certificate = new JcaX509CertificateConverter().setProvider(BC).getCertificate(certificateBuilder.build(contentSigner));
ContentSigner contentSigner = new JcaContentSignerBuilder("SHA256WithRSA").build(privateKey);
byte[] certificateBytes = certificateBuilder.build(contentSigner).getEncoded();
certificate = parseCertificate(certificateBytes);

SharedPreferences.Editor edit = settings.edit();
edit.putString("certificate", Base64.encodeToString(certificate.getEncoded(), 0));
edit.putString("certificate", Base64.encodeToString(certificateBytes, 0));
edit.apply();

setLocale(initialLocale, context);
Expand Down Expand Up @@ -180,12 +183,11 @@ private static SSLContext getSslContext(Context context, String deviceId, boolea
PrivateKey privateKey = RsaHelper.getPrivateKey(context);

// Get remote device certificate if trusted
X509Certificate remoteDeviceCertificate = null;
Certificate remoteDeviceCertificate = null;
if (isDeviceTrusted) {
SharedPreferences devicePreferences = context.getSharedPreferences(deviceId, Context.MODE_PRIVATE);
byte[] certificateBytes = Base64.decode(devicePreferences.getString("certificate", ""), 0);
X509CertificateHolder certificateHolder = new X509CertificateHolder(certificateBytes);
remoteDeviceCertificate = new JcaX509CertificateConverter().setProvider(BC).getCertificate(certificateHolder);
remoteDeviceCertificate = parseCertificate(certificateBytes);
}

// Setup keystore
Expand Down Expand Up @@ -257,9 +259,8 @@ public static String getCertificateHash(Certificate certificate) {
return formatter.toString();
}

public static Certificate parseCertificate(byte[] certificateBytes) throws IOException, CertificateException {
X509CertificateHolder certificateHolder = new X509CertificateHolder(certificateBytes);
return new JcaX509CertificateConverter().setProvider(BC).getCertificate(certificateHolder);
public static Certificate parseCertificate(byte[] certificateBytes) throws CertificateException {
return factory.generateCertificate(new ByteArrayInputStream(certificateBytes));
}

private static String getCommonNameFromCertificate(X509Certificate cert) {
Expand All @@ -269,7 +270,7 @@ private static String getCommonNameFromCertificate(X509Certificate cert) {
return IETFUtils.valueToString(rdn.getFirst().getValue());
}

public static String getVerificationKey(X509Certificate certificateA, Certificate certificateB) {
public static String getVerificationKey(Certificate certificateA, Certificate certificateB) {
try {
byte[] a = certificateA.getPublicKey().getEncoded();
byte[] b = certificateB.getPublicKey().getEncoded();
Expand Down
3 changes: 0 additions & 3 deletions src/org/kde/kdeconnect/Plugins/SftpPlugin/SimpleSftpServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.kde.kdeconnect.Device;
import org.kde.kdeconnect.Helpers.RandomHelper;
import org.kde.kdeconnect.Helpers.SecurityHelpers.RsaHelper;
import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper;

import java.io.IOException;
import java.net.Inet4Address;
Expand All @@ -35,7 +34,6 @@
import java.security.KeyPair;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.Security;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
Expand All @@ -54,7 +52,6 @@ class SimpleSftpServer {
private final SimplePublicKeyAuthenticator keyAuth = new SimplePublicKeyAuthenticator();

static {
Security.insertProviderAt(SslHelper.BC, 1);
SecurityUtils.setRegisterBouncyCastle(false);
}

Expand Down

0 comments on commit 33e8e04

Please sign in to comment.