-
-
Notifications
You must be signed in to change notification settings - Fork 155
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.
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
Add client certificate support (https://github.com/gotify/android/issues/229) #230
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,24 @@ | ||
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.SecureRandom; | ||
import java.security.cert.Certificate; | ||
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; | ||
|
@@ -46,6 +52,7 @@ public static Certificate parseCertificate(String cert) { | |
} | ||
} | ||
|
||
@RequiresApi(api = Build.VERSION_CODES.O) | ||
public static void applySslSettings(OkHttpClient.Builder builder, SSLSettings settings) { | ||
// Modified from ApiClient.applySslSettings in the client package. | ||
|
||
|
@@ -69,7 +76,41 @@ public static void applySslSettings(OkHttpClient.Builder builder, SSLSettings se | |
context.getSocketFactory(), (X509TrustManager) trustManagers[0]); | ||
} | ||
} | ||
} catch (Exception e) { | ||
|
||
if (settings.clientCert != null) { | ||
KeyStore ks = KeyStore.getInstance("PKCS12"); | ||
InputStream bs = new ByteArrayInputStream(Base64.getDecoder().decode(settings.clientCert)); | ||
ks.load(bs, settings.clientCertPassword.toCharArray()); | ||
|
||
KeyManagerFactory kmf = KeyManagerFactory.getInstance("X509"); | ||
kmf.init(ks, 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]; | ||
|
||
|
||
SSLContext context = SSLContext.getInstance("TLS"); | ||
context.init(kmf.getKeyManagers(), new TrustManager[] { trustManager }, null); | ||
builder.sslSocketFactory( | ||
context.getSocketFactory(), trustManager); | ||
} | ||
} catch (IOException iex) { | ||
String tx = iex.toString(); | ||
if (iex.toString().contains("wrong password")) { | ||
Log.e("Incorrect client certificate password.", iex); | ||
return; | ||
} | ||
|
||
Log.e("Error opening client certificate.", iex); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This catch can be omitted, because the exception message is already sufficient and says that the password is wrong / the certificate is invalid. |
||
catch (Exception e) { | ||
// We shouldn't have issues since the cert is verified on login. | ||
Log.e("Failed to apply SSL settings", e); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,11 +7,13 @@ | |
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 { | ||
|
||
|
@@ -20,6 +22,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; | ||
|
@@ -41,32 +45,50 @@ 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 (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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Settings should only be set inside There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
Caddyfile:
Docker compose file:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -79,16 +101,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); | ||
} | ||
|
There was a problem hiding this comment.
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
.