From 89c3a5f66e8fe2f03db5f1c988e1c9a30804b5a4 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 7 Sep 2026 21:45:44 +0200 Subject: [PATCH] sync: do not cache a challenge we refuse to answer The cleartext gate sits on the emission, but the 401 branch cached the Basic challenge before reaching it. So after refusing a plain-HTTP challenge the handler still believed Basic was in play: the preemptive block is skipped because it requires both caches empty, the refusal repeats, and the 401 after that logs "Basic credentials didn't work last time" about a credential that never reached the wire. --- dav/PROVENANCE.md | 5 +++++ .../bitfire/dav4jvm/BasicDigestAuthHandler.kt | 7 ++++++- .../dav4jvm/BasicDigestAuthHandlerTest.kt | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/dav/PROVENANCE.md b/dav/PROVENANCE.md index ff29a7a..10419c4 100644 --- a/dav/PROVENANCE.md +++ b/dav/PROVENANCE.md @@ -234,6 +234,11 @@ written, and nothing passes `allowCleartext = true`. Fixed anyway: `:caldav` is a plain JVM module earmarked for reuse, where the Android policy does not apply, and the mitigation would evaporate silently the day that opt-in is wired up. +A refused challenge is also not cached. Recording one we never answer leaves the +handler believing Basic is in play, so the preemptive block is skipped, the +refusal repeats, and the 401 after that reports "Basic credentials didn't work +last time" about a credential that never reached the wire. + ⚠️ **Upstream's `BasicDigestAuthHandlerTest.testBasic` was amended** — it asserted exactly this behaviour, using `http://example.com` with a Basic challenge and expecting the header. Its URL is now `https://`, and the diff --git a/dav/src/main/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandler.kt b/dav/src/main/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandler.kt index 561d71a..44e7d4d 100644 --- a/dav/src/main/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandler.kt +++ b/dav/src/main/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandler.kt @@ -116,7 +116,12 @@ class BasicDigestAuthHandler( } } - basicAuth = newBasicAuth + // ⚠️ Not cached if we would refuse to answer it. Caching a challenge + // we never answer leaves the handler believing Basic is in play: the + // preemptive block is skipped, the refusal repeats, and the 401 after + // that reports "Basic credentials didn't work last time" about a + // credential that was never put on the wire. + basicAuth = newBasicAuth?.takeIf { request.isHttps || insecureBasic } digestAuth = newDigestAuth } diff --git a/dav/src/test/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandlerTest.kt b/dav/src/test/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandlerTest.kt index 139de0b..0c7a497 100644 --- a/dav/src/test/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandlerTest.kt +++ b/dav/src/test/kotlin/at/bitfire/dav4jvm/BasicDigestAuthHandlerTest.kt @@ -60,6 +60,23 @@ class BasicDigestAuthHandlerTest { assertNull(authenticator.authenticateRequest(original, response)) } + @Test + fun aRefusedCleartextChallengeIsNotCached() { + val authenticator = BasicDigestAuthHandler(null, "user", "password") + val original = Request.Builder().url("http://example.com").build() + val response = Builder() + .request(original) + .protocol(Protocol.HTTP_1_1) + .code(401).message("Authentication required") + .header("WWW-Authenticate", "Basic realm=\"WallyWorld\"") + .build() + + assertNull(authenticator.authenticateRequest(original, response)) + // Caching it would make the *next* 401 report that the credentials did + // not work, about a credential that was never sent. + assertNull(authenticator.authenticateRequest(original, response)) + } + @Test fun cleartextBasicIsSentWhenExplicitlyAllowed() { val authenticator = BasicDigestAuthHandler(null, "user", "password", insecureBasic = true)