Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android: Don't attempt to check revocation on non-public certificates #108

Merged
merged 1 commit into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions android/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
.externalNativeBuild
.cxx
local.properties
emulator.log
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.Context
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.BeforeClass
import org.junit.Test
import org.junit.runner.RunWith
Expand Down Expand Up @@ -50,4 +51,22 @@ class CertificateVerifierTests {
val result = verifyMockRootUsage(context)
assertEquals(FAILURE_MSG, SUCCESS_MARKER, result)
}

// Note:
//
// - Full negative path (`CertificateVerifier`'s flow for unknown roots,
// are already exercised via `runMockTestSuite`).
//
// - Full positive path (`CertificateVerifier`'s flow for known roots,
// partial-chain revocation checks) already exercised via `runRealWorldTestSuite`.
@Test
fun runTestIsPublicRoot() {
val rootCAs = CertificateVerifier.getSystemRootCAs()

// Positive - can ID known roots
assertTrue(rootCAs.isNotEmpty())
for (ca in rootCAs) {
assertTrue(CertificateVerifier.isKnownRoot(ca))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,21 @@ internal object CertificateVerifier {
// If there is no stapled OCSP response, Android may use the network to attempt to fetch
// one. If OCSP checking fails, it may fall back to fetching CRLs. We allow "soft"
// failures, for example transient network errors.
//
// In the case of a non-public root, such as an internal CA or self-signed certificate,
// we opt to skip revocation checks entirely. The only exception is if the server
// provided stapled OCSP data, which is an explicit signal and won't introduce non-ideal
// platform behavior when attempting validation.
//
// This is because these are cases where a user or administrator has explicitly opted to
// trust a certificate they (at least believe) have control over. These certificates rarely
// contain revocation information as well, so these cases don't lose much.
// See https://github.com/rustls/rustls-platform-verifier/issues/69 as well.
if (ocspResponse == null && !isKnownRoot(validChain.last())) {
// Chain validation must have succeeded by this point.
return VerificationResult(StatusCode.Ok)
}

val parameters = PKIXBuilderParameters(keystore, null)

val validator = CertPathValidator.getInstance("PKIX")
Expand Down Expand Up @@ -386,4 +401,61 @@ internal object CertificateVerifier {

return String(hexChars)
}

// Check if CA root is known or not.
// Known means installed in root CA store, either a preset public CA or a custom one installed by an enterprise/user.
//
// Ref: https://source.chromium.org/chromium/chromium/src/+/main:net/android/java/src/org/chromium/net/X509Util.java;l=351
fun isKnownRoot(root: X509Certificate): Boolean {
// System keystore and cert directory must be non-null to perform checking
systemKeystore?.let { loadedSystemKeystore ->
systemCertificateDirectory?.let { loadedSystemCertificateDirectory ->

// Check the in-memory cache first
val key = Pair(root.subjectX500Principal, root.publicKey)
if (systemTrustAnchorCache.contains(key)) {
return true
}

// System trust anchors are stored under a hash of the principal.
// In case of collisions, append number.
val hash = hashPrincipal(root.subjectX500Principal)
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: This logs a unique identifier (hash value) only in cases where a file within the
// system's root trust store is not a valid X509 certificate (extremely unlikely error).
// The hash doesn't tell us any sensitive information about the invalid cert or reveal any of
// its contents - it just lets us ID the bad file if a user is having TLS failure issues.
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
}
}
}

// Not found in cache or store: non-public
return false
}
}
15 changes: 14 additions & 1 deletion rustls-platform-verifier/src/tests/verification_mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,23 @@ mock_root_test_cases! {
expected_result: Ok(()),
other_error: no_error!(),
},
// Uses a separate certificate from the one used in the "good" case to deal

// The revocation tests use a separate certificate from the one used in the "good" case to deal
// with operating systems with validation data caches (e.g. Windows).
// Linux is not included, since the webpki verifier does not presently support OCSP revocation
// checking.

// Check that self-signed certificates, which may or may not be revokved, do not return any
// kind of revocation error. It is expected that non-public certificates without revocation information
// have no revocation checking performed across platforms.
revoked_dns [ any(windows, target_os = "android", target_os = "macos") ] => TestCase {
reference_id: EXAMPLE_COM,
chain: &[include_bytes!("root1-int1-ee_example.com-revoked.crt"), ROOT1_INT1],
stapled_ocsp: None,
verification_time: verification_time(),
expected_result: Ok(()),
other_error: no_error!(),
},
stapled_revoked_dns [ any(windows, target_os = "android", target_os = "macos") ] => TestCase {
reference_id: EXAMPLE_COM,
chain: &[include_bytes!("root1-int1-ee_example.com-revoked.crt"), ROOT1_INT1],
Expand Down