sync: refuse Basic over cleartext even when challenged
insecurePreemptive gated only the preemptive branch, so a plain-HTTP server answering 401 with a Basic challenge still got the password in a header every hop can read. The flag's name was accurate; its coverage was not. The gate moves onto the Basic emission, covering both paths, and the flag becomes insecureBasic. Digest is left answered over cleartext: it never puts the password on the wire, and refusing it would break a LAN server the day the per-account cleartext opt-in ships. Not reachable in the app today -- network_security_config forbids cleartext outright and nothing passes allowCleartext -- but :caldav is a plain JVM module meant for reuse, where neither of those holds. Upstream's testBasic asserted this exact behaviour over http://, so it is amended to https:// and the cleartext cases are pinned explicitly. PROVENANCE change 9 records it as the one upstream test this port changes rather than inherits.
This commit is contained in:
@@ -214,3 +214,31 @@ from every request rather than leaking it.
|
||||
|
||||
`UrlUtils.hostToDomain` and its test are left alone — after this it has no
|
||||
production callers, and keeping it keeps the resync diff small.
|
||||
|
||||
## Change 9 — Basic is refused over cleartext even when the server asks for it
|
||||
|
||||
`insecurePreemptive` gated only the preemptive branch. A plain-HTTP server
|
||||
answering 401 with a `Basic` challenge still got
|
||||
`Authorization: Basic <user:password>` in the clear — the flag's name was
|
||||
accurate and its coverage was not.
|
||||
|
||||
The gate now sits on the Basic *emission*, so both paths are covered by one
|
||||
condition, and the flag is renamed `insecureBasic` to say what it actually
|
||||
permits. Digest is deliberately untouched: it never puts the password on the
|
||||
wire, and refusing it would break a LAN server the day the documented
|
||||
per-account cleartext opt-in ships.
|
||||
|
||||
Not currently reachable in the app — `network_security_config.xml` sets
|
||||
`cleartextTrafficPermitted="false"`, so OkHttp throws before the request is
|
||||
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.
|
||||
|
||||
⚠️ **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
|
||||
cleartext cases it used to cover are pinned explicitly by
|
||||
`cleartextBasicIsRefusedEvenWhenChallenged`,
|
||||
`cleartextBasicIsSentWhenExplicitlyAllowed` and `cleartextDigestIsStillAnswered`.
|
||||
This is the one upstream test this port deliberately changes rather than
|
||||
inherits.
|
||||
|
||||
Reference in New Issue
Block a user