From 286dd715e80724908301c4f2ccd06f82202c9f90 Mon Sep 17 00:00:00 2001 From: Osip Fatkullin Date: Thu, 17 Oct 2024 15:49:05 +0200 Subject: [PATCH] WIP: Throw specific exception for SSL Pinning failure --- .../ktor/client/engine/darwin/DarwinUtils.kt | 3 +- .../engine/darwin/KtorNSURLSessionDelegate.kt | 15 +++-- .../darwin/certificates/CertificatePinner.kt | 66 +++++++++---------- .../darwin/internal/DarwinTaskHandler.kt | 13 ++-- .../darwin/test/DarwinEngineTest.kt | 14 ++++ .../tests/utils/CommonClientTestUtils.kt | 7 +- 6 files changed, 71 insertions(+), 47 deletions(-) diff --git a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/DarwinUtils.kt b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/DarwinUtils.kt index 1611f7ad489..611474c848b 100644 --- a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/DarwinUtils.kt +++ b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/DarwinUtils.kt @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. */ package io.ktor.client.engine.darwin @@ -8,7 +8,6 @@ import io.ktor.client.call.* import io.ktor.client.engine.* import io.ktor.http.content.* import io.ktor.utils.io.* -import io.ktor.utils.io.errors.* import kotlinx.cinterop.* import kotlinx.coroutines.* import kotlinx.io.IOException diff --git a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/KtorNSURLSessionDelegate.kt b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/KtorNSURLSessionDelegate.kt index 182133eb3c5..d1fcff91b4c 100644 --- a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/KtorNSURLSessionDelegate.kt +++ b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/KtorNSURLSessionDelegate.kt @@ -1,5 +1,5 @@ /* - * Copyright 2014-2022 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. */ package io.ktor.client.engine.darwin @@ -131,11 +131,16 @@ public class KtorNSURLSessionDelegate( didReceiveChallenge: NSURLAuthenticationChallenge, completionHandler: (NSURLSessionAuthChallengeDisposition, NSURLCredential?) -> Unit ) { - val handler = challengeHandler - if (handler != null) { - handler(session, task, didReceiveChallenge, completionHandler) - } else { + val handler = challengeHandler ?: run { completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, didReceiveChallenge.proposedCredential) + return + } + + try { + handler(session, task, didReceiveChallenge, completionHandler) + } catch (cause: Throwable) { + taskHandlers[task]?.saveFailure(cause) + completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, null) } } } diff --git a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/certificates/CertificatePinner.kt b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/certificates/CertificatePinner.kt index a2df4b7db53..e282069586c 100644 --- a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/certificates/CertificatePinner.kt +++ b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/certificates/CertificatePinner.kt @@ -1,16 +1,20 @@ /* -* Copyright 2014-2021 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. -*/ + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + */ package io.ktor.client.engine.darwin.certificates import io.ktor.client.engine.darwin.* +import io.ktor.util.logging.* import kotlinx.cinterop.* +import kotlinx.io.* import platform.CoreCrypto.* import platform.CoreFoundation.* import platform.Foundation.* import platform.Security.* +private val LOG = KtorSimpleLogger("io.ktor.client.engine.darwin.certificates.CertificatePinner") + /** * Constrains which certificates are trusted. Pinning certificates defends against attacks on * certificate authorities. It also prevents connections through man-in-the-middle certificate @@ -112,36 +116,36 @@ public data class CertificatePinner( private val validateTrust: Boolean ) : ChallengeHandler { - @OptIn(ExperimentalForeignApi::class) override fun invoke( session: NSURLSession, task: NSURLSessionTask, challenge: NSURLAuthenticationChallenge, completionHandler: (NSURLSessionAuthChallengeDisposition, NSURLCredential?) -> Unit ) { + if (applyPinning(challenge)) { + completionHandler(NSURLSessionAuthChallengeUseCredential, challenge.proposedCredential) + } else { + completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, null) + } + } + + @OptIn(ExperimentalForeignApi::class) + private fun applyPinning(challenge: NSURLAuthenticationChallenge): Boolean { val hostname = challenge.protectionSpace.host val matchingPins = findMatchingPins(hostname) if (matchingPins.isEmpty()) { - println("CertificatePinner: No pins found for host") - completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, null) - return + LOG.trace { "No pins found for host" } + return false } - if (challenge.protectionSpace.authenticationMethod != - NSURLAuthenticationMethodServerTrust - ) { - println("CertificatePinner: Authentication method not suitable for pinning") - completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, null) - return + if (challenge.protectionSpace.authenticationMethod != NSURLAuthenticationMethodServerTrust) { + LOG.trace { "Authentication method not suitable for pinning" } + return false } val trust = challenge.protectionSpace.serverTrust - if (trust == null) { - println("CertificatePinner: Server trust is not available") - completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, null) - return - } + ?: throw CertificatePinnerException("Server trust is not available") if (validateTrust) { val hostCFString = CFStringCreateWithCString(null, hostname, kCFStringEncodingUTF8) @@ -150,11 +154,7 @@ public data class CertificatePinner( SecTrustSetPolicies(trust, policy) } } - if (!trust.trustIsValid()) { - println("CertificatePinner: Server trust is invalid") - completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, null) - return - } + if (!trust.trustIsValid()) throw CertificatePinnerException("Server trust is invalid") } val certCount = SecTrustGetCertificateCount(trust) @@ -163,19 +163,14 @@ public data class CertificatePinner( } if (certificates.size != certCount.toInt()) { - println("CertificatePinner: Unknown certificates") - completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, null) - return + throw CertificatePinnerException("Unknown certificates") } - val result = hasOnePinnedCertificate(certificates) - if (result) { - completionHandler(NSURLSessionAuthChallengeUseCredential, challenge.proposedCredential) - } else { + if (!hasOnePinnedCertificate(certificates)) { val message = buildErrorMessage(certificates, hostname) - println(message) - completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, null) + throw CertificatePinnerException(message) } + return true } /** @@ -199,6 +194,7 @@ public data class CertificatePinner( pin.hash == sha256 } + CertificatesInfo.HASH_ALGORITHM_SHA_1 -> { if (sha1 == null) { sha1 = publicKey.toSha1String() @@ -206,10 +202,8 @@ public data class CertificatePinner( pin.hash == sha1 } - else -> { - println("CertificatePinner: Unsupported hashAlgorithm: ${pin.hashAlgorithm}") - false - } + + else -> throw AssertionError("Unsupported hashAlgorithm: ${pin.hashAlgorithm}") } } } @@ -436,3 +430,5 @@ public data class CertificatePinner( ) } } + +public class CertificatePinnerException(message: String) : IOException(message) diff --git a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/internal/DarwinTaskHandler.kt b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/internal/DarwinTaskHandler.kt index 668f4afd5dd..0d687931984 100644 --- a/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/internal/DarwinTaskHandler.kt +++ b/ktor-client/ktor-client-darwin/darwin/src/io/ktor/client/engine/darwin/internal/DarwinTaskHandler.kt @@ -1,14 +1,12 @@ /* - * Copyright 2014-2022 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. */ package io.ktor.client.engine.darwin.internal import io.ktor.client.engine.darwin.* -import io.ktor.client.plugins.sse.* import io.ktor.client.request.* import io.ktor.http.* -import io.ktor.util.* import io.ktor.util.date.* import io.ktor.utils.io.* import io.ktor.utils.io.CancellationException @@ -28,6 +26,9 @@ internal class DarwinTaskHandler( private val requestTime: GMTDate = GMTDate() private val bodyChunks = Channel(Channel.UNLIMITED) + private var pendingFailure: Throwable? = null + get() = field?.also { field = null } + private val body: ByteReadChannel = GlobalScope.writer(callContext) { try { bodyChunks.consumeEach { @@ -54,9 +55,13 @@ internal class DarwinTaskHandler( } } + fun saveFailure(cause: Throwable) { + pendingFailure = cause + } + fun complete(task: NSURLSessionTask, didCompleteWithError: NSError?) { if (didCompleteWithError != null) { - val exception = handleNSError(requestData, didCompleteWithError) + val exception = pendingFailure ?: handleNSError(requestData, didCompleteWithError) bodyChunks.close(exception) response.completeExceptionally(exception) return diff --git a/ktor-client/ktor-client-darwin/darwin/test/DarwinEngineTest.kt b/ktor-client/ktor-client-darwin/darwin/test/DarwinEngineTest.kt index ddc3272bb41..0f1aa8c376e 100644 --- a/ktor-client/ktor-client-darwin/darwin/test/DarwinEngineTest.kt +++ b/ktor-client/ktor-client-darwin/darwin/test/DarwinEngineTest.kt @@ -272,6 +272,20 @@ class DarwinEngineTest { } } + @Test + fun testRethrowExceptionThrownDuringCustomChallenge() = runBlocking { + val challengeException = Exception("Challenge failed") + + val client = HttpClient(Darwin) { + engine { + handleChallenge { _, _, _, _ -> throw challengeException } + } + } + + val thrownException = assertFails { client.get(TEST_SERVER_TLS) } + assertTrue(thrownException === challengeException, "Expected exception to be rethrown") + } + private fun stringToNSUrlString(value: String): String { return Url(value).toNSUrl().absoluteString!! } diff --git a/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/CommonClientTestUtils.kt b/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/CommonClientTestUtils.kt index 3efe5ea95c8..4eadb362058 100644 --- a/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/CommonClientTestUtils.kt +++ b/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/CommonClientTestUtils.kt @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. */ @file:Suppress("NO_EXPLICIT_RETURN_TYPE_IN_API_MODE_WARNING", "KDocMissingDocumentation") @@ -18,6 +18,11 @@ import kotlin.time.Duration.Companion.milliseconds */ const val TEST_SERVER: String = "http://127.0.0.1:8080" +/** + * Web url with TLS for tests. + */ +const val TEST_SERVER_TLS: String = "https://127.0.0.1:8089" + /** * Websocket server url for tests. */