From cb24e515b7983133133ca38bd3e3e6354daaab76 Mon Sep 17 00:00:00 2001 From: Ben Komalo Date: Thu, 16 Jun 2011 13:37:10 -0700 Subject: [PATCH] Add one more error state to certificate process When the KeyStore fails to give us back a certificate for any reason (it was removed from the keystore perhaps), propagate the error back up. Change-Id: I4f0ef783c1665589cc8ccb43d95da43a297a3e9a --- .../emailcommon/mail/MessagingException.java | 4 +++- .../emailcommon/service/EmailServiceStatus.java | 3 +++ .../utility/EmailClientConnectionManager.java | 13 ++++--------- .../android/emailcommon/utility/SSLUtils.java | 17 ++++++++++++----- res/values/strings.xml | 8 ++++++-- src/com/android/email/Controller.java | 3 +++ .../email/MessagingExceptionStrings.java | 2 ++ .../setup/AccountCheckSettingsFragment.java | 5 ++++- 8 files changed, 37 insertions(+), 18 deletions(-) diff --git a/emailcommon/src/com/android/emailcommon/mail/MessagingException.java b/emailcommon/src/com/android/emailcommon/mail/MessagingException.java index bca956b7d..4a8cebade 100644 --- a/emailcommon/src/com/android/emailcommon/mail/MessagingException.java +++ b/emailcommon/src/com/android/emailcommon/mail/MessagingException.java @@ -63,7 +63,9 @@ public class MessagingException extends Exception { /** The server refused access */ public static final int ATTACHMENT_NOT_FOUND = 15; /** A client SSL certificate is required for connections to the server */ - public static final int CLIENT_CERTIFICATE_ERROR = 16; + public static final int CLIENT_CERTIFICATE_REQUIRED = 16; + /** The client SSL certificate specified is invalid */ + public static final int CLIENT_CERTIFICATE_ERROR = 17; protected int mExceptionType; // Exception type-specific data diff --git a/emailcommon/src/com/android/emailcommon/service/EmailServiceStatus.java b/emailcommon/src/com/android/emailcommon/service/EmailServiceStatus.java index 23df3aa16..8cd577c8d 100644 --- a/emailcommon/src/com/android/emailcommon/service/EmailServiceStatus.java +++ b/emailcommon/src/com/android/emailcommon/service/EmailServiceStatus.java @@ -37,4 +37,7 @@ public interface EmailServiceStatus { // Maybe we should automatically retry these? public static final int CONNECTION_ERROR = 0x20; + + // Client certificates used to authenticate cannot be retrieved from the system. + public static final int CLIENT_CERTIFICATE_ERROR = 0x21; } diff --git a/emailcommon/src/com/android/emailcommon/utility/EmailClientConnectionManager.java b/emailcommon/src/com/android/emailcommon/utility/EmailClientConnectionManager.java index 763ffd72b..5b50e2b38 100644 --- a/emailcommon/src/com/android/emailcommon/utility/EmailClientConnectionManager.java +++ b/emailcommon/src/com/android/emailcommon/utility/EmailClientConnectionManager.java @@ -21,8 +21,6 @@ import com.android.emailcommon.Logging; import com.android.emailcommon.utility.SSLUtils.KeyChainKeyManager; import com.android.emailcommon.utility.SSLUtils.TrackingKeyManager; -import org.apache.http.conn.ClientConnectionRequest; -import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.scheme.PlainSocketFactory; import org.apache.http.conn.scheme.Scheme; import org.apache.http.conn.scheme.SchemeRegistry; @@ -33,6 +31,8 @@ import android.content.Context; import android.net.SSLCertificateSocketFactory; import android.util.Log; +import java.security.cert.CertificateException; + import javax.net.ssl.KeyManager; /** @@ -75,7 +75,8 @@ public class EmailClientConnectionManager extends ThreadSafeClientConnManager { * {@link #makeSchemeForClientCert(String, boolean)}. */ public synchronized void registerClientCert( - Context context, String clientCertAlias, boolean trustAllServerCerts) { + Context context, String clientCertAlias, boolean trustAllServerCerts) + throws CertificateException { SchemeRegistry registry = getSchemeRegistry(); String schemeName = makeSchemeForClientCert(clientCertAlias, trustAllServerCerts); Scheme existing = registry.get(schemeName); @@ -85,12 +86,6 @@ public class EmailClientConnectionManager extends ThreadSafeClientConnManager { + clientCertAlias + "]"); } KeyManager keyManager = KeyChainKeyManager.fromAlias(context, clientCertAlias); - if (keyManager == null) { - // TODO: handle failing to retrieve credentials from the keystore. - Log.e(Logging.LOG_TAG, "Unable to retrieve credentials for alias [" - + clientCertAlias + "]"); - return; - } SSLCertificateSocketFactory underlying = SSLUtils.getSSLSocketFactory( trustAllServerCerts); underlying.setKeyManagers(new KeyManager[] { keyManager }); diff --git a/emailcommon/src/com/android/emailcommon/utility/SSLUtils.java b/emailcommon/src/com/android/emailcommon/utility/SSLUtils.java index 3191959a6..78c0c9b02 100644 --- a/emailcommon/src/com/android/emailcommon/utility/SSLUtils.java +++ b/emailcommon/src/com/android/emailcommon/utility/SSLUtils.java @@ -25,6 +25,7 @@ import android.util.Log; import java.net.Socket; import java.security.Principal; import java.security.PrivateKey; +import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.Arrays; @@ -178,16 +179,17 @@ public class SSLUtils { * If for any reason retrieval of the credentials from the system {@link KeyChain} fails, * a {@code null} value will be returned. */ - public static KeyChainKeyManager fromAlias(Context context, String alias) { + public static KeyChainKeyManager fromAlias(Context context, String alias) + throws CertificateException { X509Certificate[] certificateChain; try { certificateChain = KeyChain.getCertificateChain(context, alias); } catch (KeyChainException e) { logError(alias, "certificate chain", e); - return null; + throw new CertificateException(e); } catch (InterruptedException e) { logError(alias, "certificate chain", e); - return null; + throw new CertificateException(e); } PrivateKey privateKey; @@ -195,10 +197,15 @@ public class SSLUtils { privateKey = KeyChain.getPrivateKey(context, alias); } catch (KeyChainException e) { logError(alias, "private key", e); - return null; + throw new CertificateException(e); } catch (InterruptedException e) { logError(alias, "private key", e); - return null; + throw new CertificateException(e); + } + + if (certificateChain == null || privateKey == null) { + throw new CertificateException( + "Can't access certificate from keystore for alias [" + alias + "]"); } return new KeyChainKeyManager(alias, certificateChain, privateKey); diff --git a/res/values/strings.xml b/res/values/strings.xml index a13a7c483..96f00d0de 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -770,8 +770,12 @@ save attachment. >Cannot safely connect to server.\n(%s) - User certificate - required to connect to server. + User certificate required to connect to server. + + Certificate invalid or inaccessible. diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 2d5fa331a..96174e0f9 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -1609,6 +1609,9 @@ public class Controller { case EmailServiceStatus.ATTACHMENT_NOT_FOUND: return new MessagingException(MessagingException.ATTACHMENT_NOT_FOUND); + case EmailServiceStatus.CLIENT_CERTIFICATE_ERROR: + return new MessagingException(MessagingException.CLIENT_CERTIFICATE_ERROR); + case EmailServiceStatus.MESSAGE_NOT_FOUND: case EmailServiceStatus.FOLDER_NOT_DELETED: case EmailServiceStatus.FOLDER_NOT_RENAMED: diff --git a/src/com/android/email/MessagingExceptionStrings.java b/src/com/android/email/MessagingExceptionStrings.java index ecbf8daee..cb2c6070c 100644 --- a/src/com/android/email/MessagingExceptionStrings.java +++ b/src/com/android/email/MessagingExceptionStrings.java @@ -49,6 +49,8 @@ public class MessagingExceptionStrings { return R.string.account_setup_failed_security; case MessagingException.ACCESS_DENIED: return R.string.account_setup_failed_access_denied; + case MessagingException.CLIENT_CERTIFICATE_ERROR: + return R.string.account_setup_failed_certificate_inaccessible; } return R.string.status_network_error; // default } diff --git a/src/com/android/email/activity/setup/AccountCheckSettingsFragment.java b/src/com/android/email/activity/setup/AccountCheckSettingsFragment.java index d1edb6c04..9a12d4c33 100644 --- a/src/com/android/email/activity/setup/AccountCheckSettingsFragment.java +++ b/src/com/android/email/activity/setup/AccountCheckSettingsFragment.java @@ -645,9 +645,12 @@ public class AccountCheckSettingsFragment extends Fragment { case MessagingException.GENERAL_SECURITY: id = R.string.account_setup_failed_security; break; - case MessagingException.CLIENT_CERTIFICATE_ERROR: + case MessagingException.CLIENT_CERTIFICATE_REQUIRED: id = R.string.account_setup_failed_certificate_required; break; + case MessagingException.CLIENT_CERTIFICATE_ERROR: + id = R.string.account_setup_failed_certificate_inaccessible; + break; default: id = TextUtils.isEmpty(message) ? R.string.account_setup_failed_dlg_server_message