From: David A. Velasco Date: Thu, 13 Sep 2012 15:07:17 +0000 (+0200) Subject: Server certificates improvements: fixed problems in devices with OpenSSL implementati... X-Git-Tag: oc-android-1.4.3~183 X-Git-Url: http://git.linex4red.de/pub/Android/ownCloud.git/commitdiff_plain/bf3cf8cdffa1f9debaaaef1da13c9b8c4b9c1bb0?ds=sidebyside;hp=-c Server certificates improvements: fixed problems in devices with OpenSSL implementations 'forgetting' CertificateExceptions ; and removed useless workaround for Android 2.x --- bf3cf8cdffa1f9debaaaef1da13c9b8c4b9c1bb0 diff --git a/src/com/owncloud/android/authenticator/ConnectionCheckOperation.java b/src/com/owncloud/android/authenticator/ConnectionCheckOperation.java index 5410dec4..61492367 100644 --- a/src/com/owncloud/android/authenticator/ConnectionCheckOperation.java +++ b/src/com/owncloud/android/authenticator/ConnectionCheckOperation.java @@ -83,16 +83,24 @@ public class ConnectionCheckOperation extends RemoteOperation { } catch (JSONException e) { mLatestResult = new RemoteOperationResult(RemoteOperationResult.ResultCode.INSTANCE_NOT_CONFIGURED); - //Log.e(TAG, "JSON exception while trying connection (instance not configured) ", e); } catch (Exception e) { mLatestResult = new RemoteOperationResult(e); - //Log.e(TAG, "Unexpected exception while trying connection", e); } finally { if (get != null) get.releaseConnection(); } + + if (mLatestResult.isSuccess()) { + Log.i(TAG, "Connection check at " + urlSt + ": " + mLatestResult.getLogMessage()); + + } else if (mLatestResult.getException() != null) { + Log.e(TAG, "Connection check at " + urlSt + ": " + mLatestResult.getLogMessage(), mLatestResult.getException()); + + } else { + Log.e(TAG, "Connection check at " + urlSt + ": " + mLatestResult.getLogMessage()); + } return retval; } diff --git a/src/com/owncloud/android/network/AdvancedSslSocketFactory.java b/src/com/owncloud/android/network/AdvancedSslSocketFactory.java index 0df7f6a4..c6f1b168 100644 --- a/src/com/owncloud/android/network/AdvancedSslSocketFactory.java +++ b/src/com/owncloud/android/network/AdvancedSslSocketFactory.java @@ -24,18 +24,17 @@ import java.net.InetSocketAddress; import java.net.Socket; import java.net.SocketAddress; import java.net.UnknownHostException; -import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.Enumeration; import javax.net.SocketFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; +import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSessionContext; import javax.net.ssl.SSLSocket; -import javax.net.ssl.X509TrustManager; import org.apache.commons.httpclient.ConnectTimeoutException; import org.apache.commons.httpclient.params.HttpConnectionParams; @@ -163,16 +162,32 @@ public class AdvancedSslSocketFactory implements ProtocolSocketFactory { */ private void verifyPeerIdentity(String host, int port, Socket socket) throws IOException { try { - IOException failInHandshake = null; + CertificateCombinedException failInHandshake = null; /// 1. VERIFY THE SERVER CERTIFICATE through the registered TrustManager (that should be an instance of AdvancedX509TrustManager) try { SSLSocket sock = (SSLSocket) socket; // a new SSLSession instance is created as a "side effect" sock.startHandshake(); - } catch (IOException e) { - failInHandshake = e; - if (!(e.getCause() instanceof CertificateCombinedException)) { + + } catch (RuntimeException e) { + + if (e instanceof CertificateCombinedException) { + failInHandshake = (CertificateCombinedException) e; + } else { + Throwable cause = e.getCause(); + Throwable previousCause = null; + while (cause != null && cause != previousCause && !(cause instanceof CertificateCombinedException)) { + previousCause = cause; + cause = cause.getCause(); + } + if (cause != null && cause instanceof CertificateCombinedException) { + failInHandshake = (CertificateCombinedException)cause; + } + } + if (failInHandshake == null) { throw e; } + failInHandshake.setHostInUrl(host); + } /// 2. VERIFY HOSTNAME @@ -181,7 +196,7 @@ public class AdvancedSslSocketFactory implements ProtocolSocketFactory { if (mHostnameVerifier != null) { if (failInHandshake != null) { /// 2.1 : a new SSLSession instance was NOT created in the handshake - X509Certificate serverCert = ((CertificateCombinedException)failInHandshake.getCause()).getServerCertificate(); + X509Certificate serverCert = failInHandshake.getServerCertificate(); try { mHostnameVerifier.verify(host, serverCert); } catch (SSLException e) { @@ -190,57 +205,28 @@ public class AdvancedSslSocketFactory implements ProtocolSocketFactory { } else { /// 2.2 : a new SSLSession instance was created in the handshake - if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.HONEYCOMB) { - /// this is sure ONLY for Android >= 3.0 ; the same is true for mHostnameVerifier.verify(host, (SSLSocket)socket) - newSession = ((SSLSocket)socket).getSession(); - if (!mTrustManager.isKnownServer((X509Certificate)(newSession.getPeerCertificates()[0]))) - verifiedHostname = mHostnameVerifier.verify(host, newSession); - - } else { - //// performing the previous verification in Android versions under 2.3.x (and we don't know the exact value of x) WILL BREAK THE SSL CONTEXT, and any HTTP operation executed later through the socket WILL FAIL ; - //// it is related with A BUG IN THE OpenSSLSOcketImpl.java IN THE ANDROID CORE SYSTEM; it was fixed here: - //// http://gitorious.org/ginger/libcore/blobs/df349b3eaf4d1fa0643ab722173bc3bf20a266f5/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java - /// but we could not find out in what Android version was released the bug fix; - /// - /// besides, due to the bug, calling ((SSLSocket)socket).getSession() IS NOT SAFE ; the next workaround is an UGLY BUT SAFE solution to get it - SSLSessionContext sessionContext = mSslContext.getClientSessionContext(); - if (sessionContext != null) { - SSLSession session = null; - synchronized(sessionContext) { // a SSLSession in the SSLSessionContext can be closed while we are searching for the new one; it happens; really - Enumeration ids = sessionContext.getIds(); - while (ids.hasMoreElements()) { - session = sessionContext.getSession(ids.nextElement()); - if ( session.getPeerHost().equals(host) && - session.getPeerPort() == port && - (newSession == null || newSession.getCreationTime() < session.getCreationTime())) { - newSession = session; - } - } - } - if (newSession != null) { - if (!mTrustManager.isKnownServer((X509Certificate)(newSession.getPeerCertificates()[0]))) { - verifiedHostname = mHostnameVerifier.verify(host, newSession); - } - } else { - Log.d(TAG, "Hostname verification could not be performed because the new SSLSession was not found"); - } - } + newSession = ((SSLSocket)socket).getSession(); + if (!mTrustManager.isKnownServer((X509Certificate)(newSession.getPeerCertificates()[0]))) { + verifiedHostname = mHostnameVerifier.verify(host, newSession); } } } /// 3. Combine the exceptions to throw, if any - if (failInHandshake != null) { - if (!verifiedHostname) { - ((CertificateCombinedException)failInHandshake.getCause()).setSslPeerUnverifiedException(new SSLPeerUnverifiedException(host)); + if (!verifiedHostname) { + SSLPeerUnverifiedException pue = new SSLPeerUnverifiedException("Names in the server certificate do not match to " + host + " in the URL"); + if (failInHandshake == null) { + failInHandshake = new CertificateCombinedException((X509Certificate) newSession.getPeerCertificates()[0]); + failInHandshake.setHostInUrl(host); } - throw failInHandshake; - } else if (!verifiedHostname) { - CertificateCombinedException ce = new CertificateCombinedException((X509Certificate) newSession.getPeerCertificates()[0]); - SSLPeerUnverifiedException pue = new SSLPeerUnverifiedException(host); - ce.setSslPeerUnverifiedException(pue); - pue.initCause(ce); + failInHandshake.setSslPeerUnverifiedException(pue); + pue.initCause(failInHandshake); throw pue; + + } else if (failInHandshake != null) { + SSLHandshakeException hse = new SSLHandshakeException("Server certificate could not be verified"); + hse.initCause(failInHandshake); + throw hse; } } catch (IOException io) { diff --git a/src/com/owncloud/android/network/CertificateCombinedException.java b/src/com/owncloud/android/network/CertificateCombinedException.java index 644c7359..fad7a6f2 100644 --- a/src/com/owncloud/android/network/CertificateCombinedException.java +++ b/src/com/owncloud/android/network/CertificateCombinedException.java @@ -1,3 +1,21 @@ +/* ownCloud Android client application + * Copyright (C) 2012 Bartek Przybylski + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + package com.owncloud.android.network; import java.security.cert.CertPathValidatorException; @@ -8,9 +26,31 @@ import java.security.cert.X509Certificate; import javax.net.ssl.SSLPeerUnverifiedException; -public class CertificateCombinedException extends CertificateException { - +/** + * Exception joining all the problems that {@link AdvancedX509TrustManager} can find in + * a certificate chain for a server. + * + * This was initially created as an extension of CertificateException, but some + * implementations of the SSL socket layer in existing devices are REPLACING the CertificateException + * instances thrown by {@link javax.net.ssl.X509TrustManager#checkServerTrusted(X509Certificate[], String)} + * with SSLPeerUnverifiedException FORGETTING THE CAUSING EXCEPTION instead of wrapping it. + * + * Due to this, extending RuntimeException is necessary to get that the CertificateCombinedException + * instance reaches {@link AdvancedSslSocketFactory#verifyPeerIdentity}. + * + * BE CAREFUL. As a RuntimeException extensions, Java compilers do not require to handle it + * in client methods. Be sure to use it only when you know exactly where it will go. + * + * @author David A. Velasco + */ +public class CertificateCombinedException extends RuntimeException { + + /** Generated */ + private static final long serialVersionUID = -8875782030758554999L; + private X509Certificate mServerCert = null; + private String mHostInUrl; + private CertificateExpiredException mCertificateExpiredException = null; private CertificateNotYetValidException mCertificateNotYetValidException = null; private CertPathValidatorException mCertPathValidatorException = null; @@ -25,6 +65,14 @@ public class CertificateCombinedException extends CertificateException { return mServerCert; } + public String getHostInUrl() { + return mHostInUrl; + } + + public void setHostInUrl(String host) { + mHostInUrl = host; + } + public CertificateExpiredException getCertificateExpiredException() { return mCertificateExpiredException; } @@ -69,13 +117,15 @@ public class CertificateCombinedException extends CertificateException { return (mCertificateExpiredException != null || mCertificateNotYetValidException != null || mCertPathValidatorException != null || - mOtherCertificateException != null); + mOtherCertificateException != null || + mSslPeerUnverifiedException != null); } public boolean isRecoverable() { return (mCertificateExpiredException != null || mCertificateNotYetValidException != null || - mCertPathValidatorException != null); + mCertPathValidatorException != null || + mSslPeerUnverifiedException != null); } } diff --git a/src/com/owncloud/android/operations/ChunkedUploadFileOperation.java b/src/com/owncloud/android/operations/ChunkedUploadFileOperation.java index 18fc1efe..0716a91f 100644 --- a/src/com/owncloud/android/operations/ChunkedUploadFileOperation.java +++ b/src/com/owncloud/android/operations/ChunkedUploadFileOperation.java @@ -35,7 +35,7 @@ import eu.alefzero.webdav.WebdavUtils; public class ChunkedUploadFileOperation extends UploadFileOperation { - private static final long CHUNK_SIZE = 8192; + private static final long CHUNK_SIZE = 102400; private static final String OC_CHUNKED_HEADER = "OC-Chunked"; public ChunkedUploadFileOperation( String localPath, diff --git a/src/com/owncloud/android/operations/RemoteOperationResult.java b/src/com/owncloud/android/operations/RemoteOperationResult.java index 080e0b64..eb895ca1 100644 --- a/src/com/owncloud/android/operations/RemoteOperationResult.java +++ b/src/com/owncloud/android/operations/RemoteOperationResult.java @@ -25,20 +25,19 @@ import java.net.SocketTimeoutException; import java.net.UnknownHostException; import javax.net.ssl.SSLException; +import javax.net.ssl.SSLPeerUnverifiedException; import org.apache.commons.httpclient.ConnectTimeoutException; import org.apache.commons.httpclient.HttpException; import org.apache.commons.httpclient.HttpStatus; -import android.util.Log; - import com.owncloud.android.network.CertificateCombinedException; /** * The result of a remote operation required to an ownCloud server. * - * Provides a common classification of resulst for all the application. + * Provides a common classification of remote operation results for all the application. * * @author David A. Velasco */ @@ -58,11 +57,10 @@ public class RemoteOperationResult { HOST_NOT_AVAILABLE, NO_NETWORK_CONNECTION, SSL_ERROR, - BAD_OC_VERSION, + SSL_RECOVERABLE_PEER_UNVERIFIED, + BAD_OC_VERSION } - private static final String TAG = null; - private boolean mSuccess = false; private int mHttpCode = -1; private Exception mException = null; @@ -112,8 +110,17 @@ public class RemoteOperationResult { } else if (e instanceof UnknownHostException) { mCode = ResultCode.HOST_NOT_AVAILABLE; - } else if (e instanceof SSLException) { - mCode = ResultCode.SSL_ERROR; + } else if (e instanceof SSLException || e instanceof RuntimeException) { + CertificateCombinedException se = getCertificateCombinedException(e); + if (se != null) { + mException = se; + if (se.isRecoverable()) { + mCode = ResultCode.SSL_RECOVERABLE_PEER_UNVERIFIED; + } + + } else { + mCode = ResultCode.SSL_ERROR; + } } else { mCode = ResultCode.UNKNOWN_ERROR; @@ -139,27 +146,24 @@ public class RemoteOperationResult { } public boolean isSslRecoverableException() { - return (getSslRecoverableException() != null); + return mCode == ResultCode.SSL_RECOVERABLE_PEER_UNVERIFIED; } - public CertificateCombinedException getSslRecoverableException() { + private CertificateCombinedException getCertificateCombinedException(Exception e) { CertificateCombinedException result = null; - if (mCode == ResultCode.SSL_ERROR) { - if (mException instanceof CertificateCombinedException) - result = (CertificateCombinedException)mException; - Throwable cause = mException.getCause(); - Throwable previousCause = null; - while (cause != null && cause != previousCause && !(cause instanceof CertificateCombinedException)) { - previousCause = cause; - cause = cause.getCause(); - } - if (cause != null && cause instanceof CertificateCombinedException) - result = (CertificateCombinedException)cause; + if (e instanceof CertificateCombinedException) { + return (CertificateCombinedException)e; + } + Throwable cause = mException.getCause(); + Throwable previousCause = null; + while (cause != null && cause != previousCause && !(cause instanceof CertificateCombinedException)) { + previousCause = cause; + cause = cause.getCause(); } - if (result != null && result.isRecoverable()) - return result; - else - return null; + if (cause != null && cause instanceof CertificateCombinedException) { + result = (CertificateCombinedException)cause; + } + return result; } @@ -182,7 +186,10 @@ public class RemoteOperationResult { return "Unknown host exception"; } else if (mException instanceof SSLException) { - return "SSL exception"; + if (mCode == ResultCode.SSL_RECOVERABLE_PEER_UNVERIFIED) + return "SSL recoverable exception"; + else + return "SSL exception"; } else if (mException instanceof HttpException) { return "HTTP violation"; @@ -195,6 +202,16 @@ public class RemoteOperationResult { } } + if (mCode == ResultCode.INSTANCE_NOT_CONFIGURED) { + return "The ownCloud server is not configured!"; + + } else if (mCode == ResultCode.NO_NETWORK_CONNECTION) { + return "No network connection"; + + } else if (mCode == ResultCode.BAD_OC_VERSION) { + return "No valid ownCloud version was found at the server"; + } + return "Operation finished with HTTP status code " + mHttpCode + " (" + (isSuccess()?"success":"fail") + ")"; } diff --git a/src/com/owncloud/android/ui/activity/AuthenticatorActivity.java b/src/com/owncloud/android/ui/activity/AuthenticatorActivity.java index d13b71e4..16c807cf 100644 --- a/src/com/owncloud/android/ui/activity/AuthenticatorActivity.java +++ b/src/com/owncloud/android/ui/activity/AuthenticatorActivity.java @@ -89,7 +89,7 @@ public class AuthenticatorActivity extends AccountAuthenticatorActivity private static final String IS_SSL_CONN = "IS_SSL_CONN"; private int mStatusText, mStatusIcon; private boolean mStatusCorrect, mIsSslConn; - private RemoteOperationResult mLastSslFailedResult; + private RemoteOperationResult mLastSslUntrustedServerResult; public static final String PARAM_USERNAME = "param_Username"; public static final String PARAM_HOSTNAME = "param_Hostname"; @@ -160,7 +160,7 @@ public class AuthenticatorActivity extends AccountAuthenticatorActivity break; } case DIALOG_SSL_VALIDATOR: { - dialog = SslValidatorDialog.newInstance(this, mLastSslFailedResult, this); + dialog = SslValidatorDialog.newInstance(this, mLastSslUntrustedServerResult, this); break; } case DIALOG_CERT_NOT_SAVED: { @@ -189,7 +189,7 @@ public class AuthenticatorActivity extends AccountAuthenticatorActivity case DIALOG_CERT_NOT_SAVED: break; case DIALOG_SSL_VALIDATOR: { - ((SslValidatorDialog)dialog).updateResult(mLastSslFailedResult); + ((SslValidatorDialog)dialog).updateResult(mLastSslUntrustedServerResult); break; } default: @@ -532,15 +532,18 @@ public class AuthenticatorActivity extends AccountAuthenticatorActivity mStatusText = R.string.auth_incorrect_address_title; break; - case SSL_ERROR: + case SSL_RECOVERABLE_PEER_UNVERIFIED: mStatusIcon = R.drawable.common_error; - mStatusText = R.string.auth_ssl_general_error_title; - //mStatusText = R.string.auth_ssl_unverified_server_title; - mLastSslFailedResult = result; - if (mLastSslFailedResult.isSslRecoverableException()) - showDialog(DIALOG_SSL_VALIDATOR); - break; + mStatusText = R.string.auth_ssl_unverified_server_title; + mLastSslUntrustedServerResult = result; + showDialog(DIALOG_SSL_VALIDATOR); + break; + case SSL_ERROR: + mStatusIcon = R.drawable.common_error; + mStatusText = R.string.auth_ssl_general_error_title; + break; + case HOST_NOT_AVAILABLE: mStatusIcon = R.drawable.common_error; mStatusText = R.string.auth_unknown_host_title; @@ -563,7 +566,7 @@ public class AuthenticatorActivity extends AccountAuthenticatorActivity mStatusText = R.string.auth_unknown_error_title; break; default: - Log.e(TAG, "Incorrect connection checker result type: " + operation); + Log.e(TAG, "Incorrect connection checker result type: " + result.getHttpCode()); } setResultIconAndText(mStatusIcon, mStatusText); if (!mStatusCorrect) diff --git a/src/com/owncloud/android/ui/dialog/SslValidatorDialog.java b/src/com/owncloud/android/ui/dialog/SslValidatorDialog.java index 2a9164ef..f3523cdb 100644 --- a/src/com/owncloud/android/ui/dialog/SslValidatorDialog.java +++ b/src/com/owncloud/android/ui/dialog/SslValidatorDialog.java @@ -125,15 +125,17 @@ public class SslValidatorDialog extends Dialog { public void updateResult(RemoteOperationResult result) { - mException = result.getSslRecoverableException(); - if (mException != null) { - // "clean" view + if (result.isSslRecoverableException()) { + mException = (CertificateCombinedException) result.getException(); + + /// clean ((TextView)mView.findViewById(R.id.reason_cert_not_trusted)).setVisibility(View.GONE); ((TextView)mView.findViewById(R.id.reason_cert_expired)).setVisibility(View.GONE); ((TextView)mView.findViewById(R.id.reason_cert_not_yet_valid)).setVisibility(View.GONE); ((TextView)mView.findViewById(R.id.reason_hostname_not_verified)).setVisibility(View.GONE); ((TextView)mView.findViewById(R.id.subject)).setVisibility(View.GONE); + /// refresh if (mException.getCertPathValidatorException() != null) { ((TextView)mView.findViewById(R.id.reason_cert_not_trusted)).setVisibility(View.VISIBLE); } @@ -163,6 +165,9 @@ public class SslValidatorDialog extends Dialog { text = text.substring(text.indexOf(",") + 1); subject.setVisibility(View.VISIBLE); subject.setText(text); + } else { + // this should not happen + subject.setText(R.string.ssl_validator_certificate_not_available); } }