Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add client certificate support (https://github.com/gotify/android/issues/229) #230

Closed
wants to merge 5 commits into from
Closed

Add client certificate support (https://github.com/gotify/android/issues/229) #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
adb9955
Initial client certificates
polizz Apr 25, 2022
d39b099
Shorten client cert path name
polizz Apr 25, 2022
3381453
Place guards on Android O code
polizz Apr 29, 2022
ecaa2a0
Cleanup imports
polizz Apr 30, 2022
c9ebb10
Make client cert work with self-signed server cert
polizz May 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/src/main/java/com/github/gotify/SSLSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
public class SSLSettings {
public boolean validateSSL;
public String cert;
public String clientCert;
public String clientCertPassword;

public SSLSettings(boolean validateSSL, String cert) {
public SSLSettings(boolean validateSSL, String cert, String clientCert, String clientCertPassword) {
this.validateSSL = validateSSL;
this.cert = cert;
this.clientCert = clientCert;
this.clientCertPassword = clientCertPassword;
}
}
29 changes: 28 additions & 1 deletion app/src/main/java/com/github/gotify/Settings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,30 @@ public Settings(Context context) {
sharedPreferences = context.getSharedPreferences("gotify", Context.MODE_PRIVATE);
}

public void clientCertUri(String clientCertUri) {
sharedPreferences.edit().putString("clientCertUri", clientCertUri).apply();
}

public String clientCertUri() {
return sharedPreferences.getString("clientCertUri", null);
}

public void clientCert(String clientCert) {
sharedPreferences.edit().putString("clientCert", clientCert).apply();
}

public String clientCert() {
return sharedPreferences.getString("clientCert", null);
}

public void clientCertPass(String clientCertPass) {
sharedPreferences.edit().putString("clientCertPass", clientCertPass).apply();
}

public String clientCertPass() {
return sharedPreferences.getString("clientCertPass", "");
}

public void url(String url) {
sharedPreferences.edit().putString("url", url).apply();
}
Expand All @@ -36,6 +60,9 @@ public void clear() {
token(null);
validateSSL(true);
cert(null);
clientCert(null);
clientCertUri(null);
clientCertPass("");
}

public void user(String name, boolean admin) {
Expand Down Expand Up @@ -77,6 +104,6 @@ public void cert(String cert) {
}

public SSLSettings sslSettings() {
return new SSLSettings(validateSSL(), cert());
return new SSLSettings(validateSSL(), cert(), clientCert(), clientCertPass());
}
}
18 changes: 18 additions & 0 deletions app/src/main/java/com/github/gotify/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
import android.graphics.Bitmap;
import android.graphics.drawable.BitmapDrawable;
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.text.format.DateUtils;
import java.util.Base64;
import android.view.View;
import androidx.annotation.NonNull;
import androidx.annotation.RequiresApi;
import com.github.gotify.client.JSON;
import com.github.gotify.log.Log;
import com.google.android.material.snackbar.Snackbar;
Expand Down Expand Up @@ -77,6 +80,21 @@ public void onPrepareLoad(Drawable placeHolderDrawable) {}
};
}

@RequiresApi(api = Build.VERSION_CODES.O)
public static String binaryFileToBase64(@NonNull InputStream inputStream) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method needs a @RequiresApi annotation, see build error.

The usage of this method then must be wrapped into an if inside com.github.gotify.login.LoginActivity#onActivityResult.

byte[] bytes;

try {
bytes = new byte[inputStream.available()];
//noinspection ResultOfMethodCallIgnored
inputStream.read(bytes);
} catch (IOException e) {
throw new IllegalArgumentException("failed to read input");
}

return Base64.getEncoder().encodeToString(bytes);
}

public static String readFileFromStream(@NonNull InputStream inputStream) {
StringBuilder sb = new StringBuilder();
String currentLine;
Expand Down
55 changes: 53 additions & 2 deletions app/src/main/java/com/github/gotify/api/CertUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,28 @@
package com.github.gotify.api;

import android.annotation.SuppressLint;
import android.os.Build;
import androidx.annotation.RequiresApi;
import com.github.gotify.SSLSettings;
import com.github.gotify.Utils;
import com.github.gotify.log.Log;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.UnrecoverableKeyException;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.util.Base64;
import java.util.Collection;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
Expand Down Expand Up @@ -60,21 +70,62 @@ public static void applySslSettings(OkHttpClient.Builder builder, SSLSettings se
}

if (settings.cert != null) {
KeyManager[] keyManagers = new KeyManager[] {};
TrustManager[] trustManagers = certToTrustManager(settings.cert);

if (trustManagers != null && trustManagers.length > 0) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
if (settings.clientCert != null) {
keyManagers = getClientCerts(settings.clientCert, settings.clientCertPassword.toCharArray());
}
}

SSLContext context = SSLContext.getInstance("TLS");
context.init(new KeyManager[] {}, trustManagers, new SecureRandom());
context.init(keyManagers, trustManagers, new SecureRandom());
builder.sslSocketFactory(
context.getSocketFactory(), (X509TrustManager) trustManagers[0]);
}
} else {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
if (settings.clientCert != null) {
KeyManager[] keyManagers = getClientCerts(settings.clientCert, settings.clientCertPassword.toCharArray());

TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
TrustManagerFactory.getDefaultAlgorithm());
trustManagerFactory.init((KeyStore) null);
TrustManager[] trustManagers = trustManagerFactory.getTrustManagers();
if (trustManagers.length != 1 || !(trustManagers[0] instanceof X509TrustManager)) {
throw new IllegalStateException("Unexpected default trust managers:");
}
X509TrustManager trustManager = (X509TrustManager) trustManagers[0];
Comment on lines +93 to +100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check needed, couldn't we pass all the trust managers into context.init(keyManagers, trustManagers, null);?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that was in some of the code I used as reference. I will check those calls and if there's always a default trust manager, we could remove it.


SSLContext context = SSLContext.getInstance("TLS");
context.init(keyManagers, new TrustManager[] { trustManager }, null);
builder.sslSocketFactory(
context.getSocketFactory(), trustManager);
}
}
}
} catch (Exception e) {
}
catch (Exception e) {
// We shouldn't have issues since the cert is verified on login.
Log.e("Failed to apply SSL settings", e);
}
}

@RequiresApi(api = Build.VERSION_CODES.O)
private static KeyManager[] getClientCerts(String clientCert, char[] password)
throws CertificateException, IOException, NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException {
KeyStore ks = KeyStore.getInstance("PKCS12");
InputStream bs = new ByteArrayInputStream(Base64.getDecoder().decode(clientCert));
ks.load(bs, password);

KeyManagerFactory kmf = KeyManagerFactory.getInstance("X509");
kmf.init(ks, password);

return kmf.getKeyManagers();
}

private static TrustManager[] certToTrustManager(String cert) throws GeneralSecurityException {
CertificateFactory certificateFactory = CertificateFactory.getInstance("X.509");
Collection<? extends Certificate> certificates =
Expand Down
66 changes: 60 additions & 6 deletions app/src/main/java/com/github/gotify/login/AdvancedDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@

import android.app.AlertDialog;
import android.content.Context;
import android.os.Build;
import android.view.LayoutInflater;
import android.view.View;
import android.widget.Button;
import android.widget.CheckBox;
import android.widget.CompoundButton;
import android.widget.EditText;
import android.widget.TextView;
import androidx.annotation.Nullable;
import butterknife.BindView;
import butterknife.ButterKnife;
import com.github.gotify.R;
import com.github.gotify.Settings;

class AdvancedDialog {

Expand All @@ -20,6 +23,8 @@ class AdvancedDialog {
private CompoundButton.OnCheckedChangeListener onCheckedChangeListener;
private Runnable onClickSelectCaCertificate;
private Runnable onClickRemoveCaCertificate;
private Runnable onClickSelectClientCertificate;
private Runnable onClickRemoveClientCertificate;

AdvancedDialog(Context context) {
this.context = context;
Expand All @@ -41,32 +46,56 @@ AdvancedDialog onClickRemoveCaCertificate(Runnable onClickRemoveCaCertificate) {
return this;
}

AdvancedDialog show(boolean disableSSL, @Nullable String selectedCertificate) {
AdvancedDialog onClickSelectClientCertificate(Runnable onClickSelectClientCertificate) {
this.onClickSelectClientCertificate = onClickSelectClientCertificate;
return this;
}

AdvancedDialog onClickRemoveClientCertificate(Runnable onClickRemoveClientCertificate) {
this.onClickRemoveClientCertificate = onClickRemoveClientCertificate;
return this;
}

AdvancedDialog show(boolean disableSSL, @Nullable String selectedCaCertificate, @Nullable String selectedClientCertificate, String password, Settings settings) {
View dialogView =
LayoutInflater.from(context).inflate(R.layout.advanced_settings_dialog, null);
holder = new ViewHolder(dialogView);
holder.disableSSL.setChecked(disableSSL);
holder.disableSSL.setOnCheckedChangeListener(onCheckedChangeListener);
holder.editClientCertPass.setText(password);

if (selectedCertificate == null) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
holder.editClientCertPass.setVisibility(View.GONE);
holder.selectedClientCertificate.setVisibility(View.GONE);
holder.toggleClientCert.setVisibility(View.GONE);
}

if (selectedCaCertificate == null) {
showSelectCACertificate();
} else {
showRemoveCACertificate(selectedCertificate);
showRemoveCACertificate(selectedCaCertificate);
}

if (selectedClientCertificate == null) {
showSelectClientCertificate();
} else {
showRemoveClientCertificate(selectedClientCertificate);
}

new AlertDialog.Builder(context)
.setView(dialogView)
.setTitle(R.string.advanced_settings)
.setPositiveButton(context.getString(R.string.done), (ignored, ignored2) -> {})
.setPositiveButton(context.getString(R.string.done), (ignored, ignored2) -> {
settings.clientCertPass(holder.editClientCertPass.getText().toString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Settings should only be set inside com.github.gotify.login.LoginActivity#onCreatedClient

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my first android code, so help me here. The other buttons on the advanced screen start file selection activities that end up being handled by an onActivityResult in LoginActivity.java. I was not sure how to get the value of the client cert password except through a lambda that is run after the done button is clicked. Do you have another way? I do see settings being modified in other functions besides onCreatedClient.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm running out of things to try in creating the self-signed certificate. I've tried a couple of different things and the LOG on the login page says Hostname .... not verified. I've tried selecting my CA root and the server cert and I can see that it is trying to validate the CA cert. This is all on the master branch by the way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use caddy for testing the self-signed certificate thingy in gotify (it works for me on master and your branch)

Caddyfile:

{
  http_port 8000
  https_port 8443
}
192.168.178.2 {
  tls internal
  reverse_proxy localhost:8080
}

gotify/server listens on localhost:8080, my desktop PC is 192.168.178.2. When starting caddy run -config Caddyfile, then Caddy will create certificates at ~/.local/share/caddy/certificates/local/192.168.178.2 and the crt is located at 192.168.178.2.crt.

In gotify I'll use https://192.168.178.2:8443 as server url. This error should be logged, when the ca .crt file isn't configured inside gotify, but the connection works after the cert is configured.

Exception 
2022-05-16T07:26:07.679Z ERROR: Error while api call
Code(0) Response: 
	at com.github.gotify.api.Callback$RetrofitCallback.onFailure(Callback.java:80)
	at retrofit2.ExecutorCallAdapterFactory$ExecutorCallbackCall$1$2.run(ExecutorCallAdapterFactory.java:80)
	at android.os.Handler.handleCallback(Handler.java:883)
	at android.os.Handler.dispatchMessage(Handler.java:100)
	at android.os.Looper.loop(Looper.java:214)
	at android.app.ActivityThread.main(ActivityThread.java:7356)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)
Caused by: javax.net.ssl.SSLHandshakeException: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found.
	at com.android.org.conscrypt.ConscryptFileDescriptorSocket.startHandshake(ConscryptFileDescriptorSocket.java:231)
	at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.java:319)
	at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.java:283)
	at okhttp3.internal.connection.RealConnection.connect(RealConnection.java:168)
	at okhttp3.internal.connection.StreamAllocation.findConnection(StreamAllocation.java:257)
	at okhttp3.internal.connection.StreamAllocation.findHealthyConnection(StreamAllocation.java:135)
	at okhttp3.internal.connection.StreamAllocation.newStream(StreamAllocation.java:114)
	at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:42)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
	at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
	at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
	at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:126)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
	at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:254)
	at okhttp3.RealCall$AsyncCall.execute(RealCall.java:200)
	at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at java.lang.Thread.run(Thread.java:919)
Caused by: java.security.cert.CertificateException: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found.
	at com.android.org.conscrypt.TrustManagerImpl.checkTrustedRecursive(TrustManagerImpl.java:658)
	at com.android.org.conscrypt.TrustManagerImpl.checkTrustedRecursive(TrustManagerImpl.java:617)
	at com.android.org.conscrypt.TrustManagerImpl.checkTrusted(TrustManagerImpl.java:507)
	at com.android.org.conscrypt.TrustManagerImpl.checkTrusted(TrustManagerImpl.java:426)
	at com.android.org.conscrypt.TrustManagerImpl.getTrustedChainForServer(TrustManagerImpl.java:354)
	at android.security.net.config.NetworkSecurityTrustManager.checkServerTrusted(NetworkSecurityTrustManager.java:94)
	at android.security.net.config.RootTrustManager.checkServerTrusted(RootTrustManager.java:89)
	at com.android.org.conscrypt.Platform.checkServerTrusted(Platform.java:224)
	at com.android.org.conscrypt.ConscryptFileDescriptorSocket.verifyCertificateChain(ConscryptFileDescriptorSocket.java:407)
	at com.android.org.conscrypt.NativeCrypto.SSL_do_handshake(Native Method)
	at com.android.org.conscrypt.NativeSsl.doHandshake(NativeSsl.java:387)
	at com.android.org.conscrypt.ConscryptFileDescriptorSocket.startHandshake(ConscryptFileDescriptorSocket.java:226)
	... 23 more
Caused by: java.security.cert.CertPathValidatorException: Trust anchor for certification path not found.
	... 35 more

I still cannot get the client certificate working with my setup, I get a 421 misdirected redirect from caddy. But I don't really know what I'm doing there, never used client certificates.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try Caddy's internal CA next. I was creating a local CA and a server cert and signing it manually and configuring Caddy to use that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok this is what worked for me:

I was getting all my cert files confused and was trying to use the wrong file for the client cert. Also, by using the Caddy auto-cert I am getting around some apparent issue with how I was trying to manually create the CA and server cert (a problem for another day). In my case, the client cert and client CA are both separately generated and totally unrelated to the server cert (Caddy autogen cert or otherwise).

Commands to create the client certificate:

openssl req -x509 -newkey rsa:2048 -keyout gotify_client_cert.key -out gotify_client_cert.crt -days 3000
openssl req -new -key gotify_client_cert.key -out gotify_client_cert.csr
openssl x509 -req -days 3000 -in gotify_client_cert.csr -signkey gotify_client_cert.key -out gotify_client_cert-CA.crt
cat gotify_client_cert.crt gotify_client_cert.key > gotify_client_cert.pem
openssl pkcs12 -export -out gotify_client_cert.p12 -inkey gotify_client_cert.key -in gotify_client_cert.pem

Caddyfile:

(require_client_cert) {
    tls {
        client_auth {
            mode require_and_verify
            trusted_ca_cert_file /client_certs/gotify_client_cert-CA.crt
            trusted_leaf_cert_file /client_certs/gotify_client_cert.crt
        }
    }
}

* *.home.lan {
    import require_client_cert
    tls internal
    reverse_proxy http://gotify:80
}

Docker compose file:

version: '3.3'
services:
  caddy:
    image: caddy:2.4.6-alpine
    ports:
      - "3080:80"
      - "3443:443"
    volumes:
      - ./Caddyfile:/etc/caddy/Caddyfile
      - ./.ssh:/client_certs
  gotify:
    image: gotify/server:2.1.4
volumes:
  caddy_data1:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look at this on the weekend.

})
.show();
return this;
}

private void showSelectCACertificate() {
holder.toggleCaCert.setText(R.string.select_ca_certificate);
holder.toggleCaCert.setOnClickListener((a) -> onClickSelectCaCertificate.run());
holder.selectedCaCertificate.setText(R.string.no_certificate_selected);
holder.selectedCaCertificate.setText(R.string.no_ca_certificate_selected);
}

void showRemoveCACertificate(String certificate) {
Expand All @@ -79,16 +108,41 @@ void showRemoveCACertificate(String certificate) {
holder.selectedCaCertificate.setText(certificate);
}

private void showSelectClientCertificate() {
holder.toggleClientCert.setText(R.string.select_client_certificate);
holder.toggleClientCert.setOnClickListener((a) -> onClickSelectClientCertificate.run());
holder.selectedClientCertificate.setText(R.string.no_client_certificate_selected);
}

void showRemoveClientCertificate(String certificate) {
holder.toggleClientCert.setText(R.string.remove_client_certificate);
holder.toggleClientCert.setOnClickListener(
(a) -> {
showSelectClientCertificate();
onClickRemoveClientCertificate.run();
});
holder.selectedClientCertificate.setText(certificate);
}

class ViewHolder {
@BindView(R.id.disableSSL)
CheckBox disableSSL;

@BindView(R.id.toggle_ca_cert)
Button toggleCaCert;

@BindView(R.id.seleceted_ca_cert)
@BindView(R.id.selected_ca_cert)
TextView selectedCaCertificate;

@BindView(R.id.toggle_client_cert)
Button toggleClientCert;

@BindView(R.id.selected_client_cert)
TextView selectedClientCertificate;

@BindView(R.id.edit_client_cert_pass)
EditText editClientCertPass;

ViewHolder(View view) {
ButterKnife.bind(this, view);
}
Expand Down
Loading