Android: `isKnownRoot` never terminates when a system trust anchor has been disabled by the user

[ChoiSeungMyung]

Summary

CertificateVerifier.isKnownRoot walks the collision-suffixed aliases of a system trust anchor. Two of the three branches inside the loop continue without advancing the loop counter, so the loop re-evaluates the exact same alias forever. The thread that entered TLS verification repeats the same filesystem and keystore lookups indefinitely and never returns.

This is an availability bug, not a security one: no verification decision is weakened, the call simply never completes.

Location

android/rustls-platform-verifier/src/main/java/org/rustls/platformverifier/CertificateVerifier.kt, isKnownRoot, lines 431-462 at the current tip of main (last commit touching this file: 042330d85e8f14f8a088f6522718fcde5fb6da8d).

var i = 0
while (true) {
    val alias = "$hash.$i"

    if (!File(loadedSystemCertificateDirectory, alias).exists()) {
        break
    }

    val anchor = loadedSystemKeystore.getCertificate("system:$alias")

    // It's possible for `anchor` to be `null` if the user deleted a trust anchor.
    // Continue iterating as there may be further collisions after the deleted anchor.
    if (anchor == null) {
        continue
        // This should never happen
    } else if (anchor !is X509Certificate) {
        // SAFETY: ...
        Log.e(TAG, "anchor is not a certificate, alias: $alias")
        continue
        // If subject and public key match, it's a system root.
    } else {
        if ((root.subjectX500Principal == anchor.subjectX500Principal) && (root.publicKey == anchor.publicKey)) {
            systemTrustAnchorCache.add(key)
            return true
        }
    }

    i += 1
}

i += 1 is the last statement of the loop body. Both continue statements jump over it. When either branch is taken:

• i is unchanged, so alias is recomputed to the same string,
• the same file still exists, so the loop does not break,
• getCertificate("system:$alias") returns the same value, so the same branch is taken again.

The comment directly above the first continue ("Continue iterating as there may be further collisions after the deleted anchor") shows the intent was to advance to the next alias.

For reference, the Chromium code this is ported from (net/android/java/src/org/chromium/net/X509Util.java) puts the increment in the for update clause:

for (int i = 0; true; i++) {
    String alias = hash + '.' + i;
    ...
    if (anchor == null) continue;

There, continue still advances i. Kotlin has no C-style for, and the port moved the increment into the body, which changed what continue does.

Reproduction

The anchor == null branch is reachable exactly as its own comment describes: the alias file is present under the system cacerts directory but AndroidCAStore has no entry for it.

That is the state of a device where the user has turned off a preinstalled system CA (Settings > Security > Encryption & credentials > Trusted credentials > System > disable). The directory this file scans is $ANDROID_ROOT/etc/security/cacerts (resolved via System.getenv("ANDROID_ROOT"), normally /system), which is read-only, so <hash>.<i> stays on disk. The removal is recorded elsewhere: conscrypt's TrustedCertificateStore copies the certificate under $ANDROID_DATA/misc/keychain/cacerts-removed, and getCertificate returns null for a system alias once isDeletedSystemCertificate matches.

The upstream Chromium comment describes the same mechanism explicitly -- "the certificate remains in the system directory but is also added to another file" -- which is why its loop was written to keep advancing past that alias.

So: on such a device, verify any chain whose root subject hashes to the alias of the disabled anchor. verifyCertificateChain calls isKnownRoot(validChain.last()) (in the ocspResponse == null revocation-gating path, API >= 24) and the call never returns.

I found this by code review while vendoring the file rather than from a field report, so I have not captured a trace from a physical device; the reachability argument above is the whole of the evidence.

Impact

• The affected thread is whichever thread rustls is running the handshake on. It never completes, and keeps a core busy re-running the same File.exists() / getCertificate pair; there is no timeout and no way for the caller to recover.
• The second continue (anchor !is X509Certificate) has the same defect and additionally writes a log line on every iteration, so it would also flood logcat.

Suggested fix

Advance the counter before any branch can continue:

                 var i = 0
                 while (true) {
                     val alias = "$hash.$i"
+                    i += 1
 
                     if (!File(loadedSystemCertificateDirectory, alias).exists()) {
                         break
@@
                             return true
                         }
                     }
-
-                    i += 1
                 }

i is not read after the loop, so incrementing before the break check is harmless, and progress no longer depends on how many continue paths the body has. val alias = "$hash.${i++}" is an equivalent one-line alternative.

Happy to open a PR with either form if that helps.