wbrawner/thunderbird-android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #7418 from thunderbird/simplify_CertificateValidationException

Browse source 
Simplify `CertificateValidationException` to only be used when there's a certificate chain
This commit is contained in:
cketti 2023-12-07 13:48:45 +01:00 committed by GitHub
commit dd2ec736b0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 153 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
app/core/src/main/java/com/fsck/k9/controller/MessagingController.java
View file

@ -2449,17 +2449,10 @@ public class MessagingController {
}
public void notifyUserIfCertificateProblem(Account account, Exception exception, boolean incoming) {
if (!(exception instanceof CertificateValidationException)) {
return;
}
CertificateValidationException cve = (CertificateValidationException) exception;
if (!cve.needsUserAttention()) {
return;
}
if (exception instanceof CertificateValidationException) {
notificationController.showCertificateErrorNotification(account, incoming);
}
}
private boolean isAuthenticationProblem(Account account, boolean incoming) {
ServerSettings serverSettings = incoming ?

3
app/core/src/test/java/com/fsck/k9/controller/MessagingControllerTest.java
View file

@ -47,6 +47,7 @@ import org.mockito.stubbing.Answer;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.shadows.ShadowLog;
import static java.util.Collections.emptyList;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.ArgumentMatchers.eq;
@ -356,7 +357,7 @@ public class MessagingControllerTest extends K9RobolectricTest {
@Test
public void sendPendingMessagesSynchronous_withCertificateFailure_shouldNotify() throws MessagingException {
setupAccountWithMessageToSend();
doThrow(new CertificateValidationException("Test", new CertificateChainException("", null, null)))
doThrow(new CertificateValidationException(emptyList(), new CertificateChainException("", null, null)))
.when(backend).sendMessage(localMessageToSend1);
controller.sendPendingMessagesSynchronous(account);

89
mail/common/src/main/java/com/fsck/k9/mail/CertificateValidationException.java
View file

@ -1,91 +1,20 @@
package com.fsck.k9.mail;
import java.security.cert.CertPathValidatorException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.List;
import org.jetbrains.annotations.NotNull;
import javax.net.ssl.SSLHandshakeException;
public class CertificateValidationException extends MessagingException {
private X509Certificate[] mCertChain;
private boolean mNeedsUserAttention = false;
private final List<X509Certificate> certificateChain;
public CertificateValidationException(final String message, Throwable cause) {
super(message, cause);
scanForCause();
public CertificateValidationException(@NotNull List<X509Certificate> certificateChain, Throwable cause) {
super(cause);
this.certificateChain = certificateChain;
}
private void scanForCause() {
Throwable throwable = getCause();
/*
* User attention is required if the server certificate was deemed
* invalid or if there was a problem with a client certificate.
*
* A CertificateException is known to be thrown by the default
* X509TrustManager.checkServerTrusted() if the server certificate
* doesn't validate. The cause of the CertificateException will be a
* CertPathValidatorException. However, it's unlikely those exceptions
* will be encountered here, because they are caught in
* SecureX509TrustManager.checkServerTrusted(), which throws a
* CertificateChainException instead (an extension of
* CertificateException).
*
* A CertificateChainException will likely result in (or, be the cause
* of) an SSLHandshakeException (an extension of SSLException).
*
* The various mail protocol handlers (IMAP, POP3, ...) will catch an
* SSLException and throw a CertificateValidationException (this class)
* with the SSLException as the cause. (They may also throw a
* CertificateValidationException when STARTTLS is not available, just
* for the purpose of triggering a user notification.)
*
* SSLHandshakeException is also known to occur if the *client*
* certificate was not accepted by the server (unknown CA, certificate
* expired, etc.). In this case, the SSLHandshakeException will not have
* a CertificateChainException as a cause.
*
* KeyChainException is known to occur if the device has no client
* certificate that's associated with the alias stored in the server
* settings.
*/
while (throwable != null
&& !(throwable instanceof CertPathValidatorException)
&& !(throwable instanceof CertificateException)
&& !("android.security.KeyChainException".equals(throwable.getClass().getCanonicalName()))
&& !(throwable instanceof SSLHandshakeException)) {
throwable = throwable.getCause();
}
if (throwable != null) {
mNeedsUserAttention = true;
// See if there is a server certificate chain attached to the SSLHandshakeException
if (throwable instanceof SSLHandshakeException) {
while (throwable != null && !(throwable instanceof CertificateChainException)) {
throwable = throwable.getCause();
}
}
if (throwable != null && throwable instanceof CertificateChainException) {
mCertChain = ((CertificateChainException) throwable).getCertChain();
}
}
}
public boolean needsUserAttention() {
return mNeedsUserAttention;
}
/**
* If the cause of this {@link CertificateValidationException} was a
* {@link CertificateChainException}, then the offending chain is available
* for return.
*
* @return An {@link X509Certificate X509Certificate[]} containing the Cert.
* chain, or else null.
*/
public X509Certificate[] getCertChain() {
return mCertChain;
public List<X509Certificate> getCertificateChain() {
return certificateChain;
}
}

23
mail/common/src/main/java/com/fsck/k9/mail/ssl/CertificateChainExtractor.kt Normal file
View file

@ -0,0 +1,23 @@
package com.fsck.k9.mail.ssl
import com.fsck.k9.mail.CertificateChainException
import java.security.cert.X509Certificate
/**
* Checks if an exception chain contains a [CertificateChainException] and if so, extracts the certificate chain from it
*/
object CertificateChainExtractor {
@JvmStatic
fun extract(throwable: Throwable): List<X509Certificate>? {
return findCertificateChainException(throwable)?.certChain?.toList()
}
private tailrec fun findCertificateChainException(throwable: Throwable): CertificateChainException? {
val cause = throwable.cause
return when {
throwable is CertificateChainException -> throwable
cause == null -> null
else -> findCertificateChainException(cause)
}
}
}

93
mail/common/src/test/java/com/fsck/k9/mail/ssl/CertificateChainExtractorTest.kt Normal file
View file

@ -0,0 +1,93 @@
package com.fsck.k9.mail.ssl
import assertk.assertThat
import assertk.assertions.containsExactly
import assertk.assertions.isNotNull
import assertk.assertions.isNull
import com.fsck.k9.mail.CertificateChainException
import java.security.cert.CertificateException
import java.security.cert.CertificateFactory
import java.security.cert.X509Certificate
import javax.net.ssl.SSLException
import kotlin.test.Test
class CertificateChainExtractorTest {
@Test
fun `input is a CertificateChainException`() {
val throwable = CertificateChainException(
"irrelevant",
arrayOf(CERTIFICATE),
null,
)
val result = CertificateChainExtractor.extract(throwable)
assertThat(result).isNotNull().containsExactly(CERTIFICATE)
}
@Test
fun `SSLException containing CertificateChainException as direct child`() {
val throwable = SSLException(
CertificateChainException(
"irrelevant",
arrayOf(CERTIFICATE),
null,
),
)
val result = CertificateChainExtractor.extract(throwable)
assertThat(result).isNotNull().containsExactly(CERTIFICATE)
}
@Test
fun `SSLException containing CertificateChainException as indirect child`() {
val throwable = SSLException(
CertificateException(
CertificateChainException(
"irrelevant",
arrayOf(CERTIFICATE),
null,
),
),
)
val result = CertificateChainExtractor.extract(throwable)
assertThat(result).isNotNull().containsExactly(CERTIFICATE)
}
@Test
fun `SSLException without a cause`() {
val throwable = SSLException("irrelevant")
val result = CertificateChainExtractor.extract(throwable)
assertThat(result).isNull()
}
@Test
fun `SSLException with multiple non-CertificateChainException children`() {
val throwable = SSLException(
IllegalStateException(
NumberFormatException(),
),
)
val result = CertificateChainExtractor.extract(throwable)
assertThat(result).isNull()
}
companion object {
private val CERTIFICATE = readCertificate("mail.domain.example")
@Suppress("SameParameterValue")
private fun readCertificate(name: String): X509Certificate {
val certificateFactory = CertificateFactory.getInstance("X.509")
this::class.java.getResourceAsStream("/certificates/$name.pem")!!.let { inputStream ->
return certificateFactory.generateCertificate(inputStream) as X509Certificate
}
}
}
}

2
mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapServerSettingsValidator.kt
View file

@ -39,7 +39,7 @@ class ImapServerSettingsValidator(
} catch (e: AuthenticationFailedException) {
ServerSettingsValidationResult.AuthenticationError(e.messageFromServer)
} catch (e: CertificateValidationException) {
ServerSettingsValidationResult.CertificateError(e.certChain.toList())
ServerSettingsValidationResult.CertificateError(e.certificateChain)
} catch (e: NegativeImapResponseException) {
ServerSettingsValidationResult.ServerError(e.responseText)
} catch (e: MessagingException) {

7
mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapConnection.kt
View file

@ -15,6 +15,7 @@ import com.fsck.k9.mail.filter.Base64
import com.fsck.k9.mail.filter.PeekableInputStream
import com.fsck.k9.mail.oauth.OAuth2TokenProvider
import com.fsck.k9.mail.oauth.XOAuth2ChallengeParser
import com.fsck.k9.mail.ssl.CertificateChainExtractor
import com.fsck.k9.mail.ssl.TrustedSocketFactory
import com.fsck.k9.sasl.buildOAuthBearerInitialClientResponse
import com.jcraft.jzlib.JZlib
@ -31,7 +32,6 @@ import java.net.SocketAddress
import java.net.UnknownHostException
import java.security.GeneralSecurityException
import java.security.Security
import java.security.cert.CertificateException
import java.util.regex.Pattern
import java.util.zip.Inflater
import java.util.zip.InflaterInputStream
@ -114,8 +114,9 @@ internal class RealImapConnection(
}
private fun handleSslException(e: SSLException) {
if (e.cause is CertificateException) {
throw CertificateValidationException(e.message, e)
val certificateChain = CertificateChainExtractor.extract(e)
if (certificateChain != null) {
throw CertificateValidationException(certificateChain, e)
} else {
throw e
}

7
mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3Connection.java
View file

@ -14,6 +14,7 @@ import java.security.KeyManagementException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
@ -29,6 +30,7 @@ import com.fsck.k9.mail.MessagingException;
import com.fsck.k9.mail.MissingCapabilityException;
import com.fsck.k9.mail.filter.Base64;
import com.fsck.k9.mail.filter.Hex;
import com.fsck.k9.mail.ssl.CertificateChainExtractor;
import com.fsck.k9.mail.ssl.TrustedSocketFactory;
import javax.net.ssl.SSLException;
@ -92,8 +94,9 @@ class Pop3Connection {
performAuthentication(settings.getAuthType(), serverGreeting);
} catch (SSLException e) {
if (e.getCause() instanceof CertificateException) {
throw new CertificateValidationException(e.getMessage(), e);
List<X509Certificate> certificateChain = CertificateChainExtractor.extract(e);
if (certificateChain != null) {
throw new CertificateValidationException(certificateChain, e);
} else {
throw new MessagingException("Unable to connect", e);
}

2
mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3ServerSettingsValidator.kt
View file

@ -28,7 +28,7 @@ class Pop3ServerSettingsValidator(
} catch (e: AuthenticationFailedException) {
ServerSettingsValidationResult.AuthenticationError(e.messageFromServer)
} catch (e: CertificateValidationException) {
ServerSettingsValidationResult.CertificateError(e.certChain.toList())
ServerSettingsValidationResult.CertificateError(e.certificateChain)
} catch (e: Pop3ErrorResponse) {
ServerSettingsValidationResult.ServerError(e.responseText)
} catch (e: MessagingException) {

11
mail/protocols/pop3/src/test/java/com/fsck/k9/mail/store/pop3/Pop3ConnectionTest.kt
View file

@ -10,6 +10,7 @@ import com.fsck.k9.mail.AuthType.EXTERNAL
import com.fsck.k9.mail.AuthType.LOGIN
import com.fsck.k9.mail.AuthType.PLAIN
import com.fsck.k9.mail.AuthenticationFailedException
import com.fsck.k9.mail.CertificateChainException
import com.fsck.k9.mail.CertificateValidationException
import com.fsck.k9.mail.ConnectionSecurity
import com.fsck.k9.mail.ConnectionSecurity.NONE
@ -21,7 +22,6 @@ import com.fsck.k9.mail.helpers.TestTrustedSocketFactory
import com.fsck.k9.mail.ssl.TrustedSocketFactory
import java.io.IOException
import java.security.NoSuchAlgorithmException
import java.security.cert.CertificateException
import javax.net.ssl.SSLException
import okio.ByteString.Companion.encodeUtf8
import org.junit.Test
@ -34,15 +34,18 @@ import org.mockito.kotlin.verifyNoInteractions
class Pop3ConnectionTest {
private val socketFactory = TestTrustedSocketFactory.newInstance()
@Test(expected = CertificateValidationException::class)
fun `when TrustedSocketFactory throws SSLCertificateException, open() should throw CertificateValidationException`() {
@Test
fun `when TrustedSocketFactory throws wrapped CertificateChainException, open() should throw`() {
val server = startTlsServer()
val settings = server.createSettings(connectionSecurity = SSL_TLS_REQUIRED)
val mockSocketFactory = mock<TrustedSocketFactory> {
on { createSocket(null, settings.host, settings.port, null) } doThrow SSLException(CertificateException())
on { createSocket(null, settings.host, settings.port, null) } doThrow
SSLException(CertificateChainException("irrelevant", arrayOf(), null))
}
assertFailure {
createAndOpenPop3Connection(settings, mockSocketFactory)
}.isInstanceOf<CertificateValidationException>()
}
@Test(expected = MessagingException::class)

2
mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpServerSettingsValidator.kt
View file

@ -32,7 +32,7 @@ class SmtpServerSettingsValidator(
} catch (e: AuthenticationFailedException) {
ServerSettingsValidationResult.AuthenticationError(e.messageFromServer)
} catch (e: CertificateValidationException) {
ServerSettingsValidationResult.CertificateError(e.certChain.toList())
ServerSettingsValidationResult.CertificateError(e.certificateChain)
} catch (e: NegativeSmtpReplyException) {
ServerSettingsValidationResult.ServerError(e.replyText)
} catch (e: MessagingException) {

7
mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.kt
View file

@ -22,6 +22,7 @@ import com.fsck.k9.mail.filter.PeekableInputStream
import com.fsck.k9.mail.filter.SmtpDataStuffing
import com.fsck.k9.mail.oauth.OAuth2TokenProvider
import com.fsck.k9.mail.oauth.XOAuth2ChallengeParser
import com.fsck.k9.mail.ssl.CertificateChainExtractor
import com.fsck.k9.mail.ssl.TrustedSocketFactory
import com.fsck.k9.mail.transport.smtp.SmtpHelloResponse.Hello
import com.fsck.k9.sasl.buildOAuthBearerInitialClientResponse
@ -35,7 +36,6 @@ import java.net.InetSocketAddress
import java.net.Socket
import java.net.UnknownHostException
import java.security.GeneralSecurityException
import java.security.cert.CertificateException
import java.util.Locale
import javax.net.ssl.SSLException
import org.apache.commons.io.IOUtils
@ -223,8 +223,9 @@ class SmtpTransport(
throw e
} catch (e: SSLException) {
close()
if (e.cause is CertificateException) {
throw CertificateValidationException(e.message, e)
val certificateChain = CertificateChainExtractor.extract(e)
if (certificateChain != null) {
throw CertificateValidationException(certificateChain, e)
} else {
throw e
}