From bc6e7311e4a75e308641cfdee84fc172c7aaeb4d Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Wed, 15 Apr 2026 07:36:09 -0500 Subject: [PATCH] Android agent: retry DNS (#43464) **Related issue:** Resolves #43462 During review, Hide whitespace. Fixed Android agent to retry DNS resolution failures when waking from Doze mode, and to defer remaining certificates in a batch to the next enrollment cycle when a DNS failure persists. The fix does not eliminates DNS errors from the logs, it just handles them better. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **Bug Fixes** * Improved DNS resilience: automatic retries with backoff for DNS resolution failures (e.g., after device sleep), upfront validation of the configured server URL, and clearer failure reporting when retries are exhausted. * Certificate enrollment aborts a batch on terminal DNS failures and defers remaining certificates until connectivity is restored. * **Tests** * Added a unit test validating batch abort behavior on DNS resolution failure. --- .../main/java/com/fleetdm/agent/ApiClient.kt | 161 +++++++++++------- .../fleetdm/agent/CertificateOrchestrator.kt | 26 ++- .../agent/CertificateOrchestratorTest.kt | 94 ++++++++++ .../com/fleetdm/agent/scep/MockScepClient.kt | 4 +- android/changes/43462-android-cert-dns-retry | 1 + 5 files changed, 217 insertions(+), 69 deletions(-) create mode 100644 android/changes/43462-android-cert-dns-retry diff --git a/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt b/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt index ec7524d3e8..1c8ee0a03a 100644 --- a/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt +++ b/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt @@ -11,8 +11,11 @@ import androidx.datastore.preferences.preferencesDataStore import java.math.BigInteger import java.net.HttpURLConnection import java.net.URL +import java.net.UnknownHostException import java.util.Date +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map @@ -59,6 +62,13 @@ interface CertificateApiClient { object ApiClient : CertificateApiClient { private const val TAG = "fleet-ApiClient" + + // Retry DNS resolution failures that occur when Android wakes from Doze mode. + // The active network may be reported as connected before its DNS servers are fully operational. + // Uses exponential backoff: 1s, 2s, 4s, 8s, 16s, 32s, 64s between attempts (127s total retry window, 8 attempts). + private const val DNS_MAX_RETRIES = 7 + private const val DNS_INITIAL_RETRY_DELAY_MS = 1000L + private val json = Json { ignoreUnknownKeys = true } private lateinit var dataStore: DataStore @@ -118,78 +128,97 @@ object ApiClient : CertificateApiClient { ): Result = withContext(Dispatchers.IO) { require(method != "GET" || body == null) { "GET requests must not include a body" } - var connection: HttpURLConnection? = null - try { - val baseUrl = getBaseUrl() ?: return@withContext Result.failure( - Exception("Base URL not configured"), - ) + val baseUrl = getBaseUrl() ?: return@withContext Result.failure( + Exception("Base URL not configured"), + ) - // Validate base URL format and scheme - try { - val parsedUrl = URL(baseUrl) - if (parsedUrl.protocol !in listOf("https", "http")) { - return@withContext Result.failure( - Exception("Base URL must use HTTP or HTTPS scheme"), - ) - } - } catch (e: Exception) { + // Validate base URL format and scheme + try { + val parsedUrl = URL(baseUrl) + if (parsedUrl.protocol !in listOf("https", "http")) { return@withContext Result.failure( - Exception("Invalid base URL format: ${e.message}"), + Exception("Base URL must use HTTP or HTTPS scheme"), ) } - - val url = URL("$baseUrl$endpoint") - connection = openConnectionOnActiveNetwork(url) - - connection.apply { - requestMethod = method - useCaches = false - doInput = true - if (authorized) { - getNodeKeyOrEnroll().fold( - onFailure = { throwable -> return@withContext Result.failure(throwable) }, - onSuccess = { nodeKey -> - setRequestProperty("Authorization", "Node key $nodeKey") - }, - ) - } - connectTimeout = 15000 - readTimeout = 15000 - - if (body != null) { - requireNotNull(bodySerializer) { "bodySerializer required when body is provided" } - setRequestProperty("Content-Type", "application/json") - doOutput = true - val bodyJson = json.encodeToString(value = body, serializer = bodySerializer) - outputStream.use { it.write(bodyJson.toByteArray()) } - } - } - - val responseCode = connection.responseCode - val response = if (responseCode in 200..299) { - connection.inputStream.bufferedReader().use { it.readText() } - } else { - connection.errorStream?.bufferedReader()?.use { it.readText() } - ?: "HTTP $responseCode" - } - - Log.d(TAG, "server response from $method $endpoint ($responseCode)") - - if (responseCode in 200..299) { - val parsed = json.decodeFromString(string = response, deserializer = responseSerializer) - Result.success(parsed) - } else if (responseCode == 401) { - Result.failure(UnauthorizedException(response)) - } else if (responseCode == 404) { - Result.failure(NotFoundException(response)) - } else { - Result.failure(Exception("HTTP $responseCode: $response")) - } } catch (e: Exception) { - Result.failure(e) - } finally { - connection?.disconnect() + return@withContext Result.failure( + Exception("Invalid base URL format: ${e.message}"), + ) } + + val url = URL("$baseUrl$endpoint") + + for (dnsAttempt in 0..DNS_MAX_RETRIES) { + var connection: HttpURLConnection? = null + try { + if (dnsAttempt > 0) { + val delayMs = DNS_INITIAL_RETRY_DELAY_MS shl (dnsAttempt - 1) + Log.w(TAG, "retrying $method $endpoint after DNS failure (attempt ${dnsAttempt + 1}, delay ${delayMs}ms)") + delay(delayMs) + } + + connection = openConnectionOnActiveNetwork(url) + + connection.apply { + requestMethod = method + useCaches = false + doInput = true + if (authorized) { + getNodeKeyOrEnroll().fold( + onFailure = { throwable -> return@withContext Result.failure(throwable) }, + onSuccess = { nodeKey -> + setRequestProperty("Authorization", "Node key $nodeKey") + }, + ) + } + connectTimeout = 15000 + readTimeout = 15000 + + if (body != null) { + requireNotNull(bodySerializer) { "bodySerializer required when body is provided" } + setRequestProperty("Content-Type", "application/json") + doOutput = true + val bodyJson = json.encodeToString(value = body, serializer = bodySerializer) + outputStream.use { it.write(bodyJson.toByteArray()) } + } + } + + val responseCode = connection.responseCode + val response = if (responseCode in 200..299) { + connection.inputStream.bufferedReader().use { it.readText() } + } else { + connection.errorStream?.bufferedReader()?.use { it.readText() } + ?: "HTTP $responseCode" + } + + Log.d(TAG, "server response from $method $endpoint ($responseCode)") + + return@withContext if (responseCode in 200..299) { + val parsed = json.decodeFromString(string = response, deserializer = responseSerializer) + Result.success(parsed) + } else if (responseCode == 401) { + Result.failure(UnauthorizedException(response)) + } else if (responseCode == 404) { + Result.failure(NotFoundException(response)) + } else { + Result.failure(Exception("HTTP $responseCode: $response")) + } + } catch (e: UnknownHostException) { + Log.w(TAG, "DNS resolution failed for $method $endpoint: ${e.message}") + if (dnsAttempt >= DNS_MAX_RETRIES) { + return@withContext Result.failure(e) + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + return@withContext Result.failure(e) + } finally { + connection?.disconnect() + } + } + + // Unreachable: the loop always returns. Required by the compiler since it can't prove the range is non-empty. + Result.failure(Exception("Exhausted DNS retries for $method $endpoint")) } /** diff --git a/android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt b/android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt index fed584a5a6..80a6364b89 100644 --- a/android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt +++ b/android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt @@ -9,6 +9,7 @@ import androidx.datastore.preferences.core.stringPreferencesKey import com.fleetdm.agent.scep.ScepClient import com.fleetdm.agent.scep.ScepClientImpl import java.math.BigInteger +import java.net.UnknownHostException import java.security.PrivateKey import java.security.cert.Certificate import java.text.SimpleDateFormat @@ -824,6 +825,10 @@ class CertificateOrchestrator( * (template fetch, SCEP enrollment, status update), so sequential processing avoids * overwhelming the server and the device's network stack. * + * If DNS resolution fails for a certificate (after exhausting in-call retries), we abort the + * remaining certs since DNS failures are network-level, not cert-specific. The worker will + * return Result.retry() and WorkManager's backoff handles the DNS recovery. + * * @param context Android context for certificate installation * @param hostCertificates List of certificate templates to enroll * @return Map of certificate ID to enrollment result @@ -835,11 +840,28 @@ class CertificateOrchestrator( ): Map { Log.d(TAG, "Starting batch certificate enrollment for ${hostCertificates.size} certificates") - return hostCertificates.associate { cert -> - cert.id to enrollCertificate(context, cert.id, cert.uuid, certificateInstaller) + val results = mutableMapOf() + for (cert in hostCertificates) { + val result = enrollCertificate(context, cert.id, cert.uuid, certificateInstaller) + results[cert.id] = result + if (result is CertificateEnrollmentHandler.EnrollmentResult.Failure && result.isDnsFailure()) { + Log.w( + TAG, + "DNS resolution failed for certificate ${cert.id}, aborting batch (${hostCertificates.size - results.size} certs deferred to next run)", + ) + break + } } + return results } + /** + * Returns true if this failure was caused by a DNS resolution problem, either directly or wrapped in + * another exception (e.g. SCEP wraps UnknownHostException in ScepNetworkException). + */ + private fun CertificateEnrollmentHandler.EnrollmentResult.Failure.isDnsFailure(): Boolean = + generateSequence(exception as Throwable?) { it.cause }.any { it is UnknownHostException } + /** * Android-specific certificate installer using DevicePolicyManager. * diff --git a/android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt b/android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt index 0d99116ac5..1c69fe1d11 100644 --- a/android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt +++ b/android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt @@ -1403,4 +1403,98 @@ class CertificateOrchestratorTest { assertEquals(1, fakeApiClient.updateStatusCalls.size) assertEquals(UpdateCertificateStatusStatus.FAILED, fakeApiClient.updateStatusCalls[0].status) } + + @Test + fun `enrollCertificates aborts batch on DNS failure`() = runTest { + // Scenario: DNS failure during enrollment of cert 1. Since DNS is a network-level issue + // (not cert-specific), we should abort the batch and defer remaining certs rather than + // burning the DNS retry window on each subsequent cert. + // + // Covers two cases: + // - Direct UnknownHostException from the Fleet API template fetch. + // - UnknownHostException wrapped in ScepNetworkException during SCEP enrollment (the + // orchestrator must walk the cause chain to detect it). + + data class Case(val name: String, val configure: () -> Unit, val assertException: (Throwable?) -> Unit) + + val cases = listOf( + Case( + name = "direct UnknownHostException from API", + configure = { + fakeApiClient.getCertificateTemplateHandler = { certId -> + if (certId == 1) { + Result.failure(java.net.UnknownHostException("Unable to resolve host")) + } else { + Result.success(successfulTemplateResult(certId)) + } + } + }, + assertException = { exception -> + assertTrue( + "Expected UnknownHostException but got ${exception?.javaClass?.name}", + exception is java.net.UnknownHostException, + ) + }, + ), + Case( + name = "UnknownHostException wrapped in ScepNetworkException", + configure = { + fakeApiClient.getCertificateTemplateHandler = { certId -> + Result.success(successfulTemplateResult(certId)) + } + mockScepClient.shouldThrowNetworkException = true + mockScepClient.networkExceptionCause = java.net.UnknownHostException("Unable to resolve host") + }, + assertException = { exception -> + assertTrue( + "Expected ScepNetworkException but got ${exception?.javaClass?.name}", + exception is com.fleetdm.agent.scep.ScepNetworkException, + ) + assertTrue( + "Expected UnknownHostException in cause chain", + exception?.cause is java.net.UnknownHostException, + ) + }, + ), + ) + + for (case in cases) { + // Reset state between cases + clearDataStore() + fakeApiClient.reset() + mockScepClient.reset() + + case.configure() + + val hostCertificates = listOf( + HostCertificate(id = 1, status = "delivered", operation = "install", uuid = "uuid-1"), + HostCertificate(id = 2, status = "delivered", operation = "install", uuid = "uuid-2"), + HostCertificate(id = 3, status = "delivered", operation = "install", uuid = "uuid-3"), + ) + + val results = orchestrator.enrollCertificates(context, hostCertificates, mockInstaller) + + // Only cert 1 is in results; certs 2 and 3 were skipped + assertEquals("${case.name}: only cert 1 processed", 1, results.size) + val failure = results[1] as CertificateEnrollmentHandler.EnrollmentResult.Failure + assertTrue("${case.name}: failure is retryable", failure.isRetryable) + case.assertException(failure.exception) + // API was only called for cert 1 - certs 2 and 3 never attempted + assertEquals("${case.name}: only cert 1 fetched", listOf(1), fakeApiClient.getCertificateTemplateCalls) + } + } + + private fun successfulTemplateResult(certId: Int) = CertificateTemplateResult( + template = GetCertificateTemplateResponse( + id = certId, + name = "cert-$certId", + certificateAuthorityId = 1, + certificateAuthorityName = "TestCA", + createdAt = "2025-01-01T00:00:00Z", + subjectName = "CN=test", + certificateAuthorityType = "custom_scep_proxy", + status = "delivered", + ), + scepUrl = TestCertificateTemplateFactory.DEFAULT_SCEP_URL, + ) } diff --git a/android/app/src/test/java/com/fleetdm/agent/scep/MockScepClient.kt b/android/app/src/test/java/com/fleetdm/agent/scep/MockScepClient.kt index ab4021610c..670466816d 100644 --- a/android/app/src/test/java/com/fleetdm/agent/scep/MockScepClient.kt +++ b/android/app/src/test/java/com/fleetdm/agent/scep/MockScepClient.kt @@ -24,6 +24,7 @@ class MockScepClient : ScepClient { var shouldThrowEnrollmentException = false var shouldThrowNetworkException = false var shouldThrowCertificateException = false + var networkExceptionCause: Throwable? = null var enrollmentDelay = 0L var capturedConfig: GetCertificateTemplateResponse? = null var capturedScepUrl: String? = null @@ -43,7 +44,7 @@ class MockScepClient : ScepClient { } when { - shouldThrowNetworkException -> throw ScepNetworkException("Mock network error") + shouldThrowNetworkException -> throw ScepNetworkException("Mock network error", networkExceptionCause) shouldThrowEnrollmentException -> throw ScepEnrollmentException("Mock enrollment failed") shouldThrowCertificateException -> throw ScepCertificateException("Mock certificate error") !shouldSucceed -> throw ScepException("Mock general error") @@ -101,6 +102,7 @@ class MockScepClient : ScepClient { shouldThrowEnrollmentException = false shouldThrowNetworkException = false shouldThrowCertificateException = false + networkExceptionCause = null enrollmentDelay = 0L capturedConfig = null capturedScepUrl = null diff --git a/android/changes/43462-android-cert-dns-retry b/android/changes/43462-android-cert-dns-retry new file mode 100644 index 0000000000..ceb818ff9e --- /dev/null +++ b/android/changes/43462-android-cert-dns-retry @@ -0,0 +1 @@ +- Fixed Android agent to retry DNS resolution failures when waking from Doze mode, and to defer remaining certificates in a batch to the next enrollment cycle when a DNS failure persists.