Skip to content

Commit bf7f131

Browse files
JesusMcCloudJesusMcCloud
authored
Fix/csr (#54)
* Fixed PKCS#10 attribute canonicalisation: programmatically created attribute sets are DER-sorted, while decoded `rawAttributes` and `rawValue` retain malformed wire order and duplicates for lenient parsing. * Added `LenientSet`, a serializable ASN.1 `SET OF` collection that accepts only Kotlin sets when constructed, preserves decoded wire contents when re-encoded, and exposes `toValidatedSet()` for duplicate validation. * Decoding an ASN.1 `SET OF` into Kotlin `Set<T>` now throws instead of silently discarding duplicate elements. * PKCS#10 semantic `attributes` and `value` getters now reject malformed decoded duplicates; empty attribute values, duplicate attribute OIDs, and empty extension requests are rejected during programmatic construction.
1 parent 7d288d2 commit bf7f131

13 files changed

Lines changed: 504 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
# Changelog
22

33
## NEXT
4+
* Fixed PKCS#10 attribute canonicalisation: programmatically created attribute sets are DER-sorted, while decoded
5+
`rawAttributes` and `rawValue` retain malformed wire order and duplicates for lenient parsing.
6+
* Added `LenientSet`, a serializable ASN.1 `SET OF` collection that accepts only Kotlin sets when constructed,
7+
preserves decoded wire contents when re-encoded, and exposes `toValidatedSet()` for duplicate validation.
8+
* Decoding an ASN.1 `SET OF` into Kotlin `Set<T>` now throws instead of silently discarding duplicate elements.
9+
* PKCS#10 semantic `attributes` and `value` getters now reject malformed decoded duplicates; empty attribute values,
10+
duplicate attribute OIDs, and empty extension requests are rejected during programmatic construction.
411
* Allow `Asn1` builder unary `+` for transparent wrappers around `Asn1Element`/`Asn1Encodable`, and for serializable
512
values when a `Der` instance is in context.
613
* Change the `X509AlgorithmIdentifier` constructor to take a single nullable `parameters` element.

‎crypto/src/commonMain/kotlin/at/asitplus/awesn1/crypto/pki/Pkcs10CertificationRequestInfo.kt‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33

44
package at.asitplus.awesn1.crypto.pki
55

6-
import at.asitplus.awesn1.TagClass
6+
import at.asitplus.awesn1.Asn1Exception
77
import at.asitplus.awesn1.crypto.SubjectPublicKeyInfo
8+
import at.asitplus.awesn1.serialization.LenientSet
89
import at.asitplus.awesn1.serialization.Asn1Tag
910
import kotlinx.serialization.Serializable
11+
import kotlin.experimental.ExperimentalObjCRefinement
12+
import kotlin.native.HiddenFromObjC
1013

1114
/**
1215
*
@@ -19,22 +22,61 @@ import kotlinx.serialization.Serializable
1922
* attributes [0] Attributes{{ CRIAttributes }}
2023
* }
2124
*
25+
* Attributes { ATTRIBUTE:IOSet } ::= SET OF Attribute{{ IOSet }}
26+
*
27+
* CRIAttributes ATTRIBUTE ::= {
28+
* ... -- add any locally defined attributes here -- }
29+
*
2230
* Attribute { ATTRIBUTE:IOSet } ::= SEQUENCE {
2331
* type ATTRIBUTE.&id({IOSet}),
2432
* values SET SIZE(1..MAX) OF ATTRIBUTE.&Type({IOSet}{@type})
2533
* }
26-
*
2734
* ```
35+
*
36+
* [rawAttributes] uses [LenientSet] to hold malformed data too. **DO NOT ASSUME ANY PARTICULAR ORDER OR CONCRETE COLLECTION TYPE!** Canonical sorting happens only on encode!
37+
* Hence, the order of attributes may differ post-encode.
38+
*
39+
* This class's [equals] and [hashCode] reflect this characteristic: Order of attributes is irrelevant for equality!
2840
*/
41+
@ConsistentCopyVisibility
2942
@Serializable
30-
data class Pkcs10CertificationRequestInfo(
43+
data class Pkcs10CertificationRequestInfo private constructor(
3144
val version: Version = Version.V1,
3245
val subjectName: X500Name,
3346
val publicKey: SubjectPublicKeyInfo,
3447
@Asn1Tag(tagNumber = 0u)
35-
val attributes: List<Pkcs10CsrAttribute> = emptyList(),
48+
val rawAttributes: LenientSet<Pkcs10CsrAttribute> = LenientSet(),
3649
) {
3750

51+
constructor(
52+
version: Version = Version.V1,
53+
subjectName: X500Name,
54+
publicKey: SubjectPublicKeyInfo,
55+
attributes: Set<Pkcs10CsrAttribute> = emptySet(),
56+
) : this(version, subjectName, publicKey, rawAttributes = LenientSet(attributes)) {
57+
require(attributes.distinctBy { it.oid }.size == attributes.size) {
58+
"Multiple CSR attributes with the same OID found"
59+
}
60+
}
61+
62+
63+
/**
64+
* Returns this CertificationRequestInfo's attributes **iff* they are distinct by OID.
65+
*
66+
* @throws Asn1Exception in case duplicate OIDs are found
67+
*/
68+
@OptIn(ExperimentalObjCRefinement::class)
69+
@Suppress("WRONG_ANNOTATION_TARGET_WITH_USE_SITE_TARGET")
70+
@get:Throws(Asn1Exception::class)
71+
@HiddenFromObjC
72+
@get:HiddenFromObjC
73+
val attributes: Set<Pkcs10CsrAttribute>
74+
get() = rawAttributes.toValidatedSet().also {
75+
if (it.distinctBy { attribute -> attribute.oid }.size != it.size)
76+
throw Asn1Exception("Multiple CSR attributes with the same OID found")
77+
}
78+
79+
3880
/**
3981
* Legal CSR versions. As per [RFC2986](https://www.rfc-editor.org/rfc/rfc2986.html#section-4), only V1 is defined.
4082
*

‎crypto/src/commonMain/kotlin/at/asitplus/awesn1/crypto/pki/Pkcs10CsrAttribute.kt‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,18 @@
44
package at.asitplus.awesn1.crypto.pki
55

66
import at.asitplus.awesn1.Asn1Element
7+
import at.asitplus.awesn1.Asn1Exception
78
import at.asitplus.awesn1.Identifiable
89
import at.asitplus.awesn1.ObjectIdentifier
910
import at.asitplus.awesn1.encoding.Asn1
11+
import at.asitplus.awesn1.encoding.unaryPlus
1012
import at.asitplus.awesn1.serialization.DER
11-
import at.asitplus.awesn1.serialization.encodeToTlv
13+
import at.asitplus.awesn1.serialization.Der
14+
import at.asitplus.awesn1.serialization.LenientSet
1215
import kotlinx.serialization.Serializable
16+
import kotlinx.serialization.SerializationException
17+
import kotlin.experimental.ExperimentalObjCRefinement
18+
import kotlin.native.HiddenFromObjC
1319

1420
/**
1521
*
@@ -21,21 +27,44 @@ import kotlinx.serialization.Serializable
2127
* }
2228
* ```
2329
*/
30+
@ConsistentCopyVisibility
2431
@Serializable
25-
data class Pkcs10CsrAttribute(
32+
data class Pkcs10CsrAttribute private constructor(
2633
override val oid: ObjectIdentifier,
27-
val value: Set<Asn1Element>,
34+
val rawValue: LenientSet<Asn1Element>,
2835
) : Identifiable {
29-
constructor(id: ObjectIdentifier, value: Asn1Element) : this(id, setOf(value))
36+
constructor(oid: ObjectIdentifier, value: Set<Asn1Element>) : this(oid, LenientSet(value)) {
37+
require(value.isNotEmpty()) { "At least one attribute value is required" }
38+
}
39+
40+
constructor(id: ObjectIdentifier, singleElement: Asn1Element) : this(id, setOf(singleElement))
41+
42+
/**
43+
* Returns this attribute's values iff decoded input was non-empty and did not contain duplicates.
44+
*
45+
* @throws Asn1Exception if the encoded values were malformed
46+
*/
47+
@OptIn(ExperimentalObjCRefinement::class)
48+
@Suppress("WRONG_ANNOTATION_TARGET_WITH_USE_SITE_TARGET")
49+
@get:Throws(Asn1Exception::class)
50+
@HiddenFromObjC
51+
@get:HiddenFromObjC
52+
val value: Set<Asn1Element> get() = rawValue.toValidatedSet().also {
53+
if (it.isEmpty()) throw Asn1Exception("At least one attribute value is required")
54+
}
3055

3156
companion object {
3257
val EXTENSION_REQUEST_OID = ObjectIdentifier("1.2.840.113549.1.9.14")
3358

34-
fun ExtensionRequest(extensions: List<X509CertificateExtension>): Pkcs10CsrAttribute {
59+
/**
60+
* Throws on illegal input
61+
*/
62+
@Throws(IllegalArgumentException::class, SerializationException::class, Asn1Exception::class)
63+
fun ExtensionRequest(extensions: List<X509CertificateExtension>, der: Der = DER): Pkcs10CsrAttribute {
3564
require(extensions.isNotEmpty()) { "At least one extension is required" }
3665
return Pkcs10CsrAttribute(
3766
EXTENSION_REQUEST_OID,
38-
Asn1.Sequence { extensions.forEach { +DER.encodeToTlv(it) } },
67+
singleElement = with(der) { Asn1.Sequence { extensions.forEach { +it } } }
3968
)
4069
}
4170
}

‎crypto/src/commonTest/kotlin/at/asitplus/awesn1/crypto/CryptoDerRoundTripTest.kt‎

Lines changed: 147 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ import at.asitplus.awesn1.*
44
import at.asitplus.awesn1.crypto.pki.*
55
import at.asitplus.awesn1.encoding.Asn1
66
import at.asitplus.awesn1.serialization.DER
7+
import at.asitplus.awesn1.serialization.decodeFromTlv
8+
import at.asitplus.awesn1.serialization.encodeToTlv
79
import at.asitplus.testballoon.matrix.CompactScope
810
import at.asitplus.testballoon.matrix.matrixSuite
11+
import io.kotest.assertions.throwables.shouldThrow
912
import io.kotest.matchers.shouldBe
1013
import kotlinx.serialization.decodeFromByteArray
1114
import kotlinx.serialization.encodeToByteArray
@@ -15,6 +18,121 @@ import kotlin.time.Instant
1518
import io.kotest.property.arbitrary.arbitrary as kotestArbitrary
1619

1720
val CryptoDerRoundTripTest by matrixSuite {
21+
"PKCS #10 request info equality and hash ignore attribute order" {
22+
val ordered = certificationRequestInfo(linkedSetOf(SMALLER_ATTRIBUTE, LARGER_ATTRIBUTE))
23+
val reversed = certificationRequestInfo(linkedSetOf(LARGER_ATTRIBUTE, SMALLER_ATTRIBUTE))
24+
25+
(ordered.rawAttributes == reversed.rawAttributes) shouldBe true
26+
ordered.rawAttributes.hashCode() shouldBe reversed.rawAttributes.hashCode()
27+
ordered shouldBe reversed
28+
ordered.hashCode() shouldBe reversed.hashCode()
29+
}
30+
31+
"programmatic PKCS #10 attributes are canonically sorted" {
32+
DER.decodeFromTlv<Pkcs10CertificationRequestInfo>(
33+
DER.encodeToTlv(
34+
certificationRequestInfo(
35+
linkedSetOf(
36+
LARGER_ATTRIBUTE,
37+
SMALLER_ATTRIBUTE
38+
)
39+
)
40+
)
41+
).rawAttributes.toList() shouldBe listOf(SMALLER_ATTRIBUTE, LARGER_ATTRIBUTE)
42+
}
43+
44+
"decoded PKCS #10 rawAttributes retain wire order" {
45+
val canonical = DER.encodeToTlv(
46+
certificationRequestInfo(linkedSetOf(SMALLER_ATTRIBUTE, LARGER_ATTRIBUTE))
47+
).asSequence()
48+
val attributes = canonical.children.last().asStructure()
49+
val nonCanonical = Asn1.Sequence {
50+
canonical.children.dropLast(1).forEach { +it }
51+
+Asn1CustomStructure(attributes.children.reversed(), 0uL, TagClass.CONTEXT_SPECIFIC)
52+
}
53+
54+
DER.decodeFromByteArray<Pkcs10CertificationRequestInfo>(nonCanonical.derEncoded).rawAttributes.toList() shouldBe
55+
listOf(LARGER_ATTRIBUTE, SMALLER_ATTRIBUTE)
56+
}
57+
58+
"malformed PKCS #10 duplicate attributes decode into rawAttributes" {
59+
malformedCertificationRequestInfo().rawAttributes.map { it.oid } shouldBe listOf(
60+
DUPLICATE_ATTRIBUTE_OID, DUPLICATE_ATTRIBUTE_OID
61+
)
62+
}
63+
64+
"programmatic PKCS #10 request info rejects duplicate attribute OIDs" {
65+
shouldThrow<IllegalArgumentException> {
66+
certificationRequestInfo(
67+
setOf(
68+
Pkcs10CsrAttribute(DUPLICATE_ATTRIBUTE_OID, Asn1.Int(0)),
69+
Pkcs10CsrAttribute(DUPLICATE_ATTRIBUTE_OID, Asn1.Int(1)),
70+
)
71+
)
72+
}
73+
}
74+
75+
"programmatic PKCS #10 attribute values are canonically sorted" {
76+
DER.decodeFromTlv<Pkcs10CsrAttribute>(
77+
DER.encodeToTlv(
78+
Pkcs10CsrAttribute(
79+
ObjectIdentifier("1.2.3"),
80+
linkedSetOf(LARGER_ATTRIBUTE_VALUE, SMALLER_ATTRIBUTE_VALUE),
81+
)
82+
)
83+
).rawValue.toList() shouldBe listOf(SMALLER_ATTRIBUTE_VALUE, LARGER_ATTRIBUTE_VALUE)
84+
}
85+
86+
"decoded PKCS #10 rawValue retains wire order" {
87+
val canonical = DER.encodeToTlv(
88+
Pkcs10CsrAttribute(
89+
ObjectIdentifier("1.2.3"),
90+
linkedSetOf(SMALLER_ATTRIBUTE_VALUE, LARGER_ATTRIBUTE_VALUE),
91+
)
92+
).asSequence()
93+
val values = canonical.children.last().asStructure()
94+
val nonCanonical = Asn1.Sequence {
95+
+canonical.children.first()
96+
+Asn1CustomStructure(values.children.reversed(), Asn1Element.Tag.SET.tagValue)
97+
}
98+
99+
DER.decodeFromByteArray<Pkcs10CsrAttribute>(nonCanonical.derEncoded).rawValue.toList() shouldBe
100+
listOf(LARGER_ATTRIBUTE_VALUE, SMALLER_ATTRIBUTE_VALUE)
101+
}
102+
103+
"malformed duplicate PKCS #10 values decode but the value getter rejects them" {
104+
val canonical = DER.encodeToTlv(
105+
Pkcs10CsrAttribute(ObjectIdentifier("1.2.3"), SMALLER_ATTRIBUTE_VALUE)
106+
).asSequence()
107+
val malformed = Asn1.Sequence {
108+
+canonical.children.first()
109+
+Asn1CustomStructure(
110+
listOf(SMALLER_ATTRIBUTE_VALUE, SMALLER_ATTRIBUTE_VALUE),
111+
Asn1Element.Tag.SET.tagValue,
112+
)
113+
}
114+
val decoded = DER.decodeFromByteArray<Pkcs10CsrAttribute>(malformed.derEncoded)
115+
116+
decoded.rawValue.toList() shouldBe listOf(SMALLER_ATTRIBUTE_VALUE, SMALLER_ATTRIBUTE_VALUE)
117+
shouldThrow<Asn1Exception> { decoded.value }
118+
}
119+
120+
"empty PKCS #10 attribute values only decode leniently" {
121+
val malformed = Asn1.Sequence {
122+
+ObjectIdentifier("1.2.3")
123+
+Asn1.Set { }
124+
}
125+
val decoded = DER.decodeFromByteArray<Pkcs10CsrAttribute>(malformed.derEncoded)
126+
127+
decoded.rawValue.isEmpty() shouldBe true
128+
shouldThrow<Asn1Exception> { decoded.value }
129+
shouldThrow<IllegalArgumentException> { Pkcs10CsrAttribute(ObjectIdentifier("1.2.3"), emptySet()) }
130+
}
131+
132+
"PKCS #10 attributes rejects duplicate OIDs" {
133+
shouldThrow<Asn1Exception> { malformedCertificationRequestInfo().attributes }
134+
}
135+
18136
"Property checks" - {
19137
compact("SignatureValue from raw bit string") - { checkRoundTrip(::randomRawBitStringSignatureValue) }
20138
compact("SignatureValue from raw bytes") - { checkRoundTrip(::randomBitStringSignatureValue) }
@@ -37,6 +155,34 @@ val CryptoDerRoundTripTest by matrixSuite {
37155
}
38156
}
39157

158+
private val SMALLER_ATTRIBUTE = Pkcs10CsrAttribute(ObjectIdentifier("1.2.3"), Asn1.Int(0))
159+
private val LARGER_ATTRIBUTE = Pkcs10CsrAttribute(ObjectIdentifier("1.2.4"), Asn1.Int(0))
160+
private val SMALLER_ATTRIBUTE_VALUE = Asn1.Int(0)
161+
private val LARGER_ATTRIBUTE_VALUE = Asn1.Int(1)
162+
private val DUPLICATE_ATTRIBUTE_OID = ObjectIdentifier("1.2.3")
163+
164+
private fun certificationRequestInfo(attributes: Set<Pkcs10CsrAttribute>) = Pkcs10CertificationRequestInfo(
165+
subjectName = X500Name(emptyList()),
166+
publicKey = SubjectPublicKeyInfo.ec(ObjectIdentifier("1.2.3"), byteArrayOf(4)),
167+
attributes = attributes,
168+
)
169+
170+
private fun malformedCertificationRequestInfo(): Pkcs10CertificationRequestInfo {
171+
val valid = DER.encodeToTlv(certificationRequestInfo(emptySet())).asSequence()
172+
val malformed = Asn1.Sequence {
173+
valid.children.dropLast(1).forEach { +it }
174+
+Asn1CustomStructure(
175+
listOf(
176+
DER.encodeToTlv(Pkcs10CsrAttribute(DUPLICATE_ATTRIBUTE_OID, Asn1.Int(0))),
177+
DER.encodeToTlv(Pkcs10CsrAttribute(DUPLICATE_ATTRIBUTE_OID, Asn1.Int(1))),
178+
),
179+
0uL,
180+
TagClass.CONTEXT_SPECIFIC,
181+
)
182+
}
183+
return DER.decodeFromByteArray(malformed.derEncoded)
184+
}
185+
40186
private inline fun <reified T> CompactScope.checkRoundTrip(noinline generator: (Random) -> T) {
41187
property("value", kotestArbitrary { rs -> generator(rs.random) }) test { value ->
42188
val encoded = DER.encodeToByteArray<T>(value)
@@ -181,7 +327,7 @@ private fun randomPrivateKeyInfo(random: Random): Pkcs8PrivateKeyInfo =
181327
private fun randomPkcs10CertificationRequestInfo(random: Random) = Pkcs10CertificationRequestInfo(
182328
subjectName = X500Name(List(random.nextInt(1, 3)) { randomRelativeDistinguishedName(random) }),
183329
publicKey = randomSubjectPublicKeyInfo(random),
184-
attributes = List(random.nextInt(0, 3)) { randomAttribute(random) },
330+
attributes = (List(random.nextInt(0, 3)) { randomAttribute(random) }).toSet(),
185331
)
186332

187333
private fun randomPkcs10CertificationRequest(random: Random) = Pkcs10CertificationRequest(

‎crypto/src/commonTest/kotlin/at/asitplus/awesn1/crypto/LegacyRegression.kt‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import at.asitplus.awesn1.crypto.pki.X509CertificateExtension
3535
import at.asitplus.awesn1.serialization.ExplicitlyTagged
3636
import at.asitplus.awesn1.runWrappingAs
3737
import at.asitplus.awesn1.serialization.DER
38+
import at.asitplus.awesn1.serialization.decodeFromTlv
3839
import kotlinx.serialization.SerializationException
3940
import kotlinx.serialization.decodeFromByteArray
4041

@@ -148,13 +149,7 @@ private fun LegacyAttributeTypeAndValue.toCurrent() =
148149
private fun LegacyRelativeDistinguishedName.toCurrent() =
149150
X500RelativeDistinguishedName(attrsAndValues.map { it.toCurrent() }.toSet())
150151

151-
private fun LegacyPkcs10CertificationRequestInfo.toCurrent() =
152-
Pkcs10CertificationRequestInfo(
153-
version = Pkcs10CertificationRequestInfo.Version.V1,
154-
subjectName = X500Name(subjectName.map { it.toCurrent() }),
155-
publicKey = publicKey.toCurrent(),
156-
attributes = attributes.map { it.toCurrent() },
157-
)
152+
private fun LegacyPkcs10CertificationRequestInfo.toCurrent() =DER.decodeFromTlv<Pkcs10CertificationRequestInfo>(encodeToTlv())
158153

159154
private fun LegacyPkcs10CertificationRequest.toCurrent() =
160155
Pkcs10CertificationRequest(

0 commit comments

Comments
 (0)