From 9e6ebdc249a5205b06170e6147aa64f97cb005f7 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Thu, 14 May 2026 16:43:17 -0500 Subject: [PATCH] Added subject alternative name (SAN) support to SCEP enrollment on Android agent (#44968) **Related issue:** Resolves #44967 Video demo: https://www.youtube.com/watch?v=AnwAXPS9Ys0 # 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. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **New Features** * Android SCEP enrollment now supports Subject Alternative Name (SAN) attributes on certificate templates (DNS, email, URI, IP, and Microsoft UPN); SAN is optional and forwarded when present. * **Tests** * Added unit and integration tests for SAN parsing, CSR generation, and end-to-end enrollment verification. * **Documentation** * Added a change note describing SAN support and CSR behavior. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/44968) --- .../main/java/com/fleetdm/agent/ApiClient.kt | 3 + .../com/fleetdm/agent/scep/ScepClientImpl.kt | 38 ++- .../scep/SubjectAlternativeNameParser.kt | 88 ++++++ .../agent/CertificateEnrollmentHandlerTest.kt | 17 ++ .../fleetdm/agent/scep/ScepClientImplTest.kt | 69 ++++- .../fleetdm/agent/scep/ScepIntegrationTest.kt | 75 +++++ .../scep/SubjectAlternativeNameParserTest.kt | 261 ++++++++++++++++++ .../TestCertificateTemplateFactory.kt | 2 + .../changes/41472-android-cert-san-attributes | 1 + 9 files changed, 549 insertions(+), 5 deletions(-) create mode 100644 android/app/src/main/java/com/fleetdm/agent/scep/SubjectAlternativeNameParser.kt create mode 100644 android/app/src/test/java/com/fleetdm/agent/scep/SubjectAlternativeNameParserTest.kt create mode 100644 android/changes/41472-android-cert-san-attributes 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 b42228147c..8f62148f41 100644 --- a/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt +++ b/android/app/src/main/java/com/fleetdm/agent/ApiClient.kt @@ -606,6 +606,9 @@ data class GetCertificateTemplateResponse( @SerialName("subject_name") val subjectName: String, + @SerialName("subject_alternative_name") + val subjectAlternativeName: String? = null, + @SerialName("certificate_authority_type") val certificateAuthorityType: String, 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 5af744e943..14651ae087 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 @@ -4,6 +4,8 @@ import com.fleetdm.agent.GetCertificateTemplateResponse import org.bouncycastle.asn1.DERPrintableString import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers import org.bouncycastle.asn1.x500.X500Name +import org.bouncycastle.asn1.x509.Extension +import org.bouncycastle.asn1.x509.ExtensionsGenerator import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder import org.bouncycastle.jce.provider.BouncyCastleProvider @@ -33,7 +35,10 @@ import kotlinx.coroutines.withContext class ScepClientImpl : ScepClient { companion object { - private const val SCEP_PROFILE = "NDESCA" // Network Device Enrollment Service CA + // SCEP `message=` value (CA identifier) sent on GetCACaps and GetCACert. Per RFC 8894 + // this is OPTIONAL and only meaningful when the endpoint represents multiple CAs. + // Sending null causes jScep to omit the parameter; the server returns its default CA. + private val SCEP_PROFILE: String? = null private const val SELF_SIGNED_CERT_VALIDITY_DAYS = 100L init { @@ -79,7 +84,13 @@ class ScepClientImpl : ScepClient { val client = Client(server, verifier) // Step 5: Build Certificate Signing Request (CSR) - val csr = buildCsr(entity, keyPair, config.scepChallenge ?: "", config.signatureAlgorithm) + val csr = buildCsr( + entity, + keyPair, + config.scepChallenge ?: "", + config.signatureAlgorithm, + config.subjectAlternativeName, + ) // Step 6: Send enrollment request val response = try { @@ -165,15 +176,36 @@ class ScepClientImpl : ScepClient { throw ScepCertificateException("Failed to create self-signed certificate", e) } - private fun buildCsr(entity: X500Name, keyPair: java.security.KeyPair, challenge: String, signatureAlgorithm: String) = try { + internal fun buildCsr( + entity: X500Name, + keyPair: java.security.KeyPair, + challenge: String, + signatureAlgorithm: String, + subjectAlternativeName: String?, + ) = try { val csrBuilder = JcaPKCS10CertificationRequestBuilder(entity, keyPair.public) // Add challenge password attribute val passwordAttr = DERPrintableString(challenge) csrBuilder.addAttribute(PKCSObjectIdentifiers.pkcs_9_at_challengePassword, passwordAttr) + // Add Subject Alternative Name extension if a non-empty SAN string was provided. + val generalNames = try { + SubjectAlternativeNameParser.parse(subjectAlternativeName) + } catch (e: IllegalArgumentException) { + throw ScepCsrException("Invalid subject alternative name: ${e.message}", e) + } + if (generalNames != null) { + val extensions = ExtensionsGenerator().apply { + addExtension(Extension.subjectAlternativeName, false, generalNames) + }.generate() + csrBuilder.addAttribute(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest, extensions) + } + val contentSigner = JcaContentSignerBuilder(signatureAlgorithm).build(keyPair.private) csrBuilder.build(contentSigner) + } catch (e: ScepCsrException) { + throw e } catch (e: Exception) { throw ScepCsrException("Failed to build Certificate Signing Request", e) } diff --git a/android/app/src/main/java/com/fleetdm/agent/scep/SubjectAlternativeNameParser.kt b/android/app/src/main/java/com/fleetdm/agent/scep/SubjectAlternativeNameParser.kt new file mode 100644 index 0000000000..7e7db88143 --- /dev/null +++ b/android/app/src/main/java/com/fleetdm/agent/scep/SubjectAlternativeNameParser.kt @@ -0,0 +1,88 @@ +package com.fleetdm.agent.scep + +import org.bouncycastle.asn1.ASN1ObjectIdentifier +import org.bouncycastle.asn1.DERIA5String +import org.bouncycastle.asn1.DERSequence +import org.bouncycastle.asn1.DERTaggedObject +import org.bouncycastle.asn1.DERUTF8String +import org.bouncycastle.asn1.x509.GeneralName +import org.bouncycastle.asn1.x509.GeneralNames +import org.bouncycastle.util.IPAddress +import java.util.Locale + +/** + * Parses the comma-separated KEY=value SAN string carried on the certificate template + * (e.g. "DNS=example.com, UPN=user@corp.example.com") into a BouncyCastle GeneralNames + * structure suitable for inclusion in a PKCS#10 CSR's subjectAltName extension. + * + * Supported KEYs (case-insensitive): + * DNS -> dNSName (DERIA5String) + * EMAIL -> rfc822Name (DERIA5String) + * URI -> uniformResourceIdentifier (DERIA5String) + * IP -> iPAddress (DEROctetString, 4 bytes IPv4 or 16 bytes IPv6) + * UPN -> otherName (OID 1.3.6.1.4.1.311.20.2.3, [0] EXPLICIT DERUTF8String) + * per Microsoft KB258605 / RFC 4556 §3.2.1. + * + * Returns null for null / empty / blank input so the caller can skip adding the extension. + * Throws IllegalArgumentException for unknown KEYs, malformed tokens, empty values, or + * unparseable IP literals. + * + * Reserved characters in values: a literal `,` would split the token, so values that need + * one (most commonly URI paths/queries) must be percent-encoded per RFC 3986 — `%2C`. The + * encoded form is preserved verbatim on the issued cert, which is the canonical wire form + * per RFC 5280 §4.2.1.6. This mirrors OpenSSL's `openssl.cnf` SAN syntax. + */ +object SubjectAlternativeNameParser { + + private const val MICROSOFT_UPN_OID = "1.3.6.1.4.1.311.20.2.3" + + fun parse(sanString: String?): GeneralNames? { + if (sanString.isNullOrBlank()) return null + + val names = sanString.split(',') + .map { it.trim() } + .filter { it.isNotEmpty() } + .map { parseToken(it) } + + if (names.isEmpty()) return null + + return GeneralNames(names.toTypedArray()) + } + + private fun parseToken(token: String): GeneralName { + val eqIndex = token.indexOf('=') + require(eqIndex > 0 && eqIndex != token.length - 1) { + "Malformed SAN token: \"$token\" (expected KEY=value)" + } + // Locale.ROOT keeps the comparison locale-independent: a Turkish-locale device + // would otherwise turn "ip" into "İP", which would not match the ASCII "IP" arm + // and would fail enrollment. + val key = token.substring(0, eqIndex).trim().uppercase(Locale.ROOT) + val value = token.substring(eqIndex + 1).trim() + require(value.isNotEmpty()) { "Malformed SAN token: \"$token\" (empty value)" } + return when (key) { + "DNS" -> GeneralName(GeneralName.dNSName, DERIA5String(value)) + "EMAIL" -> GeneralName(GeneralName.rfc822Name, DERIA5String(value)) + "URI" -> GeneralName(GeneralName.uniformResourceIdentifier, DERIA5String(value)) + "IP" -> { + // BouncyCastle's IPAddress.isValid is literal-only (no DNS) and rejects + // bracketed forms, zone IDs, and anything outside dotted-quad / colon-hex. + // GeneralName(iPAddress, String) then encodes the literal to the raw 4- + // or 16-byte octet string the SAN extension requires. + require(IPAddress.isValid(value)) { + "Unparseable IP address: \"$value\" (expected IPv4 dotted-quad or IPv6 colon-hex)" + } + GeneralName(GeneralName.iPAddress, value) + } + "UPN" -> GeneralName(GeneralName.otherName, encodeUpn(value)) + else -> throw IllegalArgumentException("Unknown SAN KEY: \"$key\" (supported: DNS, EMAIL, URI, IP, UPN)") + } + } + + private fun encodeUpn(value: String): DERSequence = DERSequence( + arrayOf( + ASN1ObjectIdentifier(MICROSOFT_UPN_OID), + DERTaggedObject(true, 0, DERUTF8String(value)), + ), + ) +} diff --git a/android/app/src/test/java/com/fleetdm/agent/CertificateEnrollmentHandlerTest.kt b/android/app/src/test/java/com/fleetdm/agent/CertificateEnrollmentHandlerTest.kt index 6b73563267..c0e347a412 100644 --- a/android/app/src/test/java/com/fleetdm/agent/CertificateEnrollmentHandlerTest.kt +++ b/android/app/src/test/java/com/fleetdm/agent/CertificateEnrollmentHandlerTest.kt @@ -7,6 +7,7 @@ import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -165,4 +166,20 @@ class CertificateEnrollmentHandlerTest { assertEquals(2048, mockScepClient.capturedConfig?.keyLength) assertEquals("SHA256withRSA", mockScepClient.capturedConfig?.signatureAlgorithm) } + + @Test + fun `handler forwards subjectAlternativeName to SCEP client`() = runTest { + val san = "DNS=host.example.com, UPN=marko@corp.example.com" + val template = TestCertificateTemplateFactory.create(subjectAlternativeName = san) + handler.handleEnrollment(template, TestCertificateTemplateFactory.DEFAULT_SCEP_URL) + assertEquals(san, mockScepClient.capturedConfig?.subjectAlternativeName) + } + + @Test + fun `handler forwards null subjectAlternativeName when absent`() = runTest { + val template = TestCertificateTemplateFactory.create() + handler.handleEnrollment(template, TestCertificateTemplateFactory.DEFAULT_SCEP_URL) + assertNotNull(mockScepClient.capturedConfig) + assertNull(mockScepClient.capturedConfig?.subjectAlternativeName) + } } diff --git a/android/app/src/test/java/com/fleetdm/agent/scep/ScepClientImplTest.kt b/android/app/src/test/java/com/fleetdm/agent/scep/ScepClientImplTest.kt index f759f191a1..b66a58c30f 100644 --- a/android/app/src/test/java/com/fleetdm/agent/scep/ScepClientImplTest.kt +++ b/android/app/src/test/java/com/fleetdm/agent/scep/ScepClientImplTest.kt @@ -1,10 +1,22 @@ package com.fleetdm.agent.scep import com.fleetdm.agent.testutil.TestCertificateTemplateFactory +import org.bouncycastle.asn1.DERIA5String +import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers +import org.bouncycastle.asn1.x500.X500Name +import org.bouncycastle.asn1.x509.Extension +import org.bouncycastle.asn1.x509.Extensions +import org.bouncycastle.asn1.x509.GeneralName +import org.bouncycastle.asn1.x509.GeneralNames +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Assert.fail import org.junit.Before import org.junit.Test +import java.security.KeyPairGenerator import kotlinx.coroutines.test.runTest /** @@ -60,6 +72,59 @@ class ScepClientImplTest { } } - // Note: Testing successful enrollment requires a mock SCEP server or extensive mocking - // of jScep's Client class. Integration tests should be used for this scenario. + @Test + fun `buildCsr omits SAN extension when SAN string is null or blank`() { + listOf(null, "", " ").forEach { input -> + val csr = buildTestCsr(subjectAlternativeName = input) + assertNull( + "Expected no SAN extension for input \"$input\"", + extractSanExtension(csr), + ) + } + } + + @Test + fun `buildCsr emits a non-critical SAN extension when SAN string is present`() { + // Per-KEY encoding correctness is covered by SubjectAlternativeNameParserTest. + val csr = buildTestCsr(subjectAlternativeName = "DNS=example.com") + val ext = extractSanExtension(csr) + assertNotNull(ext) + assertFalse("SAN extension must be non-critical", ext!!.isCritical) + val names = GeneralNames.getInstance(ext.parsedValue).names + assertEquals(1, names.size) + assertEquals(GeneralName.dNSName, names[0].tagNo) + assertEquals("example.com", (names[0].name as DERIA5String).string) + } + + @Test + fun `buildCsr wraps parser exceptions as ScepCsrException`() { + try { + buildTestCsr(subjectAlternativeName = "FOO=bar") + fail("Expected ScepCsrException") + } catch (e: ScepCsrException) { + assertTrue( + "Expected wrapper message to mention SAN; got: ${e.message}", + e.message!!.contains("subject alternative name"), + ) + } + } + + private fun buildTestCsr(subjectAlternativeName: String?) = scepClient.buildCsr( + entity = X500Name("CN=Test,O=FleetDM"), + keyPair = KeyPairGenerator.getInstance("RSA").apply { initialize(2048) }.genKeyPair(), + challenge = "test-challenge", + signatureAlgorithm = "SHA256withRSA", + subjectAlternativeName = subjectAlternativeName, + ) + + /** + * Pulls the subjectAltName Extension out of a PKCS#10 CSR's extensionRequest attribute. + * Returns null if there is no extensionRequest attribute or no SAN extension inside it. + */ + private fun extractSanExtension(csr: org.bouncycastle.pkcs.PKCS10CertificationRequest): Extension? { + val attributes = csr.getAttributes(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest) + if (attributes.isEmpty()) return null + val extensions = Extensions.getInstance(attributes[0].attrValues.getObjectAt(0)) + return extensions.getExtension(Extension.subjectAlternativeName) + } } diff --git a/android/app/src/test/java/com/fleetdm/agent/scep/ScepIntegrationTest.kt b/android/app/src/test/java/com/fleetdm/agent/scep/ScepIntegrationTest.kt index ae8d7c48d6..5125fb3cca 100644 --- a/android/app/src/test/java/com/fleetdm/agent/scep/ScepIntegrationTest.kt +++ b/android/app/src/test/java/com/fleetdm/agent/scep/ScepIntegrationTest.kt @@ -4,6 +4,12 @@ import com.fleetdm.agent.GetCertificateTemplateResponse import com.fleetdm.agent.IntegrationTest import com.fleetdm.agent.IntegrationTestRule import com.fleetdm.agent.testutil.TestCertificateTemplateFactory +import org.bouncycastle.asn1.ASN1OctetString +import org.bouncycastle.asn1.DERIA5String +import org.bouncycastle.asn1.DEROctetString +import org.bouncycastle.asn1.x509.Extension +import org.bouncycastle.asn1.x509.GeneralName +import org.bouncycastle.asn1.x509.GeneralNames import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue @@ -125,6 +131,75 @@ class ScepIntegrationTest { assertTrue("Enrollment should complete within 30 seconds (took ${duration}ms)", duration < 30000) } + @IntegrationTest + @Test + fun `SAN entries on the certificate template appear on the issued certificate`() = runTest { + // End-to-end check that the SAN extension we put on the CSR is accepted by the + // SCEP CA and copied verbatim to the issued certificate. + // + // UPN (otherName) is intentionally not asserted here: the reference CA used in + // CI (micromdm/scep) only copies DNS/Email/IP/URI from the CSR to the issued + // cert via the typed fields on Go's x509.Certificate, and drops otherName. + val uniqueId = System.currentTimeMillis() + val expectedDns = listOf("a-$uniqueId.example.com", "b-$uniqueId.example.com") + val expectedEmail = listOf("a-$uniqueId@example.com", "b-$uniqueId@example.com") + val expectedUri = listOf( + "spiffe://example.org/a-$uniqueId", + "spiffe://example.org/b-$uniqueId", + ) + val expectedIpStrings = listOf("10.0.0.1", "10.0.0.2") + val expectedIpOctets = listOf(byteArrayOf(10, 0, 0, 1), byteArrayOf(10, 0, 0, 2)) + + val sanString = listOf( + "DNS=${expectedDns[0]}", + "DNS=${expectedDns[1]}", + "EMAIL=${expectedEmail[0]}", + "EMAIL=${expectedEmail[1]}", + "URI=${expectedUri[0]}", + "URI=${expectedUri[1]}", + "IP=${expectedIpStrings[0]}", + "IP=${expectedIpStrings[1]}", + ).joinToString(", ") + + val template = testTemplate.copy( + name = "san-test-cert-$uniqueId", + subjectName = "CN=SanIntegrationTest-$uniqueId,O=FleetDM,C=US", + subjectAlternativeName = sanString, + ) + + val result = scepClient.enroll(template, testScepUrl) + val leafCert = result.certificateChain[0] as java.security.cert.X509Certificate + + // Pull the SAN extension off the issued cert and parse via BouncyCastle. The + // cert's getExtensionValue returns the OCTET STRING wrapper, not the SAN + // contents directly, so we unwrap once before handing to GeneralNames. + val sanBytes = leafCert.getExtensionValue(Extension.subjectAlternativeName.id) + assertNotNull("Issued certificate has no SAN extension", sanBytes) + val sanContents = ASN1OctetString.getInstance(sanBytes).octets + val sanNames = GeneralNames.getInstance(sanContents).names + + fun ia5ValuesForTag(tag: Int): List = sanNames + .filter { it.tagNo == tag } + .map { (it.name as DERIA5String).string } + + fun ipOctetsHex(): List = sanNames + .filter { it.tagNo == GeneralName.iPAddress } + .map { (it.name as DEROctetString).octets.joinToString("") { b -> "%02x".format(b) } } + + // Order is not pinned: some CAs canonicalize SAN ordering during signing. + // Sort each side and compare to verify both entries of each type round-trip. + assertEquals(expectedDns.sorted(), ia5ValuesForTag(GeneralName.dNSName).sorted()) + assertEquals(expectedEmail.sorted(), ia5ValuesForTag(GeneralName.rfc822Name).sorted()) + assertEquals( + expectedUri.sorted(), + ia5ValuesForTag(GeneralName.uniformResourceIdentifier).sorted(), + ) + assertEquals( + expectedIpOctets.map { it.joinToString("") { b -> "%02x".format(b) } }.sorted(), + ipOctetsHex().sorted(), + ) + } + @Test fun `enrollment with unreachable server fails quickly`() = runTest { val unreachableUrl = "https://unreachable-scep-server.invalid/scep" diff --git a/android/app/src/test/java/com/fleetdm/agent/scep/SubjectAlternativeNameParserTest.kt b/android/app/src/test/java/com/fleetdm/agent/scep/SubjectAlternativeNameParserTest.kt new file mode 100644 index 0000000000..bc6154feae --- /dev/null +++ b/android/app/src/test/java/com/fleetdm/agent/scep/SubjectAlternativeNameParserTest.kt @@ -0,0 +1,261 @@ +package com.fleetdm.agent.scep + +import org.bouncycastle.asn1.ASN1ObjectIdentifier +import org.bouncycastle.asn1.ASN1Sequence +import org.bouncycastle.asn1.ASN1TaggedObject +import org.bouncycastle.asn1.DERIA5String +import org.bouncycastle.asn1.DEROctetString +import org.bouncycastle.asn1.DERUTF8String +import org.bouncycastle.asn1.x509.GeneralName +import org.bouncycastle.asn1.x509.GeneralNames +import org.junit.After +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test +import java.util.Locale + +class SubjectAlternativeNameParserTest { + + private val originalLocale = Locale.getDefault() + + @After + fun restoreLocale() { + Locale.setDefault(originalLocale) + } + + // -- helpers --------------------------------------------------------- + + private fun parseOrFail(input: String): GeneralNames { + val result = SubjectAlternativeNameParser.parse(input) + assertNotNull("Expected non-null GeneralNames for input: \"$input\"", result) + return result!! + } + + private fun parseSingle(input: String): GeneralName { + val names = parseOrFail(input).names + assertEquals("Expected exactly one entry for input: \"$input\"", 1, names.size) + return names[0] + } + + private fun assertRejects(input: String, expectedSubstring: String) { + try { + SubjectAlternativeNameParser.parse(input) + fail("Expected IllegalArgumentException for input: \"$input\"") + } catch (e: IllegalArgumentException) { + val msg = e.message ?: "" + assertTrue( + "Expected message to contain \"$expectedSubstring\" for input \"$input\", got: $msg", + msg.contains(expectedSubstring), + ) + } + } + + private fun upnUtf8Value(name: GeneralName): String { + assertEquals(GeneralName.otherName, name.tagNo) + val seq = name.name as ASN1Sequence + val tagged = seq.getObjectAt(1) as ASN1TaggedObject + // getExplicitBaseObject throws if the tag is implicit, so the multi-entry tests + // that call this helper would catch a regression that emits implicit-tagged UPN + // OtherName values without needing a separate isExplicit assertion per call. + return (tagged.getExplicitBaseObject() as DERUTF8String).string + } + + // -- null and blank inputs ------------------------------------------- + + @Test + fun `null and blank inputs return null`() { + listOf(null, "", " ", " \t\n ").forEach { input -> + assertNull( + "Expected null for input \"$input\"", + SubjectAlternativeNameParser.parse(input), + ) + } + } + + @Test + fun `non-blank input that contains only empty tokens returns null`() { + // Distinct from blank input: this string is non-blank, but every comma-separated + // token is empty after trim, so no GeneralNames entries are produced. + assertNull(SubjectAlternativeNameParser.parse(", , , ")) + } + + // -- per-KEY positive encoding --------------------------------------- + + @Test + fun `each IA5String KEY encodes its value verbatim with the right tag`() { + data class Case(val input: String, val expectedTag: Int, val expectedValue: String) + listOf( + Case("DNS=example.com", GeneralName.dNSName, "example.com"), + Case("EMAIL=user@example.com", GeneralName.rfc822Name, "user@example.com"), + Case( + "URI=spiffe://example.org/workload", + GeneralName.uniformResourceIdentifier, + "spiffe://example.org/workload", + ), + ).forEach { case -> + val name = parseSingle(case.input) + assertEquals("Wrong tag for \"${case.input}\"", case.expectedTag, name.tagNo) + assertEquals( + "Wrong value for \"${case.input}\"", + case.expectedValue, + (name.name as DERIA5String).string, + ) + } + } + + @Test + fun `URI value with percent-encoded comma passes through verbatim`() { + val name = parseSingle("URI=https://example.com/a%2Cb?x=1") + assertEquals(GeneralName.uniformResourceIdentifier, name.tagNo) + assertEquals("https://example.com/a%2Cb?x=1", (name.name as DERIA5String).string) + } + + @Test + fun `IP addresses encode to canonical raw octets`() { + // Two cases pin the BC integration: we route IP through BouncyCastle and BC + // produces the RFC 5280 §4.2.1.6 raw octet form (4 bytes for IPv4, 16 for IPv6). + // Exhaustive enumeration of IPv6 forms is BouncyCastle's responsibility, not ours. + data class Case(val input: String, val expected: ByteArray) + listOf( + Case("192.168.1.100", byteArrayOf(192.toByte(), 168.toByte(), 1, 100)), + Case( + "2001:db8::1", + byteArrayOf( + 0x20, 0x01, 0x0d, 0xb8.toByte(), + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, + ), + ), + ).forEach { case -> + val name = parseSingle("IP=${case.input}") + assertEquals("Wrong tag for \"${case.input}\"", GeneralName.iPAddress, name.tagNo) + assertArrayEquals( + "Wrong octets for \"${case.input}\"", + case.expected, + (name.name as DEROctetString).octets, + ) + } + } + + @Test + fun `UPN encodes as Microsoft otherName with EXPLICIT-tagged UTF8 value`() { + val upn = "marko@corp.example.com" + val name = parseSingle("UPN=$upn") + assertEquals(GeneralName.otherName, name.tagNo) + + // OtherName ::= SEQUENCE { type-id OID, value [0] EXPLICIT ANY DEFINED BY type-id } + val otherName = name.name as ASN1Sequence + assertEquals(2, otherName.size()) + assertEquals( + "1.3.6.1.4.1.311.20.2.3", + (otherName.getObjectAt(0) as ASN1ObjectIdentifier).id, + ) + val tagged = otherName.getObjectAt(1) as ASN1TaggedObject + assertEquals(0, tagged.tagNo) + assertTrue("UPN value must be [0] EXPLICIT", tagged.isExplicit) + assertEquals(upn, (tagged.baseObject as DERUTF8String).string) + } + + // -- multi-entry positive tests -------------------------------------- + + @Test + fun `mixed entries preserve type, value, and document order`() { + val san = "DNS=host.example.com, EMAIL=u@example.com, URI=spiffe://x/y, " + + "IP=10.0.0.1, UPN=marko@corp.example.com" + val names = parseOrFail(san).names + assertEquals(5, names.size) + + assertEquals(GeneralName.dNSName, names[0].tagNo) + assertEquals("host.example.com", (names[0].name as DERIA5String).string) + assertEquals(GeneralName.rfc822Name, names[1].tagNo) + assertEquals("u@example.com", (names[1].name as DERIA5String).string) + assertEquals(GeneralName.uniformResourceIdentifier, names[2].tagNo) + assertEquals("spiffe://x/y", (names[2].name as DERIA5String).string) + assertEquals(GeneralName.iPAddress, names[3].tagNo) + assertArrayEquals(byteArrayOf(10, 0, 0, 1), (names[3].name as DEROctetString).octets) + assertEquals("marko@corp.example.com", upnUtf8Value(names[4])) + } + + @Test + fun `repeated keys produce repeated entries in document order with distinct values`() { + val san = "DNS=a.example.com, DNS=b.example.com, EMAIL=u1@x, EMAIL=u2@x, " + + "UPN=u1@corp, UPN=u2@corp, IP=10.0.0.1, IP=10.0.0.2, " + + "URI=spiffe://x/1, URI=spiffe://x/2" + val names = parseOrFail(san).names + assertEquals(10, names.size) + + assertEquals("a.example.com", (names[0].name as DERIA5String).string) + assertEquals("b.example.com", (names[1].name as DERIA5String).string) + assertEquals("u1@x", (names[2].name as DERIA5String).string) + assertEquals("u2@x", (names[3].name as DERIA5String).string) + assertEquals("u1@corp", upnUtf8Value(names[4])) + assertEquals("u2@corp", upnUtf8Value(names[5])) + assertArrayEquals(byteArrayOf(10, 0, 0, 1), (names[6].name as DEROctetString).octets) + assertArrayEquals(byteArrayOf(10, 0, 0, 2), (names[7].name as DEROctetString).octets) + assertEquals("spiffe://x/1", (names[8].name as DERIA5String).string) + assertEquals("spiffe://x/2", (names[9].name as DERIA5String).string) + } + + // -- behavioral / format tolerance ----------------------------------- + + @Test + fun `KEY matching is case-insensitive`() { + val names = parseOrFail("dns=example.com, Email=u@x, uPn=marko@corp").names + assertEquals(3, names.size) + assertEquals(GeneralName.dNSName, names[0].tagNo) + assertEquals(GeneralName.rfc822Name, names[1].tagNo) + assertEquals(GeneralName.otherName, names[2].tagNo) + } + + @Test + fun `KEY matching is locale-insensitive (Turkish dotless-i regression)`() { + // Turkish uppercase rules turn "i" into "İ" under Locale.getDefault(); + // the parser must use Locale.ROOT so "ip" still maps to "IP". + Locale.setDefault(Locale.forLanguageTag("tr-TR")) + val names = parseOrFail("ip=10.0.0.1, dns=host.example.com").names + assertEquals(2, names.size) + assertEquals(GeneralName.iPAddress, names[0].tagNo) + assertEquals(GeneralName.dNSName, names[1].tagNo) + } + + @Test + fun `whitespace around tokens and around equals is tolerated`() { + val names = parseOrFail(" DNS = example.com , EMAIL =u@x ").names + assertEquals(2, names.size) + assertEquals(GeneralName.dNSName, names[0].tagNo) + assertEquals("example.com", (names[0].name as DERIA5String).string) + assertEquals(GeneralName.rfc822Name, names[1].tagNo) + assertEquals("u@x", (names[1].name as DERIA5String).string) + } + + @Test + fun `trailing and embedded empty tokens are skipped`() { + val names = parseOrFail("DNS=example.com, , ,").names + assertEquals(1, names.size) + assertEquals("example.com", (names[0].name as DERIA5String).string) + } + + // -- rejection cases (one table; one assertion shape) ---------------- + + @Test + fun `parser rejects malformed inputs`() { + // Exhaustively testing what BouncyCastle rejects belongs in BC's own tests; we + // keep one representative bad IPv4 and one bad IPv6 to confirm we route IP + // through IPAddress.isValid and surface the right error message. + listOf( + // KEY allow-list violations. + "FOO=bar" to "FOO", + "RFC822=user@example.com" to "RFC822", + // Token shape violations. + "DNS=ok, OOPS" to "OOPS", + "DNS=" to "DNS=", + "=value" to "=value", + // IP value violations: representative bad IPv4 and bad IPv6. + "IP=999.0.0.1" to "999", + "IP=fe80::1%eth0" to "fe80::1%eth0", + ).forEach { (input, substring) -> assertRejects(input, substring) } + } +} diff --git a/android/app/src/test/java/com/fleetdm/agent/testutil/TestCertificateTemplateFactory.kt b/android/app/src/test/java/com/fleetdm/agent/testutil/TestCertificateTemplateFactory.kt index 7390e673bf..dd061374ed 100644 --- a/android/app/src/test/java/com/fleetdm/agent/testutil/TestCertificateTemplateFactory.kt +++ b/android/app/src/test/java/com/fleetdm/agent/testutil/TestCertificateTemplateFactory.kt @@ -17,6 +17,7 @@ object TestCertificateTemplateFactory { certificateAuthorityName: String = "Test CA", createdAt: String = "2024-01-01T00:00:00Z", subjectName: String = "CN=Test,O=FleetDM", + subjectAlternativeName: String? = null, certificateAuthorityType: String = "SCEP", status: String = "active", scepChallenge: String = "test-challenge", @@ -30,6 +31,7 @@ object TestCertificateTemplateFactory { certificateAuthorityName = certificateAuthorityName, createdAt = createdAt, subjectName = subjectName, + subjectAlternativeName = subjectAlternativeName, certificateAuthorityType = certificateAuthorityType, status = status, scepChallenge = scepChallenge, diff --git a/android/changes/41472-android-cert-san-attributes b/android/changes/41472-android-cert-san-attributes new file mode 100644 index 0000000000..2e9100e76f --- /dev/null +++ b/android/changes/41472-android-cert-san-attributes @@ -0,0 +1 @@ +- Added subject alternative name (SAN) support to SCEP enrollment on Android. When a certificate template carries a `subject_alternative_name` value, the agent now includes a non-critical SAN extension on the PKCS#10 CSR, supporting `DNS`, `EMAIL`, `URI`, `IP`, and `UPN` (Microsoft otherName, OID 1.3.6.1.4.1.311.20.2.3) attribute types.