Enhancement: GeneralName support by StjepanovicSrdjan · Pull Request #353 · a-sit-plus/signum

StjepanovicSrdjan · 2025-09-11T10:20:25Z

Refactored AlternativeNames for SAN/IAN extraction
Removed detailed parsing of individual name types; now delegates decoding to GeneralName and subclasses

JesusMcCloud

Even though these are "just" the data classes, they do contain logic that check if an how constraints are fulfilled. These need comprehensive tests. I'm sure there stuff out there and you can probably integrate some from your large cert-validation PR into here

...pensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/X500Name.kt

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

...spensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/DNSName.kt

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/EDIPartyName.kt

...ble/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/IPAddressName.kt

...spensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/UriName.kt

...sable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/GeneralName.kt

JesusMcCloud

the validation logic need further offline discussion.

JesusMcCloud · 2025-09-26T08:16:25Z

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

                return@decodeRethrowing when (oid) {
                    CommonName.OID -> str.fold(onSuccess = { CommonName(it) }, onFailure = { CommonName(asn1String) })
                    Country.OID -> str.fold(onSuccess = { Country(it) }, onFailure = { Country(asn1String) })
+                    EmailAddress.OID -> str.fold(onSuccess = { EmailAddress(it) }, onFailure = { EmailAddress(asn1String) })


don't all of these enforce validation, since their default constructors are called?

You're right, fixed this legacy code in bd53929

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

...spensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/DNSName.kt

...sable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/GeneralName.kt

...spensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/DNSName.kt

...ble/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/IPAddressName.kt

...ensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/OtherName.kt

...spensable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/UriName.kt

...e/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/X400AddressName.kt

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

JesusMcCloud · 2025-10-07T07:43:50Z

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

+        private fun splitFirstUnescaped(input: String, delimiter: Char): List<String> {
+            val sb = StringBuilder()
+            var escaped = false
+            for ((i, c) in input.withIndex()) {
+                when {
+                    escaped -> {
+                        sb.append('\\').append(c) // preserve the escape
+                        escaped = false
+                    }
+                    c == '\\' -> escaped = true
+                    c == delimiter -> return listOf(sb.toString(), input.substring(i + 1))
+                    else -> sb.append(c)
+                }
+            }
+            return listOf(sb.toString())
+        }


Can't we rely on some pre-existing functionality for that? I know serialization has it (think: escaping/unescaping string is virtually any test-based format) but I would expect this to be buried somewhere inside kotlin's stdlib, or any of the dependencies we already use here. When in doublt use Android studio, as it gives you free Tokens for Gemini-powered AI-assistance

I did refactor the code to use regex, but still didn't find something pre-existing. It is now much cleaner, but not sure if it's satisfactory yet. ae61ba7

what about this? https://github.com/Kotlin/kotlinx.serialization/blob/71b5d5ee99f953b60756c4aaa2ac8e12c3fae605/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonStreams.kt#L19

JesusMcCloud · 2025-10-07T07:44:05Z

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

+        private fun splitRespectingEscapeAndQuotes(input: String, delimiter: Char): List<String> {
+            val parts = mutableListOf<String>()
+            val sb = StringBuilder()
+            var escaped = false
+            var inQuotes = false
+
+            input.forEach { c ->
+                when {
+                    escaped -> {
+                        sb.append('\\').append(c) // preserve escape
+                        escaped = false
+                    }
+                    c == '\\' -> escaped = true
+                    c == '"' -> {
+                        sb.append(c)
+                        inQuotes = !inQuotes
+                    }
+                    c == delimiter && !inQuotes -> {
+                        parts.add(sb.toString())
+                        sb.clear()
+                    }
+                    else -> sb.append(c)
+                }
+            }
+
+            parts.add(sb.toString().trim())
+            return parts
+        }
+


...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/EDIPartyName.kt

...sable/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/GeneralName.kt

JesusMcCloud

I really think i caught the last loose ends in this cycle.

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

JesusMcCloud · 2025-10-14T13:46:15Z

...mTest/kotlin/at/asitplus/signum/indispensable/pki/generalNames/GeneralNamesConstrainsTest.kt

+import at.asitplus.test.FreeSpec
+import io.kotest.matchers.shouldBe
+
+class GeneralNamesConstrainsTest : FreeSpec ({


Can we test all those against JCA / BouncyCastle in addition to your tests?

Tests are based on the BouncyCastle tests

then please add a comment to the source of the test

JesusMcCloud · 2025-10-14T13:46:35Z

.../src/jvmTest/kotlin/at/asitplus/signum/indispensable/pki/generalNames/X500NameParsingTest.kt

+import at.asitplus.test.FreeSpec
+import io.kotest.matchers.shouldBe
+
+class X500NameParsingTest : FreeSpec ({


can we also test those against bouncy castle / JCA?

Tests are based on the BouncyCastle tests

same, please link to the original BC tests in a comment

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/generalNames/EDIPartyName.kt

JesusMcCloud

nitpicks and one open point for an offline elaboration

JesusMcCloud · 2025-10-16T13:17:58Z

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

-data class RelativeDistinguishedName(val attrsAndValues: List<AttributeTypeAndValue>) : Asn1Encodable<Asn1Set> {
+class RelativeDistinguishedName private constructor(
+    val attrsAndValues: List<AttributeTypeAndValue>,
+    performValidation: Boolean = false,


I'd omit the default value, to prevent even the possibility of accidentally calling the wrong c'tor

JesusMcCloud · 2025-10-16T13:20:03Z

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

+        constructor(str: Asn1String) : this(Asn1Primitive(str.tag, str.value.encodeToByteArray())) {
+            if (isValid == false) throw Asn1Exception("Invalid Organization!")
+        }


seeing this pattern everywhere: can't we pull that up as a protected c'tor in the base class?

JesusMcCloud · 2025-10-16T13:38:19Z

...able/src/commonMain/kotlin/at/asitplus/signum/indispensable/pki/RelativeDistinguishedName.kt

+        private fun splitFirstUnescaped(input: String, delimiter: Char): List<String> {
+            val sb = StringBuilder()
+            var escaped = false
+            for ((i, c) in input.withIndex()) {
+                when {
+                    escaped -> {
+                        sb.append('\\').append(c) // preserve the escape
+                        escaped = false
+                    }
+                    c == '\\' -> escaped = true
+                    c == delimiter -> return listOf(sb.toString(), input.substring(i + 1))
+                    else -> sb.append(c)
+                }
+            }
+            return listOf(sb.toString())
+        }


what about this? https://github.com/Kotlin/kotlinx.serialization/blob/71b5d5ee99f953b60756c4aaa2ac8e12c3fae605/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonStreams.kt#L19

JesusMcCloud · 2025-10-16T13:39:32Z

...mTest/kotlin/at/asitplus/signum/indispensable/pki/generalNames/GeneralNamesConstrainsTest.kt

+import at.asitplus.test.FreeSpec
+import io.kotest.matchers.shouldBe
+
+class GeneralNamesConstrainsTest : FreeSpec ({


then please add a comment to the source of the test

JesusMcCloud · 2025-10-16T13:39:50Z

.../src/jvmTest/kotlin/at/asitplus/signum/indispensable/pki/generalNames/X500NameParsingTest.kt

+import at.asitplus.test.FreeSpec
+import io.kotest.matchers.shouldBe
+
+class X500NameParsingTest : FreeSpec ({


same, please link to the original BC tests in a comment

… types and tests

…ssuer # Conflicts: # supreme/src/commonTest/kotlin/at/asitplus/signum/supreme/sign/EphemeralSignerCommonTests.kt

# Conflicts: # CHANGELOG.md # Conflicts: # CHANGELOG.md # Conflicts: # CHANGELOG.md

…e, minor fixes in parsing from string logic

…rsing

…validation is not implemented

…idation for generalNames without one

…f attrTypeAndValues

…ing(), add tests for parsing functions

* Refactor AlternativeNames to use GeneralName * add X500Name and refactor X509Cert to use this class as subject and issuer --------- Co-authored-by: Bernd Prünster <bernd.pruenster@a-sit.at>

StjepanovicSrdjan force-pushed the enhancement/generalName branch from eb899c0 to fc73548 Compare September 11, 2025 10:30

StjepanovicSrdjan requested a review from JesusMcCloud September 11, 2025 11:15

StjepanovicSrdjan force-pushed the enhancement/generalName branch from ad53e4f to 04b54be Compare September 15, 2025 10:26

JesusMcCloud requested changes Sep 15, 2025

View reviewed changes

StjepanovicSrdjan mentioned this pull request Sep 18, 2025

Parsing X509 IpAddressName a-sit-plus/cidre#4

Merged

StjepanovicSrdjan requested a review from JesusMcCloud September 24, 2025 10:08

JesusMcCloud requested changes Sep 26, 2025

View reviewed changes

StjepanovicSrdjan force-pushed the enhancement/generalName branch 2 times, most recently from 82b093c to 09eecc5 Compare October 1, 2025 08:32

StjepanovicSrdjan requested a review from JesusMcCloud October 2, 2025 09:31

StjepanovicSrdjan force-pushed the enhancement/generalName branch from d676787 to b96e512 Compare October 2, 2025 09:33

JesusMcCloud requested changes Oct 7, 2025

View reviewed changes

StjepanovicSrdjan force-pushed the enhancement/generalName branch from b96e512 to ae61ba7 Compare October 8, 2025 09:05

StjepanovicSrdjan requested a review from JesusMcCloud October 8, 2025 10:09

JesusMcCloud requested changes Oct 14, 2025

View reviewed changes

StjepanovicSrdjan requested a review from JesusMcCloud October 15, 2025 10:58

JesusMcCloud requested changes Oct 16, 2025

View reviewed changes

StjepanovicSrdjan requested a review from JesusMcCloud October 20, 2025 08:54

JesusMcCloud force-pushed the development branch from 3dcddf0 to 82e036e Compare October 22, 2025 22:33

JesusMcCloud changed the base branch from development to next October 30, 2025 08:41

StjepanovicSrdjan added 10 commits October 30, 2025 09:45

Refactor AlternativeNames to use GeneralName; add missing GeneralName…

7347aa6

… types and tests

add X500Name and refactor X509Cert to use this class as subject and i…

e9aedbf

…ssuer # Conflicts: # supreme/src/commonTest/kotlin/at/asitplus/signum/supreme/sign/EphemeralSignerCommonTests.kt

resolve bugs raised by cherry-pick

d57d8c6

exclude validation in parsing DNSName

0c4657a

refactor AttributeTypeAndValue to be open with out Other subclass

aba32a3

add cidre dependency

785943e

refacotr IPAddressName to use CIDRE

96f2427

refactor and changelog update

420ef13

# Conflicts: # CHANGELOG.md # Conflicts: # CHANGELOG.md # Conflicts: # CHANGELOG.md

polishing

65c1b68

update cidre version

31e79dc

StjepanovicSrdjan and others added 27 commits October 30, 2025 09:54

add generalName constrains tests and minor bug fixes

35f7878

refactor IpAddressName

5820bd1

add IpAddress tests and polishing

fa92f97

improve equals in attrTypeAndValue to compare by rfc2253 string

715e2a3

polishing

d763e5a

add parsing X500Name from string and tests

0896deb

add attrType property that stores type string in AttributeTypeAndValu…

db9ab06

…e, minor fixes in parsing from string logic

polishing rfc2253 x500name parsing

991ba96

refactor IpAddressName to store rawValue when invalid/lenient asn1 pa…

7f61292

…rsing

update URIName to store rawValue for invalid UriNames

e34748e

add validation in X500Name based on AttributeTypeAndValue validation

4bd2f08

make constrains throw if generalname is invalid, but not if valid or …

65ab2cb

…validation is not implemented

integrate new cidre features

2b4b9c8

add validatedCopy method in GeneralName interface for introducing val…

c497001

…idation for generalNames without one

remove performValidation property

f712ee5

polish custom validation logic for classes without one

efca877

refactor RelativeDistinguishedName string parsing

ba4f621

polishing reviewed stuff

82466af

add tests for custom logic in unvalidated generalNames

09d37e4

add isValid pattern in RDN, prevent creating RDN with invalid order o…

f59f225

…f attrTypeAndValues

apply minor PR suggestions

8a1a4e6

remove regex from splitRespectingEscapeAndQuotes and X500Name.fromStr…

616042f

…ing(), add tests for parsing functions

polishing

44ecd29

add experimental api annotation for PKI validation stuff

8da08cc

add equals and hashcode tests for general names

2222099

polishing experimental annotation

b180a46

testballoon migration

7db7e92

JesusMcCloud force-pushed the enhancement/generalName branch from a3c4079 to 7db7e92 Compare October 30, 2025 09:19

JesusMcCloud merged commit 35625fa into a-sit-plus:next Oct 30, 2025
6 checks passed

Conversation

StjepanovicSrdjan commented Sep 11, 2025

Uh oh!

JesusMcCloud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JesusMcCloud left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JesusMcCloud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JesusMcCloud left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!