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 faa6ebb10c..307986e151 100644 --- a/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt +++ b/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt @@ -7,8 +7,13 @@ import androidx.datastore.preferences.core.Preferences import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.preferencesDataStore +import java.math.BigInteger import java.net.HttpURLConnection import java.net.URL +import java.text.SimpleDateFormat +import java.util.Date +import java.util.Locale +import java.util.TimeZone import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first @@ -23,6 +28,13 @@ import kotlinx.serialization.Transient import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonElement +/** + * Converts a java.util.Date to ISO8601 format string. + * Format: "yyyy-MM-dd'T'HH:mm:ss'Z'" (UTC timezone) + * Example: "2025-12-31T23:59:59Z" + */ +private fun Date.toISO8601String(): String = this.toInstant().toString() // Returns "2025-12-31T23:59:59Z" + val Context.prefDataStore: DataStore by preferencesDataStore(name = "pref_datastore") /** @@ -41,6 +53,9 @@ interface CertificateApiClient { status: UpdateCertificateStatusStatus, operationType: UpdateCertificateStatusOperation, detail: String? = null, + notAfter: Date? = null, + notBefore: Date? = null, + serialNumber: BigInteger? = null, ): Result } @@ -253,6 +268,9 @@ object ApiClient : CertificateApiClient { status: UpdateCertificateStatusStatus, operationType: UpdateCertificateStatusOperation, detail: String?, + notAfter: Date?, + notBefore: Date?, + serialNumber: BigInteger?, ): Result = makeRequest( endpoint = "/api/fleetd/certificates/$certificateId/status", method = "PUT", @@ -260,6 +278,9 @@ object ApiClient : CertificateApiClient { status = status, operationType = operationType, detail = detail, + notAfter = notAfter?.toISO8601String(), + notBefore = notBefore?.toISO8601String(), + serialNumber = serialNumber?.toString(), ), bodySerializer = UpdateCertificateStatusRequest.serializer(), responseSerializer = UpdateCertificateStatusResponse.serializer(), @@ -442,6 +463,12 @@ data class UpdateCertificateStatusRequest( val operationType: UpdateCertificateStatusOperation, @SerialName("detail") val detail: String? = null, + @SerialName("not_valid_after") + val notAfter: String? = null, + @SerialName("not_valid_before") + val notBefore: String? = null, + @SerialName("serial") + val serialNumber: String? = null, ) @Serializable diff --git a/android/app/src/main/java/com/fleetdm/agent/CertificateEnrollmentHandler.kt b/android/app/src/main/java/com/fleetdm/agent/CertificateEnrollmentHandler.kt index 2e4aa31d70..1cd5f83305 100644 --- a/android/app/src/main/java/com/fleetdm/agent/CertificateEnrollmentHandler.kt +++ b/android/app/src/main/java/com/fleetdm/agent/CertificateEnrollmentHandler.kt @@ -6,8 +6,10 @@ import com.fleetdm.agent.scep.ScepCsrException import com.fleetdm.agent.scep.ScepEnrollmentException import com.fleetdm.agent.scep.ScepKeyGenerationException import com.fleetdm.agent.scep.ScepNetworkException +import java.math.BigInteger import java.security.PrivateKey import java.security.cert.Certificate +import java.util.Date /** * Handles certificate enrollment business logic without Android framework dependencies. @@ -27,7 +29,7 @@ class CertificateEnrollmentHandler(private val scepClient: ScepClient, private v * Result of enrollment operation. */ sealed class EnrollmentResult { - data class Success(val alias: String) : EnrollmentResult() + data class Success(val alias: String, val notAfter: Date?, val notBefore: Date?, val serialNumber: BigInteger?) : EnrollmentResult() data class Failure(val reason: String, val exception: Exception? = null, val isRetryable: Boolean = false) : EnrollmentResult() data class PermanentlyFailed(val alias: String) : EnrollmentResult() } @@ -47,7 +49,12 @@ class CertificateEnrollmentHandler(private val scepClient: ScepClient, private v ) if (installed) { - EnrollmentResult.Success(config.name) + EnrollmentResult.Success( + alias = config.name, + notAfter = result.notAfter, + notBefore = result.notBefore, + serialNumber = result.serialNumber, + ) } else { EnrollmentResult.Failure("Certificate installation failed") } 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 cd2a562641..384e8a8521 100644 --- a/android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt +++ b/android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt @@ -8,8 +8,14 @@ import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.stringPreferencesKey import com.fleetdm.agent.scep.ScepClient import com.fleetdm.agent.scep.ScepClientImpl +import java.math.BigInteger import java.security.PrivateKey import java.security.cert.Certificate +import java.text.SimpleDateFormat +import java.time.Instant +import java.util.Date +import java.util.Locale +import java.util.TimeZone import kotlinx.coroutines.async import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow @@ -55,6 +61,24 @@ class CertificateOrchestrator( explicitNulls = false } + /** + * Converts a java.util.Date to ISO8601 format string. + * Format: "yyyy-MM-dd'T'HH:mm:ss'Z'" (UTC timezone) + * Example: "2025-12-31T23:59:59Z" + */ + private fun Date.toISO8601String(): String { + val dateFormat = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.US) + dateFormat.timeZone = TimeZone.getTimeZone("UTC") + return dateFormat.format(this) + } + + /** + * Parses an ISO8601 format string to java.util.Date. + * Format: "yyyy-MM-dd'T'HH:mm:ss'Z'" (UTC timezone) + * @throws : DateTimeParseException if the date string cannot be parsed + */ + private fun parseISO8601(dateString: String): Date = Date.from(Instant.parse(dateString)) + // Mutex to protect concurrent access to certificate storage private val certificateStorageMutex = Mutex() @@ -322,7 +346,10 @@ class CertificateOrchestrator( val results = mutableMapOf() // Step 1: Process certificates with operation="remove" - val certificatesToRemove = hostCertificates.filter { it.shouldRemove() } + // Note: UUID mismatches are now handled by enrollment (install-over), not cleanup + val certificatesToRemove = hostCertificates.filter { + it.shouldRemove() + } Log.d(TAG, "Certificates marked for removal: ${certificatesToRemove.map { it.id }}") for (hostCert in certificatesToRemove) { @@ -496,13 +523,30 @@ class CertificateOrchestrator( * @param alias Certificate alias * @param isInstall True for install operation, false for remove operation */ - internal suspend fun markCertificateUnreported(context: Context, certificateId: Int, alias: String, uuid: String, isInstall: Boolean) { + internal suspend fun markCertificateUnreported( + context: Context, + certificateId: Int, + alias: String, + uuid: String, + isInstall: Boolean, + notAfter: String? = null, + notBefore: String? = null, + serialNumber: String? = null, + ) { val status = if (isInstall) { CertificateStatus.INSTALLED_UNREPORTED } else { CertificateStatus.REMOVED_UNREPORTED } - val info = CertificateState(alias = alias, status = status, statusReportRetries = 0, uuid = uuid) + val info = CertificateState( + alias = alias, + status = status, + statusReportRetries = 0, + uuid = uuid, + notAfter = notAfter, + notBefore = notBefore, + serialNumber = serialNumber, + ) storeCertificateState(context, certificateId, info) } @@ -570,6 +614,9 @@ class CertificateOrchestrator( certificateId = certId, status = UpdateCertificateStatusStatus.VERIFIED, operationType = operationType, + notAfter = state.notAfter?.let { parseISO8601(it) }, + notBefore = state.notBefore?.let { parseISO8601(it) }, + serialNumber = state.serialNumber?.let { BigInteger(it) }, ) if (result.isSuccess) { @@ -623,10 +670,15 @@ class CertificateOrchestrator( TAG, "Certificate ID $certificateId (alias: '${storedState.alias}', uuid: $uuid) is already installed, skipping enrollment", ) - return CertificateEnrollmentHandler.EnrollmentResult.Success(storedState.alias) + return CertificateEnrollmentHandler.EnrollmentResult.Success( + alias = storedState.alias, + notAfter = null, + notBefore = null, + serialNumber = null, + ) } if (existsInKeystore && storedState.uuid != uuid) { - Log.i(TAG, "Certificate ID $certificateId uuid changed (${storedState.uuid} -> $uuid), will reinstall") + Log.i(TAG, "Certificate ID $certificateId uuid changed (${storedState.uuid} -> $uuid), will install over existing") } } @@ -653,7 +705,12 @@ class CertificateOrchestrator( // The certificate template hasn't failed on the device, but isn't ready to be processed yet. // Retry next time we fetch but don't mark as failed locally Log.i(TAG, "Certificate template ${template.name} does not have status \"delivered\": status \"${template.status}\"") - return CertificateEnrollmentHandler.EnrollmentResult.Success(template.name) + return CertificateEnrollmentHandler.EnrollmentResult.Success( + alias = template.name, + notAfter = null, + notBefore = null, + serialNumber = null, + ) } // Step 3: Create certificate installer (use provided or create default) @@ -673,14 +730,31 @@ class CertificateOrchestrator( is CertificateEnrollmentHandler.EnrollmentResult.Success -> { Log.i(TAG, "Certificate enrollment successful for ID $certificateId with alias: ${result.alias}") + // Convert certificate metadata to ISO8601 for storage + val notAfterStr = result.notAfter?.toISO8601String() + val notBeforeStr = result.notBefore?.toISO8601String() + val serialNumberStr = result.serialNumber?.toString() + // First, mark as unreported (persisted before network call) - markCertificateUnreported(context, certificateId, template.name, uuid = uuid, isInstall = true) + markCertificateUnreported( + context, + certificateId, + template.name, + uuid = uuid, + isInstall = true, + notAfter = notAfterStr, + notBefore = notBeforeStr, + serialNumber = serialNumberStr, + ) // Attempt to report status val reportResult = apiClient.updateCertificateStatus( certificateId = certificateId, status = UpdateCertificateStatusStatus.VERIFIED, operationType = UpdateCertificateStatusOperation.INSTALL, + notAfter = result.notAfter, + notBefore = result.notBefore, + serialNumber = result.serialNumber, ) if (reportResult.isSuccess) { @@ -810,6 +884,12 @@ data class CertificateState( val statusReportRetries: Int = 0, @SerialName("uuid") val uuid: String = "", + @SerialName("not_after") + val notAfter: String? = null, + @SerialName("not_before") + val notBefore: String? = null, + @SerialName("serial_number") + val serialNumber: String? = null, ) { fun shouldRetry(): Boolean = status == CertificateStatus.RETRY && retries < (MAX_CERT_INSTALL_RETRIES) fun shouldRetryStatusReport(): Boolean = statusReportRetries < MAX_STATUS_REPORT_RETRIES diff --git a/android/app/src/main/java/com/fleetdm/agent/MainActivity.kt b/android/app/src/main/java/com/fleetdm/agent/MainActivity.kt index bf37bbd5f1..db5d3cb650 100644 --- a/android/app/src/main/java/com/fleetdm/agent/MainActivity.kt +++ b/android/app/src/main/java/com/fleetdm/agent/MainActivity.kt @@ -234,6 +234,10 @@ fun DebugCertificateList(certificates: CertificateStateMap) { Text(text = "alias: ${value.alias}") Text(text = "status: ${value.status}") Text(text = "retries: ${value.retries}") + Text(text = "uuid: ${value.uuid}") + Text(text = "notBefore: ${value.notBefore}") + Text(text = "notAfter: ${value.notAfter}") + Text(text = "serial: ${value.serialNumber}") } } } diff --git a/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt b/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt index adc6fe2bd7..515360b7de 100644 --- a/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt +++ b/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt @@ -97,9 +97,18 @@ class ScepClientImpl : ScepClient { throw ScepCertificateException("No certificates returned from SCEP server") } + val leafCertificate = (certificates.first() as java.security.cert.X509Certificate) + // Extract certificate metadata from the leaf certificate + val notAfter = leafCertificate.notAfter + val notBefore = leafCertificate.notBefore + val serialNumber = leafCertificate.serialNumber + ScepResult( privateKey = keyPair.private, certificateChain = certificates, + notAfter = notAfter, + notBefore = notBefore, + serialNumber = serialNumber, ) } response.isPending -> { diff --git a/android/app/src/main/java/com/fleetdm/agent/scep/ScepResult.kt b/android/app/src/main/java/com/fleetdm/agent/scep/ScepResult.kt index a0bf73efab..7dcd0e367c 100644 --- a/android/app/src/main/java/com/fleetdm/agent/scep/ScepResult.kt +++ b/android/app/src/main/java/com/fleetdm/agent/scep/ScepResult.kt @@ -1,12 +1,23 @@ package com.fleetdm.agent.scep +import java.math.BigInteger import java.security.PrivateKey import java.security.cert.Certificate +import java.util.Date /** * Result of a successful SCEP enrollment containing the private key and certificate chain. * * @property privateKey The generated private key * @property certificateChain The certificate chain from the SCEP server (leaf certificate first) + * @property notAfter The expiration date (notAfter) of the leaf certificate + * @property notBefore The effective date (notBefore) of the leaf certificate + * @property serialNumber The serial number of the leaf certificate */ -data class ScepResult(val privateKey: PrivateKey, val certificateChain: List) +data class ScepResult( + val privateKey: PrivateKey, + val certificateChain: List, + val notAfter: Date, + val notBefore: Date, + val serialNumber: BigInteger, +) 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 2fd058f6eb..05f81cc2c4 100644 --- a/android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt +++ b/android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt @@ -284,7 +284,7 @@ class CertificateOrchestratorTest { expectApiCall = false, ), TestCase( - name = "reinstalls when uuid changes", + name = "installs over when uuid changes", initialStatus = CertificateStatus.INSTALLED, inKeystore = true, storedUuid = "uuid-1", @@ -800,17 +800,6 @@ class CertificateOrchestratorTest { expectedUuid = "uuid-2", expectedStatus = CertificateStatus.REMOVED_UNREPORTED, // API failed, stays unreported for retry ), - TestCase( - name = "stores new uuid when removing INSTALLED cert and API fails", - storedStatus = CertificateStatus.INSTALLED, - storedUuid = "uuid-1", - requestedUuid = "uuid-2", - inKeystore = true, - apiShouldFail = true, - expectApiCall = true, - expectedUuid = "uuid-2", // Must store NEW uuid, not old one - expectedStatus = CertificateStatus.REMOVED_UNREPORTED, - ), ) for (case in testCases) { @@ -924,8 +913,11 @@ class CertificateOrchestratorTest { val cleanupResults = orchestrator.cleanupRemovedCertificates(context, hostCertificates) // Assert: Cleanup + // Note: Cleanup now processes only certificate 14 (operation="remove") + // Certificate 20 is NOT processed (no longer marked for removal due to null UUID check removed) assertEquals(1, cleanupResults.size) assertTrue(cleanupResults[14] is CleanupResult.Success) + assertNull("Certificate 20 should not be in cleanup results", cleanupResults[20]) assertFalse("Old cert should be removed from keystore", fakeDeviceKeystoreManager.hasKeyPair("cert-1")) // Act: Enrollment installs new cert @@ -952,6 +944,115 @@ class CertificateOrchestratorTest { assertEquals("new-uuid", stored[20]?.uuid) } + @Test + fun `uuid change - same certificate installed over when uuid changes`() = runTest { + // Scenario: Server changes UUID for same certificate ID (e.g., certificate renewal). + // Certificate ID 123 is already installed with old-uuid. + // Server sends certificate ID 123 with new-uuid. + // Expected: Certificate installed directly over existing (no removal step). + + // Arrange: Certificate installed with old UUID + val certificateId = 123 + val alias = "cert-1" + val oldUuid = "old-uuid" + val newUuid = "new-uuid" + + storeTestCertificateInDataStore( + certificateId = certificateId, + alias = alias, + status = CertificateStatus.INSTALLED, + uuid = oldUuid, + ) + fakeDeviceKeystoreManager.installCert(alias) + + // Configure API to return certificate template + fakeApiClient.getCertificateTemplateHandler = { certId -> + if (certId == certificateId) { + Result.success( + CertificateTemplateResult( + template = GetCertificateTemplateResponse( + id = certificateId, + name = alias, + certificateAuthorityId = 1, + certificateAuthorityName = "TestCA", + createdAt = "2025-01-01T00:00:00Z", + subjectName = "CN=test", + certificateAuthorityType = "custom_scep_proxy", + status = "delivered", + ), + scepUrl = TestCertificateTemplateFactory.DEFAULT_SCEP_URL, + ), + ) + } else { + Result.failure(Exception("Unexpected cert ID: $certId")) + } + } + mockInstaller.shouldSucceed = true + + // Host certificate with new UUID and install operation + val hostCertificates = listOf( + HostCertificate( + id = certificateId, + status = "delivered", + operation = "install", + uuid = newUuid, + ), + ) + + // Act: Cleanup should NOT process certificate with UUID mismatch (only operation="remove") + val cleanupResults = orchestrator.cleanupRemovedCertificates(context, hostCertificates) + + // Assert: Cleanup should be empty (certificate not marked for removal) + assertEquals(0, cleanupResults.size) + assertTrue("Certificate should still exist in keystore", fakeDeviceKeystoreManager.hasKeyPair(alias)) + + // Assert: State unchanged after cleanup (still INSTALLED with old UUID) + val afterCleanup = getStoredCertificates() + assertEquals(1, afterCleanup.size) + assertEquals(CertificateStatus.INSTALLED, afterCleanup[certificateId]?.status) + assertEquals(alias, afterCleanup[certificateId]?.alias) + assertEquals(oldUuid, afterCleanup[certificateId]?.uuid) // UUID not updated during cleanup + + // Act: Enrollment detects UUID change and installs over existing + val installCerts = hostCertificates.filter { it.shouldInstall() } + val enrollResults = orchestrator.enrollCertificates(context, installCerts, mockInstaller) + + // Assert: Certificate successfully installed (overwrote old one) + assertEquals(1, enrollResults.size) + assertTrue( + "Expected enrollment success but got: ${enrollResults[certificateId]}", + enrollResults[certificateId] is CertificateEnrollmentHandler.EnrollmentResult.Success, + ) + assertTrue("Installer should have been called", mockInstaller.wasInstallCalled) + assertEquals(alias, mockInstaller.capturedAlias) + + // Assert: Final state is INSTALLED with new UUID (no REMOVED intermediate state) + val afterEnrollment = getStoredCertificates() + assertEquals(1, afterEnrollment.size) + assertEquals(CertificateStatus.INSTALLED, afterEnrollment[certificateId]?.status) + assertEquals(alias, afterEnrollment[certificateId]?.alias) + assertEquals(newUuid, afterEnrollment[certificateId]?.uuid) // UUID updated via install-over + + // Assert: API calls made correctly + val updateCalls = fakeApiClient.updateStatusCalls + + // 1. NO removal status should be reported (UUID mismatch doesn't trigger removal) + val removalCall = updateCalls.find { + it.certificateId == certificateId && it.operationType == UpdateCertificateStatusOperation.REMOVE + } + assertNull("No removal status should be reported", removalCall) + + // 2. Certificate template fetched during enrollment + assertTrue("Certificate template should be fetched", fakeApiClient.getCertificateTemplateCalls.contains(certificateId)) + + // 3. Installation status reported during enrollment + val installCall = updateCalls.find { + it.certificateId == certificateId && it.operationType == UpdateCertificateStatusOperation.INSTALL + } + assertNotNull("Installation status should be reported to server", installCall) + assertEquals(UpdateCertificateStatusStatus.VERIFIED, installCall?.status) + } + // ========== Test category: Status report retry logic ========== @Test 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 a1f92cc118..ab4021610c 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 @@ -60,9 +60,18 @@ class MockScepClient : ScepClient { val cert = generateSelfSignedCertificate(keyPair, subject) + // Extract certificate metadata from generated certificate + val x509Cert = cert as X509Certificate + val notAfter = x509Cert.notAfter + val notBefore = x509Cert.notBefore + val serialNumber = x509Cert.serialNumber + return ScepResult( privateKey = keyPair.private, certificateChain = listOf(cert), + notAfter = notAfter, + notBefore = notBefore, + serialNumber = serialNumber, ) } diff --git a/android/app/src/test/java/com/fleetdm/agent/testutil/FakeCertificateApiClient.kt b/android/app/src/test/java/com/fleetdm/agent/testutil/FakeCertificateApiClient.kt index fe7141b2a8..d8bcf9bdcf 100644 --- a/android/app/src/test/java/com/fleetdm/agent/testutil/FakeCertificateApiClient.kt +++ b/android/app/src/test/java/com/fleetdm/agent/testutil/FakeCertificateApiClient.kt @@ -4,6 +4,8 @@ import com.fleetdm.agent.CertificateApiClient import com.fleetdm.agent.CertificateTemplateResult import com.fleetdm.agent.UpdateCertificateStatusOperation import com.fleetdm.agent.UpdateCertificateStatusStatus +import java.math.BigInteger +import java.util.Date /** * Represents a captured call to updateCertificateStatus for test assertions. @@ -13,6 +15,9 @@ data class UpdateStatusCall( val status: UpdateCertificateStatusStatus, val operationType: UpdateCertificateStatusOperation, val detail: String?, + val notAfter: Date?, + val notBefore: Date?, + val serialNumber: BigInteger?, ) /** @@ -41,8 +46,19 @@ class FakeCertificateApiClient : CertificateApiClient { status: UpdateCertificateStatusStatus, operationType: UpdateCertificateStatusOperation, detail: String?, + notAfter: Date?, + notBefore: Date?, + serialNumber: BigInteger?, ): Result { - val call = UpdateStatusCall(certificateId, status, operationType, detail) + val call = UpdateStatusCall( + certificateId, + status, + operationType, + detail, + notAfter, + notBefore, + serialNumber, + ) _updateStatusCalls.add(call) return updateCertificateStatusHandler(call) }