android: advance the alias counter before continuing in isKnownRoot

[ChoiSeungMyung]

Fixes #245.

isKnownRoot walks the collision-suffixed aliases of a system trust anchor. i += 1 was the last statement of the loop body, and two of the three branches reach the next iteration through continue, which jumps 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 thread running the handshake never returns and keeps a core busy repeating one File.exists() and one getCertificate() call. The anchor !is X509Certificate branch additionally logs on every iteration.

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

Why the branch is reachable

Exactly as its own comment says. conscrypt's TrustedCertificateStore.getCertificate returns null for a system alias once isDeletedSystemCertificate matches, which happens after the certificate is copied into $ANDROID_DATA/misc/keychain/cacerts-removed — the state produced by disabling a preinstalled CA in Settings. The file under $ANDROID_ROOT/etc/security/cacerts stays on disk because that partition is read-only, so File.exists() still returns true.

The Chromium comment this was ported from spells out the same mechanism: "the certificate remains in the system directory but is also added to another file. Continue iterating as there may be further collisions after the deleted anchor."

isKnownRoot is reached from verifyCertificateChain at the ocspResponse == null revocation-gating path (API >= 24), so any chain whose root subject hashes to the disabled anchor's alias hits this.

The fix

Chromium keeps the increment in the update clause of a C-style for, so continue still advances there. Kotlin has no such form and the port moved it into the body, changing what continue does. Incrementing right after the alias is built restores that invariant — progress no longer depends on how many continue paths the body has, which is the property that broke here.

i is not read after the loop, so incrementing before the break check is harmless. I checked the alias sequence against the original for (int i = 0; true; i++) semantics over every combination of 0–4 alias files × {absent, null, non-X509, non-matching, matching} anchors (341 cases): visit order and termination match in all of them.

val alias = "$hash.${i++}" is an equivalent one-liner if you prefer it; I went with the separate statement to keep the mutation out of the string template.

Testing

No test is included. isKnownRoot reads systemKeystore and systemCertificateDirectory, both private vals with no injection point, and KeyStore.getInstance("AndroidCAStore") is unavailable off-device, so reaching this loop from a test would need either a seam in the object or an instrumented device with a disabled system CA. Happy to add one if you'd like it done a particular way.

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